| Message ID | 1457399146-4578-7-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Matt, On Mon, 7 Mar 2016 17:05:44 -0800, Matt Roper wrote: > A couple of the EDAC drivers have a nice memdev_dmi_entry structure for > decoding DMI memory device entries. Move the structure definition to > dmi.h so that it can be shared between those drivers and also other > parts of the kernel; the i915 graphics driver is going to need to use > this structure soon as well. Looks good in principle, a few suggestions inline below: > Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > Cc: Jean Delvare <jdelvare@suse.com> > Cc: linux-edac@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/edac/ghes_edac.c | 26 -------------------------- > drivers/edac/i7core_edac.c | 25 ------------------------- > include/linux/dmi.h | 25 +++++++++++++++++++++++++ > 3 files changed, 25 insertions(+), 51 deletions(-) > (...) > diff --git a/include/linux/dmi.h b/include/linux/dmi.h > index 5e9c74c..1eb2136 100644 > --- a/include/linux/dmi.h > +++ b/include/linux/dmi.h > @@ -78,6 +78,31 @@ struct dmi_header { > u16 handle; > } __packed; > > +struct memdev_dmi_entry { As a member of the API of the dmi subsystem, this symbol should start with dmi_. What about dmi_entry_memdev? > + u8 type; > + u8 length; > + u16 handle; > + u16 phys_mem_array_handle; > + u16 mem_err_info_handle; > + u16 total_width; > + u16 data_width; > + u16 size; > + u8 form; > + u8 device_set; > + u8 device_locator; > + u8 bank_locator; > + u8 memory_type; > + u16 type_detail; > + u16 speed; > + u8 manufacturer; > + u8 serial_number; > + u8 asset_tag; > + u8 part_number; > + u8 attributes; > + u32 extended_size; > + u16 conf_mem_clk_speed; > +} __attribute__((__packed__)); dmi_header is declared with __packed instead. I would appreciate consistency within this header file, so please use __packed too. > + > struct dmi_device { > struct list_head list; > int type; Thanks,
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c index e3fa439..429be79 100644 --- a/drivers/edac/ghes_edac.c +++ b/drivers/edac/ghes_edac.c @@ -35,32 +35,6 @@ static DEFINE_MUTEX(ghes_edac_lock); static int ghes_edac_mc_num; -/* Memory Device - Type 17 of SMBIOS spec */ -struct memdev_dmi_entry { - u8 type; - u8 length; - u16 handle; - u16 phys_mem_array_handle; - u16 mem_err_info_handle; - u16 total_width; - u16 data_width; - u16 size; - u8 form_factor; - u8 device_set; - u8 device_locator; - u8 bank_locator; - u8 memory_type; - u16 type_detail; - u16 speed; - u8 manufacturer; - u8 serial_number; - u8 asset_tag; - u8 part_number; - u8 attributes; - u32 extended_size; - u16 conf_mem_clk_speed; -} __attribute__((__packed__)); - struct ghes_edac_dimm_fill { struct mem_ctl_info *mci; unsigned count; diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c index 01087a3..b11d3be 100644 --- a/drivers/edac/i7core_edac.c +++ b/drivers/edac/i7core_edac.c @@ -1906,31 +1906,6 @@ static struct notifier_block i7_mce_dec = { .notifier_call = i7core_mce_check_error, }; -struct memdev_dmi_entry { - u8 type; - u8 length; - u16 handle; - u16 phys_mem_array_handle; - u16 mem_err_info_handle; - u16 total_width; - u16 data_width; - u16 size; - u8 form; - u8 device_set; - u8 device_locator; - u8 bank_locator; - u8 memory_type; - u16 type_detail; - u16 speed; - u8 manufacturer; - u8 serial_number; - u8 asset_tag; - u8 part_number; - u8 attributes; - u32 extended_size; - u16 conf_mem_clk_speed; -} __attribute__((__packed__)); - /* * Decode the DRAM Clock Frequency, be paranoid, make sure that all diff --git a/include/linux/dmi.h b/include/linux/dmi.h index 5e9c74c..1eb2136 100644 --- a/include/linux/dmi.h +++ b/include/linux/dmi.h @@ -78,6 +78,31 @@ struct dmi_header { u16 handle; } __packed; +struct memdev_dmi_entry { + u8 type; + u8 length; + u16 handle; + u16 phys_mem_array_handle; + u16 mem_err_info_handle; + u16 total_width; + u16 data_width; + u16 size; + u8 form; + u8 device_set; + u8 device_locator; + u8 bank_locator; + u8 memory_type; + u16 type_detail; + u16 speed; + u8 manufacturer; + u8 serial_number; + u8 asset_tag; + u8 part_number; + u8 attributes; + u32 extended_size; + u16 conf_mem_clk_speed; +} __attribute__((__packed__)); + struct dmi_device { struct list_head list; int type;
A couple of the EDAC drivers have a nice memdev_dmi_entry structure for decoding DMI memory device entries. Move the structure definition to dmi.h so that it can be shared between those drivers and also other parts of the kernel; the i915 graphics driver is going to need to use this structure soon as well. Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com> Cc: Jean Delvare <jdelvare@suse.com> Cc: linux-edac@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/edac/ghes_edac.c | 26 -------------------------- drivers/edac/i7core_edac.c | 25 ------------------------- include/linux/dmi.h | 25 +++++++++++++++++++++++++ 3 files changed, 25 insertions(+), 51 deletions(-)