From: Enric Balletbo i Serra <enric.balletbo@collabora.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org, vbendeb@chromium.org,
groeck@chromium.org, bleung@chromium.org, dtor@chromium.org,
gwendal@chromium.org, andy@infradead.org,
Collabora Kernel ML <kernel@collabora.com>,
Ayman Bagabas <ayman.bagabas@gmail.com>,
Darren Hart <dvhart@infradead.org>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Jeremy Soller <jeremy@system76.com>,
Mattias Jacobsson <2pi@mok.nu>,
Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
Rajat Jain <rajatja@google.com>,
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
Yauhen Kharuzhy <jekhor@gmail.com>,
platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH v2] platform: x86: Add ACPI driver for ChromeOS
Date: Tue, 24 Mar 2020 18:08:03 +0100 [thread overview]
Message-ID: <3444110c-d6c0-16df-9b5d-12578ed442c5@collabora.com> (raw)
In-Reply-To: <20200324164956.GE2518746@kroah.com>
Hi Greg,
On 24/3/20 17:49, Greg Kroah-Hartman wrote:
> On Tue, Mar 24, 2020 at 05:31:10PM +0100, Enric Balletbo i Serra wrote:
>> Hi Greg,
>>
>> Many thanks for your quick answer, some comments below.
>>
>> On 22/3/20 12:10, Greg Kroah-Hartman wrote:
>>> On Sun, Mar 22, 2020 at 10:43:34AM +0100, 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. 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 sysfs
>>>> directory tree originating in /sys/devices/platform/chromeos_acpi.
>>>>
>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>
>>> What is wrong with the "default" ACPI sysfs access? Why do you need a
>>> special driver just for this specific ACPI firmware?
>>>
>>
>> Please correct me if I am wrong, as I'm not an ACPI expert and I probably have
>> some ACPI leaks and misunderstandings.
>>
>> What is exporting this driver is the attributes for the non-default Chromebook
>> specific MLST ACPI method. Hence, I assumed we needed a special driver to expose
>> these values that can't be done using "default" ACPI sysfs. Note that these
>> attributes are dynamically created and are different between Chromebooks so need
>> some parsing.
>>
>> I didn't find a "standard" way to expose these attributes to userspace, so,
>> please kindly point me to one if there is one.
>
> Are you sure they aren't already there under /sys/firmware/acpi/? I
> thought all tables and methods were exported there with no need to do
> anything special.
>
That's the first I did when I started to forward port this patch from chromeos
kernel to mainline.
On my system I get:
/sys/firmware/acpi/tables#
APIC DSDT FACP FACS HPET MCFG SSDT data dynamic
(data and dynamic are empty directories)
I quickly concluded (maybe wrong) that as there is no a MLST entry it was not
exported, but maybe one of those already contains the info? Or, should I expect
a MLST entry here?
> What makes these attributes "special" from any other ACPI method?
>
I can't answer this question right now. I need to investigate more I guess ;-)
Thanks again for your answer,
Enric
>>>> +static int __init chromeos_acpi_init(void)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + chromeos_acpi.pdev = platform_device_register_simple("chromeos_acpi",
>>>> + PLATFORM_DEVID_NONE, NULL, 0);
>>>> + if (IS_ERR(chromeos_acpi.pdev)) {
>>>> + pr_err("unable to register chromeos_acpi platform device\n");
>>>> + return PTR_ERR(chromeos_acpi.pdev);
>>>> + }
>>>
>>> Only use platform devices and drivers for things that are actually
>>> platform devices and drivers. That's not what this is, it is an ACPI
>>> device and driver. Don't abuse the platform interface please.
>>>
>>
>> Ok. The purpose was to not break ChromeOS userspace since is looking for the
>> attributes inside /sys/devices/platform/chromeos_acpi. Not a good reason, I
>> know, and I assume we will need to change userspace instead, and convert this to
>> a ACPI device and driver only, right?
>
> How can any userspace be looking for anything that hasn't been submitted
> before? That's nothing to worry about, we don't have to support things
> like that :)
>
>> I'll investigate the different places in userspace where this is used and see
>> how difficult it is to do the changes.
>
> Look at /sys/firmware/acpi/ first please.
>
> thanks,
>
> greg k-h
>
next prev parent reply other threads:[~2020-03-24 17:08 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-22 9:43 [PATCH v2] platform: x86: Add ACPI driver for ChromeOS Enric Balletbo i Serra
2020-03-22 11:10 ` Greg Kroah-Hartman
2020-03-24 16:31 ` Enric Balletbo i Serra
2020-03-24 16:49 ` Greg Kroah-Hartman
2020-03-24 17:08 ` Enric Balletbo i Serra [this message]
2020-03-24 17:16 ` Greg Kroah-Hartman
2020-03-24 17:20 ` Srinivas Pandruvada
2020-03-25 20:34 ` Enric Balletbo i Serra
2020-03-25 21:41 ` Srinivas Pandruvada
2020-03-25 21:54 ` Enric Balletbo i Serra
2020-03-25 22:38 ` Srinivas Pandruvada
2020-03-22 15:21 ` Srinivas Pandruvada
2020-03-24 16:36 ` Enric Balletbo i Serra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3444110c-d6c0-16df-9b5d-12578ed442c5@collabora.com \
--to=enric.balletbo@collabora.com \
--cc=2pi@mok.nu \
--cc=andy@infradead.org \
--cc=ayman.bagabas@gmail.com \
--cc=bleung@chromium.org \
--cc=dmitry.torokhov@gmail.com \
--cc=dtor@chromium.org \
--cc=dvhart@infradead.org \
--cc=gregkh@linuxfoundation.org \
--cc=groeck@chromium.org \
--cc=gwendal@chromium.org \
--cc=jekhor@gmail.com \
--cc=jeremy@system76.com \
--cc=kernel@collabora.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mchehab+samsung@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=rajatja@google.com \
--cc=srinivas.pandruvada@linux.intel.com \
--cc=vbendeb@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).