2013-09-23 08:51:12

by Johan Hovold

[permalink] [raw]
Subject: [PATCH] driver core: prevent deferred probe with platform_driver_probe

Prevent drivers relying on platform_driver_probe from requesting
deferred probing in order to avoid further futile probe attempts (either
the driver has been unregistered or its probe function has been set to
platform_drv_probe_fail when probing is retried).

Note that several platform drivers currently return subsystem errors
from probe and that these can include -EPROBE_DEFER (e.g. if a gpio
request fails).

Signed-off-by: Johan Hovold <[email protected]>
---
drivers/base/platform.c | 17 +++++++++++++----
include/linux/platform_device.h | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..47051cd 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -488,6 +488,11 @@ static int platform_drv_probe(struct device *_dev)
if (ret && ACPI_HANDLE(_dev))
acpi_dev_pm_detach(_dev, true);

+ if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
+ dev_warn(_dev, "probe deferral not supported\n");
+ ret = -ENXIO;
+ }
+
return ret;
}

@@ -553,8 +558,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
/**
* platform_driver_probe - register driver for non-hotpluggable device
* @drv: platform driver structure
- * @probe: the driver probe routine, probably from an __init section,
- * must not return -EPROBE_DEFER.
+ * @probe: the driver probe routine, probably from an __init section
*
* Use this instead of platform_driver_register() when you know the device
* is not hotpluggable and has already been registered, and you want to
@@ -565,8 +569,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
* into system-on-chip processors, where the controller devices have been
* configured as part of board setup.
*
- * This is incompatible with deferred probing so probe() must not
- * return -EPROBE_DEFER.
+ * Note that this is incompatible with deferred probing.
*
* Returns zero if the driver registered and bound to a device, else returns
* a negative error code and with the driver not registered.
@@ -576,6 +579,12 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
{
int retval, code;

+ /*
+ * Prevent driver from requesting probe deferral to avoid further
+ * futile probe attempts.
+ */
+ drv->prevent_deferred_probe = true;
+
/* make sure driver won't have bind/unbind attributes */
drv->driver.suppress_bind_attrs = true;

diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index ce8e4ff..16f6654 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -178,6 +178,7 @@ struct platform_driver {
int (*resume)(struct platform_device *);
struct device_driver driver;
const struct platform_device_id *id_table;
+ bool prevent_deferred_probe;
};

#define to_platform_driver(drv) (container_of((drv), struct platform_driver, \
--
1.8.3.2


2013-09-23 11:01:48

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] driver core: prevent deferred probe with platform_driver_probe

On Mon, Sep 23, 2013 at 10:48:47AM +0200, Johan Hovold wrote:
> Prevent drivers relying on platform_driver_probe from requesting
> deferred probing in order to avoid further futile probe attempts (either
> the driver has been unregistered or its probe function has been set to
> platform_drv_probe_fail when probing is retried).
>
> Note that several platform drivers currently return subsystem errors
> from probe and that these can include -EPROBE_DEFER (e.g. if a gpio
> request fails).

This doesn't seem like the right end to address the problem from, it
seems like it would be better to move these drivers over to being normal
plaform drivers. Using module_platform_driver() means relying on init
ordering which is the sort of thing we're trying to get away from.


Attachments:
(No filename) (772.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-09-23 12:28:13

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH] driver core: prevent deferred probe with platform_driver_probe

On Mon, Sep 23, 2013 at 12:01:40PM +0100, Mark Brown wrote:
> On Mon, Sep 23, 2013 at 10:48:47AM +0200, Johan Hovold wrote:
> > Prevent drivers relying on platform_driver_probe from requesting
> > deferred probing in order to avoid further futile probe attempts (either
> > the driver has been unregistered or its probe function has been set to
> > platform_drv_probe_fail when probing is retried).
> >
> > Note that several platform drivers currently return subsystem errors
> > from probe and that these can include -EPROBE_DEFER (e.g. if a gpio
> > request fails).
>
> This doesn't seem like the right end to address the problem from, it
> seems like it would be better to move these drivers over to being normal
> plaform drivers. Using module_platform_driver() means relying on init
> ordering which is the sort of thing we're trying to get away from.

I actually started out doing that, but it's getting a bit hard to audit
which drivers could actually request probe deferral since gpio and later
other subsystems started returning -EPROBE_DEFER. I found six by just
grepping for gpio_request, but some of these calls can be made in helper
functions (e.g. mmc_gpio_request_cd even though that one was easy to
find).

Having a warning printed by platform_drv_probe if a platform driver
inadvertently requests probe deferral could be useful to catch any
mistakes even if we start moving probe functions out of __init.

I'll fix up the six drivers I found meanwhile.

Johan

2013-09-23 13:40:42

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] driver core: prevent deferred probe with platform_driver_probe

On Mon, Sep 23, 2013 at 02:28:06PM +0200, Johan Hovold wrote:
> On Mon, Sep 23, 2013 at 12:01:40PM +0100, Mark Brown wrote:

> > This doesn't seem like the right end to address the problem from, it
> > seems like it would be better to move these drivers over to being normal
> > plaform drivers. Using module_platform_driver() means relying on init
> > ordering which is the sort of thing we're trying to get away from.

> I actually started out doing that, but it's getting a bit hard to audit
> which drivers could actually request probe deferral since gpio and later
> other subsystems started returning -EPROBE_DEFER. I found six by just
> grepping for gpio_request, but some of these calls can be made in helper
> functions (e.g. mmc_gpio_request_cd even though that one was easy to
> find).

I think it's just the case that all drivers ought to be being converted
to module_platform_driver(), platform_driver_probe() is probably going
to be more trouble than it's worth to use going forwards as it's not
great for single zImage and other distro style usage.

> Having a warning printed by platform_drv_probe if a platform driver
> inadvertently requests probe deferral could be useful to catch any
> mistakes even if we start moving probe functions out of __init.

Haing a warning is useful, yes.


Attachments:
(No filename) (1.27 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-09-23 14:27:59

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 2/7] mmc: mvsdio: fix deferred probe from __init

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if the mmc_gpio_request_cd fails.

Cc: Nicolas Pitre <[email protected]>
Cc: Chris Ball <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/mmc/host/mvsdio.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 06c5b0b..deecee0 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -655,7 +655,7 @@ static const struct mmc_host_ops mvsd_ops = {
.enable_sdio_irq = mvsd_enable_sdio_irq,
};

-static void __init
+static void
mv_conf_mbus_windows(struct mvsd_host *host,
const struct mbus_dram_target_info *dram)
{
@@ -677,7 +677,7 @@ mv_conf_mbus_windows(struct mvsd_host *host,
}
}

-static int __init mvsd_probe(struct platform_device *pdev)
+static int mvsd_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct mmc_host *mmc = NULL;
@@ -819,7 +819,7 @@ out:
return ret;
}

-static int __exit mvsd_remove(struct platform_device *pdev)
+static int mvsd_remove(struct platform_device *pdev)
{
struct mmc_host *mmc = platform_get_drvdata(pdev);

@@ -872,7 +872,8 @@ static const struct of_device_id mvsdio_dt_ids[] = {
MODULE_DEVICE_TABLE(of, mvsdio_dt_ids);

static struct platform_driver mvsd_driver = {
- .remove = __exit_p(mvsd_remove),
+ .probe = mvsd_probe,
+ .remove = mvsd_remove,
.suspend = mvsd_suspend,
.resume = mvsd_resume,
.driver = {
@@ -881,7 +882,7 @@ static struct platform_driver mvsd_driver = {
},
};

-module_platform_driver_probe(mvsd_driver, mvsd_probe);
+module_platform_driver(mvsd_driver);

/* maximum card clock frequency (default 50MHz) */
module_param(maxfreq, int, 0);
--
1.8.3.2

2013-09-23 14:28:34

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 5/7] usb: gadget: pxa25x_udc: fix deferred probe from __init

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.

Cc: Eric Miao <[email protected]>
Cc: Russell King <[email protected]>
Cc: Haojian Zhuang <[email protected]>
Cc: Felipe Balbi <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/gadget/pxa25x_udc.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/pxa25x_udc.c b/drivers/usb/gadget/pxa25x_udc.c
index cc92074..0ac6064 100644
--- a/drivers/usb/gadget/pxa25x_udc.c
+++ b/drivers/usb/gadget/pxa25x_udc.c
@@ -2054,7 +2054,7 @@ static struct pxa25x_udc memory = {
/*
* probe - binds to the platform device
*/
-static int __init pxa25x_udc_probe(struct platform_device *pdev)
+static int pxa25x_udc_probe(struct platform_device *pdev)
{
struct pxa25x_udc *dev = &memory;
int retval, irq;
@@ -2203,7 +2203,7 @@ static void pxa25x_udc_shutdown(struct platform_device *_dev)
pullup_off();
}

-static int __exit pxa25x_udc_remove(struct platform_device *pdev)
+static int pxa25x_udc_remove(struct platform_device *pdev)
{
struct pxa25x_udc *dev = platform_get_drvdata(pdev);

@@ -2294,7 +2294,8 @@ static int pxa25x_udc_resume(struct platform_device *dev)

static struct platform_driver udc_driver = {
.shutdown = pxa25x_udc_shutdown,
- .remove = __exit_p(pxa25x_udc_remove),
+ .probe = pxa25x_udc_probe,
+ .remove = pxa25x_udc_remove,
.suspend = pxa25x_udc_suspend,
.resume = pxa25x_udc_resume,
.driver = {
@@ -2303,7 +2304,7 @@ static struct platform_driver udc_driver = {
},
};

-module_platform_driver_probe(udc_driver, pxa25x_udc_probe);
+module_platform_driver(udc_driver);

MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_AUTHOR("Frank Becker, Robert Schwebel, David Brownell");
--
1.8.3.2

2013-09-23 14:28:33

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe

Deferred probing cannot be used with platform_driver_probe as by the
time probing is retried either the driver has been unregistered or its
probe function has been set to platform_drv_probe_fail.

With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
the gpio subsystem started returning -EPROBE_DEFER, which in turn
several platform drivers using platform_driver_probe return to driver
core. Other subsystems (e.g. regulator) has since started doing the
same.

The first patch in this series prevents platform drivers using
platform_driver_probe from requesting probe deferral while warning that
it is not supported.

The remaining patches move six platform-driver probe functions that rely
on gpio_request out of __init. There are likely other probe functions
that might return -EPROBE_DEFER and should be moved out of __init as
well.

Note that the mvsdio, at91_cf and pxa25x_udc patches are completely
untested.

Johan


Johan Hovold (7):
driver core: prevent deferred probe with platform_driver_probe
mmc: mvsdio: fix deferred probe from __init
mtd: atmel_nand: fix deferred probe from __init
pcmcia: at91_cf: fix deferred probe from __init
usb: gadget: pxa25x_udc: fix deferred probe from __init
usb: phy: gpio-vbus: fix deferred probe from __init
backlight: atmel-pwm-bl: fix deferred probe from __init

drivers/base/platform.c | 17 +++++++++++++----
drivers/mmc/host/mvsdio.c | 11 ++++++-----
drivers/mtd/nand/atmel_nand.c | 13 +++++++------
drivers/pcmcia/at91_cf.c | 11 +++++------
drivers/usb/gadget/pxa25x_udc.c | 9 +++++----
drivers/usb/phy/phy-gpio-vbus-usb.c | 11 +++++------
drivers/video/backlight/atmel-pwm-bl.c | 9 +++++----
include/linux/platform_device.h | 1 +
8 files changed, 47 insertions(+), 35 deletions(-)

--
1.8.3.2

2013-09-23 14:27:57

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 3/7] mtd: atmel_nand: fix deferred probe from __init

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.

Cc: David Woodhouse <[email protected]>
Cc: Josh Wu <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/mtd/nand/atmel_nand.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 060feea..bd1ce7d 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -1139,7 +1139,7 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
return 0;
}

-static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
+static int atmel_pmecc_nand_init_params(struct platform_device *pdev,
struct atmel_nand_host *host)
{
struct mtd_info *mtd = &host->mtd;
@@ -1548,7 +1548,7 @@ static int atmel_of_init_port(struct atmel_nand_host *host,
}
#endif

-static int __init atmel_hw_nand_init_params(struct platform_device *pdev,
+static int atmel_hw_nand_init_params(struct platform_device *pdev,
struct atmel_nand_host *host)
{
struct mtd_info *mtd = &host->mtd;
@@ -1987,7 +1987,7 @@ static struct platform_driver atmel_nand_nfc_driver;
/*
* Probe for the NAND device.
*/
-static int __init atmel_nand_probe(struct platform_device *pdev)
+static int atmel_nand_probe(struct platform_device *pdev)
{
struct atmel_nand_host *host;
struct mtd_info *mtd;
@@ -2184,7 +2184,7 @@ err_nand_ioremap:
/*
* Remove a NAND device.
*/
-static int __exit atmel_nand_remove(struct platform_device *pdev)
+static int atmel_nand_remove(struct platform_device *pdev)
{
struct atmel_nand_host *host = platform_get_drvdata(pdev);
struct mtd_info *mtd = &host->mtd;
@@ -2270,7 +2270,8 @@ static struct platform_driver atmel_nand_nfc_driver = {
};

static struct platform_driver atmel_nand_driver = {
- .remove = __exit_p(atmel_nand_remove),
+ .probe = atmel_nand_probe,
+ .remove = atmel_nand_remove,
.driver = {
.name = "atmel_nand",
.owner = THIS_MODULE,
@@ -2278,7 +2279,7 @@ static struct platform_driver atmel_nand_driver = {
},
};

-module_platform_driver_probe(atmel_nand_driver, atmel_nand_probe);
+module_platform_driver(atmel_nand_driver);

MODULE_LICENSE("GPL");
MODULE_AUTHOR("Rick Bronson");
--
1.8.3.2

2013-09-23 14:27:55

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 7/7] backlight: atmel-pwm-bl: fix deferred probe from __init

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.

Cc: Richard Purdie <[email protected]>
Cc: Jingoo Han <[email protected]>
Cc: Jean-Christophe Plagniol-Villard <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/video/backlight/atmel-pwm-bl.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 0393d82..f7447f7 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -118,7 +118,7 @@ static const struct backlight_ops atmel_pwm_bl_ops = {
.update_status = atmel_pwm_bl_set_intensity,
};

-static int __init atmel_pwm_bl_probe(struct platform_device *pdev)
+static int atmel_pwm_bl_probe(struct platform_device *pdev)
{
struct backlight_properties props;
const struct atmel_pwm_bl_platform_data *pdata;
@@ -202,7 +202,7 @@ err_free_mem:
return retval;
}

-static int __exit atmel_pwm_bl_remove(struct platform_device *pdev)
+static int atmel_pwm_bl_remove(struct platform_device *pdev)
{
struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);

@@ -220,10 +220,11 @@ static struct platform_driver atmel_pwm_bl_driver = {
.name = "atmel-pwm-bl",
},
/* REVISIT add suspend() and resume() */
- .remove = __exit_p(atmel_pwm_bl_remove),
+ .probe = atmel_pwm_bl_probe,
+ .remove = atmel_pwm_bl_remove,
};

-module_platform_driver_probe(atmel_pwm_bl_driver, atmel_pwm_bl_probe);
+module_platform_driver(atmel_pwm_bl_driver);

MODULE_AUTHOR("Hans-Christian egtvedt <[email protected]>");
MODULE_DESCRIPTION("Atmel PWM backlight driver");
--
1.8.3.2

2013-09-23 14:29:52

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 6/7] usb: phy: gpio-vbus: fix deferred probe from __init

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
and 04bf3011 ("regulator: Support driver probe deferral") this driver
might return -EPROBE_DEFER if a gpio_request or regulator_get fails.

Cc: Felipe Balbi <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/phy/phy-gpio-vbus-usb.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/phy/phy-gpio-vbus-usb.c b/drivers/usb/phy/phy-gpio-vbus-usb.c
index b2f29c9..02799a5 100644
--- a/drivers/usb/phy/phy-gpio-vbus-usb.c
+++ b/drivers/usb/phy/phy-gpio-vbus-usb.c
@@ -241,7 +241,7 @@ static int gpio_vbus_set_suspend(struct usb_phy *phy, int suspend)

/* platform driver interface */

-static int __init gpio_vbus_probe(struct platform_device *pdev)
+static int gpio_vbus_probe(struct platform_device *pdev)
{
struct gpio_vbus_mach_info *pdata = dev_get_platdata(&pdev->dev);
struct gpio_vbus_data *gpio_vbus;
@@ -349,7 +349,7 @@ err_gpio:
return err;
}

-static int __exit gpio_vbus_remove(struct platform_device *pdev)
+static int gpio_vbus_remove(struct platform_device *pdev)
{
struct gpio_vbus_data *gpio_vbus = platform_get_drvdata(pdev);
struct gpio_vbus_mach_info *pdata = dev_get_platdata(&pdev->dev);
@@ -398,8 +398,6 @@ static const struct dev_pm_ops gpio_vbus_dev_pm_ops = {
};
#endif

-/* NOTE: the gpio-vbus device may *NOT* be hotplugged */
-
MODULE_ALIAS("platform:gpio-vbus");

static struct platform_driver gpio_vbus_driver = {
@@ -410,10 +408,11 @@ static struct platform_driver gpio_vbus_driver = {
.pm = &gpio_vbus_dev_pm_ops,
#endif
},
- .remove = __exit_p(gpio_vbus_remove),
+ .probe = gpio_vbus_probe,
+ .remove = gpio_vbus_remove,
};

-module_platform_driver_probe(gpio_vbus_driver, gpio_vbus_probe);
+module_platform_driver(gpio_vbus_driver);

MODULE_DESCRIPTION("simple GPIO controlled OTG transceiver driver");
MODULE_AUTHOR("Philipp Zabel");
--
1.8.3.2

2013-09-23 14:28:31

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 4/7] pcmcia: at91_cf: fix deferred probe from __init

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.

Cc: Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc: Nicolas Ferre <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/pcmcia/at91_cf.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/pcmcia/at91_cf.c b/drivers/pcmcia/at91_cf.c
index b8f5acf..de24232 100644
--- a/drivers/pcmcia/at91_cf.c
+++ b/drivers/pcmcia/at91_cf.c
@@ -245,7 +245,7 @@ static int at91_cf_dt_init(struct platform_device *pdev)
}
#endif

-static int __init at91_cf_probe(struct platform_device *pdev)
+static int at91_cf_probe(struct platform_device *pdev)
{
struct at91_cf_socket *cf;
struct at91_cf_data *board = pdev->dev.platform_data;
@@ -354,7 +354,7 @@ fail0a:
return status;
}

-static int __exit at91_cf_remove(struct platform_device *pdev)
+static int at91_cf_remove(struct platform_device *pdev)
{
struct at91_cf_socket *cf = platform_get_drvdata(pdev);

@@ -404,14 +404,13 @@ static struct platform_driver at91_cf_driver = {
.owner = THIS_MODULE,
.of_match_table = of_match_ptr(at91_cf_dt_ids),
},
- .remove = __exit_p(at91_cf_remove),
+ .probe = at91_cf_probe,
+ .remove = at91_cf_remove,
.suspend = at91_cf_suspend,
.resume = at91_cf_resume,
};

-/*--------------------------------------------------------------------------*/
-
-module_platform_driver_probe(at91_cf_driver, at91_cf_probe);
+module_platform_driver(at91_cf_driver);

MODULE_DESCRIPTION("AT91 Compact Flash Driver");
MODULE_AUTHOR("David Brownell");
--
1.8.3.2

2013-09-23 14:30:11

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 1/7] driver core: prevent deferred probe with platform_driver_probe

Prevent drivers relying on platform_driver_probe from requesting
deferred probing in order to avoid further futile probe attempts (either
the driver has been unregistered or its probe function has been set to
platform_drv_probe_fail when probing is retried).

Note that several platform drivers currently return subsystem errors
from probe and that these can include -EPROBE_DEFER (e.g. if a gpio
request fails).

Add a warning to platform_drv_probe that can be used to catch drivers
that inadvertently request probe deferral while using
platform_driver_probe.

Signed-off-by: Johan Hovold <[email protected]>
---
drivers/base/platform.c | 17 +++++++++++++----
include/linux/platform_device.h | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..47051cd 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -488,6 +488,11 @@ static int platform_drv_probe(struct device *_dev)
if (ret && ACPI_HANDLE(_dev))
acpi_dev_pm_detach(_dev, true);

+ if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
+ dev_warn(_dev, "probe deferral not supported\n");
+ ret = -ENXIO;
+ }
+
return ret;
}

@@ -553,8 +558,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
/**
* platform_driver_probe - register driver for non-hotpluggable device
* @drv: platform driver structure
- * @probe: the driver probe routine, probably from an __init section,
- * must not return -EPROBE_DEFER.
+ * @probe: the driver probe routine, probably from an __init section
*
* Use this instead of platform_driver_register() when you know the device
* is not hotpluggable and has already been registered, and you want to
@@ -565,8 +569,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
* into system-on-chip processors, where the controller devices have been
* configured as part of board setup.
*
- * This is incompatible with deferred probing so probe() must not
- * return -EPROBE_DEFER.
+ * Note that this is incompatible with deferred probing.
*
* Returns zero if the driver registered and bound to a device, else returns
* a negative error code and with the driver not registered.
@@ -576,6 +579,12 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
{
int retval, code;

+ /*
+ * Prevent driver from requesting probe deferral to avoid further
+ * futile probe attempts.
+ */
+ drv->prevent_deferred_probe = true;
+
/* make sure driver won't have bind/unbind attributes */
drv->driver.suppress_bind_attrs = true;

diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index ce8e4ff..16f6654 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -178,6 +178,7 @@ struct platform_driver {
int (*resume)(struct platform_device *);
struct device_driver driver;
const struct platform_device_id *id_table;
+ bool prevent_deferred_probe;
};

#define to_platform_driver(drv) (container_of((drv), struct platform_driver, \
--
1.8.3.2

2013-09-23 14:51:05

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe

On Mon, Sep 23, 2013 at 04:27:25PM +0200, Johan Hovold wrote:
> Deferred probing cannot be used with platform_driver_probe as by the
> time probing is retried either the driver has been unregistered or its
> probe function has been set to platform_drv_probe_fail.
>
> With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> the gpio subsystem started returning -EPROBE_DEFER, which in turn
> several platform drivers using platform_driver_probe return to driver
> core. Other subsystems (e.g. regulator) has since started doing the
> same.
>
> The first patch in this series prevents platform drivers using
> platform_driver_probe from requesting probe deferral while warning that
> it is not supported.
>
> The remaining patches move six platform-driver probe functions that rely
> on gpio_request out of __init. There are likely other probe functions
> that might return -EPROBE_DEFER and should be moved out of __init as
> well.

As usually when I read this I wonder why platform_driver_probe exists
anyway. The only advantage I can think off is that the probe functions
are in __init and thus can be disposed of later. Now you remove the
__init annotations from these probe functions. Wouldn't it be better to
convert the drivers to regular platform_driver_register instead?

Sascha

--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2013-09-23 15:20:25

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe

On Mon, Sep 23, 2013 at 04:50:29PM +0200, Sascha Hauer wrote:
> On Mon, Sep 23, 2013 at 04:27:25PM +0200, Johan Hovold wrote:
> > Deferred probing cannot be used with platform_driver_probe as by the
> > time probing is retried either the driver has been unregistered or its
> > probe function has been set to platform_drv_probe_fail.
> >
> > With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> > the gpio subsystem started returning -EPROBE_DEFER, which in turn
> > several platform drivers using platform_driver_probe return to driver
> > core. Other subsystems (e.g. regulator) has since started doing the
> > same.
> >
> > The first patch in this series prevents platform drivers using
> > platform_driver_probe from requesting probe deferral while warning that
> > it is not supported.
> >
> > The remaining patches move six platform-driver probe functions that rely
> > on gpio_request out of __init. There are likely other probe functions
> > that might return -EPROBE_DEFER and should be moved out of __init as
> > well.
>
> As usually when I read this I wonder why platform_driver_probe exists
> anyway. The only advantage I can think off is that the probe functions
> are in __init and thus can be disposed of later. Now you remove the
> __init annotations from these probe functions. Wouldn't it be better to
> convert the drivers to regular platform_driver_register instead?

Perhaps that paragraph was a bit unclear: I move them out of __init
_and_ use platform_driver_register instead of platform_driver_probe as
well.

Johan

2013-09-23 15:24:57

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe

On Mon, Sep 23, 2013 at 05:20:18PM +0200, Johan Hovold wrote:
> On Mon, Sep 23, 2013 at 04:50:29PM +0200, Sascha Hauer wrote:
> > On Mon, Sep 23, 2013 at 04:27:25PM +0200, Johan Hovold wrote:
> > > Deferred probing cannot be used with platform_driver_probe as by the
> > > time probing is retried either the driver has been unregistered or its
> > > probe function has been set to platform_drv_probe_fail.
> > >
> > > With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> > > the gpio subsystem started returning -EPROBE_DEFER, which in turn
> > > several platform drivers using platform_driver_probe return to driver
> > > core. Other subsystems (e.g. regulator) has since started doing the
> > > same.
> > >
> > > The first patch in this series prevents platform drivers using
> > > platform_driver_probe from requesting probe deferral while warning that
> > > it is not supported.
> > >
> > > The remaining patches move six platform-driver probe functions that rely
> > > on gpio_request out of __init. There are likely other probe functions
> > > that might return -EPROBE_DEFER and should be moved out of __init as
> > > well.
> >
> > As usually when I read this I wonder why platform_driver_probe exists
> > anyway. The only advantage I can think off is that the probe functions
> > are in __init and thus can be disposed of later. Now you remove the
> > __init annotations from these probe functions. Wouldn't it be better to
> > convert the drivers to regular platform_driver_register instead?
>
> Perhaps that paragraph was a bit unclear: I move them out of __init
> _and_ use platform_driver_register instead of platform_driver_probe as
> well.

Oh yes, I should have looked closer. Sorry for the noise.

Sascha

--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2013-09-23 16:53:33

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH 4/7] pcmcia: at91_cf: fix deferred probe from __init

On 23/09/2013 16:27, Johan Hovold :
> Move probe out of __init section and don't use platform_driver_probe
> which cannot be used with deferred probing.
>
> Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> this driver might return -EPROBE_DEFER if a gpio_request fails.
>
> Cc: Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
> Cc: Nicolas Ferre <[email protected]>

Acked-by: Nicolas Ferre <[email protected]>

Thanks Johan.

> Signed-off-by: Johan Hovold <[email protected]>
> ---
> drivers/pcmcia/at91_cf.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pcmcia/at91_cf.c b/drivers/pcmcia/at91_cf.c
> index b8f5acf..de24232 100644
> --- a/drivers/pcmcia/at91_cf.c
> +++ b/drivers/pcmcia/at91_cf.c
> @@ -245,7 +245,7 @@ static int at91_cf_dt_init(struct platform_device *pdev)
> }
> #endif
>
> -static int __init at91_cf_probe(struct platform_device *pdev)
> +static int at91_cf_probe(struct platform_device *pdev)
> {
> struct at91_cf_socket *cf;
> struct at91_cf_data *board = pdev->dev.platform_data;
> @@ -354,7 +354,7 @@ fail0a:
> return status;
> }
>
> -static int __exit at91_cf_remove(struct platform_device *pdev)
> +static int at91_cf_remove(struct platform_device *pdev)
> {
> struct at91_cf_socket *cf = platform_get_drvdata(pdev);
>
> @@ -404,14 +404,13 @@ static struct platform_driver at91_cf_driver = {
> .owner = THIS_MODULE,
> .of_match_table = of_match_ptr(at91_cf_dt_ids),
> },
> - .remove = __exit_p(at91_cf_remove),
> + .probe = at91_cf_probe,
> + .remove = at91_cf_remove,
> .suspend = at91_cf_suspend,
> .resume = at91_cf_resume,
> };
>
> -/*--------------------------------------------------------------------------*/
> -
> -module_platform_driver_probe(at91_cf_driver, at91_cf_probe);
> +module_platform_driver(at91_cf_driver);
>
> MODULE_DESCRIPTION("AT91 Compact Flash Driver");
> MODULE_AUTHOR("David Brownell");
>


--
Nicolas Ferre