Re: [RFC 0/5] ACPI/power-suppy add fuel-gauge support on cht-wc PMIC without USB-PD support devs

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Sun, Oct 31, 2021 at 6:25 PM Hans de Goede <hdegoede@redhat.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.
However, we might give another round of searching the documentation
for the vGPIO lines.

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 downside would be this sorta re-introduces the old ARM model
> of one board file per (special-case) board, but there are only
> 1 or 2 more x86 tablets (ii) that I know about which may also
> need such a board file. Which I think is managable and should
> not run into the original objections against the original ARM
> approach where there were way too many board files in the end.
>
> Option 3 IMHO is a no go unless someone at Intel manages to
> come up with documentation on all the virtual GPIOs which the
> Windows PMIC drivers implement as method of communicating
> between the PMIC driver and the AML code in the DSDT.
>
> I'm a bit in dubio about how to progress with this, so I would
> love to hear what others think about this. I would esp. appreciate
> Rafael's and Mika's input on this since most of the added ugliness
> in this RFC is in the ACPI code.
>
> Regards,
>
> Hans
>
>
> i) This means that the _AEI ACPI handler for the fuel-gauge will run on
> FG interrupts. This is fine it does a single I2C read and a couple of
> ACPI notifies which will get ignored. Note the interrupts are "something
> changed" pulses which don't need IRQ clearing.
>
> ii) There are not that many CHT boards with a Whiskey Cove PMIC, other
> then the GPD win/pocket with full USB-PD support and the Xiaomi Mi Pad 2
> I'm only aware of one other, the Lenovo Yoga Book YB1-X91L/F . Since this
> whole saga has gotten me quite curious and I already have the other
> 2 devices I've decided to spend some money on this and bought a 2nd hand
> Lenovo Yoga Book YB1-X91L, whose setup is similar to the Mi Pad 2.
> I should have this in about a week. I'll post a reply to this thread
> with info no how the DSDT looks on the Lenovo Yoga Book and if e.g.
> using the standard ACPI battery interface seems to be an option there.
>
> iii) Note the "power: supply: bq27xxx: Add dev helper variable to
> bq27xxx_battery_i2c_probe()" patch applies on top of the
> "power: supply: bq27xxx: Fix kernel crash on IRQ handler register error"
> bug-fix which I send out earlier.

-- 
With Best Regards,
Andy Shevchenko