Hi Zhangfei,

> -----Original Message-----
> From: zhangfei gao [mailto:zhangfei....@gmail.com]
> Sent: Thursday, April 28, 2011 8:48 AM
> To: Nath, Arindam
> Cc: c...@laptop.org; linux-mmc@vger.kernel.org; prak...@marvell.com;
> subha...@codeaurora.org; Su, Henry; Lu, Aaron; anath....@gmail.com
> Subject: Re: [PATCH v3 01/11] mmc: sd: add support for signal voltage
> switch procedure
> 
> On Wed, Apr 27, 2011 at 6:17 AM, Nath, Arindam <arindam.n...@amd.com>
> wrote:
> > Hi Zhangfei,
> >
> > [...]
> >
> >> Hi, Arindam
> >>
> >> The uhs card works OK,
> >> However after use uhs card, the io voltage keeps 1.8v, and
> >> mmc_send_app_op_cond will return error, so no chance to go back to
> >> 3.3v.
> >> The result is hs card can not be detected after using uhs card.
> >> Also some hs card have timeout issue here when reading partition
> >> table, still not know it is local reason.
> >
> > [...]
> >
> >> > @@ -781,6 +808,11 @@ int mmc_attach_sd(struct mmc_host *host)
> >> >        if (host->ocr_avail_sd)
> >> >                host->ocr_avail = host->ocr_avail_sd;
> >> >
> >> > +       /* Make sure we are at 3.3V signalling voltage */
> >> > +       err = mmc_set_signal_voltage(host,
> MMC_SIGNAL_VOLTAGE_330);
> >> > +       if (err)
> >> > +               return err;
> >>
> >> The code here may have no chance to execute, since
> >> mmc_send_app_op_cond already failed if v_io is 1.8v for hs card.
> >> Could you move the voltage setting before mmc_send_app_op_cond.
> >
> > Thanks once again for your sharp review. Yes, it makes sense to move
> mmc_set_signal_voltage() before mmc_send_app_op_cond(). Will do it.
> >
> > [...]
> >
> >> > @@ -1297,11 +1299,116 @@ out:
> >> >        spin_unlock_irqrestore(&host->lock, flags);
> >> >  }
> >> >
> >> > +static int sdhci_start_signal_voltage_switch(struct mmc_host
> *mmc,
> >> > +       struct mmc_ios *ios)
> >> > +{
> >> > +       struct sdhci_host *host;
> >> > +       u8 pwr;
> >> > +       u16 clk, ctrl;
> >> > +       u32 present_state;
> >> > +
> >> > +       host = mmc_priv(mmc);
> >> > +
> >> > +       /*
> >> > +        * Signal Voltage Switching is only applicable for Host
> >> Controllers
> >> > +        * v3.00 and above.
> >> > +        */
> >> > +       if (host->version < SDHCI_SPEC_300)
> >> > +               return 0;
> >> > +
> >> > +       /*
> >> > +        * We first check whether the request is to set signalling
> >> voltage
> >> > +        * to 3.3V. If so, we change the voltage to 3.3V and
> return
> >> quickly.
> >> > +        */
> >> > +       ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> >> > +       if ((ctrl & SDHCI_CTRL_VDD_180) &&
> >>
> >> The check here may fail, could you remove this check, for example
> >> ctrl=0x10 and no chance to 3.3v.
> >
> > If ctrl=0x10, it would mean the signaling voltage is already at 3.3V,
> so why do we need to set it to 3.3V again?
> 
> The test here shows ctrl return back to 0x10 once the card is removed
> even the io voltage is 1.8v.
> And the first time SDHCI_CTRL_VDD_180 is not set, so the default
> voltage is used if using external regulator.

Thanks for explaining how it might impact other HCs. I will make the check only 
for 3.3V. That should work for you.

Thanks,
Arindam


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to