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

[Amit Sunil Dhamne <amitsd@google.com>]

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'