Re: [PATCH RESEND v2 3/3] power: supply: Introduce MM8013 fuel gauge driver

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 13.09.2023 17:45, Sebastian Reichel wrote:
> Hi,
> 
> On Wed, Aug 23, 2023 at 04:36:15PM +0200, Konrad Dybcio wrote:
>> Add a driver for the Mitsumi MM8013 fuel gauge. The driver is a vastly
>> cleaned up and improved version of the one that shipped in some obscure
>> Lenovo downstream kernel [1], with some register definitions borrowed from
>> ChromeOS EC platform code [2].
>>
>> [1] https://github.com/adazem009/kernel_lenovo_bengal/commit/b6b346427a871715709bd22aae449b9383f3b66b
>> [2] https://chromium.googlesource.com/chromiumos/platform/ec/+/master/driver/battery/mm8013.h
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
[...]

> 
> Please use regmap.
Ack

[...]

>> +	switch (psp) {
>> +	case POWER_SUPPLY_PROP_CAPACITY:
> 
> this is in %, while the next two are in uAh. So the fuel gauge does
> not provide the current capacity in uAh
> (POWER_SUPPLY_PROP_CHARGE_NOW)?
Yes. Doesn't seem like raw values are supported.

[...]

> 
> this is just 'val->intval = (s16) ret;'.
> 
>> +		val->intval *= -1000;
Ack

> 
> and this seems to be the only property, that properly scales its
> values, assuming the hardware reports data in mA.
> 

[...]

> 
> that's in **micro** volts
Oh, I didn't read enough docs..

> 
>> +		ret = mm8013_read_reg(client, REG_VOLTAGE);
> 
> this effectively does i2c_smbus_read_word_data() and thus the
> maximum is is 65536. 65536uV = 65mV. I have serious doubts, that
> this code does what you want. The same is true for a couple of
> the other properties.
Ack

[...]

> With the next submission please include a dump of the uevent
> in sysfs in the cover letter or below the fold, so that its
> easy to validty check if the reported values look sensible.
State of what-will-be-sent in v(n+1), with additional fixups:

POWER_SUPPLY_NAME=mm8013
POWER_SUPPLY_TYPE=Battery
POWER_SUPPLY_CAPACITY=100
POWER_SUPPLY_CHARGE_FULL=7124
POWER_SUPPLY_CHARGE_FULL_DESIGN=7500
POWER_SUPPLY_CURRENT_NOW=-122000
POWER_SUPPLY_CYCLE_COUNT=27
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_STATUS=Full
POWER_SUPPLY_TEMP=324
POWER_SUPPLY_VOLTAGE_NOW=4407000


> 
> Thanks and sorry for the slow processing,
No worries

Konrad