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

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

[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.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/mmc_spi.c  |   51 +++++++++++++++++++++++++++++++++++++++++-
 include/linux/spi/mmc_spi.h |   10 ++++++++
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 3550858..a3b46b1 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -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) {
diff --git a/include/linux/spi/mmc_spi.h b/include/linux/spi/mmc_spi.h
index e9bbe3e..6ed6ee9 100644
--- a/include/linux/spi/mmc_spi.h
+++ b/include/linux/spi/mmc_spi.h
@@ -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;
 
-- 
1.5.5.1

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

[Pierre Ossman]

On Fri, 16 May 2008 20:50:57 +0400
Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org> wrote:

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

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

[Chen Gong]

> -----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: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A@public.gmane.org; 
> spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; 
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 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.
> 
> Signed-off-by: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
> ---
>  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/

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

[Anton Vorontsov]

On Sat, May 17, 2008 at 01:36:33PM +0200, Pierre Ossman wrote:
> On Fri, 16 May 2008 20:50:57 +0400
> Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org> wrote:
> 
> > 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.

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.

-- 
Anton Vorontsov
email: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
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/

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

[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.

Signed-off-by: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
---
 drivers/mmc/core/core.c  |   19 ++++++++++++++++---
 include/linux/mmc/host.h |    3 +++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 01ced4c..e455ebd 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -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)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 7ab962f..05d03a4 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -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
-- 
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/

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

[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.

Signed-off-by: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
---
 drivers/mmc/host/mmc_spi.c  |   31 +++++++++++++++++++++----------
 include/linux/spi/mmc_spi.h |    6 ++++++
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 3550858..667855a 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -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:
diff --git a/include/linux/spi/mmc_spi.h b/include/linux/spi/mmc_spi.h
index d5ca78b..915faf3 100644
--- a/include/linux/spi/mmc_spi.h
+++ b/include/linux/spi/mmc_spi.h
@@ -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;
 
-- 
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/

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

[Pierre Ossman]

On Wed, 21 May 2008 22:47:13 +0400
Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org> wrote:

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

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

Very nice. But I have some concerns:

> @@ -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.

> +	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. ;)

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

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

[Anton Vorontsov]

On Mon, May 19, 2008 at 11:02:13AM +0800, Chen Gong wrote:
> 
> > -----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: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A@public.gmane.org; 
> > spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; 
> > linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 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.
> > 
> > Signed-off-by: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
> > ---
> >  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,

-- 
Anton Vorontsov
email: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
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/

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

[Chen Gong]

 

> -----Original Message-----
> From: Anton Vorontsov [mailto:avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org] 
> Sent: 2008?5?22? 20:39
> To: Chen Gong
> Cc: Kumar Gala; David Brownell; Pierre Ossman; 
> linuxppc-dev-mnsaURCQ41sdnm+yROfE0A@public.gmane.org; 
> spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; 
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Tabi Timur
> Subject: Re: [PATCH 3/4] [MMC] mmc_spi: add polling support 
> for the carddetectline
> 
> On Mon, May 19, 2008 at 11:02:13AM +0800, Chen Gong wrote:
> > 
> > > -----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: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A@public.gmane.org;
> > > spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org;
> > > linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 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.
> > > 
> > > Signed-off-by: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
> > > ---
> > >  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, 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/

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

[Anton Vorontsov]

On Wed, May 21, 2008 at 09:28:31PM +0200, Pierre Ossman wrote:
> On Wed, 21 May 2008 22:47:13 +0400
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> 
> > 
> > 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.

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

[...]
> > +	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.

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

> 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. :-)

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

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

Ok. How about this version?

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

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

[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.

Signed-off-by: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
---
 drivers/mmc/core/core.c  |   12 +++++++++---
 include/linux/mmc/host.h |    8 ++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 01ced4c..ede5d1e 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -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)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 7ab962f..f2e9885 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -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 */
-- 
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/

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

[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.

Signed-off-by: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
---
 drivers/mmc/host/mmc_spi.c  |   18 ++++++++++++++++--
 include/linux/spi/mmc_spi.h |    6 ++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 3550858..f0f86b1 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -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:
diff --git a/include/linux/spi/mmc_spi.h b/include/linux/spi/mmc_spi.h
index d5ca78b..23c6923 100644
--- a/include/linux/spi/mmc_spi.h
+++ b/include/linux/spi/mmc_spi.h
@@ -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;
 
-- 
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/

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

[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).

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 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(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 7ef3b15..b122eb9 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -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);
 		}
 	}
diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
index a28fc2f..a2ccdf2 100644
--- a/drivers/mmc/host/at91_mci.c
+++ b/drivers/mmc/host/at91_mci.c
@@ -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") );
 	}
diff --git a/drivers/mmc/host/imxmmc.c b/drivers/mmc/host/imxmmc.c
index 95f33e8..5167679 100644
--- a/drivers/mmc/host/imxmmc.c
+++ b/drivers/mmc/host/imxmmc.c
@@ -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;
 }
 
 
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index f0f86b1..a804c7f 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -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)
diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index 65210fc..b6056bd 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -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)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index f2e9885..ef3b773 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -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);
-- 
1.5.5.1

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

[Pierre Ossman]

On Thu, 22 May 2008 22:17:13 +0400
Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org> wrote:

> On Wed, May 21, 2008 at 09:28:31PM +0200, Pierre Ossman wrote:
> > 
> > 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.

> > 
> > 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. :)

> > (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
-- 
     -- 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/

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

[Anton Vorontsov]

On Thu, May 22, 2008 at 09:34:32PM +0200, Pierre Ossman wrote:
[...]
> > 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.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

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

[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.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/core/core.c  |   12 +++++++++---
 include/linux/mmc/host.h |    8 ++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 01ced4c..ede5d1e 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -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)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 7ab962f..f2e9885 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -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 */
-- 
1.5.5.1

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

[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.

Signed-off-by: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
---
 drivers/mmc/host/mmc_spi.c  |   19 +++++++++++++++++--
 include/linux/spi/mmc_spi.h |    9 +++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 3550858..724870c 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -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:
diff --git a/include/linux/spi/mmc_spi.h b/include/linux/spi/mmc_spi.h
index d5ca78b..a3626ae 100644
--- a/include/linux/spi/mmc_spi.h
+++ b/include/linux/spi/mmc_spi.h
@@ -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;
 
-- 
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/

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

[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).

Signed-off-by: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
---
 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(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 7ef3b15..b122eb9 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -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);
 		}
 	}
diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
index 8979ad3..140c2b8 100644
--- a/drivers/mmc/host/at91_mci.c
+++ b/drivers/mmc/host/at91_mci.c
@@ -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") );
 	}
diff --git a/drivers/mmc/host/imxmmc.c b/drivers/mmc/host/imxmmc.c
index 95f33e8..5167679 100644
--- a/drivers/mmc/host/imxmmc.c
+++ b/drivers/mmc/host/imxmmc.c
@@ -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;
 }
 
 
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 724870c..85d9853 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -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)
diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index 65210fc..b6056bd 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -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)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index f2e9885..ef3b773 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -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);
-- 
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/

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

[Anton Vorontsov]

On Thu, May 22, 2008 at 09:44:57PM +0800, Chen Gong wrote:
[...]
> > > 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, 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...

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

-- 
Anton Vorontsov
email: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
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/

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

[Chen Gong]

 

> -----Original Message-----
> From: Anton Vorontsov [mailto:avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org] 
> Sent: 2008?5?26? 23:38
> To: Chen Gong
> Cc: Kumar Gala; David Brownell; Pierre Ossman; 
> linuxppc-dev-mnsaURCQ41sdnm+yROfE0A@public.gmane.org; 
> spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; 
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 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:
> [...]
> > > > 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, 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.

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

:-( 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/

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

[Pierre Ossman]

On Fri, 23 May 2008 19:43:40 +0400
Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org> wrote:

> 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.
> 
> Signed-off-by: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
> ---

Applied the whole lot. Thanks. :)

Rgds
-- 
     -- 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/

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

[Marc Pignat]

Hi all!

On Friday 23 May 2008, Anton Vorontsov wrote:
> get_ro() callback must return values >= 0 for its logical state, and
...
>  static void pxamci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index f2e9885..ef3b773 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -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/

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

[Anton Vorontsov]

On Tue, Jun 03, 2008 at 12:07:49PM +0200, Marc Pignat wrote:
> Hi all!
> 
> On Friday 23 May 2008, Anton Vorontsov wrote:
> > get_ro() callback must return values >= 0 for its logical state, and
> ...
> >  static void pxamci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > index f2e9885..ef3b773 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -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 

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.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

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

[Marc Pignat]

On Thursday 05 June 2008, Anton Vorontsov wrote:
> On Tue, Jun 03, 2008 at 12:07:49PM +0200, Marc Pignat wrote:
> > Hi all!
> > 
> > On Friday 23 May 2008, Anton Vorontsov wrote:
> > > get_ro() callback must return values >= 0 for its logical state, and
> > ...
> > >  static void pxamci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> > > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > > index f2e9885..ef3b773 100644
> > > --- a/include/linux/mmc/host.h
> > > +++ b/include/linux/mmc/host.h
> > > @@ -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 
> 
> 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.

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

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

[Anton Vorontsov]

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).

Suggested-by: Marc Pignat <marc.pignat@hevs.ch>
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---

On Thu, Jun 05, 2008 at 05:58:59PM +0200, Marc Pignat wrote:
[...]
> > >  * 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.
> 
> > 
> > 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".

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(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 85d9853..4e82f64 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -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;
 }
 
diff --git a/drivers/mmc/host/wbsd.c b/drivers/mmc/host/wbsd.c
index be624a0..9283b85 100644
--- a/drivers/mmc/host/wbsd.c
+++ b/drivers/mmc/host/wbsd.c
@@ -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 = {
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index ef3b773..753b723 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -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);
-- 
1.5.5.1

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

[Pierre Ossman]

On Thu, 5 Jun 2008 21:10:13 +0400
Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org> wrote:

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