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

[Amit Sunil Dhamne]

Hi Andre',

On 2/19/26 7:52 AM, André Draszik wrote:
> Hi Amit,
>
> I was trying out your series and noticed a few things that I didn't before:
>
> On Wed, 2026-02-18 at 21:59 +0000, Amit Sunil Dhamne via B4 Relay wrote:
>
> [...]
>
> > diff --git a/drivers/power/supply/max77759_charger.c b/drivers/power/supply/max77759_charger.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..035f16822d85de94c7707ce11c49345c714cd559
> > --- /dev/null
> > +++ b/drivers/power/supply/max77759_charger.c
> [...]
>
> > +		dev_err(chg->dev, "Invalid mode transition from %d to %d",
> > +			chg->mode, mode);
> All your format strings are missing the final \n throughout this patch.

Sorry will fix. I think this is because of my muscle memory from writing 
`tcpm_log` (which doesn't require an escape character).


> [...]
>
> > +static int max77759_charger_probe(struct platform_device *pdev)
> > +{
> > +	struct regulator_config chgin_otg_reg_cfg;
> > +	struct power_supply_config psy_cfg;
> > +	struct device *dev = &pdev->dev;
> > +	struct max77759_charger *chg;
> > +	int ret;
> > +
> > +	device_set_of_node_from_dev(dev, dev->parent);
> > +	chg = devm_kzalloc(dev, sizeof(*chg), GFP_KERNEL);
> > +	if (!chg)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, chg);
> > +	chg->dev = dev;
> > +	chg->regmap = dev_get_regmap(dev->parent, "charger");
> > +	if (!chg->regmap)
> > +		return dev_err_probe(dev, -ENODEV, "Missing regmap");
> > +
> > +	ret = devm_mutex_init(dev, &chg->lock);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failed to initialize lock");
> > +
> > +	ret = devm_mutex_init(dev, &chg->retry_lock);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret,
> > +				     "Failed to initialize retry_lock");
> > +
> > +	psy_cfg.fwnode = dev_fwnode(dev);
> > +	psy_cfg.drv_data = chg;
> > +	chg->psy = devm_power_supply_register(dev, &max77759_charger_desc,
> > +					      &psy_cfg);
> > +	if (IS_ERR(chg->psy))
> > +		return dev_err_probe(dev, -EPROBE_DEFER,
> > +				     "Failed to register psy, ret=%ld",
> > +				     PTR_ERR(chg->psy));
> Why are you returning -EPROBE_DEFER here instead of the original error? This
> is quite unusual.

Will fix. The original thought was that the 
power_supply_check_supplies() (run as part of power_supply_register()) 
wasn't deferring probe if it didn't find its provider device but I was 
mistaken.


Regards,

Amit

> Cheers,
> Andre'