Re: [PATCH net 07/10] net: pse-pd: pd692x0: release the manager power budget through devres

[Kory Maincent <kory.maincent@bootlin.com>]

On 10/18/26 10:23, Carlo Szelinsky wrote:
> pd692x0 reserves a power budget on each manager's supply in
> setup_pi_matrix(), but gives it back only on that function's own errors
> and in remove(). If probe fails later, or the request fails for one
> manager after others succeeded, the budget stays reserved on vmain. Each
> probe retry, e.g. after -EPROBE_DEFER, reserves it again until nothing
> is left.
> 
> Release the budget from a devm action instead, and drop the explicit
> release calls.

Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

Thank you!

> Fixes: 1c67f9c54cdc ("net: pse-pd: pd692x0: Fix power budget leak in manager setup error path")
> Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
> ---
>   drivers/net/pse-pd/pd692x0.c | 22 +++++++++++++++-------
>   1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
> index 209de9cec849..34f0dfbccd06 100644
> --- a/drivers/net/pse-pd/pd692x0.c
> +++ b/drivers/net/pse-pd/pd692x0.c
> @@ -1224,8 +1224,9 @@ static void pd692x0_of_put_managers(struct pd692x0_priv *priv,
>   	}
>   }
>   
> -static void pd692x0_managers_free_pw_budget(struct pd692x0_priv *priv)
> +static void pd692x0_managers_free_pw_budget(void *data)
>   {
> +	struct pd692x0_priv *priv = data;
>   	int i;
>   
>   	for (i = 0; i < PD692X0_MAX_MANAGERS; i++) {
> @@ -1279,31 +1280,39 @@ static int pd692x0_setup_pi_matrix(struct pse_controller_dev *pcdev)
>   	if (ret)
>   		goto err_of_managers;
>   
> +	/* Frees whatever pd692x0_req_managers_pw_budget() manages to
> +	 * request, on any later probe failure and on unbind. Added after
> +	 * the manager regulators so that it runs before devres unregisters
> +	 * them and their supply goes away.
> +	 */
> +	ret = devm_add_action_or_reset(&priv->client->dev,
> +				       pd692x0_managers_free_pw_budget, priv);
> +	if (ret)
> +		goto err_of_managers;
> +
>   	ret = pd692x0_req_managers_pw_budget(priv);
>   	if (ret)
>   		goto err_of_managers;
>   
>   	ret = pd692x0_set_ports_matrix(priv, manager);
>   	if (ret)
> -		goto err_managers_req_pw;
> +		goto err_of_managers;
>   
>   	/* Do not init the conf if it is already saved */
>   	if (!priv->cfg_saved) {
>   		ret = pd692x0_hw_conf_init(priv);
>   		if (ret)
> -			goto err_managers_req_pw;
> +			goto err_of_managers;
>   
>   		ret = pd692x0_save_user_byte(priv);
>   		if (ret)
> -			goto err_managers_req_pw;
> +			goto err_of_managers;
>   	}
>   
>   	pd692x0_of_put_managers(priv, manager);
>   	kfree(manager);
>   	return 0;
>   
> -err_managers_req_pw:
> -	pd692x0_managers_free_pw_budget(priv);
>   err_of_managers:
>   	pd692x0_of_put_managers(priv, manager);
>   err_free_manager:
> @@ -1855,7 +1864,6 @@ static void pd692x0_i2c_remove(struct i2c_client *client)
>   {
>   	struct pd692x0_priv *priv = i2c_get_clientdata(client);
>   
> -	pd692x0_managers_free_pw_budget(priv);
>   	firmware_upload_unregister(priv->fwl);
>   }
>   

-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com