From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ognjen =?utf-8?B?R2FsacSH?= Subject: Re: [PATCH 1/3] thinkpad_acpi: add support for inhibit_charge Date: Mon, 14 May 2018 15:38:43 +0200 Message-ID: <20180514133843.GB6417@thinkpad> References: <20180513152937.GA5072@thinkpad> <20180514094620.wfoom4mwwyqb7qe5@localhost> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ibm-acpi-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Andy Shevchenko Cc: Platform Driver , "Rafael J. Wysocki" , Henrique de Moraes Holschuh , Linux PM , "Rafael J. Wysocki" , Robert Moore , Sebastian Reichel , ACPI Devel Maling List , Kevin Locke , Darren Hart , Len Brown , ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, Andy Shevchenko , devel-E0kO6a4B6psdnm+yROfE0A@public.gmane.org List-Id: linux-acpi@vger.kernel.org On Mon, May 14, 2018 at 01:41:33PM +0300, Andy Shevchenko wrote: > On Mon, May 14, 2018 at 12:46 PM, Christoph B=F6hmwalder > 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. That's what I did, follow the driver style. > = > >> + /* 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). = That seems uncessarily complicated. Whats wrong with LT and GT operators? > Though, I find this form not so readable > since the input comes actually from the user. I agree. > 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. That needs a whole lot of refactoring for no real functional benefit. > = > -- = > 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