On Sun, Oct 31, 2021 at 6:25 PM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxxx/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
