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

[Adrian Hunter <adrian.hunter@intel.com>]

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