On 13/01/17 04:12, Y.B. Lu wrote: > Hi Adrian, > > Thanks a lot for reviewing my patches. > Please see my comments inline. > > Best regards, > Yangbo Lu > >> -----Original Message----- >> From: Adrian Hunter [mailto:adrian.hunter@xxxxxxxxx] >> Sent: Thursday, January 12, 2017 3:09 PM >> To: Y.B. Lu; linux-mmc@xxxxxxxxxxxxxxx; ulf.hansson@xxxxxxxxxx >> Subject: Re: [v2, 2/2] mmc: sdhci-of-esdhc: avoid clock glitch when >> frequency is changing >> >> On 26/12/16 11:46, Yangbo Lu wrote: >>> The eSDHC_PRSSTAT[SDSTB] bit indicates whether the internal card clock >>> is stable. This bit is for the host driver to poll clock status when >>> changing the clock frequency. It is recommended to clear >>> eSDHC_SYSCTL[SDCLKEN] to remove glitch on the card clock when the >>> frequency is changing. This patch is to disable SDCLKEN bit before >>> changing frequency and enable it after SDSTB bit is set. >>> >>> Signed-off-by: Yangbo Lu <yangbo.lu@xxxxxxx> >>> --- >>> Changes for v2: >>> - added Adrian into to list >>> --- >>> drivers/mmc/host/sdhci-esdhc.h | 5 +++++ >>> drivers/mmc/host/sdhci-of-esdhc.c | 21 ++++++++++++++++++--- >>> 2 files changed, 23 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/mmc/host/sdhci-esdhc.h >>> b/drivers/mmc/host/sdhci-esdhc.h index 8cd8449..ece8b37 100644 >>> --- a/drivers/mmc/host/sdhci-esdhc.h >>> +++ b/drivers/mmc/host/sdhci-esdhc.h >>> @@ -31,6 +31,10 @@ >>> * eSDHC register definition >>> */ >>> >>> +/* Present State Register */ >>> +#define ESDHC_PRSSTAT 0x24 >>> +#define ESDHC_CLOCK_STABLE 0x00000008 >>> + >>> /* Protocol Control Register */ >>> #define ESDHC_PROCTL 0x28 >>> #define ESDHC_CTRL_4BITBUS (0x1 << 1) >>> @@ -43,6 +47,7 @@ >>> #define ESDHC_CLOCK_MASK 0x0000fff0 >>> #define ESDHC_PREDIV_SHIFT 8 >>> #define ESDHC_DIVIDER_SHIFT 4 >>> +#define ESDHC_CLOCK_SDCLKEN 0x00000008 >>> #define ESDHC_CLOCK_PEREN 0x00000004 >>> #define ESDHC_CLOCK_HCKEN 0x00000002 >>> #define ESDHC_CLOCK_IPGEN 0x00000001 >>> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c >>> b/drivers/mmc/host/sdhci-of-esdhc.c >>> index 9a6eb44..0849885 100644 >>> --- a/drivers/mmc/host/sdhci-of-esdhc.c >>> +++ b/drivers/mmc/host/sdhci-of-esdhc.c >>> @@ -431,6 +431,7 @@ static void esdhc_of_set_clock(struct sdhci_host >> *host, unsigned int clock) >>> struct sdhci_esdhc *esdhc = sdhci_pltfm_priv(pltfm_host); >>> int pre_div = 1; >>> int div = 1; >>> + u32 timeout; >>> u32 temp; >>> >>> host->mmc->actual_clock = 0; >>> @@ -451,8 +452,8 @@ static void esdhc_of_set_clock(struct sdhci_host >> *host, unsigned int clock) >>> } >>> >>> temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL); >>> - temp &= ~(ESDHC_CLOCK_IPGEN | ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN >>> - | ESDHC_CLOCK_MASK); >>> + temp &= ~(ESDHC_CLOCK_SDCLKEN | ESDHC_CLOCK_IPGEN | >> ESDHC_CLOCK_HCKEN | >>> + ESDHC_CLOCK_PEREN | ESDHC_CLOCK_MASK); >>> sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL); >>> >>> while (host->max_clk / pre_div / 16 > clock && pre_div < 256) @@ >>> -472,7 +473,21 @@ static void esdhc_of_set_clock(struct sdhci_host >> *host, unsigned int clock) >>> | (div << ESDHC_DIVIDER_SHIFT) >>> | (pre_div << ESDHC_PREDIV_SHIFT)); >>> sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL); >>> - mdelay(1); >>> + >>> + /* Wait max 20 ms */ >>> + timeout = 20; >>> + while (!(sdhci_readl(host, ESDHC_PRSSTAT) & ESDHC_CLOCK_STABLE)) { >>> + if (timeout == 0) { >>> + pr_err("%s: Internal clock never stabilised.\n", >>> + mmc_hostname(host->mmc)); >>> + return; >>> + } >>> + timeout--; >>> + mdelay(1); >>> + } >> >> Please try to do this with readx_poll_timeout_atomic(). However we are >> not racing with anything (and one of the things that needs doing in SDHCI >> is to reduce the use of the spin lock) so you could drop the spin lock >> while using >> readx_poll_timeout() instead. >> > > [Lu Yangbo-B47093] The function readx_poll_timeout_atomic() periodically polls an address, but the addr must be > the only argument for op. Our sdhci_readl has two arguments. Any suggestion for this? > > readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) Yeah, I forgot about that, sorry. -- 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
