Re: [PATCH v3 2/3] power: supply: Add Acer Aspire 1 embedded controller driver

[Nikita Travkin <nikita@trvn.ru>]

Hans de Goede писал(а) 26.02.2024 15:59:
> Hi,
> 
> +Ilpo (fellow pdx86 maintainer)
> 
> On 2/23/24 15:32, Nikita Travkin wrote:
>> Sebastian Reichel писал(а) 22.02.2024 04:41:
>>> Hi,
>>>
>>> On Tue, Feb 20, 2024 at 04:57:13PM +0500, Nikita Travkin wrote:
>>>> Acer Aspire 1 is a Snapdragon 7c based laptop. It uses an embedded
>>>> controller to control the charging and battery management, as well as to
>>>> perform a set of misc functions.
>>>>
>>>> Unfortunately, while all this functionality is implemented in ACPI, it's
>>>> currently not possible to use ACPI to boot Linux on such Qualcomm
>>>> devices. To allow Linux to still support the features provided by EC,
>>>> this driver reimplments the relevant ACPI parts. This allows us to boot
>>>> the laptop with Device Tree and retain all the features.
>>>>
>>>> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
>>>> ---
>>>>  drivers/power/supply/Kconfig           |  14 +
>>>>  drivers/power/supply/Makefile          |   1 +
>>>>  drivers/power/supply/acer-aspire1-ec.c | 453 +++++++++++++++++++++++++++++++++
>>>
>>> I think this belongs into drivers/platform, as it handles all bits of
>>> the EC.
>>>
>>
>> Hm, I initially submitted it to power/supply following the c630 driver,
>> but I think you're right... Though I'm not sure where in platform/ I'd
>> put this driver... (+CC Hans)
>>
>> Seems like most of the things live in platform/x86 but there is no i.e.
>> platform/arm64...
>>
>> Hans, (as a maintainer for most things in platform/) what do you think
>> would be the best place to put this (and at least two more I'd expect)
>> driver in inside platform/? And can we handle it through the
>> platform-driver-x86 list?
> 
> I guess that adding a drivers/platform/aarch64 map for this makes
> sense, with some comments in the Makefile and in the Kconfig
> help explaining that this is for PC/laptop style EC drivers,
> which combine multiple logical functions in one, only!
> 
> Assuming that we are only going to use this for such EC drivers,
> using the platform-driver-x86 mailinglist for this makes sense
> since that is where are the people are with knowledge of e.g.
> userspace APIs for various typical EC functionalities.
> 
> It might even make sense to also use:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git
> 
> As git tree for this and send pull-reqs to Linus for this
> together with the other pdx86 for the same reasons.
> 
> I would be open to that as long as this is strictly limited to
> EC (like) drivers.

Yes, I believe the EC are the only "boad-specific" drivers we need for
the Windows-on-Arm devices as of today. I expect at least two more EC
drivers to be added later.

Then I will re-target this series to platform-driver-x86:

- Will add a new drivers/platform/aarch64/ dir with a Makefile and Kconfig
  that would explicitly note it's only for EC-like drivers. Will update
  the "X86 PLATFORM DRIVERS" entry in MAINTAINERS. (Or should I add a new
  entry?)
- Will add this driver there, also updating per the last Sebastian's
  comments.
- Will also move the dt binding to a new bindings/platform/ dir.

Thanks!
Nikita

> 
> Ilpo, what do you think about this ?
> 
> Regards,
> 
> Hans
>