Am 06.06.24 um 16:50 schrieb Thomas Weißschuh:
Introduce a mechanism for drivers to extend the properties implemented
by a power supply.
Motivation
----------
Various drivers, mostly in platform/x86 extend the ACPI battery driver
with additional sysfs attributes to implement more UAPIs than are
exposed through ACPI by using various side-channels, like WMI,
nonstandard ACPI or EC communication.
While the created sysfs attributes look similar to the attributes
provided by the powersupply core, there are various deficiencies:
* They don't show up in uevent payload.
* They can't be queried with the standard in-kernel APIs.
* They don't work with triggers.
* The extending driver has to reimplement all of the parsing,
formatting and sysfs display logic.
* Writing a extension driver is completely different from writing a
normal power supply driver.
* Properties can not be properly overriden.
The proposed extension API avoids all of these issues.
An extension is just a "struct power_supply_ext" with the same kind of
callbacks as in a normal "struct power_supply_desc".
The API is meant to be used via battery_hook_register(), the same way as
the current extensions.
For example my upcoming cros_ec charge control driver[0] saves 80 lines
of code with this patchset.
Contents
--------
* Patch 1 and 2 are generic preparation patches, that probably make
sense without this series.
* Patch 3 implements the extension API itself.
* Patch 4 implements a PoC locking scheme for the extension API.
* Patch 5 adds extension support to test_power.c
* Patch 6 converts the in-tree platform/x86/system76 driver to the
extension API.
Open issues
-----------
* Newly registered properties will not show up in hwmon.
To do that properly would require some changes in the hwmon core.
As far as I know, no current driver would extend the hwmon properties anyways.
* As this is only useful with the hooks of CONFIG_ACPI_BATTERY, should
it also be gated behind this or another config?
* Only one extension can be used at a time.
So far this should be enough, having more would complicate the
implementation.
* Is an rw_semaphore acceptable?
[0] https://lore.kernel.org/lkml/20240528-cros_ec-charge-control-v2-0-81fb27e1cff4@xxxxxxxxxxxxxx/
Nice, i love this proposal!
I agree that the hwmon update functionality will need some changes in the hwmon core to work,
but there would be at least one driver benefiting from this (dell-wmi-ddv). Maybe we can add
support for this at a later point in time.
The possibility of registering multiple power supply extensions on a single power supply will
be necessary to support battery charge control on Dell notebooks in the future. This is because
there will be two drivers on Dell notebooks which register battery extensions: dell-wmi-ddv and
dell-laptop (when support for battery charge control is supported someday).
How difficult would it be to support such scenarios? If its very difficult, then maybe we can implement
this later when the need arises.
Thanks,
Armin Wolf
Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
---
Thomas Weißschuh (6):
power: supply: sysfs: use power_supply_property_is_writeable()
power: supply: core: avoid iterating properties directly
power: supply: core: implement extension API
power: supply: core: add locking around extension access
power: supply: test-power: implement a power supply extension
platform/x86: system76: Use power_supply extension API
drivers/platform/x86/system76_acpi.c | 83 +++++++++---------
drivers/power/supply/power_supply.h | 9 ++
drivers/power/supply/power_supply_core.c | 136 ++++++++++++++++++++++++++++--
drivers/power/supply/power_supply_hwmon.c | 48 +++++------
drivers/power/supply/power_supply_sysfs.c | 39 ++++++---
drivers/power/supply/test_power.c | 102 ++++++++++++++++++++++
include/linux/power_supply.h | 25 ++++++
7 files changed, 357 insertions(+), 85 deletions(-)
---
base-commit: 2df0193e62cf887f373995fb8a91068562784adc
change-id: 20240602-power-supply-extensions-07d949f509d9
Best regards,
