atmel_sdhci: SDMMC_CD pin still needed for card detection despite EMMC set to non-removable

[Eugen Hristev]

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