[PATCH 1/6] mmc: sdhci-esdhc-imx: remove redundant code for manual tuning

Subsystems: multimedia card (mmc), secure digital (sd) and sdio subsystem, secure digital host controller interface (sdhci) driver, secure digital host controller interface (sdhci) nxp i.mx driver, the rest

STALE1813d

16 messages, 2 authors, 2021-08-25 · open the first message on its own page

[PATCH 1/6] mmc: sdhci-esdhc-imx: remove redundant code for manual tuning

From: haibo.chen@nxp.com
Date: 2021-08-16 13:00:24

From: Haibo Chen <haibo.chen@nxp.com>

For manual tuning method, already call esdhc_prepare_tuning()
config the necessary registers, so remove the redundant code
in esdhc_writew_le() for SDHCI_HOST_CONTROL2.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 57b19ca1ad6d..a49fac719fca 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -628,17 +628,7 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
 		else
 			new_val &= ~ESDHC_VENDOR_SPEC_VSELECT;
 		writel(new_val, host->ioaddr + ESDHC_VENDOR_SPEC);
-		if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) {
-			new_val = readl(host->ioaddr + ESDHC_MIX_CTRL);
-			if (val & SDHCI_CTRL_TUNED_CLK) {
-				new_val |= ESDHC_MIX_CTRL_SMPCLK_SEL;
-				new_val |= ESDHC_MIX_CTRL_AUTO_TUNE_EN;
-			} else {
-				new_val &= ~ESDHC_MIX_CTRL_SMPCLK_SEL;
-				new_val &= ~ESDHC_MIX_CTRL_AUTO_TUNE_EN;
-			}
-			writel(new_val , host->ioaddr + ESDHC_MIX_CTRL);
-		} else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) {
+		if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) {
 			u32 v = readl(host->ioaddr + SDHCI_AUTO_CMD_STATUS);
 			u32 m = readl(host->ioaddr + ESDHC_MIX_CTRL);
 			if (val & SDHCI_CTRL_TUNED_CLK) {
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/6] dt-bindings: mmc: fsl-imx-esdhc: add fsl,sdio-async-interrupt-enabled binding

From: haibo.chen@nxp.com
Date: 2021-08-16 13:00:25

From: Haibo Chen <haibo.chen@nxp.com>

Add a new fsl,sdio-async-interrupt-enabled binding for sdio devices
which enable the async interrupt function. When get this property,
driver will avoid to use DAT[1] for hardware auto tuning check.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml         | 10 ++++++++++
 1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
index b5baf439fbac..8a9f1775b0e2 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
@@ -122,6 +122,16 @@ properties:
       - const: state_200mhz
       - const: sleep
 
+  fsl,sdio-async-interrupt-enabled:
+    description: |
+      Recommend for SDIO cards that enables SDIO async interrupt for SDR104 and SDR50
+      operating modes. SDIO async interrupt uses DAT[1] to signal the card's interrupt.
+      uSDHC tuning mechanism must use DAT[0] and CMD signals to avoid a possible
+      conflict and incorrect delay line calculated by the uSDHC auto tuning mechanism.
+      Enabling this device tree property is only recommended for layouts that are
+      matching the SD interface length.
+    type: boolean
+
 required:
   - compatible
   - reg
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/6] mmc: sdhci-eadhc-imx: select the correct mode for auto tuning

From: haibo.chen@nxp.com
Date: 2021-08-16 13:00:25

From: Haibo Chen <haibo.chen@nxp.com>

USDHC hardware auto tuning circuit support check 1/4/8 data lines
and cmd line. Out of reset uSDHC, it default select check 4 data
lines and do not check cmd line. This is incorrect if we use 8 data
lines. So need to config the auto tuning mode according to current
bus width.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 33 ++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a49fac719fca..f18d169bc8ff 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -94,6 +94,11 @@
 
 #define ESDHC_VEND_SPEC2		0xc8
 #define ESDHC_VEND_SPEC2_EN_BUSY_IRQ	(1 << 8)
+#define ESDHC_VEND_SPEC2_AUTO_TUNE_8BIT_EN	(1 << 4)
+#define ESDHC_VEND_SPEC2_AUTO_TUNE_4BIT_EN	(0 << 4)
+#define ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN	(2 << 4)
+#define ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN	(1 << 6)
+#define ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK	(7 << 4)
 
 #define ESDHC_TUNING_CTRL		0xcc
 #define ESDHC_STD_TUNING_EN		(1 << 24)
@@ -114,6 +119,7 @@
 #define ESDHC_CTRL_4BITBUS		(0x1 << 1)
 #define ESDHC_CTRL_8BITBUS		(0x2 << 1)
 #define ESDHC_CTRL_BUSWIDTH_MASK	(0x3 << 1)
+#define USDHC_GET_BUSWIDTH(c) (c & ESDHC_CTRL_BUSWIDTH_MASK)
 
 /*
  * There is an INT DMA ERR mismatch between eSDHC and STD SDHC SPEC:
@@ -407,6 +413,30 @@ static inline void esdhc_wait_for_card_clock_gate_off(struct sdhci_host *host)
 		dev_warn(mmc_dev(host->mmc), "%s: card clock still not gate off in 100us!.\n", __func__);
 }
 
+/* Enable the auto tuning circuit to check the CMD line and BUS line */
+static inline void usdhc_auto_tuning_mode_sel(struct sdhci_host *host)
+{
+	u32 buswidth, auto_tune_buswidth;
+
+	buswidth = USDHC_GET_BUSWIDTH(readl(host->ioaddr + SDHCI_HOST_CONTROL));
+
+	switch (buswidth) {
+	case ESDHC_CTRL_8BITBUS:
+		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_8BIT_EN;
+		break;
+	case ESDHC_CTRL_4BITBUS:
+		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_4BIT_EN;
+		break;
+	default:	/* 1BITBUS */
+		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
+		break;
+	}
+
+	esdhc_clrset_le(host, ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK,
+			auto_tune_buswidth | ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN,
+			ESDHC_VEND_SPEC2);
+}
+
 static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -643,6 +673,7 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
 				v |= ESDHC_MIX_CTRL_EXE_TUNE;
 				m |= ESDHC_MIX_CTRL_FBCLK_SEL;
 				m |= ESDHC_MIX_CTRL_AUTO_TUNE_EN;
+				usdhc_auto_tuning_mode_sel(host);
 			} else {
 				v &= ~ESDHC_MIX_CTRL_EXE_TUNE;
 			}
@@ -1012,6 +1043,8 @@ static void esdhc_post_tuning(struct sdhci_host *host)
 {
 	u32 reg;
 
+	usdhc_auto_tuning_mode_sel(host);
+
 	reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
 	reg &= ~ESDHC_MIX_CTRL_EXE_TUNE;
 	reg |= ESDHC_MIX_CTRL_AUTO_TUNE_EN;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] mmc: host: sdhci-esdhc-imx.c: correct the auto-tuning setting for sdio device

From: haibo.chen@nxp.com
Date: 2021-08-16 13:00:26

From: Haibo Chen <haibo.chen@nxp.com>

USDHC contain auto tuning circuit, this circuit will work automatically after
the tuning procedurae, it can increase/decrease the delay cell according to
the outside environment change (like temperature).

Unfortunately, this auto tuning circuit can not handle the async sdio device
interrupt correctly. When sdio device use 4 data line, async sdio interrupt
will use DAT[1], if we enable auto tuning circuit check 4 data lines, include
the DAT[1], this circuit will detect this interrupt, take this as a data on
DAT[1], and adjust the delay cell wrongly.

This is the hardware design limitation, to avoid this, when sdio device enable
async interrupt, auto tuning circuit only check DAT[0] and CMD lines.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index f18d169bc8ff..ab84c29777e5 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -226,6 +226,7 @@ struct esdhc_platform_data {
 	unsigned int tuning_step;       /* The delay cell steps in tuning procedure */
 	unsigned int tuning_start_tap;	/* The start delay cell point in tuning procedure */
 	unsigned int strobe_dll_delay_target;	/* The delay cell for strobe pad (read clock) */
+	bool sdio_async_interrupt_enabled;	/* sdio device enable the async interrupt */
 };
 
 struct esdhc_soc_data {
@@ -416,6 +417,8 @@ static inline void esdhc_wait_for_card_clock_gate_off(struct sdhci_host *host)
 /* Enable the auto tuning circuit to check the CMD line and BUS line */
 static inline void usdhc_auto_tuning_mode_sel(struct sdhci_host *host)
 {
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
 	u32 buswidth, auto_tune_buswidth;
 
 	buswidth = USDHC_GET_BUSWIDTH(readl(host->ioaddr + SDHCI_HOST_CONTROL));
@@ -432,6 +435,18 @@ static inline void usdhc_auto_tuning_mode_sel(struct sdhci_host *host)
 		break;
 	}
 
+	/*
+	 * If sdio device use async interrupt, it will use DAT[1] to signal
+	 * the device's interrupt asynchronous when use 4 data lines.
+	 * Then hardware auto tuning circuit MUST NOT check the DAT[1] line,
+	 * otherwise auto tuning will be impacted by this async interrupt,
+	 * and change the delay cell incorrectly, which then cause data/cmd
+	 * errors.
+	 * This is the hardware auto tuning circuit limitation.
+	 */
+	if (imx_data->boarddata.sdio_async_interrupt_enabled)
+		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
+
 	esdhc_clrset_le(host, ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK,
 			auto_tune_buswidth | ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN,
 			ESDHC_VEND_SPEC2);
@@ -1531,6 +1546,9 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 	if (of_property_read_u32(np, "fsl,delay-line", &boarddata->delay_line))
 		boarddata->delay_line = 0;
 
+	if (of_property_read_bool(np, "fsl,sdio-async-interrupt-enabled"))
+		boarddata->sdio_async_interrupt_enabled = true;
+
 	mmc_of_parse_voltage(host->mmc, &host->ocr_mask);
 
 	if (esdhc_is_usdhc(imx_data) && !IS_ERR(imx_data->pinctrl)) {
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/6] arm64: dts: imx8mm-evk: add sdio wifi support

From: haibo.chen@nxp.com
Date: 2021-08-16 13:00:27

From: Haibo Chen <haibo.chen@nxp.com>

Add the sdio wifi support on imx8mm-evk board.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm-evk.dts  | 20 ++++++++++
 arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 39 +++++++++++++++++++
 2 files changed, 59 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dts b/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
index 4e2820d19244..41f9453c5a3f 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
@@ -15,6 +15,13 @@
 	aliases {
 		spi0 = &flexspi;
 	};
+
+	usdhc1_pwrseq: usdhc1_pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_usdhc1_gpio>;
+		reset-gpios = <&gpio2 10 GPIO_ACTIVE_LOW>;
+	};
 };
 
 &ddrc {
@@ -53,6 +60,19 @@
 	};
 };
 
+&usdhc1 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc1>;
+	pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc1_200mhz>;
+	bus-width = <4>;
+	keep-power-in-suspend;
+	mmc-pwrseq = <&usdhc1_pwrseq>;
+	fsl,sdio-async-interrupt-enabled;
+	non-removable;
+	status = "okay";
+};
+
 &usdhc3 {
 	assigned-clocks = <&clk IMX8MM_CLK_USDHC3_ROOT>;
 	assigned-clock-rates = <400000000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
index e033d0257b5a..ca623078d937 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
@@ -440,6 +440,45 @@
 		>;
 	};
 
+	pinctrl_usdhc1_gpio: usdhc1grpgpiogrp {
+		fsl,pins = <
+			MX8MM_IOMUXC_SD1_RESET_B_GPIO2_IO10	0x41
+		>;
+	};
+
+	pinctrl_usdhc1: usdhc1grp {
+		fsl,pins = <
+			MX8MM_IOMUXC_SD1_CLK_USDHC1_CLK		0x190
+			MX8MM_IOMUXC_SD1_CMD_USDHC1_CMD		0x1d0
+			MX8MM_IOMUXC_SD1_DATA0_USDHC1_DATA0	0x1d0
+			MX8MM_IOMUXC_SD1_DATA1_USDHC1_DATA1	0x1d0
+			MX8MM_IOMUXC_SD1_DATA2_USDHC1_DATA2	0x1d0
+			MX8MM_IOMUXC_SD1_DATA3_USDHC1_DATA3	0x1d0
+		>;
+	};
+
+	pinctrl_usdhc1_100mhz: usdhc1grp100mhz {
+		fsl,pins = <
+			MX8MM_IOMUXC_SD1_CLK_USDHC1_CLK		0x194
+			MX8MM_IOMUXC_SD1_CMD_USDHC1_CMD		0x1d4
+			MX8MM_IOMUXC_SD1_DATA0_USDHC1_DATA0	0x1d4
+			MX8MM_IOMUXC_SD1_DATA1_USDHC1_DATA1	0x1d4
+			MX8MM_IOMUXC_SD1_DATA2_USDHC1_DATA2	0x1d4
+			MX8MM_IOMUXC_SD1_DATA3_USDHC1_DATA3	0x1d4
+		>;
+	};
+
+	pinctrl_usdhc1_200mhz: usdhc1grp200mhz {
+		fsl,pins = <
+			MX8MM_IOMUXC_SD1_CLK_USDHC1_CLK		0x196
+			MX8MM_IOMUXC_SD1_CMD_USDHC1_CMD		0x1d6
+			MX8MM_IOMUXC_SD1_DATA0_USDHC1_DATA0	0x1d6
+			MX8MM_IOMUXC_SD1_DATA1_USDHC1_DATA1	0x1d6
+			MX8MM_IOMUXC_SD1_DATA2_USDHC1_DATA2	0x1d6
+			MX8MM_IOMUXC_SD1_DATA3_USDHC1_DATA3	0x1d6
+		>;
+	};
+
 	pinctrl_usdhc2_gpio: usdhc2grpgpiogrp {
 		fsl,pins = <
 			MX8MM_IOMUXC_GPIO1_IO15_GPIO1_IO15	0x1c4
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/6] arm64: dts: imx8mn-evk: add sdio wifi support

From: haibo.chen@nxp.com
Date: 2021-08-16 13:00:28

From: Haibo Chen <haibo.chen@nxp.com>

Add sdio wifi support on imx8mn-evk board.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi b/arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi
index 85e65f8719ea..00064f198423 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi
@@ -28,6 +28,13 @@
 		reg = <0x0 0x40000000 0 0x80000000>;
 	};
 
+	usdhc1_pwrseq: usdhc1_pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_usdhc1_gpio>;
+		reset-gpios = <&gpio2 10 GPIO_ACTIVE_LOW>;
+	};
+
 	reg_usdhc2_vmmc: regulator-usdhc2 {
 		compatible = "regulator-fixed";
 		pinctrl-names = "default";
@@ -205,6 +212,19 @@
 	};
 };
 
+&usdhc1 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc1>;
+	pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc1_200mhz>;
+	bus-width = <4>;
+	keep-power-in-suspend;
+	non-removable;
+	mmc-pwrseq = <&usdhc1_pwrseq>;
+	fsl,sdio-async-interrupt-enabled;
+	status = "okay";
+};
+
 &usdhc2 {
 	assigned-clocks = <&clk IMX8MN_CLK_USDHC2>;
 	assigned-clock-rates = <200000000>;
@@ -303,6 +323,45 @@
 		>;
 	};
 
+	pinctrl_usdhc1_gpio: usdhc1grpgpio {
+		fsl,pins = <
+			MX8MN_IOMUXC_SD1_RESET_B_GPIO2_IO10	0x41
+		>;
+	};
+
+	pinctrl_usdhc1: usdhc1grp {
+		fsl,pins = <
+			MX8MN_IOMUXC_SD1_CLK_USDHC1_CLK		0x190
+			MX8MN_IOMUXC_SD1_CMD_USDHC1_CMD		0x1d0
+			MX8MN_IOMUXC_SD1_DATA0_USDHC1_DATA0	0x1d0
+			MX8MN_IOMUXC_SD1_DATA1_USDHC1_DATA1	0x1d0
+			MX8MN_IOMUXC_SD1_DATA2_USDHC1_DATA2	0x1d0
+			MX8MN_IOMUXC_SD1_DATA3_USDHC1_DATA3	0x1d0
+		>;
+	};
+
+	pinctrl_usdhc1_100mhz: usdhc1grp100mhz {
+		fsl,pins = <
+			MX8MN_IOMUXC_SD1_CLK_USDHC1_CLK		0x194
+			MX8MN_IOMUXC_SD1_CMD_USDHC1_CMD		0x1d4
+			MX8MN_IOMUXC_SD1_DATA0_USDHC1_DATA0	0x1d4
+			MX8MN_IOMUXC_SD1_DATA1_USDHC1_DATA1	0x1d4
+			MX8MN_IOMUXC_SD1_DATA2_USDHC1_DATA2	0x1d4
+			MX8MN_IOMUXC_SD1_DATA3_USDHC1_DATA3	0x1d4
+		>;
+	};
+
+	pinctrl_usdhc1_200mhz: usdhc1grp200mhz {
+		fsl,pins = <
+			MX8MN_IOMUXC_SD1_CLK_USDHC1_CLK		0x196
+			MX8MN_IOMUXC_SD1_CMD_USDHC1_CMD		0x1d6
+			MX8MN_IOMUXC_SD1_DATA0_USDHC1_DATA0	0x1d6
+			MX8MN_IOMUXC_SD1_DATA1_USDHC1_DATA1	0x1d6
+			MX8MN_IOMUXC_SD1_DATA2_USDHC1_DATA2	0x1d6
+			MX8MN_IOMUXC_SD1_DATA3_USDHC1_DATA3	0x1d6
+		>;
+	};
+
 	pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
 		fsl,pins = <
 			MX8MN_IOMUXC_SD2_RESET_B_GPIO2_IO19	0x41
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] dt-bindings: mmc: fsl-imx-esdhc: add fsl,sdio-async-interrupt-enabled binding

From: Ulf Hansson <hidden>
Date: 2021-08-16 13:45:39

On Mon, 16 Aug 2021 at 15:00, [off-list ref] wrote:
quoted hunk
From: Haibo Chen <haibo.chen@nxp.com>

Add a new fsl,sdio-async-interrupt-enabled binding for sdio devices
which enable the async interrupt function. When get this property,
driver will avoid to use DAT[1] for hardware auto tuning check.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml         | 10 ++++++++++
 1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
index b5baf439fbac..8a9f1775b0e2 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
@@ -122,6 +122,16 @@ properties:
       - const: state_200mhz
       - const: sleep

+  fsl,sdio-async-interrupt-enabled:
+    description: |
+      Recommend for SDIO cards that enables SDIO async interrupt for SDR104 and SDR50
+      operating modes. SDIO async interrupt uses DAT[1] to signal the card's interrupt.
+      uSDHC tuning mechanism must use DAT[0] and CMD signals to avoid a possible
+      conflict and incorrect delay line calculated by the uSDHC auto tuning mechanism.
+      Enabling this device tree property is only recommended for layouts that are
+      matching the SD interface length.
+    type: boolean
We already have a common mmc property, "cap-sdio-irq", that tells
whether the controller supports SDIO irqs (which is delivered on
DAT1).

Can't you use this instead?
+
 required:
   - compatible
   - reg
--
2.17.1
Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/6] mmc: host: sdhci-esdhc-imx.c: correct the auto-tuning setting for sdio device

From: Ulf Hansson <hidden>
Date: 2021-08-16 13:53:53

On Mon, 16 Aug 2021 at 15:00, [off-list ref] wrote:
From: Haibo Chen <haibo.chen@nxp.com>

USDHC contain auto tuning circuit, this circuit will work automatically after
the tuning procedurae, it can increase/decrease the delay cell according to
the outside environment change (like temperature).

Unfortunately, this auto tuning circuit can not handle the async sdio device
interrupt correctly. When sdio device use 4 data line, async sdio interrupt
will use DAT[1], if we enable auto tuning circuit check 4 data lines, include
the DAT[1], this circuit will detect this interrupt, take this as a data on
DAT[1], and adjust the delay cell wrongly.

This is the hardware design limitation, to avoid this, when sdio device enable
async interrupt, auto tuning circuit only check DAT[0] and CMD lines.
SDIO irqs are being enabled/disabled dynamically in runtime by the mmc
core via the host ops ->enable_sdio_irq().

Rather than forcing the autotuning circuit to stay unused statically,
perhaps an option would be to disable it when the SDIO irqs becomes
enabled? Or maybe that becomes too complicated?
quoted hunk
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index f18d169bc8ff..ab84c29777e5 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -226,6 +226,7 @@ struct esdhc_platform_data {
        unsigned int tuning_step;       /* The delay cell steps in tuning procedure */
        unsigned int tuning_start_tap;  /* The start delay cell point in tuning procedure */
        unsigned int strobe_dll_delay_target;   /* The delay cell for strobe pad (read clock) */
+       bool sdio_async_interrupt_enabled;      /* sdio device enable the async interrupt */
 };

 struct esdhc_soc_data {
@@ -416,6 +417,8 @@ static inline void esdhc_wait_for_card_clock_gate_off(struct sdhci_host *host)
 /* Enable the auto tuning circuit to check the CMD line and BUS line */
 static inline void usdhc_auto_tuning_mode_sel(struct sdhci_host *host)
 {
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
        u32 buswidth, auto_tune_buswidth;

        buswidth = USDHC_GET_BUSWIDTH(readl(host->ioaddr + SDHCI_HOST_CONTROL));
@@ -432,6 +435,18 @@ static inline void usdhc_auto_tuning_mode_sel(struct sdhci_host *host)
                break;
        }

+       /*
+        * If sdio device use async interrupt, it will use DAT[1] to signal
+        * the device's interrupt asynchronous when use 4 data lines.
+        * Then hardware auto tuning circuit MUST NOT check the DAT[1] line,
+        * otherwise auto tuning will be impacted by this async interrupt,
+        * and change the delay cell incorrectly, which then cause data/cmd
+        * errors.
+        * This is the hardware auto tuning circuit limitation.
+        */
+       if (imx_data->boarddata.sdio_async_interrupt_enabled)
+               auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
+
        esdhc_clrset_le(host, ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK,
                        auto_tune_buswidth | ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN,
                        ESDHC_VEND_SPEC2);
@@ -1531,6 +1546,9 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
        if (of_property_read_u32(np, "fsl,delay-line", &boarddata->delay_line))
                boarddata->delay_line = 0;

+       if (of_property_read_bool(np, "fsl,sdio-async-interrupt-enabled"))
As stated on the DT patch, I think you can use the "cap-sdio-irq" instead.
+               boarddata->sdio_async_interrupt_enabled = true;
+
        mmc_of_parse_voltage(host->mmc, &host->ocr_mask);

        if (esdhc_is_usdhc(imx_data) && !IS_ERR(imx_data->pinctrl)) {
--
2.17.1
Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 3/6] dt-bindings: mmc: fsl-imx-esdhc: add fsl,sdio-async-interrupt-enabled binding

From: Bough Chen <haibo.chen@nxp.com>
Date: 2021-08-17 06:41:43

-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
Sent: 2021年8月16日 21:43
To: Bough Chen <haibo.chen@nxp.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>; Shawn Guo
[off-list ref]; Rob Herring [off-list ref]; Sascha Hauer
[off-list ref]; Sascha Hauer [off-list ref]; Fabio
Estevam [off-list ref]; linux-mmc [off-list ref];
dl-linux-imx [off-list ref]; DTML [off-list ref]; Linux
ARM [off-list ref]
Subject: Re: [PATCH 3/6] dt-bindings: mmc: fsl-imx-esdhc: add
fsl,sdio-async-interrupt-enabled binding

On Mon, 16 Aug 2021 at 15:00, [off-list ref] wrote:
quoted
From: Haibo Chen <haibo.chen@nxp.com>

Add a new fsl,sdio-async-interrupt-enabled binding for sdio devices
which enable the async interrupt function. When get this property,
driver will avoid to use DAT[1] for hardware auto tuning check.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml         | 10
++++++++++
quoted
 1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
index b5baf439fbac..8a9f1775b0e2 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
@@ -122,6 +122,16 @@ properties:
       - const: state_200mhz
       - const: sleep

+  fsl,sdio-async-interrupt-enabled:
+    description: |
+      Recommend for SDIO cards that enables SDIO async interrupt for
SDR104 and SDR50
quoted
+      operating modes. SDIO async interrupt uses DAT[1] to signal the
card's interrupt.
quoted
+      uSDHC tuning mechanism must use DAT[0] and CMD signals to avoid
a possible
quoted
+      conflict and incorrect delay line calculated by the uSDHC auto tuning
mechanism.
quoted
+      Enabling this device tree property is only recommended for layouts
that are
quoted
+      matching the SD interface length.
+    type: boolean
We already have a common mmc property, "cap-sdio-irq", that tells whether
the controller supports SDIO irqs (which is delivered on DAT1).

Can't you use this instead?
Hi Ulf,

Thanks for your quick reply!

According to our WiFi team reply, the sdio-irq has two types. Sync interrupt and Async interrupt.
When WiFi send out the interrupt signal during the interrupt period, if it sync with clock pad(just as
when send out data), then this is sync interrupt. When this interrupt not sync with clock, it is async
interrupt. Async interrupt has a better overall performance than sync interrupt.

Logically, auto tuning circuit should only take care of the data and cmd line, and ignore interrupt signal.
But unfortunately current i.mx-usdhc IP do not ignore interrupt signal. So it detect the interrupt signal,
and take this signal as a data signal, and adjust the delay cell accordingly. For sync interrupt, due to it
sync with clock, so no affect, but for async interrupt, it will involve wrong delay cell change randomly.

I involve a new property here, because in sdhci.c, we default use this "cap-sdio-irq" for all sdio/sd/mmc.
I need one property which can use only for sdio device, and only when sdio device enable async-interrupt.

Best Regards
Haibo Chen
quoted
+
 required:
   - compatible
   - reg
--
2.17.1
Kind regards
Uffe
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 4/6] mmc: host: sdhci-esdhc-imx.c: correct the auto-tuning setting for sdio device

From: Bough Chen <haibo.chen@nxp.com>
Date: 2021-08-17 06:58:01

-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
Sent: 2021年8月16日 21:53
To: Bough Chen <haibo.chen@nxp.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>; Shawn Guo
[off-list ref]; Rob Herring [off-list ref]; Sascha Hauer
[off-list ref]; Sascha Hauer [off-list ref]; Fabio
Estevam [off-list ref]; linux-mmc [off-list ref];
dl-linux-imx [off-list ref]; DTML [off-list ref]; Linux
ARM [off-list ref]
Subject: Re: [PATCH 4/6] mmc: host: sdhci-esdhc-imx.c: correct the auto-tuning
setting for sdio device

On Mon, 16 Aug 2021 at 15:00, [off-list ref] wrote:
quoted
From: Haibo Chen <haibo.chen@nxp.com>

USDHC contain auto tuning circuit, this circuit will work
automatically after the tuning procedurae, it can increase/decrease
the delay cell according to the outside environment change (like
temperature).
quoted
Unfortunately, this auto tuning circuit can not handle the async sdio
device interrupt correctly. When sdio device use 4 data line, async
sdio interrupt will use DAT[1], if we enable auto tuning circuit check
4 data lines, include the DAT[1], this circuit will detect this
interrupt, take this as a data on DAT[1], and adjust the delay cell wrongly.

This is the hardware design limitation, to avoid this, when sdio
device enable async interrupt, auto tuning circuit only check DAT[0] and CMD
lines.

SDIO irqs are being enabled/disabled dynamically in runtime by the mmc core
via the host ops ->enable_sdio_irq().

Rather than forcing the autotuning circuit to stay unused statically, perhaps an
option would be to disable it when the SDIO irqs becomes enabled? Or maybe
that becomes too complicated?
For interrupt in 4-bit mode, there is a definition of interrupt period, only in this period can the interrupt be
detect and recognize. The interrupt period can exist during data transfer.

So to fix this issue, one method is to disable auto tuning circuit. Another is my current method, just detect CMD
and DAT0, but at least auto tuning still can work (this method need board design keep align all data lines).
 

Best Regards
Haibo chen
quoted
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index f18d169bc8ff..ab84c29777e5 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -226,6 +226,7 @@ struct esdhc_platform_data {
        unsigned int tuning_step;       /* The delay cell steps in tuning
procedure */
quoted
        unsigned int tuning_start_tap;  /* The start delay cell point in
tuning procedure */
quoted
        unsigned int strobe_dll_delay_target;   /* The delay cell for
strobe pad (read clock) */
quoted
+       bool sdio_async_interrupt_enabled;      /* sdio device enable
the async interrupt */
quoted
 };

 struct esdhc_soc_data {
@@ -416,6 +417,8 @@ static inline void
esdhc_wait_for_card_clock_gate_off(struct sdhci_host *host)
 /* Enable the auto tuning circuit to check the CMD line and BUS line
*/  static inline void usdhc_auto_tuning_mode_sel(struct sdhci_host
*host)  {
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct pltfm_imx_data *imx_data =
+ sdhci_pltfm_priv(pltfm_host);
        u32 buswidth, auto_tune_buswidth;

        buswidth = USDHC_GET_BUSWIDTH(readl(host->ioaddr +
SDHCI_HOST_CONTROL)); @@ -432,6 +435,18 @@ static inline void
usdhc_auto_tuning_mode_sel(struct sdhci_host *host)
quoted
                break;
        }

+       /*
+        * If sdio device use async interrupt, it will use DAT[1] to signal
+        * the device's interrupt asynchronous when use 4 data lines.
+        * Then hardware auto tuning circuit MUST NOT check the DAT[1]
line,
quoted
+        * otherwise auto tuning will be impacted by this async interrupt,
+        * and change the delay cell incorrectly, which then cause
data/cmd
quoted
+        * errors.
+        * This is the hardware auto tuning circuit limitation.
+        */
+       if (imx_data->boarddata.sdio_async_interrupt_enabled)
+               auto_tune_buswidth =
+ ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
+
        esdhc_clrset_le(host,
ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK,
quoted
                        auto_tune_buswidth |
ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN,
quoted
                        ESDHC_VEND_SPEC2); @@ -1531,6 +1546,9
@@
quoted
sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
        if (of_property_read_u32(np, "fsl,delay-line",
&boarddata->delay_line))
quoted
                boarddata->delay_line = 0;

+       if (of_property_read_bool(np,
+ "fsl,sdio-async-interrupt-enabled"))
As stated on the DT patch, I think you can use the "cap-sdio-irq" instead.
quoted
+               boarddata->sdio_async_interrupt_enabled = true;
+
        mmc_of_parse_voltage(host->mmc, &host->ocr_mask);

        if (esdhc_is_usdhc(imx_data) && !IS_ERR(imx_data->pinctrl)) {
--
2.17.1
Kind regards
Uffe
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] dt-bindings: mmc: fsl-imx-esdhc: add fsl,sdio-async-interrupt-enabled binding

From: Ulf Hansson <hidden>
Date: 2021-08-17 07:37:40

On Tue, 17 Aug 2021 at 08:41, Bough Chen [off-list ref] wrote:
quoted
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
Sent: 2021年8月16日 21:43
To: Bough Chen <haibo.chen@nxp.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>; Shawn Guo
[off-list ref]; Rob Herring [off-list ref]; Sascha Hauer
[off-list ref]; Sascha Hauer [off-list ref]; Fabio
Estevam [off-list ref]; linux-mmc [off-list ref];
dl-linux-imx [off-list ref]; DTML [off-list ref]; Linux
ARM [off-list ref]
Subject: Re: [PATCH 3/6] dt-bindings: mmc: fsl-imx-esdhc: add
fsl,sdio-async-interrupt-enabled binding

On Mon, 16 Aug 2021 at 15:00, [off-list ref] wrote:
quoted
From: Haibo Chen <haibo.chen@nxp.com>

Add a new fsl,sdio-async-interrupt-enabled binding for sdio devices
which enable the async interrupt function. When get this property,
driver will avoid to use DAT[1] for hardware auto tuning check.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml         | 10
++++++++++
quoted
 1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
index b5baf439fbac..8a9f1775b0e2 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
@@ -122,6 +122,16 @@ properties:
       - const: state_200mhz
       - const: sleep

+  fsl,sdio-async-interrupt-enabled:
+    description: |
+      Recommend for SDIO cards that enables SDIO async interrupt for
SDR104 and SDR50
quoted
+      operating modes. SDIO async interrupt uses DAT[1] to signal the
card's interrupt.
quoted
+      uSDHC tuning mechanism must use DAT[0] and CMD signals to avoid
a possible
quoted
+      conflict and incorrect delay line calculated by the uSDHC auto tuning
mechanism.
quoted
+      Enabling this device tree property is only recommended for layouts
that are
quoted
+      matching the SD interface length.
+    type: boolean
We already have a common mmc property, "cap-sdio-irq", that tells whether
the controller supports SDIO irqs (which is delivered on DAT1).

Can't you use this instead?
Hi Ulf,

Thanks for your quick reply!

According to our WiFi team reply, the sdio-irq has two types. Sync interrupt and Async interrupt.
When WiFi send out the interrupt signal during the interrupt period, if it sync with clock pad(just as
when send out data), then this is sync interrupt. When this interrupt not sync with clock, it is async
interrupt. Async interrupt has a better overall performance than sync interrupt.
The async interrupt is what we refer to as SDIO irqs, which is being
delivered on DAT1.
Logically, auto tuning circuit should only take care of the data and cmd line, and ignore interrupt signal.
But unfortunately current i.mx-usdhc IP do not ignore interrupt signal. So it detect the interrupt signal,
and take this signal as a data signal, and adjust the delay cell accordingly. For sync interrupt, due to it
sync with clock, so no affect, but for async interrupt, it will involve wrong delay cell change randomly.
Okay.
I involve a new property here, because in sdhci.c, we default use this "cap-sdio-irq" for all sdio/sd/mmc.
I guess it's the similar variant of the controller for all slots then.

It can be debated whether the proper thing is to set "cap-sdio-irq"
only for the SDIO card slot. I think so, (and it's already being used
like that) if there is an embedded SDIO card attached, because
cap-sdio-irq would not make sense otherwise.
I need one property which can use only for sdio device, and only when sdio device enable async-interrupt.
If you really need a new DT property (let's discuss that more in patch
4/6), I suggest you add something along the lines of a
"broken-auto-tuning" DT property instead, because that is in principle
what this should be about, isn't it?

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/6] mmc: host: sdhci-esdhc-imx.c: correct the auto-tuning setting for sdio device

From: Ulf Hansson <hidden>
Date: 2021-08-17 08:01:58

On Tue, 17 Aug 2021 at 08:57, Bough Chen [off-list ref] wrote:
quoted
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
Sent: 2021年8月16日 21:53
To: Bough Chen <haibo.chen@nxp.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>; Shawn Guo
[off-list ref]; Rob Herring [off-list ref]; Sascha Hauer
[off-list ref]; Sascha Hauer [off-list ref]; Fabio
Estevam [off-list ref]; linux-mmc [off-list ref];
dl-linux-imx [off-list ref]; DTML [off-list ref]; Linux
ARM [off-list ref]
Subject: Re: [PATCH 4/6] mmc: host: sdhci-esdhc-imx.c: correct the auto-tuning
setting for sdio device

On Mon, 16 Aug 2021 at 15:00, [off-list ref] wrote:
quoted
From: Haibo Chen <haibo.chen@nxp.com>

USDHC contain auto tuning circuit, this circuit will work
automatically after the tuning procedurae, it can increase/decrease
the delay cell according to the outside environment change (like
temperature).
quoted
Unfortunately, this auto tuning circuit can not handle the async sdio
device interrupt correctly. When sdio device use 4 data line, async
sdio interrupt will use DAT[1], if we enable auto tuning circuit check
4 data lines, include the DAT[1], this circuit will detect this
interrupt, take this as a data on DAT[1], and adjust the delay cell wrongly.

This is the hardware design limitation, to avoid this, when sdio
device enable async interrupt, auto tuning circuit only check DAT[0] and CMD
lines.

SDIO irqs are being enabled/disabled dynamically in runtime by the mmc core
via the host ops ->enable_sdio_irq().

Rather than forcing the autotuning circuit to stay unused statically, perhaps an
option would be to disable it when the SDIO irqs becomes enabled? Or maybe
that becomes too complicated?
For interrupt in 4-bit mode, there is a definition of interrupt period, only in this period can the interrupt be
detect and recognize. The interrupt period can exist during data transfer.

So to fix this issue, one method is to disable auto tuning circuit. Another is my current method, just detect CMD
and DAT0, but at least auto tuning still can work (this method need board design keep align all data lines).
To allow DAT1 being used for SDIO irqs (async or not), the irqs needs
to be enabled internally in the SDIO card first. This is done by
writing to the CCCR register, which happens in sdio_claim_irq(). At
this point in sdio_claim_irq() the core also invokes the
->enable_sdio_irq() host ops, to allow the host to prepare itself to
accept SDIO irqs.

It sounds to me that you should be able to use the ->enable_sdio_irq
ops, as a way of understanding that the auto-tuning feature also needs
to be turned off, because it's not compatible with SDIO irqs.

Kind regards
Uffe

Best Regards
Haibo chen
quoted
quoted
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index f18d169bc8ff..ab84c29777e5 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -226,6 +226,7 @@ struct esdhc_platform_data {
        unsigned int tuning_step;       /* The delay cell steps in tuning
procedure */
quoted
        unsigned int tuning_start_tap;  /* The start delay cell point in
tuning procedure */
quoted
        unsigned int strobe_dll_delay_target;   /* The delay cell for
strobe pad (read clock) */
quoted
+       bool sdio_async_interrupt_enabled;      /* sdio device enable
the async interrupt */
quoted
 };

 struct esdhc_soc_data {
@@ -416,6 +417,8 @@ static inline void
esdhc_wait_for_card_clock_gate_off(struct sdhci_host *host)
 /* Enable the auto tuning circuit to check the CMD line and BUS line
*/  static inline void usdhc_auto_tuning_mode_sel(struct sdhci_host
*host)  {
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct pltfm_imx_data *imx_data =
+ sdhci_pltfm_priv(pltfm_host);
        u32 buswidth, auto_tune_buswidth;

        buswidth = USDHC_GET_BUSWIDTH(readl(host->ioaddr +
SDHCI_HOST_CONTROL)); @@ -432,6 +435,18 @@ static inline void
usdhc_auto_tuning_mode_sel(struct sdhci_host *host)
quoted
                break;
        }

+       /*
+        * If sdio device use async interrupt, it will use DAT[1] to signal
+        * the device's interrupt asynchronous when use 4 data lines.
+        * Then hardware auto tuning circuit MUST NOT check the DAT[1]
line,
quoted
+        * otherwise auto tuning will be impacted by this async interrupt,
+        * and change the delay cell incorrectly, which then cause
data/cmd
quoted
+        * errors.
+        * This is the hardware auto tuning circuit limitation.
+        */
+       if (imx_data->boarddata.sdio_async_interrupt_enabled)
+               auto_tune_buswidth =
+ ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
+
        esdhc_clrset_le(host,
ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK,
quoted
                        auto_tune_buswidth |
ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN,
quoted
                        ESDHC_VEND_SPEC2); @@ -1531,6 +1546,9
@@
quoted
sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
        if (of_property_read_u32(np, "fsl,delay-line",
&boarddata->delay_line))
quoted
                boarddata->delay_line = 0;

+       if (of_property_read_bool(np,
+ "fsl,sdio-async-interrupt-enabled"))
As stated on the DT patch, I think you can use the "cap-sdio-irq" instead.
quoted
+               boarddata->sdio_async_interrupt_enabled = true;
+
        mmc_of_parse_voltage(host->mmc, &host->ocr_mask);

        if (esdhc_is_usdhc(imx_data) && !IS_ERR(imx_data->pinctrl)) {
--
2.17.1
Kind regards
Uffe
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 4/6] mmc: host: sdhci-esdhc-imx.c: correct the auto-tuning setting for sdio device

From: Bough Chen <haibo.chen@nxp.com>
Date: 2021-08-17 12:29:28

-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
Sent: 2021年8月17日 16:01
To: Bough Chen <haibo.chen@nxp.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>; Shawn Guo
[off-list ref]; Rob Herring [off-list ref]; Sascha Hauer
[off-list ref]; Sascha Hauer [off-list ref]; Fabio
Estevam [off-list ref]; linux-mmc [off-list ref];
dl-linux-imx [off-list ref]; DTML [off-list ref]; Linux
ARM [off-list ref]
Subject: Re: [PATCH 4/6] mmc: host: sdhci-esdhc-imx.c: correct the auto-tuning
setting for sdio device

On Tue, 17 Aug 2021 at 08:57, Bough Chen [off-list ref] wrote:
quoted
quoted
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
Sent: 2021年8月16日 21:53
To: Bough Chen <haibo.chen@nxp.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>; Shawn Guo
[off-list ref]; Rob Herring [off-list ref]; Sascha
Hauer [off-list ref]; Sascha Hauer
[off-list ref]; Fabio Estevam [off-list ref];
linux-mmc [off-list ref]; dl-linux-imx
[off-list ref]; DTML [off-list ref]; Linux ARM
[off-list ref]
Subject: Re: [PATCH 4/6] mmc: host: sdhci-esdhc-imx.c: correct the
auto-tuning setting for sdio device

On Mon, 16 Aug 2021 at 15:00, [off-list ref] wrote:
quoted
From: Haibo Chen <haibo.chen@nxp.com>

USDHC contain auto tuning circuit, this circuit will work
automatically after the tuning procedurae, it can
increase/decrease the delay cell according to the outside
environment change (like
temperature).
quoted
Unfortunately, this auto tuning circuit can not handle the async
sdio device interrupt correctly. When sdio device use 4 data line,
async sdio interrupt will use DAT[1], if we enable auto tuning
circuit check
4 data lines, include the DAT[1], this circuit will detect this
interrupt, take this as a data on DAT[1], and adjust the delay cell wrongly.

This is the hardware design limitation, to avoid this, when sdio
device enable async interrupt, auto tuning circuit only check
DAT[0] and CMD
lines.

SDIO irqs are being enabled/disabled dynamically in runtime by the
mmc core via the host ops ->enable_sdio_irq().

Rather than forcing the autotuning circuit to stay unused
statically, perhaps an option would be to disable it when the SDIO
irqs becomes enabled? Or maybe that becomes too complicated?
For interrupt in 4-bit mode, there is a definition of interrupt
period, only in this period can the interrupt be detect and recognize. The
interrupt period can exist during data transfer.
quoted
So to fix this issue, one method is to disable auto tuning circuit.
Another is my current method, just detect CMD and DAT0, but at least auto
tuning still can work (this method need board design keep align all data lines).

To allow DAT1 being used for SDIO irqs (async or not), the irqs needs to be
enabled internally in the SDIO card first. This is done by writing to the CCCR
register, which happens in sdio_claim_irq(). At this point in sdio_claim_irq() the
core also invokes the
->enable_sdio_irq() host ops, to allow the host to prepare itself to
accept SDIO irqs.

It sounds to me that you should be able to use the ->enable_sdio_irq ops, as a
way of understanding that the auto-tuning feature also needs to be turned off,
because it's not compatible with SDIO irqs.
Yes, for current auto-tuning design, it need to be turned off for sdio interrupt.

I'm just a bit greedy, want to find a method to support auto-tuning for sdio interrupt. 😊
I will use a new property like "fsl,broken-auto-tuning" for this WiFI device in V2 patch.

Best Regards
Haibo Chen.
Kind regards
Uffe
quoted

Best Regards
Haibo chen
quoted
quoted
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index f18d169bc8ff..ab84c29777e5 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -226,6 +226,7 @@ struct esdhc_platform_data {
        unsigned int tuning_step;       /* The delay cell steps in
tuning
quoted
quoted
procedure */
quoted
        unsigned int tuning_start_tap;  /* The start delay cell
point in
tuning procedure */
quoted
        unsigned int strobe_dll_delay_target;   /* The delay cell for
strobe pad (read clock) */
quoted
+       bool sdio_async_interrupt_enabled;      /* sdio device enable
the async interrupt */
quoted
 };

 struct esdhc_soc_data {
@@ -416,6 +417,8 @@ static inline void
esdhc_wait_for_card_clock_gate_off(struct sdhci_host *host)
 /* Enable the auto tuning circuit to check the CMD line and BUS
line */  static inline void usdhc_auto_tuning_mode_sel(struct
sdhci_host
*host)  {
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct pltfm_imx_data *imx_data =
+ sdhci_pltfm_priv(pltfm_host);
        u32 buswidth, auto_tune_buswidth;

        buswidth = USDHC_GET_BUSWIDTH(readl(host->ioaddr +
SDHCI_HOST_CONTROL)); @@ -432,6 +435,18 @@ static inline void
usdhc_auto_tuning_mode_sel(struct sdhci_host *host)
quoted
                break;
        }

+       /*
+        * If sdio device use async interrupt, it will use DAT[1] to signal
+        * the device's interrupt asynchronous when use 4 data lines.
+        * Then hardware auto tuning circuit MUST NOT check the
+ DAT[1]
line,
quoted
+        * otherwise auto tuning will be impacted by this async
interrupt,
quoted
quoted
quoted
+        * and change the delay cell incorrectly, which then cause
data/cmd
quoted
+        * errors.
+        * This is the hardware auto tuning circuit limitation.
+        */
+       if (imx_data->boarddata.sdio_async_interrupt_enabled)
+               auto_tune_buswidth =
+ ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
+
        esdhc_clrset_le(host,
ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK,
quoted
                        auto_tune_buswidth |
ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN,
quoted
                        ESDHC_VEND_SPEC2); @@ -1531,6
+1546,9
quoted
quoted
@@
quoted
sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
        if (of_property_read_u32(np, "fsl,delay-line",
&boarddata->delay_line))
quoted
                boarddata->delay_line = 0;

+       if (of_property_read_bool(np,
+ "fsl,sdio-async-interrupt-enabled"))
As stated on the DT patch, I think you can use the "cap-sdio-irq" instead.
quoted
+               boarddata->sdio_async_interrupt_enabled = true;
+
        mmc_of_parse_voltage(host->mmc, &host->ocr_mask);

        if (esdhc_is_usdhc(imx_data) &&
!IS_ERR(imx_data->pinctrl)) {
--
2.17.1
Kind regards
Uffe
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] mmc: sdhci-esdhc-imx: remove redundant code for manual tuning

From: Ulf Hansson <hidden>
Date: 2021-08-24 13:53:13

On Mon, 16 Aug 2021 at 15:00, [off-list ref] wrote:
From: Haibo Chen <haibo.chen@nxp.com>

For manual tuning method, already call esdhc_prepare_tuning()
config the necessary registers, so remove the redundant code
in esdhc_writew_le() for SDHCI_HOST_CONTROL2.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
While discussions on the DT binding, etc, continue with Lucas and Rob
on patch 3 - do you want me to apply patch1 and patch2?

Kind regards
Uffe
quoted hunk
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 57b19ca1ad6d..a49fac719fca 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -628,17 +628,7 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
                else
                        new_val &= ~ESDHC_VENDOR_SPEC_VSELECT;
                writel(new_val, host->ioaddr + ESDHC_VENDOR_SPEC);
-               if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) {
-                       new_val = readl(host->ioaddr + ESDHC_MIX_CTRL);
-                       if (val & SDHCI_CTRL_TUNED_CLK) {
-                               new_val |= ESDHC_MIX_CTRL_SMPCLK_SEL;
-                               new_val |= ESDHC_MIX_CTRL_AUTO_TUNE_EN;
-                       } else {
-                               new_val &= ~ESDHC_MIX_CTRL_SMPCLK_SEL;
-                               new_val &= ~ESDHC_MIX_CTRL_AUTO_TUNE_EN;
-                       }
-                       writel(new_val , host->ioaddr + ESDHC_MIX_CTRL);
-               } else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) {
+               if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) {
                        u32 v = readl(host->ioaddr + SDHCI_AUTO_CMD_STATUS);
                        u32 m = readl(host->ioaddr + ESDHC_MIX_CTRL);
                        if (val & SDHCI_CTRL_TUNED_CLK) {
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 1/6] mmc: sdhci-esdhc-imx: remove redundant code for manual tuning

From: Bough Chen <haibo.chen@nxp.com>
Date: 2021-08-25 02:16:36

-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
Sent: 2021年8月24日 21:53
To: Bough Chen <haibo.chen@nxp.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>; Shawn Guo
[off-list ref]; Rob Herring [off-list ref]; Sascha Hauer
[off-list ref]; Sascha Hauer [off-list ref]; Fabio
Estevam [off-list ref]; linux-mmc [off-list ref];
dl-linux-imx [off-list ref]; DTML [off-list ref]; Linux
ARM [off-list ref]
Subject: Re: [PATCH 1/6] mmc: sdhci-esdhc-imx: remove redundant code for
manual tuning

On Mon, 16 Aug 2021 at 15:00, [off-list ref] wrote:
quoted
From: Haibo Chen <haibo.chen@nxp.com>

For manual tuning method, already call esdhc_prepare_tuning() config
the necessary registers, so remove the redundant code in
esdhc_writew_le() for SDHCI_HOST_CONTROL2.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
While discussions on the DT binding, etc, continue with Lucas and Rob on patch
3 - do you want me to apply patch1 and patch2?
Yes, thanks!
Let's wait comments for patch 3.

Best Regards
Haibo Chen
Kind regards
Uffe
quoted
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 57b19ca1ad6d..a49fac719fca 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -628,17 +628,7 @@ static void esdhc_writew_le(struct sdhci_host
*host, u16 val, int reg)
quoted
                else
                        new_val &=
~ESDHC_VENDOR_SPEC_VSELECT;
quoted
                writel(new_val, host->ioaddr + ESDHC_VENDOR_SPEC);
-               if (imx_data->socdata->flags &
ESDHC_FLAG_MAN_TUNING) {
quoted
-                       new_val = readl(host->ioaddr +
ESDHC_MIX_CTRL);
quoted
-                       if (val & SDHCI_CTRL_TUNED_CLK) {
-                               new_val |=
ESDHC_MIX_CTRL_SMPCLK_SEL;
quoted
-                               new_val |=
ESDHC_MIX_CTRL_AUTO_TUNE_EN;
quoted
-                       } else {
-                               new_val &=
~ESDHC_MIX_CTRL_SMPCLK_SEL;
quoted
-                               new_val &=
~ESDHC_MIX_CTRL_AUTO_TUNE_EN;
quoted
-                       }
-                       writel(new_val , host->ioaddr +
ESDHC_MIX_CTRL);
quoted
-               } else if (imx_data->socdata->flags &
ESDHC_FLAG_STD_TUNING) {
quoted
+               if (imx_data->socdata->flags &
ESDHC_FLAG_STD_TUNING)
quoted
+ {
                        u32 v = readl(host->ioaddr +
SDHCI_AUTO_CMD_STATUS);
quoted
                        u32 m = readl(host->ioaddr +
ESDHC_MIX_CTRL);
quoted
                        if (val & SDHCI_CTRL_TUNED_CLK) {
--
2.17.1

Re: [PATCH 1/6] mmc: sdhci-esdhc-imx: remove redundant code for manual tuning

From: Ulf Hansson <hidden>
Date: 2021-08-25 09:23:13

On Wed, 25 Aug 2021 at 04:16, Bough Chen [off-list ref] wrote:
quoted
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
Sent: 2021年8月24日 21:53
To: Bough Chen <haibo.chen@nxp.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>; Shawn Guo
[off-list ref]; Rob Herring [off-list ref]; Sascha Hauer
[off-list ref]; Sascha Hauer [off-list ref]; Fabio
Estevam [off-list ref]; linux-mmc [off-list ref];
dl-linux-imx [off-list ref]; DTML [off-list ref]; Linux
ARM [off-list ref]
Subject: Re: [PATCH 1/6] mmc: sdhci-esdhc-imx: remove redundant code for
manual tuning

On Mon, 16 Aug 2021 at 15:00, [off-list ref] wrote:
quoted
From: Haibo Chen <haibo.chen@nxp.com>

For manual tuning method, already call esdhc_prepare_tuning() config
the necessary registers, so remove the redundant code in
esdhc_writew_le() for SDHCI_HOST_CONTROL2.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
While discussions on the DT binding, etc, continue with Lucas and Rob on patch
3 - do you want me to apply patch1 and patch2?
Yes, thanks!
Let's wait comments for patch 3.
Alright, patch1 and pacth2 applied for next, thanks!

[...]

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help