Re: [PATCH V1 3/3] mmc: sdhci: Allow platform controlled voltage switching

[Veerabhadrarao Badiganti <vbadigan@xxxxxxxxxxxxxx>]

On 5/19/2020 11:36 AM, Adrian Hunter wrote:
> On 15/05/20 2:18 pm, Veerabhadrarao Badiganti wrote:
> > From: Vijay Viswanath <vviswana@xxxxxxxxxxxxxx>
> >
> > If vendor platform drivers are controlling whole logic of voltage
> > switching, then sdhci driver no need control vqmmc regulator.
> > So skip enabling/disable vqmmc from SDHC driver.
> >
> > Signed-off-by: Vijay Viswanath <vviswana@xxxxxxxxxxxxxx>
> > Signed-off-by: Veerabhadrarao Badiganti <vbadigan@xxxxxxxxxxxxxx>
> > ---
> >   drivers/mmc/host/sdhci.c | 32 +++++++++++++++++++-------------
> >   drivers/mmc/host/sdhci.h |  1 +
> >   2 files changed, 20 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 1bb6b67..c010823 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -4098,6 +4098,7 @@ int sdhci_setup_host(struct sdhci_host *host)
> >   	unsigned int override_timeout_clk;
> >   	u32 max_clk;
> >   	int ret;
> > +	bool enable_vqmmc = false;
> >   
> >   	WARN_ON(host == NULL);
> >   	if (host == NULL)
> > @@ -4111,9 +4112,12 @@ int sdhci_setup_host(struct sdhci_host *host)
> >   	 * the host can take the appropriate action if regulators are not
> >   	 * available.
> >   	 */
> > -	ret = mmc_regulator_get_supply(mmc);
> > -	if (ret)
> > -		return ret;
> > +	if (!mmc->supply.vqmmc) {
> > +		ret = mmc_regulator_get_supply(mmc);
> > +		if (ret)
> > +			return ret;
> > +		enable_vqmmc  = true;
> > +	}
> >   
> >   	DBG("Version:   0x%08x | Present:  0x%08x\n",
> >   	    sdhci_readw(host, SDHCI_HOST_VERSION),
> > @@ -4373,7 +4377,15 @@ int sdhci_setup_host(struct sdhci_host *host)
> >   		mmc->caps |= MMC_CAP_NEEDS_POLL;
> >   
> >   	if (!IS_ERR(mmc->supply.vqmmc)) {
> > -		ret = regulator_enable(mmc->supply.vqmmc);
> > +		if (enable_vqmmc) {
> > +			ret = regulator_enable(mmc->supply.vqmmc);
> > +			if (ret) {
> > +				pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
> > +					mmc_hostname(mmc), ret);
> > +				mmc->supply.vqmmc = ERR_PTR(-EINVAL);
> > +			}
> > +			host->vqmmc_enabled = !ret;
> > +		}
> >   
> >   		/* If vqmmc provides no 1.8V signalling, then there's no UHS */
> >   		if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
> > @@ -4386,12 +4398,6 @@ int sdhci_setup_host(struct sdhci_host *host)
> >   		if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 2700000,
> >   						    3600000))
> >   			host->flags &= ~SDHCI_SIGNALING_330;
> > -
> > -		if (ret) {
> > -			pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
> > -				mmc_hostname(mmc), ret);
> > -			mmc->supply.vqmmc = ERR_PTR(-EINVAL);
> > -		}
> >   	}
> >   
> >   	if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) {
> > @@ -4625,7 +4631,7 @@ int sdhci_setup_host(struct sdhci_host *host)
> >   	return 0;
> >   
> >   unreg:
> > -	if (!IS_ERR(mmc->supply.vqmmc))
> > +	if (host->vqmmc_enabled)
> >   		regulator_disable(mmc->supply.vqmmc);
> >   undma:
> >   	if (host->align_buffer)
> > @@ -4643,7 +4649,7 @@ void sdhci_cleanup_host(struct sdhci_host *host)
> >   {
> >   	struct mmc_host *mmc = host->mmc;
> >   
> > -	if (!IS_ERR(mmc->supply.vqmmc))
> > +	if (host->vqmmc_enabled)
> >   		regulator_disable(mmc->supply.vqmmc);
> >   
> >   	if (host->align_buffer)
> > @@ -4780,7 +4786,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
> >   
> >   	destroy_workqueue(host->complete_wq);
> >   
> > -	if (!IS_ERR(mmc->supply.vqmmc))
> > +	if (host->vqmmc_enabled)
> >   		regulator_disable(mmc->supply.vqmmc);
> >   
> >   	if (host->align_buffer)
> > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> > index 8d2a096..24d27e1 100644
> > --- a/drivers/mmc/host/sdhci.h
> > +++ b/drivers/mmc/host/sdhci.h
> > @@ -570,6 +570,7 @@ struct sdhci_host {
> >   	u32 caps1;		/* CAPABILITY_1 */
> >   	bool read_caps;		/* Capability flags have been read */
> >   
> > +	bool vqmmc_enabled;	/* Vqmmc is enabled */
> Last time around there was dissatisfaction with this variable name.  Perhaps
> change it to sdhci_core_to_disable_vqmmc

Sure Adrian. Will update this variable name.

> >   	unsigned int            ocr_avail_sdio;	/* OCR bit masks */
> >   	unsigned int            ocr_avail_sd;
> >   	unsigned int            ocr_avail_mmc;