RE: [PATCH 6/7] EDAC/{skx_common,i10nm}: Refactor show_retry_rd_err_log()

[Zhuo, Qiuxu]

Hi Tony,

> From: Luck, Tony <tony.luck@xxxxxxxxx>
> [...]
> On Thu, Apr 17, 2025 at 11:07:23PM +0800, Qiuxu Zhuo wrote:
> > +	/* CORRERRCNT register parts. */
> > +	int cecnt_num;
> > +	u32 cecnt_offsets[NUM_CECNT_REG];
> > +	u8 cecnt_widths[NUM_CECNT_REG];
> 
> YOu have added this "cecnt_widths" field and code to print in different
> formats fo value == 4 ("%.8llx") and not 4 ("%.16llx"). But no CPU (including
> Granite Rapids added by next patch) has any values other than "4".
> 
> Is there a mistake in the struct reg_rrl defintions where you intended to
> have some "8" values somewhere?

No, there isn't a mistake 😊.

Currently, all CPUs EDAC supported by {skx,i10nm}_edac indeed just have the 
value "4" for "cecnt_widths". 

> Or is this just for symmetry with the ".widths" you have for the RRL register
> (which do have varying widths).

The upcoming CPU Diamond Rapids [1] will have the value "8" for "cecnt_widths".
This is why I made the "cecnt_widths" field here, intended to easily cover this new 
CPU EDAC support in the future. 

Do you suggest not using the "cecnt_widths" field for now (since it currently only has 
the value 4 and the code appears somewhat redundant) until we add the EDAC support 
for Diamond Rapids in the future? Or we can keep the "cecnt_widths" field?  
Either option is OK to me 😊.
 
[1] https://github.com/torvalds/linux/blob/master/arch/x86/include/asm/intel-family.h#L200

Thanks!
-Qiuxu