[v3, 1/7] mmc: sdhci-of-esdhc: add peripheral clock support
Y.B. Lu
yangbo.lu at nxp.com
Tue Apr 11 01:14:48 EDT 2017
Hi Adrian,
> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter at intel.com]
> Sent: Monday, April 10, 2017 8:36 PM
> To: Y.B. Lu; linux-mmc at vger.kernel.org; devicetree at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org; ulf.hansson at linaro.org; 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 at nxp.com>
>
> Apart from minor comments:
>
> Acked-by: Adrian Hunter <adrian.hunter at intel.com>
>
> > ---
> > 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.
> > + }
> > +}
> > +
> > 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)
> >
More information about the linux-arm-kernel
mailing list