* [PATCH v2 0/4] Add ARTPEC-8 support to DWMMC controller @ 2021-12-06 14:29 Mårten Lindahl 2021-12-06 14:29 ` [PATCH v2 1/4] dt-bindings: mmc: exynos-dw-mshc: Add support for ARTPEC-8 Mårten Lindahl ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Mårten Lindahl @ 2021-12-06 14:29 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Jaehoon Chung Cc: Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, Mårten Lindahl Hi! The ARTPEC-8 SoC has a DWMMC controller that is compatible with the Exynos 7 version v2.70a. The main differences from Exynos 7 is that it does not support HS400 and has an extended data read timeout. To run this controller we need to add compatibility for ARTPEC-8, because we need a quirk to separate the configuration of the TMOUT register from the non ARTPEC-8 versions. This patchset is dependent on 2 changes that has been added to the mmc git next branch, but has not yet been merged to mainline: Patch 2 of this patchset depends on commit 0e6f2c4c2072b ("mmc: dw_mmc: add common capabilities to replace caps"). Patch 3 of this patchset depends on commit d5bc33487eab3 ("mmc: dw_mmc: Allow lower TMOUT value than maximum"). Kind regards Mårten Lindahl Changes in v2: - Change compatible string vendor prefix - Removed unnecessary comment - Change 1<<0 to BIT(0) Mårten Lindahl (4): dt-bindings: mmc: exynos-dw-mshc: Add support for ARTPEC-8 mmc: dw_mmc-exynos: Add support for ARTPEC-8 mmc: dw_mmc: Add quirk for extended data read timeout mmc: dw_mmc: Do not wait for DTO in case of error .../bindings/mmc/exynos-dw-mshc.txt | 2 + drivers/mmc/host/dw_mmc-exynos.c | 52 +++++++++++++++---- drivers/mmc/host/dw_mmc.c | 41 +++++++++++++-- drivers/mmc/host/dw_mmc.h | 6 +++ 4 files changed, 86 insertions(+), 15 deletions(-) -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/4] dt-bindings: mmc: exynos-dw-mshc: Add support for ARTPEC-8 2021-12-06 14:29 [PATCH v2 0/4] Add ARTPEC-8 support to DWMMC controller Mårten Lindahl @ 2021-12-06 14:29 ` Mårten Lindahl 2021-12-07 9:40 ` Krzysztof Kozlowski 2021-12-06 14:29 ` [PATCH v2 2/4] mmc: dw_mmc-exynos: " Mårten Lindahl ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Mårten Lindahl @ 2021-12-06 14:29 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Jaehoon Chung Cc: Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, Mårten Lindahl The ARTPEC-8 SoC has a DWMMC controller that is compatible with the Exynos 7 version v2.70a. The main differences from Exynos 7 is that it does not support HS400 and has extended data read timeout. Add compatibility string "axis,artpec8-dw-mshc" for ARTPEC-8. Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> --- v2: - Change compatible string vendor prefix Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt index 0419a63f73a0..753e9d7d8956 100644 --- a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt +++ b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt @@ -22,6 +22,8 @@ Required Properties: specific extensions. - "samsung,exynos7-dw-mshc-smu": for controllers with Samsung Exynos7 specific extensions having an SMU. + - "axis,artpec8-dw-mshc": for controllers with ARTPEC-8 specific + extensions. * samsung,dw-mshc-ciu-div: Specifies the divider value for the card interface unit (ciu) clock. This property is applicable only for Exynos5 SoC's and -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: mmc: exynos-dw-mshc: Add support for ARTPEC-8 2021-12-06 14:29 ` [PATCH v2 1/4] dt-bindings: mmc: exynos-dw-mshc: Add support for ARTPEC-8 Mårten Lindahl @ 2021-12-07 9:40 ` Krzysztof Kozlowski 0 siblings, 0 replies; 12+ messages in thread From: Krzysztof Kozlowski @ 2021-12-07 9:40 UTC (permalink / raw) To: Mårten Lindahl, Ulf Hansson, Rob Herring, Jaehoon Chung Cc: Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc On 06/12/2021 15:29, Mårten Lindahl wrote: > The ARTPEC-8 SoC has a DWMMC controller that is compatible with the > Exynos 7 version v2.70a. The main differences from Exynos 7 is that it > does not support HS400 and has extended data read timeout. > > Add compatibility string "axis,artpec8-dw-mshc" for ARTPEC-8. > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > --- > > v2: > - Change compatible string vendor prefix > > Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt > index 0419a63f73a0..753e9d7d8956 100644 > --- a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt > +++ b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt > @@ -22,6 +22,8 @@ Required Properties: > specific extensions. > - "samsung,exynos7-dw-mshc-smu": for controllers with Samsung Exynos7 > specific extensions having an SMU. > + - "axis,artpec8-dw-mshc": for controllers with ARTPEC-8 specific > + extensions. > > * samsung,dw-mshc-ciu-div: Specifies the divider value for the card interface > unit (ciu) clock. This property is applicable only for Exynos5 SoC's and > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] mmc: dw_mmc-exynos: Add support for ARTPEC-8 2021-12-06 14:29 [PATCH v2 0/4] Add ARTPEC-8 support to DWMMC controller Mårten Lindahl 2021-12-06 14:29 ` [PATCH v2 1/4] dt-bindings: mmc: exynos-dw-mshc: Add support for ARTPEC-8 Mårten Lindahl @ 2021-12-06 14:29 ` Mårten Lindahl 2021-12-07 9:40 ` Krzysztof Kozlowski 2021-12-06 14:29 ` [PATCH v2 3/4] mmc: dw_mmc: Add quirk for extended data read timeout Mårten Lindahl 2021-12-06 14:29 ` [PATCH v2 4/4] mmc: dw_mmc: Do not wait for DTO in case of error Mårten Lindahl 3 siblings, 1 reply; 12+ messages in thread From: Mårten Lindahl @ 2021-12-06 14:29 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Jaehoon Chung Cc: Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, Mårten Lindahl The ARTPEC-8 SoC has a DWMMC controller that is compatible with the Exynos 7 version v2.70a. The main differences from Exynos 7 is that it does not support HS400 and has extended data read timeout. This patch adds compatibility string "axis,artpec8-dw-mshc" for ARTPEC-8, and DW_MCI_TYPE_ARTPEC8 is added to the dw_mci_exynos_type. Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> --- v2: - Change compatible string vendor prefix drivers/mmc/host/dw_mmc-exynos.c | 47 ++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c index f76eeeb0cc53..86486e6659de 100644 --- a/drivers/mmc/host/dw_mmc-exynos.c +++ b/drivers/mmc/host/dw_mmc-exynos.c @@ -28,6 +28,7 @@ enum dw_mci_exynos_type { DW_MCI_TYPE_EXYNOS5420_SMU, DW_MCI_TYPE_EXYNOS7, DW_MCI_TYPE_EXYNOS7_SMU, + DW_MCI_TYPE_ARTPEC8, }; /* Exynos implementation specific driver private data */ @@ -69,6 +70,9 @@ static struct dw_mci_exynos_compatible { }, { .compatible = "samsung,exynos7-dw-mshc-smu", .ctrl_type = DW_MCI_TYPE_EXYNOS7_SMU, + }, { + .compatible = "axis,artpec8-dw-mshc", + .ctrl_type = DW_MCI_TYPE_ARTPEC8, }, }; @@ -81,7 +85,8 @@ static inline u8 dw_mci_exynos_get_ciu_div(struct dw_mci *host) else if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS4210) return EXYNOS4210_FIXED_CIU_CLK_DIV; else if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || - priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) + priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU || + priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) return SDMMC_CLKSEL_GET_DIV(mci_readl(host, CLKSEL64)) + 1; else return SDMMC_CLKSEL_GET_DIV(mci_readl(host, CLKSEL)) + 1; @@ -133,7 +138,8 @@ static void dw_mci_exynos_set_clksel_timing(struct dw_mci *host, u32 timing) u32 clksel; if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || - priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) + priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU || + priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) clksel = mci_readl(host, CLKSEL64); else clksel = mci_readl(host, CLKSEL); @@ -141,7 +147,8 @@ static void dw_mci_exynos_set_clksel_timing(struct dw_mci *host, u32 timing) clksel = (clksel & ~SDMMC_CLKSEL_TIMING_MASK) | timing; if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || - priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) + priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU || + priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) mci_writel(host, CLKSEL64, clksel); else mci_writel(host, CLKSEL, clksel); @@ -210,14 +217,16 @@ static int dw_mci_exynos_resume_noirq(struct device *dev) return ret; if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || - priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) + priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU || + priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) clksel = mci_readl(host, CLKSEL64); else clksel = mci_readl(host, CLKSEL); if (clksel & SDMMC_CLKSEL_WAKEUP_INT) { if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || - priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) + priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU || + priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) mci_writel(host, CLKSEL64, clksel); else mci_writel(host, CLKSEL, clksel); @@ -238,7 +247,8 @@ static void dw_mci_exynos_config_hs400(struct dw_mci *host, u32 timing) * Not supported to configure register * related to HS400 */ - if (priv->ctrl_type < DW_MCI_TYPE_EXYNOS5420) { + if ((priv->ctrl_type < DW_MCI_TYPE_EXYNOS5420) || + (priv->ctrl_type == DW_MCI_TYPE_ARTPEC8)) { if (timing == MMC_TIMING_MMC_HS400) dev_warn(host->dev, "cannot configure HS400, unsupported chipset\n"); @@ -394,7 +404,8 @@ static inline u8 dw_mci_exynos_get_clksmpl(struct dw_mci *host) struct dw_mci_exynos_priv_data *priv = host->priv; if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || - priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) + priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU || + priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) return SDMMC_CLKSEL_CCLK_SAMPLE(mci_readl(host, CLKSEL64)); else return SDMMC_CLKSEL_CCLK_SAMPLE(mci_readl(host, CLKSEL)); @@ -406,13 +417,15 @@ static inline void dw_mci_exynos_set_clksmpl(struct dw_mci *host, u8 sample) struct dw_mci_exynos_priv_data *priv = host->priv; if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || - priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) + priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU || + priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) clksel = mci_readl(host, CLKSEL64); else clksel = mci_readl(host, CLKSEL); clksel = SDMMC_CLKSEL_UP_SAMPLE(clksel, sample); if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || - priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) + priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU || + priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) mci_writel(host, CLKSEL64, clksel); else mci_writel(host, CLKSEL, clksel); @@ -425,7 +438,8 @@ static inline u8 dw_mci_exynos_move_next_clksmpl(struct dw_mci *host) u8 sample; if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || - priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) + priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU || + priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) clksel = mci_readl(host, CLKSEL64); else clksel = mci_readl(host, CLKSEL); @@ -434,7 +448,8 @@ static inline u8 dw_mci_exynos_move_next_clksmpl(struct dw_mci *host) clksel = SDMMC_CLKSEL_UP_SAMPLE(clksel, sample); if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || - priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) + priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU || + priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) mci_writel(host, CLKSEL64, clksel); else mci_writel(host, CLKSEL, clksel); @@ -543,6 +558,14 @@ static const struct dw_mci_drv_data exynos_drv_data = { .prepare_hs400_tuning = dw_mci_exynos_prepare_hs400_tuning, }; +static const struct dw_mci_drv_data artpec_drv_data = { + .common_caps = MMC_CAP_CMD23, + .init = dw_mci_exynos_priv_init, + .set_ios = dw_mci_exynos_set_ios, + .parse_dt = dw_mci_exynos_parse_dt, + .execute_tuning = dw_mci_exynos_execute_tuning, +}; + static const struct of_device_id dw_mci_exynos_match[] = { { .compatible = "samsung,exynos4412-dw-mshc", .data = &exynos_drv_data, }, @@ -556,6 +579,8 @@ static const struct of_device_id dw_mci_exynos_match[] = { .data = &exynos_drv_data, }, { .compatible = "samsung,exynos7-dw-mshc-smu", .data = &exynos_drv_data, }, + { .compatible = "axis,artpec8-dw-mshc", + .data = &artpec_drv_data, }, {}, }; MODULE_DEVICE_TABLE(of, dw_mci_exynos_match); -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] mmc: dw_mmc-exynos: Add support for ARTPEC-8 2021-12-06 14:29 ` [PATCH v2 2/4] mmc: dw_mmc-exynos: " Mårten Lindahl @ 2021-12-07 9:40 ` Krzysztof Kozlowski 0 siblings, 0 replies; 12+ messages in thread From: Krzysztof Kozlowski @ 2021-12-07 9:40 UTC (permalink / raw) To: Mårten Lindahl, Ulf Hansson, Rob Herring, Jaehoon Chung Cc: Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc On 06/12/2021 15:29, Mårten Lindahl wrote: > The ARTPEC-8 SoC has a DWMMC controller that is compatible with the > Exynos 7 version v2.70a. The main differences from Exynos 7 is that it > does not support HS400 and has extended data read timeout. > > This patch adds compatibility string "axis,artpec8-dw-mshc" for > ARTPEC-8, and DW_MCI_TYPE_ARTPEC8 is added to the dw_mci_exynos_type. > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > --- > > v2: > - Change compatible string vendor prefix > > drivers/mmc/host/dw_mmc-exynos.c | 47 ++++++++++++++++++++++++-------- > 1 file changed, 36 insertions(+), 11 deletions(-) > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] mmc: dw_mmc: Add quirk for extended data read timeout 2021-12-06 14:29 [PATCH v2 0/4] Add ARTPEC-8 support to DWMMC controller Mårten Lindahl 2021-12-06 14:29 ` [PATCH v2 1/4] dt-bindings: mmc: exynos-dw-mshc: Add support for ARTPEC-8 Mårten Lindahl 2021-12-06 14:29 ` [PATCH v2 2/4] mmc: dw_mmc-exynos: " Mårten Lindahl @ 2021-12-06 14:29 ` Mårten Lindahl 2021-12-07 9:41 ` Krzysztof Kozlowski 2021-12-08 14:41 ` Ulf Hansson 2021-12-06 14:29 ` [PATCH v2 4/4] mmc: dw_mmc: Do not wait for DTO in case of error Mårten Lindahl 3 siblings, 2 replies; 12+ messages in thread From: Mårten Lindahl @ 2021-12-06 14:29 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Jaehoon Chung Cc: Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, Mårten Lindahl Current dw_mci driver supports a TMOUT register which consists of a 24 bit field (TMOUT[31:8]) for the DATA_TIMEOUT. The maximum value of this field is 0xFFFFFF, which with a 200MHz clock will give a full DRTO of: 0xFFFFFF / 200000000 => ~84 ms However, the ARTPEC-8 SoC DWMMC IP version has a TMOUT register with an extended DATA_TIMEOUT field, which supports longer timers for the DRTO. In this version the DATA_TIMEOUT field is split into two, which with the same 200MHz clock as above will allow a maximum timeout of: ((TMOUT[10:8] -1) * 0xFFFFFF + TMOUT[31:11] * 8) / 200000000 => ~587 ms Add a quirk to support this. The quirk is enabled for ARTPEC-8 SoCs. Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> --- v2: - Removed unnecessary comment - Change 1<<0 to BIT(0) drivers/mmc/host/dw_mmc-exynos.c | 5 +++++ drivers/mmc/host/dw_mmc.c | 33 ++++++++++++++++++++++++++++---- drivers/mmc/host/dw_mmc.h | 6 ++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c index 86486e6659de..1b625642c5b4 100644 --- a/drivers/mmc/host/dw_mmc-exynos.c +++ b/drivers/mmc/host/dw_mmc-exynos.c @@ -127,6 +127,11 @@ static int dw_mci_exynos_priv_init(struct dw_mci *host) DQS_CTRL_GET_RD_DELAY(priv->saved_strobe_ctrl); } + if (priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) { + /* Quirk needed for ARTPEC-8 SoCs */ + host->quirks |= DW_MMC_QUIRK_EXTENDED_TMOUT; + } + host->bus_hz /= (priv->ciu_div + 1); return 0; diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index f2a14a434bef..45ea9fd97a6a 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1289,6 +1289,7 @@ static void dw_mci_set_data_timeout(struct dw_mci *host, { u32 clk_div, tmout; u64 tmp; + unsigned int tmp2; clk_div = (mci_readl(host, CLKDIV) & 0xFF) * 2; if (clk_div == 0) @@ -1301,10 +1302,28 @@ static void dw_mci_set_data_timeout(struct dw_mci *host, tmout = 0xFF; /* Set maximum */ /* TMOUT[31:8] (DATA_TIMEOUT) */ - if (!tmp || tmp > 0xFFFFFF) - tmout |= (0xFFFFFF << 8); - else - tmout |= (tmp & 0xFFFFFF) << 8; + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) { + /* + * Extended HW timer (max = 0x6FFFFF2): + * ((TMOUT[10:8] - 1) * 0xFFFFFF + TMOUT[31:11] * 8) + */ + if (!tmp || tmp > 0x6FFFFF2) + tmout |= (0xFFFFFF << 8); + else { + /* TMOUT[10:8] */ + tmp2 = (((unsigned int)tmp / 0xFFFFFF) + 1) & 0x7; + tmout |= tmp2 << 8; + + /* TMOUT[31:11] */ + tmp = tmp - ((tmp2 - 1) * 0xFFFFFF); + tmout |= (tmp & 0xFFFFF8) << 8; + } + } else { + if (!tmp || tmp > 0xFFFFFF) + tmout |= (0xFFFFFF << 8); + else + tmout |= (tmp & 0xFFFFFF) << 8; + } mci_writel(host, TMOUT, tmout); dev_dbg(host->dev, "timeout_ns: %u => TMOUT[31:8]: 0x%#08x", @@ -2005,9 +2024,15 @@ static void dw_mci_set_drto(struct dw_mci *host) if (drto_div == 0) drto_div = 1; + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) + drto_clks = (((drto_clks & 0x7) - 1) * 0xFFFFFF) + + ((drto_clks & 0xFFFFF8)); + drto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * drto_clks * drto_div, host->bus_hz); + dev_dbg(host->dev, "drto_ms: %u\n", drto_ms); + /* add a bit spare time */ drto_ms += 10; diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h index 771d5afa3136..3b6510d4a684 100644 --- a/drivers/mmc/host/dw_mmc.h +++ b/drivers/mmc/host/dw_mmc.h @@ -118,6 +118,7 @@ struct dw_mci_dma_slave { * @part_buf: Simple buffer for partial fifo reads/writes. * @push_data: Pointer to FIFO push function. * @pull_data: Pointer to FIFO pull function. + * @quirks: Set of quirks that apply to specific versions of the IP. * @vqmmc_enabled: Status of vqmmc, should be true or false. * @irq_flags: The flags to be passed to request_irq. * @irq: The irq value to be passed to request_irq. @@ -223,6 +224,8 @@ struct dw_mci { void (*push_data)(struct dw_mci *host, void *buf, int cnt); void (*pull_data)(struct dw_mci *host, void *buf, int cnt); + u32 quirks; + bool vqmmc_enabled; unsigned long irq_flags; /* IRQ flags */ int irq; @@ -274,6 +277,9 @@ struct dw_mci_board { struct dma_pdata *data; }; +/* Support for longer data read timeout */ +#define DW_MMC_QUIRK_EXTENDED_TMOUT BIT(0) + #define DW_MMC_240A 0x240a #define DW_MMC_280A 0x280a -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] mmc: dw_mmc: Add quirk for extended data read timeout 2021-12-06 14:29 ` [PATCH v2 3/4] mmc: dw_mmc: Add quirk for extended data read timeout Mårten Lindahl @ 2021-12-07 9:41 ` Krzysztof Kozlowski 2021-12-08 14:41 ` Ulf Hansson 1 sibling, 0 replies; 12+ messages in thread From: Krzysztof Kozlowski @ 2021-12-07 9:41 UTC (permalink / raw) To: Mårten Lindahl, Ulf Hansson, Rob Herring, Jaehoon Chung Cc: Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc On 06/12/2021 15:29, Mårten Lindahl wrote: > Current dw_mci driver supports a TMOUT register which consists of a 24 > bit field (TMOUT[31:8]) for the DATA_TIMEOUT. The maximum value of this > field is 0xFFFFFF, which with a 200MHz clock will give a full DRTO of: > > 0xFFFFFF / 200000000 => ~84 ms > > However, the ARTPEC-8 SoC DWMMC IP version has a TMOUT register with an > extended DATA_TIMEOUT field, which supports longer timers for the DRTO. > In this version the DATA_TIMEOUT field is split into two, which with the > same 200MHz clock as above will allow a maximum timeout of: > > ((TMOUT[10:8] -1) * 0xFFFFFF + TMOUT[31:11] * 8) / 200000000 => ~587 ms > > Add a quirk to support this. The quirk is enabled for ARTPEC-8 SoCs. > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > --- > > v2: > - Removed unnecessary comment > - Change 1<<0 to BIT(0) > > drivers/mmc/host/dw_mmc-exynos.c | 5 +++++ > drivers/mmc/host/dw_mmc.c | 33 ++++++++++++++++++++++++++++---- > drivers/mmc/host/dw_mmc.h | 6 ++++++ > 3 files changed, 40 insertions(+), 4 deletions(-) > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] mmc: dw_mmc: Add quirk for extended data read timeout 2021-12-06 14:29 ` [PATCH v2 3/4] mmc: dw_mmc: Add quirk for extended data read timeout Mårten Lindahl 2021-12-07 9:41 ` Krzysztof Kozlowski @ 2021-12-08 14:41 ` Ulf Hansson 2021-12-08 21:04 ` Marten Lindahl 1 sibling, 1 reply; 12+ messages in thread From: Ulf Hansson @ 2021-12-08 14:41 UTC (permalink / raw) To: Mårten Lindahl Cc: Rob Herring, Krzysztof Kozlowski, Jaehoon Chung, Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc On Mon, 6 Dec 2021 at 15:29, Mårten Lindahl <marten.lindahl@axis.com> wrote: > > Current dw_mci driver supports a TMOUT register which consists of a 24 > bit field (TMOUT[31:8]) for the DATA_TIMEOUT. The maximum value of this > field is 0xFFFFFF, which with a 200MHz clock will give a full DRTO of: > > 0xFFFFFF / 200000000 => ~84 ms > > However, the ARTPEC-8 SoC DWMMC IP version has a TMOUT register with an > extended DATA_TIMEOUT field, which supports longer timers for the DRTO. > In this version the DATA_TIMEOUT field is split into two, which with the > same 200MHz clock as above will allow a maximum timeout of: > > ((TMOUT[10:8] -1) * 0xFFFFFF + TMOUT[31:11] * 8) / 200000000 => ~587 ms > > Add a quirk to support this. The quirk is enabled for ARTPEC-8 SoCs. > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > --- > > v2: > - Removed unnecessary comment > - Change 1<<0 to BIT(0) > > drivers/mmc/host/dw_mmc-exynos.c | 5 +++++ > drivers/mmc/host/dw_mmc.c | 33 ++++++++++++++++++++++++++++---- > drivers/mmc/host/dw_mmc.h | 6 ++++++ > 3 files changed, 40 insertions(+), 4 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c > index 86486e6659de..1b625642c5b4 100644 > --- a/drivers/mmc/host/dw_mmc-exynos.c > +++ b/drivers/mmc/host/dw_mmc-exynos.c > @@ -127,6 +127,11 @@ static int dw_mci_exynos_priv_init(struct dw_mci *host) > DQS_CTRL_GET_RD_DELAY(priv->saved_strobe_ctrl); > } > > + if (priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) { > + /* Quirk needed for ARTPEC-8 SoCs */ > + host->quirks |= DW_MMC_QUIRK_EXTENDED_TMOUT; > + } > + > host->bus_hz /= (priv->ciu_div + 1); > > return 0; > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index f2a14a434bef..45ea9fd97a6a 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -1289,6 +1289,7 @@ static void dw_mci_set_data_timeout(struct dw_mci *host, > { > u32 clk_div, tmout; > u64 tmp; > + unsigned int tmp2; > > clk_div = (mci_readl(host, CLKDIV) & 0xFF) * 2; > if (clk_div == 0) > @@ -1301,10 +1302,28 @@ static void dw_mci_set_data_timeout(struct dw_mci *host, > tmout = 0xFF; /* Set maximum */ > > /* TMOUT[31:8] (DATA_TIMEOUT) */ > - if (!tmp || tmp > 0xFFFFFF) > - tmout |= (0xFFFFFF << 8); > - else > - tmout |= (tmp & 0xFFFFFF) << 8; > + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) { Adding an option for dealing with quirks, should be avoided if possible. That's because we want to avoid sprinkling common dw_mmc code with variant specific code, as it will sooner or later turn into a nightmare to maintain. In this case, I suggest you look into extending the struct dw_mci_drv_data with some new callback (perhaps ->set_data_timeout()) and then implement it for your variant. If that callback is present, it should take precedence over the generic dw_mci_set_data_timeout(), if you get what I mean. Moreover, if some common dw_mmc code needs to be called from your callback, I suggest we make that code available through exported dw_mmc library functions instead. > + /* > + * Extended HW timer (max = 0x6FFFFF2): > + * ((TMOUT[10:8] - 1) * 0xFFFFFF + TMOUT[31:11] * 8) > + */ > + if (!tmp || tmp > 0x6FFFFF2) > + tmout |= (0xFFFFFF << 8); > + else { > + /* TMOUT[10:8] */ > + tmp2 = (((unsigned int)tmp / 0xFFFFFF) + 1) & 0x7; > + tmout |= tmp2 << 8; > + > + /* TMOUT[31:11] */ > + tmp = tmp - ((tmp2 - 1) * 0xFFFFFF); > + tmout |= (tmp & 0xFFFFF8) << 8; > + } > + } else { > + if (!tmp || tmp > 0xFFFFFF) > + tmout |= (0xFFFFFF << 8); > + else > + tmout |= (tmp & 0xFFFFFF) << 8; > + } > > mci_writel(host, TMOUT, tmout); > dev_dbg(host->dev, "timeout_ns: %u => TMOUT[31:8]: 0x%#08x", > @@ -2005,9 +2024,15 @@ static void dw_mci_set_drto(struct dw_mci *host) > if (drto_div == 0) > drto_div = 1; > > + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) > + drto_clks = (((drto_clks & 0x7) - 1) * 0xFFFFFF) + > + ((drto_clks & 0xFFFFF8)); > + > drto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * drto_clks * drto_div, > host->bus_hz); > > + dev_dbg(host->dev, "drto_ms: %u\n", drto_ms); > + > /* add a bit spare time */ > drto_ms += 10; > > diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h > index 771d5afa3136..3b6510d4a684 100644 > --- a/drivers/mmc/host/dw_mmc.h > +++ b/drivers/mmc/host/dw_mmc.h > @@ -118,6 +118,7 @@ struct dw_mci_dma_slave { > * @part_buf: Simple buffer for partial fifo reads/writes. > * @push_data: Pointer to FIFO push function. > * @pull_data: Pointer to FIFO pull function. > + * @quirks: Set of quirks that apply to specific versions of the IP. > * @vqmmc_enabled: Status of vqmmc, should be true or false. > * @irq_flags: The flags to be passed to request_irq. > * @irq: The irq value to be passed to request_irq. > @@ -223,6 +224,8 @@ struct dw_mci { > void (*push_data)(struct dw_mci *host, void *buf, int cnt); > void (*pull_data)(struct dw_mci *host, void *buf, int cnt); > > + u32 quirks; > + > bool vqmmc_enabled; > unsigned long irq_flags; /* IRQ flags */ > int irq; > @@ -274,6 +277,9 @@ struct dw_mci_board { > struct dma_pdata *data; > }; > > +/* Support for longer data read timeout */ > +#define DW_MMC_QUIRK_EXTENDED_TMOUT BIT(0) > + > #define DW_MMC_240A 0x240a > #define DW_MMC_280A 0x280a > Kind regards Uffe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] mmc: dw_mmc: Add quirk for extended data read timeout 2021-12-08 14:41 ` Ulf Hansson @ 2021-12-08 21:04 ` Marten Lindahl 0 siblings, 0 replies; 12+ messages in thread From: Marten Lindahl @ 2021-12-08 21:04 UTC (permalink / raw) To: Ulf Hansson Cc: Mårten Lindahl, Rob Herring, Krzysztof Kozlowski, Jaehoon Chung, Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc On Wed, Dec 08, 2021 at 03:41:24PM +0100, Ulf Hansson wrote: > On Mon, 6 Dec 2021 at 15:29, Mårten Lindahl <marten.lindahl@axis.com> wrote: > > > > Current dw_mci driver supports a TMOUT register which consists of a 24 > > bit field (TMOUT[31:8]) for the DATA_TIMEOUT. The maximum value of this > > field is 0xFFFFFF, which with a 200MHz clock will give a full DRTO of: > > > > 0xFFFFFF / 200000000 => ~84 ms > > > > However, the ARTPEC-8 SoC DWMMC IP version has a TMOUT register with an > > extended DATA_TIMEOUT field, which supports longer timers for the DRTO. > > In this version the DATA_TIMEOUT field is split into two, which with the > > same 200MHz clock as above will allow a maximum timeout of: > > > > ((TMOUT[10:8] -1) * 0xFFFFFF + TMOUT[31:11] * 8) / 200000000 => ~587 ms > > > > Add a quirk to support this. The quirk is enabled for ARTPEC-8 SoCs. > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > > --- > > > > v2: > > - Removed unnecessary comment > > - Change 1<<0 to BIT(0) > > > > drivers/mmc/host/dw_mmc-exynos.c | 5 +++++ > > drivers/mmc/host/dw_mmc.c | 33 ++++++++++++++++++++++++++++---- > > drivers/mmc/host/dw_mmc.h | 6 ++++++ > > 3 files changed, 40 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c > > index 86486e6659de..1b625642c5b4 100644 > > --- a/drivers/mmc/host/dw_mmc-exynos.c > > +++ b/drivers/mmc/host/dw_mmc-exynos.c > > @@ -127,6 +127,11 @@ static int dw_mci_exynos_priv_init(struct dw_mci *host) > > DQS_CTRL_GET_RD_DELAY(priv->saved_strobe_ctrl); > > } > > > > + if (priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) { > > + /* Quirk needed for ARTPEC-8 SoCs */ > > + host->quirks |= DW_MMC_QUIRK_EXTENDED_TMOUT; > > + } > > + > > host->bus_hz /= (priv->ciu_div + 1); > > > > return 0; > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > > index f2a14a434bef..45ea9fd97a6a 100644 > > --- a/drivers/mmc/host/dw_mmc.c > > +++ b/drivers/mmc/host/dw_mmc.c > > @@ -1289,6 +1289,7 @@ static void dw_mci_set_data_timeout(struct dw_mci *host, > > { > > u32 clk_div, tmout; > > u64 tmp; > > + unsigned int tmp2; > > > > clk_div = (mci_readl(host, CLKDIV) & 0xFF) * 2; > > if (clk_div == 0) > > @@ -1301,10 +1302,28 @@ static void dw_mci_set_data_timeout(struct dw_mci *host, > > tmout = 0xFF; /* Set maximum */ > > > > /* TMOUT[31:8] (DATA_TIMEOUT) */ > > - if (!tmp || tmp > 0xFFFFFF) > > - tmout |= (0xFFFFFF << 8); > > - else > > - tmout |= (tmp & 0xFFFFFF) << 8; > > + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) { > > Adding an option for dealing with quirks, should be avoided if > possible. That's because we want to avoid sprinkling common dw_mmc > code with variant specific code, as it will sooner or later turn into > a nightmare to maintain. > > In this case, I suggest you look into extending the struct > dw_mci_drv_data with some new callback (perhaps ->set_data_timeout()) > and then implement it for your variant. > If that callback is present, it should take precedence over the > generic dw_mci_set_data_timeout(), if you get what I mean. Hi Ulf! Thanks! That's a good idea. I will implement the callback approach instead of a quirk for this patch. It will be two callbacks; one for the set_data_timeout(), and one for the set_drto(). > > Moreover, if some common dw_mmc code needs to be called from your > callback, I suggest we make that code available through exported > dw_mmc library functions instead. > I don't think I need to export anything for this patch. Kind regards Mårten > > + /* > > + * Extended HW timer (max = 0x6FFFFF2): > > + * ((TMOUT[10:8] - 1) * 0xFFFFFF + TMOUT[31:11] * 8) > > + */ > > + if (!tmp || tmp > 0x6FFFFF2) > > + tmout |= (0xFFFFFF << 8); > > + else { > > + /* TMOUT[10:8] */ > > + tmp2 = (((unsigned int)tmp / 0xFFFFFF) + 1) & 0x7; > > + tmout |= tmp2 << 8; > > + > > + /* TMOUT[31:11] */ > > + tmp = tmp - ((tmp2 - 1) * 0xFFFFFF); > > + tmout |= (tmp & 0xFFFFF8) << 8; > > + } > > + } else { > > + if (!tmp || tmp > 0xFFFFFF) > > + tmout |= (0xFFFFFF << 8); > > + else > > + tmout |= (tmp & 0xFFFFFF) << 8; > > + } > > > > mci_writel(host, TMOUT, tmout); > > dev_dbg(host->dev, "timeout_ns: %u => TMOUT[31:8]: 0x%#08x", > > @@ -2005,9 +2024,15 @@ static void dw_mci_set_drto(struct dw_mci *host) > > if (drto_div == 0) > > drto_div = 1; > > > > + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) > > + drto_clks = (((drto_clks & 0x7) - 1) * 0xFFFFFF) + > > + ((drto_clks & 0xFFFFF8)); > > + > > drto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * drto_clks * drto_div, > > host->bus_hz); > > > > + dev_dbg(host->dev, "drto_ms: %u\n", drto_ms); > > + > > /* add a bit spare time */ > > drto_ms += 10; > > > > diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h > > index 771d5afa3136..3b6510d4a684 100644 > > --- a/drivers/mmc/host/dw_mmc.h > > +++ b/drivers/mmc/host/dw_mmc.h > > @@ -118,6 +118,7 @@ struct dw_mci_dma_slave { > > * @part_buf: Simple buffer for partial fifo reads/writes. > > * @push_data: Pointer to FIFO push function. > > * @pull_data: Pointer to FIFO pull function. > > + * @quirks: Set of quirks that apply to specific versions of the IP. > > * @vqmmc_enabled: Status of vqmmc, should be true or false. > > * @irq_flags: The flags to be passed to request_irq. > > * @irq: The irq value to be passed to request_irq. > > @@ -223,6 +224,8 @@ struct dw_mci { > > void (*push_data)(struct dw_mci *host, void *buf, int cnt); > > void (*pull_data)(struct dw_mci *host, void *buf, int cnt); > > > > + u32 quirks; > > + > > bool vqmmc_enabled; > > unsigned long irq_flags; /* IRQ flags */ > > int irq; > > @@ -274,6 +277,9 @@ struct dw_mci_board { > > struct dma_pdata *data; > > }; > > > > +/* Support for longer data read timeout */ > > +#define DW_MMC_QUIRK_EXTENDED_TMOUT BIT(0) > > + > > #define DW_MMC_240A 0x240a > > #define DW_MMC_280A 0x280a > > > > Kind regards > Uffe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] mmc: dw_mmc: Do not wait for DTO in case of error 2021-12-06 14:29 [PATCH v2 0/4] Add ARTPEC-8 support to DWMMC controller Mårten Lindahl ` (2 preceding siblings ...) 2021-12-06 14:29 ` [PATCH v2 3/4] mmc: dw_mmc: Add quirk for extended data read timeout Mårten Lindahl @ 2021-12-06 14:29 ` Mårten Lindahl 2021-12-08 14:53 ` Ulf Hansson 3 siblings, 1 reply; 12+ messages in thread From: Mårten Lindahl @ 2021-12-06 14:29 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Jaehoon Chung Cc: Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, Mårten Lindahl When running the ARTPEC-8 DWMMC IP version, and a data error interrupt comes during a data read transfer, there is no guarantee for the data transfer over interrupt (DTO) to come within the specified data timeout. This case is handled by the dto_timer handler which will complete the request with the comment: /* * If DTO interrupt does NOT come in sending data state, * we should notify the driver to terminate current transfer * and report a data timeout to the core. */ But since the ARTPEC-8 DWMMC IP version, supports an extended TMOUT register which allows longer timeouts than the non ARTPEC-8 version does, waiting for the dto_timer to complete the request in error cases may cause the request to take significantly longer time than necessary. This is specifically true for the failing steps during tuning of a device. Fix this by completing the request when the error interrupt comes. Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> --- drivers/mmc/host/dw_mmc.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 45ea9fd97a6a..d6b76f47b1a2 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -2777,11 +2777,19 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) if (pending & DW_MCI_DATA_ERROR_FLAGS) { spin_lock(&host->irq_lock); + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) + del_timer(&host->dto_timer); + /* if there is an error report DATA_ERROR */ mci_writel(host, RINTSTS, DW_MCI_DATA_ERROR_FLAGS); host->data_status = pending; smp_wmb(); /* drain writebuffer */ set_bit(EVENT_DATA_ERROR, &host->pending_events); + + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) + /* In case of error, we cannot expect a DTO */ + set_bit(EVENT_DATA_COMPLETE, &host->pending_events); + tasklet_schedule(&host->tasklet); spin_unlock(&host->irq_lock); -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] mmc: dw_mmc: Do not wait for DTO in case of error 2021-12-06 14:29 ` [PATCH v2 4/4] mmc: dw_mmc: Do not wait for DTO in case of error Mårten Lindahl @ 2021-12-08 14:53 ` Ulf Hansson 2021-12-08 22:01 ` Marten Lindahl 0 siblings, 1 reply; 12+ messages in thread From: Ulf Hansson @ 2021-12-08 14:53 UTC (permalink / raw) To: Mårten Lindahl Cc: Rob Herring, Krzysztof Kozlowski, Jaehoon Chung, Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc On Mon, 6 Dec 2021 at 15:29, Mårten Lindahl <marten.lindahl@axis.com> wrote: > > When running the ARTPEC-8 DWMMC IP version, and a data error interrupt > comes during a data read transfer, there is no guarantee for the data > transfer over interrupt (DTO) to come within the specified data timeout. > This case is handled by the dto_timer handler which will complete the > request with the comment: > > /* > * If DTO interrupt does NOT come in sending data state, > * we should notify the driver to terminate current transfer > * and report a data timeout to the core. > */ > > But since the ARTPEC-8 DWMMC IP version, supports an extended TMOUT > register which allows longer timeouts than the non ARTPEC-8 version > does, waiting for the dto_timer to complete the request in error cases > may cause the request to take significantly longer time than necessary. > This is specifically true for the failing steps during tuning of a > device. > > Fix this by completing the request when the error interrupt comes. > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> Okay, this change looks a bit inconvenient to move into variant specific callbacks. So, maybe the "quirks" flag makes sense, after all. However, I would still look at using callbacks and library functions, for the part implemented in patch3. When it comes to the order of the patches in the series, I suggest flipping things around and making patch2 the final piece. Otherwise the support for the artpec variant will be broken between patch2 and patch4, right? Kind regards Uffe > --- > drivers/mmc/host/dw_mmc.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 45ea9fd97a6a..d6b76f47b1a2 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -2777,11 +2777,19 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) > if (pending & DW_MCI_DATA_ERROR_FLAGS) { > spin_lock(&host->irq_lock); > > + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) > + del_timer(&host->dto_timer); > + > /* if there is an error report DATA_ERROR */ > mci_writel(host, RINTSTS, DW_MCI_DATA_ERROR_FLAGS); > host->data_status = pending; > smp_wmb(); /* drain writebuffer */ > set_bit(EVENT_DATA_ERROR, &host->pending_events); > + > + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) > + /* In case of error, we cannot expect a DTO */ > + set_bit(EVENT_DATA_COMPLETE, &host->pending_events); > + > tasklet_schedule(&host->tasklet); > > spin_unlock(&host->irq_lock); > -- > 2.20.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] mmc: dw_mmc: Do not wait for DTO in case of error 2021-12-08 14:53 ` Ulf Hansson @ 2021-12-08 22:01 ` Marten Lindahl 0 siblings, 0 replies; 12+ messages in thread From: Marten Lindahl @ 2021-12-08 22:01 UTC (permalink / raw) To: Ulf Hansson Cc: Mårten Lindahl, Rob Herring, Krzysztof Kozlowski, Jaehoon Chung, Doug Anderson, kernel, linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc On Wed, Dec 08, 2021 at 03:53:54PM +0100, Ulf Hansson wrote: > On Mon, 6 Dec 2021 at 15:29, Mårten Lindahl <marten.lindahl@axis.com> wrote: > > > > When running the ARTPEC-8 DWMMC IP version, and a data error interrupt > > comes during a data read transfer, there is no guarantee for the data > > transfer over interrupt (DTO) to come within the specified data timeout. > > This case is handled by the dto_timer handler which will complete the > > request with the comment: > > > > /* > > * If DTO interrupt does NOT come in sending data state, > > * we should notify the driver to terminate current transfer > > * and report a data timeout to the core. > > */ > > > > But since the ARTPEC-8 DWMMC IP version, supports an extended TMOUT > > register which allows longer timeouts than the non ARTPEC-8 version > > does, waiting for the dto_timer to complete the request in error cases > > may cause the request to take significantly longer time than necessary. > > This is specifically true for the failing steps during tuning of a > > device. > > > > Fix this by completing the request when the error interrupt comes. > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > Hi Ulf! > Okay, this change looks a bit inconvenient to move into variant > specific callbacks. So, maybe the "quirks" flag makes sense, after > all. However, I would still look at using callbacks and library > functions, for the part implemented in patch3. Yes, I don't see how this patch can be easily made with callbacks, but definitely for patch3. So then I move the definition of the quirk from patch3 to this patch. > > When it comes to the order of the patches in the series, I suggest > flipping things around and making patch2 the final piece. Otherwise > the support for the artpec variant will be broken between patch2 and > patch4, right? Ok, you mean there may be a risk that the ARTPEC-8 dw_mmc does not work if the support is enabled in patch2, but patch3 and patch4 is not in place? That is a good point, but it actually does work quite fine (most of the time) without the extended timeout function. But it does not use the full function of the data timeout, and the HW timeout is most often set to full timeout (0xFFFFFF => 587ms with 200MHz), but the SW timer is limited to a lower value (0xFFFFFF => 84 + 10 ms with 200MHz). My reasoning is: patch1 - dtbindings for ARTPEC-8 patch2 - adding ARTPEC-8 to dw_mmc-exynos patch3 - implement ARTPEC-8 specific function for data timeout patch4 - add quirk to abort the extended timeout in case of errors, used by ARTPEC-8 so, this means patch3 and patch4 depends on patch2, and patch4 depends on patch3. Kind regards Mårten > > Kind regards > Uffe > > > --- > > drivers/mmc/host/dw_mmc.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > > index 45ea9fd97a6a..d6b76f47b1a2 100644 > > --- a/drivers/mmc/host/dw_mmc.c > > +++ b/drivers/mmc/host/dw_mmc.c > > @@ -2777,11 +2777,19 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) > > if (pending & DW_MCI_DATA_ERROR_FLAGS) { > > spin_lock(&host->irq_lock); > > > > + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) > > + del_timer(&host->dto_timer); > > + > > /* if there is an error report DATA_ERROR */ > > mci_writel(host, RINTSTS, DW_MCI_DATA_ERROR_FLAGS); > > host->data_status = pending; > > smp_wmb(); /* drain writebuffer */ > > set_bit(EVENT_DATA_ERROR, &host->pending_events); > > + > > + if (host->quirks & DW_MMC_QUIRK_EXTENDED_TMOUT) > > + /* In case of error, we cannot expect a DTO */ > > + set_bit(EVENT_DATA_COMPLETE, &host->pending_events); > > + > > tasklet_schedule(&host->tasklet); > > > > spin_unlock(&host->irq_lock); > > -- > > 2.20.1 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-12-08 22:04 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-06 14:29 [PATCH v2 0/4] Add ARTPEC-8 support to DWMMC controller Mårten Lindahl 2021-12-06 14:29 ` [PATCH v2 1/4] dt-bindings: mmc: exynos-dw-mshc: Add support for ARTPEC-8 Mårten Lindahl 2021-12-07 9:40 ` Krzysztof Kozlowski 2021-12-06 14:29 ` [PATCH v2 2/4] mmc: dw_mmc-exynos: " Mårten Lindahl 2021-12-07 9:40 ` Krzysztof Kozlowski 2021-12-06 14:29 ` [PATCH v2 3/4] mmc: dw_mmc: Add quirk for extended data read timeout Mårten Lindahl 2021-12-07 9:41 ` Krzysztof Kozlowski 2021-12-08 14:41 ` Ulf Hansson 2021-12-08 21:04 ` Marten Lindahl 2021-12-06 14:29 ` [PATCH v2 4/4] mmc: dw_mmc: Do not wait for DTO in case of error Mårten Lindahl 2021-12-08 14:53 ` Ulf Hansson 2021-12-08 22:01 ` Marten Lindahl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).