Re: [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Thomas,

On 17.11.21 18:57, Thomas Weißschuh wrote:
On 2021-11-16 17:56+0100, Thomas Koch wrote:
thank you very much for working on this. It is high time that we leave
external kernel modules for ThinkPads behind us.

On 13.11.21 11:42, Thomas Weißschuh wrote:
Hi,

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, Nicolo' Piazzalunga, Thomas Koch:

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?
S-o-b/Co-developed-by/Tested-by is fine with me.

I tested your patches.

Hardware:

- ThinkPad X220, BAT0
- ThinkPad T450s, BAT0+BAT1
- ThinkPad X1C6, BAT0

Test Results:

1. force-discharge

Everythings works as expected
- Writing including disengaging w/ "auto" : OK
- Reading: OK

- Battery discharging: OK
- Disengaging with "auto": OK

2. inhibit-charge

Works as expected:
- Writing: OK

- Disengaging with "auto": OK


Discrepancies:
- Battery charge inhibited: BAT0 OK, BAT1 no effect e.g. continues charging
- Reading: always returns "auto"

I tested it on a T460s with two batteries and there inhibit-charge works
fine for both batteries.
What does not work is setting force-discharge for both batteries at the same
time.
This makes somewhat sense as on a physical level probably only one of them can
be used at a time.

My experience confirms your consideration. The ThinkPad battery circuit
can handle exactly one battery at a time i.e.
- Charging, AC connected
- Forced discharging,  AC connected
- Discharging, AC disconnected
The other battery is always idle during this time.

Mark Pearson: Could you confirm that this is the intended behaviour?

In my changes queued for v2 of the series[0] I added validation of the written
settings and an EIO is now reported if the settings were not applied, so this
should help userspace handle this situatoin.

The plan is to submit v2 after the first round of review for the core PM
changes.

Please wait until i'm finished with testing your queued v2. I am getting
errors here and would first like to rule out homemade problems with my
kernel build and/or base version.

[0] https://git.sr.ht/~t-8ch/linux/tree/charge-control


--
Freundliche Grüße / Kind regards,
Thomas Koch

Mail : linrunner@xxxxxxx
Web  : https://linrunner.de/tlp




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux