[PATCH v2 0/3] pinctrl: cherryview: Extend the DMI quirk to Intel_Strago systems

[PATCH v2 0/3] pinctrl: cherryview: Extend the DMI quirk to Intel_Strago systems

[Mika Westerberg]

Hi,

It turned out that there are bunch of other Chromebooks that need the DMI
quirk as well. Most of these are based on Intel_Strago reference design so
we can identify them by using DMI_PRODUCT_FAMILY introduced in this series.
The systems without the family string we add them separately.

Related bugzilla entry:

  https://bugzilla.kernel.org/show_bug.cgi?id=194945

I'm including fix from Yongjun as well because I did not find it in
v4.12-rc1 possibly because it just was forgotten.

Changes from v1:

  * Drop BIOS date, we can't use it because there are many different
    Celes Chromebooks out there but their BIOS release date varies (and
    they miss the prodyct family information as well).

  * Name the machines consistently.

Mika Westerberg (2):
  firmware: dmi: Add DMI_PRODUCT_FAMILY identification string
  pinctrl: cherryview: Extend the Chromebook DMI quirk to Intel_Strago systems

Wei Yongjun (1):
  pinctrl: cherryview: Add terminate entry for dmi_system_id tables

 drivers/firmware/dmi-id.c                  |  2 ++
 drivers/firmware/dmi_scan.c                |  1 +
 drivers/pinctrl/intel/pinctrl-cherryview.c | 24 +++++++++++++++++++-----
 include/linux/mod_devicetable.h            |  1 +
 4 files changed, 23 insertions(+), 5 deletions(-)

-- 
2.11.0

[PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string

[Mika Westerberg]

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>
---
 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

[PATCH v2 2/3] pinctrl: cherryview: Add terminate entry for dmi_system_id tables

[Mika Westerberg]

From: Wei Yongjun <weiyongjun1@huawei.com>

Make sure dmi_system_id tables are NULL terminated.

Fixes: 703650278372 ("pinctrl: cherryview: Add a quirk to make Acer Chromebook keyboard work again")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 2debba62fac9..e35d0fe4c737 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1547,7 +1547,8 @@ static const struct dmi_system_id chv_no_valid_mask[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Edgar"),
 			DMI_MATCH(DMI_BIOS_DATE, "05/21/2016"),
 		},
-	}
+	},
+	{}
 };
 
 static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
-- 
2.11.0

[PATCH v2 3/3] pinctrl: cherryview: Extend the Chromebook DMI quirk to Intel_Strago systems

[Mika Westerberg]

It turns out there are quite many Chromebooks out there that have the
same keyboard issue than Acer Chromebook. All of them are based on
Intel_Strago reference and report their DMI_PRODUCT_FAMILY as
"Intel_Strago" (Samsung Chromebook 3 and Cyan Chromebooks are exceptions
for which we add separate entries).

Instead of adding each machine to the quirk table, we use
DMI_PRODUCT_FAMILY of "Intel_Strago" that hopefully covers most of the
machines out there currently.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=194945
Suggested: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index e35d0fe4c737..20f1b4493994 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1539,13 +1539,26 @@ static void chv_gpio_irq_handler(struct irq_desc *desc)
  * is not listed below.
  */
 static const struct dmi_system_id chv_no_valid_mask[] = {
+	/* See https://bugzilla.kernel.org/show_bug.cgi?id=194945 */
 	{
-		/* See https://bugzilla.kernel.org/show_bug.cgi?id=194945 */
-		.ident = "Acer Chromebook (CYAN)",
+		.ident = "Intel_Strago based Chromebooks (All models)",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Edgar"),
-			DMI_MATCH(DMI_BIOS_DATE, "05/21/2016"),
+			DMI_MATCH(DMI_PRODUCT_FAMILY, "Intel_Strago"),
+		},
+	},
+	{
+		.ident = "Acer Chromebook R11 (Cyan)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Cyan"),
+		},
+	},
+	{
+		.ident = "Samsung Chromebook 3 (Celes)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Celes"),
 		},
 	},
 	{}
-- 
2.11.0

Re: [PATCH v2 3/3] pinctrl: cherryview: Extend the Chromebook DMI quirk to Intel_Strago systems

[Andy Shevchenko]

On Wed, May 17, 2017 at 1:25 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> It turns out there are quite many Chromebooks out there that have the
> same keyboard issue than Acer Chromebook. All of them are based on
> Intel_Strago reference and report their DMI_PRODUCT_FAMILY as
> "Intel_Strago" (Samsung Chromebook 3 and Cyan Chromebooks are exceptions
> for which we add separate entries).
>
> Instead of adding each machine to the quirk table, we use
> DMI_PRODUCT_FAMILY of "Intel_Strago" that hopefully covers most of the
> machines out there currently.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=194945
> Suggested: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index e35d0fe4c737..20f1b4493994 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -1539,13 +1539,26 @@ static void chv_gpio_irq_handler(struct irq_desc *desc)
>   * is not listed below.
>   */
>  static const struct dmi_system_id chv_no_valid_mask[] = {
> +       /* See https://bugzilla.kernel.org/show_bug.cgi?id=194945 */
>         {
> -               /* See https://bugzilla.kernel.org/show_bug.cgi?id=194945 */
> -               .ident = "Acer Chromebook (CYAN)",
> +               .ident = "Intel_Strago based Chromebooks (All models)",
>                 .matches = {
>                         DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
> -                       DMI_MATCH(DMI_PRODUCT_NAME, "Edgar"),
> -                       DMI_MATCH(DMI_BIOS_DATE, "05/21/2016"),
> +                       DMI_MATCH(DMI_PRODUCT_FAMILY, "Intel_Strago"),
> +               },
> +       },
> +       {
> +               .ident = "Acer Chromebook R11 (Cyan)",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Cyan"),
> +               },
> +       },
> +       {
> +               .ident = "Samsung Chromebook 3 (Celes)",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Celes"),
>                 },
>         },
>         {}
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] pinctrl: cherryview: Add terminate entry for dmi_system_id tables

[Andy Shevchenko]

On Wed, May 17, 2017 at 1:25 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> Make sure dmi_system_id tables are NULL terminated.
>

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: 703650278372 ("pinctrl: cherryview: Add a quirk to make Acer Chromebook keyboard work again")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index 2debba62fac9..e35d0fe4c737 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -1547,7 +1547,8 @@ static const struct dmi_system_id chv_no_valid_mask[] = {
>                         DMI_MATCH(DMI_PRODUCT_NAME, "Edgar"),
>                         DMI_MATCH(DMI_BIOS_DATE, "05/21/2016"),
>                 },
> -       }
> +       },
> +       {}
>  };
>
>  static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string

[Andy Shevchenko]

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

Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string

[Linus Walleij]

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

Re: [PATCH v2 2/3] pinctrl: cherryview: Add terminate entry for dmi_system_id tables

[Linus Walleij]

On Wed, May 17, 2017 at 12:25 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> Make sure dmi_system_id tables are NULL terminated.
>
> Fixes: 703650278372 ("pinctrl: cherryview: Add a quirk to make Acer Chromebook keyboard work again")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

I already had this patch queued.

Yours,
Linus Walleij

Re: [PATCH v2 3/3] pinctrl: cherryview: Extend the Chromebook DMI quirk to Intel_Strago systems

[Linus Walleij]

On Wed, May 17, 2017 at 12:25 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> It turns out there are quite many Chromebooks out there that have the
> same keyboard issue than Acer Chromebook. All of them are based on
> Intel_Strago reference and report their DMI_PRODUCT_FAMILY as
> "Intel_Strago" (Samsung Chromebook 3 and Cyan Chromebooks are exceptions
> for which we add separate entries).
>
> Instead of adding each machine to the quirk table, we use
> DMI_PRODUCT_FAMILY of "Intel_Strago" that hopefully covers most of the
> machines out there currently.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=194945
> Suggested: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied with Andy's review tag.

Yours,
Linus Walleij

Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string

[Jean Delvare]

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

Re: [PATCH v2 2/3] pinctrl: cherryview: Add terminate entry for dmi_system_id tables

[Jean Delvare]

On Wed, 17 May 2017 13:25:13 +0300, Mika Westerberg wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
> 
> Make sure dmi_system_id tables are NULL terminated.
> 
> Fixes: 703650278372 ("pinctrl: cherryview: Add a quirk to make Acer Chromebook keyboard work again")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index 2debba62fac9..e35d0fe4c737 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -1547,7 +1547,8 @@ static const struct dmi_system_id chv_no_valid_mask[] = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "Edgar"),
>  			DMI_MATCH(DMI_BIOS_DATE, "05/21/2016"),
>  		},
> -	}
> +	},
> +	{}
>  };
>  
>  static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)

Reviewed-by: Jean Delvare <jdelvare@suse.de>

This should go to stable trees IMHO.

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string

[Mika Westerberg]

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.

Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string

[Jean Delvare]

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

Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string

[Mika Westerberg]

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!

Re: [PATCH v2 1/3] firmware: dmi: Add DMI_PRODUCT_FAMILY identification string

[Jean Delvare]

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

Re: [PATCH v2 2/3] pinctrl: cherryview: Add terminate entry for dmi_system_id tables

[Linus Walleij]

On Thu, Jun 1, 2017 at 11:30 AM, Jean Delvare <jdelvare@suse.de> wrote:
> On Wed, 17 May 2017 13:25:13 +0300, Mika Westerberg wrote:
>> From: Wei Yongjun <weiyongjun1@huawei.com>
>>
>> Make sure dmi_system_id tables are NULL terminated.
>>
>> Fixes: 703650278372 ("pinctrl: cherryview: Add a quirk to make Acer Chromebook keyboard work again")
>> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
(...)
>
> This should go to stable trees IMHO.

It is already upstream, but you can suggest directly to Greg to pick it to
the stable trees.

Yours,
Linus Walleij