lkml.org 
[lkml]   [2021]   [Oct]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [RFC 0/5] ACPI/power-suppy add fuel-gauge support on cht-wc PMIC without USB-PD support devs
    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

    \
     
     \ /
      Last update: 2021-10-31 20:51    [W:6.724 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site