[PATCH] mmc: mmci.c: Defer probe() in case of missing GPIOs

[PATCH] mmc: mmci.c: Defer probe() in case of missing GPIOs

[Roland Stigge]

If the GPIOs used by the MMCI driver are not registered yet when the driver is
probe()d, they can't be used. This happens if the mmci driver is probed before
the respective GPIO controller (e.g. on the LPC32xx EA3250 board, the PCA9532
GPIO controller would be initialized via DT after mmci). Therefore, we defer
mmci in this case.

Signed-off-by: Roland Stigge <stigge@antcom.de>

---
 drivers/mmc/host/mmci.c |    8 ++++++++
 1 file changed, 8 insertions(+)

--- linux-2.6.orig/drivers/mmc/host/mmci.c
+++ linux-2.6/drivers/mmc/host/mmci.c
@@ -1424,6 +1424,10 @@ static int __devinit mmci_probe(struct a
 	writel(0, host->base + MMCIMASK1);
 	writel(0xfff, host->base + MMCICLEAR);
 
+	if (plat->gpio_cd < 0) {
+		ret = -EPROBE_DEFER;
+		goto err_gpio_cd;
+	}
 	if (gpio_is_valid(plat->gpio_cd)) {
 		ret = gpio_request(plat->gpio_cd, DRIVER_NAME " (cd)");
 		if (ret == 0)
@@ -1447,6 +1451,10 @@ static int __devinit mmci_probe(struct a
 		if (ret >= 0)
 			host->gpio_cd_irq = gpio_to_irq(plat->gpio_cd);
 	}
+	if (plat->gpio_wp < 0) {
+		ret = -EPROBE_DEFER;
+		goto err_gpio_wp;
+	}
 	if (gpio_is_valid(plat->gpio_wp)) {
 		ret = gpio_request(plat->gpio_wp, DRIVER_NAME " (wp)");
 		if (ret == 0)

[PATCH] mmc: mmci.c: Defer probe() in case of missing GPIOs

[Russell King - ARM Linux]

On Sat, Jun 16, 2012 at 04:14:59PM +0200, Roland Stigge wrote:
> If the GPIOs used by the MMCI driver are not registered yet when the
> driver is probe()d, they can't be used. This happens if the mmci driver
> is probed before the respective GPIO controller (e.g. on the LPC32xx
> EA3250 board, the PCA9532 GPIO controller would be initialized via DT
> after mmci). Therefore, we defer mmci in this case.

This code is wrong.  There are platforms where plat->gpio_cd is negative
(because there isn't an associated GPIO) and we still expect the driver
to successfully bind.  In that case, the driver gets the CD and WP
information via the status callback.

So this is an incompatible change with existing (and required) driver
behaviour.

[PATCH] mmc: mmci.c: Defer probe() in case of missing GPIOs

[Roland Stigge]

On 16/06/12 16:26, Russell King - ARM Linux wrote:
> On Sat, Jun 16, 2012 at 04:14:59PM +0200, Roland Stigge wrote:
>> If the GPIOs used by the MMCI driver are not registered yet when the
>> driver is probe()d, they can't be used. This happens if the mmci driver
>> is probed before the respective GPIO controller (e.g. on the LPC32xx
>> EA3250 board, the PCA9532 GPIO controller would be initialized via DT
>> after mmci). Therefore, we defer mmci in this case.
> 
> This code is wrong.  There are platforms where plat->gpio_cd is negative
> (because there isn't an associated GPIO) and we still expect the driver
> to successfully bind.  In that case, the driver gets the CD and WP
> information via the status callback.
> 
> So this is an incompatible change with existing (and required) driver
> behaviour.

As someone just told me, in the case of no GPIO, we would have gpio_cd
== -ENODEV. Would it be sufficient to check for -ENODEV (in which case
we would do without GPIO), and otherwise return -EPROBE_DEFER?

Or is it necessary to flag DT-bound GPIOs somehow additionally to handle
this new case?

Thanks in advance,

Roland

[PATCH] mmc: mmci.c: Defer probe() in case of missing GPIOs

[Russell King - ARM Linux]

On Sat, Jun 16, 2012 at 04:57:48PM +0200, Roland Stigge wrote:
> On 16/06/12 16:26, Russell King - ARM Linux wrote:
> > On Sat, Jun 16, 2012 at 04:14:59PM +0200, Roland Stigge wrote:
> >> If the GPIOs used by the MMCI driver are not registered yet when the
> >> driver is probe()d, they can't be used. This happens if the mmci driver
> >> is probed before the respective GPIO controller (e.g. on the LPC32xx
> >> EA3250 board, the PCA9532 GPIO controller would be initialized via DT
> >> after mmci). Therefore, we defer mmci in this case.
> > 
> > This code is wrong.  There are platforms where plat->gpio_cd is negative
> > (because there isn't an associated GPIO) and we still expect the driver
> > to successfully bind.  In that case, the driver gets the CD and WP
> > information via the status callback.
> > 
> > So this is an incompatible change with existing (and required) driver
> > behaviour.
> 
> As someone just told me, in the case of no GPIO, we would have gpio_cd
> == -ENODEV. Would it be sufficient to check for -ENODEV (in which case
> we would do without GPIO), and otherwise return -EPROBE_DEFER?

Sigh.  New rules which jump up when no one's read the bloody code.
That's intensely frustrating.

Why can't the *new* DT parsing code set the GPIOs to -EPROBE_DEFER when
*it* wants them to mean "we should defer" ?  Why should the DT code
have sole use over "-1" ?  That would be *far* more logical and wouldn't
clash with anything in existing use.