On Mon, 14 May 2018, Christoph Böhmwalder wrote:
> > + case INHIBIT_CHARGE:
> > + if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, ret,
> > battery))
> > + return -ENODEV;
> > +
> > + /* The inhibit charge status is in the first bit */
> > + *ret = *ret & 0x01;
> > + return 0;
Do we know what is in the other bits? If so, please document the ACPI
method using a comment somewhere in the driver code, like you did for
SET_INHIBIT.
> > default:
> > pr_crit("wrong parameter: %d", what);
> > return -EINVAL;
> > @@ -9343,6 +9357,21 @@ static int tpacpi_battery_set(int what, int battery,
> > int value)
> > return -ENODEV;
> > }
> > return 0;
> > + case INHIBIT_CHARGE:
> > + /* When setting inhbitit charge, we set a default vaulue of
>
> This comment does not adhere to the Linux coding style
Much on the driver doesn't, because it is _OLD_. But yeah, it is
preferrable to fix this as we add code, so it would be good to have all
new (and modified) comments switched to modern kernel style.
> > + case INHIBIT_CHARGE:
> > + if (!battery_info.batteries[battery].inhibit_support)
> > + return -ENODEV;
> > + /* 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?
Indeed... with a comment that says 0 = main battery, 1 = extra/dock
battery or something.
--
Henrique Holschuh
------------------------------------------------------------------------------
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
