Message-ID: <CAFTm+6CQ+EkMyt35bLzb0+GcHaCiCk8XFP5-dSwev90on4GT=A@mail.gmail.com>
Date: Mon, 30 Jun 2025 08:13:23 -0500
From: "Mr. Chromebox" <mrchromebox@...il.com>
To: Pedro Nariyoshi <pedro.nariyoshi@...il.com>
Cc: hans@...erkuil.nl, Hans Verkuil <hverkuil@...all.nl>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, Benson Leung <bleung@...omium.org>, 
	Guenter Roeck <groeck@...omium.org>, linux-media@...r.kernel.org, 
	chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: cros-ec-cec: Add Fizz board variants, so driver
 can detect them

> (Sorry to those re-receiving this message, my email client added HTML tags to the links and I didn't notice until too late)

same apologies from me

On Mon, Jun 30, 2025 at 7:52 AM Pedro Nariyoshi
<pedro.nariyoshi@...il.com> wrote:
>
> (Sorry to those re-receiving this message, my email client added HTML tags to the links and I didn't notice until too late)
>
> Hello Hans,
> I don't particularly have a preference on what we match against. I went in this direction because I felt it was less disruptive to add elements to a table, than rewrite the matching function and the table (especially without the hardware to validate the changes on). I started looking at devices based on the same board on the chromium developer list (searching for "Fizz"):
>
> https://www.chromium.org/chromium-os/developer-library/reference/development/developer-information-for-chrome-os-devices/
>
> This list gave me Kench, Sion, Teemo and Wukong. Then I looked at the supported list for the coreboot implementation I am using:
>
> https://github.com/MrChromebox/scripts/blob/main/functions.sh
>
> The remaining boards seem to be rare variants of these devices (google meet hardware, i7 variants of the same board, etc). This gave me the rest of the boards I have added. Although, I now realize, I should have just gone to the source of coreboot:
>
> https://github.com/coreboot/coreboot/blob/main/src/mainboard/google/fizz/mainboard.c#L184-L194
>
> This made me realize I am missing Karma (an acer all-in-one) and Endeavor (another google meet variant). I could add these two missing boards, but I think it'd be helpful to have someone from Google chime in as to whether it'd make sense to change the matching scheme instead. Depending on what they think, I'd be open to rewrite the patch whichever way it is decided.
> Regards,
>
> Pedro Nariyoshi
>
> On Mon, Jun 30, 2025 at 2:25 AM <hans@...erkuil.nl> wrote:
>>
>> Hi Pedro,
>>
>> On 28/06/2025 20:14, Pedro Nariyoshi wrote:
>> > I recently reflashed a Chromebox (Wukong variant of the Fizz board) with
>> > coreboot and I noticed that the cec driver refused to load with a bit of
>> > tinkering, I realized that the dmi_match_table was expecting the product
>> > name to be Fizz, but `dmidecode` reports `Wukong` as the product name. I
>> > am not sure if this is the best approach, but adding this patch lets me
>> > load the driver and it works properly.
>> >
>> > Alternatively, we could instead match the DMI_PRODUCT_FAMILY, instead of
>> > DMI_SYS_VENDOR and DMI_PRODUCT_NAME. In my board at least, that says
>> > "Google_Fizz".
>> >
>> > I am open to suggestions for alternative solutions and I hope I did't
>> > break any rules (this is my first kernel patch). (And sorry for the
>> > previous submissions with errors in the subject line)
>> >
>> > Signed-off-by: Pedro Nariyoshi <pedro.nariyoshi@...il.com>
>>
>> Thank you for your patch, but I would like to have someone from Google
>> review this as well.
>>
>> The number of entries keeps increasing, so perhaps switching to
>> DMI_PRODUCT_FAMILY would make sense. But I have no insight in how this
>> is done internally at Google.

DMI_PRODUCT_FAMILY is the baseboard/reference board name (Fizz, Puff,
Brask, etc). The individual boards/retail products each have their own
codename as well, which Google sets as the ChromeOS HWID in their
custom coreboot firmware. With upstream coreboot firmware, the
individual board names are used for the DMI_PRODUCT_NAME as the HWID
doesn't exist there, which is why the existing matching works with
Google's firmware but not mine.

I highly recommend switching to use the DMI_PRODUCT_FAMILY instead, as
it's deterministic and a simpler match. Using it along with
DMI_SYS_VENDOR is fine.

>> One question for Pedro: where did you get all the other code names
>> from? Based on the commit message you have the Wukong variant, but how
>> did you find all the other variants?

they're available from many sources online, including my website
(mrchromebox.tech) and the coreboot source code

>> Regards,
>>
>>         Hans
>>

regards,
Matt

>> > ---
>> >  drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 9 ++++++++-
>> >  1 file changed, 8 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
>> > index 419b9a7abcce..a26473c3cd84 100644
>> > --- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
>> > +++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
>> > @@ -302,8 +302,15 @@ static const char *const port_ab_conns[] = { "Port A", "Port B", NULL };
>> >  static const char *const port_d_conns[] = { "Port D", NULL };
>> >
>> >  static const struct cec_dmi_match cec_dmi_match_table[] = {
>> > -     /* Google Fizz */
>> > +     /* Google Fizz and variants*/
>> >       { "Google", "Fizz", "0000:00:02.0", port_b_conns },
>> > +     { "Google", "Bleemo", "0000:00:02.0", port_b_conns },
>> > +     { "Google", "Excelsior", "0000:00:02.0", port_b_conns },
>> > +     { "Google", "Jax", "0000:00:02.0", port_b_conns },
>> > +     { "Google", "Kench", "0000:00:02.0", port_b_conns },
>> > +     { "Google", "Sion", "0000:00:02.0", port_b_conns },
>> > +     { "Google", "Teemo", "0000:00:02.0", port_b_conns },
>> > +     { "Google", "Wukong", "0000:00:02.0", port_b_conns },
>> >       /* Google Brask */
>> >       { "Google", "Brask", "0000:00:02.0", port_b_conns },
>> >       /* Google Moli */
>>

Powered by blists - more mailing lists