Date:   Mon, 1 Nov 2021 20:13:16 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Mark Gross <markgross@...nel.org>,
        Andy Shevchenko <andy@...radead.org>,
        Wolfram Sang <wsa@...-dreams.de>,
        Sebastian Reichel <sre@...nel.org>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Ard Biesheuvel <ardb@...nel.org>, Len Brown <lenb@...nel.org>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        Yauhen Kharuzhy <jekhor@...il.com>,
        Tsuchiya Yuto <kitakar@...il.com>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        linux-i2c <linux-i2c@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-efi <linux-efi@...r.kernel.org>
Subject: Re: [RFC 0/5] ACPI/power-suppy add fuel-gauge support on cht-wc PMIC
 without USB-PD support devs

Hi,

On 10/31/21 20:49, Andy Shevchenko wrote:
> On Sun, Oct 31, 2021 at 6:25 PM Hans de Goede <hdegoede@...hat.com> wrote:
>>
>> Hi All,
>>
>> Together with my earlier series to hookup the charger, Vbus boost converter
>> and USB role-switching:
>> https://lore.kernel.org/platform-driver-x86/20211030182813.116672-1-hdegoede@redhat.com/T/#t
>>
>> This series also adds battery-monitoring support on the Xiaomi Mi Pad 2
>> and the generic parts of it should also be usable on other devices with
>> the same PMIC setup.
>>
>> I've marked this series as a RFC because I'm not happy about the amount of
>> DMI quirks this series requires. The 3 separate quirks in
>> drivers/acpi/x86/utils.c are a bit much, but esp. when combined with also
>> the changes needed in drivers/gpio/gpiolib-acpi.c it all becomes a bit too
>> much special casing for just a single device.
>>
>> So I've been thinking about alternatives for this and I've come up with
>> 3 ways to deal with this:
>>
>> 1. This patch set.
>>
>> 2. Instead of the quirks in drivers/acpi/x86/utils.c, write an old-fashioned
>> "board" .c file/module which autoloads based on a DMI match and manually
>> instantiates i2c-clients for the BQ27520 fuel-gauge and the KTD20260 LED ctrlr.
>> Combined with not giving an IRQ to the fuel-gauge i2c-client (i), this allows
>> completely dropping the gpiolib-acpi.c changes and only requires 1 quirk for
>> the 2nd PWM controller in drivers/acpi/x86/utils.c. As an added bonus this
>> approach will also removes the need to add ACPI enumeration support to the
>> bq27xxx_battery code.
>>
>> 3. While working on this I noticed that the Mi Pad 2 DSDT actually has
>> full ac and battery ACPI code in its DSDT, which Linux was not trying to
>> use because of the Whiskey Cove PMIC ACPI HID in acpi_ac_blacklist[] in
>> drivers/apci/ac.c, resp. a missing _DEP for the ACPI battery.
>>
>> With the native drivers disabled (the default in 5.15-rc7 without patches),
>> both those things fixed and a fix to intel_pmic_regs_handler() in
>> drivers/acpi/pmic/intel_pmic.c, battery monitoring actually starts working
>> somwhat!
>>
>> I say somewhat because changes are not detected until userspace polls
>> the power_supply and switching from charge/device to host mode and
>> back does not work at all. This is due to the AML code for this relying
>> on _AEI ACPI events on virtual GPIOs on the PMIC :|  This means that we
>> would need to reverse engineer which events these virtual GPIO interrupts
>> represent; and then somehow rework the whole MFD + child driver setup
>> to deliver, e.g. extcon/pwrsrc events to a to-be-written GPIO driver
>> which supports these virtual GPIOs, while at the same time also keeping
>> normal native driver support since boards which USB-PD support need the
>> native drivers...  So OTOH this option has the promise of solving this
>> in a generic way which may work on more boards, OTOH it is a big mess
>> and we lack documentation for it.  Interestingly enough the ACPI
>> battery/ac code also takes ownership of the notification LED, downgrading
>> it from a full RGB led to a green charging LED, which is both a pre
>> and a con at the same time (since we would loose full RGB function).
>>
>> ###
> 
>> Although I started out with implementing option 1, I now think I
>> Would personally prefer option 2. This isolates most of the code
>> needed to support some of these special boards into a single
>> (per board) file which can be build as a module which can be
>> autoloaded, rather then growing vmlinuz by adding quirks there.
> 
> Even before reading this my attention was on option 2 as well.

Its good to hear that you think this is likely the best option too.
I hope to send out another RFC patch-series taking this approach
instead soon.

> However, we might give another round of searching the documentation
> for the vGPIO lines.

Having those would be good regardless.

> Meanwhile, have you tried to see if Android tree(s) has(ve) the
> patches related to all this? (I'm a bit sceptical they do the right
> thing and most probably just fall into board files case)

The Android code takes the native driver path, when the EFI firmware
sees it is about to exec Xiaomi's Android bootloader (I think it
checks the signature) it sets OSID = 0x04 which makes all the
ACPI devices which patch 1/5 of this RFC makes "always_present"
return 0xf from their _STA method and it disables the troublesome
_AEI handler too when OSID==4.

So basically it does everything which this RFC series does with
quirks automatically correct based on the OSID. But we cannot
influence this ourselves, there is a BIOS option for it, but
that gets overridden by OS autodetect code at boot.

The fact that the Android on the tablet also goes the use
native charger + fuel-gauge drivers route does to me is a further
hint that using native drivers is probably the right thing to do
(OTOH some of the code in the Android port the device ships with
 is not so great).

Regards,

Hans

Powered by blists - more mailing lists