linux-kernel - Re: [PATCH v2 2/2] mmc: sdhci-omap: use regulator_get_optional() and reuse pbias in sdhci_omap_regulator_get

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] [day] [month] [year] [list]

Message-ID: <460f183c-8d88-48e3-b8c9-37d0184e1a33@intel.com>
Date: Fri, 29 Aug 2025 09:11:34 +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 2/2] mmc: sdhci-omap: use regulator_get_optional() and
 reuse pbias in sdhci_omap_regulator_get_caps()

On 25/08/2025 15:21, Alexander Stein wrote:
> From: Matthias Schiffer <matthias.schiffer@...group.com>
> 
> We actually want to get an error return instead of a dummy regulator
> when a supply is not set.

Please explain why here.

> Change regulator_get() to
> regulator_get_optional() for the vqmmc supply and reuse omap_host->pbias,
> which is already initialized at this point.

Probably better to make "reuse omap_host->pbias" a separate patch

> 
> This change also avoids warning messages:
> 
>     sdhci-omap 48060000.mmc: supply pbias not found, using dummy regulator
>     sdhci-omap 48060000.mmc: supply vqmmc not found, using dummy regulator
> 
> Fixes: de5ccd2af71f ("mmc: sdhci-omap: Handle voltages to add support omap4")
> Signed-off-by: Matthias Schiffer <matthias.schiffer@...group.com>
> Signed-off-by: Alexander Stein <alexander.stein@...tq-group.com>
> ---
>  drivers/mmc/host/sdhci-omap.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index 08d5a82b7d01b..4623781adba7b 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -939,16 +939,10 @@ static const struct sdhci_ops sdhci_omap_ops = {
>  	.set_timeout = sdhci_omap_set_timeout,
>  };
>  
> -static unsigned int sdhci_omap_regulator_get_caps(struct device *dev,
> -						  const char *name)
> +static unsigned int sdhci_omap_regulator_get_caps(struct regulator *reg)
>  {
> -	struct regulator *reg;
>  	unsigned int caps = 0;
>  
> -	reg = regulator_get(dev, name);
> -	if (IS_ERR(reg))
> -		return ~0U;
> -
>  	if (regulator_is_supported_voltage(reg, 1700000, 1950000) > 0)
>  		caps |= SDHCI_CAN_VDD_180;
>  	if (regulator_is_supported_voltage(reg, 2700000, 3150000) > 0)
> @@ -956,8 +950,6 @@ static unsigned int sdhci_omap_regulator_get_caps(struct device *dev,
>  	if (regulator_is_supported_voltage(reg, 3150000, 3600000) > 0)
>  		caps |= SDHCI_CAN_VDD_330;
>  
> -	regulator_put(reg);
> -
>  	return caps;
>  }
>  
> @@ -967,11 +959,20 @@ static int sdhci_omap_set_capabilities(struct sdhci_host *host)
>  	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
>  	struct device *dev = omap_host->dev;
>  	const u32 mask = SDHCI_CAN_VDD_180 | SDHCI_CAN_VDD_300 | SDHCI_CAN_VDD_330;
> -	unsigned int pbias, vqmmc, caps = 0;
> +	unsigned int pbias = ~0U, vqmmc = ~0U, caps = 0;
> +	struct	regulator *reg_vqmmc;
>  	u32 reg;
>  
> -	pbias = sdhci_omap_regulator_get_caps(dev, "pbias");
> -	vqmmc = sdhci_omap_regulator_get_caps(dev, "vqmmc");
> +	if (!IS_ERR(omap_host->pbias))
> +		pbias = sdhci_omap_regulator_get_caps(omap_host->pbias);
> +
> +	/* mmc->supply.vqmmc is not initialized yet */
> +	reg_vqmmc = regulator_get_optional(dev, "vqmmc");
> +	if (!IS_ERR(reg_vqmmc)) {
> +		vqmmc = sdhci_omap_regulator_get_caps(reg_vqmmc);
> +		regulator_put(reg_vqmmc);
> +	}
> +
>  	caps = pbias & vqmmc;
>  
>  	if (pbias != ~0U && vqmmc == ~0U)