Re: [PATCH 1/3] thinkpad_acpi: add support for inhibit_charge

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

 



On Mon, May 14, 2018 at 12:46 PM, Christoph Böhmwalder
<christoph@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel




[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux