Messages in this thread Patch in this message | | | Date | Tue, 13 Aug 2019 16:59:47 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v2 06/24] EDAC: Kill EDAC_DIMM_PTR() macro |
| |
> Subject: Re: [PATCH v2 06/24] EDAC: Kill EDAC_DIMM_PTR() macro
s/Kill/Replace ... with/
On Mon, Jun 24, 2019 at 03:09:06PM +0000, Robert Richter wrote: > Get rid of this macro and instead use the new function > edac_get_dimm(). Also introduce the edac_get_dimm_by_index() function > for later use.
Some blurb about *why* you're doing this won't hurt here.
> diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c > index bd3be25d0d3f..8050f9577fe6 100644 > --- a/drivers/edac/ghes_edac.c > +++ b/drivers/edac/ghes_edac.c > @@ -97,9 +97,8 @@ static void ghes_edac_dmidecode(const struct dmi_header *dh, void *arg) > > if (dh->type == DMI_ENTRY_MEM_DEVICE) { > struct memdev_dmi_entry *entry = (struct memdev_dmi_entry *)dh; > - struct dimm_info *dimm = EDAC_DIMM_PTR(mci->layers, mci->dimms, > - mci->n_layers, > - dimm_fill->count, 0, 0); > + struct dimm_info *dimm = edac_get_dimm(mci, dimm_fill->count, > + 0, 0);
You can let it stick out.
> u16 rdr_mask = BIT(7) | BIT(13); > > if (entry->size == 0xffff) {
...
> diff --git a/drivers/edac/i5400_edac.c b/drivers/edac/i5400_edac.c > index 6f8bcdb9256a..a50a8707337b 100644 > --- a/drivers/edac/i5400_edac.c > +++ b/drivers/edac/i5400_edac.c > @@ -1196,8 +1196,8 @@ static int i5400_init_dimms(struct mem_ctl_info *mci) > if (!MTR_DIMMS_PRESENT(mtr)) > continue; > > - dimm = EDAC_DIMM_PTR(mci->layers, mci->dimms, mci->n_layers, > - channel / 2, channel % 2, slot); > + dimm = edac_get_dimm(mci, channel / 2, channel % 2, > + slot);
Also, let it stick out.
> @@ -669,4 +639,56 @@ struct mem_ctl_info { > bool fake_inject_ue; > u16 fake_inject_count; > }; > + > +/** > + * edac_get_dimm_by_index - Get DIMM info from a memory controller > + * given by an index > + * > + * @mci: a struct mem_ctl_info > + * @index: index in the memory controller's DIMM array > + * > + * Returns a struct dimm_info*.
or NULL on failure.
> + */ > +static inline struct dimm_info * > +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]; > +} > + > +/** > + * edac_get_dimm - Get DIMM info from a memory controller given by > + * [layer0,layer1,layer2] position > + * > + * @mci: a struct mem_ctl_info > + * @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 "dimms[layer0]";
macro? Copy-paste I guess :)
Below too. > + * > + * For 2 layers, this macro is similar to allocate a bi-dimensional array > + * and to return "dimms[layer0][layer1]"; > + * > + * For 3 layers, this macro is similar to allocate a tri-dimensional array > + * and to return "dimms[layer0][layer1][layer2]"; > + */ > +static inline struct dimm_info * > +edac_get_dimm(struct mem_ctl_info *mci, int layer0, int layer1, int layer2) > +{ > + int index = layer0; > + > + if (index >= 0 && mci->n_layers > 1)
Can layer0 be negative here to warrant that check?
> + index = index * mci->layers[1].size + layer1; > + if (index >= 0 && mci->n_layers > 2)
Same here.
> + index = index * mci->layers[2].size + layer2; > + > + return edac_get_dimm_by_index(mci, index); > +} > + > #endif > --
Thx.
-- Regards/Gruss, Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
| |