Hi Zhangfei, > -----Original Message----- > From: zhangfei gao [mailto:zhangfei.gao@xxxxxxxxx] > Sent: Thursday, April 28, 2011 8:48 AM > To: Nath, Arindam > Cc: cjb@xxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; prakity@xxxxxxxxxxx; > subhashj@xxxxxxxxxxxxxx; Su, Henry; Lu, Aaron; anath.amd@xxxxxxxxx > 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.Nath@xxxxxxx> > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html