On Mon, Jun 24, 2019 at 03:09:09PM +0000, Robert Richter wrote:
> We do not need to calculate the offset in the mc's dimm array, let's
> just store the index in struct dimm_info and we can get rid of this
> macro.
>
> Signed-off-by: Robert Richter <rrich...@marvell.com>
> ---
> drivers/edac/edac_mc.c | 13 ++++--------
> drivers/edac/edac_mc_sysfs.c | 20 ++++--------------
> include/linux/edac.h | 41 ------------------------------------
> 3 files changed, 8 insertions(+), 66 deletions(-)
I like this cleanup a lot. Good!
> diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
> index c959e8b1643c..c44bc83e8502 100644
> --- a/drivers/edac/edac_mc.c
> +++ b/drivers/edac/edac_mc.c
> @@ -318,7 +318,7 @@ struct mem_ctl_info *edac_mc_alloc(unsigned mc_num,
> unsigned size, tot_dimms = 1, count = 1;
> unsigned tot_csrows = 1, tot_channels = 1, tot_errcount = 0;
> void *pvt, *p, *ptr = NULL;
> - int i, j, row, chn, n, len, off;
> + int idx, i, j, row, chn, n, len;
> bool per_rank = false;
>
> BUG_ON(n_layers > EDAC_MAX_LAYERS || n_layers == 0);
> @@ -426,20 +426,15 @@ struct mem_ctl_info *edac_mc_alloc(unsigned 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;
> - }
Btw, right around here there's a comment:
/*
* Allocate and fill the dimm structs
*/
and since you're cleaning up all this, can you please add another patch
which takes all that code under the comment and see if you can carve it
out into a separate helper edac_alloc_dimms() or so. Because that
edac_mc_alloc() function is huuuge.
Btw, the code under
/*
* Alocate and fill the csrow/channels structs
*/
begs to be a separate function too :-)
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
