* Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string
2017-05-17 10:25 ` [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string Mika Westerberg
@ 2017-05-17 10:31 ` Andy Shevchenko
2017-05-23 8:05 ` Linus Walleij
` (2 subsequent siblings)
3 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2017-05-17 10:31 UTC (permalink / raw)
To: Mika Westerberg
Cc: Linus Walleij, Jean Delvare, Heikki Krogerus, Andy Shevchenko,
Dmitry Torokhov, Wei Yongjun, bbaude, mildred-bug.kernel,
barnacs, lvuksta, linux-kernel, linux-gpio, Kelly French
On Wed, May 17, 2017 at 1:25 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> Sometimes it is more convenient to be able to match a whole family of
> products, like in case of bunch of Chromebooks based on Intel_Strago to
> apply a driver quirk instead of quirking each machine one-by-one.
>
> This adds support for DMI_PRODUCT_FAMILY identification string and also
> exports it to the userspace through sysfs attribute just like the
> existing ones.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> drivers/firmware/dmi-id.c | 2 ++
> drivers/firmware/dmi_scan.c | 1 +
> include/linux/mod_devicetable.h | 1 +
> 3 files changed, 4 insertions(+)
>
> diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
> index 44c01390d035..dc269cb288c2 100644
> --- a/drivers/firmware/dmi-id.c
> +++ b/drivers/firmware/dmi-id.c
> @@ -47,6 +47,7 @@ DEFINE_DMI_ATTR_WITH_SHOW(product_name, 0444, DMI_PRODUCT_NAME);
> DEFINE_DMI_ATTR_WITH_SHOW(product_version, 0444, DMI_PRODUCT_VERSION);
> DEFINE_DMI_ATTR_WITH_SHOW(product_serial, 0400, DMI_PRODUCT_SERIAL);
> DEFINE_DMI_ATTR_WITH_SHOW(product_uuid, 0400, DMI_PRODUCT_UUID);
> +DEFINE_DMI_ATTR_WITH_SHOW(product_family, 0400, DMI_PRODUCT_FAMILY);
> DEFINE_DMI_ATTR_WITH_SHOW(board_vendor, 0444, DMI_BOARD_VENDOR);
> DEFINE_DMI_ATTR_WITH_SHOW(board_name, 0444, DMI_BOARD_NAME);
> DEFINE_DMI_ATTR_WITH_SHOW(board_version, 0444, DMI_BOARD_VERSION);
> @@ -191,6 +192,7 @@ static void __init dmi_id_init_attr_table(void)
> ADD_DMI_ATTR(product_version, DMI_PRODUCT_VERSION);
> ADD_DMI_ATTR(product_serial, DMI_PRODUCT_SERIAL);
> ADD_DMI_ATTR(product_uuid, DMI_PRODUCT_UUID);
> + ADD_DMI_ATTR(product_family, DMI_PRODUCT_FAMILY);
> ADD_DMI_ATTR(board_vendor, DMI_BOARD_VENDOR);
> ADD_DMI_ATTR(board_name, DMI_BOARD_NAME);
> ADD_DMI_ATTR(board_version, DMI_BOARD_VERSION);
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 54be60ead08f..93f7acdaac7a 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -430,6 +430,7 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
> dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6);
> dmi_save_ident(dm, DMI_PRODUCT_SERIAL, 7);
> dmi_save_uuid(dm, DMI_PRODUCT_UUID, 8);
> + dmi_save_ident(dm, DMI_PRODUCT_FAMILY, 26);
> break;
> case 2: /* Base Board Information */
> dmi_save_ident(dm, DMI_BOARD_VENDOR, 4);
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 566fda587fcf..3f74ef2281e8 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -467,6 +467,7 @@ enum dmi_field {
> DMI_PRODUCT_VERSION,
> DMI_PRODUCT_SERIAL,
> DMI_PRODUCT_UUID,
> + DMI_PRODUCT_FAMILY,
> DMI_BOARD_VENDOR,
> DMI_BOARD_NAME,
> DMI_BOARD_VERSION,
> --
> 2.11.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string
2017-05-17 10:25 ` [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string Mika Westerberg
2017-05-17 10:31 ` Andy Shevchenko
@ 2017-05-23 8:05 ` Linus Walleij
2017-06-01 9:29 ` Jean Delvare
2017-06-01 12:42 ` Jean Delvare
3 siblings, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2017-05-23 8:05 UTC (permalink / raw)
To: Mika Westerberg
Cc: Jean Delvare, Heikki Krogerus, Andy Shevchenko, Dmitry Torokhov,
Wei Yongjun, bbaude, mildred-bug.kernel, barnacs, lvuksta,
linux-kernel, linux-gpio, Kelly French
On Wed, May 17, 2017 at 12:25 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> Sometimes it is more convenient to be able to match a whole family of
> products, like in case of bunch of Chromebooks based on Intel_Strago to
> apply a driver quirk instead of quirking each machine one-by-one.
>
> This adds support for DMI_PRODUCT_FAMILY identification string and also
> exports it to the userspace through sysfs attribute just like the
> existing ones.
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
OK patch applied for fixes with Andy's Review tag.
Didn't see this v2 first, sorry.
Andy's nod make me feel confident to merge this into firmware.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string
2017-05-17 10:25 ` [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string Mika Westerberg
2017-05-17 10:31 ` Andy Shevchenko
2017-05-23 8:05 ` Linus Walleij
@ 2017-06-01 9:29 ` Jean Delvare
2017-06-01 10:09 ` Mika Westerberg
2017-06-01 12:42 ` Jean Delvare
3 siblings, 1 reply; 17+ messages in thread
From: Jean Delvare @ 2017-06-01 9:29 UTC (permalink / raw)
To: Mika Westerberg
Cc: Linus Walleij, Heikki Krogerus, Andy Shevchenko, Dmitry Torokhov,
Wei Yongjun, bbaude, mildred-bug.kernel, barnacs, lvuksta,
linux-kernel, linux-gpio, Kelly French
Hi all,
Sorry for the late reply.
On Wed, 17 May 2017 13:25:12 +0300, Mika Westerberg wrote:
> Sometimes it is more convenient to be able to match a whole family of
> products, like in case of bunch of Chromebooks based on Intel_Strago to
> apply a driver quirk instead of quirking each machine one-by-one.
>
> This adds support for DMI_PRODUCT_FAMILY identification string and also
> exports it to the userspace through sysfs attribute just like the
> existing ones.
dmidecode currently provides no direct access to this string. Do you
think it should?
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> drivers/firmware/dmi-id.c | 2 ++
> drivers/firmware/dmi_scan.c | 1 +
> include/linux/mod_devicetable.h | 1 +
> 3 files changed, 4 insertions(+)
>
> diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
> index 44c01390d035..dc269cb288c2 100644
> --- a/drivers/firmware/dmi-id.c
> +++ b/drivers/firmware/dmi-id.c
> (...)
> @@ -191,6 +192,7 @@ static void __init dmi_id_init_attr_table(void)
> ADD_DMI_ATTR(product_version, DMI_PRODUCT_VERSION);
> ADD_DMI_ATTR(product_serial, DMI_PRODUCT_SERIAL);
> ADD_DMI_ATTR(product_uuid, DMI_PRODUCT_UUID);
> + ADD_DMI_ATTR(product_family, DMI_PRODUCT_FAMILY);
Alignment, please!
> ADD_DMI_ATTR(board_vendor, DMI_BOARD_VENDOR);
> ADD_DMI_ATTR(board_name, DMI_BOARD_NAME);
> ADD_DMI_ATTR(board_version, DMI_BOARD_VERSION);
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 54be60ead08f..93f7acdaac7a 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -430,6 +430,7 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
> dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6);
> dmi_save_ident(dm, DMI_PRODUCT_SERIAL, 7);
> dmi_save_uuid(dm, DMI_PRODUCT_UUID, 8);
> + dmi_save_ident(dm, DMI_PRODUCT_FAMILY, 26);
This field only exists since SMBIOS 2.4. For older implementations, you
are accessing a random location of the DMI table. Most likely you'll
hit a character in one of the strings associated with the system
information structure. In turn this character will be interpreted as a
DMI string number. With some luck, number will be >= 32, so you'll get
a non-existent string and dmi_string will return "". But you could hit
a string terminator (0) and return the 1st string of the structure
instead (most likely the system manufacturer.)
Note that the problem is not specific to this field, it is just more
likely to break because all other fields are defined by SMBIOS 2.0, or
for the product UUID, SMBIOS 2.1. The fact that all dmi_save_*
functions blindly assume that the structure is long enough to contain
all the fields they want to save is problematic. This should be fixed
separately.
> break;
> case 2: /* Base Board Information */
> dmi_save_ident(dm, DMI_BOARD_VENDOR, 4);
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 566fda587fcf..3f74ef2281e8 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -467,6 +467,7 @@ enum dmi_field {
> DMI_PRODUCT_VERSION,
> DMI_PRODUCT_SERIAL,
> DMI_PRODUCT_UUID,
> + DMI_PRODUCT_FAMILY,
> DMI_BOARD_VENDOR,
> DMI_BOARD_NAME,
> DMI_BOARD_VERSION,
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string
2017-06-01 9:29 ` Jean Delvare
@ 2017-06-01 10:09 ` Mika Westerberg
2017-06-01 10:54 ` Jean Delvare
0 siblings, 1 reply; 17+ messages in thread
From: Mika Westerberg @ 2017-06-01 10:09 UTC (permalink / raw)
To: Jean Delvare
Cc: Linus Walleij, Heikki Krogerus, Andy Shevchenko, Dmitry Torokhov,
Wei Yongjun, bbaude, mildred-bug.kernel, barnacs, lvuksta,
linux-kernel, linux-gpio, Kelly French
On Thu, Jun 01, 2017 at 11:29:26AM +0200, Jean Delvare wrote:
> Hi all,
>
> Sorry for the late reply.
>
> On Wed, 17 May 2017 13:25:12 +0300, Mika Westerberg wrote:
> > Sometimes it is more convenient to be able to match a whole family of
> > products, like in case of bunch of Chromebooks based on Intel_Strago to
> > apply a driver quirk instead of quirking each machine one-by-one.
> >
> > This adds support for DMI_PRODUCT_FAMILY identification string and also
> > exports it to the userspace through sysfs attribute just like the
> > existing ones.
>
> dmidecode currently provides no direct access to this string. Do you
> think it should?
Yeah, why not. I always just run "dmidecode" without any arguments and
that field is printed nicely among others :)
> > Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> > drivers/firmware/dmi-id.c | 2 ++
> > drivers/firmware/dmi_scan.c | 1 +
> > include/linux/mod_devicetable.h | 1 +
> > 3 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
> > index 44c01390d035..dc269cb288c2 100644
> > --- a/drivers/firmware/dmi-id.c
> > +++ b/drivers/firmware/dmi-id.c
> > (...)
> > @@ -191,6 +192,7 @@ static void __init dmi_id_init_attr_table(void)
> > ADD_DMI_ATTR(product_version, DMI_PRODUCT_VERSION);
> > ADD_DMI_ATTR(product_serial, DMI_PRODUCT_SERIAL);
> > ADD_DMI_ATTR(product_uuid, DMI_PRODUCT_UUID);
> > + ADD_DMI_ATTR(product_family, DMI_PRODUCT_FAMILY);
>
> Alignment, please!
The patch is already applied and I suppose merged in v4.12-rc3+. Should
I send a fixup patch to fix this?
>
> > ADD_DMI_ATTR(board_vendor, DMI_BOARD_VENDOR);
> > ADD_DMI_ATTR(board_name, DMI_BOARD_NAME);
> > ADD_DMI_ATTR(board_version, DMI_BOARD_VERSION);
> > diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> > index 54be60ead08f..93f7acdaac7a 100644
> > --- a/drivers/firmware/dmi_scan.c
> > +++ b/drivers/firmware/dmi_scan.c
> > @@ -430,6 +430,7 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
> > dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6);
> > dmi_save_ident(dm, DMI_PRODUCT_SERIAL, 7);
> > dmi_save_uuid(dm, DMI_PRODUCT_UUID, 8);
> > + dmi_save_ident(dm, DMI_PRODUCT_FAMILY, 26);
>
> This field only exists since SMBIOS 2.4. For older implementations, you
> are accessing a random location of the DMI table. Most likely you'll
> hit a character in one of the strings associated with the system
> information structure. In turn this character will be interpreted as a
> DMI string number. With some luck, number will be >= 32, so you'll get
> a non-existent string and dmi_string will return "". But you could hit
> a string terminator (0) and return the 1st string of the structure
> instead (most likely the system manufacturer.)
>
> Note that the problem is not specific to this field, it is just more
> likely to break because all other fields are defined by SMBIOS 2.0, or
> for the product UUID, SMBIOS 2.1. The fact that all dmi_save_*
> functions blindly assume that the structure is long enough to contain
> all the fields they want to save is problematic. This should be fixed
> separately.
I see. Since you are more familiar with the DMI code, do you have time
to do that or should I try?
Thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string
2017-06-01 10:09 ` Mika Westerberg
@ 2017-06-01 10:54 ` Jean Delvare
2017-06-01 11:05 ` Mika Westerberg
0 siblings, 1 reply; 17+ messages in thread
From: Jean Delvare @ 2017-06-01 10:54 UTC (permalink / raw)
To: Mika Westerberg
Cc: Linus Walleij, Heikki Krogerus, Andy Shevchenko, Dmitry Torokhov,
Wei Yongjun, bbaude, mildred-bug.kernel, barnacs, lvuksta,
linux-kernel, linux-gpio, Kelly French
On Thu, 1 Jun 2017 13:09:40 +0300, Mika Westerberg wrote:
> On Thu, Jun 01, 2017 at 11:29:26AM +0200, Jean Delvare wrote:
> > Hi all,
> >
> > Sorry for the late reply.
> >
> > On Wed, 17 May 2017 13:25:12 +0300, Mika Westerberg wrote:
> > > Sometimes it is more convenient to be able to match a whole family of
> > > products, like in case of bunch of Chromebooks based on Intel_Strago to
> > > apply a driver quirk instead of quirking each machine one-by-one.
> > >
> > > This adds support for DMI_PRODUCT_FAMILY identification string and also
> > > exports it to the userspace through sysfs attribute just like the
> > > existing ones.
> >
> > dmidecode currently provides no direct access to this string. Do you
> > think it should?
>
> Yeah, why not. I always just run "dmidecode" without any arguments and
> that field is printed nicely among others :)
Correct. But I know many people out there use option -s for various
purposes.
> > > (...)
> > > diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
> > > index 44c01390d035..dc269cb288c2 100644
> > > --- a/drivers/firmware/dmi-id.c
> > > +++ b/drivers/firmware/dmi-id.c
> > > (...)
> > > @@ -191,6 +192,7 @@ static void __init dmi_id_init_attr_table(void)
> > > ADD_DMI_ATTR(product_version, DMI_PRODUCT_VERSION);
> > > ADD_DMI_ATTR(product_serial, DMI_PRODUCT_SERIAL);
> > > ADD_DMI_ATTR(product_uuid, DMI_PRODUCT_UUID);
> > > + ADD_DMI_ATTR(product_family, DMI_PRODUCT_FAMILY);
> >
> > Alignment, please!
>
> The patch is already applied and I suppose merged in v4.12-rc3+. Should
> I send a fixup patch to fix this?
I will do it, no worry.
> > > ADD_DMI_ATTR(board_vendor, DMI_BOARD_VENDOR);
> > > ADD_DMI_ATTR(board_name, DMI_BOARD_NAME);
> > > ADD_DMI_ATTR(board_version, DMI_BOARD_VERSION);
> > > diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> > > index 54be60ead08f..93f7acdaac7a 100644
> > > --- a/drivers/firmware/dmi_scan.c
> > > +++ b/drivers/firmware/dmi_scan.c
> > > @@ -430,6 +430,7 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
> > > dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6);
> > > dmi_save_ident(dm, DMI_PRODUCT_SERIAL, 7);
> > > dmi_save_uuid(dm, DMI_PRODUCT_UUID, 8);
> > > + dmi_save_ident(dm, DMI_PRODUCT_FAMILY, 26);
> >
> > This field only exists since SMBIOS 2.4. For older implementations, you
> > are accessing a random location of the DMI table. Most likely you'll
> > hit a character in one of the strings associated with the system
> > information structure. In turn this character will be interpreted as a
> > DMI string number. With some luck, number will be >= 32, so you'll get
> > a non-existent string and dmi_string will return "". But you could hit
> > a string terminator (0) and return the 1st string of the structure
> > instead (most likely the system manufacturer.)
> >
> > Note that the problem is not specific to this field, it is just more
> > likely to break because all other fields are defined by SMBIOS 2.0, or
> > for the product UUID, SMBIOS 2.1. The fact that all dmi_save_*
> > functions blindly assume that the structure is long enough to contain
> > all the fields they want to save is problematic. This should be fixed
> > separately.
>
> I see. Since you are more familiar with the DMI code, do you have time
> to do that or should I try?
I'm already working on it.
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string
2017-06-01 10:54 ` Jean Delvare
@ 2017-06-01 11:05 ` Mika Westerberg
0 siblings, 0 replies; 17+ messages in thread
From: Mika Westerberg @ 2017-06-01 11:05 UTC (permalink / raw)
To: Jean Delvare
Cc: Linus Walleij, Heikki Krogerus, Andy Shevchenko, Dmitry Torokhov,
Wei Yongjun, bbaude, mildred-bug.kernel, barnacs, lvuksta,
linux-kernel, linux-gpio, Kelly French
On Thu, Jun 01, 2017 at 12:54:51PM +0200, Jean Delvare wrote:
> On Thu, 1 Jun 2017 13:09:40 +0300, Mika Westerberg wrote:
> > On Thu, Jun 01, 2017 at 11:29:26AM +0200, Jean Delvare wrote:
> > > Hi all,
> > >
> > > Sorry for the late reply.
> > >
> > > On Wed, 17 May 2017 13:25:12 +0300, Mika Westerberg wrote:
> > > > Sometimes it is more convenient to be able to match a whole family of
> > > > products, like in case of bunch of Chromebooks based on Intel_Strago to
> > > > apply a driver quirk instead of quirking each machine one-by-one.
> > > >
> > > > This adds support for DMI_PRODUCT_FAMILY identification string and also
> > > > exports it to the userspace through sysfs attribute just like the
> > > > existing ones.
> > >
> > > dmidecode currently provides no direct access to this string. Do you
> > > think it should?
> >
> > Yeah, why not. I always just run "dmidecode" without any arguments and
> > that field is printed nicely among others :)
>
> Correct. But I know many people out there use option -s for various
> purposes.
OK, I can send a patch adding it there.
> > > > (...)
> > > > diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
> > > > index 44c01390d035..dc269cb288c2 100644
> > > > --- a/drivers/firmware/dmi-id.c
> > > > +++ b/drivers/firmware/dmi-id.c
> > > > (...)
> > > > @@ -191,6 +192,7 @@ static void __init dmi_id_init_attr_table(void)
> > > > ADD_DMI_ATTR(product_version, DMI_PRODUCT_VERSION);
> > > > ADD_DMI_ATTR(product_serial, DMI_PRODUCT_SERIAL);
> > > > ADD_DMI_ATTR(product_uuid, DMI_PRODUCT_UUID);
> > > > + ADD_DMI_ATTR(product_family, DMI_PRODUCT_FAMILY);
> > >
> > > Alignment, please!
> >
> > The patch is already applied and I suppose merged in v4.12-rc3+. Should
> > I send a fixup patch to fix this?
>
> I will do it, no worry.
>
> > > > ADD_DMI_ATTR(board_vendor, DMI_BOARD_VENDOR);
> > > > ADD_DMI_ATTR(board_name, DMI_BOARD_NAME);
> > > > ADD_DMI_ATTR(board_version, DMI_BOARD_VERSION);
> > > > diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> > > > index 54be60ead08f..93f7acdaac7a 100644
> > > > --- a/drivers/firmware/dmi_scan.c
> > > > +++ b/drivers/firmware/dmi_scan.c
> > > > @@ -430,6 +430,7 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
> > > > dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6);
> > > > dmi_save_ident(dm, DMI_PRODUCT_SERIAL, 7);
> > > > dmi_save_uuid(dm, DMI_PRODUCT_UUID, 8);
> > > > + dmi_save_ident(dm, DMI_PRODUCT_FAMILY, 26);
> > >
> > > This field only exists since SMBIOS 2.4. For older implementations, you
> > > are accessing a random location of the DMI table. Most likely you'll
> > > hit a character in one of the strings associated with the system
> > > information structure. In turn this character will be interpreted as a
> > > DMI string number. With some luck, number will be >= 32, so you'll get
> > > a non-existent string and dmi_string will return "". But you could hit
> > > a string terminator (0) and return the 1st string of the structure
> > > instead (most likely the system manufacturer.)
> > >
> > > Note that the problem is not specific to this field, it is just more
> > > likely to break because all other fields are defined by SMBIOS 2.0, or
> > > for the product UUID, SMBIOS 2.1. The fact that all dmi_save_*
> > > functions blindly assume that the structure is long enough to contain
> > > all the fields they want to save is problematic. This should be fixed
> > > separately.
> >
> > I see. Since you are more familiar with the DMI code, do you have time
> > to do that or should I try?
>
> I'm already working on it.
Thanks!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string
2017-05-17 10:25 ` [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string Mika Westerberg
` (2 preceding siblings ...)
2017-06-01 9:29 ` Jean Delvare
@ 2017-06-01 12:42 ` Jean Delvare
3 siblings, 0 replies; 17+ messages in thread
From: Jean Delvare @ 2017-06-01 12:42 UTC (permalink / raw)
To: Mika Westerberg
Cc: Linus Walleij, Heikki Krogerus, Andy Shevchenko, Dmitry Torokhov,
Wei Yongjun, bbaude, mildred-bug.kernel, barnacs, lvuksta,
linux-kernel, linux-gpio, Kelly French
I did not notice before, but now that I'm testing...
On Wed, 17 May 2017 13:25:12 +0300, Mika Westerberg wrote:
> --- a/drivers/firmware/dmi-id.c
> +++ b/drivers/firmware/dmi-id.c
> @@ -47,6 +47,7 @@ DEFINE_DMI_ATTR_WITH_SHOW(product_name, 0444, DMI_PRODUCT_NAME);
> DEFINE_DMI_ATTR_WITH_SHOW(product_version, 0444, DMI_PRODUCT_VERSION);
> DEFINE_DMI_ATTR_WITH_SHOW(product_serial, 0400, DMI_PRODUCT_SERIAL);
> DEFINE_DMI_ATTR_WITH_SHOW(product_uuid, 0400, DMI_PRODUCT_UUID);
> +DEFINE_DMI_ATTR_WITH_SHOW(product_family, 0400, DMI_PRODUCT_FAMILY);
> DEFINE_DMI_ATTR_WITH_SHOW(board_vendor, 0444, DMI_BOARD_VENDOR);
> DEFINE_DMI_ATTR_WITH_SHOW(board_name, 0444, DMI_BOARD_NAME);
> DEFINE_DMI_ATTR_WITH_SHOW(board_version, 0444, DMI_BOARD_VERSION);
I see no reason to hide this field from users. Permissions 0444 would
seem more appropriate. I'll include that change in my patch if there
are no objections.
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 17+ messages in thread