| Message ID | 20180425120937.29867-6-kishon@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25/04/18 15:09, Kishon Vijay Abraham I wrote: > Though MMC controller can indicate HS200/HS400 mode capability (by > using "mmc-hs200-1_8v"/"mmc-hs400-1_8v" dt property), if the IO lines > in the board is connected to 3.3v supply, HS200/HS400 mode cannot be > supported. Such boards have "no-1-8-v" property in their dts file. > Disable HS200/HS400 mode for boards which have "no-1-8-v" set. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > Acked-by: Tony Lindgren <tony@atomide.com> > --- > drivers/mmc/host/sdhci.c | 1 + > include/linux/mmc/host.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 2ededa7f43df..b5f047b5f3ae 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -3672,6 +3672,7 @@ int sdhci_setup_host(struct sdhci_host *host) > if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) { > host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | > SDHCI_SUPPORT_DDR50); > + mmc->caps2 &= ~MMC_CAP2_HSX00_1_8V; Seems weird for sdhci to clear flags it never set. Also couldn't we reasonably expect dt properties to be consistent? Is this really about setting SDHCI_QUIRK2_NO_1_8_V in your driver and expecting it to override other dt properties? Although that still begs the question why anyone would set dt properties that the hardware doesn't support? I guess you should also clear MMC_CAP2_HS400_ES. > } > > /* Any UHS-I mode in caps implies SDR12 and SDR25 support. */ > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > index 7c6eaf63f5ce..58832451767b 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -345,6 +345,7 @@ struct mmc_host { > #define MMC_CAP2_HS400_1_2V (1 << 16) /* Can support HS400 1.2V */ > #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \ > MMC_CAP2_HS400_1_2V) > +#define MMC_CAP2_HSX00_1_8V (MMC_CAP2_HS200_1_8V_SDR | MMC_CAP2_HS400_1_8V) > #define MMC_CAP2_HSX00_1_2V (MMC_CAP2_HS200_1_2V_SDR | MMC_CAP2_HS400_1_2V) > #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17) > #define MMC_CAP2_NO_WRITE_PROTECT (1 << 18) /* No physical write protect pin, assume that card is always read-write */ > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Adrian, On Thursday 26 April 2018 02:25 PM, Adrian Hunter wrote: > On 25/04/18 15:09, Kishon Vijay Abraham I wrote: >> Though MMC controller can indicate HS200/HS400 mode capability (by >> using "mmc-hs200-1_8v"/"mmc-hs400-1_8v" dt property), if the IO lines >> in the board is connected to 3.3v supply, HS200/HS400 mode cannot be >> supported. Such boards have "no-1-8-v" property in their dts file. >> Disable HS200/HS400 mode for boards which have "no-1-8-v" set. >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> Acked-by: Tony Lindgren <tony@atomide.com> >> --- >> drivers/mmc/host/sdhci.c | 1 + >> include/linux/mmc/host.h | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index 2ededa7f43df..b5f047b5f3ae 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -3672,6 +3672,7 @@ int sdhci_setup_host(struct sdhci_host *host) >> if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) { >> host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | >> SDHCI_SUPPORT_DDR50); >> + mmc->caps2 &= ~MMC_CAP2_HSX00_1_8V; > > Seems weird for sdhci to clear flags it never set. Also couldn't we > reasonably expect dt properties to be consistent? Is this really about > setting SDHCI_QUIRK2_NO_1_8_V in your driver and expecting it to override > other dt properties? Although that still begs the question why anyone would > set dt properties that the hardware doesn't support? I guess you should > also clear MMC_CAP2_HS400_ES. The SoC might support a specific mode like HS200. So the SoC specific dtsi file might have dt properties for HS200 mode set. But the board which uses the SoC might be modeled in a way a particular mode cannot be used (like IO lines not connected to 1.8v). One option is to use /delete-property/ in the board dts file. But since "no-1-8-v" property already indicates HS200 mode cannot be supported, having a /delete-property/ might be redundant. I can reset caps2 in sdhci-omap after invoking mmc_of_parse if you feel that makes more sense. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/04/18 13:08, Kishon Vijay Abraham I wrote: > Hi Adrian, > > On Thursday 26 April 2018 02:25 PM, Adrian Hunter wrote: >> On 25/04/18 15:09, Kishon Vijay Abraham I wrote: >>> Though MMC controller can indicate HS200/HS400 mode capability (by >>> using "mmc-hs200-1_8v"/"mmc-hs400-1_8v" dt property), if the IO lines >>> in the board is connected to 3.3v supply, HS200/HS400 mode cannot be >>> supported. Such boards have "no-1-8-v" property in their dts file. >>> Disable HS200/HS400 mode for boards which have "no-1-8-v" set. >>> >>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >>> Acked-by: Tony Lindgren <tony@atomide.com> >>> --- >>> drivers/mmc/host/sdhci.c | 1 + >>> include/linux/mmc/host.h | 1 + >>> 2 files changed, 2 insertions(+) >>> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> index 2ededa7f43df..b5f047b5f3ae 100644 >>> --- a/drivers/mmc/host/sdhci.c >>> +++ b/drivers/mmc/host/sdhci.c >>> @@ -3672,6 +3672,7 @@ int sdhci_setup_host(struct sdhci_host *host) >>> if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) { >>> host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | >>> SDHCI_SUPPORT_DDR50); >>> + mmc->caps2 &= ~MMC_CAP2_HSX00_1_8V; >> >> Seems weird for sdhci to clear flags it never set. Also couldn't we >> reasonably expect dt properties to be consistent? Is this really about >> setting SDHCI_QUIRK2_NO_1_8_V in your driver and expecting it to override >> other dt properties? Although that still begs the question why anyone would >> set dt properties that the hardware doesn't support? I guess you should >> also clear MMC_CAP2_HS400_ES. > > The SoC might support a specific mode like HS200. So the SoC specific dtsi file > might have dt properties for HS200 mode set. But the board which uses the SoC > might be modeled in a way a particular mode cannot be used (like IO lines not > connected to 1.8v). One option is to use /delete-property/ in the board dts > file. But since "no-1-8-v" property already indicates HS200 mode cannot be > supported, having a /delete-property/ might be redundant. > > I can reset caps2 in sdhci-omap after invoking mmc_of_parse if you feel that > makes more sense. Just add the explanation to the commit message, add a comment to the code, and also clear MMC_CAP_1_8V_DDR and MMC_CAP2_HS400_ES and MMC_CAP_UHS_*. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 2ededa7f43df..b5f047b5f3ae 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -3672,6 +3672,7 @@ int sdhci_setup_host(struct sdhci_host *host) if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) { host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50); + mmc->caps2 &= ~MMC_CAP2_HSX00_1_8V; } /* Any UHS-I mode in caps implies SDR12 and SDR25 support. */ diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 7c6eaf63f5ce..58832451767b 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -345,6 +345,7 @@ struct mmc_host { #define MMC_CAP2_HS400_1_2V (1 << 16) /* Can support HS400 1.2V */ #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \ MMC_CAP2_HS400_1_2V) +#define MMC_CAP2_HSX00_1_8V (MMC_CAP2_HS200_1_8V_SDR | MMC_CAP2_HS400_1_8V) #define MMC_CAP2_HSX00_1_2V (MMC_CAP2_HS200_1_2V_SDR | MMC_CAP2_HS400_1_2V) #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17) #define MMC_CAP2_NO_WRITE_PROTECT (1 << 18) /* No physical write protect pin, assume that card is always read-write */