| Message ID | 1629117508-4886-1-git-send-email-haibo.chen@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/6] mmc: sdhci-esdhc-imx: remove redundant code for manual tuning | expand |
On Mon, 16 Aug 2021 at 15:00, <haibo.chen@nxp.com> wrote: > > 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
> -----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 > <shawnguo@kernel.org>; Rob Herring <robh+dt@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Sascha Hauer <kernel@pengutronix.de>; Fabio > Estevam <festevam@gmail.com>; linux-mmc <linux-mmc@vger.kernel.org>; > dl-linux-imx <linux-imx@nxp.com>; DTML <devicetree@vger.kernel.org>; Linux > ARM <linux-arm-kernel@lists.infradead.org> > 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, <haibo.chen@nxp.com> wrote: > > > > 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? > 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 > > + > > required: > > - compatible > > - reg > > -- > > 2.17.1 > > > > Kind regards > Uffe
> -----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 > <shawnguo@kernel.org>; Rob Herring <robh+dt@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Sascha Hauer <kernel@pengutronix.de>; Fabio > Estevam <festevam@gmail.com>; linux-mmc <linux-mmc@vger.kernel.org>; > dl-linux-imx <linux-imx@nxp.com>; DTML <devicetree@vger.kernel.org>; Linux > ARM <linux-arm-kernel@lists.infradead.org> > 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@nxp.com> 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? 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 > > > > > 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
On Tue, 17 Aug 2021 at 08:57, Bough Chen <haibo.chen@nxp.com> wrote: > > > -----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 > > <shawnguo@kernel.org>; Rob Herring <robh+dt@kernel.org>; Sascha Hauer > > <s.hauer@pengutronix.de>; Sascha Hauer <kernel@pengutronix.de>; Fabio > > Estevam <festevam@gmail.com>; linux-mmc <linux-mmc@vger.kernel.org>; > > dl-linux-imx <linux-imx@nxp.com>; DTML <devicetree@vger.kernel.org>; Linux > > ARM <linux-arm-kernel@lists.infradead.org> > > 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@nxp.com> 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? > > 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 > > > > > > > > > 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
> -----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 > <shawnguo@kernel.org>; Rob Herring <robh+dt@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Sascha Hauer <kernel@pengutronix.de>; Fabio > Estevam <festevam@gmail.com>; linux-mmc <linux-mmc@vger.kernel.org>; > dl-linux-imx <linux-imx@nxp.com>; DTML <devicetree@vger.kernel.org>; Linux > ARM <linux-arm-kernel@lists.infradead.org> > 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@nxp.com> wrote: > > > > > -----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 > > > <shawnguo@kernel.org>; Rob Herring <robh+dt@kernel.org>; Sascha > > > Hauer <s.hauer@pengutronix.de>; Sascha Hauer > > > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; > > > linux-mmc <linux-mmc@vger.kernel.org>; dl-linux-imx > > > <linux-imx@nxp.com>; DTML <devicetree@vger.kernel.org>; Linux ARM > > > <linux-arm-kernel@lists.infradead.org> > > > 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@nxp.com> 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? > > > > 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@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
On Mon, 16 Aug 2021 at 15:00, <haibo.chen@nxp.com> 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 > --- > 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 >
> -----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 > <shawnguo@kernel.org>; Rob Herring <robh+dt@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Sascha Hauer <kernel@pengutronix.de>; Fabio > Estevam <festevam@gmail.com>; linux-mmc <linux-mmc@vger.kernel.org>; > dl-linux-imx <linux-imx@nxp.com>; DTML <devicetree@vger.kernel.org>; Linux > ARM <linux-arm-kernel@lists.infradead.org> > Subject: Re: [PATCH 1/6] mmc: sdhci-esdhc-imx: remove redundant code for > manual tuning > > On Mon, 16 Aug 2021 at 15:00, <haibo.chen@nxp.com> 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? Yes, thanks! Let's wait comments for patch 3. Best Regards Haibo Chen > > Kind regards > Uffe > > > --- > > 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 > >
On Wed, 25 Aug 2021 at 04:16, Bough Chen <haibo.chen@nxp.com> wrote: > > > -----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 > > <shawnguo@kernel.org>; Rob Herring <robh+dt@kernel.org>; Sascha Hauer > > <s.hauer@pengutronix.de>; Sascha Hauer <kernel@pengutronix.de>; Fabio > > Estevam <festevam@gmail.com>; linux-mmc <linux-mmc@vger.kernel.org>; > > dl-linux-imx <linux-imx@nxp.com>; DTML <devicetree@vger.kernel.org>; Linux > > ARM <linux-arm-kernel@lists.infradead.org> > > Subject: Re: [PATCH 1/6] mmc: sdhci-esdhc-imx: remove redundant code for > > manual tuning > > > > On Mon, 16 Aug 2021 at 15:00, <haibo.chen@nxp.com> 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? > > Yes, thanks! > Let's wait comments for patch 3. Alright, patch1 and pacth2 applied for next, thanks! [...] Kind regards Uffe
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) {