Skip to contentSkip to conversation

mmc_spi: add polling support for the card detect line

PATCH In review Anton Vorontsov 4 participants 31 messages May 16, 2008 – Jun 20, 2008

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

PATCH 3/4 mmc_spi: add polling support for the card detect line

Some boards do not use interrupts on the CD line, so we want to poll the CD and see if there was a change. 1 second poll interval seems resonable.

---
 drivers/mmc/host/mmc_spi.c  |   51 +++++++++++++++++++++++++++++++++++++++++-
 include/linux/spi/mmc_spi.h |   10 ++++++++
 2 files changed, 59 insertions(+), 2 deletions(-)
drivers/mmc/host/mmc_spi.c +49 -2
index 3550858..a3b46b1 100644
@@ -31,6 +31,7 @@
 #include <linux/crc7.h>
 #include <linux/crc-itu-t.h>
 #include <linux/scatterlist.h>
+#include <linux/workqueue.h>
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>		/* for R1_SPI_* bit values */
@@ -122,6 +123,11 @@ struct mmc_spi_host {
 
 	struct mmc_spi_platform_data	*pdata;
 
+	/* stores last Card-Detect status (when polling) */
+	int			cd_status;
+	struct workqueue_struct	*cd_poll_wqueue;
+	struct delayed_work	cd_poll_work;
+
 	/* for bulk data transfers */
 	struct spi_transfer	token, t, crc, early_status;
 	struct spi_message	m;
@@ -1155,6 +1161,26 @@ mmc_spi_detect_irq(int irq, void *mmc)
 	return IRQ_HANDLED;
 }
 
+static void mmc_spi_cd_poll_work(struct work_struct *work)
+{
+	struct mmc_spi_host *host = container_of(work, struct mmc_spi_host,
+						 cd_poll_work.work);
+	struct mmc_host *mmc = host->mmc;
+	int old_cd;
+
+	dev_dbg(&host->spi->dev, "polling for card detect...\n");
+
+	old_cd = host->cd_status;
+	host->cd_status = host->pdata->get_cd(mmc->parent);
+	if (old_cd != host->cd_status) {
+		/* ugh... this is ugly, but better than code duplication */
+		mmc_spi_detect_irq(NO_IRQ, mmc);
+	}
+
+	queue_delayed_work(host->cd_poll_wqueue, &host->cd_poll_work,
+			   MMC_SPI_POLL_INT);
+}
+
 struct count_children {
 	unsigned	n;
 	struct bus_type	*bus;
@@ -1323,13 +1349,28 @@ static int mmc_spi_probe(struct spi_device *spi)
 	if (status != 0)
 		goto fail_add_host;
 
-	dev_info(&spi->dev, "SD/MMC host %s%s%s%s\n",
+	if (host->pdata && host->pdata->get_cd) {
+		host->cd_status = host->pdata->get_cd(mmc->parent);
+		INIT_DELAYED_WORK(&host->cd_poll_work, mmc_spi_cd_poll_work);
+		host->cd_poll_wqueue = create_singlethread_workqueue(
+						mmc->class_dev.bus_id);
+		if (!host->cd_poll_wqueue) {
+			status = -ENOMEM;
+			goto fail_add_host;
+		}
+		queue_delayed_work(host->cd_poll_wqueue, &host->cd_poll_work,
+				   MMC_SPI_POLL_INT);
+	}
+
+	dev_info(&spi->dev, "SD/MMC host %s%s%s%s%s\n",
 			mmc->class_dev.bus_id,
 			host->dma_dev ? "" : ", no DMA",
 			(host->pdata && host->pdata->get_ro)
 				? "" : ", no WP",
 			(host->pdata && host->pdata->setpower)
-				? "" : ", no poweroff");
+				? "" : ", no poweroff",
+			(host->pdata && host->pdata->get_cd)
+				? ", cd polling" : "");
 	return 0;
 
 fail_add_host:
@@ -1362,6 +1403,12 @@ static int __devexit mmc_spi_remove(struct spi_device *spi)
 		if (host->pdata && host->pdata->exit)
 			host->pdata->exit(&spi->dev, mmc);
 
+		if (host->pdata && host->pdata->get_cd) {
+			cancel_rearming_delayed_workqueue(
+				host->cd_poll_wqueue, &host->cd_poll_work);
+			destroy_workqueue(host->cd_poll_wqueue);
+		}
+
 		mmc_remove_host(mmc);
 
 		if (host->dma_dev) {
include/linux/spi/mmc_spi.h +10 -0
index e9bbe3e..6ed6ee9 100644
@@ -1,6 +1,10 @@
 #ifndef __LINUX_SPI_MMC_SPI_H
 #define __LINUX_SPI_MMC_SPI_H
 
+#include <asm/param.h> /* for HZ */
+
+#define MMC_SPI_POLL_INT HZ
+
 struct device;
 struct mmc_host;
 
@@ -21,6 +25,12 @@ struct mmc_spi_platform_data {
 	/* sense switch on sd cards */
 	int (*get_ro)(struct device *);
 
+	/*
+	 * if board does not use CD interrupts, driver can poll the CD
+	 * line using this function.
+	 */
+	int (*get_cd)(struct device *);
+
 	/* how long to debounce card detect, in msecs */
 	u16 detect_delay;
 
Signature · 1 line
1.5.5.1
#1 lore raw
Quoted 4 lines · On Fri, 16 May 2008 20:50:57 +0400 Anton Vorontsov…

Some boards do not use interrupts on the CD line, so we want to poll the CD and see if there was a change. 1 second poll interval seems resonable.

The idea isn't bad, but I'm not sure about the mechanism.

To poll a MMC slot, you do not really need a card detect at all. The MMC layer can just shoot off some requests and see if anything responds. The PXA driver (if my memory serves me right) already does this. So the best idea there would be to add this feature to the MMC core and let the host indicate that it needs it via MMC_CAP_NEEDS_POLL or something like that.

The card detection pin then becomes an optimisation, something that is also useful in other ways. Have the host driver check the card detection pin at the start of every request, and quickly fail it if there is no card present.

That should give you what you want with much more flexibility for other uses as well.

Rgds

Signature · 9 lines
     -- Pierre Ossman

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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft 
Defy all challenges. Microsoft(R) Visual Studio 2008. 
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#2 lore raw
Quoted 17 lines · Pierre Ossman
Quoted 4 lines · On Fri, 16 May 2008 20:50:57 +0400 Anton Vorontsov…

Some boards do not use interrupts on the CD line, so we want to poll the CD and see if there was a change. 1 second poll interval seems resonable.

The idea isn't bad, but I'm not sure about the mechanism.

To poll a MMC slot, you do not really need a card detect at all. The MMC layer can just shoot off some requests and see if anything responds. The PXA driver (if my memory serves me right) already does this. So the best idea there would be to add this feature to the MMC core and let the host indicate that it needs it via MMC_CAP_NEEDS_POLL or something like that.

Quoted 4 lines

The card detection pin then becomes an optimisation, something that is also useful in other ways. Have the host driver check the card detection pin at the start of every request, and quickly fail it if there is no card present.

Calling get_cd() for every request smells like overhead, especially given that that get_cd() could ask for GPIO status via relatively slow bus (like I2C GPIO expanders). So, polling seems most reasonable solution here, no need to call it very often.

How about these patches? Tested with and without get_cd() optimization.

Signature · 8 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#3 lore raw

PATCH 1/2 mmc: add support for card-detection polling

Some hosts (and boards that use mmc_spi) do not use interrupts on the CD line, so they can't trigger mmc_detect_change. We want to poll the card and see if there was a change. 1 second poll interval seems resonable.

This patch also implements .get_cd() host operation, that could be used by the hosts that are able to report card-detect status without need to talk MMC.

---
 drivers/mmc/core/core.c  |   19 ++++++++++++++++---
 include/linux/mmc/host.h |    3 +++
 2 files changed, 19 insertions(+), 3 deletions(-)
drivers/mmc/core/core.c +16 -3
index 01ced4c..e455ebd 100644
@@ -631,6 +631,16 @@ void mmc_rescan(struct work_struct *work)
 
 	mmc_bus_get(host);
 
+	if (host->ops->get_cd) {
+		int old_cd_status = host->cd_status;
+
+		host->cd_status = !!host->ops->get_cd(host);
+		if (!(old_cd_status ^ host->cd_status)) {
+			mmc_bus_put(host);
+			goto out;
+		}
+	}
+
 	if (host->bus_ops == NULL) {
 		/*
 		 * Only we can add a new handler, so it's safe to
@@ -652,7 +662,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_sdio(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		/*
@@ -662,7 +672,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_sd(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		/*
@@ -672,7 +682,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_mmc(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		mmc_release_host(host);
@@ -683,6 +693,9 @@ void mmc_rescan(struct work_struct *work)
 
 		mmc_bus_put(host);
 	}
+out:
+	if (host->caps & MMC_CAP_NEEDS_POLL)
+		mmc_schedule_delayed_work(&host->detect, HZ);
 }
 
 void mmc_start_host(struct mmc_host *host)
include/linux/mmc/host.h +3 -0
index 7ab962f..05d03a4 100644
@@ -53,6 +53,7 @@ struct mmc_host_ops {
 	void	(*request)(struct mmc_host *host, struct mmc_request *req);
 	void	(*set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 	int	(*get_ro)(struct mmc_host *host);
+	int	(*get_cd)(struct mmc_host *host);
 	void	(*enable_sdio_irq)(struct mmc_host *host, int enable);
 };
 
@@ -94,6 +95,7 @@ struct mmc_host {
 #define MMC_CAP_SD_HIGHSPEED	(1 << 3)	/* Can do SD high-speed timing */
 #define MMC_CAP_SDIO_IRQ	(1 << 4)	/* Can signal pending SDIO IRQs */
 #define MMC_CAP_SPI		(1 << 5)	/* Talks only SPI protocols */
+#define MMC_CAP_NEEDS_POLL	(1 << 6)	/* Needs polling for card-detection */
 
 	/* host specific block data */
 	unsigned int		max_seg_size;	/* see blk_queue_max_segment_size */
@@ -114,6 +116,7 @@ struct mmc_host {
 	unsigned int		use_spi_crc:1;
 	unsigned int		claimed:1;	/* host exclusively claimed */
 	unsigned int		bus_dead:1;	/* bus has been released */
+	unsigned int		cd_status:1;	/* card-detect status */
 #ifdef CONFIG_MMC_DEBUG
 	unsigned int		removed:1;	/* host is being removed */
 #endif
Signature · 7 lines
1.5.5.1


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#4 lore raw

PATCH 2/2 mmc_spi: add support for card-detection polling

In reply to Anton Vorontsov

If platform_data lacks init() callback (solely used to request card-detect interrupt), we mark the host as MMC_CAP_NEEDS_POLL.

get_cd() host operation provided to optimize polling.

p.s. Since mmc_host_ops no longer the same for every instance of mmc_spi, struct mmc_host_ops can't be const and should be allocated dynamically.

---
 drivers/mmc/host/mmc_spi.c  |   31 +++++++++++++++++++++----------
 include/linux/spi/mmc_spi.h |    6 ++++++
 2 files changed, 27 insertions(+), 10 deletions(-)
drivers/mmc/host/mmc_spi.c +21 -10
index 3550858..667855a 100644
@@ -115,6 +115,7 @@ struct scratch {
 
 struct mmc_spi_host {
 	struct mmc_host		*mmc;
+	struct mmc_host_ops	mmc_spi_ops;
 	struct spi_device	*spi;
 
 	unsigned char		power_mode;
@@ -1131,13 +1132,12 @@ static int mmc_spi_get_ro(struct mmc_host *mmc)
 	return 0;
 }
 
+static int mmc_spi_get_cd(struct mmc_host *mmc)
+{
+	struct mmc_spi_host *host = mmc_priv(mmc);
 
-static const struct mmc_host_ops mmc_spi_ops = {
-	.request	= mmc_spi_request,
-	.set_ios	= mmc_spi_set_ios,
-	.get_ro		= mmc_spi_get_ro,
-};
-
+	return host->pdata->get_cd(mmc->parent);
+}
 
 /****************************************************************************/
 
@@ -1236,8 +1236,12 @@ static int mmc_spi_probe(struct spi_device *spi)
 	mmc = mmc_alloc_host(sizeof(*host), &spi->dev);
 	if (!mmc)
 		goto nomem;
+	host = mmc_priv(mmc);
 
-	mmc->ops = &mmc_spi_ops;
+	host->mmc_spi_ops.request = mmc_spi_request,
+	host->mmc_spi_ops.set_ios = mmc_spi_set_ios,
+	host->mmc_spi_ops.get_ro = mmc_spi_get_ro,
+	mmc->ops = &host->mmc_spi_ops;
 	mmc->max_blk_size = MMC_SPI_BLOCKSIZE;
 
 	/* As long as we keep track of the number of successfully
@@ -1256,7 +1260,6 @@ static int mmc_spi_probe(struct spi_device *spi)
 	mmc->f_min = 400000;
 	mmc->f_max = spi->max_speed_hz;
 
-	host = mmc_priv(mmc);
 	host->mmc = mmc;
 	host->spi = spi;
 
@@ -1323,13 +1326,21 @@ static int mmc_spi_probe(struct spi_device *spi)
 	if (status != 0)
 		goto fail_add_host;
 
-	dev_info(&spi->dev, "SD/MMC host %s%s%s%s\n",
+	if (host->pdata && !host->pdata->init)
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+
+	if (host->pdata && host->pdata->get_cd)
+		host->mmc_spi_ops.get_cd = mmc_spi_get_cd;
+
+	dev_info(&spi->dev, "SD/MMC host %s%s%s%s%s\n",
 			mmc->class_dev.bus_id,
 			host->dma_dev ? "" : ", no DMA",
 			(host->pdata && host->pdata->get_ro)
 				? "" : ", no WP",
 			(host->pdata && host->pdata->setpower)
-				? "" : ", no poweroff");
+				? "" : ", no poweroff",
+			(mmc->caps & MMC_CAP_NEEDS_POLL)
+				? ", cd polling" : "");
 	return 0;
 
 fail_add_host:
include/linux/spi/mmc_spi.h +6 -0
index d5ca78b..915faf3 100644
@@ -23,6 +23,12 @@ struct mmc_spi_platform_data {
 	/* sense switch on sd cards */
 	int (*get_ro)(struct device *);
 
+	/*
+	 * if board does not use CD interrupts, driver can poll the CD
+	 * line using this function.
+	 */
+	int (*get_cd)(struct device *);
+
 	/* how long to debounce card detect, in msecs */
 	u16 detect_delay;
 
Signature · 6 lines
1.5.5.1

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#5 lore raw
In reply to Anton Vorontsov
Quoted 6 lines · On Wed, 21 May 2008 22:47:13 +0400 Anton Vorontsov…

Calling get_cd() for every request smells like overhead, especially given that that get_cd() could ask for GPIO status via relatively slow bus (like I2C GPIO expanders). So, polling seems most reasonable solution here, no need to call it very often.

Fair enough. You should probably add a comment about this somewhere so that people do not call get_cd() in the core request function and similar places. Place it so that both get_cd() and get_ro() are covered though, as it should be relevant for both.

Quoted 2 lines

How about these patches? Tested with and without get_cd() optimization.

Very nice. But I have some concerns:

Review struct mmc_host_ops { · line 58

Quoted 7 lines
(quoted diff) +1 -0
@@ -53,6 +53,7 @@ struct mmc_host_ops {
 	void	(*request)(struct mmc_host *host, struct mmc_request *req);
 	void	(*set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 	int	(*get_ro)(struct mmc_host *host);
+	int	(*get_cd)(struct mmc_host *host);
 	void	(*enable_sdio_irq)(struct mmc_host *host, int enable);
 };

I'm always nervous when it comes to adding more callbacks, but I suppose I've resisted this one for far too long already.

Review struct mmc_host_ops {

Quoted 10 lines
(quoted diff) +10 -0
+	if (host->ops->get_cd) {
+		int old_cd_status = host->cd_status;
+
+		host->cd_status = !!host->ops->get_cd(host);
+		if (!(old_cd_status ^ host->cd_status)) {
+			mmc_bus_put(host);
+			goto out;
+		}
+	}
+

This should only be done when there is no bus handler. Since we are polling, we might actually miss the user removing and reinserting the card. The only way to check for that is to poke the card and see if it is still alive. This also means you won't need that state variable.

Also, that second if clause seems fit for an obfuscation contest. ;)

Quoted 3 lines

p.s. Since mmc_host_ops no longer the same for every instance of mmc_spi, struct mmc_host_ops can't be const and should be allocated dynamically.

This can be solved by allowing get_cd() to return an error that will be treated as if get_cd() wasn't defined. -ENODEV seems suitable.

(get_ro() needs the same treatment, but I haven't gotten around to that)

Rgds

Signature · 9 lines
     -- Pierre Ossman

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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#6 lore raw
Quoted 10 lines · Pierre Ossman
Quoted 6 lines · On Wed, 21 May 2008 22:47:13 +0400 Anton Vorontsov <[email protected]>…

Calling get_cd() for every request smells like overhead, especially given that that get_cd() could ask for GPIO status via relatively slow bus (like I2C GPIO expanders). So, polling seems most reasonable solution here, no need to call it very often.

Quoted 4 lines

Fair enough. You should probably add a comment about this somewhere so that people do not call get_cd() in the core request function and similar places. Place it so that both get_cd() and get_ro() are covered though, as it should be relevant for both.

I think this is applicable to the .set_ios() too.

[...]

Quoted 7 lines
Quoted 7 lines
(quoted diff) +7 -0
+	if (host->ops->get_cd) {
+		int old_cd_status = host->cd_status;
+
+		host->cd_status = !!host->ops->get_cd(host);
+		if (!(old_cd_status ^ host->cd_status)) {
+			mmc_bus_put(host);
+			goto out;
Quoted 8 lines

drivers/mmc/host/mmc_spi.c mmc_spi_detect_irq(int irq, void *mmc) · line 1183

Quoted 3 lines
drivers/mmc/host/mmc_spi.c +3 -0
+		}
+	}
+

This should only be done when there is no bus handler. Since we are polling, we might actually miss the user removing and reinserting the card. The only way to check for that is to poke the card and see if it is still alive. This also means you won't need that state variable.

Yeah, this makes sense. Indeed pretty easy to trigger [if poll interval increased to 3 seconds, for example].

Quoted 1 line

Also, that second if clause seems fit for an obfuscation contest. ;)

cd_status was a bitfield, so I thought that bit operations would be appropriate. :-)

Quoted 6 lines
Quoted 3 lines

p.s. Since mmc_host_ops no longer the same for every instance of mmc_spi, struct mmc_host_ops can't be const and should be allocated dynamically.

This can be solved by allowing get_cd() to return an error that will be treated as if get_cd() wasn't defined. -ENODEV seems suitable.

-ENOSYS (not implemented) sounds better for this purpose...

Quoted 2 lines

(get_ro() needs the same treatment, but I haven't gotten around to that)

Ok. How about this version?

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#7 lore raw

PATCH 1/3 mmc: add support for card-detection polling

Some hosts (and boards that use mmc_spi) do not use interrupts on the CD line, so they can't trigger mmc_detect_change. We want to poll the card and see if there was a change. 1 second poll interval seems resonable.

This patch also implements .get_cd() host operation, that could be used by the hosts that are able to report card-detect status without need to talk MMC.

---
 drivers/mmc/core/core.c  |   12 +++++++++---
 include/linux/mmc/host.h |    8 ++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)
drivers/mmc/core/core.c +9 -3
index 01ced4c..ede5d1e 100644
@@ -638,6 +638,9 @@ void mmc_rescan(struct work_struct *work)
 		 */
 		mmc_bus_put(host);
 
+		if (host->ops->get_cd && host->ops->get_cd(host) == 0)
+			goto out;
+
 		mmc_claim_host(host);
 
 		mmc_power_up(host);
@@ -652,7 +655,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_sdio(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		/*
@@ -662,7 +665,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_sd(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		/*
@@ -672,7 +675,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_mmc(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		mmc_release_host(host);
@@ -683,6 +686,9 @@ void mmc_rescan(struct work_struct *work)
 
 		mmc_bus_put(host);
 	}
+out:
+	if (host->caps & MMC_CAP_NEEDS_POLL)
+		mmc_schedule_delayed_work(&host->detect, HZ);
 }
 
 void mmc_start_host(struct mmc_host *host)
include/linux/mmc/host.h +8 -0
index 7ab962f..f2e9885 100644
@@ -51,8 +51,15 @@ struct mmc_ios {
 
 struct mmc_host_ops {
 	void	(*request)(struct mmc_host *host, struct mmc_request *req);
+	/*
+	 * Avoid calling these three functions too often or in a "fast path",
+	 * since underlaying controller might implement them in an expensive
+	 * and/or slow way.
+	 */
 	void	(*set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 	int	(*get_ro)(struct mmc_host *host);
+	int	(*get_cd)(struct mmc_host *host);
+
 	void	(*enable_sdio_irq)(struct mmc_host *host, int enable);
 };
 
@@ -94,6 +101,7 @@ struct mmc_host {
 #define MMC_CAP_SD_HIGHSPEED	(1 << 3)	/* Can do SD high-speed timing */
 #define MMC_CAP_SDIO_IRQ	(1 << 4)	/* Can signal pending SDIO IRQs */
 #define MMC_CAP_SPI		(1 << 5)	/* Talks only SPI protocols */
+#define MMC_CAP_NEEDS_POLL	(1 << 6)	/* Needs polling for card-detection */
 
 	/* host specific block data */
 	unsigned int		max_seg_size;	/* see blk_queue_max_segment_size */
Signature · 7 lines
1.5.5.1


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#8 lore raw

PATCH 2/3 mmc_spi: add support for card-detection polling

In reply to Anton Vorontsov

If platform_data lacks init() callback (solely used to request card-detect interrupt), we mark the host as MMC_CAP_NEEDS_POLL.

get_cd() host operation provided to optimize polling.

---
 drivers/mmc/host/mmc_spi.c  |   18 ++++++++++++++++--
 include/linux/spi/mmc_spi.h |    6 ++++++
 2 files changed, 22 insertions(+), 2 deletions(-)
drivers/mmc/host/mmc_spi.c +16 -2
index 3550858..f0f86b1 100644
@@ -1131,11 +1131,20 @@ static int mmc_spi_get_ro(struct mmc_host *mmc)
 	return 0;
 }
 
+static int mmc_spi_get_cd(struct mmc_host *mmc)
+{
+	struct mmc_spi_host *host = mmc_priv(mmc);
+
+	if (host->pdata && host->pdata->get_cd)
+		return host->pdata->get_cd(mmc->parent);
+	return -ENOSYS;
+}
 
 static const struct mmc_host_ops mmc_spi_ops = {
 	.request	= mmc_spi_request,
 	.set_ios	= mmc_spi_set_ios,
 	.get_ro		= mmc_spi_get_ro,
+	.get_cd		= mmc_spi_get_cd,
 };
 
 
@@ -1323,13 +1332,18 @@ static int mmc_spi_probe(struct spi_device *spi)
 	if (status != 0)
 		goto fail_add_host;
 
-	dev_info(&spi->dev, "SD/MMC host %s%s%s%s\n",
+	if (host->pdata && !host->pdata->init)
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+
+	dev_info(&spi->dev, "SD/MMC host %s%s%s%s%s\n",
 			mmc->class_dev.bus_id,
 			host->dma_dev ? "" : ", no DMA",
 			(host->pdata && host->pdata->get_ro)
 				? "" : ", no WP",
 			(host->pdata && host->pdata->setpower)
-				? "" : ", no poweroff");
+				? "" : ", no poweroff",
+			(mmc->caps & MMC_CAP_NEEDS_POLL)
+				? ", cd polling" : "");
 	return 0;
 
 fail_add_host:
include/linux/spi/mmc_spi.h +6 -0
index d5ca78b..23c6923 100644
@@ -23,6 +23,12 @@ struct mmc_spi_platform_data {
 	/* sense switch on sd cards */
 	int (*get_ro)(struct device *);
 
+	/*
+	 * If board does not use CD interrupts, driver can optimize polling
+	 * using this function.
+	 */
+	int (*get_cd)(struct device *);
+
 	/* how long to debounce card detect, in msecs */
 	u16 detect_delay;
 
Signature · 7 lines
1.5.5.1


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#9 lore raw

PATCH 3/3 mmc: change .get_ro() callback semantics

In reply to Anton Vorontsov

get_ro() callback must return values >= 0 for its logical state, and negative errno values in case of error.

If particular host instance doesn't support RO/WP switch, it should return -ENOSYS.

This patch changes some hosts in two ways:

1. Now functions should be smart to not return negative values in "RO asserted" case (particularly gpio_ calls could return negative values for the outermost GPIOs).

Also, board code usually passes get_ro() callbacks that directly return gpioreg & bit result, so imxmmc, pxamci and mmc_spi's get_ro() handlers need take special care when returning platform's values to the mmc core.

2. In case of host instance didn't implement get_ro() callback, it should really return -ENOSYS and let the mmc core decide what to do about it (mmc core thinks the same way as the hosts, so it isn't functional change).

---
 drivers/mmc/core/sd.c       |    4 ++--
 drivers/mmc/host/at91_mci.c |    2 +-
 drivers/mmc/host/imxmmc.c   |    9 ++++++---
 drivers/mmc/host/mmc_spi.c  |    9 ++++++---
 drivers/mmc/host/pxamci.c   |    9 ++++++---
 include/linux/mmc/host.h    |    3 +++
 6 files changed, 24 insertions(+), 12 deletions(-)
drivers/mmc/core/sd.c +2 -2
index 7ef3b15..b122eb9 100644
@@ -494,13 +494,13 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 	 * Check if read-only switch is active.
 	 */
 	if (!oldcard) {
-		if (!host->ops->get_ro) {
+		if (!host->ops->get_ro || host->ops->get_ro(host) < 0) {
 			printk(KERN_WARNING "%s: host does not "
 				"support reading read-only "
 				"switch. assuming write-enable.\n",
 				mmc_hostname(host));
 		} else {
-			if (host->ops->get_ro(host))
+			if (host->ops->get_ro(host) > 0)
 				mmc_card_set_readonly(card);
 		}
 	}
drivers/mmc/host/at91_mci.c +1 -1
index a28fc2f..a2ccdf2 100644
@@ -794,7 +794,7 @@ static int at91_mci_get_ro(struct mmc_host *mmc)
 	struct at91mci_host *host = mmc_priv(mmc);
 
 	if (host->board->wp_pin) {
-		read_only = gpio_get_value(host->board->wp_pin);
+		read_only = !!gpio_get_value(host->board->wp_pin);
 		printk(KERN_WARNING "%s: card is %s\n", mmc_hostname(mmc),
 				(read_only ? "read-only" : "read-write") );
 	}
drivers/mmc/host/imxmmc.c +6 -3
index 95f33e8..5167679 100644
@@ -889,9 +889,12 @@ static int imxmci_get_ro(struct mmc_host *mmc)
 	struct imxmci_host *host = mmc_priv(mmc);
 
 	if (host->pdata && host->pdata->get_ro)
-		return host->pdata->get_ro(mmc_dev(mmc));
-	/* Host doesn't support read only detection so assume writeable */
-	return 0;
+		return !!host->pdata->get_ro(mmc_dev(mmc));
+	/*
+	 * Board doesn't support read only detection; let the mmc core
+	 * decide what to do.
+	 */
+	return -ENOSYS;
 }
 
 
drivers/mmc/host/mmc_spi.c +6 -3
index f0f86b1..a804c7f 100644
@@ -1126,9 +1126,12 @@ static int mmc_spi_get_ro(struct mmc_host *mmc)
 	struct mmc_spi_host *host = mmc_priv(mmc);
 
 	if (host->pdata && host->pdata->get_ro)
-		return host->pdata->get_ro(mmc->parent);
-	/* board doesn't support read only detection; assume writeable */
-	return 0;
+		return !!host->pdata->get_ro(mmc->parent);
+	/*
+	 * Board doesn't support read only detection; let the mmc core
+	 * decide what to do.
+	 */
+	return -ENOSYS;
 }
 
 static int mmc_spi_get_cd(struct mmc_host *mmc)
drivers/mmc/host/pxamci.c +6 -3
index 65210fc..b6056bd 100644
@@ -361,9 +361,12 @@ static int pxamci_get_ro(struct mmc_host *mmc)
 	struct pxamci_host *host = mmc_priv(mmc);
 
 	if (host->pdata && host->pdata->get_ro)
-		return host->pdata->get_ro(mmc_dev(mmc));
-	/* Host doesn't support read only detection so assume writeable */
-	return 0;
+		return !!host->pdata->get_ro(mmc_dev(mmc));
+	/*
+	 * Board doesn't support read only detection; let the mmc core
+	 * decide what to do.
+	 */
+	return -ENOSYS;
 }
 
 static void pxamci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
include/linux/mmc/host.h +3 -0
index f2e9885..ef3b773 100644
@@ -55,6 +55,9 @@ struct mmc_host_ops {
 	 * Avoid calling these three functions too often or in a "fast path",
 	 * since underlaying controller might implement them in an expensive
 	 * and/or slow way.
+	 *
+	 * .get_ro and .get_cd should return >= 0 for their logical values,
+	 * or negative errno value in case of error.
 	 */
 	void	(*set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 	int	(*get_ro)(struct mmc_host *host);
Signature · 1 line
1.5.5.1
#10 lore raw
In reply to Anton Vorontsov
Quoted 9 lines · On Thu, 22 May 2008 22:17:13 +0400 Anton Vorontsov…
Quoted 5 lines · Pierre Ossman

Fair enough. You should probably add a comment about this somewhere so that people do not call get_cd() in the core request function and similar places. Place it so that both get_cd() and get_ro() are covered though, as it should be relevant for both.

I think this is applicable to the .set_ios() too.

Indeed. That tends to be more obvious that it is costly though.

Quoted 6 lines
Quoted 3 lines

This can be solved by allowing get_cd() to return an error that will be treated as if get_cd() wasn't defined. -ENODEV seems suitable.

-ENOSYS (not implemented) sounds better for this purpose...

Even better. :)

Quoted 5 lines
Quoted 2 lines

(get_ro() needs the same treatment, but I haven't gotten around to that)

Ok. How about this version?

Perfect. Unless there is some way you can pass flags in platform data? Using the lack of an init() to determine the need for polling is a bit circumstantial.

Rgds

Signature · 9 lines
     -- Pierre Ossman

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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#11 lore raw

On Thu, May 22, 2008 at 09:34:32PM +0200, Pierre Ossman wrote:
[...]

Quoted 6 lines
Quoted 2 lines

Ok. How about this version?

Perfect. Unless there is some way you can pass flags in platform data? Using the lack of an init() to determine the need for polling is a bit circumstantial.

Aha, that's an idea, thanks. Implemented in the updated patches.

p.s.
I think, in case of capabilities, it's users' responsibility to not issue bogus flags, so mmc_spi doesn't mask anything, it just passes platform's caps. This also avoids drivers update when (if) new caps will be implemented.

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#12 lore raw

PATCH 1/3 mmc: add support for card-detection polling

Some hosts (and boards that use mmc_spi) do not use interrupts on the CD line, so they can't trigger mmc_detect_change. We want to poll the card and see if there was a change. 1 second poll interval seems resonable.

This patch also implements .get_cd() host operation, that could be used by the hosts that are able to report card-detect status without need to talk MMC.

---
 drivers/mmc/core/core.c  |   12 +++++++++---
 include/linux/mmc/host.h |    8 ++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)
drivers/mmc/core/core.c +9 -3
index 01ced4c..ede5d1e 100644
@@ -638,6 +638,9 @@ void mmc_rescan(struct work_struct *work)
 		 */
 		mmc_bus_put(host);
 
+		if (host->ops->get_cd && host->ops->get_cd(host) == 0)
+			goto out;
+
 		mmc_claim_host(host);
 
 		mmc_power_up(host);
@@ -652,7 +655,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_sdio(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		/*
@@ -662,7 +665,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_sd(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		/*
@@ -672,7 +675,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_mmc(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		mmc_release_host(host);
@@ -683,6 +686,9 @@ void mmc_rescan(struct work_struct *work)
 
 		mmc_bus_put(host);
 	}
+out:
+	if (host->caps & MMC_CAP_NEEDS_POLL)
+		mmc_schedule_delayed_work(&host->detect, HZ);
 }
 
 void mmc_start_host(struct mmc_host *host)
include/linux/mmc/host.h +8 -0
index 7ab962f..f2e9885 100644
@@ -51,8 +51,15 @@ struct mmc_ios {
 
 struct mmc_host_ops {
 	void	(*request)(struct mmc_host *host, struct mmc_request *req);
+	/*
+	 * Avoid calling these three functions too often or in a "fast path",
+	 * since underlaying controller might implement them in an expensive
+	 * and/or slow way.
+	 */
 	void	(*set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 	int	(*get_ro)(struct mmc_host *host);
+	int	(*get_cd)(struct mmc_host *host);
+
 	void	(*enable_sdio_irq)(struct mmc_host *host, int enable);
 };
 
@@ -94,6 +101,7 @@ struct mmc_host {
 #define MMC_CAP_SD_HIGHSPEED	(1 << 3)	/* Can do SD high-speed timing */
 #define MMC_CAP_SDIO_IRQ	(1 << 4)	/* Can signal pending SDIO IRQs */
 #define MMC_CAP_SPI		(1 << 5)	/* Talks only SPI protocols */
+#define MMC_CAP_NEEDS_POLL	(1 << 6)	/* Needs polling for card-detection */
 
 	/* host specific block data */
 	unsigned int		max_seg_size;	/* see blk_queue_max_segment_size */
Signature · 1 line
1.5.5.1
#13 lore raw

Re: [PATCH 1/3] mmc: add support for card-detection polling

Quoted 10 lines · On Fri, 23 May 2008 19:43:40 +0400 Anton Vorontsov…

Some hosts (and boards that use mmc_spi) do not use interrupts on the CD line, so they can't trigger mmc_detect_change. We want to poll the card and see if there was a change. 1 second poll interval seems resonable.

This patch also implements .get_cd() host operation, that could be used by the hosts that are able to report card-detect status without need to talk MMC.

---

Applied the whole lot. Thanks. :)

Rgds

Signature · 9 lines
     -- Pierre Ossman

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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#14 lore raw

PATCH 2/3 mmc_spi: add support for card-detection polling

In reply to Anton Vorontsov

This patch adds new platform data variable "caps", so platforms could pass theirs capabilities into MMC core (for example, platforms without interrupt on the CD line will most probably want to pass MMC_CAP_NEEDS_POLL).

New platform get_cd() callback provided to optimize polling.

---
 drivers/mmc/host/mmc_spi.c  |   19 +++++++++++++++++--
 include/linux/spi/mmc_spi.h |    9 +++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)
drivers/mmc/host/mmc_spi.c +17 -2
index 3550858..724870c 100644
@@ -1131,11 +1131,20 @@ static int mmc_spi_get_ro(struct mmc_host *mmc)
 	return 0;
 }
 
+static int mmc_spi_get_cd(struct mmc_host *mmc)
+{
+	struct mmc_spi_host *host = mmc_priv(mmc);
+
+	if (host->pdata && host->pdata->get_cd)
+		return host->pdata->get_cd(mmc->parent);
+	return -ENOSYS;
+}
 
 static const struct mmc_host_ops mmc_spi_ops = {
 	.request	= mmc_spi_request,
 	.set_ios	= mmc_spi_set_ios,
 	.get_ro		= mmc_spi_get_ro,
+	.get_cd		= mmc_spi_get_cd,
 };
 
 
@@ -1319,17 +1328,23 @@ static int mmc_spi_probe(struct spi_device *spi)
 			goto fail_glue_init;
 	}
 
+	/* pass platform capabilities, if any */
+	if (host->pdata)
+		mmc->caps |= host->pdata->caps;
+
 	status = mmc_add_host(mmc);
 	if (status != 0)
 		goto fail_add_host;
 
-	dev_info(&spi->dev, "SD/MMC host %s%s%s%s\n",
+	dev_info(&spi->dev, "SD/MMC host %s%s%s%s%s\n",
 			mmc->class_dev.bus_id,
 			host->dma_dev ? "" : ", no DMA",
 			(host->pdata && host->pdata->get_ro)
 				? "" : ", no WP",
 			(host->pdata && host->pdata->setpower)
-				? "" : ", no poweroff");
+				? "" : ", no poweroff",
+			(mmc->caps & MMC_CAP_NEEDS_POLL)
+				? ", cd polling" : "");
 	return 0;
 
 fail_add_host:
include/linux/spi/mmc_spi.h +9 -0
index d5ca78b..a3626ae 100644
@@ -23,6 +23,15 @@ struct mmc_spi_platform_data {
 	/* sense switch on sd cards */
 	int (*get_ro)(struct device *);
 
+	/*
+	 * If board does not use CD interrupts, driver can optimize polling
+	 * using this function.
+	 */
+	int (*get_cd)(struct device *);
+
+	/* Capabilities to pass into mmc core (e.g. MMC_CAP_NEEDS_POLL). */
+	unsigned long caps;
+
 	/* how long to debounce card detect, in msecs */
 	u16 detect_delay;
 
Signature · 7 lines
1.5.5.1


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#15 lore raw

PATCH 3/3 mmc: change .get_ro() callback semantics

In reply to Anton Vorontsov

get_ro() callback must return values >= 0 for its logical state, and negative errno values in case of error.

If particular host instance doesn't support RO/WP switch, it should return -ENOSYS.

This patch changes some hosts in two ways:

1. Now functions should be smart to not return negative values in "RO asserted" case (particularly gpio_ calls could return negative values for the outermost GPIOs).

Also, board code usually passes get_ro() callbacks that directly return gpioreg & bit result, so imxmmc, pxamci and mmc_spi's get_ro() handlers need take special care when returning platform's values to the mmc core.

2. In case of host instance didn't implement get_ro() callback, it should really return -ENOSYS and let the mmc core decide what to do about it (mmc core thinks the same way as the hosts, so it isn't functional change).

---
 drivers/mmc/core/sd.c       |    4 ++--
 drivers/mmc/host/at91_mci.c |    2 +-
 drivers/mmc/host/imxmmc.c   |    9 ++++++---
 drivers/mmc/host/mmc_spi.c  |    9 ++++++---
 drivers/mmc/host/pxamci.c   |    9 ++++++---
 include/linux/mmc/host.h    |    3 +++
 6 files changed, 24 insertions(+), 12 deletions(-)
drivers/mmc/core/sd.c +2 -2
index 7ef3b15..b122eb9 100644
@@ -494,13 +494,13 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 	 * Check if read-only switch is active.
 	 */
 	if (!oldcard) {
-		if (!host->ops->get_ro) {
+		if (!host->ops->get_ro || host->ops->get_ro(host) < 0) {
 			printk(KERN_WARNING "%s: host does not "
 				"support reading read-only "
 				"switch. assuming write-enable.\n",
 				mmc_hostname(host));
 		} else {
-			if (host->ops->get_ro(host))
+			if (host->ops->get_ro(host) > 0)
 				mmc_card_set_readonly(card);
 		}
 	}
drivers/mmc/host/at91_mci.c +1 -1
index 8979ad3..140c2b8 100644
@@ -797,7 +797,7 @@ static int at91_mci_get_ro(struct mmc_host *mmc)
 	struct at91mci_host *host = mmc_priv(mmc);
 
 	if (host->board->wp_pin) {
-		read_only = gpio_get_value(host->board->wp_pin);
+		read_only = !!gpio_get_value(host->board->wp_pin);
 		printk(KERN_WARNING "%s: card is %s\n", mmc_hostname(mmc),
 				(read_only ? "read-only" : "read-write") );
 	}
drivers/mmc/host/imxmmc.c +6 -3
index 95f33e8..5167679 100644
@@ -889,9 +889,12 @@ static int imxmci_get_ro(struct mmc_host *mmc)
 	struct imxmci_host *host = mmc_priv(mmc);
 
 	if (host->pdata && host->pdata->get_ro)
-		return host->pdata->get_ro(mmc_dev(mmc));
-	/* Host doesn't support read only detection so assume writeable */
-	return 0;
+		return !!host->pdata->get_ro(mmc_dev(mmc));
+	/*
+	 * Board doesn't support read only detection; let the mmc core
+	 * decide what to do.
+	 */
+	return -ENOSYS;
 }
 
 
drivers/mmc/host/mmc_spi.c +6 -3
index 724870c..85d9853 100644
@@ -1126,9 +1126,12 @@ static int mmc_spi_get_ro(struct mmc_host *mmc)
 	struct mmc_spi_host *host = mmc_priv(mmc);
 
 	if (host->pdata && host->pdata->get_ro)
-		return host->pdata->get_ro(mmc->parent);
-	/* board doesn't support read only detection; assume writeable */
-	return 0;
+		return !!host->pdata->get_ro(mmc->parent);
+	/*
+	 * Board doesn't support read only detection; let the mmc core
+	 * decide what to do.
+	 */
+	return -ENOSYS;
 }
 
 static int mmc_spi_get_cd(struct mmc_host *mmc)
drivers/mmc/host/pxamci.c +6 -3
index 65210fc..b6056bd 100644
@@ -361,9 +361,12 @@ static int pxamci_get_ro(struct mmc_host *mmc)
 	struct pxamci_host *host = mmc_priv(mmc);
 
 	if (host->pdata && host->pdata->get_ro)
-		return host->pdata->get_ro(mmc_dev(mmc));
-	/* Host doesn't support read only detection so assume writeable */
-	return 0;
+		return !!host->pdata->get_ro(mmc_dev(mmc));
+	/*
+	 * Board doesn't support read only detection; let the mmc core
+	 * decide what to do.
+	 */
+	return -ENOSYS;
 }
 
 static void pxamci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
include/linux/mmc/host.h +3 -0
index f2e9885..ef3b773 100644
@@ -55,6 +55,9 @@ struct mmc_host_ops {
 	 * Avoid calling these three functions too often or in a "fast path",
 	 * since underlaying controller might implement them in an expensive
 	 * and/or slow way.
+	 *
+	 * .get_ro and .get_cd should return >= 0 for their logical values,
+	 * or negative errno value in case of error.
 	 */
 	void	(*set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 	int	(*get_ro)(struct mmc_host *host);
Signature · 6 lines
1.5.5.1

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#16 lore raw

Re: [PATCH 3/3] mmc: change .get_ro() callback semantics

Hi all!

Quoted 1 line · Anton Vorontsov

get_ro() callback must return values >= 0 for its logical state, and

...

include/linux/mmc/host.h struct mmc_host_ops { · line 61

Quoted 13 lines

static void pxamci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

include/linux/mmc/host.h +3 -0
index f2e9885..ef3b773 100644
@@ -55,6 +55,9 @@ struct mmc_host_ops {
 	 * Avoid calling these three functions too often or in a "fast path",
 	 * since underlaying controller might implement them in an expensive
 	 * and/or slow way.
+	 *
+	 * .get_ro and .get_cd should return >= 0 for their logical values,
+	 * or negative errno value in case of error.
 	 */

I would suggest to use something more strict (bulletproof), something like:

/*
* get_ro will return:

 *   0 for a read/write card
 *   1 for a read-only card 
 *   -ENOSYS when not supported
 *   or a negative errno when something bad happened
 * 
 * get_cd will return:
 *   0 for a absent card
 *   1 for a present card 
 *   -ENOSYS when not supported
 *   or a negative errno when something bad happened

*/

I think we have missed one important information: which context these callbacks can rely on (hard_irq, soft_irq, ...).

Best regards

Marc

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#17 lore raw

Re: [PATCH 3/3] mmc: change .get_ro() callback semantics

Quoted 21 lines · Marc Pignat

Hi all!

Quoted 1 line · Anton Vorontsov

get_ro() callback must return values >= 0 for its logical state, and

...

include/linux/mmc/host.h struct mmc_host_ops { · line 61

Quoted 13 lines

static void pxamci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

include/linux/mmc/host.h +3 -0
index f2e9885..ef3b773 100644
@@ -55,6 +55,9 @@ struct mmc_host_ops {
 	 * Avoid calling these three functions too often or in a "fast path",
 	 * since underlaying controller might implement them in an expensive
 	 * and/or slow way.
+	 *
+	 * .get_ro and .get_cd should return >= 0 for their logical values,
+	 * or negative errno value in case of error.
 	 */

I would suggest to use something more strict (bulletproof), something like:

Quoted 4 lines

/*
* get_ro will return:
* 0 for a read/write card
* 1 for a read-only card

This isn't always practical. For example, host is using u8 register for the status, so it might safely return u8 & mask, that will always fit into int. Or very smart/adventurous authors might be aware that, for the particular host, mask's bit isn't last, and safely do uXX & mask. :-)

The above is weak argument of course, since it is about optimization.

As an counter-evidence, the strict scheme that you described probably less error prone. But is it? To implement it we'll need something like WARN_ON(ret > 0 && ret != 1) to catch erroneous users. Take a closer look though, will it catch uXX & lastbit case? Nope. :-)

We can catch bogus users though... via hack (_assuming_ that there are no errno values of 1 << (sizeof(int) * 8 - 1)), i.e. WARN_ON(ret == (1 << (sizeof(int) * 8 - 1)). Though, to do so, we don't need the strict scheme, this debugging hack will work in the current scheme too.

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#18 lore raw

Re: [PATCH 3/3] mmc: change .get_ro() callback semantics

Quoted 27 lines · Anton Vorontsov
Quoted 21 lines · Marc Pignat

Hi all!

Quoted 1 line · Anton Vorontsov

get_ro() callback must return values >= 0 for its logical state, and

...

include/linux/mmc/host.h struct mmc_host_ops { · line 61

Quoted 13 lines

static void pxamci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

include/linux/mmc/host.h +3 -0
index f2e9885..ef3b773 100644
@@ -55,6 +55,9 @@ struct mmc_host_ops {
 	 * Avoid calling these three functions too often or in a "fast path",
 	 * since underlaying controller might implement them in an expensive
 	 * and/or slow way.
+	 *
+	 * .get_ro and .get_cd should return >= 0 for their logical values,
+	 * or negative errno value in case of error.
 	 */

I would suggest to use something more strict (bulletproof), something like:

Quoted 4 lines

/*
* get_ro will return:
* 0 for a read/write card
* 1 for a read-only card

Quoted 6 lines

This isn't always practical. For example, host is using u8 register for the status, so it might safely return u8 & mask, that will always fit into int. Or very smart/adventurous authors might be aware that, for the particular host, mask's bit isn't last, and safely do uXX & mask. :-)

The above is weak argument of course, since it is about optimization.

Ack, we will gain at most 1-4 assembly instructions, in a function that is unlikely to be called more than once a second.

Quoted 5 lines

As an counter-evidence, the strict scheme that you described probably less error prone. But is it? To implement it we'll need something like WARN_ON(ret > 0 && ret != 1) to catch erroneous users. Take a closer look though, will it catch uXX & lastbit case? Nope. :-)

WARN_ON(ret > 0 && ret != 1 || ret == INT_MIN) will do ;)

I agree with you once more, I never thinked about a runtime check.

I don't really want to see a WARN_ON(foo) after each call to get_ro or get_cd.

But I'm sure if we specify "give me a positive value when a card is detected", someone will write gpio & bit, and three years later, someone will fall in the (gpio & lastbit < 0 case).

So we should specify: "give me 1 whan a card is present, 0 when not, -ENOSYS if you don't know and a negative errno when something else goes wrong".

Best regards

Marc

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php

#19 lore raw

PATCH 1/1 mmc: toughen get_ro() and get_cd() return values

For the sake of safety, document that drivers should return only 1 or 0 from the get_ro() and get_cd() callbacks. Also document context in which these callbacks should be executed.

wbsd driver modified to comply with this requirement.

Also, fix mmc_spi driver to not return raw values from the platform get_cd hook (oops).

---

On Thu, Jun 05, 2008 at 05:58:59PM +0200, Marc Pignat wrote:
[...]

Quoted 24 lines
Quoted 10 lines
Quoted 3 lines

* get_ro will return:
* 0 for a read/write card
* 1 for a read-only card

This isn't always practical. For example, host is using u8 register for the status, so it might safely return u8 & mask, that will always fit into int. Or very smart/adventurous authors might be aware that, for the particular host, mask's bit isn't last, and safely do uXX & mask. :-)

The above is weak argument of course, since it is about optimization.

Ack, we will gain at most 1-4 assembly instructions, in a function that is unlikely to be called more than once a second.

Quoted 5 lines

As an counter-evidence, the strict scheme that you described probably less error prone. But is it? To implement it we'll need something like WARN_ON(ret > 0 && ret != 1) to catch erroneous users. Take a closer look though, will it catch uXX & lastbit case? Nope. :-)

WARN_ON(ret > 0 && ret != 1 || ret == INT_MIN) will do ;)

I agree with you once more, I never thinked about a runtime check.

Quoted 8 lines

I don't really want to see a WARN_ON(foo) after each call to get_ro or get_cd.

But I'm sure if we specify "give me a positive value when a card is detected", someone will write gpio & bit, and three years later, someone will fall in the (gpio & lastbit < 0 case).

So we should specify: "give me 1 whan a card is present, 0 when not, -ENOSYS if you don't know and a negative errno when something else goes wrong".

Well, ok.

Pierre, I see you didn't yet pushed out the mmc tree, so.. would you prefer this patch folded into 0/3 series and resent?

 drivers/mmc/host/mmc_spi.c |    2 +-
 drivers/mmc/host/wbsd.c    |    2 +-
 include/linux/mmc/host.h   |   16 ++++++++++++++--
 3 files changed, 16 insertions(+), 4 deletions(-)
drivers/mmc/host/mmc_spi.c +1 -1
index 85d9853..4e82f64 100644
@@ -1139,7 +1139,7 @@ static int mmc_spi_get_cd(struct mmc_host *mmc)
 	struct mmc_spi_host *host = mmc_priv(mmc);
 
 	if (host->pdata && host->pdata->get_cd)
-		return host->pdata->get_cd(mmc->parent);
+		return !!host->pdata->get_cd(mmc->parent);
 	return -ENOSYS;
 }
 
drivers/mmc/host/wbsd.c +1 -1
index be624a0..9283b85 100644
@@ -939,7 +939,7 @@ static int wbsd_get_ro(struct mmc_host *mmc)
 
 	spin_unlock_bh(&host->lock);
 
-	return csr & WBSD_WRPT;
+	return !!(csr & WBSD_WRPT);
 }
 
 static const struct mmc_host_ops wbsd_ops = {
include/linux/mmc/host.h +14 -2
index ef3b773..753b723 100644
@@ -56,8 +56,20 @@ struct mmc_host_ops {
 	 * since underlaying controller might implement them in an expensive
 	 * and/or slow way.
 	 *
-	 * .get_ro and .get_cd should return >= 0 for their logical values,
-	 * or negative errno value in case of error.
+	 * Also note that these functions might sleep, so don't call them
+	 * in the atomic contexts!
+	 *
+	 * Return values for the get_ro callback should be:
+	 *   0 for a read/write card
+	 *   1 for a read-only card
+	 *   -ENOSYS when not supported (equal to NULL callback)
+	 *   or a negative errno value when something bad happened
+	 *
+	 * Return values for the get_ro callback should be:
+	 *   0 for a absent card
+	 *   1 for a present card
+	 *   -ENOSYS when not supported (equal to NULL callback)
+	 *   or a negative errno value when something bad happened
 	 */
 	void	(*set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 	int	(*get_ro)(struct mmc_host *host);
Signature · 1 line
1.5.5.1
#20 lore raw

Re: [PATCH] mmc: toughen get_ro() and get_cd() return values

Quoted 4 lines · On Thu, 5 Jun 2008 21:10:13 +0400 Anton Vorontsov…

Pierre, I see you didn't yet pushed out the mmc tree, so.. would you prefer this patch folded into 0/3 series and resent?

This works fine. :)

rgds

Signature · 14 lines
     -- 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. Use end-to-end encryption where
  possible.

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
#21 lore raw

Re: [PATCH] mmc: toughen get_ro() and get_cd() return values

Quoted 9 lines · Pierre Ossman
Quoted 4 lines · On Thu, 5 Jun 2008 21:10:13 +0400 Anton Vorontsov <[email protected]>…

Pierre, I see you didn't yet pushed out the mmc tree, so.. would you prefer this patch folded into 0/3 series and resent?

This works fine. :)

Great, patches follows. Also added missing changes for the at91_mci host. Now, I hope, every host is really converted.

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#22 lore raw

PATCH 1/3 mmc: add support for card-detection polling

Some hosts (and boards that use mmc_spi) do not use interrupts on the CD line, so they can't trigger mmc_detect_change. We want to poll the card and see if there was a change. 1 second poll interval seems resonable.

This patch also implements .get_cd() host operation, that could be used by the hosts that are able to report card-detect status without need to talk MMC.

---
 drivers/mmc/core/core.c  |   12 +++++++++---
 include/linux/mmc/host.h |   11 +++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)
drivers/mmc/core/core.c +9 -3
index 01ced4c..ede5d1e 100644
@@ -638,6 +638,9 @@ void mmc_rescan(struct work_struct *work)
 		 */
 		mmc_bus_put(host);
 
+		if (host->ops->get_cd && host->ops->get_cd(host) == 0)
+			goto out;
+
 		mmc_claim_host(host);
 
 		mmc_power_up(host);
@@ -652,7 +655,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_sdio(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		/*
@@ -662,7 +665,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_sd(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		/*
@@ -672,7 +675,7 @@ void mmc_rescan(struct work_struct *work)
 		if (!err) {
 			if (mmc_attach_mmc(host, ocr))
 				mmc_power_off(host);
-			return;
+			goto out;
 		}
 
 		mmc_release_host(host);
@@ -683,6 +686,9 @@ void mmc_rescan(struct work_struct *work)
 
 		mmc_bus_put(host);
 	}
+out:
+	if (host->caps & MMC_CAP_NEEDS_POLL)
+		mmc_schedule_delayed_work(&host->detect, HZ);
 }
 
 void mmc_start_host(struct mmc_host *host)
include/linux/mmc/host.h +11 -0
index 7ab962f..6188e19 100644
@@ -51,8 +51,18 @@ struct mmc_ios {
 
 struct mmc_host_ops {
 	void	(*request)(struct mmc_host *host, struct mmc_request *req);
+	/*
+	 * Avoid calling these three functions too often or in a "fast path",
+	 * since underlaying controller might implement them in an expensive
+	 * and/or slow way.
+	 *
+	 * Also note that these functions might sleep, so don't call them
+	 * in the atomic contexts!
+	 */
 	void	(*set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 	int	(*get_ro)(struct mmc_host *host);
+	int	(*get_cd)(struct mmc_host *host);
+
 	void	(*enable_sdio_irq)(struct mmc_host *host, int enable);
 };
 
@@ -94,6 +104,7 @@ struct mmc_host {
 #define MMC_CAP_SD_HIGHSPEED	(1 << 3)	/* Can do SD high-speed timing */
 #define MMC_CAP_SDIO_IRQ	(1 << 4)	/* Can signal pending SDIO IRQs */
 #define MMC_CAP_SPI		(1 << 5)	/* Talks only SPI protocols */
+#define MMC_CAP_NEEDS_POLL	(1 << 6)	/* Needs polling for card-detection */
 
 	/* host specific block data */
 	unsigned int		max_seg_size;	/* see blk_queue_max_segment_size */
Signature · 1 line
1.5.5.4
#23 lore raw

PATCH 2/3 mmc_spi: add support for card-detection polling

In reply to Anton Vorontsov

This patch adds new platform data variable "caps", so platforms could pass theirs capabilities into MMC core (for example, platforms without interrupt on the CD line will most probably want to pass MMC_CAP_NEEDS_POLL).

New platform get_cd() callback provided to optimize polling.

---
 drivers/mmc/host/mmc_spi.c  |   19 +++++++++++++++++--
 include/linux/spi/mmc_spi.h |    9 +++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)
drivers/mmc/host/mmc_spi.c +17 -2
index 3550858..547eb85 100644
@@ -1131,11 +1131,20 @@ static int mmc_spi_get_ro(struct mmc_host *mmc)
 	return 0;
 }
 
+static int mmc_spi_get_cd(struct mmc_host *mmc)
+{
+	struct mmc_spi_host *host = mmc_priv(mmc);
+
+	if (host->pdata && host->pdata->get_cd)
+		return !!host->pdata->get_cd(mmc->parent);
+	return -ENOSYS;
+}
 
 static const struct mmc_host_ops mmc_spi_ops = {
 	.request	= mmc_spi_request,
 	.set_ios	= mmc_spi_set_ios,
 	.get_ro		= mmc_spi_get_ro,
+	.get_cd		= mmc_spi_get_cd,
 };
 
 
@@ -1319,17 +1328,23 @@ static int mmc_spi_probe(struct spi_device *spi)
 			goto fail_glue_init;
 	}
 
+	/* pass platform capabilities, if any */
+	if (host->pdata)
+		mmc->caps |= host->pdata->caps;
+
 	status = mmc_add_host(mmc);
 	if (status != 0)
 		goto fail_add_host;
 
-	dev_info(&spi->dev, "SD/MMC host %s%s%s%s\n",
+	dev_info(&spi->dev, "SD/MMC host %s%s%s%s%s\n",
 			mmc->class_dev.bus_id,
 			host->dma_dev ? "" : ", no DMA",
 			(host->pdata && host->pdata->get_ro)
 				? "" : ", no WP",
 			(host->pdata && host->pdata->setpower)
-				? "" : ", no poweroff");
+				? "" : ", no poweroff",
+			(mmc->caps & MMC_CAP_NEEDS_POLL)
+				? ", cd polling" : "");
 	return 0;
 
 fail_add_host:
include/linux/spi/mmc_spi.h +9 -0
index d5ca78b..a3626ae 100644
@@ -23,6 +23,15 @@ struct mmc_spi_platform_data {
 	/* sense switch on sd cards */
 	int (*get_ro)(struct device *);
 
+	/*
+	 * If board does not use CD interrupts, driver can optimize polling
+	 * using this function.
+	 */
+	int (*get_cd)(struct device *);
+
+	/* Capabilities to pass into mmc core (e.g. MMC_CAP_NEEDS_POLL). */
+	unsigned long caps;
+
 	/* how long to debounce card detect, in msecs */
 	u16 detect_delay;
 
Signature · 1 line
1.5.5.4
#24 lore raw

PATCH 3/3 mmc: change .get_ro() callback semantics

In reply to Anton Vorontsov

Now get_ro() callback must return 0/1 values for its logical states, and negative errno values in case of error. If particular host instance doesn't support RO/WP switch, it should return -ENOSYS.

This patch changes some hosts in two ways:

1. Now functions should be smart to not return negative values in "RO asserted" case (particularly gpio_ calls could return negative values for the outermost GPIOs).

Also, board code usually passes get_ro() callbacks that directly return gpioreg & bit result, so at91_mci, imxmmc, pxamci and mmc_spi's get_ro() handlers need take special care when returning platform's values to the mmc core.

2. In case of host instance didn't implement get_ro() callback, it should really return -ENOSYS and let the mmc core decide what to do about it (mmc core thinks the same way as the hosts, so it isn't functional change).

---
 drivers/mmc/core/sd.c       |    4 ++--
 drivers/mmc/host/at91_mci.c |   18 +++++++-----------
 drivers/mmc/host/imxmmc.c   |    9 ++++++---
 drivers/mmc/host/mmc_spi.c  |    9 ++++++---
 drivers/mmc/host/pxamci.c   |    9 ++++++---
 drivers/mmc/host/wbsd.c     |    2 +-
 include/linux/mmc/host.h    |   12 ++++++++++++
 7 files changed, 40 insertions(+), 23 deletions(-)
drivers/mmc/core/sd.c +2 -2
index 7ef3b15..b122eb9 100644
@@ -494,13 +494,13 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 	 * Check if read-only switch is active.
 	 */
 	if (!oldcard) {
-		if (!host->ops->get_ro) {
+		if (!host->ops->get_ro || host->ops->get_ro(host) < 0) {
 			printk(KERN_WARNING "%s: host does not "
 				"support reading read-only "
 				"switch. assuming write-enable.\n",
 				mmc_hostname(host));
 		} else {
-			if (host->ops->get_ro(host))
+			if (host->ops->get_ro(host) > 0)
 				mmc_card_set_readonly(card);
 		}
 	}
drivers/mmc/host/at91_mci.c +7 -11
index 8979ad3..b9d4ed6 100644
@@ -793,19 +793,15 @@ static irqreturn_t at91_mmc_det_irq(int irq, void *_host)
 
 static int at91_mci_get_ro(struct mmc_host *mmc)
 {
-	int read_only = 0;
 	struct at91mci_host *host = mmc_priv(mmc);
 
-	if (host->board->wp_pin) {
-		read_only = gpio_get_value(host->board->wp_pin);
-		printk(KERN_WARNING "%s: card is %s\n", mmc_hostname(mmc),
-				(read_only ? "read-only" : "read-write") );
-	}
-	else {
-		printk(KERN_WARNING "%s: host does not support reading read-only "
-				"switch.  Assuming write-enable.\n", mmc_hostname(mmc));
-	}
-	return read_only;
+	if (host->board->wp_pin)
+		return !!gpio_get_value(host->board->wp_pin);
+	/*
+	 * Board doesn't support read only detection; let the mmc core
+	 * decide what to do.
+	 */
+	return -ENOSYS;
 }
 
 static const struct mmc_host_ops at91_mci_ops = {
drivers/mmc/host/imxmmc.c +6 -3
index 95f33e8..5167679 100644
@@ -889,9 +889,12 @@ static int imxmci_get_ro(struct mmc_host *mmc)
 	struct imxmci_host *host = mmc_priv(mmc);
 
 	if (host->pdata && host->pdata->get_ro)
-		return host->pdata->get_ro(mmc_dev(mmc));
-	/* Host doesn't support read only detection so assume writeable */
-	return 0;
+		return !!host->pdata->get_ro(mmc_dev(mmc));
+	/*
+	 * Board doesn't support read only detection; let the mmc core
+	 * decide what to do.
+	 */
+	return -ENOSYS;
 }
 
 
drivers/mmc/host/mmc_spi.c +6 -3
index 547eb85..4e82f64 100644
@@ -1126,9 +1126,12 @@ static int mmc_spi_get_ro(struct mmc_host *mmc)
 	struct mmc_spi_host *host = mmc_priv(mmc);
 
 	if (host->pdata && host->pdata->get_ro)
-		return host->pdata->get_ro(mmc->parent);
-	/* board doesn't support read only detection; assume writeable */
-	return 0;
+		return !!host->pdata->get_ro(mmc->parent);
+	/*
+	 * Board doesn't support read only detection; let the mmc core
+	 * decide what to do.
+	 */
+	return -ENOSYS;
 }
 
 static int mmc_spi_get_cd(struct mmc_host *mmc)
drivers/mmc/host/pxamci.c +6 -3
index 65210fc..b6056bd 100644
@@ -361,9 +361,12 @@ static int pxamci_get_ro(struct mmc_host *mmc)
 	struct pxamci_host *host = mmc_priv(mmc);
 
 	if (host->pdata && host->pdata->get_ro)
-		return host->pdata->get_ro(mmc_dev(mmc));
-	/* Host doesn't support read only detection so assume writeable */
-	return 0;
+		return !!host->pdata->get_ro(mmc_dev(mmc));
+	/*
+	 * Board doesn't support read only detection; let the mmc core
+	 * decide what to do.
+	 */
+	return -ENOSYS;
 }
 
 static void pxamci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
drivers/mmc/host/wbsd.c +1 -1
index be624a0..9283b85 100644
@@ -939,7 +939,7 @@ static int wbsd_get_ro(struct mmc_host *mmc)
 
 	spin_unlock_bh(&host->lock);
 
-	return csr & WBSD_WRPT;
+	return !!(csr & WBSD_WRPT);
 }
 
 static const struct mmc_host_ops wbsd_ops = {
include/linux/mmc/host.h +12 -0
index 6188e19..753b723 100644
@@ -58,6 +58,18 @@ struct mmc_host_ops {
 	 *
 	 * Also note that these functions might sleep, so don't call them
 	 * in the atomic contexts!
+	 *
+	 * Return values for the get_ro callback should be:
+	 *   0 for a read/write card
+	 *   1 for a read-only card
+	 *   -ENOSYS when not supported (equal to NULL callback)
+	 *   or a negative errno value when something bad happened
+	 *
+	 * Return values for the get_ro callback should be:
+	 *   0 for a absent card
+	 *   1 for a present card
+	 *   -ENOSYS when not supported (equal to NULL callback)
+	 *   or a negative errno value when something bad happened
 	 */
 	void	(*set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 	int	(*get_ro)(struct mmc_host *host);
Signature · 1 line
1.5.5.4
#25 lore raw

Re: [PATCH] mmc: toughen get_ro() and get_cd() return values

In reply to Anton Vorontsov
Quoted 4 lines · On Tue, 17 Jun 2008 18:16:07 +0400 Anton Vorontsov <[email protected]>…

Great, patches follows. Also added missing changes for the at91_mci host. Now, I hope, every host is really converted.

Applied the new set, thanks.

Signature · 8 lines
     -- 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. Use end-to-end encryption where
  possible.
#26 lore raw

RE: [PATCH 3/4] [MMC] mmc_spi: add polling support for the card detectline

In reply to Anton Vorontsov
Quoted 13 lines

-----Original Message-----
From: linuxppc-dev-bounces+b11801=freescale.com-mnsaURCQ41sdnm+yROfE0A@public.gmane.org [mailto:linuxppc-dev-bounces+b11801=freescale.com-mnsaURCQ41sdnm+yROfE0A@public.gmane.org] On Behalf Of Anton Vorontsov
Sent: 2008?5?17? 0:51
To: Kumar Gala; David Brownell; Pierre Ossman
Cc: [email protected]; spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; [email protected]; Tabi Timur
Subject: [PATCH 3/4] [MMC] mmc_spi: add polling support for
the card detectline

Quoted 7 lines

Some boards do not use interrupts on the CD line, so we want to poll the CD and see if there was a change. 1 second poll interval seems resonable.

---
 drivers/mmc/host/mmc_spi.c  |   51 

I want to know whether every card working well ? Because I'm working For 83xx MMC-over-SPI support, the presented concrete methods I used as you do,
but only 256M card seems good, 512M or 1G/2G card all have some problems

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#27 lore raw

Re: [PATCH 3/4] [MMC] mmc_spi: add polling support for the card detectline

Quoted 17 lines · Chen Gong
Quoted 17 lines

-----Original Message-----
From: linuxppc-dev-bounces+b11801=freescale.com-mnsaURCQ41sdnm+yROfE0A@public.gmane.org [mailto:linuxppc-dev-bounces+b11801=freescale.com-mnsaURCQ41sdnm+yROfE0A@public.gmane.org] On Behalf Of Anton Vorontsov
Sent: 2008?5?17? 0:51
To: Kumar Gala; David Brownell; Pierre Ossman
Cc: [email protected]; spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; [email protected]; Tabi Timur
Subject: [PATCH 3/4] [MMC] mmc_spi: add polling support for
the card detectline

Some boards do not use interrupts on the CD line, so we want to poll the CD and see if there was a change. 1 second poll interval seems resonable.

Quoted 8 lines
Quoted 3 lines
---
 drivers/mmc/host/mmc_spi.c  |   51 

I want to know whether every card working well ? Because I'm working For 83xx MMC-over-SPI support, the presented concrete methods I used as you do,
but only 256M card seems good, 512M or 1G/2G card all have some problems

So far I don't have cards larger than 256M. Will try to get one though.

Thanks,

Signature · 8 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#28 lore raw

RE: [PATCH 3/4] [MMC] mmc_spi: add polling support for the carddetectline

Quoted 40 lines

-----Original Message-----
From: Anton Vorontsov [mailto:[email protected]]
Sent: 2008?5?22? 20:39
To: Chen Gong
Cc: Kumar Gala; David Brownell; Pierre Ossman; [email protected]; spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; [email protected]; Tabi Timur
Subject: Re: [PATCH 3/4] [MMC] mmc_spi: add polling support
for the carddetectline

Quoted 11 lines · Chen Gong
Quoted 11 lines

-----Original Message-----
From: linuxppc-dev-bounces+b11801=freescale.com-mnsaURCQ41sdnm+yROfE0A@public.gmane.org [mailto:linuxppc-dev-bounces+b11801=freescale.com-mnsaURCQ41sdnm+yROfE0A@public.gmane.org] On Behalf Of Anton Vorontsov
Sent: 2008?5?17? 0:51
To: Kumar Gala; David Brownell; Pierre Ossman
Cc: [email protected]; spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; [email protected]; Tabi Timur
Subject: [PATCH 3/4] [MMC] mmc_spi: add polling support

for the card

Quoted 3 lines
Quoted 3 lines

detectline

Some boards do not use interrupts on the CD line, so we

want to poll

Quoted 1 line
Quoted 1 line

the CD and see if there was a change. 1 second poll

interval seems

Quoted 7 lines
Quoted 5 lines

resonable.

---
 drivers/mmc/host/mmc_spi.c  |   51 

I want to know whether every card working well ? Because

I'm working

Quoted 9 lines
Quoted 1 line

For 83xx MMC-over-SPI support, the presented concrete

methods I used

Quoted 1 line

as you do, but only 256M card seems good, 512M or 1G/2G

card all have

Quoted 1 line

some problems

So far I don't have cards larger than 256M. Will try to get one though.

Thanks, if you have any new test report pls contact me.

BTW, a fews days ago Joakim Tjernlund made a heavy fix for spi_mpc83xx.c to resolve clock glitch. But it looks no any impact for me, but maybe it has positive effect for you.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#29 lore raw

Re: [PATCH 3/4] [MMC] mmc_spi: add polling support for the carddetectline

On Thu, May 22, 2008 at 09:44:57PM +0800, Chen Gong wrote:
[...]

Quoted 4 lines
Quoted 4 lines
Quoted 1 line

I want to know whether every card working well ? Because

I'm working

Quoted 1 line

For 83xx MMC-over-SPI support, the presented concrete

methods I used

Quoted 8 lines
Quoted 7 lines
Quoted 1 line

as you do, but only 256M card seems good, 512M or 1G/2G

card all have

Quoted 1 line

some problems

So far I don't have cards larger than 256M. Will try to get one though.

Thanks, if you have any new test report pls contact me.

Just tested with Apacer 1GB MicroSD (connected through dumb adapter), it works quite well here. I've successfully wrote 10 mb file onto ext2 formatted partition...

Quoted 3 lines

BTW, a fews days ago Joakim Tjernlund made a heavy fix for spi_mpc83xx.c to resolve clock glitch. But it looks no any impact for me, but maybe it has positive effect for you.

Yup, I use it.

Btw, which mmc driver are you using? The one from MPC8610HPCD BSP or mainline's mmc_spi?

Signature · 8 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#30 lore raw

RE: [PATCH 3/4] [MMC] mmc_spi: add polling support for thecarddetectline

Quoted 23 lines

-----Original Message-----
From: Anton Vorontsov [mailto:[email protected]]
Sent: 2008?5?26? 23:38
To: Chen Gong
Cc: Kumar Gala; David Brownell; Pierre Ossman; [email protected]; spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; [email protected]; Tabi Timur
Subject: Re: [PATCH 3/4] [MMC] mmc_spi: add polling support
for thecarddetectline

On Thu, May 22, 2008 at 09:44:57PM +0800, Chen Gong wrote:
[...]

Quoted 8 lines
Quoted 8 lines
Quoted 1 line

I want to know whether every card working well ? Because

I'm working

Quoted 1 line

For 83xx MMC-over-SPI support, the presented concrete

methods I used

Quoted 1 line

as you do, but only 256M card seems good, 512M or 1G/2G

card all have

Quoted 1 line

some problems

Quoted 8 lines
Quoted 4 lines
Quoted 3 lines

So far I don't have cards larger than 256M. Will try to get one though.

Thanks, if you have any new test report pls contact me.

Just tested with Apacer 1GB MicroSD (connected through dumb adapter), it works quite well here. I've successfully wrote 10 mb file onto ext2 formatted partition...

Oh, thanks for your info. The card I used is Kingston 1G SD card.

Quoted 10 lines
Quoted 3 lines

BTW, a fews days ago Joakim Tjernlund made a heavy fix for spi_mpc83xx.c to resolve clock glitch. But it looks no any

impact for

Quoted 1 line

me, but maybe it has positive effect for you.

Yup, I use it.

Btw, which mmc driver are you using? The one from MPC8610HPCD BSP or mainline's mmc_spi?

:-( the mmc driver now in the MPC8610HPCD BSP is ported from uClinux
by me, it has many problems, because at that time MMC-over-SPI driver
doen't exist in the kernel.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/