[PATCH RESEND] platform/chrome: cros_ec_lpc: Add support for newer Google devices using custom coreboot firmware or stock SeaBIOS.

[PATCH RESEND] platform/chrome: cros_ec_lpc: Add support for newer Google devices using custom coreboot firmware or stock SeaBIOS.

[Vittorio Gambaletta (VittGam)]

Custom coreboot firmware does not contain "Google_*" as BIOS
version string; also, booting through stock SeaBIOS will present
an empty BIOS version string to Linux. So the generic match at
the top would not work.

A previous patch added the "GOOGLE" match for DMI system vendor,
but newer machines such as Skylake-based Caroline use "Google"
instead.

Also amend the previous comment adding a note that this is needed
for stock SeaBIOS too.

Signed-off-by: Vittorio Gambaletta <linuxbugs@vittgam.net>
Signed-off-by: Salvatore Bellizzi <lkml@seppia.net>

---

--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -347,7 +347,8 @@
 	},
 	{
 		/*
-		 * If the box is running custom coreboot firmware then the
+		 * If the box is running custom coreboot firmware,
+		 * or is booting Linux through stock SeaBIOS, then the
 		 * DMI BIOS version string will not be matched by "Google_",
 		 * but the system vendor string will still be matched by
 		 * "GOOGLE".
@@ -358,6 +359,16 @@
 		},
 	},
 	{
+		/*
+		 * Newer machines use "Google" instead of "GOOGLE" as
+		 * DMI system vendor string.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BIOS_VENDOR, "coreboot"),
+			DMI_MATCH(DMI_SYS_VENDOR, "Google"),
+		},
+	},
+	{
 		/* x86-link, the Chromebook Pixel. */
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),

Re: [PATCH RESEND] platform/chrome: cros_ec_lpc: Add support for newer Google devices using custom coreboot firmware or stock SeaBIOS.

[Dmitry Torokhov]

Hi Vittorio,

On Mon, May 21, 2018 at 07:44:35AM +0200, Vittorio Gambaletta (VittGam) wrote:
> Custom coreboot firmware does not contain "Google_*" as BIOS
> version string; also, booting through stock SeaBIOS will present
> an empty BIOS version string to Linux. So the generic match at
> the top would not work.
> 
> A previous patch added the "GOOGLE" match for DMI system vendor,
> but newer machines such as Skylake-based Caroline use "Google"
> instead.
> 
> Also amend the previous comment adding a note that this is needed
> for stock SeaBIOS too.

I think this driver is incorrectly structured and adding new DMI
signatures should not be needed. The newer hardware has the device
GOOG0004 in their ACPI tables, we should not need additional DMI checks.
DMI checks should only be needed for much older hardware, such as
Link/Peppy that did not have EC described in DSDT.

Let me see if I can fix it up.

> 
> Signed-off-by: Vittorio Gambaletta <linuxbugs@vittgam.net>
> Signed-off-by: Salvatore Bellizzi <lkml@seppia.net>
> 
> ---
> 
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -347,7 +347,8 @@
>  	},
>  	{
>  		/*
> -		 * If the box is running custom coreboot firmware then the
> +		 * If the box is running custom coreboot firmware,
> +		 * or is booting Linux through stock SeaBIOS, then the
>  		 * DMI BIOS version string will not be matched by "Google_",
>  		 * but the system vendor string will still be matched by
>  		 * "GOOGLE".
> @@ -358,6 +359,16 @@
>  		},
>  	},
>  	{
> +		/*
> +		 * Newer machines use "Google" instead of "GOOGLE" as
> +		 * DMI system vendor string.
> +		 */
> +		.matches = {
> +			DMI_MATCH(DMI_BIOS_VENDOR, "coreboot"),
> +			DMI_MATCH(DMI_SYS_VENDOR, "Google"),
> +		},
> +	},
> +	{
>  		/* x86-link, the Chromebook Pixel. */
>  		.matches = {
>  			DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),

Thanks.

-- 
Dmitry