2008-06-03 10:08:03

by Marc Pignat

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

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


2008-06-05 14:43:24

by Anton Vorontsov

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

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: [email protected]
irc://irc.freenode.net/bd2

2008-06-05 15:59:19

by Marc Pignat

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

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



2008-06-05 17:10:26

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH] 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).

Suggested-by: Marc Pignat <[email protected]>
Signed-off-by: Anton Vorontsov <[email protected]>
---

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

2008-06-14 14:37:30

by Pierre Ossman

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

On Thu, 5 Jun 2008 21:10:13 +0400
Anton Vorontsov <[email protected]> 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.

2008-06-17 14:16:24

by Anton Vorontsov

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

On Sat, Jun 14, 2008 at 04:36:40PM +0200, Pierre Ossman wrote:
> On Thu, 5 Jun 2008 21:10:13 +0400
> Anton Vorontsov <[email protected]> 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. :)

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

Thanks,

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

2008-06-17 14:17:25

by Anton Vorontsov

[permalink] [raw]
Subject: [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.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/mmc/core/core.c | 12 +++++++++---
include/linux/mmc/host.h | 11 +++++++++++
2 files changed, 20 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..6188e19 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -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 */
--
1.5.5.4

2008-06-17 14:17:37

by Anton Vorontsov

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

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 <[email protected]>
---
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..547eb85 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.4

2008-06-17 14:17:52

by Anton Vorontsov

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

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

Signed-off-by: Anton Vorontsov <[email protected]>
---
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(-)

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..b9d4ed6 100644
--- a/drivers/mmc/host/at91_mci.c
+++ b/drivers/mmc/host/at91_mci.c
@@ -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 = {
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 547eb85..4e82f64 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/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 6188e19..753b723 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -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);
--
1.5.5.4

2008-06-20 15:40:44

by Pierre Ossman

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

On Tue, 17 Jun 2008 18:16:07 +0400
Anton Vorontsov <[email protected]> wrote:

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

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