[PATCH 0/2] misc: atmel_pwm: fix probe dependencies

[PATCH 0/2] misc: atmel_pwm: fix probe dependencies

[Johan Hovold]

Two drivers (atmel-pwm-bl and leds-atmel-pwm) currently depend on the
atmel_pwm driver to have bound to any pwm-device before their devices
are probed.

The first patch adds deferred-probing support to the atmel_pwm driver to
handle such dependencies. Note that the atmel-pwm-bl driver in
linux-next supports deferred probing since commit 9d3fde86b ("backlight:
atmel-pwm-bl: fix deferred probe from __init").

Although deferred probing is sufficient to deal with the dependency
problem, relying on deferred probing for the backlight driver is not
desirable as it may cause unnecessary delays before enabling the
backlight at boot. The second patch fixes this by making sure
pwm-devices are probed before any backlight devices.

Johan

Johan Hovold (2):
  misc: atmel_pwm: add deferred-probing support
  misc: atmel_pwm: set initcall level to subsys

 drivers/misc/atmel_pwm.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

-- 
1.8.4

[PATCH 1/2] misc: atmel_pwm: add deferred-probing support

[Johan Hovold]

Two drivers (atmel-pwm-bl and leds-atmel-pwm) currently depend on the
atmel_pwm driver to have bound to any pwm-device before their devices
are probed.

Support deferred probing of such devices by making sure to return
-EPROBE_DEFER from pwm_channel_alloc when no pwm-device has yet been
bound.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/misc/atmel_pwm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/atmel_pwm.c b/drivers/misc/atmel_pwm.c
index 494d050..a6dc56e 100644
--- a/drivers/misc/atmel_pwm.c
+++ b/drivers/misc/atmel_pwm.c
@@ -90,8 +90,10 @@ int pwm_channel_alloc(int index, struct pwm_channel *ch)
 	unsigned long	flags;
 	int		status = 0;
 
-	/* insist on PWM init, with this signal pinned out */
-	if (!pwm || !(pwm->mask & 1 << index))
+	if (!pwm)
+		return -EPROBE_DEFER;
+
+	if (!(pwm->mask & 1 << index))
 		return -ENODEV;
 
 	if (index < 0 || index >= PWM_NCHAN || !ch)
-- 
1.8.4

[PATCH 2/2] misc: atmel_pwm: set initcall level to subsys

[Johan Hovold]

Even with the atmel_pwm driver and the atmel-pwm-bl backlight driver
supporting deferred probing, we still want to make sure that any
pwm-device is available when the backlight devices are probed to avoid
any unnecessary delays before enabling the backlight.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/misc/atmel_pwm.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/atmel_pwm.c b/drivers/misc/atmel_pwm.c
index a6dc56e..0d0f599 100644
--- a/drivers/misc/atmel_pwm.c
+++ b/drivers/misc/atmel_pwm.c
@@ -395,7 +395,17 @@ static struct platform_driver atmel_pwm_driver = {
 	 */
 };
 
-module_platform_driver_probe(atmel_pwm_driver, pwm_probe);
+static int __init pwm_init(void)
+{
+	return platform_driver_probe(&atmel_pwm_driver, pwm_probe);
+}
+subsys_initcall(pwm_init);
+
+static void __exit pwm_exit(void)
+{
+	platform_driver_unregister(&atmel_pwm_driver);
+}
+module_exit(pwm_exit);
 
 MODULE_DESCRIPTION("Driver for AT32/AT91 PWM module");
 MODULE_LICENSE("GPL");
-- 
1.8.4

[PATCH 2/2] misc: atmel_pwm: set initcall level to subsys

[Greg Kroah-Hartman]

On Tue, Oct 22, 2013 at 06:32:40PM +0200, Johan Hovold wrote:
> Even with the atmel_pwm driver and the atmel-pwm-bl backlight driver
> supporting deferred probing, we still want to make sure that any
> pwm-device is available when the backlight devices are probed to avoid
> any unnecessary delays before enabling the backlight.
> 
> Signed-off-by: Johan Hovold <jhovold@gmail.com>
> ---
>  drivers/misc/atmel_pwm.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/atmel_pwm.c b/drivers/misc/atmel_pwm.c
> index a6dc56e..0d0f599 100644
> --- a/drivers/misc/atmel_pwm.c
> +++ b/drivers/misc/atmel_pwm.c
> @@ -395,7 +395,17 @@ static struct platform_driver atmel_pwm_driver = {
>  	 */
>  };
>  
> -module_platform_driver_probe(atmel_pwm_driver, pwm_probe);
> +static int __init pwm_init(void)
> +{
> +	return platform_driver_probe(&atmel_pwm_driver, pwm_probe);
> +}
> +subsys_initcall(pwm_init);

I really hate this type of patch, as it's papering over the real
problem.  What happens when someone else moves their driver to this
level?  Then you are back to the original problem.

This is what deferred probing was supposed to fix.  If it doesn't, then
something else needs to be done, or fix the deferred probing mess...

Sorry, I can't take this.

greg k-h

[PATCH 2/2] misc: atmel_pwm: set initcall level to subsys

[Johan Hovold]

On Tue, Oct 29, 2013 at 04:22:49PM -0700, Greg KH wrote:
> On Tue, Oct 22, 2013 at 06:32:40PM +0200, Johan Hovold wrote:
> > Even with the atmel_pwm driver and the atmel-pwm-bl backlight driver
> > supporting deferred probing, we still want to make sure that any
> > pwm-device is available when the backlight devices are probed to avoid
> > any unnecessary delays before enabling the backlight.
> > 
> > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> > ---
> >  drivers/misc/atmel_pwm.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/misc/atmel_pwm.c b/drivers/misc/atmel_pwm.c
> > index a6dc56e..0d0f599 100644
> > --- a/drivers/misc/atmel_pwm.c
> > +++ b/drivers/misc/atmel_pwm.c
> > @@ -395,7 +395,17 @@ static struct platform_driver atmel_pwm_driver = {
> >  	 */
> >  };
> >  
> > -module_platform_driver_probe(atmel_pwm_driver, pwm_probe);
> > +static int __init pwm_init(void)
> > +{
> > +	return platform_driver_probe(&atmel_pwm_driver, pwm_probe);
> > +}
> > +subsys_initcall(pwm_init);
> 
> I really hate this type of patch, as it's papering over the real
> problem.  What happens when someone else moves their driver to this
> level?  Then you are back to the original problem.

Yes, it's crude, but it's currently the only way to express a preferred
probe order.

> This is what deferred probing was supposed to fix.  If it doesn't, then
> something else needs to be done, or fix the deferred probing mess...

Deferred probing (the first patch) fixes the dependency problem, but may
introduce delays. I can live with that.

> Sorry, I can't take this.

Fair enough.

Thanks,
Johan

[PATCH 1/2] misc: atmel_pwm: add deferred-probing support

[Johan Hovold]

On Tue, Oct 22, 2013 at 06:32:39PM +0200, Johan Hovold wrote:
> Two drivers (atmel-pwm-bl and leds-atmel-pwm) currently depend on the
> atmel_pwm driver to have bound to any pwm-device before their devices
> are probed.
> 
> Support deferred probing of such devices by making sure to return
> -EPROBE_DEFER from pwm_channel_alloc when no pwm-device has yet been
> bound.
> 
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

This patch, 5c6d6fd1 ("misc: atmel_pwm: add deferred-probing support"),
and 9d3fde86 ("backlight: atmel-pwm-bl: fix deferred probe from __init")
in linux-next should probably be considered for inclusion in stable.

Without them the atmel-pwm-bl driver may always fail to probe depending
on link order.

Thanks,
Johan

> ---
>  drivers/misc/atmel_pwm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/atmel_pwm.c b/drivers/misc/atmel_pwm.c
> index 494d050..a6dc56e 100644
> --- a/drivers/misc/atmel_pwm.c
> +++ b/drivers/misc/atmel_pwm.c
> @@ -90,8 +90,10 @@ int pwm_channel_alloc(int index, struct pwm_channel *ch)
>  	unsigned long	flags;
>  	int		status = 0;
>  
> -	/* insist on PWM init, with this signal pinned out */
> -	if (!pwm || !(pwm->mask & 1 << index))
> +	if (!pwm)
> +		return -EPROBE_DEFER;
> +
> +	if (!(pwm->mask & 1 << index))
>  		return -ENODEV;
>  
>  	if (index < 0 || index >= PWM_NCHAN || !ch)
> -- 
> 1.8.4
> 

[PATCH 1/2] misc: atmel_pwm: add deferred-probing support

[Greg Kroah-Hartman]

On Wed, Oct 30, 2013 at 12:25:51PM +0100, Johan Hovold wrote:
> On Tue, Oct 22, 2013 at 06:32:39PM +0200, Johan Hovold wrote:
> > Two drivers (atmel-pwm-bl and leds-atmel-pwm) currently depend on the
> > atmel_pwm driver to have bound to any pwm-device before their devices
> > are probed.
> > 
> > Support deferred probing of such devices by making sure to return
> > -EPROBE_DEFER from pwm_channel_alloc when no pwm-device has yet been
> > bound.
> > 
> > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> 
> This patch, 5c6d6fd1 ("misc: atmel_pwm: add deferred-probing support"),
> and 9d3fde86 ("backlight: atmel-pwm-bl: fix deferred probe from __init")
> in linux-next should probably be considered for inclusion in stable.
> 
> Without them the atmel-pwm-bl driver may always fail to probe depending
> on link order.

Ok, I'll watch this for when it hits Linus's tree, thanks for letting me
know.

greg k-h

[PATCH 2/2] misc: atmel_pwm: set initcall level to subsys

[Arnd Bergmann]

On Wednesday 30 October 2013, Johan Hovold wrote:
> > I really hate this type of patch, as it's papering over the real
> > problem.  What happens when someone else moves their driver to this
> > level?  Then you are back to the original problem.
> 
> Yes, it's crude, but it's currently the only way to express a preferred
> probe order.

Does the same problem occur with the generic PWM subsystem?

The atmel_pwm driver apparently is the last PWM driver that has
not been converted to the generic subsystem yet. Doing that would
also let us drop the atmel-pwm-bl driver in favor of the generic
backlight.

	Arnd

[PATCH 2/2] misc: atmel_pwm: set initcall level to subsys

[Bo Shen]

Hi Arnd,

On 10/31/2013 04:46, Arnd Bergmann wrote:
> On Wednesday 30 October 2013, Johan Hovold wrote:
>>> I really hate this type of patch, as it's papering over the real
>>> problem.  What happens when someone else moves their driver to this
>>> level?  Then you are back to the original problem.
>>
>> Yes, it's crude, but it's currently the only way to express a preferred
>> probe order.
>
> Does the same problem occur with the generic PWM subsystem?
>
> The atmel_pwm driver apparently is the last PWM driver that has
> not been converted to the generic subsystem yet. Doing that would
> also let us drop the atmel-pwm-bl driver in favor of the generic
> backlight.

The atmel pwm driver is on the way to generic subsystem. more 
information at:
--->8---
PWM: atmel-pwm: add PWM controller driver:
    https://patchwork.kernel.org/patch/2963081/
---<8---

Best Regards,
Bo Shen

[PATCH 2/2] misc: atmel_pwm: set initcall level to subsys

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:19 Thu 31 Oct     , Bo Shen wrote:
> Hi Arnd,
> 
> On 10/31/2013 04:46, Arnd Bergmann wrote:
> >On Wednesday 30 October 2013, Johan Hovold wrote:
> >>>I really hate this type of patch, as it's papering over the real
> >>>problem.  What happens when someone else moves their driver to this
> >>>level?  Then you are back to the original problem.
> >>
> >>Yes, it's crude, but it's currently the only way to express a preferred
> >>probe order.
> >
> >Does the same problem occur with the generic PWM subsystem?
> >
> >The atmel_pwm driver apparently is the last PWM driver that has
> >not been converted to the generic subsystem yet. Doing that would
> >also let us drop the atmel-pwm-bl driver in favor of the generic
> >backlight.
> 
> The atmel pwm driver is on the way to generic subsystem. more
> information at:
> --->8---
> PWM: atmel-pwm: add PWM controller driver:
>    https://patchwork.kernel.org/patch/2963081/

so the fix can be drop in favor of the conversion

Best Regards,
J.
> ---<8---
> 
> Best Regards,
> Bo Shen
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] misc: atmel_pwm: set initcall level to subsys

[Johan Hovold]

On Wed, Oct 30, 2013 at 09:46:17PM +0100, Arnd Bergmann wrote:
> On Wednesday 30 October 2013, Johan Hovold wrote:
> > > I really hate this type of patch, as it's papering over the real
> > > problem.  What happens when someone else moves their driver to this
> > > level?  Then you are back to the original problem.
> > 
> > Yes, it's crude, but it's currently the only way to express a preferred
> > probe order.
> 
> Does the same problem occur with the generic PWM subsystem?

It does not suffer from the problem with failed probe as it supports
deferred probing, but there could be added delays depending on probe
order.

> The atmel_pwm driver apparently is the last PWM driver that has
> not been converted to the generic subsystem yet. Doing that would
> also let us drop the atmel-pwm-bl driver in favor of the generic
> backlight.

As Bo mentioned, it's work in progress that hasn't been merged yet.

Johan

[PATCH 2/2] misc: atmel_pwm: set initcall level to subsys

[Johan Hovold]

On Thu, Oct 31, 2013 at 06:30:37AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 09:19 Thu 31 Oct     , Bo Shen wrote:
> > Hi Arnd,
> > 
> > On 10/31/2013 04:46, Arnd Bergmann wrote:
> > >On Wednesday 30 October 2013, Johan Hovold wrote:
> > >>>I really hate this type of patch, as it's papering over the real
> > >>>problem.  What happens when someone else moves their driver to this
> > >>>level?  Then you are back to the original problem.
> > >>
> > >>Yes, it's crude, but it's currently the only way to express a preferred
> > >>probe order.
> > >
> > >Does the same problem occur with the generic PWM subsystem?
> > >
> > >The atmel_pwm driver apparently is the last PWM driver that has
> > >not been converted to the generic subsystem yet. Doing that would
> > >also let us drop the atmel-pwm-bl driver in favor of the generic
> > >backlight.
> > 
> > The atmel pwm driver is on the way to generic subsystem. more
> > information at:
> > --->8---
> > PWM: atmel-pwm: add PWM controller driver:
> >    https://patchwork.kernel.org/patch/2963081/
> 
> so the fix can be drop in favor of the conversion

This patch (2/2) has been dropped, but the dependency issue still has to
be addressed (by adding deferred-probing support). The atmel-pwm-bl
driver has not been removed yet and the fix is also needed in stable.

Johan