On Wed, Apr 27, 2016 at 11:27:07PM +0300, Adrian Hunter wrote: > On 24/04/2016 12:56 p.m., Dong Aisheng wrote: > >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? > > If you want a fix for stable kernels, then I guess it is OK. In that case > it is better to have a fix that applies cleanly to older kernels > i.e. not on top of re-factoring > Okay, i would think more about how to remove SDHCI_QUIRK2_NO_1_8_V first. Regards Dong Aisheng > > > >>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
