On Wed, Jan 03, 2018 at 04:42:35PM +0200, Andy Shevchenko wrote: > On Wed, Jan 3, 2018 at 1:59 PM, Ognjen Galic <smclt30p@xxxxxxxxx> wrote: > > thinkpad_acpi registers two new attributes for each battery: > > > > 1) Charge start threshold > > /sys/class/power_supply/BATN/charge_start_threshold > > > > Valid values are [0, 99]. A value of 0 turns off the > > start threshold wear control. > > > > 2) Charge stop threshold > > /sys/class/power_supply/BATN/charge_stop_threshold > > > > Valid values are [1, 100]. A value of 100 turns off > > the stop threshold wear control. This must be > > configured first. > > > > Signed-off-by: Ognjen Galic <smclt30p@xxxxxxxxx> > > Thanks for an update! > > > --- > > > > Notes: > > > * Change int to acpi_status in tpacpi_battery_acpi_eval > > Then you need to check return code with ACPI_FAILURE() or ACPI_SUCCESS() macros. > That is one of the additional burden while I can't see usefulness of > ACPI return codes in that function and why I suggested to use plain > int. Thos macros basically do the same stuff I do manually. I think that most of the kernel does errors on a non-0 basis, and ACPI is no exception. That being said, I think that using a simple non-0 check if more clean than using those macros. Isn't this: if (!acpi_eval.... nicer than this? if ACPI_FAILURE(acpi_eval... It just adds more stuff to get wrong. And I don't really think that the ACPI implementation will change those macros anytime soon, or ever. Keeping it simple. > > One more question: why don't you use dev_err()/dev_*() macros instead > of pr_*() ones? > (Note, it's a question, needs a bit of discussion, I would like to > hear a rationale of this, I think it might be one) Techically, I'm not adding a device this I did not use those macros. I'm only adding more attributes to a device that the battery module manages. > > -- > With Best Regards, > Andy Shevchenko
