linux-kernel - Re: [PATCH v2 1/2] mmc: sdhci-omap: properly check regulator_is_supported

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dba29577-c685-40bb-8bed-482aad351edc@intel.com>
Date: Fri, 29 Aug 2025 07:55:14 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Alexander Stein <alexander.stein@...tq-group.com>, Vignesh Raghavendra
	<vigneshr@...com>, Ulf Hansson <ulf.hansson@...aro.org>, Liam Girdwood
	<lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Tony Lindgren
	<tony@...mide.com>
CC: Matthias Schiffer <matthias.schiffer@...group.com>,
	<linux-mmc@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] mmc: sdhci-omap: properly check
 regulator_is_supported_voltage() return

On 25/08/2025 15:21, Alexander Stein wrote:
> From: Matthias Schiffer <matthias.schiffer@...group.com>
> 
> 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@...group.com>
> Signed-off-by: Alexander Stein <alexander.stein@...tq-group.com>
> ---
> 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);