Re: [RESEDN PATCH v2 3/4] mmc: sdhci: Don't try to switch to unsupported voltage

[Shawn Lin]

å 2016/9/23 15:12, Adrian Hunter åé:
> On 23/09/16 03:12, Shawn Lin wrote:
> > From: Ziyuan Xu <xzy.xu@xxxxxxxxxxxxxx>
> >
> > Sdhci shouldn't switch to the unsupported voltage if claiming
> > that it can not support the requested voltage. Let's fix it.
> >
> > Signed-off-by: Ziyuan Xu <xzy.xu@xxxxxxxxxxxxxx>
> > Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
> > ---
> >
> > Changes in v2: None
> >
> >  drivers/mmc/host/sdhci.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 4805566..b1f1edd 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -1845,7 +1845,8 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
> >
> >  	switch (ios->signal_voltage) {
> >  	case MMC_SIGNAL_VOLTAGE_330:
> > -		if (!(host->flags & SDHCI_SIGNALING_330))
> > +		if (!(host->flags & SDHCI_SIGNALING_330) ||
> > +		    !(host->caps & SDHCI_CAN_VDD_330))
>
> You are mixing signal voltage and supply voltage here.  Note, I don't

Oh, I see what you mean, driver could still set 3.3 voltage supply
even they don't support 3V3 signal voltage.

> believe all drivers set the capabilities correctly when they are using
> regulators, but in any case this is not the place in the code to make such
> assumptions.

Indeed, there is a risk of breaking some drivers if they don't set the
capabilities correctly.

So, I think we could drop this patch since patch 4 is actually what we
need in order to slove our problem. :)

> Better for the driver to remove SDHCI_SIGNALING_330 from flags when it is
> not supported.
>
> >  			return -EINVAL;
> >  		/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
> >  		ctrl &= ~SDHCI_CTRL_VDD_180;
> > @@ -1872,7 +1873,8 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
> >
> >  		return -EAGAIN;
> >  	case MMC_SIGNAL_VOLTAGE_180:
> > -		if (!(host->flags & SDHCI_SIGNALING_180))
> > +		if (!(host->flags & SDHCI_SIGNALING_180) ||
> > +		    !(host->caps & SDHCI_CAN_VDD_180))
>
> You are mixing signal voltage and supply voltage here, and this is not
> correct: not being able to do 1.8V supply does not mean you can't do 1.8V
> signaling.  The SDHCI way of determining supported signal voltages is based
> on supported transfer modes.
>
> If the driver can't get the transfer modes correct (e.g. we still don't have
> MMC_CAP2_3_3V_ONLY_DDR or equivalent), remove SDHCI_SIGNALING_180 from the
> flags.
>
> >  			return -EINVAL;
> >  		if (!IS_ERR(mmc->supply.vqmmc)) {
> >  			ret = mmc_regulator_set_vqmmc(mmc, ios);


--
Best Regards
Shawn Lin