> -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx] > Sent: 2021年8月17日 16:01 > To: Bough Chen <haibo.chen@xxxxxxx> > Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>; Shawn Guo > <shawnguo@xxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Sascha Hauer > <s.hauer@xxxxxxxxxxxxxx>; Sascha Hauer <kernel@xxxxxxxxxxxxxx>; Fabio > Estevam <festevam@xxxxxxxxx>; linux-mmc <linux-mmc@xxxxxxxxxxxxxxx>; > dl-linux-imx <linux-imx@xxxxxxx>; DTML <devicetree@xxxxxxxxxxxxxxx>; Linux > ARM <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx> > 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 <haibo.chen@xxxxxxx> wrote: > > > > > -----Original Message----- > > > From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx] > > > Sent: 2021年8月16日 21:53 > > > To: Bough Chen <haibo.chen@xxxxxxx> > > > Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>; Shawn Guo > > > <shawnguo@xxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Sascha > > > Hauer <s.hauer@xxxxxxxxxxxxxx>; Sascha Hauer > > > <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; > > > linux-mmc <linux-mmc@xxxxxxxxxxxxxxx>; dl-linux-imx > > > <linux-imx@xxxxxxx>; DTML <devicetree@xxxxxxxxxxxxxxx>; Linux ARM > > > <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx> > > > 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, <haibo.chen@xxxxxxx> wrote: > > > > > > > > From: Haibo Chen <haibo.chen@xxxxxxx> > > > > > > > > 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? > > > > 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. > 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 > > > > > > > Best Regards > > Haibo chen > > > > > > > > > > > > > Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx> > > > > --- > > > > 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
