[U-Boot] [PATCH] mmc: implement SDHCI card detect

[Ibai Erkiaga Elorza <IBAIE@xilinx.com>]

Hi Michal,

> -----Original Message-----
> From: Michal Simek <michal.simek@xilinx.com>
> Sent: 27 May 2019 06:53
> To: Ibai Erkiaga Elorza <IBAIE@xilinx.com>; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH] mmc: implement SDHCI card detect
> 
> On 23. 05. 19 15:54, Ibai Erkiaga wrote:
> > Card detect function implemented for SDHCI framework.
> >
> > Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> > ---
> >
> >  drivers/mmc/sdhci.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
> > e2bb90a..cb4db8d 100644
> > --- a/drivers/mmc/sdhci.c
> > +++ b/drivers/mmc/sdhci.c
> > @@ -390,6 +390,21 @@ static int sdhci_send_command(struct mmc *mmc,
> struct mmc_cmd *cmd,
> >  		return -ECOMM;
> >  }
> >
> > +#if IS_ENABLED(CONFIG_DM_MMC)
> > +static int sdhci_get_cd(struct udevice *dev) {
> > +	struct mmc *mmc = mmc_get_mmc_dev(dev); #else static int
> > +sdhci_get_cd(struct mmc *mmc) { #endif
> > +	u32 state;
> > +	struct sdhci_host *host = mmc->priv;
> > +
> > +	state = sdhci_readl(host, SDHCI_PRESENT_STATE);
> > +	return (state & SDHCI_CARD_PRESENT);
> 
> This should be much more robust. It should at least handle cases where you
> have broken-cd, cd-gpios, cd-inverted cases.
> This code will likely work with boards which have CD connected properly but
> it is enabled for all which will cause a lot of issues.
> 
You are right, this implementation is quite basic and the idea was just to get rid-off the polling message for the golden use case (properly connected CD).

> If you want to add this functionality one by one then new Kconfig should be
> used but I am not big fan of that.
> 
The broken-cd Kconfig is already in the MMC framework so I would say that you can use this implementation if your CD is properly connected and just stay as current implementation using CONFIG_MMC_BROKEN_CD.

> > +}
> > +
> >  #if defined(CONFIG_DM_MMC) && defined(MMC_SUPPORTS_TUNING)
> static
> > int sdhci_execute_tuning(struct udevice *dev, uint opcode)  { @@
> > -627,6 +642,7 @@ int sdhci_probe(struct udevice *dev)  const struct
> > dm_mmc_ops sdhci_ops = {
> >  	.send_cmd	= sdhci_send_command,
> >  	.set_ios	= sdhci_set_ios,
> > +	.get_cd		= sdhci_get_cd,
> >  #ifdef MMC_SUPPORTS_TUNING
> >  	.execute_tuning	= sdhci_execute_tuning,
> >  #endif
> > @@ -635,6 +651,7 @@ const struct dm_mmc_ops sdhci_ops = {  static
> > const struct mmc_ops sdhci_ops = {
> >  	.send_cmd	= sdhci_send_command,
> >  	.set_ios	= sdhci_set_ios,
> > +	.get_cd		= sdhci_get_cd,
> >  	.init		= sdhci_init,
> >  };
> >  #endif
> >
> 
> Thanks,
> Michal

Thanks,
Ibai