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@xxxxxxxxxx> >> --- [...] > > 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
