linux-kernel - Re: [PATCH v4 2/5] power: supply: add support for max77759 fuel gauge

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] [day] [month] [year] [list]

Message-ID: <793c02e4-fa7a-4c13-bfc3-bb058e211b5f@uclouvain.be>
Date: Wed, 25 Jun 2025 14:12:21 +0200
From: Thomas Antoine <t.antoine@...ouvain.be>
To: Sebastian Reichel <sebastian.reichel@...labora.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Dimitri Fedrau <dima.fedrau@...il.com>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Peter Griffin <peter.griffin@...aro.org>,
 André Draszik <andre.draszik@...aro.org>,
 Tudor Ambarus <tudor.ambarus@...aro.org>,
 Alim Akhtar <alim.akhtar@...sung.com>, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org
Subject: Re: [PATCH v4 2/5] power: supply: add support for max77759 fuel gauge

Hi,

On 6/22/25 11:26 PM, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, May 23, 2025 at 02:51:45PM +0200, Thomas Antoine via B4 Relay wrote:
>> From: Thomas Antoine <t.antoine@...ouvain.be>
>>
>> The interface of the Maxim MAX77759 fuel gauge has a lot of common with the
>> Maxim MAX1720x. A major difference is the lack of non-volatile memory
>> slave address. No slave is available at address 0xb of the i2c bus, which
>> is coherent with the following driver from google: line 5836 disables
>> non-volatile memory for m5 gauge.
>>
>> Link: https://android.googlesource.com/kernel/google-modules/bms/+/1a68c36bef474573cc8629cc1d121eb6a81ab68c/max1720x_battery.c
>>
>> Other differences include the lack of V_BATT register to read the battery
>> level. The voltage must instead be read from V_CELL, the lowest voltage of
>> all cells. The mask to identify the chip is different. The computation of
>> the charge must also be changed to take into account TASKPERIOD, which
>> can add a factor 2 to the result.
>>
>> Add support for the MAX77759 by taking into account all of those
>> differences based on chip type.
>>
>> Do not advertise temp probes using the non-volatile memory as those are
>> not available.
>>
>> The regmap was proposed by André Draszik in
>>
>> Link: https://lore.kernel.org/all/d1bade77b5281c1de6b2ddcb4dbbd033e455a116.camel@linaro.org/
>>
>> Signed-off-by: Thomas Antoine <t.antoine@...ouvain.be>
>> ---
>>  drivers/power/supply/max1720x_battery.c | 265 ++++++++++++++++++++++++++++----
>>  1 file changed, 238 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c
>> index 68b5314ecf3a234f906ec8fe400e586855b69cd9..c9ad452ada9d0a2a51f37d04fd8c3260be522405 100644
>> --- a/drivers/power/supply/max1720x_battery.c
>> +++ b/drivers/power/supply/max1720x_battery.c
>> @@ -37,6 +37,7 @@
>>  #define MAX172XX_REPCAP			0x05	/* Average capacity */
>>  #define MAX172XX_REPSOC			0x06	/* Percentage of charge */
>>  #define MAX172XX_TEMP			0x08	/* Temperature */
>> +#define MAX172XX_VCELL			0x09	/* Lowest cell voltage */
> [...]
>>  	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> [...]
>> +			ret = regmap_read(info->regmap, MAX172XX_VCELL, &reg_val);
>> +			val->intval = max172xx_cell_voltage_to_ps(reg_val);
> 
> I haven't reviewed this fully due to all the feedback you already
> got from Peter Griffin and the DT binding being broken, but something
> that catched my eye:
> 
> POWER_SUPPLY_PROP_VOLTAGE_NOW provides the voltage of the whole
> battery and not of a single cell. E.g. a typical Li-Ion battery
> with two serial cells has a nominal voltage of roughly 7.4V while
> each cell has just 3.7V.
> 
> Greetings,
> 
> -- Sebastian

Downstream just reports this value which is usually a bit over 4V when I
record it.

Also from what I saw online, it seems that the battery does output
voltages around 3.86V as it is written on the battery:
https://cdn.shopify.com/s/files/1/0092/8133/9443/files/QeqxTLOL6eAp6OpZ.jpg?v=1728588266&width=2048

So I guess that there could only be a single cell in the battery? Which
would explain why they only report the lowest one.

It thus should be ok if we consider only the Google Pixel 6 (Pro). If we
need to take into account the possibility that the chip would be
used with other batteries, we could take from the devicetree the number
of cells and only provide the voltage if the number of cells is 1. Would
this be a good solution?

There is also a VSYS register (0xb1) but I couldn't find anything about it.
Taking a LSB of 156uV (twice the one of VCell), I see a clear correlation
with VCell, except for it being very slightly lower.
As it is mostly guesswork, I don't think it would be a good idea to use this
without any confirmation it is correct.

Best regards,
Thomas