[RFC,V3,15/21] mmc: sdhci: UHS-II support, modify set_power() to handle vdd2

Message ID 20200710111140.29725-1-benchuanggli@gmail.com
State New
Headers show
Series
  • Untitled series #44483
Related show

Commit Message

Ben Chuang July 10, 2020, 11:11 a.m.
From: AKASHI Takahiro <takahiro.akashi@linaro.org>


VDD2 is used for powering UHS-II interface.
Modify sdhci_set_power_and_bus_voltage(), sdhci_set_power_noreg()
and sdhci_set_power_noreg() to handle VDD2.

Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw>

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

---
 drivers/mmc/host/sdhci-omap.c     |  2 +-
 drivers/mmc/host/sdhci-pci-core.c |  4 +--
 drivers/mmc/host/sdhci-pxav3.c    |  4 +--
 drivers/mmc/host/sdhci-xenon.c    |  4 +--
 drivers/mmc/host/sdhci.c          | 42 ++++++++++++++++++++++++-------
 drivers/mmc/host/sdhci.h          |  9 +++----
 6 files changed, 43 insertions(+), 22 deletions(-)

-- 
2.27.0

Comments

Adrian Hunter Aug. 21, 2020, 2:11 p.m. | #1
On 10/07/20 2:11 pm, Ben Chuang wrote:
> From: AKASHI Takahiro <takahiro.akashi@linaro.org>

> 

> VDD2 is used for powering UHS-II interface.

> Modify sdhci_set_power_and_bus_voltage(), sdhci_set_power_noreg()

> and sdhci_set_power_noreg() to handle VDD2.


vdd2 is always 1.8 V and I suspect there may never be support for anything
else, so we should start with 1.8 V only.

Also can we create uhs2_set_power_reg() and uhs2_set_power_noreg() and use
the existing ->set_power() callback

> 

> Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw>

> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

> ---

>  drivers/mmc/host/sdhci-omap.c     |  2 +-

>  drivers/mmc/host/sdhci-pci-core.c |  4 +--

>  drivers/mmc/host/sdhci-pxav3.c    |  4 +--

>  drivers/mmc/host/sdhci-xenon.c    |  4 +--

>  drivers/mmc/host/sdhci.c          | 42 ++++++++++++++++++++++++-------

>  drivers/mmc/host/sdhci.h          |  9 +++----

>  6 files changed, 43 insertions(+), 22 deletions(-)

> 

> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c

> index 1ec74c2d5c17..1926585debe5 100644

> --- a/drivers/mmc/host/sdhci-omap.c

> +++ b/drivers/mmc/host/sdhci-omap.c

> @@ -678,7 +678,7 @@ static void sdhci_omap_set_clock(struct sdhci_host *host, unsigned int clock)

>  }

>  

>  static void sdhci_omap_set_power(struct sdhci_host *host, unsigned char mode,

> -			  unsigned short vdd)

> +			  unsigned short vdd, unsigned short vdd2)

>  {

>  	struct mmc_host *mmc = host->mmc;

>  

> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c

> index bb6802448b2f..40f5a24a8982 100644

> --- a/drivers/mmc/host/sdhci-pci-core.c

> +++ b/drivers/mmc/host/sdhci-pci-core.c

> @@ -629,12 +629,12 @@ static int bxt_get_cd(struct mmc_host *mmc)

>  #define SDHCI_INTEL_PWR_TIMEOUT_UDELAY	100

>  

>  static void sdhci_intel_set_power(struct sdhci_host *host, unsigned char mode,

> -				  unsigned short vdd)

> +				  unsigned short vdd, unsigned short vdd2)

>  {

>  	int cntr;

>  	u8 reg;

>  

> -	sdhci_set_power(host, mode, vdd);

> +	sdhci_set_power(host, mode, vdd, -1);

>  

>  	if (mode == MMC_POWER_OFF)

>  		return;

> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c

> index e55037ceda73..457e9425339a 100644

> --- a/drivers/mmc/host/sdhci-pxav3.c

> +++ b/drivers/mmc/host/sdhci-pxav3.c

> @@ -298,12 +298,12 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)

>  }

>  

>  static void pxav3_set_power(struct sdhci_host *host, unsigned char mode,

> -			    unsigned short vdd)

> +			    unsigned short vdd, unsigned short vdd2)

>  {

>  	struct mmc_host *mmc = host->mmc;

>  	u8 pwr = host->pwr;

>  

> -	sdhci_set_power_noreg(host, mode, vdd);

> +	sdhci_set_power_noreg(host, mode, vdd, -1);

>  

>  	if (host->pwr == pwr)

>  		return;

> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c

> index 4703cd540c7f..2b0ebb91895a 100644

> --- a/drivers/mmc/host/sdhci-xenon.c

> +++ b/drivers/mmc/host/sdhci-xenon.c

> @@ -214,12 +214,12 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,

>  }

>  

>  static void xenon_set_power(struct sdhci_host *host, unsigned char mode,

> -		unsigned short vdd)

> +		unsigned short vdd, unsigned short vdd2)

>  {

>  	struct mmc_host *mmc = host->mmc;

>  	u8 pwr = host->pwr;

>  

> -	sdhci_set_power_noreg(host, mode, vdd);

> +	sdhci_set_power_noreg(host, mode, vdd, -1);

>  

>  	if (host->pwr == pwr)

>  		return;

> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c

> index d38d734ec83f..ca3d4a506e01 100644

> --- a/drivers/mmc/host/sdhci.c

> +++ b/drivers/mmc/host/sdhci.c

> @@ -2089,12 +2089,15 @@ void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)

>  EXPORT_SYMBOL_GPL(sdhci_set_clock);

>  

>  static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,

> -				unsigned short vdd)

> +				unsigned short vdd, unsigned short vdd2)

>  {

>  	struct mmc_host *mmc = host->mmc;

>  

>  	mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);

>  

> +	if (mmc->caps & MMC_CAP_UHS2 && !IS_ERR(mmc->supply.vmmc2))

> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc2, vdd2);

> +

>  	if (mode != MMC_POWER_OFF)

>  		sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL);

>  	else

> @@ -2102,7 +2105,7 @@ static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,

>  }

>  

>  void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

> -			   unsigned short vdd)

> +			   unsigned short vdd, unsigned short vdd2)

>  {

>  	u8 pwr = 0;

>  

> @@ -2133,6 +2136,20 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

>  		}

>  	}

>  

> +	if (mode != MMC_POWER_OFF) {

> +		if (vdd2 != (unsigned short)-1) {

> +			switch (1 << vdd2) {

> +			case MMC_VDD2_165_195:

> +				pwr |= SDHCI_VDD2_POWER_180;

> +				break;

> +			default:

> +				WARN(1, "%s: Invalid vdd2 %#x\n",

> +				     mmc_hostname(host->mmc), vdd2);

> +				break;

> +			}

> +		}

> +	}

> +

>  	if (host->pwr == pwr)

>  		return;

>  

> @@ -2159,7 +2176,13 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

>  			sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);

>  

>  		pwr |= SDHCI_POWER_ON;

> +		if (vdd2 != (unsigned short)-1)

> +			pwr |= SDHCI_VDD2_POWER_ON;

> +

> +		sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL);

> +		mdelay(5);

>  		sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);

> +		mdelay(5);

>  

>  		if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)

>  			sdhci_runtime_pm_bus_on(host);

> @@ -2175,12 +2198,12 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

>  EXPORT_SYMBOL_GPL(sdhci_set_power_noreg);

>  

>  void sdhci_set_power(struct sdhci_host *host, unsigned char mode,

> -		     unsigned short vdd)

> +		     unsigned short vdd, unsigned short vdd2)

>  {

>  	if (IS_ERR(host->mmc->supply.vmmc))

> -		sdhci_set_power_noreg(host, mode, vdd);

> +		sdhci_set_power_noreg(host, mode, vdd, vdd2);

>  	else

> -		sdhci_set_power_reg(host, mode, vdd);

> +		sdhci_set_power_reg(host, mode, vdd, vdd2);

>  }

>  EXPORT_SYMBOL_GPL(sdhci_set_power);

>  

> @@ -2192,14 +2215,15 @@ EXPORT_SYMBOL_GPL(sdhci_set_power);

>   */

>  void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,

>  				     unsigned char mode,

> -				     unsigned short vdd)

> +				     unsigned short vdd,

> +				     unsigned short vdd2)

>  {

>  	if (!IS_ERR(host->mmc->supply.vmmc)) {

>  		struct mmc_host *mmc = host->mmc;

>  

>  		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);

>  	}

> -	sdhci_set_power_noreg(host, mode, vdd);

> +	sdhci_set_power_noreg(host, mode, vdd, -1);

>  }

>  EXPORT_SYMBOL_GPL(sdhci_set_power_and_bus_voltage);

>  

> @@ -2377,9 +2401,9 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

>  	}

>  

>  	if (host->ops->set_power)

> -		host->ops->set_power(host, ios->power_mode, ios->vdd);

> +		host->ops->set_power(host, ios->power_mode, ios->vdd, -1);

>  	else

> -		sdhci_set_power(host, ios->power_mode, ios->vdd);

> +		sdhci_set_power(host, ios->power_mode, ios->vdd, -1);

>  

>  	/* 4.0 host support */

>  	if (host->version >= SDHCI_SPEC_400) {

> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h

> index ecf55394ea46..f6732f33f29f 100644

> --- a/drivers/mmc/host/sdhci.h

> +++ b/drivers/mmc/host/sdhci.h

> @@ -689,7 +689,7 @@ struct sdhci_ops {

>  

>  	void	(*set_clock)(struct sdhci_host *host, unsigned int clock);

>  	void	(*set_power)(struct sdhci_host *host, unsigned char mode,

> -			     unsigned short vdd);

> +			     unsigned short vdd, unsigned short vdd2);

>  

>  	u32		(*irq)(struct sdhci_host *host, u32 intmask);

>  

> @@ -852,13 +852,10 @@ u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,

>  		   unsigned int *actual_clock);

>  void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);

>  void sdhci_enable_clk(struct sdhci_host *host, u16 clk);

> -void sdhci_set_power(struct sdhci_host *host, unsigned char mode,

> -		     unsigned short vdd);

>  void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,

>  				     unsigned char mode,

> -				     unsigned short vdd);

> -void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

> -			   unsigned short vdd);

> +				     unsigned short vdd,

> +				     unsigned short vdd2);

>  void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq);

>  int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq);

>  void sdhci_set_bus_width(struct sdhci_host *host, int width);

>
AKASHI Takahiro Sept. 14, 2020, 5:45 a.m. | #2
Adrian,

On Fri, Aug 21, 2020 at 05:11:18PM +0300, Adrian Hunter wrote:
> On 10/07/20 2:11 pm, Ben Chuang wrote:

> > From: AKASHI Takahiro <takahiro.akashi@linaro.org>

> > 

> > VDD2 is used for powering UHS-II interface.

> > Modify sdhci_set_power_and_bus_voltage(), sdhci_set_power_noreg()

> > and sdhci_set_power_noreg() to handle VDD2.

> 

> vdd2 is always 1.8 V and I suspect there may never be support for anything

> else, so we should start with 1.8 V only.


What do you mean here?
You don't want to add an extra argument, vdd2, to sdhci_set_power().
Correct?

> Also can we create uhs2_set_power_reg() and uhs2_set_power_noreg() and use

> the existing ->set_power() callback


Again what do you expect here?

Do you want to see any platform-specific mmc driver who supports UHS-II
to implement its own call back like:

void sdhci_foo_set_power(struct sdhci_host *host, unsigned char mode,
                                  unsigned short vdd)
{
        sdhci_set_power(host, mode,vdd);

        /* in case that sdhci_uhs2 module is not inserted */
        if (!(mmc->caps & MMC_CAP_UHS2))
                return;

        /* vdd2 specific operation */
        if (IS_ERR_OR_NULL(host->mmc->supply.vmmc2))
                sdhci_uhs2_set_power_noreg(host, mode);
        else
                sdhci_uhs2_set_power_reg(host, mode);

        /* maybe more platform-specific initialization */
}

struct sdhci_ops sdhci_foo_ops = {
        .set_power = sdhci_foo_set_power,
        ...
}

Is this what you mean?
(I'm not quite sure yet that sdhci_ush2_set_power_noreg() can be split off
from sdhci_set_power_noreg().)

-Takahiro Akashi

}

> > 

> > Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw>

> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

> > ---

> >  drivers/mmc/host/sdhci-omap.c     |  2 +-

> >  drivers/mmc/host/sdhci-pci-core.c |  4 +--

> >  drivers/mmc/host/sdhci-pxav3.c    |  4 +--

> >  drivers/mmc/host/sdhci-xenon.c    |  4 +--

> >  drivers/mmc/host/sdhci.c          | 42 ++++++++++++++++++++++++-------

> >  drivers/mmc/host/sdhci.h          |  9 +++----

> >  6 files changed, 43 insertions(+), 22 deletions(-)

> > 

> > diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c

> > index 1ec74c2d5c17..1926585debe5 100644

> > --- a/drivers/mmc/host/sdhci-omap.c

> > +++ b/drivers/mmc/host/sdhci-omap.c

> > @@ -678,7 +678,7 @@ static void sdhci_omap_set_clock(struct sdhci_host *host, unsigned int clock)

> >  }

> >  

> >  static void sdhci_omap_set_power(struct sdhci_host *host, unsigned char mode,

> > -			  unsigned short vdd)

> > +			  unsigned short vdd, unsigned short vdd2)

> >  {

> >  	struct mmc_host *mmc = host->mmc;

> >  

> > diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c

> > index bb6802448b2f..40f5a24a8982 100644

> > --- a/drivers/mmc/host/sdhci-pci-core.c

> > +++ b/drivers/mmc/host/sdhci-pci-core.c

> > @@ -629,12 +629,12 @@ static int bxt_get_cd(struct mmc_host *mmc)

> >  #define SDHCI_INTEL_PWR_TIMEOUT_UDELAY	100

> >  

> >  static void sdhci_intel_set_power(struct sdhci_host *host, unsigned char mode,

> > -				  unsigned short vdd)

> > +				  unsigned short vdd, unsigned short vdd2)

> >  {

> >  	int cntr;

> >  	u8 reg;

> >  

> > -	sdhci_set_power(host, mode, vdd);

> > +	sdhci_set_power(host, mode, vdd, -1);

> >  

> >  	if (mode == MMC_POWER_OFF)

> >  		return;

> > diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c

> > index e55037ceda73..457e9425339a 100644

> > --- a/drivers/mmc/host/sdhci-pxav3.c

> > +++ b/drivers/mmc/host/sdhci-pxav3.c

> > @@ -298,12 +298,12 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)

> >  }

> >  

> >  static void pxav3_set_power(struct sdhci_host *host, unsigned char mode,

> > -			    unsigned short vdd)

> > +			    unsigned short vdd, unsigned short vdd2)

> >  {

> >  	struct mmc_host *mmc = host->mmc;

> >  	u8 pwr = host->pwr;

> >  

> > -	sdhci_set_power_noreg(host, mode, vdd);

> > +	sdhci_set_power_noreg(host, mode, vdd, -1);

> >  

> >  	if (host->pwr == pwr)

> >  		return;

> > diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c

> > index 4703cd540c7f..2b0ebb91895a 100644

> > --- a/drivers/mmc/host/sdhci-xenon.c

> > +++ b/drivers/mmc/host/sdhci-xenon.c

> > @@ -214,12 +214,12 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,

> >  }

> >  

> >  static void xenon_set_power(struct sdhci_host *host, unsigned char mode,

> > -		unsigned short vdd)

> > +		unsigned short vdd, unsigned short vdd2)

> >  {

> >  	struct mmc_host *mmc = host->mmc;

> >  	u8 pwr = host->pwr;

> >  

> > -	sdhci_set_power_noreg(host, mode, vdd);

> > +	sdhci_set_power_noreg(host, mode, vdd, -1);

> >  

> >  	if (host->pwr == pwr)

> >  		return;

> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c

> > index d38d734ec83f..ca3d4a506e01 100644

> > --- a/drivers/mmc/host/sdhci.c

> > +++ b/drivers/mmc/host/sdhci.c

> > @@ -2089,12 +2089,15 @@ void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)

> >  EXPORT_SYMBOL_GPL(sdhci_set_clock);

> >  

> >  static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,

> > -				unsigned short vdd)

> > +				unsigned short vdd, unsigned short vdd2)

> >  {

> >  	struct mmc_host *mmc = host->mmc;

> >  

> >  	mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);

> >  

> > +	if (mmc->caps & MMC_CAP_UHS2 && !IS_ERR(mmc->supply.vmmc2))

> > +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc2, vdd2);

> > +

> >  	if (mode != MMC_POWER_OFF)

> >  		sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL);

> >  	else

> > @@ -2102,7 +2105,7 @@ static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,

> >  }

> >  

> >  void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

> > -			   unsigned short vdd)

> > +			   unsigned short vdd, unsigned short vdd2)

> >  {

> >  	u8 pwr = 0;

> >  

> > @@ -2133,6 +2136,20 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

> >  		}

> >  	}

> >  

> > +	if (mode != MMC_POWER_OFF) {

> > +		if (vdd2 != (unsigned short)-1) {

> > +			switch (1 << vdd2) {

> > +			case MMC_VDD2_165_195:

> > +				pwr |= SDHCI_VDD2_POWER_180;

> > +				break;

> > +			default:

> > +				WARN(1, "%s: Invalid vdd2 %#x\n",

> > +				     mmc_hostname(host->mmc), vdd2);

> > +				break;

> > +			}

> > +		}

> > +	}

> > +

> >  	if (host->pwr == pwr)

> >  		return;

> >  

> > @@ -2159,7 +2176,13 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

> >  			sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);

> >  

> >  		pwr |= SDHCI_POWER_ON;

> > +		if (vdd2 != (unsigned short)-1)

> > +			pwr |= SDHCI_VDD2_POWER_ON;

> > +

> > +		sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL);

> > +		mdelay(5);

> >  		sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);

> > +		mdelay(5);

> >  

> >  		if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)

> >  			sdhci_runtime_pm_bus_on(host);

> > @@ -2175,12 +2198,12 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

> >  EXPORT_SYMBOL_GPL(sdhci_set_power_noreg);

> >  

> >  void sdhci_set_power(struct sdhci_host *host, unsigned char mode,

> > -		     unsigned short vdd)

> > +		     unsigned short vdd, unsigned short vdd2)

> >  {

> >  	if (IS_ERR(host->mmc->supply.vmmc))

> > -		sdhci_set_power_noreg(host, mode, vdd);

> > +		sdhci_set_power_noreg(host, mode, vdd, vdd2);

> >  	else

> > -		sdhci_set_power_reg(host, mode, vdd);

> > +		sdhci_set_power_reg(host, mode, vdd, vdd2);

> >  }

> >  EXPORT_SYMBOL_GPL(sdhci_set_power);

> >  

> > @@ -2192,14 +2215,15 @@ EXPORT_SYMBOL_GPL(sdhci_set_power);

> >   */

> >  void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,

> >  				     unsigned char mode,

> > -				     unsigned short vdd)

> > +				     unsigned short vdd,

> > +				     unsigned short vdd2)

> >  {

> >  	if (!IS_ERR(host->mmc->supply.vmmc)) {

> >  		struct mmc_host *mmc = host->mmc;

> >  

> >  		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);

> >  	}

> > -	sdhci_set_power_noreg(host, mode, vdd);

> > +	sdhci_set_power_noreg(host, mode, vdd, -1);

> >  }

> >  EXPORT_SYMBOL_GPL(sdhci_set_power_and_bus_voltage);

> >  

> > @@ -2377,9 +2401,9 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

> >  	}

> >  

> >  	if (host->ops->set_power)

> > -		host->ops->set_power(host, ios->power_mode, ios->vdd);

> > +		host->ops->set_power(host, ios->power_mode, ios->vdd, -1);

> >  	else

> > -		sdhci_set_power(host, ios->power_mode, ios->vdd);

> > +		sdhci_set_power(host, ios->power_mode, ios->vdd, -1);

> >  

> >  	/* 4.0 host support */

> >  	if (host->version >= SDHCI_SPEC_400) {

> > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h

> > index ecf55394ea46..f6732f33f29f 100644

> > --- a/drivers/mmc/host/sdhci.h

> > +++ b/drivers/mmc/host/sdhci.h

> > @@ -689,7 +689,7 @@ struct sdhci_ops {

> >  

> >  	void	(*set_clock)(struct sdhci_host *host, unsigned int clock);

> >  	void	(*set_power)(struct sdhci_host *host, unsigned char mode,

> > -			     unsigned short vdd);

> > +			     unsigned short vdd, unsigned short vdd2);

> >  

> >  	u32		(*irq)(struct sdhci_host *host, u32 intmask);

> >  

> > @@ -852,13 +852,10 @@ u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,

> >  		   unsigned int *actual_clock);

> >  void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);

> >  void sdhci_enable_clk(struct sdhci_host *host, u16 clk);

> > -void sdhci_set_power(struct sdhci_host *host, unsigned char mode,

> > -		     unsigned short vdd);

> >  void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,

> >  				     unsigned char mode,

> > -				     unsigned short vdd);

> > -void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

> > -			   unsigned short vdd);

> > +				     unsigned short vdd,

> > +				     unsigned short vdd2);

> >  void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq);

> >  int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq);

> >  void sdhci_set_bus_width(struct sdhci_host *host, int width);

> > 

>
Adrian Hunter Sept. 14, 2020, 6:36 a.m. | #3
On 14/09/20 8:45 am, AKASHI Takahiro wrote:
> Adrian,

> 

> On Fri, Aug 21, 2020 at 05:11:18PM +0300, Adrian Hunter wrote:

>> On 10/07/20 2:11 pm, Ben Chuang wrote:

>>> From: AKASHI Takahiro <takahiro.akashi@linaro.org>

>>>

>>> VDD2 is used for powering UHS-II interface.

>>> Modify sdhci_set_power_and_bus_voltage(), sdhci_set_power_noreg()

>>> and sdhci_set_power_noreg() to handle VDD2.

>>

>> vdd2 is always 1.8 V and I suspect there may never be support for anything

>> else, so we should start with 1.8 V only.

> 

> What do you mean here?

> You don't want to add an extra argument, vdd2, to sdhci_set_power().

> Correct?


Yes

> 

>> Also can we create uhs2_set_power_reg() and uhs2_set_power_noreg() and use

>> the existing ->set_power() callback

> 

> Again what do you expect here?

> 

> Do you want to see any platform-specific mmc driver who supports UHS-II

> to implement its own call back like:


Not exactly.  I expect there to be a common implementation in sdhci-uhs2.c
called sdhci_uhs2_set_power() for example, that drivers can use by setting
their .set_power = sdhci_uhs2_set_power.  If they need platform-specific
code as well then their platform-specific code can call
sdhci_uhs2_set_power() if desired.

> 

> void sdhci_foo_set_power(struct sdhci_host *host, unsigned char mode,

>                                   unsigned short vdd)

> {

>         sdhci_set_power(host, mode,vdd);

> 

>         /* in case that sdhci_uhs2 module is not inserted */

>         if (!(mmc->caps & MMC_CAP_UHS2))

>                 return;

> 

>         /* vdd2 specific operation */

>         if (IS_ERR_OR_NULL(host->mmc->supply.vmmc2))

>                 sdhci_uhs2_set_power_noreg(host, mode);

>         else

>                 sdhci_uhs2_set_power_reg(host, mode);

> 

>         /* maybe more platform-specific initialization */

> }

> 

> struct sdhci_ops sdhci_foo_ops = {

>         .set_power = sdhci_foo_set_power,

>         ...

> }

> 

> Is this what you mean?

> (I'm not quite sure yet that sdhci_ush2_set_power_noreg() can be split off

> from sdhci_set_power_noreg().)

> 

> -Takahiro Akashi

Patch

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 1ec74c2d5c17..1926585debe5 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -678,7 +678,7 @@  static void sdhci_omap_set_clock(struct sdhci_host *host, unsigned int clock)
 }
 
 static void sdhci_omap_set_power(struct sdhci_host *host, unsigned char mode,
-			  unsigned short vdd)
+			  unsigned short vdd, unsigned short vdd2)
 {
 	struct mmc_host *mmc = host->mmc;
 
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index bb6802448b2f..40f5a24a8982 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -629,12 +629,12 @@  static int bxt_get_cd(struct mmc_host *mmc)
 #define SDHCI_INTEL_PWR_TIMEOUT_UDELAY	100
 
 static void sdhci_intel_set_power(struct sdhci_host *host, unsigned char mode,
-				  unsigned short vdd)
+				  unsigned short vdd, unsigned short vdd2)
 {
 	int cntr;
 	u8 reg;
 
-	sdhci_set_power(host, mode, vdd);
+	sdhci_set_power(host, mode, vdd, -1);
 
 	if (mode == MMC_POWER_OFF)
 		return;
diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index e55037ceda73..457e9425339a 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -298,12 +298,12 @@  static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
 }
 
 static void pxav3_set_power(struct sdhci_host *host, unsigned char mode,
-			    unsigned short vdd)
+			    unsigned short vdd, unsigned short vdd2)
 {
 	struct mmc_host *mmc = host->mmc;
 	u8 pwr = host->pwr;
 
-	sdhci_set_power_noreg(host, mode, vdd);
+	sdhci_set_power_noreg(host, mode, vdd, -1);
 
 	if (host->pwr == pwr)
 		return;
diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
index 4703cd540c7f..2b0ebb91895a 100644
--- a/drivers/mmc/host/sdhci-xenon.c
+++ b/drivers/mmc/host/sdhci-xenon.c
@@ -214,12 +214,12 @@  static void xenon_set_uhs_signaling(struct sdhci_host *host,
 }
 
 static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
-		unsigned short vdd)
+		unsigned short vdd, unsigned short vdd2)
 {
 	struct mmc_host *mmc = host->mmc;
 	u8 pwr = host->pwr;
 
-	sdhci_set_power_noreg(host, mode, vdd);
+	sdhci_set_power_noreg(host, mode, vdd, -1);
 
 	if (host->pwr == pwr)
 		return;
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d38d734ec83f..ca3d4a506e01 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2089,12 +2089,15 @@  void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 EXPORT_SYMBOL_GPL(sdhci_set_clock);
 
 static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,
-				unsigned short vdd)
+				unsigned short vdd, unsigned short vdd2)
 {
 	struct mmc_host *mmc = host->mmc;
 
 	mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
 
+	if (mmc->caps & MMC_CAP_UHS2 && !IS_ERR(mmc->supply.vmmc2))
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc2, vdd2);
+
 	if (mode != MMC_POWER_OFF)
 		sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL);
 	else
@@ -2102,7 +2105,7 @@  static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,
 }
 
 void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
-			   unsigned short vdd)
+			   unsigned short vdd, unsigned short vdd2)
 {
 	u8 pwr = 0;
 
@@ -2133,6 +2136,20 @@  void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
 		}
 	}
 
+	if (mode != MMC_POWER_OFF) {
+		if (vdd2 != (unsigned short)-1) {
+			switch (1 << vdd2) {
+			case MMC_VDD2_165_195:
+				pwr |= SDHCI_VDD2_POWER_180;
+				break;
+			default:
+				WARN(1, "%s: Invalid vdd2 %#x\n",
+				     mmc_hostname(host->mmc), vdd2);
+				break;
+			}
+		}
+	}
+
 	if (host->pwr == pwr)
 		return;
 
@@ -2159,7 +2176,13 @@  void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
 			sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
 
 		pwr |= SDHCI_POWER_ON;
+		if (vdd2 != (unsigned short)-1)
+			pwr |= SDHCI_VDD2_POWER_ON;
+
+		sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL);
+		mdelay(5);
 		sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
+		mdelay(5);
 
 		if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)
 			sdhci_runtime_pm_bus_on(host);
@@ -2175,12 +2198,12 @@  void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
 EXPORT_SYMBOL_GPL(sdhci_set_power_noreg);
 
 void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
-		     unsigned short vdd)
+		     unsigned short vdd, unsigned short vdd2)
 {
 	if (IS_ERR(host->mmc->supply.vmmc))
-		sdhci_set_power_noreg(host, mode, vdd);
+		sdhci_set_power_noreg(host, mode, vdd, vdd2);
 	else
-		sdhci_set_power_reg(host, mode, vdd);
+		sdhci_set_power_reg(host, mode, vdd, vdd2);
 }
 EXPORT_SYMBOL_GPL(sdhci_set_power);
 
@@ -2192,14 +2215,15 @@  EXPORT_SYMBOL_GPL(sdhci_set_power);
  */
 void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
 				     unsigned char mode,
-				     unsigned short vdd)
+				     unsigned short vdd,
+				     unsigned short vdd2)
 {
 	if (!IS_ERR(host->mmc->supply.vmmc)) {
 		struct mmc_host *mmc = host->mmc;
 
 		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
 	}
-	sdhci_set_power_noreg(host, mode, vdd);
+	sdhci_set_power_noreg(host, mode, vdd, -1);
 }
 EXPORT_SYMBOL_GPL(sdhci_set_power_and_bus_voltage);
 
@@ -2377,9 +2401,9 @@  void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	}
 
 	if (host->ops->set_power)
-		host->ops->set_power(host, ios->power_mode, ios->vdd);
+		host->ops->set_power(host, ios->power_mode, ios->vdd, -1);
 	else
-		sdhci_set_power(host, ios->power_mode, ios->vdd);
+		sdhci_set_power(host, ios->power_mode, ios->vdd, -1);
 
 	/* 4.0 host support */
 	if (host->version >= SDHCI_SPEC_400) {
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index ecf55394ea46..f6732f33f29f 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -689,7 +689,7 @@  struct sdhci_ops {
 
 	void	(*set_clock)(struct sdhci_host *host, unsigned int clock);
 	void	(*set_power)(struct sdhci_host *host, unsigned char mode,
-			     unsigned short vdd);
+			     unsigned short vdd, unsigned short vdd2);
 
 	u32		(*irq)(struct sdhci_host *host, u32 intmask);
 
@@ -852,13 +852,10 @@  u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
 		   unsigned int *actual_clock);
 void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
 void sdhci_enable_clk(struct sdhci_host *host, u16 clk);
-void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
-		     unsigned short vdd);
 void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
 				     unsigned char mode,
-				     unsigned short vdd);
-void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
-			   unsigned short vdd);
+				     unsigned short vdd,
+				     unsigned short vdd2);
 void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq);
 int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq);
 void sdhci_set_bus_width(struct sdhci_host *host, int width);