* [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register
@ 2026-02-09 16:40 Ahmad Fatoum
2026-02-09 16:45 ` Ahmad Fatoum
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2026-02-09 16:40 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum, Michael Tretter
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register
2026-02-09 16:40 [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register Ahmad Fatoum
@ 2026-02-09 16:45 ` Ahmad Fatoum
2026-02-10 8:29 ` Michael Tretter
2026-02-23 11:37 ` Sascha Hauer
2 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2026-02-09 16:45 UTC (permalink / raw)
To: barebox; +Cc: Michael Tretter
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 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register
2026-02-09 16:40 [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register Ahmad Fatoum
2026-02-09 16:45 ` Ahmad Fatoum
@ 2026-02-10 8:29 ` Michael Tretter
2026-02-23 11:37 ` Sascha Hauer
2 siblings, 0 replies; 6+ messages in thread
From: Michael Tretter @ 2026-02-10 8:29 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox
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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register
2026-02-09 16:40 [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register Ahmad Fatoum
2026-02-09 16:45 ` Ahmad Fatoum
2026-02-10 8:29 ` Michael Tretter
@ 2026-02-23 11:37 ` Sascha Hauer
2026-02-23 11:40 ` Ahmad Fatoum
2 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2026-02-23 11:37 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox, Michael Tretter
On Mon, Feb 09, 2026 at 05:40:44PM +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")
> 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(-)
Given that this doesn't resolve Michaels issue I won't apply it for
master. Anyway, the patch might still be correct, so what should I do
with it?
Sascha
>
> 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
>
>
>
--
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 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register
2026-02-23 11:37 ` Sascha Hauer
@ 2026-02-23 11:40 ` Ahmad Fatoum
2026-02-23 11:41 ` Sascha Hauer
0 siblings, 1 reply; 6+ messages in thread
From: Ahmad Fatoum @ 2026-02-23 11:40 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox, Michael Tretter
Hello Sascha,
On 2/23/26 12:37 PM, Sascha Hauer wrote:
> On Mon, Feb 09, 2026 at 05:40:44PM +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")
>> 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(-)
>
> Given that this doesn't resolve Michaels issue I won't apply it for
> master. Anyway, the patch might still be correct, so what should I do
> with it?
Let's leave it unapplied for now until the reported issue is fixed (I
heard you have access to the HW in question, so I am hopful :-).
Once the original issue is fixed, this can be applied of top if it was
not found to be breaking anything.
Cheers,
Ahmad
>
> Sascha
>
>>
>> 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
>>
>>
>>
>
--
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 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register
2026-02-23 11:40 ` Ahmad Fatoum
@ 2026-02-23 11:41 ` Sascha Hauer
0 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2026-02-23 11:41 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox, Michael Tretter
On Mon, Feb 23, 2026 at 12:40:33PM +0100, Ahmad Fatoum wrote:
> Hello Sascha,
>
> On 2/23/26 12:37 PM, Sascha Hauer wrote:
> > On Mon, Feb 09, 2026 at 05:40:44PM +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")
> >> 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(-)
> >
> > Given that this doesn't resolve Michaels issue I won't apply it for
> > master. Anyway, the patch might still be correct, so what should I do
> > with it?
>
> Let's leave it unapplied for now until the reported issue is fixed (I
> heard you have access to the HW in question, so I am hopful :-).
Indeed :)
>
> Once the original issue is fixed, this can be applied of top if it was
> not found to be breaking anything.
Ok.
Sascha
--
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 |
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-02-23 11:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-09 16:40 [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register Ahmad Fatoum
2026-02-09 16:45 ` Ahmad Fatoum
2026-02-10 8:29 ` Michael Tretter
2026-02-23 11:37 ` Sascha Hauer
2026-02-23 11:40 ` Ahmad Fatoum
2026-02-23 11:41 ` Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox