All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Koch <linrunner@gmx.net>
To: "Thomas Weißschuh" <linux@weissschuh.net>
Cc: linux-pm@vger.kernel.org, Sebastian Reichel <sre@kernel.org>,
	ibm-acpi-devel@lists.sourceforge.net,
	platform-driver-x86@vger.kernel.org,
	Mark Gross <markgross@kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Henrique de Moraes Holschuh <hmh@hmh.eng.br>,
	linux-kernel@vger.kernel.org, bberg@redhat.com,
	hadess@hadess.net, markpearson@lenovo.com,
	nicolopiazzalunga@gmail.com, njoshi1@lenovo.com,
	smclt30p@gmail.com
Subject: Re: [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge)
Date: Wed, 17 Nov 2021 19:36:08 +0100	[thread overview]
Message-ID: <16d33b74-1b04-8b9e-7799-4f4b3a89c004@gmx.net> (raw)
In-Reply-To: <d1bc62e9-a5da-4c23-b31f-8ba718faf4a3@t-8ch.de>

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@gmx.net
Web  : https://linrunner.de/tlp

  parent reply	other threads:[~2021-11-17 18:36 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-13 10:42 [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Thomas Weißschuh
2021-11-13 10:42 ` [PATCH 1/4] power: supply: add charge_behaviour attributes Thomas Weißschuh
2021-11-13 10:42 ` [PATCH 2/4] power: supply: add helpers for charge_behaviour sysfs Thomas Weißschuh
2021-11-13 10:42 ` [PATCH 3/4] platform/x86: thinkpad_acpi: support force-discharge Thomas Weißschuh
2021-11-16 20:35   ` [External] " Mark Pearson
2021-11-13 10:42 ` [PATCH 4/4] platform/x86: thinkpad_acpi: support inhibit-charge Thomas Weißschuh
2021-11-16 10:58   ` Hans de Goede
2021-11-16 12:18     ` Thomas Weißschuh
2021-11-16 20:52   ` [External] " Mark Pearson
2021-11-16 23:44     ` Thomas Weißschuh
2021-11-17 15:09       ` Mark Pearson
2021-11-16 16:56 ` [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Thomas Koch
2021-11-17 17:57   ` Thomas Weißschuh
2021-11-17 18:20     ` [External] " Mark Pearson
2021-11-17 18:36     ` Thomas Koch [this message]
2021-11-23 23:27 Thomas Weißschuh
2021-12-03 21:33 ` Sebastian Reichel
2021-12-04 13:56   ` Hans de Goede
2021-12-16 15:41   ` Hans de Goede
2021-12-21 16:06   ` Hans de Goede
2021-12-04 16:04 ` Thomas Koch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=16d33b74-1b04-8b9e-7799-4f4b3a89c004@gmx.net \
    --to=linrunner@gmx.net \
    --cc=bberg@redhat.com \
    --cc=hadess@hadess.net \
    --cc=hdegoede@redhat.com \
    --cc=hmh@hmh.eng.br \
    --cc=ibm-acpi-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux@weissschuh.net \
    --cc=markgross@kernel.org \
    --cc=markpearson@lenovo.com \
    --cc=nicolopiazzalunga@gmail.com \
    --cc=njoshi1@lenovo.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=smclt30p@gmail.com \
    --cc=sre@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.