* [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support @ 2025-04-21 18:13 Thomas Antoine via B4 Relay 2025-04-21 18:13 ` [PATCH v3 1/5] power: supply: correct capacity computation Thomas Antoine via B4 Relay ` (5 more replies) 0 siblings, 6 replies; 17+ messages in thread From: Thomas Antoine via B4 Relay @ 2025-04-21 18:13 UTC (permalink / raw) To: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, Dimitri Fedrau Cc: linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm, Thomas Antoine The Google Pixel 6 has a Maxim MAX77759 which provides a fuel gauge with an interface with a lot in common with the Maxim max1720x. Modify the Maxim MAX1720x driver to be compatible with the Maxim MAX77759 and enable it for the gs101-oriole board. The voltage, current, capacity, temperature and charge have all been tested and show coherent results. The charge full design and capacity equal the ones seen on android, the ratio between average charge and average current does predict pretty accurately the time to empty under a constant workload and temperature is coherent with the dynamic state of the device. Health is not enabled as it always reports overheating. The time to empty is wrong by about a factor 2 and is thus also disabled. Signed-off-by: Thomas Antoine <t.antoine@uclouvain.be> --- Changes in v3: - Update base tree to avoid conflicts - Fix capacity computation for max1720x - Add separate properties for the max7759 to disable non-functional ones - Take TASKPERIOD into account for voltage computation of max77759 - Simplify vcell computation (Dimitri Fedrau) - Switch has_nvmem to bool and keep it only in chip_data (Dimitri Fedrau) - Drop the yes_range from the write table (Sebastian Reichel) - Add test_power_supply_properties.sh to cover letter (Sebastian Reichel) - Switch back some changes to binding and actually use allOf:if: to restrict constraints (Krzysztof Kozlowski) - Fix style errors - Link to v2: https://lore.kernel.org/r/20250102-b4-gs101_max77759_fg-v2-0-87959abeb7ff@uclouvain.be Changes in v2: - Add fallback for voltage measurement (André Draszik) - Add regmap for the max77759 (André Draszik) - Add chip identification for the max77759 (André Draszik, Peter Griffin) - Move RSense value to a devicetree property shunt-resistor-micro-ohms (Dimitri Fedrau, André Draszik) - Use allOf:if to narrow binding per variant (Krzysztof Kozlowski) - Remove binding example (Krzysztof Kozlowski) - Change defconfig order to follow savedefconfig (Krzysztof Kozlowski) - Fix style errors - Link to v1: https://lore.kernel.org/r/20241202-b4-gs101_max77759_fg-v1-0-98d2fa7bfe30@uclouvain.be tools/testing/selftests/power_supply/test_power_supply_properties.sh: ok 1 max77759-fg.exists ok 2 max77759-fg.uevent.NAME ok 3 max77759-fg.sysfs.type ok 4 max77759-fg.uevent.TYPE ok 5 max77759-fg.sysfs.usb_type # SKIP ok 6 max77759-fg.sysfs.online # SKIP ok 7 max77759-fg.sysfs.present ok 8 max77759-fg.sysfs.status # SKIP ok 9 max77759-fg.sysfs.capacity ok 10 max77759-fg.sysfs.capacity_level # SKIP ok 11 max77759-fg.sysfs.model_name ok 12 max77759-fg.sysfs.manufacturer ok 13 max77759-fg.sysfs.serial_number # SKIP ok 14 max77759-fg.sysfs.technology # SKIP ok 15 max77759-fg.sysfs.cycle_count # SKIP ok 16 max77759-fg.sysfs.scope # SKIP ok 17 max77759-fg.sysfs.input_current_limit # SKIP(Dimitri Fedrau) ok 18 max77759-fg.sysfs.input_voltage_limit # SKIP ok 19 max77759-fg.sysfs.voltage_now ok 20 max77759-fg.sysfs.voltage_min # SKIP ok 21 max77759-fg.sysfs.voltage_max # SKIP ok 22 max77759-fg.sysfs.voltage_min_design # SKIP ok 23 max77759-fg.sysfs.voltage_max_design # SKIP ok 24 max77759-fg.sysfs.current_now ok 25 max77759-fg.sysfs.current_max # SKIP ok 26 max77759-fg.sysfs.charge_now # SKIP ok 27 max77759-fg.sysfs.charge_full ok 28 max77759-fg.sysfs.charge_full_design ok 29 max77759-fg.sysfs.power_now # SKIP ok 30 max77759-fg.sysfs.energy_now # SKIP ok 31 max77759-fg.sysfs.energy_full # SKIP ok 32 max77759-fg.sysfs.energy_full_design # SKIP ok 33 max77759-fg.sysfs.energy_full_design # SKIP --- Thomas Antoine (5): power: supply: correct capacity computation power: supply: add support for max77759 fuel gauge dt-bindings: power: supply: add max77759-fg flavor arm64: defconfig: enable Maxim max1720x driver arm64: dts: exynos: gs101-oriole: enable Maxim max77759 fuel gauge .../bindings/power/supply/maxim,max17201.yaml | 34 ++- .../boot/dts/exynos/google/gs101-pixel-common.dtsi | 10 + arch/arm64/configs/defconfig | 1 + drivers/power/supply/max1720x_battery.c | 271 ++++++++++++++++++--- 4 files changed, 280 insertions(+), 36 deletions(-) --- base-commit: e48e99b6edf41c69c5528aa7ffb2daf3c59ee105 change-id: 20241202-b4-gs101_max77759_fg-402e231a4b33 Best regards, -- Thomas Antoine <t.antoine@uclouvain.be> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 1/5] power: supply: correct capacity computation 2025-04-21 18:13 [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support Thomas Antoine via B4 Relay @ 2025-04-21 18:13 ` Thomas Antoine via B4 Relay 2025-04-22 9:46 ` Dimitri Fedrau 2025-04-21 18:13 ` [PATCH v3 2/5] power: supply: add support for max77759 fuel gauge Thomas Antoine via B4 Relay ` (4 subsequent siblings) 5 siblings, 1 reply; 17+ messages in thread From: Thomas Antoine via B4 Relay @ 2025-04-21 18:13 UTC (permalink / raw) To: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, Dimitri Fedrau Cc: linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm, Thomas Antoine From: Thomas Antoine <t.antoine@uclouvain.be> From the datasheet of the MAX17201/17205, the LSB should be "5.0μVh/RSENSE". The current computation sets it at 0.5mAh=5.0μVh/10mOhm which does not take into account the value of rsense which can be different from 10mOhm. Change the computation to fit the specs. Signed-off-by: Thomas Antoine <t.antoine@uclouvain.be> --- drivers/power/supply/max1720x_battery.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c index ea3912fd1de8bfd0d029c16f276316d06e1b105c..cca5f8b5071fb731f9b60420239ea03d46cb1bf3 100644 --- a/drivers/power/supply/max1720x_battery.c +++ b/drivers/power/supply/max1720x_battery.c @@ -288,9 +288,10 @@ static int max172xx_voltage_to_ps(unsigned int reg) return reg * 1250; /* in uV */ } -static int max172xx_capacity_to_ps(unsigned int reg) +static int max172xx_capacity_to_ps(unsigned int reg, + struct max1720x_device_info *info) { - return reg * 500; /* in uAh */ + return reg * (500000 / info->rsense); /* in uAh */ } /* -- 2.49.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/5] power: supply: correct capacity computation 2025-04-21 18:13 ` [PATCH v3 1/5] power: supply: correct capacity computation Thomas Antoine via B4 Relay @ 2025-04-22 9:46 ` Dimitri Fedrau 2025-04-23 7:48 ` Thomas Antoine 0 siblings, 1 reply; 17+ messages in thread From: Dimitri Fedrau @ 2025-04-22 9:46 UTC (permalink / raw) To: t.antoine Cc: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm Hi Thomas, On Mon, Apr 21, 2025 at 08:13:32PM +0200, Thomas Antoine via B4 Relay wrote: > From: Thomas Antoine <t.antoine@uclouvain.be> > > From the datasheet of the MAX17201/17205, the LSB should be > "5.0μVh/RSENSE". The current computation sets it at 0.5mAh=5.0μVh/10mOhm > which does not take into account the value of rsense which can be > different from 10mOhm. > > Change the computation to fit the specs. > > Signed-off-by: Thomas Antoine <t.antoine@uclouvain.be> > --- > drivers/power/supply/max1720x_battery.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c > index ea3912fd1de8bfd0d029c16f276316d06e1b105c..cca5f8b5071fb731f9b60420239ea03d46cb1bf3 100644 > --- a/drivers/power/supply/max1720x_battery.c > +++ b/drivers/power/supply/max1720x_battery.c > @@ -288,9 +288,10 @@ static int max172xx_voltage_to_ps(unsigned int reg) > return reg * 1250; /* in uV */ > } > > -static int max172xx_capacity_to_ps(unsigned int reg) > +static int max172xx_capacity_to_ps(unsigned int reg, > + struct max1720x_device_info *info) > { > - return reg * 500; /* in uAh */ > + return reg * (500000 / info->rsense); /* in uAh */ > } > > /* > > -- > 2.49.0 > > thanks for finding this. Reviewed-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com> Best regards, Dimitri Fedrau ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/5] power: supply: correct capacity computation 2025-04-22 9:46 ` Dimitri Fedrau @ 2025-04-23 7:48 ` Thomas Antoine 0 siblings, 0 replies; 17+ messages in thread From: Thomas Antoine @ 2025-04-23 7:48 UTC (permalink / raw) To: Dimitri Fedrau Cc: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm Hi Dimitri, On 4/22/25 11:46, Dimitri Fedrau wrote: > Hi Thomas, > > On Mon, Apr 21, 2025 at 08:13:32PM +0200, Thomas Antoine via B4 Relay wrote: >> From: Thomas Antoine <t.antoine@uclouvain.be> [...] >> -static int max172xx_capacity_to_ps(unsigned int reg) >> +static int max172xx_capacity_to_ps(unsigned int reg, >> + struct max1720x_device_info *info) >> { >> - return reg * 500; /* in uAh */ >> + return reg * (500000 / info->rsense); /* in uAh */ >> } >> >> /* >> >> -- >> 2.49.0 >> >> > thanks for finding this. > > Reviewed-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com> > > Best regards, > Dimitri Fedrau I just realized I forgot to change the function calls in this patch, it is only changed in the next patch. This will not compile as the function call does not pass info as argument. I will change this in the next version. Best regards, Thomas Antoine ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 2/5] power: supply: add support for max77759 fuel gauge 2025-04-21 18:13 [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support Thomas Antoine via B4 Relay 2025-04-21 18:13 ` [PATCH v3 1/5] power: supply: correct capacity computation Thomas Antoine via B4 Relay @ 2025-04-21 18:13 ` Thomas Antoine via B4 Relay 2025-04-22 18:48 ` Dimitri Fedrau 2025-04-30 0:36 ` Sebastian Reichel 2025-04-21 18:13 ` [PATCH v3 3/5] dt-bindings: power: supply: add max77759-fg flavor Thomas Antoine via B4 Relay ` (3 subsequent siblings) 5 siblings, 2 replies; 17+ messages in thread From: Thomas Antoine via B4 Relay @ 2025-04-21 18:13 UTC (permalink / raw) To: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, Dimitri Fedrau Cc: linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm, Thomas Antoine From: Thomas Antoine <t.antoine@uclouvain.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@uclouvain.be> --- drivers/power/supply/max1720x_battery.c | 270 ++++++++++++++++++++++++++++---- 1 file changed, 237 insertions(+), 33 deletions(-) diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c index cca5f8b5071fb731f9b60420239ea03d46cb1bf3..969d3a7c2baa7e1d23c5175942d975b277c8554c 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 */ #define MAX172XX_CURRENT 0x0A /* Actual current */ #define MAX172XX_AVG_CURRENT 0x0B /* Average current */ #define MAX172XX_FULL_CAP 0x10 /* Calculated full capacity */ @@ -50,19 +51,32 @@ #define MAX172XX_DEV_NAME_TYPE_MASK GENMASK(3, 0) #define MAX172XX_DEV_NAME_TYPE_MAX17201 BIT(0) #define MAX172XX_DEV_NAME_TYPE_MAX17205 (BIT(0) | BIT(2)) +#define MAX77759_DEV_NAME_TYPE_MASK GENMASK(15, 9) +#define MAX77759_DEV_NAME_TYPE_MAX77759 0x31 #define MAX172XX_QR_TABLE10 0x22 +#define MAX77759_TASKPERIOD 0x3C +#define MAX77759_TASKPERIOD_175MS 0x1680 +#define MAX77759_TASKPERIOD_351MS 0x2D00 #define MAX172XX_BATT 0xDA /* Battery voltage */ #define MAX172XX_ATAVCAP 0xDF static const char *const max1720x_manufacturer = "Maxim Integrated"; static const char *const max17201_model = "MAX17201"; static const char *const max17205_model = "MAX17205"; +static const char *const max77759_model = "MAX77759"; + +enum chip_id { + MAX1720X_ID, + MAX77759_ID, +}; struct max1720x_device_info { struct regmap *regmap; struct regmap *regmap_nv; struct i2c_client *ancillary; int rsense; + int charge_full_design; + enum chip_id id; }; /* @@ -271,6 +285,80 @@ static const enum power_supply_property max1720x_battery_props[] = { POWER_SUPPLY_PROP_MANUFACTURER, }; +/* + * Registers 0x80 up to 0xaf which contain the model for the fuel gauge + * algorithm (stored in nvmem for the max1720x) are locked. They can + * be unlocked by writing 0x59 to 0x62 and 0xc4 to 0x63. They should be + * enabled in the regmap if the driver is extended to manage the model. + */ +static const struct regmap_range max77759_registers[] = { + regmap_reg_range(0x00, 0x4f), + regmap_reg_range(0xb0, 0xbf), + regmap_reg_range(0xd0, 0xd0), + regmap_reg_range(0xdc, 0xdf), + regmap_reg_range(0xfb, 0xfb), + regmap_reg_range(0xff, 0xff), +}; + +static const struct regmap_range max77759_ro_registers[] = { + regmap_reg_range(0x3d, 0x3d), + regmap_reg_range(0xfb, 0xfb), + regmap_reg_range(0xff, 0xff), +}; + +static const struct regmap_access_table max77759_write_table = { + .no_ranges = max77759_ro_registers, + .n_no_ranges = ARRAY_SIZE(max77759_ro_registers), +}; + +static const struct regmap_access_table max77759_rd_table = { + .yes_ranges = max77759_registers, + .n_yes_ranges = ARRAY_SIZE(max77759_registers), +}; + +static const struct regmap_config max77759_regmap_cfg = { + .reg_bits = 8, + .val_bits = 16, + .max_register = 0xff, + .wr_table = &max77759_write_table, + .rd_table = &max77759_rd_table, + .val_format_endian = REGMAP_ENDIAN_LITTLE, + .cache_type = REGCACHE_NONE, +}; + +static const enum power_supply_property max77759_battery_props[] = { + POWER_SUPPLY_PROP_PRESENT, + POWER_SUPPLY_PROP_CAPACITY, + POWER_SUPPLY_PROP_VOLTAGE_NOW, + POWER_SUPPLY_PROP_CHARGE_FULL, + POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN, + POWER_SUPPLY_PROP_CHARGE_AVG, + POWER_SUPPLY_PROP_TEMP, + POWER_SUPPLY_PROP_CURRENT_NOW, + POWER_SUPPLY_PROP_CURRENT_AVG, + POWER_SUPPLY_PROP_MODEL_NAME, + POWER_SUPPLY_PROP_MANUFACTURER, +}; + + +struct chip_data { + bool has_nvmem; + const struct regmap_config *regmap_cfg; + enum chip_id id; +}; + +static const struct chip_data max1720x_data = { + .has_nvmem = true, + .regmap_cfg = &max1720x_regmap_cfg, + .id = MAX1720X_ID, +}; + +static const struct chip_data max77759_data = { + .has_nvmem = false, + .regmap_cfg = &max77759_regmap_cfg, + .id = MAX77759_ID, +}; + /* Convert regs value to power_supply units */ static int max172xx_time_to_ps(unsigned int reg) @@ -288,10 +376,36 @@ static int max172xx_voltage_to_ps(unsigned int reg) return reg * 1250; /* in uV */ } +static int max172xx_cell_voltage_to_ps(unsigned int reg) +{ + return reg * 625 / 8; /* in uV */ +} + static int max172xx_capacity_to_ps(unsigned int reg, - struct max1720x_device_info *info) + struct max1720x_device_info *info, + int *intval) { - return reg * (500000 / info->rsense); /* in uAh */ + int lsb = 1; + int reg_val; + int ret; + + if (info->id == MAX77759_ID) { + ret = regmap_read(info->regmap, MAX77759_TASKPERIOD, ®_val); + if (ret) + return ret; + + switch (reg_val) { + case MAX77759_TASKPERIOD_175MS: + break; + case MAX77759_TASKPERIOD_351MS: + lsb = 2; + break; + default: + return -ENODEV; + } + } + *intval = reg * (500000 / info->rsense) * lsb; /* in uAh */ + return 0; } /* @@ -306,6 +420,28 @@ static int max172xx_temperature_to_ps(unsigned int reg) return val * 10 / 256; /* in tenths of deg. C */ } +static const char *max1720x_devname_to_model(unsigned int reg_val, + union power_supply_propval *val, + struct max1720x_device_info *info) +{ + switch (info->id) { + case MAX1720X_ID: + reg_val = FIELD_GET(MAX172XX_DEV_NAME_TYPE_MASK, reg_val); + if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17201) + return max17201_model; + else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17205) + return max17205_model; + return NULL; + case MAX77759_ID: + reg_val = FIELD_GET(MAX77759_DEV_NAME_TYPE_MASK, reg_val); + if (reg_val == MAX77759_DEV_NAME_TYPE_MAX77759) + return max77759_model; + return NULL; + default: + return NULL; + } +} + /* * Calculating current registers resolution: * @@ -390,19 +526,31 @@ static int max1720x_battery_get_property(struct power_supply *psy, val->intval = max172xx_percent_to_ps(reg_val); break; case POWER_SUPPLY_PROP_VOLTAGE_NOW: - ret = regmap_read(info->regmap, MAX172XX_BATT, ®_val); - val->intval = max172xx_voltage_to_ps(reg_val); + if (info->id == MAX1720X_ID) { + ret = regmap_read(info->regmap, MAX172XX_BATT, ®_val); + val->intval = max172xx_voltage_to_ps(reg_val); + } else if (info->id == MAX77759_ID) { + ret = regmap_read(info->regmap, MAX172XX_VCELL, ®_val); + val->intval = max172xx_cell_voltage_to_ps(reg_val); + } else + return -ENODEV; break; case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN: ret = regmap_read(info->regmap, MAX172XX_DESIGN_CAP, ®_val); - val->intval = max172xx_capacity_to_ps(reg_val); + if (ret) + break; + ret = max172xx_capacity_to_ps(reg_val, info, &val->intval); break; case POWER_SUPPLY_PROP_CHARGE_AVG: ret = regmap_read(info->regmap, MAX172XX_REPCAP, ®_val); - val->intval = max172xx_capacity_to_ps(reg_val); + if (ret) + break; + + ret = max172xx_capacity_to_ps(reg_val, info, &val->intval); break; case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG: ret = regmap_read(info->regmap, MAX172XX_TTE, ®_val); + pr_info("RAW TTE: %d", reg_val); val->intval = max172xx_time_to_ps(reg_val); break; case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG: @@ -423,17 +571,15 @@ static int max1720x_battery_get_property(struct power_supply *psy, break; case POWER_SUPPLY_PROP_CHARGE_FULL: ret = regmap_read(info->regmap, MAX172XX_FULL_CAP, ®_val); - val->intval = max172xx_capacity_to_ps(reg_val); + if (ret) + break; + ret = max172xx_capacity_to_ps(reg_val, info, &val->intval); break; case POWER_SUPPLY_PROP_MODEL_NAME: ret = regmap_read(info->regmap, MAX172XX_DEV_NAME, ®_val); - reg_val = FIELD_GET(MAX172XX_DEV_NAME_TYPE_MASK, reg_val); - if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17201) - val->strval = max17201_model; - else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17205) - val->strval = max17205_model; - else - return -ENODEV; + val->strval = max1720x_devname_to_model(reg_val, val, info); + if (!val->strval) + ret = -ENODEV; break; case POWER_SUPPLY_PROP_MANUFACTURER: val->strval = max1720x_manufacturer; @@ -527,7 +673,6 @@ static int max1720x_probe_nvmem(struct i2c_client *client, .priv = info, }; struct nvmem_device *nvmem; - unsigned int val; int ret; info->ancillary = i2c_new_ancillary_device(client, "nvmem", 0xb); @@ -549,18 +694,6 @@ static int max1720x_probe_nvmem(struct i2c_client *client, return PTR_ERR(info->regmap_nv); } - ret = regmap_read(info->regmap_nv, MAX1720X_NRSENSE, &val); - if (ret < 0) { - dev_err(dev, "Failed to read sense resistor value\n"); - return ret; - } - - info->rsense = val; - if (!info->rsense) { - dev_warn(dev, "RSense not calibrated, set 10 mOhms!\n"); - info->rsense = 1000; /* in regs in 10^-5 */ - } - nvmem = devm_nvmem_register(dev, &nvmem_config); if (IS_ERR(nvmem)) { dev_err(dev, "Could not register nvmem!"); @@ -570,6 +703,38 @@ static int max1720x_probe_nvmem(struct i2c_client *client, return 0; } +static int max1720x_get_rsense(struct device *dev, + struct max1720x_device_info *info, + const struct chip_data *data) +{ + unsigned int val; + int ret; + + if (data->has_nvmem) { + ret = regmap_read(info->regmap_nv, MAX1720X_NRSENSE, &val); + if (ret < 0) { + dev_err(dev, "Failed to read RSense from nvmem\n"); + return ret; + } + + info->rsense = val; + if (!info->rsense) { + dev_warn(dev, "RSense not calibrated, set 10 mOhms!\n"); + info->rsense = 1000; /* in regs in 10^-5 */ + } + } else { + ret = of_property_read_u32(dev->of_node, + "shunt-resistor-micro-ohms", &val); + if (ret) { + dev_err(dev, "Failed to read RSense from devicetree\n"); + return ret; + } + info->rsense = val/10; + } + + return 0; +} + static const struct power_supply_desc max1720x_bat_desc = { .name = "max1720x", .no_thermal = true, @@ -579,32 +744,70 @@ static const struct power_supply_desc max1720x_bat_desc = { .get_property = max1720x_battery_get_property, }; +static const struct power_supply_desc max77759_bat_desc = { + .name = "max77759-fg", + .no_thermal = true, + .type = POWER_SUPPLY_TYPE_BATTERY, + .properties = max77759_battery_props, + .num_properties = ARRAY_SIZE(max77759_battery_props), + .get_property = max1720x_battery_get_property, +}; + static int max1720x_probe(struct i2c_client *client) { struct power_supply_config psy_cfg = {}; struct device *dev = &client->dev; struct max1720x_device_info *info; struct power_supply *bat; + const struct chip_data *data; + const struct power_supply_desc *bat_desc; int ret; info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL); if (!info) return -ENOMEM; + data = device_get_match_data(dev); + if (!data) + return dev_err_probe(dev, -EINVAL, "Failed to get chip data\n"); + psy_cfg.drv_data = info; psy_cfg.fwnode = dev_fwnode(dev); - psy_cfg.attr_grp = max1720x_groups; + switch (data->id) { + case MAX1720X_ID: + psy_cfg.attr_grp = max1720x_groups; + bat_desc = &max1720x_bat_desc; + break; + case MAX77759_ID: + bat_desc = &max77759_bat_desc; + break; + default: + return dev_err_probe(dev, -EINVAL, "Unsupported chip\n"); + } i2c_set_clientdata(client, info); - info->regmap = devm_regmap_init_i2c(client, &max1720x_regmap_cfg); + + info->id = data->id; + info->regmap = devm_regmap_init_i2c(client, data->regmap_cfg); if (IS_ERR(info->regmap)) return dev_err_probe(dev, PTR_ERR(info->regmap), "regmap initialization failed\n"); - ret = max1720x_probe_nvmem(client, info); + if (data->has_nvmem) { + ret = max1720x_probe_nvmem(client, info); + if (ret) + return dev_err_probe(dev, ret, "Failed to probe nvmem\n"); + } + + ret = of_property_read_u32(dev->of_node, + "charge-full-design-microamp-hours", &info->charge_full_design); + if (ret) + info->charge_full_design = 0; + + ret = max1720x_get_rsense(dev, info, data); if (ret) - return dev_err_probe(dev, ret, "Failed to probe nvmem\n"); + return dev_err_probe(dev, ret, "Failed to get RSense\n"); - bat = devm_power_supply_register(dev, &max1720x_bat_desc, &psy_cfg); + bat = devm_power_supply_register(dev, bat_desc, &psy_cfg); if (IS_ERR(bat)) return dev_err_probe(dev, PTR_ERR(bat), "Failed to register power supply\n"); @@ -613,7 +816,8 @@ static int max1720x_probe(struct i2c_client *client) } static const struct of_device_id max1720x_of_match[] = { - { .compatible = "maxim,max17201" }, + { .compatible = "maxim,max17201", .data = (void *) &max1720x_data }, + { .compatible = "maxim,max77759-fg", .data = (void *) &max77759_data }, {} }; MODULE_DEVICE_TABLE(of, max1720x_of_match); -- 2.49.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/5] power: supply: add support for max77759 fuel gauge 2025-04-21 18:13 ` [PATCH v3 2/5] power: supply: add support for max77759 fuel gauge Thomas Antoine via B4 Relay @ 2025-04-22 18:48 ` Dimitri Fedrau 2025-04-23 8:02 ` Thomas Antoine 2025-04-30 0:36 ` Sebastian Reichel 1 sibling, 1 reply; 17+ messages in thread From: Dimitri Fedrau @ 2025-04-22 18:48 UTC (permalink / raw) To: t.antoine Cc: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm Hi Thomas, On Mon, Apr 21, 2025 at 08:13:33PM +0200, Thomas Antoine via B4 Relay wrote: > From: Thomas Antoine <t.antoine@uclouvain.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@uclouvain.be> > --- > drivers/power/supply/max1720x_battery.c | 270 ++++++++++++++++++++++++++++---- > 1 file changed, 237 insertions(+), 33 deletions(-) > > diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c > index cca5f8b5071fb731f9b60420239ea03d46cb1bf3..969d3a7c2baa7e1d23c5175942d975b277c8554c 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 */ > #define MAX172XX_CURRENT 0x0A /* Actual current */ > #define MAX172XX_AVG_CURRENT 0x0B /* Average current */ > #define MAX172XX_FULL_CAP 0x10 /* Calculated full capacity */ > @@ -50,19 +51,32 @@ > #define MAX172XX_DEV_NAME_TYPE_MASK GENMASK(3, 0) > #define MAX172XX_DEV_NAME_TYPE_MAX17201 BIT(0) > #define MAX172XX_DEV_NAME_TYPE_MAX17205 (BIT(0) | BIT(2)) > +#define MAX77759_DEV_NAME_TYPE_MASK GENMASK(15, 9) > +#define MAX77759_DEV_NAME_TYPE_MAX77759 0x31 > #define MAX172XX_QR_TABLE10 0x22 > +#define MAX77759_TASKPERIOD 0x3C > +#define MAX77759_TASKPERIOD_175MS 0x1680 > +#define MAX77759_TASKPERIOD_351MS 0x2D00 I think it would be more readable if MAX77759_ defines are separated to the MAX172XX defines instead of mixing them up. > #define MAX172XX_BATT 0xDA /* Battery voltage */ > #define MAX172XX_ATAVCAP 0xDF > > static const char *const max1720x_manufacturer = "Maxim Integrated"; > static const char *const max17201_model = "MAX17201"; > static const char *const max17205_model = "MAX17205"; > +static const char *const max77759_model = "MAX77759"; > + > +enum chip_id { > + MAX1720X_ID, > + MAX77759_ID, > +}; > > struct max1720x_device_info { > struct regmap *regmap; > struct regmap *regmap_nv; > struct i2c_client *ancillary; > int rsense; > + int charge_full_design; Don't see charge_full_design is used somewhere besides reading it from device-tree and it isn't part of the bindings. If not needed, remove it. > + enum chip_id id; > }; > > [...] > +static int max172xx_cell_voltage_to_ps(unsigned int reg) > +{ > + return reg * 625 / 8; /* in uV */ > +} > + > static int max172xx_capacity_to_ps(unsigned int reg, > - struct max1720x_device_info *info) > + struct max1720x_device_info *info, > + int *intval) > { > - return reg * (500000 / info->rsense); /* in uAh */ > + int lsb = 1; > + int reg_val; The naming of reg_val is somehow confusing because of reg. Better rename it to something like reg_task_period or similar and reg_val should be of type unsigned int. > + int ret; > + > + if (info->id == MAX77759_ID) { > + ret = regmap_read(info->regmap, MAX77759_TASKPERIOD, ®_val); > + if (ret) > + return ret; > + > + switch (reg_val) { > + case MAX77759_TASKPERIOD_175MS: > + break; > + case MAX77759_TASKPERIOD_351MS: > + lsb = 2; > + break; > + default: > + return -ENODEV; > + } > + } > + *intval = reg * (500000 / info->rsense) * lsb; /* in uAh */ > + return 0; nit: add newline before return. > } > > /* > @@ -306,6 +420,28 @@ static int max172xx_temperature_to_ps(unsigned int reg) > return val * 10 / 256; /* in tenths of deg. C */ > } > > +static const char *max1720x_devname_to_model(unsigned int reg_val, > + union power_supply_propval *val, > + struct max1720x_device_info *info) > +{ > + switch (info->id) { > + case MAX1720X_ID: > + reg_val = FIELD_GET(MAX172XX_DEV_NAME_TYPE_MASK, reg_val); > + if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17201) > + return max17201_model; > + else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17205) > + return max17205_model; > + return NULL; nit: return NULL in else case. > + case MAX77759_ID: > + reg_val = FIELD_GET(MAX77759_DEV_NAME_TYPE_MASK, reg_val); > + if (reg_val == MAX77759_DEV_NAME_TYPE_MAX77759) > + return max77759_model; > + return NULL; nit: return NULL in else case. > + default: > + return NULL; > + } > +} > + > /* > * Calculating current registers resolution: > * > @@ -390,19 +526,31 @@ static int max1720x_battery_get_property(struct power_supply *psy, > val->intval = max172xx_percent_to_ps(reg_val); > break; > case POWER_SUPPLY_PROP_VOLTAGE_NOW: > - ret = regmap_read(info->regmap, MAX172XX_BATT, ®_val); > - val->intval = max172xx_voltage_to_ps(reg_val); > + if (info->id == MAX1720X_ID) { > + ret = regmap_read(info->regmap, MAX172XX_BATT, ®_val); > + val->intval = max172xx_voltage_to_ps(reg_val); > + } else if (info->id == MAX77759_ID) { > + ret = regmap_read(info->regmap, MAX172XX_VCELL, ®_val); > + val->intval = max172xx_cell_voltage_to_ps(reg_val); > + } else > + return -ENODEV; > break; > case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN: > ret = regmap_read(info->regmap, MAX172XX_DESIGN_CAP, ®_val); > - val->intval = max172xx_capacity_to_ps(reg_val); > + if (ret) > + break; I would keep max172xx_capacity_to_ps as it was before and add the calculation for the MAX77759 after handling the MAX1720X case. Creating a function max77759_capacity_to_ps that further processes the value calculated by max172xx_capacity_to_ps or just inline this code. Otherwise the naming of the function is somehow confusing. > + ret = max172xx_capacity_to_ps(reg_val, info, &val->intval); > break; > case POWER_SUPPLY_PROP_CHARGE_AVG: > ret = regmap_read(info->regmap, MAX172XX_REPCAP, ®_val); > - val->intval = max172xx_capacity_to_ps(reg_val); > + if (ret) > + break; > + Same as above. > + ret = max172xx_capacity_to_ps(reg_val, info, &val->intval); > break; > case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG: > ret = regmap_read(info->regmap, MAX172XX_TTE, ®_val); > + pr_info("RAW TTE: %d", reg_val); Remove pr_info. > val->intval = max172xx_time_to_ps(reg_val); > break; > case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG: > @@ -423,17 +571,15 @@ static int max1720x_battery_get_property(struct power_supply *psy, > break; > case POWER_SUPPLY_PROP_CHARGE_FULL: > ret = regmap_read(info->regmap, MAX172XX_FULL_CAP, ®_val); > - val->intval = max172xx_capacity_to_ps(reg_val); ... > + if (ret) > + break; > + ret = max172xx_capacity_to_ps(reg_val, info, &val->intval); > break; > case POWER_SUPPLY_PROP_MODEL_NAME: > ret = regmap_read(info->regmap, MAX172XX_DEV_NAME, ®_val); > - reg_val = FIELD_GET(MAX172XX_DEV_NAME_TYPE_MASK, reg_val); > - if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17201) > - val->strval = max17201_model; > - else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17205) > - val->strval = max17205_model; > - else > - return -ENODEV; > + val->strval = max1720x_devname_to_model(reg_val, val, info); Wouldn't it be better to just inline this function ? > + if (!val->strval) > + ret = -ENODEV; > { [...] > struct power_supply_config psy_cfg = {}; > struct device *dev = &client->dev; > struct max1720x_device_info *info; > struct power_supply *bat; > + const struct chip_data *data; > + const struct power_supply_desc *bat_desc; > int ret; > > info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL); > if (!info) > return -ENOMEM; > > + data = device_get_match_data(dev); > + if (!data) > + return dev_err_probe(dev, -EINVAL, "Failed to get chip data\n"); > + > psy_cfg.drv_data = info; > psy_cfg.fwnode = dev_fwnode(dev); > - psy_cfg.attr_grp = max1720x_groups; > + switch (data->id) { > + case MAX1720X_ID: > + psy_cfg.attr_grp = max1720x_groups; > + bat_desc = &max1720x_bat_desc; > + break; > + case MAX77759_ID: > + bat_desc = &max77759_bat_desc; > + break; > + default: > + return dev_err_probe(dev, -EINVAL, "Unsupported chip\n"); > + } nit: add empty line > i2c_set_clientdata(client, info); > - info->regmap = devm_regmap_init_i2c(client, &max1720x_regmap_cfg); > + > + info->id = data->id; > + info->regmap = devm_regmap_init_i2c(client, data->regmap_cfg); > if (IS_ERR(info->regmap)) > return dev_err_probe(dev, PTR_ERR(info->regmap), > "regmap initialization failed\n"); > > - ret = max1720x_probe_nvmem(client, info); > + if (data->has_nvmem) { > + ret = max1720x_probe_nvmem(client, info); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to probe nvmem\n"); > + } > + > + ret = of_property_read_u32(dev->of_node, > + "charge-full-design-microamp-hours", &info->charge_full_design); > + if (ret) > + info->charge_full_design = 0; > + > + ret = max1720x_get_rsense(dev, info, data); > if (ret) > - return dev_err_probe(dev, ret, "Failed to probe nvmem\n"); > + return dev_err_probe(dev, ret, "Failed to get RSense\n"); > > - bat = devm_power_supply_register(dev, &max1720x_bat_desc, &psy_cfg); > + bat = devm_power_supply_register(dev, bat_desc, &psy_cfg); > if (IS_ERR(bat)) > return dev_err_probe(dev, PTR_ERR(bat), > "Failed to register power supply\n"); > @@ -613,7 +816,8 @@ static int max1720x_probe(struct i2c_client *client) > } > > static const struct of_device_id max1720x_of_match[] = { > - { .compatible = "maxim,max17201" }, > + { .compatible = "maxim,max17201", .data = (void *) &max1720x_data }, > + { .compatible = "maxim,max77759-fg", .data = (void *) &max77759_data }, > {} > }; > MODULE_DEVICE_TABLE(of, max1720x_of_match); > > -- > 2.49.0 > > Best regards, Dimitri Fedrau ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/5] power: supply: add support for max77759 fuel gauge 2025-04-22 18:48 ` Dimitri Fedrau @ 2025-04-23 8:02 ` Thomas Antoine 0 siblings, 0 replies; 17+ messages in thread From: Thomas Antoine @ 2025-04-23 8:02 UTC (permalink / raw) To: Dimitri Fedrau Cc: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm Hi Dimitri, On 4/22/25 20:48, Dimitri Fedrau wrote: > Hi Thomas, > > On Mon, Apr 21, 2025 at 08:13:33PM +0200, Thomas Antoine via B4 Relay wrote: >> From: Thomas Antoine <t.antoine@uclouvain.be> [...] >> #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 */ >> #define MAX172XX_CURRENT 0x0A /* Actual current */ >> #define MAX172XX_AVG_CURRENT 0x0B /* Average current */ >> #define MAX172XX_FULL_CAP 0x10 /* Calculated full capacity */ >> @@ -50,19 +51,32 @@ >> #define MAX172XX_DEV_NAME_TYPE_MASK GENMASK(3, 0) >> #define MAX172XX_DEV_NAME_TYPE_MAX17201 BIT(0) >> #define MAX172XX_DEV_NAME_TYPE_MAX17205 (BIT(0) | BIT(2)) >> +#define MAX77759_DEV_NAME_TYPE_MASK GENMASK(15, 9) >> +#define MAX77759_DEV_NAME_TYPE_MAX77759 0x31 >> #define MAX172XX_QR_TABLE10 0x22 >> +#define MAX77759_TASKPERIOD 0x3C >> +#define MAX77759_TASKPERIOD_175MS 0x1680 >> +#define MAX77759_TASKPERIOD_351MS 0x2D00 > I think it would be more readable if MAX77759_ defines are separated to > the MAX172XX defines instead of mixing them up. Will fix in v4. >> #define MAX172XX_BATT 0xDA /* Battery voltage */ >> #define MAX172XX_ATAVCAP 0xDF >> >> static const char *const max1720x_manufacturer = "Maxim Integrated"; >> static const char *const max17201_model = "MAX17201"; >> static const char *const max17205_model = "MAX17205"; >> +static const char *const max77759_model = "MAX77759"; >> + >> +enum chip_id { >> + MAX1720X_ID, >> + MAX77759_ID, >> +}; >> >> struct max1720x_device_info { >> struct regmap *regmap; >> struct regmap *regmap_nv; >> struct i2c_client *ancillary; >> int rsense; >> + int charge_full_design; > Don't see charge_full_design is used somewhere besides reading it from > device-tree and it isn't part of the bindings. If not needed, remove it. > Its a leftover of a previous experimentation, will remove in next version. >> + enum chip_id id; >> }; >> >> > [...] > >> +static int max172xx_cell_voltage_to_ps(unsigned int reg) >> +{ >> + return reg * 625 / 8; /* in uV */ >> +} >> + >> static int max172xx_capacity_to_ps(unsigned int reg, >> - struct max1720x_device_info *info) >> + struct max1720x_device_info *info, >> + int *intval) >> { >> - return reg * (500000 / info->rsense); /* in uAh */ >> + int lsb = 1; >> + int reg_val; > The naming of reg_val is somehow confusing because of reg. Better rename > it to something like reg_task_period or similar and reg_val should be of > type unsigned int. > Will change in v4. >> + int ret; >> + >> + if (info->id == MAX77759_ID) { >> + ret = regmap_read(info->regmap, MAX77759_TASKPERIOD, ®_val); >> + if (ret) >> + return ret; >> + >> + switch (reg_val) { >> + case MAX77759_TASKPERIOD_175MS: >> + break; >> + case MAX77759_TASKPERIOD_351MS: >> + lsb = 2; >> + break; >> + default: >> + return -ENODEV; >> + } >> + } >> + *intval = reg * (500000 / info->rsense) * lsb; /* in uAh */ >> + return 0; > nit: add newline before return. > Will fix in v4 >> } >> >> /* >> @@ -306,6 +420,28 @@ static int max172xx_temperature_to_ps(unsigned int reg) >> return val * 10 / 256; /* in tenths of deg. C */ >> } >> >> +static const char *max1720x_devname_to_model(unsigned int reg_val, >> + union power_supply_propval *val, >> + struct max1720x_device_info *info) >> +{ >> + switch (info->id) { >> + case MAX1720X_ID: >> + reg_val = FIELD_GET(MAX172XX_DEV_NAME_TYPE_MASK, reg_val); >> + if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17201) >> + return max17201_model; >> + else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17205) >> + return max17205_model; >> + return NULL; > nit: return NULL in else case. > >> + case MAX77759_ID: >> + reg_val = FIELD_GET(MAX77759_DEV_NAME_TYPE_MASK, reg_val); >> + if (reg_val == MAX77759_DEV_NAME_TYPE_MAX77759) >> + return max77759_model; >> + return NULL; > nit: return NULL in else case. > Will fix both in v4. >> + default: >> + return NULL; >> + } >> +} >> + >> /* >> * Calculating current registers resolution: >> * >> @@ -390,19 +526,31 @@ static int max1720x_battery_get_property(struct power_supply *psy, >> val->intval = max172xx_percent_to_ps(reg_val); >> break; >> case POWER_SUPPLY_PROP_VOLTAGE_NOW: >> - ret = regmap_read(info->regmap, MAX172XX_BATT, ®_val); >> - val->intval = max172xx_voltage_to_ps(reg_val); >> + if (info->id == MAX1720X_ID) { >> + ret = regmap_read(info->regmap, MAX172XX_BATT, ®_val); >> + val->intval = max172xx_voltage_to_ps(reg_val); >> + } else if (info->id == MAX77759_ID) { >> + ret = regmap_read(info->regmap, MAX172XX_VCELL, ®_val); >> + val->intval = max172xx_cell_voltage_to_ps(reg_val); >> + } else >> + return -ENODEV; >> break; >> case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN: >> ret = regmap_read(info->regmap, MAX172XX_DESIGN_CAP, ®_val); >> - val->intval = max172xx_capacity_to_ps(reg_val); >> + if (ret) >> + break; > I would keep max172xx_capacity_to_ps as it was before and add the > calculation for the MAX77759 after handling the MAX1720X case. Creating > a function max77759_capacity_to_ps that further processes the value > calculated by max172xx_capacity_to_ps or just inline this code. > Otherwise the naming of the function is somehow confusing. > Will change for v4. >> + ret = max172xx_capacity_to_ps(reg_val, info, &val->intval); >> break; >> case POWER_SUPPLY_PROP_CHARGE_AVG: >> ret = regmap_read(info->regmap, MAX172XX_REPCAP, ®_val); >> - val->intval = max172xx_capacity_to_ps(reg_val); >> + if (ret) >> + break; >> + > Same as above. > >> + ret = max172xx_capacity_to_ps(reg_val, info, &val->intval); >> break; >> case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG: >> ret = regmap_read(info->regmap, MAX172XX_TTE, ®_val); >> + pr_info("RAW TTE: %d", reg_val); > Remove pr_info. > Once again debug I forgot, sorry for this. >> val->intval = max172xx_time_to_ps(reg_val); >> break; >> case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG: >> @@ -423,17 +571,15 @@ static int max1720x_battery_get_property(struct power_supply *psy, >> break; >> case POWER_SUPPLY_PROP_CHARGE_FULL: >> ret = regmap_read(info->regmap, MAX172XX_FULL_CAP, ®_val); >> - val->intval = max172xx_capacity_to_ps(reg_val); > ... > >> + if (ret) >> + break; >> + ret = max172xx_capacity_to_ps(reg_val, info, &val->intval); >> break; >> case POWER_SUPPLY_PROP_MODEL_NAME: >> ret = regmap_read(info->regmap, MAX172XX_DEV_NAME, ®_val); >> - reg_val = FIELD_GET(MAX172XX_DEV_NAME_TYPE_MASK, reg_val); >> - if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17201) >> - val->strval = max17201_model; >> - else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17205) >> - val->strval = max17205_model; >> - else >> - return -ENODEV; >> + val->strval = max1720x_devname_to_model(reg_val, val, info); > Wouldn't it be better to just inline this function ? > I think my reason for this was that this case became quite long and indented compared to all the others. If you think it is better, I will inline it for v4. >> + if (!val->strval) >> + ret = -ENODEV; >> { > [...] > >> struct power_supply_config psy_cfg = {}; >> struct device *dev = &client->dev; >> struct max1720x_device_info *info; >> struct power_supply *bat; >> + const struct chip_data *data; >> + const struct power_supply_desc *bat_desc; >> int ret; >> >> info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL); >> if (!info) >> return -ENOMEM; >> >> + data = device_get_match_data(dev); >> + if (!data) >> + return dev_err_probe(dev, -EINVAL, "Failed to get chip data\n"); >> + >> psy_cfg.drv_data = info; >> psy_cfg.fwnode = dev_fwnode(dev); >> - psy_cfg.attr_grp = max1720x_groups; >> + switch (data->id) { >> + case MAX1720X_ID: >> + psy_cfg.attr_grp = max1720x_groups; >> + bat_desc = &max1720x_bat_desc; >> + break; >> + case MAX77759_ID: >> + bat_desc = &max77759_bat_desc; >> + break; >> + default: >> + return dev_err_probe(dev, -EINVAL, "Unsupported chip\n"); >> + } > nit: add empty line > Will add in v4. [...] Best regards, Thomas Antoine ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/5] power: supply: add support for max77759 fuel gauge 2025-04-21 18:13 ` [PATCH v3 2/5] power: supply: add support for max77759 fuel gauge Thomas Antoine via B4 Relay 2025-04-22 18:48 ` Dimitri Fedrau @ 2025-04-30 0:36 ` Sebastian Reichel 2025-04-30 13:49 ` Thomas Antoine 1 sibling, 1 reply; 17+ messages in thread From: Sebastian Reichel @ 2025-04-30 0:36 UTC (permalink / raw) To: t.antoine Cc: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Dimitri Fedrau, linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm [-- Attachment #1: Type: text/plain, Size: 2655 bytes --] Hi, On Mon, Apr 21, 2025 at 08:13:33PM +0200, Thomas Antoine via B4 Relay wrote: > From: Thomas Antoine <t.antoine@uclouvain.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@uclouvain.be> > --- [...] (I actually skipped reviewing big parts here for now) > > + ret = of_property_read_u32(dev->of_node, > + "shunt-resistor-micro-ohms", &val); device_property_read_u32(dev, ...) > [...] > + ret = of_property_read_u32(dev->of_node, > + "charge-full-design-microamp-hours", &info->charge_full_design); > + if (ret) > + info->charge_full_design = 0; This is not in the DT binding and thus not allowed. Also I will NAK adding it to the DT binding, since the following should be used: Documentation/devicetree/bindings/power/supply/battery.yaml followed by using power_supply_get_battery_info() in this driver. Adding this support is probably a good idea, since it allows providing all kind of static information from DT and you are missing the non-volatile memory part from the existing chip. Note that the power-supply core will pick up and expose any of these properties automatically. > + ret = max1720x_get_rsense(dev, info, data); > if (ret) > - return dev_err_probe(dev, ret, "Failed to probe nvmem\n"); > + return dev_err_probe(dev, ret, "Failed to get RSense\n"); You can either drop this error print, or the ones in max1720x_get_rsense(). No need to print two errors. Greetings, -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/5] power: supply: add support for max77759 fuel gauge 2025-04-30 0:36 ` Sebastian Reichel @ 2025-04-30 13:49 ` Thomas Antoine 0 siblings, 0 replies; 17+ messages in thread From: Thomas Antoine @ 2025-04-30 13:49 UTC (permalink / raw) To: Sebastian Reichel Cc: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Dimitri Fedrau, linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm Hi, On 4/30/25 02:36, Sebastian Reichel wrote: > Hi, > > On Mon, Apr 21, 2025 at 08:13:33PM +0200, Thomas Antoine via B4 Relay wrote: >> From: Thomas Antoine <t.antoine@uclouvain.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@uclouvain.be> >> --- > > [...] (I actually skipped reviewing big parts here for now) > >> >> + ret = of_property_read_u32(dev->of_node, >> + "shunt-resistor-micro-ohms", &val); > > device_property_read_u32(dev, ...) > >> [...] >> + ret = of_property_read_u32(dev->of_node, >> + "charge-full-design-microamp-hours", &info->charge_full_design); >> + if (ret) >> + info->charge_full_design = 0; > > This is not in the DT binding and thus not allowed. Also I will NAK > adding it to the DT binding, since the following should be used: > > Documentation/devicetree/bindings/power/supply/battery.yaml > > followed by using power_supply_get_battery_info() in this driver. > > Adding this support is probably a good idea, since it allows > providing all kind of static information from DT and you are > missing the non-volatile memory part from the existing chip. > > Note that the power-supply core will pick up and expose any > of these properties automatically. > As I said to Dimitri Fedrau, this is actually a mistake on my part coming from an attempt at getting things working which failed. Charge full design should be correctly computed now so I don't think it would be useful to get it from DT. >> + ret = max1720x_get_rsense(dev, info, data); >> if (ret) >> - return dev_err_probe(dev, ret, "Failed to probe nvmem\n"); >> + return dev_err_probe(dev, ret, "Failed to get RSense\n"); > > You can either drop this error print, or the ones in > max1720x_get_rsense(). No need to print two errors. > > Greetings, > > -- Sebastian I will do that in v4. Best regards, Thomas ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 3/5] dt-bindings: power: supply: add max77759-fg flavor 2025-04-21 18:13 [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support Thomas Antoine via B4 Relay 2025-04-21 18:13 ` [PATCH v3 1/5] power: supply: correct capacity computation Thomas Antoine via B4 Relay 2025-04-21 18:13 ` [PATCH v3 2/5] power: supply: add support for max77759 fuel gauge Thomas Antoine via B4 Relay @ 2025-04-21 18:13 ` Thomas Antoine via B4 Relay 2025-04-22 10:05 ` Krzysztof Kozlowski 2025-04-21 18:13 ` [PATCH v3 4/5] arm64: defconfig: enable Maxim max1720x driver Thomas Antoine via B4 Relay ` (2 subsequent siblings) 5 siblings, 1 reply; 17+ messages in thread From: Thomas Antoine via B4 Relay @ 2025-04-21 18:13 UTC (permalink / raw) To: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, Dimitri Fedrau Cc: linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm, Thomas Antoine From: Thomas Antoine <t.antoine@uclouvain.be> The Maxim MAX77759 is an IC used to manage the power supply of the battery and the USB-C. Based on drivers from google, it contains at least a PMIC, a fuel gauge, a TCPCI and a charger. Use max77759-fg compatible to avoid conflict with drivers for other functions. The Maxim MAX77759 has no non-volatile memory so it doesn't require an address and instead requires a value for the current sensing resistor. Signed-off-by: Thomas Antoine <t.antoine@uclouvain.be> --- .../bindings/power/supply/maxim,max17201.yaml | 34 ++++++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max17201.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max17201.yaml index fe3dd9bd5585618e45220c51023391a5b21acfd2..4823021ff16b170db83abd0b974986a307c05089 100644 --- a/Documentation/devicetree/bindings/power/supply/maxim,max17201.yaml +++ b/Documentation/devicetree/bindings/power/supply/maxim,max17201.yaml @@ -9,13 +9,11 @@ title: Maxim MAX17201 fuel gauge maintainers: - Dimitri Fedrau <dima.fedrau@gmail.com> -allOf: - - $ref: power-supply.yaml# - properties: compatible: oneOf: - const: maxim,max17201 + - const: maxim,max77759-fg - items: - enum: - maxim,max17205 @@ -39,6 +37,36 @@ required: - reg - reg-names +allOf: + - $ref: power-supply.yaml# + - if: + properties: + compatible: + contains: + enum: + - maxim,max17201 + then: + properties: + reg: + minItems: 2 + maxItems: 2 + - if: + properties: + compatible: + contains: + enum: + - maxim,max77759-fg + then: + properties: + reg: + items: + minItems: 1 + maxItems: 1 + shunt-resistor-micro-ohms: + description: The value of current sense resistor in microohms. + required: + - shunt-resistor-micro-ohms + unevaluatedProperties: false examples: -- 2.49.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/5] dt-bindings: power: supply: add max77759-fg flavor 2025-04-21 18:13 ` [PATCH v3 3/5] dt-bindings: power: supply: add max77759-fg flavor Thomas Antoine via B4 Relay @ 2025-04-22 10:05 ` Krzysztof Kozlowski 2025-04-23 8:05 ` Thomas Antoine 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2025-04-22 10:05 UTC (permalink / raw) To: Thomas Antoine Cc: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, Dimitri Fedrau, linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm On Mon, Apr 21, 2025 at 08:13:34PM GMT, Thomas Antoine wrote: > +allOf: > + - $ref: power-supply.yaml# > + - if: > + properties: > + compatible: > + contains: > + enum: > + - maxim,max17201 > + then: > + properties: > + reg: > + minItems: 2 > + maxItems: 2 > + - if: > + properties: > + compatible: > + contains: > + enum: > + - maxim,max77759-fg > + then: > + properties: > + reg: > + items: > + minItems: 1 If there is going to be resend, drop minItems. > + maxItems: 1 > + shunt-resistor-micro-ohms: > + description: The value of current sense resistor in microohms. Property should be defined top-level list of properties and in other variant if:then: you disallow it if it is not applicable at all (shunt-resistor-micro-ohms: false). Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/5] dt-bindings: power: supply: add max77759-fg flavor 2025-04-22 10:05 ` Krzysztof Kozlowski @ 2025-04-23 8:05 ` Thomas Antoine 2025-04-23 13:45 ` Krzysztof Kozlowski 0 siblings, 1 reply; 17+ messages in thread From: Thomas Antoine @ 2025-04-23 8:05 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, Dimitri Fedrau, linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm On 4/22/25 12:05, Krzysztof Kozlowski wrote: > On Mon, Apr 21, 2025 at 08:13:34PM GMT, Thomas Antoine wrote: >> +allOf: >> + - $ref: power-supply.yaml# >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - maxim,max17201 >> + then: >> + properties: >> + reg: >> + minItems: 2 >> + maxItems: 2 >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - maxim,max77759-fg >> + then: >> + properties: >> + reg: >> + items: >> + minItems: 1 > If there is going to be resend, drop minItems. > Will drop it in v4. >> + maxItems: 1 >> + shunt-resistor-micro-ohms: >> + description: The value of current sense resistor in microohms. > Property should be defined top-level list of properties and in other > variant if:then: you disallow it if it is not applicable at all > (shunt-resistor-micro-ohms: false). > Will change in v4. > Best regards, > Krzysztof > Best regards, Thomas Antoine ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/5] dt-bindings: power: supply: add max77759-fg flavor 2025-04-23 8:05 ` Thomas Antoine @ 2025-04-23 13:45 ` Krzysztof Kozlowski 2025-04-30 13:51 ` Thomas Antoine 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2025-04-23 13:45 UTC (permalink / raw) To: Thomas Antoine Cc: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, Dimitri Fedrau, linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm On 23/04/2025 10:05, Thomas Antoine wrote: > On 4/22/25 12:05, Krzysztof Kozlowski wrote: >> On Mon, Apr 21, 2025 at 08:13:34PM GMT, Thomas Antoine wrote: >>> +allOf: >>> + - $ref: power-supply.yaml# >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - maxim,max17201 >>> + then: >>> + properties: >>> + reg: >>> + minItems: 2 >>> + maxItems: 2 >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - maxim,max77759-fg >>> + then: >>> + properties: >>> + reg: >>> + items: >>> + minItems: 1 >> If there is going to be resend, drop minItems. >> > Will drop it in v4. > One more thing - your reg-names are now incorrectly constrained - where are their constraints? You need to test your bindings and DTS before you post. If by any chance community robots found more issues (e.g. you sent something untested), you should address it. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/5] dt-bindings: power: supply: add max77759-fg flavor 2025-04-23 13:45 ` Krzysztof Kozlowski @ 2025-04-30 13:51 ` Thomas Antoine 0 siblings, 0 replies; 17+ messages in thread From: Thomas Antoine @ 2025-04-30 13:51 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, Dimitri Fedrau, linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm Hello, On 4/23/25 15:45, Krzysztof Kozlowski wrote: > On 23/04/2025 10:05, Thomas Antoine wrote: >> On 4/22/25 12:05, Krzysztof Kozlowski wrote: >>> On Mon, Apr 21, 2025 at 08:13:34PM GMT, Thomas Antoine wrote: >>>> +allOf: >>>> + - $ref: power-supply.yaml# >>>> + - if: >>>> + properties: >>>> + compatible: >>>> + contains: >>>> + enum: >>>> + - maxim,max17201 >>>> + then: >>>> + properties: >>>> + reg: >>>> + minItems: 2 >>>> + maxItems: 2 >>>> + - if: >>>> + properties: >>>> + compatible: >>>> + contains: >>>> + enum: >>>> + - maxim,max77759-fg >>>> + then: >>>> + properties: >>>> + reg: >>>> + items: >>>> + minItems: 1 >>> If there is going to be resend, drop minItems. >>> >> Will drop it in v4. >> > One more thing - your reg-names are now incorrectly constrained - where > are their constraints? > > You need to test your bindings and DTS before you post. If by any chance > community robots found more issues (e.g. you sent something untested), > you should address it. > > > Best regards, > Krzysztof I did run the test but I obviously must have done it wrong. I will check it out such that it does not happen again. Best regards, Thomas ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 4/5] arm64: defconfig: enable Maxim max1720x driver 2025-04-21 18:13 [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support Thomas Antoine via B4 Relay ` (2 preceding siblings ...) 2025-04-21 18:13 ` [PATCH v3 3/5] dt-bindings: power: supply: add max77759-fg flavor Thomas Antoine via B4 Relay @ 2025-04-21 18:13 ` Thomas Antoine via B4 Relay 2025-04-21 18:13 ` [PATCH v3 5/5] arm64: dts: exynos: gs101-oriole: enable Maxim max77759 fuel gauge Thomas Antoine via B4 Relay 2025-04-21 21:53 ` [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support Rob Herring (Arm) 5 siblings, 0 replies; 17+ messages in thread From: Thomas Antoine via B4 Relay @ 2025-04-21 18:13 UTC (permalink / raw) To: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, Dimitri Fedrau Cc: linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm, Thomas Antoine From: Thomas Antoine <t.antoine@uclouvain.be> Enable the Maxim MAX1720x as it is used by the gs101-oriole (Google Pixel 6) board. Signed-off-by: Thomas Antoine <t.antoine@uclouvain.be> --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 5bb8f09422a22116781169611482179b10798c14..4feaebca340b1bc613c4f4ea9f1ff3b6686b092d 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -687,6 +687,7 @@ CONFIG_BATTERY_QCOM_BATTMGR=m CONFIG_BATTERY_SBS=m CONFIG_BATTERY_BQ27XXX=y CONFIG_BATTERY_MAX17042=m +CONFIG_BATTERY_MAX1720X=m CONFIG_CHARGER_MT6360=m CONFIG_CHARGER_BQ25890=m CONFIG_CHARGER_BQ25980=m -- 2.49.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 5/5] arm64: dts: exynos: gs101-oriole: enable Maxim max77759 fuel gauge 2025-04-21 18:13 [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support Thomas Antoine via B4 Relay ` (3 preceding siblings ...) 2025-04-21 18:13 ` [PATCH v3 4/5] arm64: defconfig: enable Maxim max1720x driver Thomas Antoine via B4 Relay @ 2025-04-21 18:13 ` Thomas Antoine via B4 Relay 2025-04-21 21:53 ` [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support Rob Herring (Arm) 5 siblings, 0 replies; 17+ messages in thread From: Thomas Antoine via B4 Relay @ 2025-04-21 18:13 UTC (permalink / raw) To: Rob Herring, Peter Griffin, André Draszik, Tudor Ambarus, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar, Catalin Marinas, Will Deacon, Sebastian Reichel, Dimitri Fedrau Cc: linux-arm-kernel, linux-samsung-soc, devicetree, linux-kernel, linux-pm, Thomas Antoine From: Thomas Antoine <t.antoine@uclouvain.be> Add the node for the Maxim MAX77759 fuel gauge as a slave of the i2c. The TODO is still applicable given there are other slaves on the bus (e.g. PCA9468, other MAX77759 functions and the MAX20339 OVP). Signed-off-by: Thomas Antoine <t.antoine@uclouvain.be> --- arch/arm64/boot/dts/exynos/google/gs101-pixel-common.dtsi | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/arch/arm64/boot/dts/exynos/google/gs101-pixel-common.dtsi b/arch/arm64/boot/dts/exynos/google/gs101-pixel-common.dtsi index b25230495c64dce60916b7cd5dcb9a7cce5d0e4e..84fc10c3562958ab1621f24644709e85a9433b9b 100644 --- a/arch/arm64/boot/dts/exynos/google/gs101-pixel-common.dtsi +++ b/arch/arm64/boot/dts/exynos/google/gs101-pixel-common.dtsi @@ -10,6 +10,7 @@ #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/input/input.h> +#include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/usb/pd.h> #include "gs101-pinctrl.h" #include "gs101.dtsi" @@ -188,6 +189,15 @@ usbc0_role_sw: endpoint { }; }; }; + + fuel-gauge@36 { + compatible = "maxim,max77759-fg"; + reg = <0x36>; + reg-names = "m5"; + interrupt-parent = <&gpa9>; + interrupts = <3 IRQ_TYPE_LEVEL_LOW>; + shunt-resistor-micro-ohms = <5000>; + }; }; &pinctrl_far_alive { -- 2.49.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support 2025-04-21 18:13 [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support Thomas Antoine via B4 Relay ` (4 preceding siblings ...) 2025-04-21 18:13 ` [PATCH v3 5/5] arm64: dts: exynos: gs101-oriole: enable Maxim max77759 fuel gauge Thomas Antoine via B4 Relay @ 2025-04-21 21:53 ` Rob Herring (Arm) 5 siblings, 0 replies; 17+ messages in thread From: Rob Herring (Arm) @ 2025-04-21 21:53 UTC (permalink / raw) To: Thomas Antoine Cc: linux-arm-kernel, linux-samsung-soc, Alim Akhtar, linux-pm, Tudor Ambarus, Sebastian Reichel, linux-kernel, Peter Griffin, Conor Dooley, Dimitri Fedrau, André Draszik, Krzysztof Kozlowski, Will Deacon, devicetree, Catalin Marinas On Mon, 21 Apr 2025 20:13:31 +0200, Thomas Antoine wrote: > The Google Pixel 6 has a Maxim MAX77759 which provides a fuel gauge with > an interface with a lot in common with the Maxim max1720x. > > Modify the Maxim MAX1720x driver to be compatible with the Maxim MAX77759 > and enable it for the gs101-oriole board. > > The voltage, current, capacity, temperature and charge have all been > tested and show coherent results. The charge full design and capacity > equal the ones seen on android, the ratio between average charge and > average current does predict pretty accurately the time to empty under > a constant workload and temperature is coherent with the dynamic state > of the device. > > Health is not enabled as it always reports overheating. The time to empty > is wrong by about a factor 2 and is thus also disabled. > > Signed-off-by: Thomas Antoine <t.antoine@uclouvain.be> > > --- > Changes in v3: > - Update base tree to avoid conflicts > - Fix capacity computation for max1720x > - Add separate properties for the max7759 to disable non-functional ones > - Take TASKPERIOD into account for voltage computation of max77759 > - Simplify vcell computation (Dimitri Fedrau) > - Switch has_nvmem to bool and keep it only in chip_data (Dimitri Fedrau) > - Drop the yes_range from the write table (Sebastian Reichel) > - Add test_power_supply_properties.sh to cover letter (Sebastian Reichel) > - Switch back some changes to binding and actually use allOf:if: to > restrict constraints (Krzysztof Kozlowski) > - Fix style errors > - Link to v2: https://lore.kernel.org/r/20250102-b4-gs101_max77759_fg-v2-0-87959abeb7ff@uclouvain.be > > Changes in v2: > - Add fallback for voltage measurement (André Draszik) > - Add regmap for the max77759 (André Draszik) > - Add chip identification for the max77759 (André Draszik, Peter Griffin) > - Move RSense value to a devicetree property shunt-resistor-micro-ohms > (Dimitri Fedrau, André Draszik) > - Use allOf:if to narrow binding per variant (Krzysztof Kozlowski) > - Remove binding example (Krzysztof Kozlowski) > - Change defconfig order to follow savedefconfig (Krzysztof Kozlowski) > - Fix style errors > - Link to v1: https://lore.kernel.org/r/20241202-b4-gs101_max77759_fg-v1-0-98d2fa7bfe30@uclouvain.be > > tools/testing/selftests/power_supply/test_power_supply_properties.sh: > ok 1 max77759-fg.exists > ok 2 max77759-fg.uevent.NAME > ok 3 max77759-fg.sysfs.type > ok 4 max77759-fg.uevent.TYPE > ok 5 max77759-fg.sysfs.usb_type # SKIP > ok 6 max77759-fg.sysfs.online # SKIP > ok 7 max77759-fg.sysfs.present > ok 8 max77759-fg.sysfs.status # SKIP > ok 9 max77759-fg.sysfs.capacity > ok 10 max77759-fg.sysfs.capacity_level # SKIP > ok 11 max77759-fg.sysfs.model_name > ok 12 max77759-fg.sysfs.manufacturer > ok 13 max77759-fg.sysfs.serial_number # SKIP > ok 14 max77759-fg.sysfs.technology # SKIP > ok 15 max77759-fg.sysfs.cycle_count # SKIP > ok 16 max77759-fg.sysfs.scope # SKIP > ok 17 max77759-fg.sysfs.input_current_limit # SKIP(Dimitri Fedrau) > ok 18 max77759-fg.sysfs.input_voltage_limit # SKIP > ok 19 max77759-fg.sysfs.voltage_now > ok 20 max77759-fg.sysfs.voltage_min # SKIP > ok 21 max77759-fg.sysfs.voltage_max # SKIP > ok 22 max77759-fg.sysfs.voltage_min_design # SKIP > ok 23 max77759-fg.sysfs.voltage_max_design # SKIP > ok 24 max77759-fg.sysfs.current_now > ok 25 max77759-fg.sysfs.current_max # SKIP > ok 26 max77759-fg.sysfs.charge_now # SKIP > ok 27 max77759-fg.sysfs.charge_full > ok 28 max77759-fg.sysfs.charge_full_design > ok 29 max77759-fg.sysfs.power_now # SKIP > ok 30 max77759-fg.sysfs.energy_now # SKIP > ok 31 max77759-fg.sysfs.energy_full # SKIP > ok 32 max77759-fg.sysfs.energy_full_design # SKIP > ok 33 max77759-fg.sysfs.energy_full_design # SKIP > > --- > Thomas Antoine (5): > power: supply: correct capacity computation > power: supply: add support for max77759 fuel gauge > dt-bindings: power: supply: add max77759-fg flavor > arm64: defconfig: enable Maxim max1720x driver > arm64: dts: exynos: gs101-oriole: enable Maxim max77759 fuel gauge > > .../bindings/power/supply/maxim,max17201.yaml | 34 ++- > .../boot/dts/exynos/google/gs101-pixel-common.dtsi | 10 + > arch/arm64/configs/defconfig | 1 + > drivers/power/supply/max1720x_battery.c | 271 ++++++++++++++++++--- > 4 files changed, 280 insertions(+), 36 deletions(-) > --- > base-commit: e48e99b6edf41c69c5528aa7ffb2daf3c59ee105 > change-id: 20241202-b4-gs101_max77759_fg-402e231a4b33 > > Best regards, > -- > Thomas Antoine <t.antoine@uclouvain.be> > > > My bot found new DTB warnings on the .dts files added or changed in this series. Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not. No need to reply unless the platform maintainer has comments. If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date: pip3 install dtschema --upgrade This patch series was applied (using b4) to base: Base: using specified base-commit e48e99b6edf41c69c5528aa7ffb2daf3c59ee105 If this is not the correct base, please add 'base-commit' tag (or use b4 which does this automatically) New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/exynos/' for 20250421-b4-gs101_max77759_fg-v3-0-50cd8caf9017@uclouvain.be: arch/arm64/boot/dts/exynos/google/gs101-raven.dtb: fuel-gauge@36 (maxim,max77759-fg): reg: [[54]] is too short from schema $id: http://devicetree.org/schemas/power/supply/maxim,max17201.yaml# arch/arm64/boot/dts/exynos/google/gs101-raven.dtb: fuel-gauge@36 (maxim,max77759-fg): reg-names: ['m5'] is too short from schema $id: http://devicetree.org/schemas/power/supply/maxim,max17201.yaml# arch/arm64/boot/dts/exynos/google/gs101-raven.dtb: fuel-gauge@36 (maxim,max77759-fg): Unevaluated properties are not allowed ('reg-names' was unexpected) from schema $id: http://devicetree.org/schemas/power/supply/maxim,max17201.yaml# arch/arm64/boot/dts/exynos/google/gs101-oriole.dtb: fuel-gauge@36 (maxim,max77759-fg): reg: [[54]] is too short from schema $id: http://devicetree.org/schemas/power/supply/maxim,max17201.yaml# arch/arm64/boot/dts/exynos/google/gs101-oriole.dtb: fuel-gauge@36 (maxim,max77759-fg): reg-names: ['m5'] is too short from schema $id: http://devicetree.org/schemas/power/supply/maxim,max17201.yaml# arch/arm64/boot/dts/exynos/google/gs101-oriole.dtb: fuel-gauge@36 (maxim,max77759-fg): Unevaluated properties are not allowed ('reg-names' was unexpected) from schema $id: http://devicetree.org/schemas/power/supply/maxim,max17201.yaml# ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2025-04-30 13:51 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-21 18:13 [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support Thomas Antoine via B4 Relay 2025-04-21 18:13 ` [PATCH v3 1/5] power: supply: correct capacity computation Thomas Antoine via B4 Relay 2025-04-22 9:46 ` Dimitri Fedrau 2025-04-23 7:48 ` Thomas Antoine 2025-04-21 18:13 ` [PATCH v3 2/5] power: supply: add support for max77759 fuel gauge Thomas Antoine via B4 Relay 2025-04-22 18:48 ` Dimitri Fedrau 2025-04-23 8:02 ` Thomas Antoine 2025-04-30 0:36 ` Sebastian Reichel 2025-04-30 13:49 ` Thomas Antoine 2025-04-21 18:13 ` [PATCH v3 3/5] dt-bindings: power: supply: add max77759-fg flavor Thomas Antoine via B4 Relay 2025-04-22 10:05 ` Krzysztof Kozlowski 2025-04-23 8:05 ` Thomas Antoine 2025-04-23 13:45 ` Krzysztof Kozlowski 2025-04-30 13:51 ` Thomas Antoine 2025-04-21 18:13 ` [PATCH v3 4/5] arm64: defconfig: enable Maxim max1720x driver Thomas Antoine via B4 Relay 2025-04-21 18:13 ` [PATCH v3 5/5] arm64: dts: exynos: gs101-oriole: enable Maxim max77759 fuel gauge Thomas Antoine via B4 Relay 2025-04-21 21:53 ` [PATCH v3 0/5] Google Pixel 6 (oriole): max77759 fuel gauge enablement and driver support Rob Herring (Arm)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).