Re: [PATCH v6 5/6] power: supply: max77759: add charger driver

[Amit Sunil Dhamne]

Hi André,


On 2/17/26 5:14 AM, André Draszik wrote:
> Hi Amit,
>
> All below comments are only minor, feel free to ignore them.
>
> On Sat, 2026-02-14 at 03:12 +0000, Amit Sunil Dhamne via B4 Relay wrote:
>> From: Amit Sunil Dhamne <amitsd@xxxxxxxxxx>
>>
>> Add support for MAX77759 battery charger driver. This is a 4A 1-Cell
>> Li+/LiPoly dual input switch mode charger. While the device can support
>> USB & wireless charger inputs, this implementation only supports USB
>> input. This implementation supports both buck and boost modes.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@xxxxxxxxxx>
>> ---
>>  MAINTAINERS                             |   6 +
>>  drivers/power/supply/Kconfig            |  11 +
>>  drivers/power/supply/Makefile           |   1 +
>>  drivers/power/supply/max77759_charger.c | 768 ++++++++++++++++++++++++++++++++
>>  4 files changed, 786 insertions(+)
> [...]
>
>> diff --git a/drivers/power/supply/max77759_charger.c b/drivers/power/supply/max77759_charger.c
>> new file mode 100644
>> index 000000000000..d4e02764ba04
>> --- /dev/null
>> +++ b/drivers/power/supply/max77759_charger.c
>> @@ -0,0 +1,768 @@
> [...]
>
>> +
>> +/* USB input current limits (in uA) */
>> +static const struct linear_range chgin_ilim_ranges[] = {
>> +	LINEAR_RANGE(100000, 0x3, 0x7F, 25000),
>> +};
> Shouldn't this one also have a entry for 0x00...0x02:
> 	LINEAR_RANGE(100000, 0x0, 0x2, 0),
>
> Then you can also drop the umax() call in get_input_current_limit().
>
> Ah, I see now there is no linear_range_get_selector_within_array(),
> meaning the code is fine as-is, unless you want to add that as
> well :-)
>
>
> [...]

I would go with the code being as is for now.  :-)


>
>> +static int max77759_charger_init(struct max77759_charger *chg)
>> +{
>> +	struct power_supply_battery_info *info;
>> +	u32 regval, fast_chg_curr, fv;
>> +	int ret;
>> +
>> +	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_00, &regval);
>> +	if (ret)
>> +		return ret;
>> +
>> +	chg->mode = FIELD_GET(MAX77759_CHGR_REG_CHG_CNFG_00_MODE, regval);
>> +	ret = charger_set_mode(chg, MAX77759_CHGR_MODE_OFF);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (power_supply_get_battery_info(chg->psy, &info)) {
>> +		fv = CHG_FV_DEFAULT_MV;
>> +		fast_chg_curr = CHG_CC_DEFAULT_UA;
>> +	} else {
>> +		fv = info->constant_charge_voltage_max_uv / 1000;
>> +		fast_chg_curr = info->constant_charge_current_max_ua;
>> +	}
>> +
>> +	ret = set_fast_charge_current_limit(chg, fast_chg_curr);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = set_float_voltage_limit(chg, fv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = unlock_prot_regs(chg, true);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Disable wireless charging input */
>> +	ret = regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_12,
>> +				 MAX77759_CHGR_REG_CHG_CNFG_12_WCINSEL, 0);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_18,
>> +				 MAX77759_CHGR_REG_CHG_CNFG_18_WDTEN, 0);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return unlock_prot_regs(chg, false);
> Should early error returns here try to lock the protection again? Something
> like:
>
> +	ret = unlock_prot_regs(chg, true);
> +	if (ret)
> +		return ret;
> +
> +	/* Disable wireless charging input */
> +	ret = regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_12,
> +				 MAX77759_CHGR_REG_CHG_CNFG_12_WCINSEL, 0);
> +	if (ret)
> +		goto relock;
> +
> +	ret = regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_18,
> +				 MAX77759_CHGR_REG_CHG_CNFG_18_WDTEN, 0);
> +	if (ret)
> +		goto relock;
> +
> +	return unlock_prot_regs(chg, false);
> +
> +relock:
> +	(void) unlock_prot_regs(chg, false);
> +	return ret;
>
> I guess if one of the regmap_update_bits() failed, then locking the
> registers might not work either, so I have no strong opinion on
> adding that.

Nice catch!

I need to send a next revision to keep the Linux Test Robot happy. I
will address this issue in that.


>
> With or without updates:
>
> Reviewed-by: André Draszik <andre.draszik@xxxxxxxxxx>
>
LGTM! Thanks!


Regards,

Amit

> Cheers,
> Andre'