All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Ognjen Galic <smclt30p-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Andy Shevchenko
	<andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"Rafael J. Wysocki"
	<rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"Rafael J. Wysocki" <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>,
	Len Brown <lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Robert Moore
	<robert.moore-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	ACPI Devel Maling List
	<linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	devel-E0kO6a4B6psdnm+yROfE0A@public.gmane.org,
	Darren Hart <dvhart-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
	Andy Shevchenko <andy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
	Henrique de Moraes Holschuh
	<ibm-acpi-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>,
	Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Platform Driver
	<platform-driver-x86-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	Linux PM <linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Kevin Locke <kevin-yFOG++GfaLUI8/NkjqMgMw@public.gmane.org>
Subject: Re: [PATCH 1/3] thinkpad_acpi: add support for inhibit_charge
Date: Mon, 14 May 2018 13:41:33 +0300	[thread overview]
Message-ID: <CAHp75VdPOH7J1X4ZXmRsQ1yrB5n5RRmhPneWOnu7TFOAysgE6Q@mail.gmail.com> (raw)
In-Reply-To: <20180514094620.wfoom4mwwyqb7qe5@localhost>

On Mon, May 14, 2018 at 12:46 PM, Christoph Böhmwalder
<christoph@boehmwalder.at> 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 != 0 && value != 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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andy.shevchenko at gmail.com>
To: devel@acpica.org
Subject: Re: [Devel] [PATCH 1/3] thinkpad_acpi: add support for inhibit_charge
Date: Mon, 14 May 2018 13:41:33 +0300	[thread overview]
Message-ID: <CAHp75VdPOH7J1X4ZXmRsQ1yrB5n5RRmhPneWOnu7TFOAysgE6Q@mail.gmail.com> (raw)
In-Reply-To: 20180514094620.wfoom4mwwyqb7qe5@localhost

[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]

On Mon, May 14, 2018 at 12:46 PM, Christoph Böhmwalder
<christoph(a)boehmwalder.at> 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 != 0 && value != 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

  reply	other threads:[~2018-05-14 10:41 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-13 15:29 [PATCH 1/3] thinkpad_acpi: add support for inhibit_charge Ognjen Galic
2018-05-14  9:46 ` Christoph Böhmwalder
2018-05-14 10:41   ` Andy Shevchenko [this message]
2018-05-14 10:41     ` [Devel] " Andy Shevchenko
     [not found]     ` <CAHp75VdPOH7J1X4ZXmRsQ1yrB5n5RRmhPneWOnu7TFOAysgE6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-05-14 13:38       ` Ognjen Galić
2018-05-14 11:39   ` Henrique de Moraes Holschuh
     [not found]     ` <20180514113928.dnsxjqov7l6rvorb-ZGHd14iZgfaRjzvQDGKj+xxZW9W5cXbT@public.gmane.org>
2018-05-14 13:40       ` Ognjen Galić
2018-05-14 15:41         ` Henrique de Moraes Holschuh
2018-05-14 13:36   ` Ognjen Galić

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=CAHp75VdPOH7J1X4ZXmRsQ1yrB5n5RRmhPneWOnu7TFOAysgE6Q@mail.gmail.com \
    --to=andy.shevchenko-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=andy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
    --cc=devel-E0kO6a4B6psdnm+yROfE0A@public.gmane.org \
    --cc=dvhart-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
    --cc=ibm-acpi-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org \
    --cc=ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    --cc=kevin-yFOG++GfaLUI8/NkjqMgMw@public.gmane.org \
    --cc=lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=platform-driver-x86-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org \
    --cc=robert.moore-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=smclt30p-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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.