| Message ID | 1363266691-15757-12-git-send-email-fabio.porcedda@gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Thursday 14 March 2013, Fabio Porcedda wrote: > This patch converts the drivers to use the > module_platform_driver_probe() macro which makes the code smaller and > a bit simpler. > > Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Arnd Bergmann <arnd@arndb.de> > --- > drivers/misc/atmel_pwm.c | 12 +----------- > drivers/misc/ep93xx_pwm.c | 13 +------------ > 2 files changed, 2 insertions(+), 23 deletions(-) The patch itself seems fine, but there are two issues around it: * The PWM drivers should really get moved to drivers/pwm and converted to the new PWM subsystem. I don't know if Hartley or Hans-Christian have plans to do that already. * Regarding the use of module_platform_driver_probe, I'm a little worried about the interactions with deferred probing. I don't think there are any regressions, but we should probably make people aware that one cannot return -EPROBE_DEFER from a platform_driver_probe function. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 14, 2013 at 01:58:05PM +0000, Arnd Bergmann wrote: > On Thursday 14 March 2013, Fabio Porcedda wrote: > > This patch converts the drivers to use the > > module_platform_driver_probe() macro which makes the code smaller and > > a bit simpler. > > > > Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: Arnd Bergmann <arnd@arndb.de> > > --- > > drivers/misc/atmel_pwm.c | 12 +----------- > > drivers/misc/ep93xx_pwm.c | 13 +------------ > > 2 files changed, 2 insertions(+), 23 deletions(-) > > The patch itself seems fine, but there are two issues around it: > > * The PWM drivers should really get moved to drivers/pwm and converted to the new > PWM subsystem. I don't know if Hartley or Hans-Christian have plans to do > that already. > > * Regarding the use of module_platform_driver_probe, I'm a little worried about > the interactions with deferred probing. I don't think there are any regressions, > but we should probably make people aware that one cannot return -EPROBE_DEFER > from a platform_driver_probe function. I'm worried about this aswell. I think platform_driver_probe shouldn't be used anymore. Even if a driver does not explicitly make use of -EPROBE_DEFER, it leaks in very quickly if a driver for example uses a regulator and just returns the error value from regulator_get. Sascha
On Thu, Mar 14, 2013 at 3:06 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote: > On Thu, Mar 14, 2013 at 01:58:05PM +0000, Arnd Bergmann wrote: >> On Thursday 14 March 2013, Fabio Porcedda wrote: >> > This patch converts the drivers to use the >> > module_platform_driver_probe() macro which makes the code smaller and >> > a bit simpler. >> > >> > Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com> >> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> > Cc: Arnd Bergmann <arnd@arndb.de> >> > --- >> > drivers/misc/atmel_pwm.c | 12 +----------- >> > drivers/misc/ep93xx_pwm.c | 13 +------------ >> > 2 files changed, 2 insertions(+), 23 deletions(-) >> >> The patch itself seems fine, but there are two issues around it: >> >> * The PWM drivers should really get moved to drivers/pwm and converted to the new >> PWM subsystem. I don't know if Hartley or Hans-Christian have plans to do >> that already. >> >> * Regarding the use of module_platform_driver_probe, I'm a little worried about >> the interactions with deferred probing. I don't think there are any regressions, >> but we should probably make people aware that one cannot return -EPROBE_DEFER >> from a platform_driver_probe function. The use of module_platform_driver_probe() doesn't change anything about that, it's exactly the same thing as using "return platform_driver_probe()". I'm right or I'm missing something? Maybe are you just speaking about the misuse of "platform_driver_probe"? Best regards Fabio Porcedda > > I'm worried about this aswell. I think platform_driver_probe shouldn't > be used anymore. Even if a driver does not explicitly make use of > -EPROBE_DEFER, it leaks in very quickly if a driver for example uses a > regulator and just returns the error value from regulator_get. > 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 | -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 15 March 2013, Fabio Porcedda wrote: > >> * Regarding the use of module_platform_driver_probe, I'm a little worried about > >> the interactions with deferred probing. I don't think there are any regressions, > >> but we should probably make people aware that one cannot return -EPROBE_DEFER > >> from a platform_driver_probe function. > > The use of module_platform_driver_probe() doesn't change anything about that, > it's exactly the same thing as using "return platform_driver_probe()". > I'm right or I'm missing something? Maybe are you just speaking about > the misuse of "platform_driver_probe"? Yes, that was what I meant. The point is that if we need to review or remove all uses of platform_driver_probe, it would be better not to introduce a module_platform_driver_probe() interface to make it easier to use. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, March 14, 2013 6:58 AM, Arnd Bergmann wrote: > On Thursday 14 March 2013, Fabio Porcedda wrote: >> This patch converts the drivers to use the >> module_platform_driver_probe() macro which makes the code smaller and >> a bit simpler. >> >> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Arnd Bergmann <arnd@arndb.de> >> --- >> drivers/misc/atmel_pwm.c | 12 +----------- >> drivers/misc/ep93xx_pwm.c | 13 +------------ >> 2 files changed, 2 insertions(+), 23 deletions(-) > > The patch itself seems fine, but there are two issues around it: > > * The PWM drivers should really get moved to drivers/pwm and converted to the new > PWM subsystem. I don't know if Hartley or Hans-Christian have plans to do > that already. Arnd, Ill look at converting the ep93xx pwm driver to the PWM subsystem. The only issue is the current driver exposes a sysfs interface that I think is not available in that subsystem. >* Regarding the use of module_platform_driver_probe, I'm a little worried about > the interactions with deferred probing. I don't think there are any regressions, > but we should probably make people aware that one cannot return -EPROBE_DEFER > from a platform_driver_probe function. The ep93xx pwm driver does not need to use platform_driver_probe(). It can be changed to use module_platform_driver() by just moving the .probe to the platform_driver. This driver was added before module_platform_driver() was available and I used the platform_driver_probe() thinking it would save a couple lines of code. I'll change this in a bit. Right now I'm trying to work out why kernel 3.8 is not booting on the ep93xx. I had 3.6.6 on my development board and 3.7 works fine but 3.8 hangs without uncompressing the kernel. Regards, Hartley -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 15, 2013 at 12:28 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 15 March 2013, Fabio Porcedda wrote: >> >> * Regarding the use of module_platform_driver_probe, I'm a little worried about >> >> the interactions with deferred probing. I don't think there are any regressions, >> >> but we should probably make people aware that one cannot return -EPROBE_DEFER >> >> from a platform_driver_probe function. >> >> The use of module_platform_driver_probe() doesn't change anything about that, >> it's exactly the same thing as using "return platform_driver_probe()". >> I'm right or I'm missing something? Maybe are you just speaking about >> the misuse of "platform_driver_probe"? > > Yes, that was what I meant. The point is that if we need to review or remove > all uses of platform_driver_probe, it would be better not to introduce a > module_platform_driver_probe() interface to make it easier to use. Just to let you know, the module_platform_driver_probe() macro is already in v3.9-rc1 and is already used by some drivers. In linux-next there are already many patches that use that macro. Best regards -- Fabio Porcedda -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 15 March 2013, H Hartley Sweeten wrote: > Arnd, > > Ill look at converting the ep93xx pwm driver to the PWM subsystem. The only issue is > the current driver exposes a sysfs interface that I think is not available in that subsystem. You can probably keep providing that interface if you have active users. > >* Regarding the use of module_platform_driver_probe, I'm a little worried about > > the interactions with deferred probing. I don't think there are any regressions, > > but we should probably make people aware that one cannot return -EPROBE_DEFER > > from a platform_driver_probe function. > > The ep93xx pwm driver does not need to use platform_driver_probe(). It can be changed > to use module_platform_driver() by just moving the .probe to the platform_driver. This > driver was added before module_platform_driver() was available and I used the > platform_driver_probe() thinking it would save a couple lines of code. > > I'll change this in a bit. Right now I'm trying to work out why kernel 3.8 is not booting > on the ep93xx. I had 3.6.6 on my development board and 3.7 works fine but 3.8 hangs > without uncompressing the kernel. Ok, thanks! Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 15, 2013 at 9:18 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 15 March 2013, H Hartley Sweeten wrote: >> Arnd, >> >> Ill look at converting the ep93xx pwm driver to the PWM subsystem. The only issue is >> the current driver exposes a sysfs interface that I think is not available in that subsystem. > > You can probably keep providing that interface if you have active users. > >> >* Regarding the use of module_platform_driver_probe, I'm a little worried about >> > the interactions with deferred probing. I don't think there are any regressions, >> > but we should probably make people aware that one cannot return -EPROBE_DEFER >> > from a platform_driver_probe function. >> >> The ep93xx pwm driver does not need to use platform_driver_probe(). It can be changed >> to use module_platform_driver() by just moving the .probe to the platform_driver. This >> driver was added before module_platform_driver() was available and I used the >> platform_driver_probe() thinking it would save a couple lines of code. Since by using platform_driver_probe() the function ep93xx_pwm_probe() is freed after initialization, is better to use module_platform_drive_probe(). IMHO i don't see any good reason to use module_platform_driver() for this driver. Best regards Fabio Porcedda >> I'll change this in a bit. Right now I'm trying to work out why kernel 3.8 is not booting >> on the ep93xx. I had 3.6.6 on my development board and 3.7 works fine but 3.8 hangs >> without uncompressing the kernel. > > Ok, thanks! > > Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 18 March 2013, Fabio Porcedda wrote: > Since by using platform_driver_probe() the function > ep93xx_pwm_probe() is freed after initialization, > is better to use module_platform_drive_probe(). > IMHO i don't see any good reason to use module_platform_driver() for > this driver. As I commented earlier, the platform_driver_probe() and module_platform_drive_probe() interfaces are rather dangerous in combination with deferred probing, I would much prefer Harley's patch. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 18, 2013 at 11:58 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Monday 18 March 2013, Fabio Porcedda wrote: >> Since by using platform_driver_probe() the function >> ep93xx_pwm_probe() is freed after initialization, >> is better to use module_platform_drive_probe(). >> IMHO i don't see any good reason to use module_platform_driver() for >> this driver. > > As I commented earlier, the platform_driver_probe() and > module_platform_drive_probe() interfaces are rather dangerous in combination > with deferred probing, I would much prefer Harley's patch. Since those drivers don't use -EPROBE_DEFER i was thinking that they don't use deferred probing. I'm missing something? Best regards Fabio Porcedda > Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 18 March 2013, Fabio Porcedda wrote: > > On Mon, Mar 18, 2013 at 11:58 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Monday 18 March 2013, Fabio Porcedda wrote: > >> Since by using platform_driver_probe() the function > >> ep93xx_pwm_probe() is freed after initialization, > >> is better to use module_platform_drive_probe(). > >> IMHO i don't see any good reason to use module_platform_driver() for > >> this driver. > > > > As I commented earlier, the platform_driver_probe() and > > module_platform_drive_probe() interfaces are rather dangerous in combination > > with deferred probing, I would much prefer Harley's patch. > > Since those drivers don't use -EPROBE_DEFER i was thinking that they don't use > deferred probing. > I'm missing something? clk_get() may return -EPROBE_DEFER after ep93xx is converted to use the common clk API. We currently return the value of clk_get from the probe() function, which will automatically do the right thing as long as the probe function remains reachable. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 18, 2013 at 12:28 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Monday 18 March 2013, Fabio Porcedda wrote: >> >> On Mon, Mar 18, 2013 at 11:58 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> > On Monday 18 March 2013, Fabio Porcedda wrote: >> >> Since by using platform_driver_probe() the function >> >> ep93xx_pwm_probe() is freed after initialization, >> >> is better to use module_platform_drive_probe(). >> >> IMHO i don't see any good reason to use module_platform_driver() for >> >> this driver. >> > >> > As I commented earlier, the platform_driver_probe() and >> > module_platform_drive_probe() interfaces are rather dangerous in combination >> > with deferred probing, I would much prefer Harley's patch. >> >> Since those drivers don't use -EPROBE_DEFER i was thinking that they don't use >> deferred probing. >> I'm missing something? > > clk_get() may return -EPROBE_DEFER after ep93xx is converted to use the > common clk API. We currently return the value of clk_get from the probe() > function, which will automatically do the right thing as long as the probe > function remains reachable. Thanks for the explanation. Regards Fabio Porcedda > Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 19, 2013 at 9:55 AM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote: > On Mon, Mar 18, 2013 at 12:28 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> On Monday 18 March 2013, Fabio Porcedda wrote: >>> On Mon, Mar 18, 2013 at 11:58 AM, Arnd Bergmann <arnd@arndb.de> wrote: >>> > On Monday 18 March 2013, Fabio Porcedda wrote: >>> >> Since by using platform_driver_probe() the function >>> >> ep93xx_pwm_probe() is freed after initialization, >>> >> is better to use module_platform_drive_probe(). >>> >> IMHO i don't see any good reason to use module_platform_driver() for >>> >> this driver. >>> > >>> > As I commented earlier, the platform_driver_probe() and >>> > module_platform_drive_probe() interfaces are rather dangerous in combination >>> > with deferred probing, I would much prefer Harley's patch. >>> >>> Since those drivers don't use -EPROBE_DEFER i was thinking that they don't use >>> deferred probing. >>> I'm missing something? >> >> clk_get() may return -EPROBE_DEFER after ep93xx is converted to use the >> common clk API. We currently return the value of clk_get from the probe() >> function, which will automatically do the right thing as long as the probe >> function remains reachable. > > Thanks for the explanation. Hmm, so we may have drivers that (now) work perfectly fine with module_platform_driver_probe()/platform_driver_probe(), but will start failing suddenly in the future? I guess we need a big fat WARN_ON(-EPROBE_DEFER) in platform_driver_probe() to catch these? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 19 March 2013, Geert Uytterhoeven wrote: > Hmm, so we may have drivers that (now) work perfectly fine with > module_platform_driver_probe()/platform_driver_probe(), but will start > failing suddenly in the future? They will fail if someone changes the initialization order. That would already break drivers before deferred probing support (and was the reason we added feature in the first place), but now we can be much more liberal with the order in which drivers are initialized, except when they are using platform_driver_probe() > I guess we need a big fat WARN_ON(-EPROBE_DEFER) in > platform_driver_probe() to catch these? Yes, very good idea. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 19, 2013 at 5:48 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday 19 March 2013, Geert Uytterhoeven wrote: >> Hmm, so we may have drivers that (now) work perfectly fine with >> module_platform_driver_probe()/platform_driver_probe(), but will start >> failing suddenly in the future? > > They will fail if someone changes the initialization order. That would > already break drivers before deferred probing support (and was the reason > we added feature in the first place), but now we can be much more liberal > with the order in which drivers are initialized, except when they are > using platform_driver_probe() > >> I guess we need a big fat WARN_ON(-EPROBE_DEFER) in >> platform_driver_probe() to catch these? > > Yes, very good idea. > > Arnd If it's fine, I'll send a patch for that. Regards -- Fabio Porcedda -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 19 March 2013, Fabio Porcedda wrote: > On Tue, Mar 19, 2013 at 5:48 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Tuesday 19 March 2013, Geert Uytterhoeven wrote: > >> Hmm, so we may have drivers that (now) work perfectly fine with > >> module_platform_driver_probe()/platform_driver_probe(), but will start > >> failing suddenly in the future? > > > > They will fail if someone changes the initialization order. That would > > already break drivers before deferred probing support (and was the reason > > we added feature in the first place), but now we can be much more liberal > > with the order in which drivers are initialized, except when they are > > using platform_driver_probe() > > > >> I guess we need a big fat WARN_ON(-EPROBE_DEFER) in > >> platform_driver_probe() to catch these? > > > > Yes, very good idea. > > If it's fine, I'll send a patch for that. That would be cool, yes. I looked at it earlier (after sending my email above) and couldn't find an easy way to do it though, because platform_drv_probe does not know whether it is called from platform_driver_probe or not. Maybe using something other than platform_driver_register would work here. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 19, 2013 at 6:59 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday 19 March 2013, Fabio Porcedda wrote: >> On Tue, Mar 19, 2013 at 5:48 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> > On Tuesday 19 March 2013, Geert Uytterhoeven wrote: >> >> Hmm, so we may have drivers that (now) work perfectly fine with >> >> module_platform_driver_probe()/platform_driver_probe(), but will start >> >> failing suddenly in the future? >> > >> > They will fail if someone changes the initialization order. That would >> > already break drivers before deferred probing support (and was the reason >> > we added feature in the first place), but now we can be much more liberal >> > with the order in which drivers are initialized, except when they are >> > using platform_driver_probe() >> > >> >> I guess we need a big fat WARN_ON(-EPROBE_DEFER) in >> >> platform_driver_probe() to catch these? >> > >> > Yes, very good idea. >> >> If it's fine, I'll send a patch for that. > > That would be cool, yes. I looked at it earlier (after sending my email above) > and couldn't find an easy way to do it though, because platform_drv_probe > does not know whether it is called from platform_driver_probe or not. > > Maybe using something other than platform_driver_register would work here. > > Arnd I think we can check inside the deferred_probe_work_func() if the dev->probe function pointer is equal to platform_drv_probe_fail(). Regards -- Fabio Porcedda -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 20 March 2013, Fabio Porcedda wrote: > I think we can check inside the deferred_probe_work_func() > if the dev->probe function pointer is equal to platform_drv_probe_fail(). I think it's too late by then, because that would only warn if we try to probe it again, but when platform_driver_probe() does not succeed immediately, it unregisters the driver again, so we never get there. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 20, 2013 at 11:20 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday 20 March 2013, Fabio Porcedda wrote: >> I think we can check inside the deferred_probe_work_func() >> if the dev->probe function pointer is equal to platform_drv_probe_fail(). > > I think it's too late by then, because that would only warn if we try to probe > it again, but when platform_driver_probe() does not succeed immediately, it Maybe you mean "does succeed immediately" ? > unregisters the driver again, so we never get there. -- Fabio Porcedda -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 20 March 2013, Fabio Porcedda wrote: > > On Wed, Mar 20, 2013 at 11:20 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Wednesday 20 March 2013, Fabio Porcedda wrote: > >> I think we can check inside the deferred_probe_work_func() > >> if the dev->probe function pointer is equal to platform_drv_probe_fail(). > > > > I think it's too late by then, because that would only warn if we try to probe > > it again, but when platform_driver_probe() does not succeed immediately, it > > Maybe you mean "does succeed immediately" ? I mean in this code (simplified for the sake of discussion) int __init_or_module platform_driver_probe(struct platform_driver *drv, int (*probe)(struct platform_device *)) { int retval, code; drv->probe = probe; retval = code = platform_driver_register(drv); drv->probe = NULL; if (code == 0 && list_empty(&drv->driver.p->klist_devices.k_list)) retval = -ENODEV; drv->driver.probe = platform_drv_probe_fail; if (code != retval) platform_driver_unregister(drv); return retval; } we assume that all devices are bound to drivers during the call to platform_driver_register, and if the device list is empty afterwards, we unregister the driver and will never get to the deferred probing stage. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 20, 2013 at 12:46 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday 20 March 2013, Fabio Porcedda wrote: >> >> On Wed, Mar 20, 2013 at 11:20 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> > On Wednesday 20 March 2013, Fabio Porcedda wrote: >> >> I think we can check inside the deferred_probe_work_func() >> >> if the dev->probe function pointer is equal to platform_drv_probe_fail(). >> > >> > I think it's too late by then, because that would only warn if we try to probe >> > it again, but when platform_driver_probe() does not succeed immediately, it >> >> Maybe you mean "does succeed immediately" ? > > I mean in this code (simplified for the sake of discussion) > > int __init_or_module platform_driver_probe(struct platform_driver *drv, > int (*probe)(struct platform_device *)) > { > int retval, code; > > drv->probe = probe; > retval = code = platform_driver_register(drv); > > drv->probe = NULL; > if (code == 0 && list_empty(&drv->driver.p->klist_devices.k_list)) > retval = -ENODEV; > drv->driver.probe = platform_drv_probe_fail; > > if (code != retval) > platform_driver_unregister(drv); > return retval; > } > > we assume that all devices are bound to drivers during the call to > platform_driver_register, and if the device list is empty afterwards, > we unregister the driver and will never get to the deferred probing > stage. Thanks for the explanation, I understand now that is not that simple. I was hoping it was easier. Regards -- Fabio Porcedda -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/misc/atmel_pwm.c b/drivers/misc/atmel_pwm.c index 28f5aaa..494d050 100644 --- a/drivers/misc/atmel_pwm.c +++ b/drivers/misc/atmel_pwm.c @@ -393,17 +393,7 @@ static struct platform_driver atmel_pwm_driver = { */ }; -static int __init pwm_init(void) -{ - return platform_driver_probe(&atmel_pwm_driver, pwm_probe); -} -module_init(pwm_init); - -static void __exit pwm_exit(void) -{ - platform_driver_unregister(&atmel_pwm_driver); -} -module_exit(pwm_exit); +module_platform_driver_probe(atmel_pwm_driver, pwm_probe); MODULE_DESCRIPTION("Driver for AT32/AT91 PWM module"); MODULE_LICENSE("GPL"); diff --git a/drivers/misc/ep93xx_pwm.c b/drivers/misc/ep93xx_pwm.c index 16d7179..96787ec 100644 --- a/drivers/misc/ep93xx_pwm.c +++ b/drivers/misc/ep93xx_pwm.c @@ -365,18 +365,7 @@ static struct platform_driver ep93xx_pwm_driver = { .remove = __exit_p(ep93xx_pwm_remove), }; -static int __init ep93xx_pwm_init(void) -{ - return platform_driver_probe(&ep93xx_pwm_driver, ep93xx_pwm_probe); -} - -static void __exit ep93xx_pwm_exit(void) -{ - platform_driver_unregister(&ep93xx_pwm_driver); -} - -module_init(ep93xx_pwm_init); -module_exit(ep93xx_pwm_exit); +module_platform_driver_probe(ep93xx_pwm_driver, ep93xx_pwm_probe); MODULE_AUTHOR("Matthieu Crapet <mcrapet@gmail.com>, " "H Hartley Sweeten <hsweeten@visionengravers.com>");
This patch converts the drivers to use the module_platform_driver_probe() macro which makes the code smaller and a bit simpler. Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Arnd Bergmann <arnd@arndb.de> --- drivers/misc/atmel_pwm.c | 12 +----------- drivers/misc/ep93xx_pwm.c | 13 +------------ 2 files changed, 2 insertions(+), 23 deletions(-)