From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C168CC433E0 for ; Fri, 5 Jun 2020 11:17:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A1D7C206FA for ; Fri, 5 Jun 2020 11:17:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726874AbgFELRU (ORCPT ); Fri, 5 Jun 2020 07:17:20 -0400 Received: from cloudserver094114.home.pl ([79.96.170.134]:53692 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726678AbgFELRU (ORCPT ); Fri, 5 Jun 2020 07:17:20 -0400 Received: from 89-64-83-87.dynamic.chello.pl (89.64.83.87) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.415) id 01eedc837bead76a; Fri, 5 Jun 2020 13:17:17 +0200 From: "Rafael J. Wysocki" To: Enric Balletbo i Serra Cc: "Rafael J. Wysocki" , Linux Kernel Mailing List , ACPI Devel Maling List , Len Brown , Collabora Kernel ML , Guenter Roeck , Benson Leung , Dmitry Torokhov , Gwendal Grignou , vbendeb@chromium.org, Andy Shevchenko , Ayman Bagabas , Benjamin Tissoires , =?utf-8?B?Qmxhxb4=?= Hrastnik , Darren Hart , Dmitry Torokhov , Greg Kroah-Hartman , Hans de Goede , Jeremy Soller , Mattias Jacobsson <2pi@mok.nu>, Mauro Carvalho Chehab , Rajat Jain , Srinivas Pandruvada , platform-driver-x86@vger.kernel.org Subject: Re: [PATCH v4] platform: x86: Add ACPI driver for ChromeOS Date: Fri, 05 Jun 2020 13:17:15 +0200 Message-ID: <10490419.gsntqH5CaE@kreacher> In-Reply-To: References: <20200413134611.478441-1-enric.balletbo@collabora.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > 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. 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? 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? 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 present in the list at https://uefi.org/acpi_id_list There are two ways to address that. One would be to take the GOOG prefix (present in the list above), append a proper unique number (if I were to guess, I would say that 0001 had been reserved already) to it and then put the resulting device ID into the firmware, to be returned _HID for the device in question (you can add a _CID returning "GGL0001" so it can be found by the old invalid ID at least from the kernel). The other one would be to properly register the GGL prefix for Google and establish a process for allocating IDs with that prefix internally. 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? Thanks!