Re: [PATCH v2 1/2] mmc: sdhci-omap: properly check regulator_is_supported_voltage() return

[Adrian Hunter]

On 25/08/2025 15:21, Alexander Stein wrote:
> From: Matthias Schiffer <matthias.schiffer@xxxxxxxxxxxx>
> 
> regulator_is_supported_voltage() returns negative values on errors.
> 
> Note that this patch might result in regressions on existing boards that
> should support voltage switching, but don't have their regulators set up
> correctly.
> 
> Fixes: de5ccd2af71f ("mmc: sdhci-omap: Handle voltages to add support omap4")

It is not very clear what is being fixed nor why.

It changes from "setting a capability if there is an error" to
"not setting a capability if there is an error".  Both seem less
than ideal.

Also "might result in regressions" is really not suitable for
stable, which is probably where it will end up if it has a Fixes tag.

> Signed-off-by: Matthias Schiffer <matthias.schiffer@xxxxxxxxxxxx>
> Signed-off-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
> ---
> New in v2
> 
>  drivers/mmc/host/sdhci-omap.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index b5d7c1a80a92f..08d5a82b7d01b 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -949,11 +949,11 @@ static unsigned int sdhci_omap_regulator_get_caps(struct device *dev,
>  	if (IS_ERR(reg))
>  		return ~0U;
>  
> -	if (regulator_is_supported_voltage(reg, 1700000, 1950000))
> +	if (regulator_is_supported_voltage(reg, 1700000, 1950000) > 0)

Why is the error ignored?

>  		caps |= SDHCI_CAN_VDD_180;
> -	if (regulator_is_supported_voltage(reg, 2700000, 3150000))
> +	if (regulator_is_supported_voltage(reg, 2700000, 3150000) > 0)
>  		caps |= SDHCI_CAN_VDD_300;
> -	if (regulator_is_supported_voltage(reg, 3150000, 3600000))
> +	if (regulator_is_supported_voltage(reg, 3150000, 3600000) > 0)
>  		caps |= SDHCI_CAN_VDD_330;
>  
>  	regulator_put(reg);