Skip to content

Commit

Permalink
EDAC: Remove EDAC_DIMM_OFF() macro
Browse files Browse the repository at this point in the history
The EDAC_DIMM_OFF() macro takes 5 arguments to get the DIMM's index.
Simplify this by storing the index in struct dimm_info to avoid its
calculation and remove the EDAC_DIMM_OFF() macro. The index can be
directly used then.

Another advantage is that edac_mc_alloc() could be used even if the
exact size of the layers is unknown. Only the number of DIMMs would be
needed.

Rename iterator variable to idx, while at it. The name is more handy,
esp. when searching for it in the code.

Signed-off-by: Robert Richter <rrichter@marvell.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>
Cc: James Morse <james.morse@arm.com>
Cc: Tony Luck <tony.luck@intel.com>
Link: https://lkml.kernel.org/r/20191106093239.25517-3-rrichter@marvell.com
  • Loading branch information
Robert Richter authored and Borislav Petkov committed Nov 9, 2019
1 parent bc9ad9e commit 977b1ce
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 72 deletions.
28 changes: 13 additions & 15 deletions drivers/edac/edac_mc.c
Original file line number Diff line number Diff line change
Expand Up @@ -314,25 +314,27 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
struct dimm_info *dimm;
u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
unsigned int pos[EDAC_MAX_LAYERS];
unsigned int size, tot_dimms = 1, count = 1;
unsigned int idx, size, tot_dimms = 1, count = 1;
unsigned int tot_csrows = 1, tot_channels = 1, tot_errcount = 0;
void *pvt, *p, *ptr = NULL;
int i, j, row, chn, n, len, off;
int i, j, row, chn, n, len;
bool per_rank = false;

BUG_ON(n_layers > EDAC_MAX_LAYERS || n_layers == 0);

/*
* Calculate the total amount of dimms and csrows/cschannels while
* in the old API emulation mode
*/
for (i = 0; i < n_layers; i++) {
tot_dimms *= layers[i].size;
if (layers[i].is_virt_csrow)
tot_csrows *= layers[i].size;
for (idx = 0; idx < n_layers; idx++) {
tot_dimms *= layers[idx].size;

if (layers[idx].is_virt_csrow)
tot_csrows *= layers[idx].size;
else
tot_channels *= layers[i].size;
tot_channels *= layers[idx].size;

if (layers[i].type == EDAC_MC_LAYER_CHIP_SELECT)
if (layers[idx].type == EDAC_MC_LAYER_CHIP_SELECT)
per_rank = true;
}

Expand Down Expand Up @@ -425,19 +427,15 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
memset(&pos, 0, sizeof(pos));
row = 0;
chn = 0;
for (i = 0; i < tot_dimms; i++) {
for (idx = 0; idx < tot_dimms; idx++) {
chan = mci->csrows[row]->channels[chn];
off = EDAC_DIMM_OFF(layer, n_layers, pos[0], pos[1], pos[2]);
if (off < 0 || off >= tot_dimms) {
edac_mc_printk(mci, KERN_ERR, "EDAC core bug: EDAC_DIMM_OFF is trying to do an illegal data access\n");
goto error;
}

dimm = kzalloc(sizeof(**mci->dimms), GFP_KERNEL);
if (!dimm)
goto error;
mci->dimms[off] = dimm;
mci->dimms[idx] = dimm;
dimm->mci = mci;
dimm->idx = idx;

/*
* Copy DIMM location and initialize it.
Expand Down
20 changes: 4 additions & 16 deletions drivers/edac/edac_mc_sysfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -557,14 +557,8 @@ static ssize_t dimmdev_ce_count_show(struct device *dev,
{
struct dimm_info *dimm = to_dimm(dev);
u32 count;
int off;

off = EDAC_DIMM_OFF(dimm->mci->layers,
dimm->mci->n_layers,
dimm->location[0],
dimm->location[1],
dimm->location[2]);
count = dimm->mci->ce_per_layer[dimm->mci->n_layers-1][off];

count = dimm->mci->ce_per_layer[dimm->mci->n_layers-1][dimm->idx];
return sprintf(data, "%u\n", count);
}

Expand All @@ -574,14 +568,8 @@ static ssize_t dimmdev_ue_count_show(struct device *dev,
{
struct dimm_info *dimm = to_dimm(dev);
u32 count;
int off;

off = EDAC_DIMM_OFF(dimm->mci->layers,
dimm->mci->n_layers,
dimm->location[0],
dimm->location[1],
dimm->location[2]);
count = dimm->mci->ue_per_layer[dimm->mci->n_layers-1][off];

count = dimm->mci->ue_per_layer[dimm->mci->n_layers-1][dimm->idx];
return sprintf(data, "%u\n", count);
}

Expand Down
45 changes: 4 additions & 41 deletions include/linux/edac.h
Original file line number Diff line number Diff line change
Expand Up @@ -362,47 +362,6 @@ struct edac_mc_layer {
*/
#define EDAC_MAX_LAYERS 3

/**
* EDAC_DIMM_OFF - Macro responsible to get a pointer offset inside a pointer
* array for the element given by [layer0,layer1,layer2]
* position
*
* @layers: a struct edac_mc_layer array, describing how many elements
* were allocated for each layer
* @nlayers: Number of layers at the @layers array
* @layer0: layer0 position
* @layer1: layer1 position. Unused if n_layers < 2
* @layer2: layer2 position. Unused if n_layers < 3
*
* For 1 layer, this macro returns "var[layer0] - var";
*
* For 2 layers, this macro is similar to allocate a bi-dimensional array
* and to return "var[layer0][layer1] - var";
*
* For 3 layers, this macro is similar to allocate a tri-dimensional array
* and to return "var[layer0][layer1][layer2] - var".
*
* A loop could be used here to make it more generic, but, as we only have
* 3 layers, this is a little faster.
*
* By design, layers can never be 0 or more than 3. If that ever happens,
* a NULL is returned, causing an OOPS during the memory allocation routine,
* with would point to the developer that he's doing something wrong.
*/
#define EDAC_DIMM_OFF(layers, nlayers, layer0, layer1, layer2) ({ \
int __i; \
if ((nlayers) == 1) \
__i = layer0; \
else if ((nlayers) == 2) \
__i = (layer1) + ((layers[1]).size * (layer0)); \
else if ((nlayers) == 3) \
__i = (layer2) + ((layers[2]).size * ((layer1) + \
((layers[1]).size * (layer0)))); \
else \
__i = -EINVAL; \
__i; \
})

struct dimm_info {
struct device dev;

Expand All @@ -412,6 +371,7 @@ struct dimm_info {
unsigned int location[EDAC_MAX_LAYERS];

struct mem_ctl_info *mci; /* the parent */
unsigned int idx; /* index within the parent dimm array */

u32 grain; /* granularity of reported error in bytes */
enum dev_type dtype; /* memory device type */
Expand Down Expand Up @@ -654,6 +614,9 @@ edac_get_dimm_by_index(struct mem_ctl_info *mci, int index)
if (index < 0 || index >= mci->tot_dimms)
return NULL;

if (WARN_ON_ONCE(mci->dimms[index]->idx != index))
return NULL;

return mci->dimms[index];
}

Expand Down

0 comments on commit 977b1ce

Please sign in to comment.