From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Subject: Re: [PATCH 1/3] thinkpad_acpi: add support for inhibit_charge Date: Mon, 14 May 2018 13:41:33 +0300 Message-ID: References: <20180513152937.GA5072@thinkpad> <20180514094620.wfoom4mwwyqb7qe5@localhost> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20180514094620.wfoom4mwwyqb7qe5@localhost> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ibm-acpi-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Ognjen Galic , Andy Shevchenko , "Rafael J. Wysocki" , "Rafael J. Wysocki" , Len Brown , Robert Moore , ACPI Devel Maling List , devel-E0kO6a4B6psdnm+yROfE0A@public.gmane.org, Darren Hart , Andy Shevchenko , Henrique de Moraes Holschuh , Sebastian Reichel , Platform Driver , ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, Linux PM , Kevin Locke List-Id: linux-acpi@vger.kernel.org T24gTW9uLCBNYXkgMTQsIDIwMTggYXQgMTI6NDYgUE0sIENocmlzdG9waCBCw7ZobXdhbGRlcgo8 Y2hyaXN0b3BoQGJvZWhtd2FsZGVyLmF0PiB3cm90ZToKPiBPbiBTdW4sIE1heSAxMywgMjAxOCBh dCAwNToyOTozN1BNICswMjAwLCBPZ25qZW4gR2FsaWMgd3JvdGU6Cj4+IExlbm92byBUaGlua1Bh ZCBzeXN0ZW1zIHN1cHBvcnQgdGhlIHByZXZlbnRpb24gb2YKPj4gYmF0dGVyeSBjaGFyZ2luZyB2 aWEgYSBtYW51YWwgb3ZlcnJpZGUgY2FsbGVkIEluaGliaXQgQ2hhcmdlLgo+Pgo+PiBUaGlzIHBh dGNoIGFkZHMgc3VwcG9ydCBmb3IgdGhhdCBhdHRyaWJ1dGUgYW5kIGV4cG9zZXMgaXQgdmlhIHRo ZQo+PiBiYXR0ZXJ5IEFDUEkgZHJpdmVyIGluIHRoZSBnZW5lcmljIGxvY2F0aW9uOgo+Pgo+PiAv c3lzL2NsYXNzL3Bvd2VyX3N1cHBseS9CQVRYL2luaGliaXRfY2hhcmdlCgo+PiArICAgICAgICAg ICAgIC8qIFdoZW4gc2V0dGluZyBpbmhiaXRpdCBjaGFyZ2UsIHdlIHNldCBhIGRlZmF1bHQgdmF1 bHVlIG9mCj4KPiBUaGlzIGNvbW1lbnQgZG9lcyBub3QgYWRoZXJlIHRvIHRoZSBMaW51eCBjb2Rp bmcgc3R5bGUKCldoaWxlIHlvdSBhcmUgcmlnaHQgaW4gcHJpbmNpcGxlLCB0aGUgd2hvbGUgZHJp dmVyIGlzIHNvIG9sZCBhbmQgdXNlcwp0aGlzIHN0eWxlLiBTbywgZm9yIHN1Y2ggY2FzZXMgd2Us IGFzIG1haW50YWluZXJzLCBwcmVmZXIgbGVzcwpkZXZpYXRpb24gd29yaywgaS5lLiBrZWVwaW5n IHRoZQpzdHlsZSBpcyBhIGdvb2QgdGhpbmcgdG8gZG8uCgo+PiArICAgICAgICAgICAgIC8qIFRo ZSBvbmx5IHZhbGlkIHZhbHVlcyBhcmUgMSBhbmQgMCAqLwo+PiArICAgICAgICAgICAgIGlmICh2 YWx1ZSAhPSAwICYmIHZhbHVlICE9IDEpCj4KPiBJJ20gbm90IHN1cmUsIGJ1dCBtYXliZSBgaWYg KHZhbHVlIDwgMilgIGlzIGJldHRlciBoZXJlPwoKU2luY2UgaXQncyBhYm91dCBpbnRlZ2VyLWFz LWEtYm9vbGVhbiwgdGVzdCBmb3IgYml0IDAgd291bGQgYmUKc3VmZmljaWVudCwgaS5lLiB+QklU KDApLiBUaG91Z2gsIEkgZmluZCB0aGlzIGZvcm0gbm90IHNvIHJlYWRhYmxlCnNpbmNlIHRoZSBp bnB1dCBjb21lcyBhY3R1YWxseSBmcm9tIHRoZSB1c2VyLgpJdCB3b3VsZCBiZSBuaWNlIHRvIGhh dmUganVzdCBrc3RydG9ib29sKCkgY2FsbGVkIGluc3RlYWQgZm9yIHN1Y2gKb3B0aW9ucywgYnV0 IHNlZSBhYm92ZS4gSXQgd291bGQgbmVlZCBhIChodWdlKSByZWZhY3RvcmluZyBvZiB0aGUKZHJp dmVyIGZpcnN0LgoKLS0gCldpdGggQmVzdCBSZWdhcmRzLApBbmR5IFNoZXZjaGVua28KCi0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLQpDaGVjayBvdXQgdGhlIHZpYnJhbnQgdGVjaCBjb21tdW5pdHkgb24g b25lIG9mIHRoZSB3b3JsZCdzIG1vc3QKZW5nYWdpbmcgdGVjaCBzaXRlcywgU2xhc2hkb3Qub3Jn ISBodHRwOi8vc2RtLmxpbmsvc2xhc2hkb3QKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KaWJtLWFjcGktZGV2ZWwgbWFpbGluZyBsaXN0CmlibS1hY3BpLWRl dmVsQGxpc3RzLnNvdXJjZWZvcmdlLm5ldApodHRwczovL2xpc3RzLnNvdXJjZWZvcmdlLm5ldC9s aXN0cy9saXN0aW5mby9pYm0tYWNwaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============2656347439644193420==" MIME-Version: 1.0 From: Andy Shevchenko Subject: Re: [Devel] [PATCH 1/3] thinkpad_acpi: add support for inhibit_charge Date: Mon, 14 May 2018 13:41:33 +0300 Message-ID: In-Reply-To: 20180514094620.wfoom4mwwyqb7qe5@localhost List-ID: To: devel@acpica.org --===============2656347439644193420== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On Mon, May 14, 2018 at 12:46 PM, Christoph B=C3=B6hmwalder wrote: > On Sun, May 13, 2018 at 05:29:37PM +0200, Ognjen Galic wrote: >> Lenovo ThinkPad systems support the prevention of >> battery charging via a manual override called Inhibit Charge. >> >> This patch adds support for that attribute and exposes it via the >> battery ACPI driver in the generic location: >> >> /sys/class/power_supply/BATX/inhibit_charge >> + /* When setting inhbitit charge, we set a default vaulue of > > This comment does not adhere to the Linux coding style While you are right in principle, the whole driver is so old and uses this style. So, for such cases we, as maintainers, prefer less deviation work, i.e. keeping the style is a good thing to do. >> + /* The only valid values are 1 and 0 */ >> + if (value !=3D 0 && value !=3D 1) > > I'm not sure, but maybe `if (value < 2)` is better here? Since it's about integer-as-a-boolean, test for bit 0 would be sufficient, i.e. ~BIT(0). Though, I find this form not so readable since the input comes actually from the user. It would be nice to have just kstrtobool() called instead for such options, but see above. It would need a (huge) refactoring of the driver first. -- = With Best Regards, Andy Shevchenko --===============2656347439644193420==--