Hello Vincent, On Tue, Jul 21, 2026 at 02:57:34PM -0400, Vincent Cloutier wrote: > From: Vincent Cloutier > > MAX17055 can use charge-full-design-microamp-hours and > charge-term-current-microamp from monitored-battery for its POR > configuration. Prepare quantized DesignCap, IChgTerm, and EZ Config dQAcc > values in the power-supply registration callback after the core has parsed > battery information. > > Validate all supplied values before changing the sparse configuration. > Reject zero, values that quantize to zero, capacities that cannot produce > dQAcc, and positive termination currents outside the signed register range. > Limit this path to MAX17055 because the other supported gauges require > complete characterization data. > > Follow the documented POR sequence. Wait for FSTAT.DNR, save HibCfg, exit > hibernate, write stable battery registers once and verify them after 1 ms, > issue command writes directly, poll until refresh clears, and restore > HibCfg. Clear POR only after every step succeeds. > > Keep driver-backed properties unavailable while initialization is > incomplete. If an I/O error interrupts the transaction, restore hibernate > before the next attempt and retry periodically so a transient startup > failure does not become permanent. > > Assisted-by: OpenCode:gpt-5.6-sol > Signed-off-by: Vincent Cloutier > --- Please split this into smaller patches doing one thing at a time, which makes it a lot easier to review and also helps to identify potential bugs with git bisect in the future. At least the init_complete and the change from 'struct work_struct work' to 'struct delayed_work work' could be done in advance. > drivers/power/supply/max17042_battery.c | 335 +++++++++++++++++++++--- > include/linux/power/max17042_battery.h | 2 + > 2 files changed, 306 insertions(+), 31 deletions(-) > > diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c > index d409d2f0d383..b25bf26135e0 100644 > --- a/drivers/power/supply/max17042_battery.c > +++ b/drivers/power/supply/max17042_battery.c > @@ -62,18 +62,30 @@ > #define MAX17042_RESISTANCE_LSB 1 / 4096 /* Ω */ > #define MAX17042_TEMPERATURE_LSB 1 / 256 /* °C */ > > +#define MAX17055_DQACC_DIV 32 > +#define MAX17055_DPACC_FACTOR 44138 > +#define MAX17055_DPACC_VCHG_FACTOR 51200 > +#define MAX17055_FSTAT_DNR_BIT BIT(0) > +#define MAX17055_DNR_POLL_US 10000 > +#define MAX17055_DNR_TIMEOUT_US 2000000 > +#define MAX17055_INIT_RETRY_DELAY_MS 10000 > +#define MAX17055_REFRESH_POLL_US 10000 > +#define MAX17055_REFRESH_TIMEOUT_US 1000000 > + > struct max17042_chip { > struct device *dev; > struct regmap *regmap; > struct power_supply *battery; > enum max170xx_chip_type chip_type; > struct max17042_config_data *config_data; > - struct work_struct work; > - int init_complete; > + struct delayed_work work; > int irq; > int task_period; > bool enable_current_sense; > bool enable_por_init; > + bool init_complete; > + bool hib_restore_pending; > + u16 hib_cfg; > unsigned int r_sns; > int vmin; /* in millivolts */ > int vmax; /* in millivolts */ > @@ -256,7 +268,7 @@ static int max17042_get_property(struct power_supply *psy, > u32 data; > u64 data64; > > - if (!chip->init_complete) > + if (!READ_ONCE(chip->init_complete)) > return -EAGAIN; > > switch (psp) { > @@ -566,6 +578,24 @@ static int max17042_write_verify_reg(struct regmap *map, u8 reg, u32 value) > return ret; > } > > +static int max17055_write_verify_reg(struct regmap *map, u8 reg, u32 value) > +{ > + u32 read_value; > + int ret; > + > + ret = regmap_write(map, reg, value); > + if (ret) > + return ret; > + > + usleep_range(1000, 2000); > + > + ret = regmap_read(map, reg, &read_value); > + if (ret) > + return ret; > + > + return read_value == value ? 0 : -EIO; > +} > + > static inline void max17042_override_por(struct regmap *map, > u8 reg, u16 value) > { > @@ -801,8 +831,12 @@ static inline void max17042_override_por_values(struct max17042_chip *chip) > max17042_override_por(map, MAX17042_CONFIG, config->config); > max17042_override_por(map, MAX17042_SHDNTIMER, config->shdntimer); > > - max17042_override_por(map, MAX17042_DesignCap, config->design_cap); > - max17042_override_por(map, MAX17042_ICHGTerm, config->ichgt_term); > + if (chip->chip_type != MAXIM_DEVICE_TYPE_MAX17055) { > + max17042_override_por(map, MAX17042_DesignCap, > + config->design_cap); > + max17042_override_por(map, MAX17042_ICHGTerm, > + config->ichgt_term); > + } > > max17042_override_por(map, MAX17042_AtRate, config->at_rate); > max17042_override_por(map, MAX17042_LearnCFG, config->learn_cfg); > @@ -812,8 +846,10 @@ static inline void max17042_override_por_values(struct max17042_chip *chip) > > max17042_override_por(map, MAX17042_FullCAP, config->fullcap); > max17042_override_por(map, MAX17042_FullCAPNom, config->fullcapnom); > - max17042_override_por(map, MAX17042_dQacc, config->dqacc); > - max17042_override_por(map, MAX17042_dPacc, config->dpacc); > + if (chip->chip_type != MAXIM_DEVICE_TYPE_MAX17055) { > + max17042_override_por(map, MAX17042_dQacc, config->dqacc); > + max17042_override_por(map, MAX17042_dPacc, config->dpacc); > + } > > max17042_override_por(map, MAX17042_RCOMP0, config->rcomp0); > max17042_override_por(map, MAX17042_TempCo, config->tcompc0); > @@ -847,26 +883,162 @@ static inline void max17042_override_por_values(struct max17042_chip *chip) > max17042_override_por(map, MAX17055_ModelCfg, config->model_cfg); > } > > -static int max17042_init_chip(struct max17042_chip *chip) > +static int max17055_override_battery_values(struct max17042_chip *chip) > { > + struct max17042_config_data *config = chip->config_data; > struct regmap *map = chip->regmap; > + unsigned int design_cap; > + unsigned int model_cfg; > + unsigned int dqacc; > + u64 dpacc; > int ret; > > + if (config->design_cap) { > + ret = max17055_write_verify_reg(map, MAX17042_DesignCap, > + config->design_cap); > + if (ret) > + return ret; > + } > + > + if (config->dqacc) { > + ret = max17055_write_verify_reg(map, MAX17042_dQacc, > + config->dqacc); > + if (ret) > + return ret; > + } > + > + if (config->ichgt_term) { > + ret = max17055_write_verify_reg(map, MAX17042_ICHGTerm, > + config->ichgt_term); > + if (ret) > + return ret; > + } > + > + if (!config->design_cap && !config->dqacc) > + return 0; > + > + ret = regmap_read(map, MAX17042_DesignCap, &design_cap); > + if (ret) > + return ret; > + > + ret = regmap_read(map, MAX17042_dQacc, &dqacc); > + if (ret) > + return ret; > + > + ret = regmap_read(map, MAX17055_ModelCfg, &model_cfg); > + if (ret) > + return ret; > + > + if (!design_cap || !dqacc) > + return -ERANGE; > + > + dpacc = (u64)dqacc * > + (model_cfg & MAX17055_MODELCFG_VCHG_BIT ? > + MAX17055_DPACC_VCHG_FACTOR : MAX17055_DPACC_FACTOR); > + do_div(dpacc, design_cap); > + if (dpacc > U16_MAX) > + return -ERANGE; > + > + return max17055_write_verify_reg(map, MAX17042_dPacc, (u16)dpacc); > +} > + > +static int max17055_restore_hibernate(struct max17042_chip *chip) > +{ > + int restore_hib_ret; > + int soft_wakeup_ret; > + > + soft_wakeup_ret = regmap_write(chip->regmap, MAX17055_SoftWakeup, 0); > + restore_hib_ret = max17055_write_verify_reg(chip->regmap, > + MAX17055_HibCfg, > + chip->hib_cfg); > + if (!soft_wakeup_ret && !restore_hib_ret) > + chip->hib_restore_pending = false; > + > + return soft_wakeup_ret ?: restore_hib_ret; > +} > + > +static int max17055_init_chip(struct max17042_chip *chip) > +{ > + struct regmap *map = chip->regmap; > + unsigned int hib_cfg; > + unsigned int model_cfg; > + unsigned int fstat; > + int restore_ret; > + int ret; > + > + if (chip->hib_restore_pending) { > + ret = max17055_restore_hibernate(chip); > + if (ret) > + return ret; > + } > + > + ret = regmap_read_poll_timeout(map, MAX17042_FSTAT, fstat, > + !(fstat & MAX17055_FSTAT_DNR_BIT), > + MAX17055_DNR_POLL_US, > + MAX17055_DNR_TIMEOUT_US); > + if (ret) > + return ret; > + > + ret = regmap_read(map, MAX17055_HibCfg, &hib_cfg); > + if (ret) > + return ret; > + > + chip->hib_cfg = hib_cfg; > + chip->hib_restore_pending = true; > + > + ret = regmap_write(map, MAX17055_SoftWakeup, 0x0090); > + if (ret) > + goto restore_hibernate; > + > + ret = max17055_write_verify_reg(map, MAX17055_HibCfg, 0); > + if (ret) > + goto restore_hibernate; > + > + ret = regmap_write(map, MAX17055_SoftWakeup, 0); > + if (ret) > + goto restore_hibernate; > + > max17042_override_por_values(chip); > > + ret = max17055_override_battery_values(chip); > + if (ret) > + goto restore_hibernate; > + > + ret = regmap_write_bits(map, MAX17055_ModelCfg, > + MAX17055_MODELCFG_REFRESH_BIT, > + MAX17055_MODELCFG_REFRESH_BIT); > + if (ret) > + goto restore_hibernate; > + > + ret = regmap_read_poll_timeout(map, MAX17055_ModelCfg, model_cfg, > + !(model_cfg & > + MAX17055_MODELCFG_REFRESH_BIT), > + MAX17055_REFRESH_POLL_US, > + MAX17055_REFRESH_TIMEOUT_US); > + > +restore_hibernate: > + restore_ret = max17055_restore_hibernate(chip); > + if (restore_ret) > + return restore_ret; > + > + return ret; > +} > + > +static int max17042_init_chip(struct max17042_chip *chip) > +{ > + struct regmap *map = chip->regmap; > + int ret; > + > if (chip->chip_type == MAXIM_DEVICE_TYPE_MAX17055) { > - regmap_write_bits(map, MAX17055_ModelCfg, > - MAX17055_MODELCFG_REFRESH_BIT, > - MAX17055_MODELCFG_REFRESH_BIT); > - } > + ret = max17055_init_chip(chip); > + if (ret) > + return ret; > + } else { > + max17042_override_por_values(chip); > > - /* After Power up, the MAX17042 requires 500mS in order > - * to perform signal debouncing and initial SOC reporting > - */ > - msleep(500); > + /* Allow signal debouncing and initial SOC reporting. */ > + msleep(500); > > - if (chip->chip_type != MAXIM_DEVICE_TYPE_MAX17055) { > - /* Initialize configuration */ > max17042_write_config_regs(chip); > > /* write cell characterization data */ > @@ -902,8 +1074,7 @@ static int max17042_init_chip(struct max17042_chip *chip) > } > > /* Init complete, Clear the POR bit */ > - regmap_update_bits(map, MAX17042_STATUS, STATUS_POR_BIT, 0x0); > - return 0; > + return regmap_clear_bits(map, MAX17042_STATUS, STATUS_POR_BIT); another correct, but unrelated change. > } > > static void max17042_set_soc_threshold(struct max17042_chip *chip, u16 off) > @@ -989,18 +1160,28 @@ static irqreturn_t max17042_thread_handler(int id, void *dev) > > static void max17042_init_worker(struct work_struct *work) > { > - struct max17042_chip *chip = container_of(work, > + struct max17042_chip *chip = container_of(to_delayed_work(work), > struct max17042_chip, work); > - int ret; > + int ret = 0; > > /* Initialize registers according to values from config_data */ > - if (chip->enable_por_init && chip->config_data) { > + if (chip->enable_por_init && chip->config_data) > ret = max17042_init_chip(chip); > - if (ret) > - return; > + > + if (ret) { > + if (chip->chip_type == MAXIM_DEVICE_TYPE_MAX17055) { > + dev_warn_ratelimited(chip->dev, > + "initialization failed: %d, retrying\n", ret); > + queue_delayed_work(system_freezable_wq, &chip->work, > + msecs_to_jiffies(MAX17055_INIT_RETRY_DELAY_MS)); > + } else { > + dev_err(chip->dev, "initialization failed: %d\n", ret); > + } > + return; > } > > - chip->init_complete = 1; > + WRITE_ONCE(chip->init_complete, true); > + power_supply_changed(chip->battery); > } > > #ifdef CONFIG_OF > @@ -1060,6 +1241,93 @@ static int max17042_init_defaults(struct max17042_chip *chip) > return 0; > } > > +static int max17042_apply_battery_properties(struct max17042_chip *chip, > + struct power_supply_battery_info *info) > +{ > + struct max17042_config_data *config; > + struct device *dev = chip->dev; > + bool have_design_cap; > + bool have_ichgt_term; > + u16 design_cap = 0; > + u16 ichgt_term = 0; > + u16 dqacc = 0; > + u64 data64; > + > + if (!info || chip->chip_type != MAXIM_DEVICE_TYPE_MAX17055) > + return 0; > + > + have_design_cap = chip->enable_current_sense && > + info->charge_full_design_uah >= 0; > + have_ichgt_term = chip->enable_current_sense && > + info->charge_term_current_ua >= 0; > + if (!have_design_cap && !have_ichgt_term) > + return 0; > + > + if (have_design_cap) { > + if (!info->charge_full_design_uah) > + return dev_err_probe(dev, -EINVAL, > + "battery design capacity must be positive\n"); If it's negative you set have_design_cap=false. This would only be printed for charge_full_design_uah = 0. It seems more sensible to also set have_design_cap to false for this case and drop this extra check. Greetings, -- Sebastian > + data64 = (u64)info->charge_full_design_uah * chip->r_sns; > + do_div(data64, MAX17042_CAPACITY_LSB); > + if (!data64) > + return dev_err_probe(dev, -ERANGE, > + "battery design capacity is too small for sense resistor\n"); > + if (data64 > U16_MAX) > + return dev_err_probe(dev, -ERANGE, > + "battery design capacity exceeds register range\n"); > + > + design_cap = (u16)data64; > + dqacc = design_cap / MAX17055_DQACC_DIV; > + if (!dqacc) > + return dev_err_probe(dev, -ERANGE, > + "battery design capacity is too small for EZ config\n"); > + } > + > + if (have_ichgt_term) { > + if (!info->charge_term_current_ua) > + return dev_err_probe(dev, -EINVAL, > + "charge termination current must be positive\n"); > + > + data64 = (u64)info->charge_term_current_ua * chip->r_sns; > + do_div(data64, MAX17042_CURRENT_LSB); > + if (!data64) > + return dev_err_probe(dev, -ERANGE, > + "charge termination current is too small for sense resistor\n"); > + if (data64 > S16_MAX) > + return dev_err_probe(dev, -ERANGE, > + "charge termination current exceeds positive register range\n"); > + > + ichgt_term = (u16)data64; > + } > + > + config = chip->config_data; > + if (!config) { > + config = devm_kzalloc(dev, sizeof(*config), GFP_KERNEL); > + if (!config) > + return -ENOMEM; > + } > + > + if (have_design_cap) { > + config->design_cap = design_cap; > + config->dqacc = dqacc; > + } > + if (have_ichgt_term) > + config->ichgt_term = ichgt_term; > + > + chip->config_data = config; > + chip->enable_por_init = true; > + > + return 0; > +} > + > +static int max17042_init_battery(struct power_supply *psy) > +{ > + struct max17042_chip *chip = power_supply_get_drvdata(psy); > + > + return max17042_apply_battery_properties(chip, psy->battery_info); > +} > + > static const struct regmap_config max17042_regmap_config = { > .name = "max17042", > .reg_bits = 8, > @@ -1113,6 +1381,7 @@ static const struct power_supply_desc max17042_psy_desc = { > .set_property = max17042_set_property, > .property_is_writeable = max17042_property_is_writeable, > .external_power_changed = power_supply_changed, > + .init = max17042_init_battery, > .properties = max17042_battery_props, > .num_properties = ARRAY_SIZE(max17042_battery_props), > }; > @@ -1123,6 +1392,7 @@ static const struct power_supply_desc max17042_no_current_sense_psy_desc = { > .get_property = max17042_get_property, > .set_property = max17042_set_property, > .property_is_writeable = max17042_property_is_writeable, > + .init = max17042_init_battery, > .properties = max17042_battery_props, > .num_properties = ARRAY_SIZE(max17042_battery_props) - 2, > }; > @@ -1242,15 +1512,18 @@ static int max17042_probe(struct i2c_client *client, struct device *dev, int irq > > chip->irq = irq; > > - regmap_read(chip->regmap, MAX17042_STATUS, &val); > + ret = regmap_read(chip->regmap, MAX17042_STATUS, &val); > + if (ret) > + return dev_err_probe(dev, ret, "failed to read status\n"); > + > if (val & STATUS_POR_BIT) { > - ret = devm_work_autocancel(dev, &chip->work, > - max17042_init_worker); > + ret = devm_delayed_work_autocancel(dev, &chip->work, > + max17042_init_worker); > if (ret) > return ret; > - schedule_work(&chip->work); > + queue_delayed_work(system_freezable_wq, &chip->work, 0); > } else { > - chip->init_complete = 1; > + WRITE_ONCE(chip->init_complete, true); > } > > return 0; > diff --git a/include/linux/power/max17042_battery.h b/include/linux/power/max17042_battery.h > index 13aeab1597c6..810e068eafd2 100644 > --- a/include/linux/power/max17042_battery.h > +++ b/include/linux/power/max17042_battery.h > @@ -25,6 +25,7 @@ > #define MAX17042_CHARACTERIZATION_DATA_SIZE 48 > > #define MAX17055_MODELCFG_REFRESH_BIT BIT(15) > +#define MAX17055_MODELCFG_VCHG_BIT BIT(10) > > enum max17042_register { > MAX17042_STATUS = 0x00, > @@ -124,6 +125,7 @@ enum max17055_register { > > MAX17055_ConvgCfg = 0x49, > MAX17055_VFRemCap = 0x4A, > + MAX17055_SoftWakeup = 0x60, > > MAX17055_STATUS2 = 0xB0, > MAX17055_POWER = 0xB1, > -- > 2.55.0 > >