On Wed, Jun 10, 2020 at 11:21 PM Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx> wrote: > > Hi Rafael, > > Many thanks for your feedback. See my answers inline. > > On 5/6/20 13:17, Rafael J. Wysocki wrote: > > On Tuesday, April 14, 2020 4:35:38 PM CEST Enric Balletbo i Serra wrote: > >> Hi Rafael, > >> > >> On 13/4/20 22:41, Rafael J. Wysocki wrote: > >>> On Mon, Apr 13, 2020 at 3:46 PM Enric Balletbo i Serra > >>> <enric.balletbo@xxxxxxxxxxxxx> wrote: > >>>> > >>>> This driver attaches to the ChromeOS ACPI device and then exports the values > >>>> reported by the ACPI in a sysfs directory. These values are not exported > >>>> via the standard ACPI tables, hence a specific driver is needed to do > >>>> it. > >>> > >>> So how exactly are they exported? > >>> > >> > >> They are exported through sysfs. > >> > >>>> The ACPI values are presented in the string form (numbers as decimal > >>>> values) or binary blobs, and can be accessed as the contents of the > >>>> appropriate read only files in the standard ACPI devices sysfs directory tree. > >>> > >>> My understanding based on a cursory look at the patch is that there is > >>> an ACPI device with _HID equal to "GGL0001" and one or more special > >>> methods under it that return values which you want to export over > >>> sysfs as binary attributes. They appear to be read-only. > >>> > >> > >> Exactly, there is an ACPI device equal to "GGL0001" and one special method > >> called MLST that returns a list of the other control methods supported by the > >> Chrome OS hardware device. The driver calls the special MLST method and goes > >> through the list. > >> > >>> I guess that these data are to be consubed by user space? > >>> > >> > >> Yes, this is used by user space, to be more specific ChromeOS userspace uses it. > > > > Well, let me start over. > > > > The subject and changelog of this patch are not precise enough IMO and there is > > not enough information in the latter. > > > > Ok, I can improve that. Please do. > > It is not clear what "ACPI driver for ChromeOS" means. There may be many ACPI > > drivers in a Linux-based system as a rule. > > > > It is unclear what the ChromeOS ACPI device is and why it is there. Is there > > any documentation of it you can point me to? > > > > I'm afraid, I don't think there is any documentation, I'll ask around. > > > It is unclear what you mean by "These values are not exported via the standard > > ACPI tables". > > > > It looks like (but it is not actually documented in any way) the idea is to > > get to the ACPI device object with _HID returning "GGL0001", evaluate the > > MLST method under it and then evaluate the methods listed by it and export the > > data returned by them via sysfs, under the "GGL0001" device on the "acpi" bus. > > Is this correct? > > > > Yes, this is correct. > > > If so, there is a couple of issues here. > > > > First off, GGL0001 is not a valid ACPI device ID, because the GGL prefix is not [cut] > > Next, device attributes in sysfs are part of the kernel ABI and once defined, > > they cannot change (exceptions happen, but rarely), so you must guarantee > > that whatever appears in there, will always be present for devices with the > > given device ID in the future in the same format. > > > > Can you actually guarantee that? If so, what is that guarantee based on? > > > > Although is not really documented, can we say that this is a standard "de facto" > assuming that there are lots of devices in the field and for a long time with > that? Can this be a guarantee? I would like the firmware interface to be documented in Documentation/firmware-guide/acpi/ in the first place, given the lack of any existing documentation of it that can be pointed to. Thanks!
