linux-kernel - Re: [PATCH] power: supply: bq27xxx: Return the value instead of -ENODATA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3acaabf2ec63418891e3f698c16b9a21@XBOX01.axis.com>
Date:   Thu, 1 Apr 2021 01:51:28 +0000
From:   Hermes Zhang <Hermes.Zhang@...s.com>
To:     Pali Rohár <pali@...nel.org>,
        Hermes Zhang <Hermes.Zhang@...s.com>
CC:     "lars@...afoo.de" <lars@...afoo.de>,
        Sebastian Reichel <sre@...nel.org>, kernel <kernel@...s.com>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] power: supply: bq27xxx: Return the value instead of
 -ENODATA

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.

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

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?

Best Regards,

Hermes