[PATCH v2 0/4] Add ARTPEC-8 support to DWMMC controller

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

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

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

On 06/12/2021 15:29, Mårten Lindahl wrote:

Reviewed-by: Krzysztof Kozlowski <redacted>

Best regards,
Krzysztof

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

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

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

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

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


Best regards,
Krzysztof

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

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

On Mon, 6 Dec 2021 at 15:29, Mårten Lindahl [off-list ref] 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 <redacted>

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

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

On Wed, Dec 08, 2021 at 03:53:54PM +0100, Ulf Hansson wrote:

On Mon, 6 Dec 2021 at 15:29, Mårten Lindahl [off-list ref] wrote:

quoted

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

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

quoted

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

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

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

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

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


Best regards,
Krzysztof

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

On Mon, 6 Dec 2021 at 15:29, Mårten Lindahl [off-list ref] wrote:

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.

Kind regards
Uffe

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

On Wed, Dec 08, 2021 at 03:41:24PM +0100, Ulf Hansson wrote:

On Mon, 6 Dec 2021 at 15:29, Mårten Lindahl [off-list ref] wrote:

quoted

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

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

quoted

+               /*
+                * 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