mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: barebox@lists.infradead.org, Michael Tretter <m.tretter@pengutronix.de>
Subject: Re: [PATCH master] mci: dw_mmc: drop CD from mask programmed into interrupt status register
Date: Mon, 23 Feb 2026 12:40:33 +0100	[thread overview]
Message-ID: <fbe0f299-7942-4599-92b4-d2d5fbbf1e28@pengutronix.de> (raw)
In-Reply-To: <aZw76S9tX_b2J0xM@pengutronix.de>

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 |




  reply	other threads:[~2026-02-23 11:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-09 16:40 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 [this message]
2026-02-23 11:41     ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fbe0f299-7942-4599-92b4-d2d5fbbf1e28@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=m.tretter@pengutronix.de \
    --cc=s.hauer@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox