Hi, On 12/3/21 22:33, Sebastian Reichel wrote: > Hi, > > On Wed, Nov 24, 2021 at 12:27:00AM +0100, Thomas Weißschuh wrote: >> This series adds support for the charge_behaviour property to the power >> subsystem and thinkpad_acpi driver. >> >> As thinkpad_acpi has to use the 'struct power_supply' created by the generic >> ACPI driver it has to rely on custom sysfs attributes instead of proper >> power_supply properties to implement this property. >> >> Patch 1: Adds the power_supply documentation and basic public API >> Patch 2: Adds helpers to power_supply core to help drivers implement the >> charge_behaviour attribute >> Patch 3: Adds support for force-discharge to thinkpad_acpi. >> Patch 4: Adds support for inhibit-discharge to thinkpad_acpi. >> >> Patch 3 and 4 are largely taken from other patches and adapted to the new API. >> (Links are in the patch trailer) >> >> Ognjen Galic: >> >> Your S-o-b is on the original inhibit_charge and force_discharge patches. >> I would like to add you as Co-developed-by but to do that it will also require >> your S-o-b. Could you give your sign-offs for the new patches, so you can be >> properly attributed? >> >> Sebastian Reichel: >> >> Currently the series does not actually support the property as a proper >> powersupply property handled fully by power_supply_sysfs.c because there would >> be no user for this property. > > I'm not too happy how the acpi-battery hooks work, but that's not > your fault and this patchset does not really make the situation > worse. So: > > Acked-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> I haven't looked at the thinkpad_apci.c bits closely yet (for this new version), but assuming those are ready for merging too, we need to discuss about how to merge this. The thinkpad_acpi code has already seen quite a lot of changes in -next, so I would like the thinkpad_acpi changes to go upstream through the platform-drivers-x86.git tree to avoid conflicts. As such I think it is best if you (Sebastian) can prepare an immutable branch with patch 1 + 2 for me to merge. Then even if patch 3 + 4 need more work, Thomas can just respin those on top of the immutable branch. Alternatively I can take the entire series upstream through the platform-drivers-x86.git tree if that is ok with you (Sebastian). Either way please let me know how you want to proceed with this. Regards, Hans >> Previous discussions about the API: >> >> https://lore.kernel.org/platform-driver-x86/20211108192852.357473-1-linux@xxxxxxxxxxxxxx/ >> https://lore.kernel.org/platform-driver-x86/21569a89-8303-8573-05fb-c2fec29983d1@xxxxxxxxx/ >> >> v1: https://lore.kernel.org/lkml/20211113104225.141333-1-linux@xxxxxxxxxxxxxx/ >> v1 -> v2: >> >> * Use sysfs_emit-APIs instead of plain sprintf >> * More cecks for actual feature availability >> * Validation of the written values >> * Read inhibit-charge via BICG instead of PSSG (peak shift state) >> * Don't mangle error numbers in charge_behaviour_store() >> >> Open points: >> >> Thomas Koch has observed that on a T450s with two batteries >> inhibit-charge on BAT0 will affect both batteries and for BAT1 it is ignored >> entirely, this seems to be a bug in the EC. >> On my T460s with two batteries it works correctly. >> >> Thomas Weißschuh (4): >> power: supply: add charge_behaviour attributes >> power: supply: add helpers for charge_behaviour sysfs >> platform/x86: thinkpad_acpi: support force-discharge >> platform/x86: thinkpad_acpi: support inhibit-charge >> >> Documentation/ABI/testing/sysfs-class-power | 14 ++ >> drivers/platform/x86/thinkpad_acpi.c | 191 +++++++++++++++++++- >> drivers/power/supply/power_supply_sysfs.c | 51 ++++++ >> include/linux/power_supply.h | 16 ++ >> 4 files changed, 268 insertions(+), 4 deletions(-) >> >> >> base-commit: 66f4beaa6c1d28161f534471484b2daa2de1dce0 >> -- >> 2.34.0 >>
