linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos
@ 2017-05-29 13:54 Benoît Thébaudeau
  2017-05-29 13:54 ` [PATCH v2 2/5] mmc: sdhci-esdhc: Add SDHCI_QUIRK_32BIT_DMA_ADDR Benoît Thébaudeau
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Benoît Thébaudeau @ 2017-05-29 13:54 UTC (permalink / raw)
  To: linux-kernel, linux-mmc
  Cc: Ulf Hansson, Adrian Hunter, Fabio Estevam, Wolfram Sang,
	Benoît Thébaudeau

Fix various English mistakes and typos in comments and in printed
strings.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
---
Changes v1 -> v2: new patch.
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 47 +++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index b5c7241..6095a14 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -95,7 +95,7 @@
 #define ESDHC_CTRL_BUSWIDTH_MASK	(0x3 << 1)
 
 /*
- * There is an INT DMA ERR mis-match between eSDHC and STD SDHC SPEC:
+ * There is an INT DMA ERR mismatch between eSDHC and STD SDHC SPEC:
  * Bit25 is used in STD SPEC, and is reserved in fsl eSDHC design,
  * but bit28 is used as the INT DMA ERR in fsl eSDHC design.
  * Define this macro DMA error INT for fsl eSDHC
@@ -110,12 +110,12 @@
  * In exact block transfer, the controller doesn't complete the
  * operations automatically as required at the end of the
  * transfer and remains on hold if the abort command is not sent.
- * As a result, the TC flag is not asserted and SW  received timeout
+ * As a result, the TC flag is not asserted and SW received timeout
  * exeception. Bit1 of Vendor Spec registor is used to fix it.
  */
 #define ESDHC_FLAG_MULTIBLK_NO_INT	BIT(1)
 /*
- * The flag enables the workaround for ESDHC errata ENGcm07207 which
+ * The flag enables the workaround for ESDHC erratum ENGcm07207 which
  * affects i.MX25 and i.MX35.
  */
 #define ESDHC_FLAG_ENGCM07207		BIT(2)
@@ -131,7 +131,7 @@
 /* The IP has SDHCI_CAPABILITIES_1 register */
 #define ESDHC_FLAG_HAVE_CAP1		BIT(6)
 /*
- * The IP has errata ERR004536
+ * The IP has erratum ERR004536
  * uSDHC: ADMA Length Mismatch Error occurs if the AHB read access is slow,
  * when reading data from the card
  */
@@ -141,7 +141,7 @@
 /* The IP supports HS400 mode */
 #define ESDHC_FLAG_HS400		BIT(9)
 
-/* A higher clock ferquency than this rate requires strobell dll control */
+/* A clock frequency higher than this rate requires strobe dll control */
 #define ESDHC_STROBE_DLL_CLK_FREQ	100000000
 
 struct esdhc_soc_data {
@@ -197,7 +197,7 @@ struct pltfm_imx_data {
 	struct clk *clk_ahb;
 	struct clk *clk_per;
 	enum {
-		NO_CMD_PENDING,      /* no multiblock command pending*/
+		NO_CMD_PENDING,      /* no multiblock command pending */
 		MULTIBLK_IN_PROCESS, /* exact multiblock cmd in process */
 		WAIT_FOR_INT,        /* sent CMD12, waiting for response INT */
 	} multiblock_status;
@@ -286,7 +286,7 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
 		 * ADMA2 capability of esdhc, but this bit is messed up on
 		 * some SOCs (e.g. on MX25, MX35 this bit is set, but they
 		 * don't actually support ADMA2). So set the BROKEN_ADMA
-		 * uirk on MX25/35 platforms.
+		 * quirk on MX25/35 platforms.
 		 */
 
 		if (val & SDHCI_CAN_DO_ADMA1) {
@@ -351,7 +351,7 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
 		if ((val & SDHCI_INT_CARD_INT) && !esdhc_is_usdhc(imx_data)) {
 			/*
 			 * Clear and then set D3CD bit to avoid missing the
-			 * card interrupt.  This is a eSDHC controller problem
+			 * card interrupt. This is an eSDHC controller problem
 			 * so we need to apply the following workaround: clear
 			 * and set D3CD bit will make eSDHC re-sample the card
 			 * interrupt. In case a card interrupt was lost,
@@ -604,7 +604,7 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
 		 * Do not touch buswidth bits here. This is done in
 		 * esdhc_pltfm_bus_width.
 		 * Do not touch the D3CD bit either which is used for the
-		 * SDIO interrupt errata workaround.
+		 * SDIO interrupt erratum workaround.
 		 */
 		mask = 0xffff & ~(ESDHC_CTRL_BUSWIDTH_MASK | ESDHC_CTRL_D3CD);
 
@@ -763,7 +763,7 @@ static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
 	writel(reg, host->ioaddr + ESDHC_MIX_CTRL);
 	writel(val << 8, host->ioaddr + ESDHC_TUNE_CTRL_STATUS);
 	dev_dbg(mmc_dev(host->mmc),
-		"tunning with delay 0x%x ESDHC_TUNE_CTRL_STATUS 0x%x\n",
+		"tuning with delay 0x%x ESDHC_TUNE_CTRL_STATUS 0x%x\n",
 			val, readl(host->ioaddr + ESDHC_TUNE_CTRL_STATUS));
 }
 
@@ -807,7 +807,7 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
 	ret = mmc_send_tuning(host->mmc, opcode, NULL);
 	esdhc_post_tuning(host);
 
-	dev_dbg(mmc_dev(host->mmc), "tunning %s at 0x%x ret %d\n",
+	dev_dbg(mmc_dev(host->mmc), "tuning %s at 0x%x ret %d\n",
 		ret ? "failed" : "passed", avg, ret);
 
 	return ret;
@@ -847,15 +847,15 @@ static int esdhc_change_pinstate(struct sdhci_host *host,
 }
 
 /*
- * For HS400 eMMC, there is a data_strobe line, this signal is generated
+ * For HS400 eMMC, there is a data_strobe line. This signal is generated
  * by the device and used for data output and CRC status response output
  * in HS400 mode. The frequency of this signal follows the frequency of
- * CLK generated by host. Host receive the data which is aligned to the
+ * CLK generated by host. The host receives the data which is aligned to the
  * edge of data_strobe line. Due to the time delay between CLK line and
  * data_strobe line, if the delay time is larger than one clock cycle,
- * then CLK and data_strobe line will misaligned, read error shows up.
+ * then CLK and data_strobe line will be misaligned, read error shows up.
  * So when the CLK is higher than 100MHz, each clock cycle is short enough,
- * host should config the delay target.
+ * host should configure the delay target.
  */
 static void esdhc_set_strobe_dll(struct sdhci_host *host)
 {
@@ -895,7 +895,7 @@ static void esdhc_reset_tuning(struct sdhci_host *host)
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
 	u32 ctrl;
 
-	/* Rest the tuning circurt */
+	/* Reset the tuning circuit */
 	if (esdhc_is_usdhc(imx_data)) {
 		if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) {
 			ctrl = readl(host->ioaddr + ESDHC_MIX_CTRL);
@@ -976,7 +976,7 @@ static unsigned int esdhc_get_max_timeout_count(struct sdhci_host *host)
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
 
-	/* Doc Errata: the uSDHC actual maximum timeout count is 1 << 29 */
+	/* Doc Erratum: the uSDHC actual maximum timeout count is 1 << 29 */
 	return esdhc_is_usdhc(imx_data) ? 1 << 29 : 1 << 27;
 }
 
@@ -1032,10 +1032,10 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
 
 		/*
 		 * ROM code will change the bit burst_length_enable setting
-		 * to zero if this usdhc is choosed to boot system. Change
+		 * to zero if this usdhc is chosen to boot system. Change
 		 * it back here, otherwise it will impact the performance a
 		 * lot. This bit is used to enable/disable the burst length
-		 * for the external AHB2AXI bridge, it's usefully especially
+		 * for the external AHB2AXI bridge. It's useful especially
 		 * for INCR transfer because without burst length indicator,
 		 * the AHB2AXI bridge does not know the burst length in
 		 * advance. And without burst length indicator, AHB INCR
@@ -1045,7 +1045,7 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
 			| ESDHC_BURST_LEN_EN_INCR,
 			host->ioaddr + SDHCI_HOST_CONTROL);
 		/*
-		* errata ESDHC_FLAG_ERR004536 fix for MX6Q TO1.2 and MX6DL
+		* erratum ESDHC_FLAG_ERR004536 fix for MX6Q TO1.2 and MX6DL
 		* TO1.1, it's harmless for MX6SL
 		*/
 		writel(readl(host->ioaddr + 0x6c) | BIT(7),
@@ -1104,7 +1104,7 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 
 	mmc_of_parse_voltage(np, &host->ocr_mask);
 
-	/* sdr50 and sdr104 needs work on 1.8v signal voltage */
+	/* sdr50 and sdr104 need work on 1.8v signal voltage */
 	if ((boarddata->support_vsel) && esdhc_is_usdhc(imx_data) &&
 	    !IS_ERR(imx_data->pins_default)) {
 		imx_data->pins_100mhz = pinctrl_lookup_state(imx_data->pinctrl,
@@ -1116,7 +1116,8 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 			dev_warn(mmc_dev(host->mmc),
 				"could not get ultra high speed state, work on normal mode\n");
 			/*
-			 * fall back to not support uhs by specify no 1.8v quirk
+			 * fall back to not supporting uhs by specifying no
+			 * 1.8v quirk
 			 */
 			host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
 		}
@@ -1272,7 +1273,7 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 		dev_warn(mmc_dev(host->mmc), "could not get default state\n");
 
 	if (imx_data->socdata->flags & ESDHC_FLAG_ENGCM07207)
-		/* Fix errata ENGcm07207 present on i.MX25 and i.MX35 */
+		/* Fix erratum ENGcm07207 present on i.MX25 and i.MX35 */
 		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK
 			| SDHCI_QUIRK_BROKEN_ADMA;
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/5] mmc: sdhci-esdhc: Add SDHCI_QUIRK_32BIT_DMA_ADDR
  2017-05-29 13:54 [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos Benoît Thébaudeau
@ 2017-05-29 13:54 ` Benoît Thébaudeau
  2017-05-29 13:54 ` [PATCH v2 3/5] mmc: sdhci-esdhc-imx: Fix DAT line software reset Benoît Thébaudeau
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Benoît Thébaudeau @ 2017-05-29 13:54 UTC (permalink / raw)
  To: linux-kernel, linux-mmc
  Cc: Ulf Hansson, Adrian Hunter, Fabio Estevam, Wolfram Sang,
	Benoît Thébaudeau

The eSDHC can only DMA from 32-bit-aligned addresses.

This fixes the following test cases of mmc_test:
  11:	Badly aligned write
  12:	Badly aligned read
  13:	Badly aligned multi-block write
  14:	Badly aligned multi-block read

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes v1 -> v2: none.
---
 drivers/mmc/host/sdhci-esdhc.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index c4bbd74..e7893f2 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -19,6 +19,7 @@
  */
 
 #define ESDHC_DEFAULT_QUIRKS	(SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
+				SDHCI_QUIRK_32BIT_DMA_ADDR | \
 				SDHCI_QUIRK_NO_BUSY_IRQ | \
 				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
 				SDHCI_QUIRK_PIO_NEEDS_DELAY | \
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/5] mmc: sdhci-esdhc-imx: Fix DAT line software reset
  2017-05-29 13:54 [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos Benoît Thébaudeau
  2017-05-29 13:54 ` [PATCH v2 2/5] mmc: sdhci-esdhc: Add SDHCI_QUIRK_32BIT_DMA_ADDR Benoît Thébaudeau
@ 2017-05-29 13:54 ` Benoît Thébaudeau
  2017-05-29 13:54 ` [PATCH v2 4/5] mmc: sdhci-esdhc-imx: Allow all supported prescaler values Benoît Thébaudeau
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Benoît Thébaudeau @ 2017-05-29 13:54 UTC (permalink / raw)
  To: linux-kernel, linux-mmc
  Cc: Ulf Hansson, Adrian Hunter, Fabio Estevam, Wolfram Sang,
	Benoît Thébaudeau

On i.MX25, the eSDHC DAT line software reset (SYSCTL.RSTD) unexpectedly
clears at least the data transfer width (PROCTL.DTW), which then results
in data CRC errors. This behavior is not documented, but it has actually
been observed. Consequently, the DAT line software resets triggered by
sdhci.c in case of errors caused unrecoverable errors.

Fix this by making sure that the DAT line software reset does not alter
the Host Control register. This behavior being undocumented, it may also
be present on other i.MX SoCs, so apply this fix for the whole i.MX
family.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes v1 -> v2: none.
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 59 ++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 6095a14..a0b7d4c 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -579,7 +579,7 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
-	u32 new_val;
+	u32 new_val = 0;
 	u32 mask;
 
 	switch (reg) {
@@ -610,29 +610,46 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
 
 		esdhc_clrset_le(host, mask, new_val, reg);
 		return;
+	case SDHCI_SOFTWARE_RESET:
+		if (val & SDHCI_RESET_DATA)
+			new_val = readl(host->ioaddr + SDHCI_HOST_CONTROL);
+		break;
 	}
 	esdhc_clrset_le(host, 0xff, val, reg);
 
-	/*
-	 * The esdhc has a design violation to SDHC spec which tells
-	 * that software reset should not affect card detection circuit.
-	 * But esdhc clears its SYSCTL register bits [0..2] during the
-	 * software reset.  This will stop those clocks that card detection
-	 * circuit relies on.  To work around it, we turn the clocks on back
-	 * to keep card detection circuit functional.
-	 */
-	if ((reg == SDHCI_SOFTWARE_RESET) && (val & 1)) {
-		esdhc_clrset_le(host, 0x7, 0x7, ESDHC_SYSTEM_CONTROL);
-		/*
-		 * The reset on usdhc fails to clear MIX_CTRL register.
-		 * Do it manually here.
-		 */
-		if (esdhc_is_usdhc(imx_data)) {
-			/* the tuning bits should be kept during reset */
-			new_val = readl(host->ioaddr + ESDHC_MIX_CTRL);
-			writel(new_val & ESDHC_MIX_CTRL_TUNING_MASK,
-					host->ioaddr + ESDHC_MIX_CTRL);
-			imx_data->is_ddr = 0;
+	if (reg == SDHCI_SOFTWARE_RESET) {
+		if (val & SDHCI_RESET_ALL) {
+			/*
+			 * The esdhc has a design violation to SDHC spec which
+			 * tells that software reset should not affect card
+			 * detection circuit. But esdhc clears its SYSCTL
+			 * register bits [0..2] during the software reset. This
+			 * will stop those clocks that card detection circuit
+			 * relies on. To work around it, we turn the clocks on
+			 * back to keep card detection circuit functional.
+			 */
+			esdhc_clrset_le(host, 0x7, 0x7, ESDHC_SYSTEM_CONTROL);
+			/*
+			 * The reset on usdhc fails to clear MIX_CTRL register.
+			 * Do it manually here.
+			 */
+			if (esdhc_is_usdhc(imx_data)) {
+				/*
+				 * the tuning bits should be kept during reset
+				 */
+				new_val = readl(host->ioaddr + ESDHC_MIX_CTRL);
+				writel(new_val & ESDHC_MIX_CTRL_TUNING_MASK,
+						host->ioaddr + ESDHC_MIX_CTRL);
+				imx_data->is_ddr = 0;
+			}
+		} else if (val & SDHCI_RESET_DATA) {
+			/*
+			 * The eSDHC DAT line software reset clears at least the
+			 * data transfer width on i.MX25, so make sure that the
+			 * Host Control register is unaffected.
+			 */
+			esdhc_clrset_le(host, 0xff, new_val,
+					SDHCI_HOST_CONTROL);
 		}
 	}
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 4/5] mmc: sdhci-esdhc-imx: Allow all supported prescaler values
  2017-05-29 13:54 [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos Benoît Thébaudeau
  2017-05-29 13:54 ` [PATCH v2 2/5] mmc: sdhci-esdhc: Add SDHCI_QUIRK_32BIT_DMA_ADDR Benoît Thébaudeau
  2017-05-29 13:54 ` [PATCH v2 3/5] mmc: sdhci-esdhc-imx: Fix DAT line software reset Benoît Thébaudeau
@ 2017-05-29 13:54 ` Benoît Thébaudeau
  2017-05-29 13:54 ` [PATCH v2 5/5] mmc: sdhci-esdhc-imx: Remove the ENGcm07207 workaround Benoît Thébaudeau
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Benoît Thébaudeau @ 2017-05-29 13:54 UTC (permalink / raw)
  To: linux-kernel, linux-mmc
  Cc: Ulf Hansson, Adrian Hunter, Fabio Estevam, Wolfram Sang,
	Benoît Thébaudeau

On i.MX, SYSCTL.SDCLKFS may always be set to 0 in order to make the SD
clock frequency prescaler divide by 1 in SDR mode, even with the eSDHC.
The previous minimum prescaler value of 2 in SDR mode with the eSDHC was
a code remnant from PowerPC, which actually has this limitation on
earlier revisions.

In DDR mode, the prescaler can divide by up to 512.

The maximum SD clock frequency in High Speed mode is 50 MHz. On i.MX25,
this change makes it possible to get 48 MHz from the USB PLL
(240 MHz / 5 / 1) instead of only 40 MHz from the USB PLL
(240 MHz / 3 / 2) or 33.25 MHz from the AHB clock (133 MHz / 2 / 2).

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes v1 -> v2: none.
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a0b7d4c..e0d06d9 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -674,7 +674,8 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
 	unsigned int host_clock = pltfm_host->clock;
-	int pre_div = 2;
+	int ddr_pre_div = imx_data->is_ddr ? 2 : 1;
+	int pre_div = 1;
 	int div = 1;
 	u32 temp, val;
 
@@ -689,28 +690,23 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 		return;
 	}
 
-	if (esdhc_is_usdhc(imx_data) && !imx_data->is_ddr)
-		pre_div = 1;
-
 	temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL);
 	temp &= ~(ESDHC_CLOCK_IPGEN | ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN
 		| ESDHC_CLOCK_MASK);
 	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
 
-	while (host_clock / pre_div / 16 > clock && pre_div < 256)
+	while (host_clock / (16 * pre_div * ddr_pre_div) > clock &&
+			pre_div < 256)
 		pre_div *= 2;
 
-	while (host_clock / pre_div / div > clock && div < 16)
+	while (host_clock / (div * pre_div * ddr_pre_div) > clock && div < 16)
 		div++;
 
-	host->mmc->actual_clock = host_clock / pre_div / div;
+	host->mmc->actual_clock = host_clock / (div * pre_div * ddr_pre_div);
 	dev_dbg(mmc_dev(host->mmc), "desired SD clock: %d, actual: %d\n",
 		clock, host->mmc->actual_clock);
 
-	if (imx_data->is_ddr)
-		pre_div >>= 2;
-	else
-		pre_div >>= 1;
+	pre_div >>= 1;
 	div--;
 
 	temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 5/5] mmc: sdhci-esdhc-imx: Remove the ENGcm07207 workaround
  2017-05-29 13:54 [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos Benoît Thébaudeau
                   ` (2 preceding siblings ...)
  2017-05-29 13:54 ` [PATCH v2 4/5] mmc: sdhci-esdhc-imx: Allow all supported prescaler values Benoît Thébaudeau
@ 2017-05-29 13:54 ` Benoît Thébaudeau
  2017-05-29 14:01 ` [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos Fabio Estevam
  2017-05-30  6:36 ` Adrian Hunter
  5 siblings, 0 replies; 7+ messages in thread
From: Benoît Thébaudeau @ 2017-05-29 13:54 UTC (permalink / raw)
  To: linux-kernel, linux-mmc
  Cc: Ulf Hansson, Adrian Hunter, Fabio Estevam, Wolfram Sang,
	Benoît Thébaudeau

The SDHCI_QUIRK_NO_MULTIBLOCK quirk was used as a workaround for the
ENGcm07207 erratum. However, it caused excruciatingly slow SD transfers
(300 kB/s on average), and this erratum actually does not imply that
multiple-block transfers are not supported, so this was overkill.

The suggested workaround for this erratum is to set SYSCTL.RSTA, but the
simple DAT line software reset (which resets the DMA circuit among
others) triggered by sdhci_finish_data() in case of errors seems to be
sufficient. Indeed, generating errors in a controlled manner on i.MX25
using the FEVT register right in the middle of read data transfers
without this quirk shows that nothing is written to the buffer by the
eSDHC past CMD12, and no extra Auto CMD12 is sent with AC12EN set, so
the data transfers on AHB are properly aborted. For write data
transfers, neither extra data nor extra Auto CMD12 is sent, as expected.
Moreover, after intensive stress tests on i.MX25, removing
SDHCI_QUIRK_NO_MULTIBLOCK seems to be safe.

SDHCI_QUIRK_BROKEN_ADMA has nothing to do with ENGcm07207, so set
ESDHC_FLAG_ERR004536 for the devices that had ESDHC_FLAG_ENGCM07207 set
in order to continue getting SDHCI_QUIRK_BROKEN_ADMA.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes v1 -> v2:
 - Spelling fixes moved to a separate patch (suggested by Adrian
   Hunter).
 - Comments added about SDHCI_QUIRK_BROKEN_ADMA.
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index e0d06d9..dc542f9 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -115,11 +115,6 @@
  */
 #define ESDHC_FLAG_MULTIBLK_NO_INT	BIT(1)
 /*
- * The flag enables the workaround for ESDHC erratum ENGcm07207 which
- * affects i.MX25 and i.MX35.
- */
-#define ESDHC_FLAG_ENGCM07207		BIT(2)
-/*
  * The flag tells that the ESDHC controller is an USDHC block that is
  * integrated on the i.MX6 series.
  */
@@ -134,6 +129,8 @@
  * The IP has erratum ERR004536
  * uSDHC: ADMA Length Mismatch Error occurs if the AHB read access is slow,
  * when reading data from the card
+ * This flag is also set for i.MX25 and i.MX35 in order to get
+ * SDHCI_QUIRK_BROKEN_ADMA, but for different reasons (ADMA capability bits).
  */
 #define ESDHC_FLAG_ERR004536		BIT(7)
 /* The IP supports HS200 mode */
@@ -149,11 +146,11 @@ struct esdhc_soc_data {
 };
 
 static struct esdhc_soc_data esdhc_imx25_data = {
-	.flags = ESDHC_FLAG_ENGCM07207,
+	.flags = ESDHC_FLAG_ERR004536,
 };
 
 static struct esdhc_soc_data esdhc_imx35_data = {
-	.flags = ESDHC_FLAG_ENGCM07207,
+	.flags = ESDHC_FLAG_ERR004536,
 };
 
 static struct esdhc_soc_data esdhc_imx51_data = {
@@ -1285,11 +1282,6 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	if (IS_ERR(imx_data->pins_default))
 		dev_warn(mmc_dev(host->mmc), "could not get default state\n");
 
-	if (imx_data->socdata->flags & ESDHC_FLAG_ENGCM07207)
-		/* Fix erratum ENGcm07207 present on i.MX25 and i.MX35 */
-		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK
-			| SDHCI_QUIRK_BROKEN_ADMA;
-
 	if (esdhc_is_usdhc(imx_data)) {
 		host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
 		host->mmc->caps |= MMC_CAP_1_8V_DDR;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos
  2017-05-29 13:54 [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos Benoît Thébaudeau
                   ` (3 preceding siblings ...)
  2017-05-29 13:54 ` [PATCH v2 5/5] mmc: sdhci-esdhc-imx: Remove the ENGcm07207 workaround Benoît Thébaudeau
@ 2017-05-29 14:01 ` Fabio Estevam
  2017-05-30  6:36 ` Adrian Hunter
  5 siblings, 0 replies; 7+ messages in thread
From: Fabio Estevam @ 2017-05-29 14:01 UTC (permalink / raw)
  To: Benoît Thébaudeau
  Cc: linux-kernel, linux-mmc@vger.kernel.org, Ulf Hansson,
	Adrian Hunter, Fabio Estevam, Wolfram Sang

On Mon, May 29, 2017 at 10:54 AM, Benoît Thébaudeau <benoit@wsystem.com> wrote:
> Fix various English mistakes and typos in comments and in printed
> strings.
>
> Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos
  2017-05-29 13:54 [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos Benoît Thébaudeau
                   ` (4 preceding siblings ...)
  2017-05-29 14:01 ` [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos Fabio Estevam
@ 2017-05-30  6:36 ` Adrian Hunter
  5 siblings, 0 replies; 7+ messages in thread
From: Adrian Hunter @ 2017-05-30  6:36 UTC (permalink / raw)
  To: Benoît Thébaudeau, linux-kernel, linux-mmc
  Cc: Ulf Hansson, Fabio Estevam, Wolfram Sang

On 29/05/17 16:54, Benoît Thébaudeau wrote:
> Fix various English mistakes and typos in comments and in printed
> strings.
> 
> Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>

There are 2 more I noted below.  Otherwise:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> Changes v1 -> v2: new patch.
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 47 +++++++++++++++++++-------------------
>  1 file changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index b5c7241..6095a14 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -95,7 +95,7 @@
>  #define ESDHC_CTRL_BUSWIDTH_MASK	(0x3 << 1)
>  
>  /*
> - * There is an INT DMA ERR mis-match between eSDHC and STD SDHC SPEC:
> + * There is an INT DMA ERR mismatch between eSDHC and STD SDHC SPEC:
>   * Bit25 is used in STD SPEC, and is reserved in fsl eSDHC design,
>   * but bit28 is used as the INT DMA ERR in fsl eSDHC design.
>   * Define this macro DMA error INT for fsl eSDHC
> @@ -110,12 +110,12 @@
>   * In exact block transfer, the controller doesn't complete the
>   * operations automatically as required at the end of the
>   * transfer and remains on hold if the abort command is not sent.
> - * As a result, the TC flag is not asserted and SW  received timeout
> + * As a result, the TC flag is not asserted and SW received timeout
>   * exeception. Bit1 of Vendor Spec registor is used to fix it.

exeception -> exception
registor -> register

>   */
>  #define ESDHC_FLAG_MULTIBLK_NO_INT	BIT(1)
>  /*
> - * The flag enables the workaround for ESDHC errata ENGcm07207 which
> + * The flag enables the workaround for ESDHC erratum ENGcm07207 which
>   * affects i.MX25 and i.MX35.
>   */
>  #define ESDHC_FLAG_ENGCM07207		BIT(2)
> @@ -131,7 +131,7 @@
>  /* The IP has SDHCI_CAPABILITIES_1 register */
>  #define ESDHC_FLAG_HAVE_CAP1		BIT(6)
>  /*
> - * The IP has errata ERR004536
> + * The IP has erratum ERR004536
>   * uSDHC: ADMA Length Mismatch Error occurs if the AHB read access is slow,
>   * when reading data from the card
>   */
> @@ -141,7 +141,7 @@
>  /* The IP supports HS400 mode */
>  #define ESDHC_FLAG_HS400		BIT(9)
>  
> -/* A higher clock ferquency than this rate requires strobell dll control */
> +/* A clock frequency higher than this rate requires strobe dll control */
>  #define ESDHC_STROBE_DLL_CLK_FREQ	100000000
>  
>  struct esdhc_soc_data {
> @@ -197,7 +197,7 @@ struct pltfm_imx_data {
>  	struct clk *clk_ahb;
>  	struct clk *clk_per;
>  	enum {
> -		NO_CMD_PENDING,      /* no multiblock command pending*/
> +		NO_CMD_PENDING,      /* no multiblock command pending */
>  		MULTIBLK_IN_PROCESS, /* exact multiblock cmd in process */
>  		WAIT_FOR_INT,        /* sent CMD12, waiting for response INT */
>  	} multiblock_status;
> @@ -286,7 +286,7 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
>  		 * ADMA2 capability of esdhc, but this bit is messed up on
>  		 * some SOCs (e.g. on MX25, MX35 this bit is set, but they
>  		 * don't actually support ADMA2). So set the BROKEN_ADMA
> -		 * uirk on MX25/35 platforms.
> +		 * quirk on MX25/35 platforms.
>  		 */
>  
>  		if (val & SDHCI_CAN_DO_ADMA1) {
> @@ -351,7 +351,7 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
>  		if ((val & SDHCI_INT_CARD_INT) && !esdhc_is_usdhc(imx_data)) {
>  			/*
>  			 * Clear and then set D3CD bit to avoid missing the
> -			 * card interrupt.  This is a eSDHC controller problem
> +			 * card interrupt. This is an eSDHC controller problem
>  			 * so we need to apply the following workaround: clear
>  			 * and set D3CD bit will make eSDHC re-sample the card
>  			 * interrupt. In case a card interrupt was lost,
> @@ -604,7 +604,7 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
>  		 * Do not touch buswidth bits here. This is done in
>  		 * esdhc_pltfm_bus_width.
>  		 * Do not touch the D3CD bit either which is used for the
> -		 * SDIO interrupt errata workaround.
> +		 * SDIO interrupt erratum workaround.
>  		 */
>  		mask = 0xffff & ~(ESDHC_CTRL_BUSWIDTH_MASK | ESDHC_CTRL_D3CD);
>  
> @@ -763,7 +763,7 @@ static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
>  	writel(reg, host->ioaddr + ESDHC_MIX_CTRL);
>  	writel(val << 8, host->ioaddr + ESDHC_TUNE_CTRL_STATUS);
>  	dev_dbg(mmc_dev(host->mmc),
> -		"tunning with delay 0x%x ESDHC_TUNE_CTRL_STATUS 0x%x\n",
> +		"tuning with delay 0x%x ESDHC_TUNE_CTRL_STATUS 0x%x\n",
>  			val, readl(host->ioaddr + ESDHC_TUNE_CTRL_STATUS));
>  }
>  
> @@ -807,7 +807,7 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
>  	ret = mmc_send_tuning(host->mmc, opcode, NULL);
>  	esdhc_post_tuning(host);
>  
> -	dev_dbg(mmc_dev(host->mmc), "tunning %s at 0x%x ret %d\n",
> +	dev_dbg(mmc_dev(host->mmc), "tuning %s at 0x%x ret %d\n",
>  		ret ? "failed" : "passed", avg, ret);
>  
>  	return ret;
> @@ -847,15 +847,15 @@ static int esdhc_change_pinstate(struct sdhci_host *host,
>  }
>  
>  /*
> - * For HS400 eMMC, there is a data_strobe line, this signal is generated
> + * For HS400 eMMC, there is a data_strobe line. This signal is generated
>   * by the device and used for data output and CRC status response output
>   * in HS400 mode. The frequency of this signal follows the frequency of
> - * CLK generated by host. Host receive the data which is aligned to the
> + * CLK generated by host. The host receives the data which is aligned to the
>   * edge of data_strobe line. Due to the time delay between CLK line and
>   * data_strobe line, if the delay time is larger than one clock cycle,
> - * then CLK and data_strobe line will misaligned, read error shows up.
> + * then CLK and data_strobe line will be misaligned, read error shows up.
>   * So when the CLK is higher than 100MHz, each clock cycle is short enough,
> - * host should config the delay target.
> + * host should configure the delay target.
>   */
>  static void esdhc_set_strobe_dll(struct sdhci_host *host)
>  {
> @@ -895,7 +895,7 @@ static void esdhc_reset_tuning(struct sdhci_host *host)
>  	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
>  	u32 ctrl;
>  
> -	/* Rest the tuning circurt */
> +	/* Reset the tuning circuit */
>  	if (esdhc_is_usdhc(imx_data)) {
>  		if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) {
>  			ctrl = readl(host->ioaddr + ESDHC_MIX_CTRL);
> @@ -976,7 +976,7 @@ static unsigned int esdhc_get_max_timeout_count(struct sdhci_host *host)
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
>  
> -	/* Doc Errata: the uSDHC actual maximum timeout count is 1 << 29 */
> +	/* Doc Erratum: the uSDHC actual maximum timeout count is 1 << 29 */
>  	return esdhc_is_usdhc(imx_data) ? 1 << 29 : 1 << 27;
>  }
>  
> @@ -1032,10 +1032,10 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
>  
>  		/*
>  		 * ROM code will change the bit burst_length_enable setting
> -		 * to zero if this usdhc is choosed to boot system. Change
> +		 * to zero if this usdhc is chosen to boot system. Change
>  		 * it back here, otherwise it will impact the performance a
>  		 * lot. This bit is used to enable/disable the burst length
> -		 * for the external AHB2AXI bridge, it's usefully especially
> +		 * for the external AHB2AXI bridge. It's useful especially
>  		 * for INCR transfer because without burst length indicator,
>  		 * the AHB2AXI bridge does not know the burst length in
>  		 * advance. And without burst length indicator, AHB INCR
> @@ -1045,7 +1045,7 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
>  			| ESDHC_BURST_LEN_EN_INCR,
>  			host->ioaddr + SDHCI_HOST_CONTROL);
>  		/*
> -		* errata ESDHC_FLAG_ERR004536 fix for MX6Q TO1.2 and MX6DL
> +		* erratum ESDHC_FLAG_ERR004536 fix for MX6Q TO1.2 and MX6DL
>  		* TO1.1, it's harmless for MX6SL
>  		*/
>  		writel(readl(host->ioaddr + 0x6c) | BIT(7),
> @@ -1104,7 +1104,7 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
>  
>  	mmc_of_parse_voltage(np, &host->ocr_mask);
>  
> -	/* sdr50 and sdr104 needs work on 1.8v signal voltage */
> +	/* sdr50 and sdr104 need work on 1.8v signal voltage */
>  	if ((boarddata->support_vsel) && esdhc_is_usdhc(imx_data) &&
>  	    !IS_ERR(imx_data->pins_default)) {
>  		imx_data->pins_100mhz = pinctrl_lookup_state(imx_data->pinctrl,
> @@ -1116,7 +1116,8 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
>  			dev_warn(mmc_dev(host->mmc),
>  				"could not get ultra high speed state, work on normal mode\n");
>  			/*
> -			 * fall back to not support uhs by specify no 1.8v quirk
> +			 * fall back to not supporting uhs by specifying no
> +			 * 1.8v quirk
>  			 */
>  			host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
>  		}
> @@ -1272,7 +1273,7 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  		dev_warn(mmc_dev(host->mmc), "could not get default state\n");
>  
>  	if (imx_data->socdata->flags & ESDHC_FLAG_ENGCM07207)
> -		/* Fix errata ENGcm07207 present on i.MX25 and i.MX35 */
> +		/* Fix erratum ENGcm07207 present on i.MX25 and i.MX35 */
>  		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK
>  			| SDHCI_QUIRK_BROKEN_ADMA;
>  
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-05-30  6:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-29 13:54 [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos Benoît Thébaudeau
2017-05-29 13:54 ` [PATCH v2 2/5] mmc: sdhci-esdhc: Add SDHCI_QUIRK_32BIT_DMA_ADDR Benoît Thébaudeau
2017-05-29 13:54 ` [PATCH v2 3/5] mmc: sdhci-esdhc-imx: Fix DAT line software reset Benoît Thébaudeau
2017-05-29 13:54 ` [PATCH v2 4/5] mmc: sdhci-esdhc-imx: Allow all supported prescaler values Benoît Thébaudeau
2017-05-29 13:54 ` [PATCH v2 5/5] mmc: sdhci-esdhc-imx: Remove the ENGcm07207 workaround Benoît Thébaudeau
2017-05-29 14:01 ` [PATCH v2 1/5] mmc: sdhci-esdhc-imx: Fix some English mistakes and typos Fabio Estevam
2017-05-30  6:36 ` Adrian Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).