ChromeOS EC LPC lacks DMI match for newer machines, which
use "Google" DMI_SYS_VENDOR as opposed to "GOOGLE" in older models.
This patch adds DMI definition for MrChomebox's custom Coreboots builds,
which we (Chrultrabook Project) are using.
Signed-off-by: Alicja Michalska <[email protected]>
---
drivers/platform/chrome/cros_ec_lpc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 500a61b093e4..6ac993be4eb1 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -533,6 +533,14 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "Glimmer"),
},
},
+ /* DMI doesn't match modern machines running custom firmware */
+ {
+ /* MrChromebox's firmware */
+ .matches = {
+ DMI_MATCH(DMI_BIOS_VENDOR, "coreboot"),
+ DMI_MATCH(DMI_BIOS_VERSION, "MrChromebox-"),
+ },
+ },
/* A small number of non-Chromebook/box machines also use the ChromeOS EC */
{
/* the Framework Laptop */
--
2.41.0
On Fri, Jul 21, 2023 at 12:37:15AM +0200, Alicja Michalska wrote:
> ChromeOS EC LPC lacks DMI match for newer machines, which
> use "Google" DMI_SYS_VENDOR as opposed to "GOOGLE" in older models.
I'm confused about the sentence as it looks irrelevant to the patch.
> This patch adds DMI definition for MrChomebox's custom Coreboots builds,
> which we (Chrultrabook Project) are using.
s/This patch adds/Add/. Search "imperative mood" in [1].
Looks like a typo: s/MrChomebox/MrChromebox/.
If you get chance to send next version, please shorten the commit title.
I guess "platform/chrome: cros_ec_lpc: Add DMI match for MrChromebox" should
be quite explicit.
[1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> + /* DMI doesn't match modern machines running custom firmware */
Remove the line.
> + {
> + /* MrChromebox's firmware */
> + .matches = {
> + DMI_MATCH(DMI_BIOS_VENDOR, "coreboot"),
> + DMI_MATCH(DMI_BIOS_VERSION, "MrChromebox-"),
> + },
> + },
Put the block after "A small number of non-Chromebook/box machines also use
the ChromeOS EC"[2].
[2]: https://elixir.bootlin.com/linux/v6.4/source/drivers/platform/chrome/cros_ec_lpc.c#L533
Thank you for your feedback.
I've explained the reason behind adding this patch, but we'll go with
different approach next time around.
Since we're discussing this, I would like to suggest removal of DMI
matches for EOL machines from lines 503...535 (Link, Samus, Peppy, Glimmer).
Those machines aren't supported by Google anymore. Patch I suggested
will match DMI while running custom firmware.
If maintainers are okay with it, I will submit a patch removing DMI
matches for stock firmware running on those machines since it's not
needed anymore.
On 21/07/2023 08:39, Tzung-Bi Shih wrote:
> On Fri, Jul 21, 2023 at 12:37:15AM +0200, Alicja Michalska wrote:
>> ChromeOS EC LPC lacks DMI match for newer machines, which
>> use "Google" DMI_SYS_VENDOR as opposed to "GOOGLE" in older models.
>
> I'm confused about the sentence as it looks irrelevant to the patch.
>
>> This patch adds DMI definition for MrChomebox's custom Coreboots builds,
>> which we (Chrultrabook Project) are using.
>
> s/This patch adds/Add/. Search "imperative mood" in [1].
>
> Looks like a typo: s/MrChomebox/MrChromebox/.
>
> If you get chance to send next version, please shorten the commit title.
> I guess "platform/chrome: cros_ec_lpc: Add DMI match for MrChromebox" should
> be quite explicit.
>
> [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
>> + /* DMI doesn't match modern machines running custom firmware */
>
> Remove the line.
>
>> + {
>> + /* MrChromebox's firmware */
>> + .matches = {
>> + DMI_MATCH(DMI_BIOS_VENDOR, "coreboot"),
>> + DMI_MATCH(DMI_BIOS_VERSION, "MrChromebox-"),
>> + },
>> + },
>
> Put the block after "A small number of non-Chromebook/box machines also use
> the ChromeOS EC"[2].
>
> [2]: https://elixir.bootlin.com/linux/v6.4/source/drivers/platform/chrome/cros_ec_lpc.c#L533
On Fri, Jul 21, 2023 at 10:16 AM Alicja Michalska <[email protected]> wrote:
> I've explained the reason behind adding this patch, but we'll go with
> different approach next time around.
FWIW, I'm also confused about your first sentence the same way
Tzung-Bi is. If two people are confused by parts of your description,
then maybe it needs improvement :)
> Since we're discussing this, I would like to suggest removal of DMI
> matches for EOL machines from lines 503...535 (Link, Samus, Peppy, Glimmer).
>
> Those machines aren't supported by Google anymore. Patch I suggested
> will match DMI while running custom firmware.
>
> If maintainers are okay with it, I will submit a patch removing DMI
> matches for stock firmware running on those machines since it's not
> needed anymore.
That seems actively harmful. These devices continue to work just fine
with their stock BIOS, even if Google no longer supports updating the
Google-built OS. That doesn't mean people can't boot other OS'es
(e.g., their own ChromiumOS builds; or other Linux distros) on them.
Brian
That's understandable :)
I spoke with Matt (MrChromebox). It seems like he tried to upstream
similar patch to what I suggested here few years ago, but it never got
merged.
Reason for this is that pre-Skylake machines had different DMI vendor name.
For example, that's my DMI information from ELDRID (Google/Volteer
baseboard) running MrChromebox's firmware and mainline kernel:
[...]
Vendor: coreboot
Manufacturer: Google
[...]
There's a match for coreboot/GOOGLE, but not for coreboot/Google - which
is the case for all modern-ish machines made past 2018 (reef, octopus,
hatch, volteer, brya and so on).
As for the concerns regarding removal of outdated DMIs - it's
understandable, but mainline doesn't work correctly on stock firmware.
In order to get mainline Linux running correctly on any ChromeOS device,
user has to flash firmware that contains our patches.
We currently support over 100 machines, starting with first machines
that used Coreboot (SandyBridge) up to AlderLake (at the moment).
Flashing those machines (pre-CR50) is as simple as removing Write
Protect screw and running MrChromebox's script from ChromeOS shell.
Once that's done, Chromebooks behave in the same exact way as UEFI
systems (because we're using EDK2 as Coreboot payload in our builds)
with correct ACPI tables and other numerous fixes that are missing from
stock firmware.
https://chrultrabook.github.io/docs/docs/getting-started.html
On 21/07/2023 19:28, Brian Norris wrote:
> On Fri, Jul 21, 2023 at 10:16 AM Alicja Michalska <[email protected]> wrote:
>> I've explained the reason behind adding this patch, but we'll go with
>> different approach next time around.
>
> FWIW, I'm also confused about your first sentence the same way
> Tzung-Bi is. If two people are confused by parts of your description,
> then maybe it needs improvement :)
>
>> Since we're discussing this, I would like to suggest removal of DMI
>> matches for EOL machines from lines 503...535 (Link, Samus, Peppy, Glimmer).
>>
>> Those machines aren't supported by Google anymore. Patch I suggested
>> will match DMI while running custom firmware.
>>
>> If maintainers are okay with it, I will submit a patch removing DMI
>> matches for stock firmware running on those machines since it's not
>> needed anymore.
>
> That seems actively harmful. These devices continue to work just fine
> with their stock BIOS, even if Google no longer supports updating the
> Google-built OS. That doesn't mean people can't boot other OS'es
> (e.g., their own ChromiumOS builds; or other Linux distros) on them.
>
> Brian
On Thu, Jul 20, 2023 at 4:37 PM Alicja Michalska <[email protected]> wrote:
>
> ChromeOS EC LPC lacks DMI match for newer machines, which
> use "Google" DMI_SYS_VENDOR as opposed to "GOOGLE" in older models.
> This patch adds DMI definition for MrChomebox's custom Coreboots builds,
> which we (Chrultrabook Project) are using.
>
> Signed-off-by: Alicja Michalska <[email protected]>
> ---
> drivers/platform/chrome/cros_ec_lpc.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 500a61b093e4..6ac993be4eb1 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -533,6 +533,14 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
> DMI_MATCH(DMI_PRODUCT_NAME, "Glimmer"),
> },
> },
> + /* DMI doesn't match modern machines running custom firmware */
> + {
> + /* MrChromebox's firmware */
> + .matches = {
> + DMI_MATCH(DMI_BIOS_VENDOR, "coreboot"),
> + DMI_MATCH(DMI_BIOS_VERSION, "MrChromebox-"),
> + },
> + },
> /* A small number of non-Chromebook/box machines also use the ChromeOS EC */
> {
> /* the Framework Laptop */
> --
> 2.41.0
+ Mrchromebox
Hrmm, it looks like this table is only used if the GOOG0004 ACPI
device wasn't found. Is the MrChromebox fw missing this ACPI device?