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

[Wolfgang Wiedmeyer]

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@xxxxxxxxxxxx>
>> ---
>>  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@xxxxxxxxxxxx
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE  048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc

Attachment: signature.asc
Description: PGP signature