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@xxxxxxxxxxxxxxx>
> Cc: Jean Delvare <jdelvare@xxxxxxxx>
> Cc: linux-edac@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> 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,
--
Jean Delvare
SUSE L3 Support
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
