| Message ID | 862104c26398781e4937c3fca3994cbe75d820ff.1512070562.git.tony.luck@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 30, 2017 at 12:40:41PM -0800, Tony Luck wrote: > There are now non-volatile versions of DIMMs. Add a new entry to > "enum mem_type" and update places that use it with new strings. > > Signed-off-by: Tony Luck <tony.luck@intel.com> > --- > drivers/edac/edac_mc.c | 1 + > drivers/edac/edac_mc_sysfs.c | 3 ++- > include/linux/edac.h | 3 +++ > 3 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c > index 480072139b7a..8178e74decbf 100644 > --- a/drivers/edac/edac_mc.c > +++ b/drivers/edac/edac_mc.c > @@ -215,6 +215,7 @@ const char * const edac_mem_types[] = { > [MEM_LRDDR3] = "Load-Reduced DDR3 RAM", > [MEM_DDR4] = "Unbuffered DDR4 RAM", > [MEM_RDDR4] = "Registered DDR4 RAM", > + [MEM_NVDIMM] = "Non-volatile RAM", > }; > EXPORT_SYMBOL_GPL(edac_mem_types); > > diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c > index e4fcfa84fbd3..53cbb3518efc 100644 > --- a/drivers/edac/edac_mc_sysfs.c > +++ b/drivers/edac/edac_mc_sysfs.c > @@ -110,7 +110,8 @@ static const char * const mem_types[] = { > [MEM_DDR3] = "Unbuffered-DDR3", > [MEM_RDDR3] = "Registered-DDR3", > [MEM_DDR4] = "Unbuffered-DDR4", > - [MEM_RDDR4] = "Registered-DDR4" > + [MEM_RDDR4] = "Registered-DDR4", > + [MEM_NVDIMM] = "Non-volatile RAM", > }; WTF, there are *two* string arrays with memory types?! How did that happen? Can you please remove that above in a prepatch and switch to edac_mem_types? Or should we switch to this mem_types array instead and kill edac_mem_types since former is visible in sysfs? :-( Grrr.
On Mon, Dec 04, 2017 at 11:37:11PM +0100, Borislav Petkov wrote: > On Thu, Nov 30, 2017 at 12:40:41PM -0800, Tony Luck wrote: > WTF, there are *two* string arrays with memory types?! How did that > happen? > > Can you please remove that above in a prepatch and switch to > edac_mem_types? Or should we switch to this mem_types array instead and > kill edac_mem_types since former is visible in sysfs? > > :-( Grrr. Oops. I didn't stare hard enough as I walked the "grep" output to just blindly add a new case everywhere for the new type. Need to resolve the differences: 1) MEM_LRDDR3 only appears in one of the tables 2) edac_mem_types[] is exported from edac_mc.c and used by two drivers 3) mem_types[] is static in edac_mc_sysfs.c, but values are visible to user via sysfs 4) Strings are different between the two :-( Since the drivers that use edac_mem_types[] only do so in edac_dbg() output, I think it's safe to declare those strings as the non-canon ones. So the plan: copy the strings from mem_types[] to edac_mem_types[] and then make edac_mc_sysfs.c pick up the exported list from edac_mc.c instead of having its own static version. Both files have the same build scope: edac_core-y := edac_mc.o edac_device.o edac_mc_sysfs.o so there shouldn't be any weird dependency problems. -Tony
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c index 480072139b7a..8178e74decbf 100644 --- a/drivers/edac/edac_mc.c +++ b/drivers/edac/edac_mc.c @@ -215,6 +215,7 @@ const char * const edac_mem_types[] = { [MEM_LRDDR3] = "Load-Reduced DDR3 RAM", [MEM_DDR4] = "Unbuffered DDR4 RAM", [MEM_RDDR4] = "Registered DDR4 RAM", + [MEM_NVDIMM] = "Non-volatile RAM", }; EXPORT_SYMBOL_GPL(edac_mem_types); diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index e4fcfa84fbd3..53cbb3518efc 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -110,7 +110,8 @@ static const char * const mem_types[] = { [MEM_DDR3] = "Unbuffered-DDR3", [MEM_RDDR3] = "Registered-DDR3", [MEM_DDR4] = "Unbuffered-DDR4", - [MEM_RDDR4] = "Registered-DDR4" + [MEM_RDDR4] = "Registered-DDR4", + [MEM_NVDIMM] = "Non-volatile RAM", }; static const char * const dev_types[] = { diff --git a/include/linux/edac.h b/include/linux/edac.h index cd75c173fd00..bffb97828ed6 100644 --- a/include/linux/edac.h +++ b/include/linux/edac.h @@ -186,6 +186,7 @@ static inline char *mc_event_error_type(const unsigned int err_type) * @MEM_RDDR4: Registered DDR4 RAM * This is a variant of the DDR4 memories. * @MEM_LRDDR4: Load-Reduced DDR4 memory. + * @MEM_NVDIMM: Non-volatile RAM */ enum mem_type { MEM_EMPTY = 0, @@ -209,6 +210,7 @@ enum mem_type { MEM_DDR4, MEM_RDDR4, MEM_LRDDR4, + MEM_NVDIMM, }; #define MEM_FLAG_EMPTY BIT(MEM_EMPTY) @@ -231,6 +233,7 @@ enum mem_type { #define MEM_FLAG_DDR4 BIT(MEM_DDR4) #define MEM_FLAG_RDDR4 BIT(MEM_RDDR4) #define MEM_FLAG_LRDDR4 BIT(MEM_LRDDR4) +#define MEM_FLAG_NVDIMM BIT(MEM_NVDIMM) /** * enum edac-type - Error Detection and Correction capabilities and mode
There are now non-volatile versions of DIMMs. Add a new entry to "enum mem_type" and update places that use it with new strings. Signed-off-by: Tony Luck <tony.luck@intel.com> --- drivers/edac/edac_mc.c | 1 + drivers/edac/edac_mc_sysfs.c | 3 ++- include/linux/edac.h | 3 +++ 3 files changed, 6 insertions(+), 1 deletion(-)