platform-driver-x86.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Koch <linrunner@gmx.net>
To: "Hans de Goede" <hdegoede@redhat.com>,
	"Nicolò Piazzalunga" <nicolopiazzalunga@gmail.com>,
	"Mark Pearson" <markpearson@lenovo.com>,
	platform-driver-x86@vger.kernel.org,
	"Nitin Joshi1" <njoshi1@lenovo.com>,
	"Sebastian Reichel" <sre@kernel.org>,
	"Bastien Nocera" <hadess@hadess.net>,
	"Benjamin Berg" <bberg@redhat.com>
Cc: jwrdegoede@fedoraproject.org, smclt30p@gmail.com
Subject: Re: [PATCH 1/3] thinkpad_acpi: add support for force_discharge
Date: Wed, 29 Sep 2021 12:45:38 +0200	[thread overview]
Message-ID: <a6c448f7-d0c5-2652-46df-590085b47bd1@gmx.net> (raw)
In-Reply-To: <0eeb17bd-59f7-075e-f789-3814cc9a0505@redhat.com>

Hi Hans,


On 29.09.21 11:55, Hans de Goede wrote:

 > Hi,

 >

 > On 9/29/21 7:47 AM, Thomas Koch wrote:

 >> Hi Hans,

 >>

 >> On 27.09.21 17:12, Hans de Goede wrote:

 >>> Hi Nicolò,

 >>>

 >>> On 9/27/21 5:00 PM, Nicolò Piazzalunga wrote:

 >>>> Hi,

 >>>>

 >>>> On 9/27/21 3:59 PM, Mark Pearson wrote:

 >>>>> Do let me know if there are some important use cases for these so
I can go back to the FW team and discuss supporting them properly.

 >>>>

 >>>> The important use cases are force discharge and inhibit charge.

 >>>> These at present are dealt with using tpacpi-bat, which relies on
(out of tree) acpi_call.

 >>>> See also your previous reply.

 >>>

 >>> I can see how those can be useful in certain circumstances.

 >>>

 >>> I can also understand how Lenovo does not want these to be

 >>> available by default everywhere.

 >>>

 >>> I think a good compromise would be to add a bool module option

 >>> which defaults to false to enable these.

 >>

 >>  From the user perspective, I don't agree that this is a good

 >> compromise. Users simply want to recalibrate their battery.

 >

 > But can't they already do this by just unplugging the AC and

 > then let the machine die ?Of course, but who wants that?

 > How is this different, does it somehow magically flicks on the

 > AC just before the battery deep-discharge protection kicks in

 > so no unsaved work is lost ?

But this is exactly how it works. There is no risk of data loss.



The EC resets the force_discharge flag at the end of the discharge and
the ThinkPad is seamlessly switched back to AC power. Just magical. No
power interruptions.



Maybe now you understand why Linux users want this so badly?


 > I have never used this, but that seems unlikelyYet it works.

 > Or is the advantage of this that the AC being present results in

 > no nagging low battery warnings as well as no auto-shutdown

 > near the end ?

 >

 > If the advantage of this is indeed the no nagging + auto-shutdown

 > a better fix would be to do some userspace work to allow a

 > full, undisturbed, discharge while not on AC for calibration

 > purposes. That would also work on any vendor laptop, which would

 > be a big added bonus.

 >

 > I do believe that Lenovo does want to support doing a calibration

 > cycle under Linux. This whole thread got resurfaced / revived by

 > Mark because of discussions with Lenovo about offering better

 > battery management options under Linux.>> Having to

 >> set the module option beforehand is an unnecessary hurdle imho.

 >>

 >> Of course a module option again leads to support overhead in "user

 >> space". Then tlp-stat -b would have to inform the user that

 >> force_discharge is unfortunately not available, but he should try to set

 >> the module option. What would that be good for?

 >>

 >> While I respect Mark's official opinion, I would like to counter with my

 >> experience from 10 years of TLP development and support:

 >>

 >> The calls for force_discharge work unmodified since the 2012 ThinkPads

 >> (T420/X220) on all models that also support charge thresholds.

 >>

 >> They also work reliably, otherwise the issue tracker at tpacpi-bat and

 >> TLP would be full of user issues.

 >>

 >> inhibit_charge is probably used rather rarely, at least no TLP user has

 >> asked for it.

 >

 > Ok, so I think we should look at the end goal here, which is doing

 > a battery calibration run, which AFAIK means discharging until

 > the machine turns off (by itself rather then an emergency shutdown

 > by the OS), followed by charging to 100%/full in one go.


Fine. There you have it, the patch is the working kernel solution for
your end goal and even more.



'tlp recalibrate' already does the user space job

handling



/sys/class/power_supply/BATx/force_discharge = 1 (TLP 1.4)



and includes setting



/sys/class/power_supply/AC/charge_control_end_threshold = 100



for a full recharge.



Mark may initiate GUI support if he so desires.

 > So as mentioned before if we can disable the emergency shutdown

 > then we should be able to do this on all vendor laptops.

 >

 > But I guess the real question here is, what do the Lenovo tools

 > under Windows do?

 >

 > Mark, I assume that the Lenovo battery management tool under Windows

 > does allow doing a calibration, can you figure out how this works?

 >

 > Maybe that does actually use force_disharge and things got mixed up

 > when you asked the firmware team about all 3 features in one email,

 > and the other 2 are only for testing purposes?

 >

 >> @Mark: what is Lenovo's position on the calls for charge thresholds

 >> already included in thinkpad_acpi? Are they also internal?

 >

 > I'm not Mark, but I believe that these are officially supported,

 > as mentioned before Lenovo does want to offer better battery management

 > options under Linux.

 >

 > Regards,

 >

 > Hans

 >


Freundliche Grüße / Kind regards,

Thomas Koch



Mail : linrunner@gmx.net

Web  : https://linrunner.de/tlp


  reply	other threads:[~2021-09-29 10:45 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-17 14:01 [PATCH 1/3] thinkpad_acpi: add support for force_discharge Nicolo' Piazzalunga
2021-04-07 10:24 ` Hans de Goede
2021-04-07 10:33   ` Barnabás Pőcze
2021-04-08 13:51     ` Sebastian Reichel
2021-04-08 18:18       ` Thomas Koch
2021-04-09 18:33       ` Thomas Koch
2021-04-13  8:05         ` Hans de Goede
2021-04-17 11:49           ` Thomas Koch
2021-04-17 17:03             ` Hans de Goede
2021-05-19 14:45               ` Nicolo' Piazzalunga
2021-04-07 12:19   ` Thomas Koch
2021-04-07 17:48     ` [External] " Mark Pearson
     [not found]       ` <VI1PR09MB2302B7C3AD8014CC98D36AA595759@VI1PR09MB2302.eurprd09.prod.outlook.com>
2021-04-12 17:10         ` Mark Pearson
2021-09-27 13:59   ` Mark Pearson
2021-09-27 15:00     ` Nicolò Piazzalunga
2021-09-27 15:12       ` Hans de Goede
2021-09-27 16:50         ` [External] " Mark Pearson
2021-09-29  5:47         ` Thomas Koch
2021-09-29  9:55           ` Hans de Goede
2021-09-29 10:45             ` Thomas Koch [this message]
2021-09-29 10:56               ` Hans de Goede
2021-09-29 13:45                 ` [External] " Mark Pearson

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=a6c448f7-d0c5-2652-46df-590085b47bd1@gmx.net \
    --to=linrunner@gmx.net \
    --cc=bberg@redhat.com \
    --cc=hadess@hadess.net \
    --cc=hdegoede@redhat.com \
    --cc=jwrdegoede@fedoraproject.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).