Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1370692 > unrolled thread

[PATCH v2 0/3] SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST removal

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/3] SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST removal Ludovic Desroches <ludovic.desroches@atmel.com> - 2016-04-04 17:30 +0200
    [PATCH v2 2/3] mmc: sdhci-of-at91: implement specific set_clock function Ludovic Desroches <ludovic.desroches@atmel.com> - 2016-04-04 17:30 +0200
    [PATCH v2 3/3] mmc: sdhci: removal of SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST Ludovic Desroches <ludovic.desroches@atmel.com> - 2016-04-04 17:30 +0200

#1370692 — [PATCH v2 0/3] SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST removal

Hi,

I have recently observed that the quirk
SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST I have introduced doesn't fix all
the bugs relative to the internal clock disabling while configuring the SD
clock. This delay was introduced because of a re-synchronisation done when
disabling the internal clock.

Unfortunately, we can still have clock stabilization bug even if it occurs
rarely. Moreover, trying to use presets, disabling the internal clock causes an
unexpected switch to the base clock. It should be solved on next revision of
the chip.

For those reasons plus the non acceptance of new quirks, I have decided to
remove it and to implement my own set_clock() function. In ordrer to reduce
code duplication with the sdhci set_clock function, I moved some of the
code in a new sdhci_compute_clock_config() function.

Regards

Changes:
- v2:
  - sdhci_compute_clock_config uses a returned value instead of an
  out-parameter to provide the clock configuration.


Ludovic Desroches (3):
  mmc: sdhci: introduce sdhci_compute_clock_config
  mmc: sdhci-of-at91: implement specific set_clock function
  mmc: sdhci: removal of SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST

 drivers/mmc/host/sdhci-of-at91.c | 48 ++++++++++++++++++++++++++++++++++++++--
 drivers/mmc/host/sdhci.c         | 31 +++++++++++++++++---------
 drivers/mmc/host/sdhci.h         |  6 +----
 3 files changed, 67 insertions(+), 18 deletions(-)

-- 
2.5.0

[toc] | [next] | [standalone]

#1370693 — [PATCH v2 2/3] mmc: sdhci-of-at91: implement specific set_clock function

Disabling the internal clock while configuring the SD card clock can
lead to internal clock stabilization issue and/or unexpected switch to
the base clock when using presets.
A quirk SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST was introduced to fix
these bugs. The cause was assumed to be a too long internal
re-synchronisation but it seems in some cases the delay (even if longer)
doesn't fix this bug. The safest workaround is to not disable/enable the
internal clock during the SD card clock configuration.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/mmc/host/sdhci-of-at91.c | 48 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
index 2703aa9..544850b 100644
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/mmc/host.h>
@@ -37,8 +38,52 @@ struct sdhci_at91_priv {
 	struct clk *mainck;
 };
 
+static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	u16 clk;
+	unsigned long timeout;
+
+	host->mmc->actual_clock = 0;
+
+	/*
+	 * There is no requirement to disable the internal clock before
+	 * changing the SD clock configuration. Moreover, disabling the
+	 * internal clock, changing the configuration and re-enabling the
+	 * internal clock causes some bugs. It can prevent to get the internal
+	 * clock stable flag ready and an unexpected switch to the base clock
+	 * when using presets.
+	 */
+	clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+	clk &= SDHCI_CLOCK_INT_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+
+	if (clock == 0)
+		return;
+
+	clk = sdhci_compute_clock_config(host, clock);
+
+	clk |= SDHCI_CLOCK_INT_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+
+	/* Wait max 20 ms */
+	timeout = 20;
+	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
+		& SDHCI_CLOCK_INT_STABLE)) {
+		if (timeout == 0) {
+			pr_err("%s: Internal clock never stabilised.\n",
+			       mmc_hostname(host->mmc));
+			return;
+		}
+		timeout--;
+		mdelay(1);
+	}
+
+	clk |= SDHCI_CLOCK_CARD_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+}
+
 static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
-	.set_clock		= sdhci_set_clock,
+	.set_clock		= sdhci_at91_set_clock,
 	.set_bus_width		= sdhci_set_bus_width,
 	.reset			= sdhci_reset,
 	.set_uhs_signaling	= sdhci_set_uhs_signaling,
@@ -46,7 +91,6 @@ static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
 
 static const struct sdhci_pltfm_data soc_data_sama5d2 = {
 	.ops = &sdhci_at91_sama5d2_ops,
-	.quirks2 = SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST,
 };
 
 static const struct of_device_id sdhci_at91_dt_match[] = {
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1370695 — [PATCH v2 3/3] mmc: sdhci: removal of SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST

SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST quirk is not used anymore so
remove it.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/mmc/host/sdhci.c | 2 --
 drivers/mmc/host/sdhci.h | 5 -----
 2 files changed, 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2c3dede..e2cfe6c 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1191,8 +1191,6 @@ void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	host->mmc->actual_clock = 0;
 
 	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
-	if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
-		mdelay(1);
 
 	if (clock == 0)
 		return;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 23ddd6e..4f60105 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -417,11 +417,6 @@ struct sdhci_host {
 #define SDHCI_QUIRK2_ACMD23_BROKEN			(1<<14)
 /* Broken Clock divider zero in controller */
 #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
-/*
- * When internal clock is disabled, a delay is needed before modifying the
- * SD clock frequency or enabling back the internal clock.
- */
-#define SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST	(1<<16)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
2.5.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web