-----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