[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