Re: [PATCH v2 3/5] power: regulator: Exit from regulator_set_suspend_value on poorly described regulators

Hi Marek,

On 2024-09-25 04:21, Marek Vasut wrote:
> In case the DT regulator node does not contain 'regulator-max-microvolt'
> property and does not contain 'regulator-state-mem' subnode (like the
> test.dts regul1_scmi: reg@1 {} regulator node), then regulator_pre_probe()
> will parse this regulator node and set uc_pdata->suspend_on = true and
> uc_pdata->suspend_uV = uc_pdata->max_uV, where uc_pdata->max_uV is set
> to -ENODATA because "regulator-max-microvolt" is missing, and therefore
> uc_pdata->suspend_uV is also -ENODATA. In case regulator_autoset() is
> used afterward, it will attempt to call regulator_set_suspend_value()
> with uV = uc_pdata->suspend_uV = -ENODATA and fail with -EINVAL. Check
> for this case in regulator_set_suspend_value() and immediately return 0,
> because there is no way to set meaningful suspend voltage, so do nothing
> and retain the existing settings of the regulator.

I sent a different fix for this some time ago, please check it out:

https://patchwork.ozlabs.org/patch/1964571/

Regards,
Jonas

> 
> Signed-off-by: Marek Vasut <[email protected]>
> ---
> Cc: Ben Wolsieffer <[email protected]>
> Cc: Caleb Connolly <[email protected]>
> Cc: Chris Morgan <[email protected]>
> Cc: Dragan Simic <[email protected]>
> Cc: Eugen Hristev <[email protected]>
> Cc: Francesco Dolcini <[email protected]>
> Cc: Heinrich Schuchardt <[email protected]>
> Cc: Jaehoon Chung <[email protected]>
> Cc: Jagan Teki <[email protected]>
> Cc: Jonas Karlman <[email protected]>
> Cc: Kever Yang <[email protected]>
> Cc: Matteo Lisi <[email protected]>
> Cc: Mattijs Korpershoek <[email protected]>
> Cc: Max Krummenacher <[email protected]>
> Cc: Neil Armstrong <[email protected]>
> Cc: Patrice Chotard <[email protected]>
> Cc: Patrick Delaunay <[email protected]>
> Cc: Philipp Tomsich <[email protected]>
> Cc: Quentin Schulz <[email protected]>
> Cc: Sam Day <[email protected]>
> Cc: Simon Glass <[email protected]>
> Cc: Sumit Garg <[email protected]>
> Cc: Svyatoslav Ryhel <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Cc: Tom Rini <[email protected]>
> Cc: Volodymyr Babchuk <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> V2: New patch
> ---
>  drivers/power/regulator/regulator-uclass.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> index 14cf3159203..3c05fdf1966 100644
> --- a/drivers/power/regulator/regulator-uclass.c
> +++ b/drivers/power/regulator/regulator-uclass.c
> @@ -85,6 +85,10 @@ int regulator_set_suspend_value(struct udevice *dev, int uV)
>  	const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
>  	struct dm_regulator_uclass_plat *uc_pdata;
>  
> +	/* Regulator did not set limits, assume already configured. */
> +	if (uV == -ENODATA)
> +		return 0;
> +
>  	uc_pdata = dev_get_uclass_plat(dev);
>  	if (uc_pdata->min_uV != -ENODATA && uV < uc_pdata->min_uV)
>  		return -EINVAL;