Messages in this thread | | | From | Gwendal Grignou <> | Date | Wed, 28 Apr 2021 22:36:46 -0700 | Subject | Re: [PATCH v2] platform/chrome: Don't populate lightbar device if it isn't there |
| |
On Fri, Apr 9, 2021 at 6:59 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Stephen Boyd (2020-09-10 17:53:07) > > Quoting Enric Balletbo i Serra (2020-09-10 08:49:42) > > > On 10/9/20 16:52, Guenter Roeck wrote: > > > > On Thu, Sep 10, 2020 at 7:32 AM Enric Balletbo i Serra > > > > <enric.balletbo@collabora.com> wrote: > > > >> On 10/9/20 16:18, Guenter Roeck wrote: > > > >>> On Wed, Sep 9, 2020 at 3:42 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > >>>> @@ -206,6 +209,17 @@ static int ec_device_probe(struct platform_device *pdev) > > > >>>> } > > > >>>> } > > > >>>> > > > >>>> + if (!strcmp(ec_platform->ec_name, CROS_EC_DEV_NAME) && > > > >>>> + !cros_ec_get_lightbar_version(ec, NULL, NULL)) { > > > >>> > > > >>> Any idea why the lightbar code doesn't use cros_ec_check_features() ? > > > >>> There is a definition for EC_FEATURE_LIGHTBAR, but it doesn't seem to > > > >>> be used. It would be much more convenient if that feature check could > > > >>> be used instead of moving the get_lightbar_version command and its > > > >>> helper function around. > > > >>> > > > >> > > > >> IIRC it was to support a very old device, the Pixel Chromebook (Link). This flag > > > >> is not set in this device but has a lightbar, hence we had this 'weird' way to > > > >> detect the lightbar. > > > >> > > > > > > > > If that is the only reason, wouldn't it be better to use something > > > > else (eg dmi_match) to determine if the system in question is a Pixel > > > > Chromebook (Link) ? > > > > > > > > if (!strcmp(ec_platform->ec_name, CROS_EC_DEV_NAME) && > > > > (cros_ec_check_features(ec, EC_FEATURE_LIGHTBAR) || > > > > dmi_match(DMI_PRODUCT_NAME, "Link")) {
check_features supporting lightbar was introduced with crrev.com/c/263463 in 2015. It is present devices with lightbar like Pixel C and backported to Pixel 2 (samus) with crrev.com/c/274047. Remaining boards that have lightbar support and not check_features support are: link bolt (reference board for early samus).
Therefore, that solution is right. I will send a patch based on that code snippet.
Gwendal.
> > > > > > > > > > That looks a better solution, indeed. And definetely I'd prefer use the check > > > features way. > > > > > > Gwendal, can you confirm that the Pixel Chromebook (Link) is the _only_ one > > > affected? This one is the only that comes to my mind but I might miss others. > > > > > > I think that Samus has this flag (I can double check) and this was discussed > > > with you (long, long time ago :-) ) > > > > > > > Sounds fine by me. I'll wait for Gwendal to inform us. > > Anything come of this? I haven't seen any updates.
| |