Re: [PATCH v3 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups
From: Sebastian Reichel <hidden>
Date: 2021-08-05 17:19:56
Hi Hans, On Sun, Aug 01, 2021 at 03:30:55PM +0200, Hans de Goede wrote:
Hi all, Changes in v3: - Replace "depends on X86" with "depends on IOSF_MBI" as the new code uses symbols which are only defined when IOSF_MBI support is enabled. Depending on this is ok since IOSF_MBI support should always be enabled on devices which use the AXP288 PMIC. Changes in v2: - Add a "depends on X86" to Kconfig since the iosf_mbi functions are X86 only (the AXP288 PMIC is only used on X86 devices). And here is the v1 cover-letter again: The I2C-bus to the XPower AXP288 is shared between the Linux kernel and the SoCs P-Unit. The P-Unit has a semaphore which the kernel must "lock" before it may use the bus and while the kernel holds the semaphore the CPU and GPU power-states must not be changed otherwise the system will freeze. This is a complex process, which is quite expensive. To ensure that no unguarded I2C-bus accesses happen, the semaphore is taken by the I2C-bus-driver for every I2C transfer. When upower refreshes its battery stats it reads all the power-supply properties at once, leading to the semaphore getting hammered which sometimes causes the system to hang. Andrejus maintains a large "fleet" of affected Cherry Trail tablets and was seeing these hangs semi regularly. After discussing this with me Andrejus wrote the caching patch in this series which greatly reduces the number of semaphore accesses and since then there have been no reports of hangs in the fleet of devices which he maintains. I've cleaned up Andrejus work a bit before submitting it upstream and while working on this I found a slew of other issues in this driver which bugged me enough to write a bunch of cleanup patches. I've also added some extra patches to also reduce the semaphore use during driver probe.
Thanks, I applied the whole series. The double goto from the last patch is a bit ugly and can be avoided by switching the driver to devm_*(). Do you mind doing so? -- Sebastian
Regards,
Hans
Andrejus Basovas (1):
power: supply: axp288_fuel_gauge: Refresh all registers in one go
Hans de Goede (9):
power: supply: axp288_fuel_gauge: Fix define alignment
power: supply: axp288_fuel_gauge: Remove debugfs support
power: supply: axp288_fuel_gauge: Silence the chatty IRQ mapping code
power: supply: axp288_fuel_gauge: Report register-address on readb /
writeb errors
power: supply: axp288_fuel_gauge: Drop retry logic from
fuel_gauge_reg_readb()
power: supply: axp288_fuel_gauge: Store struct device pointer in
axp288_fg_info
power: supply: axp288_fuel_gauge: Only read PWR_OP_MODE,
FG_LOW_CAP_REG regs once
power: supply: axp288_fuel_gauge: Move the AXP20X_CC_CTRL check
together with the other checks
power: supply: axp288_fuel_gauge: Take the P-Unit semaphore only once
during probe()
drivers/power/supply/Kconfig | 2 +-
drivers/power/supply/axp288_fuel_gauge.c | 489 +++++++++--------------
2 files changed, 187 insertions(+), 304 deletions(-)
--
2.31.1 Attachments
- signature.asc [application/pgp-signature] 833 bytes