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: <87d1jqx0r0.fsf@machinist.wiedmeyer.de>
Date:   Mon, 26 Sep 2016 15:13:23 +0200
From:   Wolfgang Wiedmeyer <wolfgit@...dmeyer.de>
To:     Krzysztof Kozlowski <krzk@...nel.org>
Cc:     Wolfgang Wiedmeyer <wolfgit@...dmeyer.de>, 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


Krzysztof Kozlowski writes:

> 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?

I'm not able to test other applications than the Galaxy S3.

> 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?

Ok, I'll include a check if maxim,rsns-microohm exists and do the
fallback to VFSOC if it's not there.

Thanks,
Wolfgang

-- 
Website: https://fossencdi.org
Jabber: wolfgang@...dmeyer.de
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE  048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc

Download attachment "signature.asc" of type "application/pgp-signature" (819 bytes)