Hi Adrian, > -----Original Message----- > From: linux-mmc-owner@xxxxxxxxxxxxxxx [mailto:linux-mmc- > owner@xxxxxxxxxxxxxxx] On Behalf Of Y.B. Lu > Sent: Tuesday, April 11, 2017 1:15 PM > To: Adrian Hunter; linux-mmc@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; ulf.hansson@xxxxxxxxxx; Rob Herring; > Mark Rutland; Catalin Marinas; Will Deacon > Cc: Xiaobo Xie > Subject: RE: [v3, 1/7] mmc: sdhci-of-esdhc: add peripheral clock support > > Hi Adrian, > > > > -----Original Message----- > > From: Adrian Hunter [mailto:adrian.hunter@xxxxxxxxx] > > Sent: Monday, April 10, 2017 8:36 PM > > To: Y.B. Lu; linux-mmc@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; ulf.hansson@xxxxxxxxxx; Rob > > Herring; Mark Rutland; Catalin Marinas; Will Deacon > > Cc: Xiaobo Xie > > Subject: Re: [v3, 1/7] mmc: sdhci-of-esdhc: add peripheral clock > > support > > > > On 27/03/17 10:49, Yangbo Lu wrote: > > > eSDHC could select peripheral clock or platform clock as clock > > > source by the PCS bit of eSDHC Control Register, and this bit > > > couldn't be reset by software reset for all. In default, the > > > platform clock is used. But we have to use peripheral clock since it > > > has a higher frequency to support eMMC > > > HS200 mode and SD UHS-I mode. This patch is to add peripheral clock > > > support and use it instead of platform clock if it's declared in > > > eSDHC > > dts node. > > > > > > Signed-off-by: Yangbo Lu <yangbo.lu@xxxxxxx> > > > > Apart from minor comments: > > > > Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx> > > > > > --- > > > Changes for v2: > > > - None > > > Changes for v3: > > > - None > > > --- > > > drivers/mmc/host/sdhci-esdhc.h | 1 + > > > drivers/mmc/host/sdhci-of-esdhc.c | 70 > > > +++++++++++++++++++++++++++++++++++++-- > > > 2 files changed, 69 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/mmc/host/sdhci-esdhc.h > > > b/drivers/mmc/host/sdhci-esdhc.h index ece8b37..5343fc0 100644 > > > --- a/drivers/mmc/host/sdhci-esdhc.h > > > +++ b/drivers/mmc/host/sdhci-esdhc.h > > > @@ -54,6 +54,7 @@ > > > > > > /* Control Register for DMA transfer */ > > > #define ESDHC_DMA_SYSCTL 0x40c > > > +#define ESDHC_PERIPHERAL_CLK_SEL 0x00080000 > > > #define ESDHC_DMA_SNOOP 0x00000040 > > > > > > #endif /* _DRIVERS_MMC_SDHCI_ESDHC_H */ diff --git > > > a/drivers/mmc/host/sdhci-of-esdhc.c > > > b/drivers/mmc/host/sdhci-of-esdhc.c > > > index ff37e74..7ce1caf 100644 > > > --- a/drivers/mmc/host/sdhci-of-esdhc.c > > > +++ b/drivers/mmc/host/sdhci-of-esdhc.c > > > @@ -19,6 +19,7 @@ > > > #include <linux/delay.h> > > > #include <linux/module.h> > > > #include <linux/sys_soc.h> > > > +#include <linux/clk.h> > > > #include <linux/mmc/host.h> > > > #include "sdhci-pltfm.h" > > > #include "sdhci-esdhc.h" > > > @@ -30,6 +31,7 @@ struct sdhci_esdhc { > > > u8 vendor_ver; > > > u8 spec_ver; > > > bool quirk_incorrect_hostver; > > > + unsigned int peripheral_clock; > > > }; > > > > > > /** > > > @@ -414,15 +416,25 @@ static int esdhc_of_enable_dma(struct > > > sdhci_host > > > *host) static unsigned int esdhc_of_get_max_clock(struct sdhci_host > > > *host) { > > > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > > > + struct sdhci_esdhc *esdhc = sdhci_pltfm_priv(pltfm_host); > > > > > > - return pltfm_host->clock; > > > + if (esdhc->peripheral_clock) > > > + return esdhc->peripheral_clock; > > > + else > > > + return pltfm_host->clock; > > > } > > > > > > static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host) > > > { > > > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > > > + struct sdhci_esdhc *esdhc = sdhci_pltfm_priv(pltfm_host); > > > + unsigned int clock; > > > > > > - return pltfm_host->clock / 256 / 16; > > > + if (esdhc->peripheral_clock) > > > + clock = esdhc->peripheral_clock; > > > + else > > > + clock = pltfm_host->clock; > > > + return clock / 256 / 16; > > > } > > > > > > static void esdhc_of_set_clock(struct sdhci_host *host, unsigned > > > int > > > clock) @@ -512,6 +524,33 @@ static void > > esdhc_pltfm_set_bus_width(struct sdhci_host *host, int width) > > > sdhci_writel(host, ctrl, ESDHC_PROCTL); } > > > > > > +static void esdhc_clock_enable(struct sdhci_host *host, bool > > > +enable) { > > > + u32 val; > > > + u32 timeout; > > > + > > > + val = sdhci_readl(host, ESDHC_SYSTEM_CONTROL); > > > + > > > + if (enable) > > > + val |= ESDHC_CLOCK_SDCLKEN; > > > + else > > > + val &= ~ESDHC_CLOCK_SDCLKEN; > > > + > > > + sdhci_writel(host, val, ESDHC_SYSTEM_CONTROL); > > > + > > > + timeout = 20; > > > + val = ESDHC_CLOCK_STABLE; > > > + while (!(sdhci_readl(host, ESDHC_PRSSTAT) & val)) { > > > + if (timeout == 0) { > > > + pr_err("%s: Internal clock never stabilised.\n", > > > + mmc_hostname(host->mmc)); > > > + break; > > > + } > > > + timeout--; > > > + mdelay(1); > > > > If the time to stabilize is much less that 1 ms then this loop can > > waste time. Have a look at the change in sdhci.c. > > > > [Lu Yangbo-B47093] Thanks a lot. The method in sdhci.c is really better. > I will send next version to use it soon. > > Currently another place in esdhc driver could also change to use this > method. I will send a separate patch for that later. > [Lu Yangbo-B47093] I just sent another patch to fix the mdelay problem. https://patchwork.kernel.org/patch/9700193/ Thank you very much. > > > > + } > > > +} > > > + > > > static void esdhc_reset(struct sdhci_host *host, u8 mask) { > > > sdhci_reset(host, mask); > > > @@ -613,6 +652,9 @@ static void esdhc_init(struct platform_device > > > *pdev, struct sdhci_host *host) { > > > struct sdhci_pltfm_host *pltfm_host; > > > struct sdhci_esdhc *esdhc; > > > + struct device_node *np; > > > + struct clk *clk; > > > + u32 val; > > > u16 host_ver; > > > > > > pltfm_host = sdhci_priv(host); > > > @@ -626,6 +668,30 @@ static void esdhc_init(struct platform_device > > *pdev, struct sdhci_host *host) > > > esdhc->quirk_incorrect_hostver = true; > > > else > > > esdhc->quirk_incorrect_hostver = false; > > > + > > > + np = pdev->dev.of_node; > > > + clk = of_clk_get(np, 0); > > > > Should there be a clk_put somewhere? > > [Lu Yangbo-B47093] Will add it after driver gets the clock value. > > > > > > + if (!IS_ERR(clk)) { > > > + /* > > > + * esdhc->peripheral_clock would be assigned with a value > > > + * which is eSDHC base clock when use periperal clock. > > > + * For ls1046a, the clock value got by common clk API is > > > + * peripheral clock while the eSDHC base clock is 1/2 > > > + * peripheral clock. > > > + */ > > > + if (of_device_is_compatible(np, "fsl,ls1046a-esdhc")) > > > + esdhc->peripheral_clock = clk_get_rate(clk) / 2; > > > + else > > > + esdhc->peripheral_clock = clk_get_rate(clk); > > > + } > > > + > > > + if (esdhc->peripheral_clock) { > > > + esdhc_clock_enable(host, false); > > > + val = sdhci_readl(host, ESDHC_DMA_SYSCTL); > > > + val |= ESDHC_PERIPHERAL_CLK_SEL; > > > + sdhci_writel(host, val, ESDHC_DMA_SYSCTL); > > > + esdhc_clock_enable(host, true); > > > + } > > > } > > > > > > static int sdhci_esdhc_probe(struct platform_device *pdev) > > > > > -- > 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 -- 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
