Re: [PATCH 3/3] MMC/SD: add callback function to detect card

[Matt Fleming <matt@console-pimps.org>]

On Tue, Aug 31, 2010 at 10:23:23PM +0300, Adrian Hunter wrote:
> Matt Fleming wrote:
> >On Fri, Aug 27, 2010 at 08:05:13PM +0100, Chris Ball wrote:
> >>Hi,
> >>
> >>On Fri, Dec 04, 2009 at 03:55:09PM +0800, r66093@freescale.com wrote:
> >>>From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >>>
> >>>Add callback function to check if the card has been removed.
> >>>
> >>>in order to check if the card has been removed, the function mmc_send_status will send commad CMD13 to card and ask the card to send its status register to driver, which will generate interrupt repeatly and make the system bad.
> >>>Therefore, get_cd callback is used to detect the card if the driver has.
> >>>
> >>>Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >>>---
> >>> drivers/mmc/core/mmc.c |    5 ++++-
> >>> drivers/mmc/core/sd.c  |    5 ++++-
> >>> 2 files changed, 8 insertions(+), 2 deletions(-)
> >>>
> >>>diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> >>>index 06084db..c5c676d 100644
> >>>--- a/drivers/mmc/core/mmc.c
> >>>+++ b/drivers/mmc/core/mmc.c
> >>>@@ -494,7 +494,10 @@ static void mmc_detect(struct mmc_host *host)
> >>> 	/*
> >>> 	 * Just check if our card has been removed.
> >>> 	 */
> >>>-	err = mmc_send_status(host->card, NULL);
> >>>+	if (host->ops->get_cd)
> >>>+		err = !host->ops->get_cd(host);
> >>>+	else
> >>>+		err = mmc_send_status(host->card, NULL);
> >>> 	mmc_release_host(host);
> >>>diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> >>>index cd81c39..3cf1f38 100644
> >>>--- a/drivers/mmc/core/sd.c
> >>>+++ b/drivers/mmc/core/sd.c
> >>>@@ -548,7 +548,10 @@ static void mmc_sd_detect(struct mmc_host *host)
> >>> 	/*
> >>> 	 * Just check if our card has been removed.
> >>> 	 */
> >>>-	err = mmc_send_status(host->card, NULL);
> >>>+	if (host->ops->get_cd)
> >>>+		err = !host->ops->get_cd(host);
> >>>+	else
> >>>+		err = mmc_send_status(host->card, NULL);
> >>> 	mmc_release_host(host);
> >>>-- 
> >>>1.6.4
> >>This patchset wasn't replied to -- any comments on it from the list?
> >
> >I think this change makes sense, although it needs to be a bit smarter
> >with error handling because the get_cd() functions can return -ENOSYS if
> >they are not implemented (but the function pointer is still valid).
> >
> >	int err = 0;
> >
> >	if (host->ops->get_cd) {
> >		err = host->ops->get_cd(host);
> >		if (err >= 0)
> >			err = !err;
> >	}
> >
> >	if (!host->ops->get_cd || err < 0)
> >		err = mmc_send_status(host->card, NULL);
> >
> >Thoughts?
> 
> I am not sure it won't cause problems.
> 
> If you change the card then card detect will return true because
> there is a card but send status will fail because the card is not
> initialised.  So the two do not seem equivalent.

Are you sure that the SEND_STATUS will fail if the card hasn't been
initialised? We don't actually look at the response from the SEND_STATUS
command, just whether the command itself failed or succeeded. Even if
the card isn't initialised, the command should succeed. And when we
return from mmc_rescan() we'll initialise the card anyway.

I'm not sure it won't cause problems either and I'd feel a lot more
confident if someone could test it on their hardware though ;-) None of
my boards implement the get_cd function.