Hi, On Wed, Jun 25, 2025 at 02:12:21PM +0200, Thomas Antoine wrote: > > On Fri, May 23, 2025 at 02:51:45PM +0200, Thomas Antoine via B4 Relay wrote: > >> From: Thomas Antoine <t.antoine@xxxxxxxxxxxx> > >> > >> 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@xxxxxxxxxx/ > >> > >> Signed-off-by: Thomas Antoine <t.antoine@xxxxxxxxxxxx> > >> --- > >> 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, ®_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? That's fine with me. But getting the DT binding prepared will take some time (the DT property should definetly go via Documentation/devicetree/bindings/power/supply/battery.yaml), we had some prior discussions around this IIRC. It might be sensible to move adding VOLTAGE_NOW support to an extra series. > 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. I see the datasheet does not have the register listed for < 0x10. Have you tried contacting the original Google driver author to shed some light to this? Greetings, -- Sebastian
Attachment:
signature.asc
Description: PGP signature
