On Fri, Apr 22, 2016 at 8:30 PM, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > On 15/04/16 20:29, Dong Aisheng wrote: >> Currently when card type supports EXT_CSD_CARD_TYPE_DDR_1_8V >> which means it can work on DDR mode with either 3.3v IO or 1.8v >> IO voltage. MMC core will first try 1.8v then 3.3v if host claims >> MMC_CAP_1_8V_DDR support. >> However the host driver voltage switch code does not check NO_1_8_V >> quirk which may set a wrong 1.8v and causes the card fixed to 3.3v >> VIO un-work. >> >> Checking 1.8V quirk before setting it to avoid such issue. > > We need to look forward to when SDHCI_QUIRK2_NO_1_8_V doesn't exist. > Yes, i did try to clean up SDHCI_QUIRK2_NO_1_8_V. However, it seems not that simply. Currently we may still need it before we got a better way. My point is whether we should stop fixing the exist issue in SDHCI driver just caused by we want to clean up it later? > There are two possibilities: > > 1. Add MMC_CAP_3_3V_DDR and support to core and use that instead of > MMC_CAP_1_8V_DDR. You'll need Ulf's feedback on that. > Yes, we can do it. But the point is for DDR50/SD3.0/SDIO3.0/HS200/HS400 cards we have the same situation and still need QUIRK2_NO_1_8_V. And i somehow a bit wonder whether we should retrieve the speed mode support from device tree since it's actually controller capability. IO range capability is another thing. Probably we may start another topic to discuss it specificly. > 2. Replace ->start_signal_voltage_switch() i.e. > > host->mmc_host_ops.start_signal_voltage_switch = > esdhci_start_signal_voltage_switch; > > You will also need to change all calls to > sdhci_start_signal_voltage_switch() with > host->mmc->ops->start_signal_voltage_switch(), but that needs to be done anyway. > esdhc can fully use the common sdhci_start_signal_voltag_switch. Since it's already support QUIRK_NO_1_8_V, we don't want to invent imx voltage swith currently, but fix and use it first. Regards Dong Aisheng >> >> CC: stable <stable@xxxxxxxxxxxxxxx> >> Signed-off-by: Dong Aisheng <aisheng.dong@xxxxxxx> >> --- >> drivers/mmc/host/sdhci.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index 2338aab..96ccb15 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -1683,6 +1683,8 @@ static int sdhci_do_signal_voltage_switch(struct sdhci_host *host, >> >> return -EAGAIN; >> case MMC_SIGNAL_VOLTAGE_180: >> + if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) >> + return -EINVAL; >> /* >> * Enable 1.8V Signal Enable in the Host Control2 >> * register >> > -- 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
