power: supply: bq27xxx: Return the value instead of -ENODATA

From: Hermes Zhang <chenhuiz@axis.com>

It might be better to return value (e.g. 65535) instead of an error
(e.g. No data available) for the time property.

Normally a common function will handle the read string and parse to
integer for all the properties, but will have problem when read the
time property because need to handle the NODATA error as non-error.
So it will make simple for application which indicate success when
read a number, otherwise as an error to handle.

Signed-off-by: Hermes Zhang <chenhuiz@axis.com>
---
 drivers/power/supply/bq27xxx_battery.c | 3 ---
 1 file changed, 3 deletions(-)

On Thursday 01 April 2021 01:51:28 Hermes Zhang wrote:
> On 3/31/21 10:02 PM, Pali Rohár wrote:
> > @@ -1655,9 +1655,6 @@ static int bq27xxx_battery_read_time(struct bq27xxx_device_info *di, u8 reg)
> >  		return tval;
> >  	}
> >  
> > -	if (tval == 65535)
> > -		return -ENODATA;
> > -
> >  	return tval * 60;
> > I'm not sure if this is correct change. If value 65535 is special which
> > indicates that data are not available then driver should not return
> > (converted) value 65535*60. If -ENODATA is there to indicate that data
> > are not available then -ENODATA should not be used.
> >
> > And if there is application which does not handle -ENODATA for state
> > when data are not available then it is a bug in application.
> 
> Yeah, I just have a feeling return -ENODATA for time_to_full/empty is
> not good here. Because:
> 
> 1. From chip datasheet, it mentioned return 65535 when it's not
> available (e.g. read time_to_full when discharging), but the driver
> changes behavior here.

So if chip reports that value is not available (by special value 65535)
then driver should not report that data are available with value 3932100s.

> 2. There is other case will return -ENODATA (e.g. the gauge not
> calibrated), so it will confuse application which is real failure.

In both cases, driver does not have (meaningful) value, so it reports
-ENODATA. If you think that some other error code is better for 2. then
we can discuss about it.

But -ENODATA seems a correct error code for reporting when data are not
available.

> Could we change the value in minute instead of seconds in
> bq27xxx_battery_read_time(), so that means driver do nothing but only
> pass the value from the chip?

No, according to API, driver must report value in seconds, not in
minutes, see documentation: Documentation/power/power_supply_class.rst

  All voltages, currents, charges, energies, time and temperatures in µV,
  µA, µAh, µWh, seconds and tenths of degree Celsius unless otherwise
  stated. It's driver's job to convert its raw values to units in which
  this class operates.

There is no API for reporting value in minutes.