On 5/1/23 15:21, Zixun Li wrote:

Hi,

...

Can you find some place to set this bit in the atmel sdhci driver, and not in the core? The MC1R register is specific to at91 device.

I've overridden get_cd of the driver, below is the patch:

From e186af71297e9ae6ce241a85bff64683949f0e1b Mon Sep 17 00:00:00 2001 From: Zixun LI zli@ogga.fr Date: Mon, 1 May 2023 14:02:21 +0200 Subject: [PATCH] atmel_sdhci: Force card-detect if MMC_CAP_NONREMOVABLE.

Signed-off-by: Zixun LI zli@ogga.fr

drivers/mmc/atmel_sdhci.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)

diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c index 37b0beeed4..fa32055d1f 100644 --- a/drivers/mmc/atmel_sdhci.c +++ b/drivers/mmc/atmel_sdhci.c @@ -15,6 +15,9 @@ #define ATMEL_SDHC_MIN_FREQ 400000 #define ATMEL_SDHC_GCK_RATE 240000000

+#define ATMEL_SDHC_MC1R 0x204 +#define ATMEL_SDHC_MC1R_FCD 0x80

• #ifndef CONFIG_DM_MMC int atmel_sdhci_init(void *regbase, u32 id) {

@@ -59,8 +62,42 @@ static int atmel_sdhci_deferred_probe(struct sdhci_host *host) return sdhci_probe(dev); }

+static int atmel_sdhci_get_cd(struct sdhci_host *host) +{

•   struct mmc *mmc = host->mmc;
  

•   int value;
  

If you use readb to store things into 'value', you should use a single byte type and not int.

•   /* If nonremovable, assume that the card is always present. */
  

•   if (mmc->cfg->host_caps & MMC_CAP_NONREMOVABLE)
  

•   {
  

•           value = sdhci_readb(host, ATMEL_SDHC_MC1R);
  

•           sdhci_writeb(host, ATMEL_SDHC_MC1R_FCD | value, ATMEL_SDHC_MC1R);
  

Can't we do this in a different place? When we parse the DT for example. If the card is non-removable, this is described as a property in the DT. In this way you don't have to recreate all the below code from the common sdhci_get_cd, and you avoid rewriting the MC1R every time the get_cd is called.

Eugen

•           return 1;
  

•   }
  

•   /* If polling, assume that the card is always present. */
  

•   if (mmc->cfg->host_caps & MMC_CAP_NEEDS_POLL)
  

•           return 1;
  

+#if CONFIG_IS_ENABLED(DM_GPIO)

•   value = dm_gpio_get_value(&host->cd_gpio);
  

•   if (value >= 0) {
  

•           if (mmc->cfg->host_caps & MMC_CAP_CD_ACTIVE_HIGH)
  

•                   return !value;
  

•           else
  

•                   return value;
  

•   }
  

+#endif

•   value = !!(sdhci_readl(host, SDHCI_PRESENT_STATE) &
  

•              SDHCI_CARD_PRESENT);
  

•   if (mmc->cfg->host_caps & MMC_CAP_CD_ACTIVE_HIGH)
  

•           return !value;
  

•   else
  

•           return value;
  

+}

• static const struct sdhci_ops atmel_sdhci_ops = { .deferred_probe = atmel_sdhci_deferred_probe,

•   .get_cd         = atmel_sdhci_get_cd,
  

  };

  static int atmel_sdhci_probe(struct udevice *dev)

-- 2.40.1

Hi Zixun,

I cannot apply your patch, there is some error. can you use

git send-email yourfile.patch --to u-boot@lists.denx.de --to eugen.hristev@collabora.com

such that I can grab it from patchwork ?

Thanks, Eugen

On 5/4/23 17:59, Zixun Li wrote:

...

Can't we do this in a different place? When we parse the DT for example. If the card is non-removable, this is described as a property in the DT. In this way you don't have to recreate all the below code from the common sdhci_get_cd, and you avoid rewriting the MC1R every time the get_cd is called.

The problem is inside sdhci_init() there is a sdhci_reset(), FCD bit will lost if we set it too early.

Then I've looked at the kernel driver, setting this bit is also a workaround of SAMA5D2 series, who doesn't drive CMD if using CD GPIO line.

https://github.com/linux4sam/linux-at91/blob/068eaa6b4b087b3a86fc4624d0f4083...

I've refactored the code, setting FCD bit after sdhci_probe() with a separate function to make the purpose more clear.

From 90177e7af8226b88dfbfc08639f768881562a3a2 Mon Sep 17 00:00:00 2001 From: Zixun LI zli@ogga.fr Date: Thu, 4 May 2023 16:30:41 +0200 Subject: [PATCH] atmel_sdhci: Force card-detect if MMC_CAP_NONREMOVABLE.

---

drivers/mmc/atmel_sdhci.c | 40 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c index 37b0beeed4..ae56266f57 100644 --- a/drivers/mmc/atmel_sdhci.c +++ b/drivers/mmc/atmel_sdhci.c @@ -15,6 +15,9 @@ #define ATMEL_SDHC_MIN_FREQ 400000 #define ATMEL_SDHC_GCK_RATE 240000000

+#define ATMEL_SDHC_MC1R 0x204 +#define ATMEL_SDHC_MC1R_FCD 0x80

• #ifndef CONFIG_DM_MMC int atmel_sdhci_init(void *regbase, u32 id) {

@@ -52,11 +55,38 @@ struct atmel_sdhci_plat { struct mmc mmc; };

+static void atmel_sdhci_config_fcd(struct sdhci_host *host) +{

•   u8 mc1r;
  

•   /* If nonremovable, assume that the card is always present.
  

•    *
  

•    * WA: SAMA5D2 doesn't drive CMD if using CD GPIO line.
  

•    */
  

•   if ((host->mmc->cfg->host_caps & MMC_CAP_NONREMOVABLE)
  

+#if CONFIG_IS_ENABLED(DM_GPIO)

•           || dm_gpio_get_value(&host->cd_gpio) >= 0
  

+#endif

•      )
  

•   {
  

•           sdhci_readb(host, ATMEL_SDHC_MC1R);
  

•           mc1r |= ATMEL_SDHC_MC1R_FCD;
  

•           sdhci_writeb(host, mc1r, ATMEL_SDHC_MC1R);
  

•   }
  

+}

• static int atmel_sdhci_deferred_probe(struct sdhci_host *host) { struct udevice *dev = host->mmc->dev;

•   int ret;
  
  

•   return sdhci_probe(dev);
  

•   ret = sdhci_probe(dev);
  

•   if (ret)
  

•           return ret;
  

•   atmel_sdhci_config_fcd(host);
  

•   return 0;
  

  }

  static const struct sdhci_ops atmel_sdhci_ops = {

@@ -120,7 +150,13 @@ static int atmel_sdhci_probe(struct udevice *dev)

     clk_free(&clk);

•   return sdhci_probe(dev);
  

•   ret = sdhci_probe(dev);
  

•   if (ret)
  

•           return ret;
  

•   atmel_sdhci_config_fcd(host);
  

•   return 0;
  

  }

  static int atmel_sdhci_bind(struct udevice *dev)

-- 2.40.1