[bug report] power: supply: generic-adc-battery: Use GPIO descriptors

[bug report] power: supply: generic-adc-battery: Use GPIO descriptors

[Dan Carpenter]

Hello Linus Walleij,

The patch b0327ffb133f: "power: supply: generic-adc-battery: Use GPIO
descriptors" from Oct 30, 2020, leads to the following static checker
warning:

	drivers/power/supply/generic-adc-battery.c:97 gab_charge_finished()
	warn: signedness bug returning '(-4095)'

drivers/power/supply/generic-adc-battery.c
    93  static bool gab_charge_finished(struct gab *adc_bat)
    94  {
    95          if (!adc_bat->charge_finished)
    96                  return false;
    97          return gpiod_get_value(adc_bat->charge_finished);

The gpiod_get_value() can return negative error codes so it's not
necessarily clear if that should translate to a true as it does now or
to a false...

    98  }

regards,
dan carpenter

Re: [bug report] power: supply: generic-adc-battery: Use GPIO descriptors

[Linus Walleij]

On Fri, Dec 4, 2020 at 3:53 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:

> The patch b0327ffb133f: "power: supply: generic-adc-battery: Use GPIO
> descriptors" from Oct 30, 2020, leads to the following static checker
> warning:
>
>         drivers/power/supply/generic-adc-battery.c:97 gab_charge_finished()
>         warn: signedness bug returning '(-4095)'
>
> drivers/power/supply/generic-adc-battery.c
>     93  static bool gab_charge_finished(struct gab *adc_bat)
>     94  {
>     95          if (!adc_bat->charge_finished)
>     96                  return false;
>     97          return gpiod_get_value(adc_bat->charge_finished);
>
> The gpiod_get_value() can return negative error codes so it's not
> necessarily clear if that should translate to a true as it does now or
> to a false...

It *can* do that but in 9999 cases out of 10000 it does not. Mostly
because it just boils down to readin a register and masking a bit
which does not fail. This only happens in the rare circumstance that
the GPIO is on some slow bus that lost contact with the bridged
device (such as an I2C or SPI chip).

I agree it is not a perfect (in the mathematical sense) API but
in most cases the odd case of this call returning negative is
ignored and there are thousands of in-kernel users.

If this is a real problem I would rather turn the call into a bool
and ignore errors on the few that care to test for them than trying
to augment all sites using this call to handle negative
return codes.

Yours,
Linus Walleij