Re: [PATCH v4] platform: x86: Add ACPI driver for ChromeOS

["Rafael J. Wysocki" <rafael@kernel.org>]

On Thu, Jul 9, 2020 at 11:31 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Rafael,
>
> On 11/6/20 13:06, Enric Balletbo i Serra wrote:
> > Hi,
> >
> > On 11/6/20 0:43, Dmitry Torokhov wrote:
> >> On Wed, Jun 10, 2020 at 09:52:12PM +0000, Mario.Limonciello@dell.com wrote:
> >>>> -----Original Message-----
> >>>> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >>>> Sent: Wednesday, June 10, 2020 4:41 PM
> >>>> To: Limonciello, Mario
> >>>> Cc: enric.balletbo@collabora.com; rjw@rjwysocki.net; rafael@kernel.org;
> >>>> linux-kernel@vger.kernel.org; linux-acpi@vger.kernel.org; lenb@kernel.org;
> >>>> kernel@collabora.com; groeck@chromium.org; bleung@chromium.org;
> >>>> dtor@chromium.org; gwendal@chromium.org; vbendeb@chromium.org;
> >>>> andy@infradead.org; ayman.bagabas@gmail.com; benjamin.tissoires@redhat.com;
> >>>> blaz@mxxn.io; dvhart@infradead.org; gregkh@linuxfoundation.org;
> >>>> hdegoede@redhat.com; jeremy@system76.com; 2pi@mok.nu;
> >>>> mchehab+samsung@kernel.org; rajatja@google.com;
> >>>> srinivas.pandruvada@linux.intel.com; platform-driver-x86@vger.kernel.org
> >>>> Subject: Re: [PATCH v4] platform: x86: Add ACPI driver for ChromeOS
> >>>>
> >>>>
> >>>> [EXTERNAL EMAIL]
> >>>>
> >>>> On Wed, Jun 10, 2020 at 09:28:36PM +0000, Mario.Limonciello@dell.com wrote:
> >>>>>>
> >>>>>> To give you some references, if I'm not wrong, this prefix is used in
> >>>> all
> >>>>>> or
> >>>>>> almost all Intel Chromebook devices (auron, cyan, eve, fizz, hatch,
> >>>>>> octopus,
> >>>>>> poppy, strago ...) The ACPI source for this device can be found here
> >>>> [1],
> >>>>>> and,
> >>>>>> if not all, almost all Intel based Chromebooks are shipped with the
> >>>>>> firmware
> >>>>>> that supports this.
> >>>>>
> >>>>> You can potentially carry a small patch in your downstream kernel for the
> >>>>> legacy stuff until it reaches EOL.  At least for the new stuff you could
> >>>>> enact a process that properly reserves unique numbers and changes the
> >>>> driver
> >>>>> when the interface provided by the ACPI device has changed.
> >>>>
> >>>> If we use this prefix for hatch EOL is ~7 years from now.
> >>>>
> >>>
> >>> Isn't the whole point of the ACPI registry and choosing an ID?  You know internally
> >>> if you need to change the interface that a new ID is needed and a new driver will
> >>> be needed that comprehends that ID change.  So if you can't guarantee that 0001 is
> >>> the same driver interface in every firmware implementation google used then that is
> >>> where this falls apart.
> >>>
> >
> > As far as I know GGL0001 has the same driver interface in every firmware
> > implementation Google used. But I'll ask to make sure.
> >
> >>> I know there is a long support lifecycle but you're talking about rebasing
> >>> to new LTS kernels a handful of times between now and then.  If the interface really
> >>> is stable the patch should be small and it shouldn't be a large amount of technical
> >>> debt to carry downstream until EOL.
> >>
> >> I think we are talking about different things actually. Let's forget
> >> about Chrome OS and downstream kernels. We have devices that have
> >> already been shipped and in hands of users. Some of them are old, some
> >> of them are new. We can't not enforce that firmware for these devices
> >> will be either released or updated. Therefore, if we want expose this
> >> device in mainline kernel, we need to have it handle "GGL0001" HID in
> >> addition to whatever proper HID we may select for it.
> >>
> >
> > FWIW, after investigate a bit more, although GGL is not in the ACPI ID list it
> > is in the PNP ID list [1]. So as far as I understand GGL0001 is valid ID. I know
> > that PNP ID is the legacy identifier but since this was here for long time and
> > will be here also for long time, I am wondering if we can take that as an
> > argument to have GGL0001 as a valid device to be exposed in the kernel.
> >
>
> So, as the GGL prefix is a valid ID in the PNP ID list, is this a valid argument
> to take in consideration this patch and resolves your concern regarding the ID?

Yes, it does, thanks!