linux-kernel - Re: [PATCH 1/2] power: supply: max17042_battery: use VF SOC register for capacity property

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: <20160926111615.GB8140@kozik-lap>
Date:   Mon, 26 Sep 2016 13:16:15 +0200
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Wolfgang Wiedmeyer <wolfgit@...dmeyer.de>
Cc:     sre@...nel.org, dbaryshkov@...il.com, dwmw2@...radead.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] power: supply: max17042_battery: use VF SOC register
 for capacity property

On Sun, Sep 25, 2016 at 11:10:10PM +0200, Wolfgang Wiedmeyer wrote:
> The capacity property uses the RepSOC register to report the current state
> of charge. This register did not provide a reliable SOC value during my
> testing with the max17047 variant on a Galaxy S3 (Trats2/GT-I9300). The
> reported value did not change or even stayed zero in some cases.
> However, the VF SOC register provided an accurate SOC value at all times.
> It uses the voltage fuel gauge to determine the SOC.
> 
> Signed-off-by: Wolfgang Wiedmeyer <wolfgit@...dmeyer.de>
> ---
>  drivers/power/max17042_battery.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c
> index da7a75f..20cb1fd 100644
> --- a/drivers/power/max17042_battery.c
> +++ b/drivers/power/max17042_battery.c
> @@ -246,7 +246,7 @@ static int max17042_get_property(struct power_supply *psy,
>  		val->intval = data * 625 / 8;
>  		break;
>  	case POWER_SUPPLY_PROP_CAPACITY:
> -		ret = regmap_read(map, MAX17042_RepSOC, &data);
> +		ret = regmap_read(map, MAX17042_VFSOC, &data);
>  		if (ret < 0)
>  			return ret;

The RepSOC is for ModelGauge m3 which requires current sense resistor. I
don't remember whether the resistor is present on Trats2. If not, then
m1 is used. However in both cases (m1 and m3) the battery
characteristics (cell information) should be loaded which in case of DT
driver is not supported.

Overall, I am not sure whether your change is correct. It might fix this
particular scenario because:
1. We are not providing the cell information,
2. We mre not providing the SNS resistor value so we are in m1 mode (if
there is no SNS resistor).  but it might break other applications where
SNS is present and cell configuration is provided. Unless you tested it
in such?

Probably this should be based on Device Tree property describing what is
configured (e.g. missing model data). Maybe existing maxim,rsns-microohm
could be used - in case of lack of it, fall back to reading VFSOC?

Best regards,
Krzysztof