Re: [PATCH 1/1] MMC: core/core.c: mmc_rescan detects card change in one run

Previous thread: [GIT PULL] ALSA fixes for 2.6.29-rc5 by Takashi Iwai on Thursday, February 19, 2009 - 8:22 am. (1 message)

Next thread: GFS2: Add blktrace support to glocks by Steven Whitehouse on Thursday, February 19, 2009 - 9:55 am. (5 messages)
From: Jorg Schummer
Date: Thursday, February 19, 2009 - 8:26 am

With this patch, mmc_rescan can detect the removal of an mmc card and
the insertion of (possibly another) card in the same run. This means
that a card change can be detected without having to call
mmc_detect_change multiple times.

This change should generalise the core such that it can be easily used
by hosts which provide a mechanism to detect only the presence of a card
reader cover, which has to be taken off in order to insert a card. Other
hosts ("card detect" or "MMC_CAP_NEEDS_POLL") each receive an event when
a card is removed and when a card is inserted, so it is sufficient for
them if mmc_rescan handles only one event at a time. "Cover detect"
hosts, however, only receive events about the cover status. This means
that between 2 subsequent events, both a card removal and a card
insertion can occur. In this case, the current version of mmc_rescan
would only detect the removal of the previous card but not the insertion
of the new card.

This patch has been tested with the "cover detect" mechanism. If you
can, please test it with "card detect" and/or "polling" card readers.

Signed-off-by: Jorg Schummer <ext-jorg.2.schummer@nokia.com>
---
 drivers/mmc/core/core.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index df6ce4a..cd2e29f 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -740,6 +740,22 @@ void mmc_rescan(struct work_struct *work)
 
 	mmc_bus_get(host);
 
+	/* if there is a card registered */
+	if (host->bus_ops != NULL) {
+
+		if (host->bus_ops->detect && !host->bus_dead) {
+
+			/* check whether the card is still present */
+			host->bus_ops->detect(host);
+
+			/* release the bus and update bus status in case
+			   the card was removed */
+			mmc_bus_put(host);
+			mmc_bus_get(host);
+		}
+	}
+
+	/* if there is no card registered */
 	if (host->bus_ops == NULL) {
 		/*
 		 * Only we can add a new handler, so it's safe to
@@ ...
From: Pierre Ossman
Date: Monday, March 2, 2009 - 1:09 pm

On Thu, 19 Feb 2009 17:26:26 +0200

Perhaps it's more clear if you grab the lock for the first section,
release it after and then regrab it for the second section. A bit less

The else section got a bit small here with that code removed. Perhaps
we should instead have:

	if (host->bus_ops != NULL) {
		mmc_bus_put(host);
		goto out;
	}

With some adjusted comments to make the flow clear.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
--

From: =?ISO-8859-1?Q?J=F6rg?= Schummer
Date: Tuesday, March 3, 2009 - 12:24 am

Hello,

thanks for your reply.


Yes, you're probably right, I should take read- and maintainability into

I guess you meant

	else {
		mmc_bus_put(host);
		goto out;
	}

since it would be the else clause of

	if (host->bus_ops == NULL)


Also: Are you sure "goto out" should be added in the else-branch? (Since
it's not present in the end of the corresponding if-branch either, and
is technically not needed.)

Regards,
Jörg


--

From: Pierre Ossman
Date: Tuesday, March 3, 2009 - 1:55 am

On Tue, 03 Mar 2009 09:24:19 +0200

No, I meant to put it further up and unindent the detection code.
Basically you'll be reversing the if-else blocks, but using a goto
instead of an else.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
--

From: Jorg Schummer
Date: Tuesday, March 31, 2009 - 7:51 am

With this patch, mmc_rescan can detect the removal of an mmc card and
the insertion of (possibly another) card in the same run. This means
that a card change can be detected without having to call
mmc_detect_change multiple times.

This change generalises the core such that it can be easily used by
hosts which provide a mechanism to detect only the presence of a card
reader cover, which has to be taken off in order to insert a card. Other
hosts ("card detect" or "MMC_CAP_NEEDS_POLL") each receive an event when
a card is removed and when a card is inserted, so it is sufficient for
them if mmc_rescan handles only one event at a time. "Cover detect"
hosts, however, only receive events about the cover status. This means
that between 2 subsequent events, both a card removal and a card
insertion can occur. In this case, the pre-patch version of mmc_rescan
would only detect the removal of the previous card but not the insertion
of the new card.

Signed-off-by: Jorg Schummer <ext-jorg.2.schummer@nokia.com>
---
 drivers/mmc/core/core.c |   99 ++++++++++++++++++++++++++---------------------
 1 files changed, 55 insertions(+), 44 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index df6ce4a..5970719 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -740,61 +740,72 @@ void mmc_rescan(struct work_struct *work)
 
 	mmc_bus_get(host);
 
-	if (host->bus_ops == NULL) {
-		/*
-		 * Only we can add a new handler, so it's safe to
-		 * release the lock here.
-		 */
+	/* if there is a card registered, check whether it is still present */
+	if ((host->bus_ops != NULL) && host->bus_ops->detect && !host->bus_dead)
+		host->bus_ops->detect(host);
+
+	mmc_bus_put(host);
+
+
+	mmc_bus_get(host);
+
+	/* if there still is a card present, stop here */
+	if (host->bus_ops != NULL) {
 		mmc_bus_put(host);
+		goto out;
+	}
 
-		if (host->ops->get_cd && host->ops->get_cd(host) == 0)
-			goto out;
+	/* detect a newly inserted card */
 ...
From: Pierre Ossman
Date: Friday, April 10, 2009 - 9:39 am

On Tue, 31 Mar 2009 17:51:21 +0300

Queued, thanks.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org
  TigerVNC, core developer          http://www.tigervnc.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
--

From: =?ISO-8859-1?Q?J=F6rg?= Schummer
Date: Wednesday, June 3, 2009 - 2:47 am

Hi Pierre,

is there any news about this patch?

Regards,
Jörg

From: Pierre Ossman
Date: Wednesday, June 3, 2009 - 3:24 am

On Wed, 03 Jun 2009 12:47:48 +0300

It's in my -next branch and will be submitted upstream in the next
merge window.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org
  TigerVNC, core developer          http://www.tigervnc.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
--

Previous thread: [GIT PULL] ALSA fixes for 2.6.29-rc5 by Takashi Iwai on Thursday, February 19, 2009 - 8:22 am. (1 message)

Next thread: GFS2: Add blktrace support to glocks by Steven Whitehouse on Thursday, February 19, 2009 - 9:55 am. (5 messages)