[PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register

[PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register

[Ahmad Fatoum]

We don't usually care for the card detect in barebox and just
unconditionally try to access the card. If it fails, a probe can be
attempted later via setting mmcX.probe=1.

The dw_mmc driver diverges from this and enables all kinds
of interrupts in the interrupt status polled after submitting commands,
including card-detect.

This appears to be problematic, when we have a GPIO card detect and are
not using the card detect hardware function.

Align behavior with Linux and skip polling for hardware card-detect when
we have a GPIO card detect. The current behavior was inherited from
U-Boot, which still does it this way and it's possible it would be affected
by this issue as well, once U-Boot updates to use DTs >= v6.19-rc1, provided
that the ROCK 5 support in U-Boot uses CONFIG_OF_UPSTREAM.

Fixes: a3bf6c16f77d ("dts: update to v6.19-rc1")
Reported-by: Michael Tretter <m.tretter@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/dw_mmc.c | 33 ++++++++++++++++++++++++++++++++-
 drivers/mci/dw_mmc.h |  1 +
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/mci/dw_mmc.c b/drivers/mci/dw_mmc.c
index 30e5d56944b6..9ddf931eb513 100644
--- a/drivers/mci/dw_mmc.c
+++ b/drivers/mci/dw_mmc.c
@@ -12,6 +12,7 @@
 #include <driver.h>
 #include <malloc.h>
 #include <clock.h>
+#include <linux/gpio/consumer.h>
 #include <init.h>
 #include <mci.h>
 #include <io.h>
@@ -23,6 +24,20 @@
 #include <errno.h>
 #include "dw_mmc.h"
 
+/* Most importantly, it doesn't contain DWMCI_INTMSK_CD as to allow
+ * for GPIO card detect, unlike DWMCI_INTMSK_ALL.
+ * The list is taken from Linux v6.19 with addition of DWMCI_INTMSK_FRUN
+ * as barebox had inherited this into DWMCI_DATA_ERR from U-Boot
+ */
+#define DW_MCI_DATA_ERROR_FLAGS	(DWMCI_INTMSK_DRTO | DWMCI_INTMSK_DCRC | \
+				 DWMCI_INTMSK_HTO | DWMCI_INTMSK_SBE  | \
+				 DWMCI_INTMSK_EBE | DWMCI_INTMSK_HLE | \
+				 DWMCI_INTMSK_FRUN)
+#define DW_MCI_CMD_ERROR_FLAGS	(DWMCI_INTMSK_RTO | DWMCI_INTMSK_RCRC | \
+				 DWMCI_INTMSK_RE | DWMCI_INTMSK_HLE)
+
+#define DW_MCI_ERROR_FLAGS	(DW_MCI_DATA_ERROR_FLAGS | \
+				 DW_MCI_CMD_ERROR_FLAGS)
 
 struct dwmci_host {
 	struct mci_host mci;
@@ -37,6 +52,7 @@ struct dwmci_host {
 	u32 fifoth_val;
 	u32 pwren_value;
 	dma_addr_t idmac_dma;
+	struct gpio_desc *cd_gpio;
 };
 
 struct dwmci_idmac {
@@ -285,7 +301,13 @@ dwmci_cmd(struct mci_host *mci, struct mci_cmd *cmd, struct mci_data *data)
 		}
 	}
 
-	dwmci_writel(host, DWMCI_RINTSTS, DWMCI_INTMSK_ALL);
+	/*
+	 * Enable interrupts for command done, data over, data empty,
+	 * receive ready and error such as transmit, receive timeout, crc error
+	 */
+	dwmci_writel(host, DWMCI_RINTSTS, DWMCI_INTMSK_CDONE | DWMCI_INTMSK_DTO |
+		   DWMCI_INTMSK_TXDR | DWMCI_INTMSK_RXDR |
+		   DW_MCI_ERROR_FLAGS);
 
 	if (data) {
 
@@ -586,6 +608,15 @@ static int dw_mmc_probe(struct device *dev)
 		reset_control_deassert(rst);
 	}
 
+	/*
+	 * We don't actually care for the cd-gpios and it's usually connected
+	 * just to a dumb lever shorting to ground, but let's mux as GPIO
+	 * to be on the safe side.
+	 */
+	host->cd_gpio = gpiod_get_optional(dev, "cd", GPIOD_IN);
+	if (IS_ERR(host->cd_gpio))
+		dev_dbg(dev, "Failed to get cd-gpios: %pe (ignoring)\n", host->cd_gpio);
+
 	iores = dev_request_mem_resource(dev, 0);
 	if (IS_ERR(iores))
 		return PTR_ERR(iores);
diff --git a/drivers/mci/dw_mmc.h b/drivers/mci/dw_mmc.h
index 23fa116d7514..eb5331d1ccb2 100644
--- a/drivers/mci/dw_mmc.h
+++ b/drivers/mci/dw_mmc.h
@@ -53,6 +53,7 @@
 
 /* Interrupt Mask register */
 #define DWMCI_INTMSK_ALL	0xffffffff
+#define DWMCI_INTMSK_CD		BIT(0)
 #define DWMCI_INTMSK_RE		BIT(1)
 #define DWMCI_INTMSK_CDONE	BIT(2)
 #define DWMCI_INTMSK_DTO	BIT(3)
-- 
2.47.3

Re: [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register

[Ahmad Fatoum]

Hi,

On 2/9/26 5:40 PM, Ahmad Fatoum wrote:
> We don't usually care for the card detect in barebox and just
> unconditionally try to access the card. If it fails, a probe can be
> attempted later via setting mmcX.probe=1.
> 
> The dw_mmc driver diverges from this and enables all kinds
> of interrupts in the interrupt status polled after submitting commands,
> including card-detect.
> 
> This appears to be problematic, when we have a GPIO card detect and are
> not using the card detect hardware function.
> 
> Align behavior with Linux and skip polling for hardware card-detect when
> we have a GPIO card detect. The current behavior was inherited from
> U-Boot, which still does it this way and it's possible it would be affected
> by this issue as well, once U-Boot updates to use DTs >= v6.19-rc1, provided
> that the ROCK 5 support in U-Boot uses CONFIG_OF_UPSTREAM.
> 
> Fixes: a3bf6c16f77d ("dts: update to v6.19-rc1")
> Reported-by: Michael Tretter <m.tretter@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---

I should have added: This is only commpile-tested. Would be great if
someone (Michael?) could give this a boot on a Radxa ROCK 5.

Thanks,
Ahmad

>  drivers/mci/dw_mmc.c | 33 ++++++++++++++++++++++++++++++++-
>  drivers/mci/dw_mmc.h |  1 +
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mci/dw_mmc.c b/drivers/mci/dw_mmc.c
> index 30e5d56944b6..9ddf931eb513 100644
> --- a/drivers/mci/dw_mmc.c
> +++ b/drivers/mci/dw_mmc.c
> @@ -12,6 +12,7 @@
>  #include <driver.h>
>  #include <malloc.h>
>  #include <clock.h>
> +#include <linux/gpio/consumer.h>
>  #include <init.h>
>  #include <mci.h>
>  #include <io.h>
> @@ -23,6 +24,20 @@
>  #include <errno.h>
>  #include "dw_mmc.h"
>  
> +/* Most importantly, it doesn't contain DWMCI_INTMSK_CD as to allow
> + * for GPIO card detect, unlike DWMCI_INTMSK_ALL.
> + * The list is taken from Linux v6.19 with addition of DWMCI_INTMSK_FRUN
> + * as barebox had inherited this into DWMCI_DATA_ERR from U-Boot
> + */
> +#define DW_MCI_DATA_ERROR_FLAGS	(DWMCI_INTMSK_DRTO | DWMCI_INTMSK_DCRC | \
> +				 DWMCI_INTMSK_HTO | DWMCI_INTMSK_SBE  | \
> +				 DWMCI_INTMSK_EBE | DWMCI_INTMSK_HLE | \
> +				 DWMCI_INTMSK_FRUN)
> +#define DW_MCI_CMD_ERROR_FLAGS	(DWMCI_INTMSK_RTO | DWMCI_INTMSK_RCRC | \
> +				 DWMCI_INTMSK_RE | DWMCI_INTMSK_HLE)
> +
> +#define DW_MCI_ERROR_FLAGS	(DW_MCI_DATA_ERROR_FLAGS | \
> +				 DW_MCI_CMD_ERROR_FLAGS)
>  
>  struct dwmci_host {
>  	struct mci_host mci;
> @@ -37,6 +52,7 @@ struct dwmci_host {
>  	u32 fifoth_val;
>  	u32 pwren_value;
>  	dma_addr_t idmac_dma;
> +	struct gpio_desc *cd_gpio;
>  };
>  
>  struct dwmci_idmac {
> @@ -285,7 +301,13 @@ dwmci_cmd(struct mci_host *mci, struct mci_cmd *cmd, struct mci_data *data)
>  		}
>  	}
>  
> -	dwmci_writel(host, DWMCI_RINTSTS, DWMCI_INTMSK_ALL);
> +	/*
> +	 * Enable interrupts for command done, data over, data empty,
> +	 * receive ready and error such as transmit, receive timeout, crc error
> +	 */
> +	dwmci_writel(host, DWMCI_RINTSTS, DWMCI_INTMSK_CDONE | DWMCI_INTMSK_DTO |
> +		   DWMCI_INTMSK_TXDR | DWMCI_INTMSK_RXDR |
> +		   DW_MCI_ERROR_FLAGS);
>  
>  	if (data) {
>  
> @@ -586,6 +608,15 @@ static int dw_mmc_probe(struct device *dev)
>  		reset_control_deassert(rst);
>  	}
>  
> +	/*
> +	 * We don't actually care for the cd-gpios and it's usually connected
> +	 * just to a dumb lever shorting to ground, but let's mux as GPIO
> +	 * to be on the safe side.
> +	 */
> +	host->cd_gpio = gpiod_get_optional(dev, "cd", GPIOD_IN);
> +	if (IS_ERR(host->cd_gpio))
> +		dev_dbg(dev, "Failed to get cd-gpios: %pe (ignoring)\n", host->cd_gpio);
> +
>  	iores = dev_request_mem_resource(dev, 0);
>  	if (IS_ERR(iores))
>  		return PTR_ERR(iores);
> diff --git a/drivers/mci/dw_mmc.h b/drivers/mci/dw_mmc.h
> index 23fa116d7514..eb5331d1ccb2 100644
> --- a/drivers/mci/dw_mmc.h
> +++ b/drivers/mci/dw_mmc.h
> @@ -53,6 +53,7 @@
>  
>  /* Interrupt Mask register */
>  #define DWMCI_INTMSK_ALL	0xffffffff
> +#define DWMCI_INTMSK_CD		BIT(0)
>  #define DWMCI_INTMSK_RE		BIT(1)
>  #define DWMCI_INTMSK_CDONE	BIT(2)
>  #define DWMCI_INTMSK_DTO	BIT(3)

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register

[Michael Tretter]

On Mon, 09 Feb 2026 17:40:44 +0100, Ahmad Fatoum wrote:
> We don't usually care for the card detect in barebox and just
> unconditionally try to access the card. If it fails, a probe can be
> attempted later via setting mmcX.probe=1.
> 
> The dw_mmc driver diverges from this and enables all kinds
> of interrupts in the interrupt status polled after submitting commands,
> including card-detect.
> 
> This appears to be problematic, when we have a GPIO card detect and are
> not using the card detect hardware function.
> 
> Align behavior with Linux and skip polling for hardware card-detect when
> we have a GPIO card detect. The current behavior was inherited from
> U-Boot, which still does it this way and it's possible it would be affected
> by this issue as well, once U-Boot updates to use DTs >= v6.19-rc1, provided
> that the ROCK 5 support in U-Boot uses CONFIG_OF_UPSTREAM.
> 
> Fixes: a3bf6c16f77d ("dts: update to v6.19-rc1")

Even with this patch, I still observe the DATA ERROR on CMD13 and the
timeouts.

Michael

> Reported-by: Michael Tretter <m.tretter@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/mci/dw_mmc.c | 33 ++++++++++++++++++++++++++++++++-
>  drivers/mci/dw_mmc.h |  1 +
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mci/dw_mmc.c b/drivers/mci/dw_mmc.c
> index 30e5d56944b6..9ddf931eb513 100644
> --- a/drivers/mci/dw_mmc.c
> +++ b/drivers/mci/dw_mmc.c
> @@ -12,6 +12,7 @@
>  #include <driver.h>
>  #include <malloc.h>
>  #include <clock.h>
> +#include <linux/gpio/consumer.h>
>  #include <init.h>
>  #include <mci.h>
>  #include <io.h>
> @@ -23,6 +24,20 @@
>  #include <errno.h>
>  #include "dw_mmc.h"
>  
> +/* Most importantly, it doesn't contain DWMCI_INTMSK_CD as to allow
> + * for GPIO card detect, unlike DWMCI_INTMSK_ALL.
> + * The list is taken from Linux v6.19 with addition of DWMCI_INTMSK_FRUN
> + * as barebox had inherited this into DWMCI_DATA_ERR from U-Boot
> + */
> +#define DW_MCI_DATA_ERROR_FLAGS	(DWMCI_INTMSK_DRTO | DWMCI_INTMSK_DCRC | \
> +				 DWMCI_INTMSK_HTO | DWMCI_INTMSK_SBE  | \
> +				 DWMCI_INTMSK_EBE | DWMCI_INTMSK_HLE | \
> +				 DWMCI_INTMSK_FRUN)
> +#define DW_MCI_CMD_ERROR_FLAGS	(DWMCI_INTMSK_RTO | DWMCI_INTMSK_RCRC | \
> +				 DWMCI_INTMSK_RE | DWMCI_INTMSK_HLE)
> +
> +#define DW_MCI_ERROR_FLAGS	(DW_MCI_DATA_ERROR_FLAGS | \
> +				 DW_MCI_CMD_ERROR_FLAGS)
>  
>  struct dwmci_host {
>  	struct mci_host mci;
> @@ -37,6 +52,7 @@ struct dwmci_host {
>  	u32 fifoth_val;
>  	u32 pwren_value;
>  	dma_addr_t idmac_dma;
> +	struct gpio_desc *cd_gpio;
>  };
>  
>  struct dwmci_idmac {
> @@ -285,7 +301,13 @@ dwmci_cmd(struct mci_host *mci, struct mci_cmd *cmd, struct mci_data *data)
>  		}
>  	}
>  
> -	dwmci_writel(host, DWMCI_RINTSTS, DWMCI_INTMSK_ALL);
> +	/*
> +	 * Enable interrupts for command done, data over, data empty,
> +	 * receive ready and error such as transmit, receive timeout, crc error
> +	 */
> +	dwmci_writel(host, DWMCI_RINTSTS, DWMCI_INTMSK_CDONE | DWMCI_INTMSK_DTO |
> +		   DWMCI_INTMSK_TXDR | DWMCI_INTMSK_RXDR |
> +		   DW_MCI_ERROR_FLAGS);
>  
>  	if (data) {
>  
> @@ -586,6 +608,15 @@ static int dw_mmc_probe(struct device *dev)
>  		reset_control_deassert(rst);
>  	}
>  
> +	/*
> +	 * We don't actually care for the cd-gpios and it's usually connected
> +	 * just to a dumb lever shorting to ground, but let's mux as GPIO
> +	 * to be on the safe side.
> +	 */
> +	host->cd_gpio = gpiod_get_optional(dev, "cd", GPIOD_IN);
> +	if (IS_ERR(host->cd_gpio))
> +		dev_dbg(dev, "Failed to get cd-gpios: %pe (ignoring)\n", host->cd_gpio);
> +
>  	iores = dev_request_mem_resource(dev, 0);
>  	if (IS_ERR(iores))
>  		return PTR_ERR(iores);
> diff --git a/drivers/mci/dw_mmc.h b/drivers/mci/dw_mmc.h
> index 23fa116d7514..eb5331d1ccb2 100644
> --- a/drivers/mci/dw_mmc.h
> +++ b/drivers/mci/dw_mmc.h
> @@ -53,6 +53,7 @@
>  
>  /* Interrupt Mask register */
>  #define DWMCI_INTMSK_ALL	0xffffffff
> +#define DWMCI_INTMSK_CD		BIT(0)
>  #define DWMCI_INTMSK_RE		BIT(1)
>  #define DWMCI_INTMSK_CDONE	BIT(2)
>  #define DWMCI_INTMSK_DTO	BIT(3)
> -- 
> 2.47.3
> 
>