All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
@ 2018-07-26 13:36 ` David Rivshin
  0 siblings, 0 replies; 12+ messages in thread
From: David Rivshin @ 2018-07-26 13:36 UTC (permalink / raw)
  To: Thierry Reding, Keerthy, Ladislav Michl, Neil Armstrong
  Cc: linux-pwm, Tony Lindgren, Pavel Machek, Thomas Gleixner,
	linux-omap, linux-arm-kernel

From: David Rivshin <DRivshin@allworx.com>


If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
data won't be set yet.

Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
Cc: <stable@vger.kernel.org> # 4.17+
Signed-off-by: David Rivshin <drivshin@allworx.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Pavel Machek <pavel@ucw.cz>
---
Changes in v2:
* Added Pavel's Acked-by/Tested-by [1]

[1] https://lkml.org/lkml/2018/7/16/346

 drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
index 665da3c8fbceb..d3d7ea7a53146 100644
--- a/drivers/pwm/pwm-omap-dmtimer.c
+++ b/drivers/pwm/pwm-omap-dmtimer.c
@@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
 
 	timer_pdata = dev_get_platdata(&timer_pdev->dev);
 	if (!timer_pdata) {
-		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
-		ret = -EINVAL;
+		dev_info(&pdev->dev,
+			 "dmtimer pdata structure NULL, deferring probe\n");
+		ret = -EPROBE_DEFER;
 		goto put;
 	}
 

base-commit: d72e90f33aa4709ebecc5005562f52335e106a60
-- 
2.17.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
@ 2018-07-26 13:36 ` David Rivshin
  0 siblings, 0 replies; 12+ messages in thread
From: David Rivshin @ 2018-07-26 13:36 UTC (permalink / raw)
  To: linux-arm-kernel

From: David Rivshin <DRivshin@allworx.com>


If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
data won't be set yet.

Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
Cc: <stable@vger.kernel.org> # 4.17+
Signed-off-by: David Rivshin <drivshin@allworx.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Pavel Machek <pavel@ucw.cz>
---
Changes in v2:
* Added Pavel's Acked-by/Tested-by [1]

[1] https://lkml.org/lkml/2018/7/16/346

 drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
index 665da3c8fbceb..d3d7ea7a53146 100644
--- a/drivers/pwm/pwm-omap-dmtimer.c
+++ b/drivers/pwm/pwm-omap-dmtimer.c
@@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
 
 	timer_pdata = dev_get_platdata(&timer_pdev->dev);
 	if (!timer_pdata) {
-		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
-		ret = -EINVAL;
+		dev_info(&pdev->dev,
+			 "dmtimer pdata structure NULL, deferring probe\n");
+		ret = -EPROBE_DEFER;
 		goto put;
 	}
 

base-commit: d72e90f33aa4709ebecc5005562f52335e106a60
-- 
2.17.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
  2018-07-26 13:36 ` David Rivshin
@ 2018-07-26 18:54   ` Ladislav Michl
  -1 siblings, 0 replies; 12+ messages in thread
From: Ladislav Michl @ 2018-07-26 18:54 UTC (permalink / raw)
  To: David Rivshin
  Cc: linux-pwm, Neil Armstrong, Tony Lindgren, Keerthy,
	Thierry Reding, Pavel Machek, Thomas Gleixner, linux-omap,
	linux-arm-kernel

On Thu, Jul 26, 2018 at 09:36:58AM -0400, David Rivshin wrote:
> From: David Rivshin <DRivshin@allworx.com>
> 
> If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
> data won't be set yet.
> 
> Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
> Cc: <stable@vger.kernel.org> # 4.17+
> Signed-off-by: David Rivshin <drivshin@allworx.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Tested-by: Pavel Machek <pavel@ucw.cz>
> ---
> Changes in v2:
> * Added Pavel's Acked-by/Tested-by [1]
> 
> [1] https://lkml.org/lkml/2018/7/16/346
> 
>  drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
> index 665da3c8fbceb..d3d7ea7a53146 100644
> --- a/drivers/pwm/pwm-omap-dmtimer.c
> +++ b/drivers/pwm/pwm-omap-dmtimer.c
> @@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
>  
>  	timer_pdata = dev_get_platdata(&timer_pdev->dev);
>  	if (!timer_pdata) {
> -		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> -		ret = -EINVAL;
> +		dev_info(&pdev->dev,
> +			 "dmtimer pdata structure NULL, deferring probe\n");

This seems to be a bit verbose for EPROBE_DEFER case. Could we either remove
it as it is done later in pdata->request_by_node(timer) failure case or at
least make it dev_dbg? Otherwise thank you and with mentioned change
Acked-by: Ladislav Michl <ladis@linux-mips.org>

> +		ret = -EPROBE_DEFER;
>  		goto put;
>  	}
>  
> 
> base-commit: d72e90f33aa4709ebecc5005562f52335e106a60
> -- 
> 2.17.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
@ 2018-07-26 18:54   ` Ladislav Michl
  0 siblings, 0 replies; 12+ messages in thread
From: Ladislav Michl @ 2018-07-26 18:54 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jul 26, 2018 at 09:36:58AM -0400, David Rivshin wrote:
> From: David Rivshin <DRivshin@allworx.com>
> 
> If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
> data won't be set yet.
> 
> Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
> Cc: <stable@vger.kernel.org> # 4.17+
> Signed-off-by: David Rivshin <drivshin@allworx.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Tested-by: Pavel Machek <pavel@ucw.cz>
> ---
> Changes in v2:
> * Added Pavel's Acked-by/Tested-by [1]
> 
> [1] https://lkml.org/lkml/2018/7/16/346
> 
>  drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
> index 665da3c8fbceb..d3d7ea7a53146 100644
> --- a/drivers/pwm/pwm-omap-dmtimer.c
> +++ b/drivers/pwm/pwm-omap-dmtimer.c
> @@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
>  
>  	timer_pdata = dev_get_platdata(&timer_pdev->dev);
>  	if (!timer_pdata) {
> -		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> -		ret = -EINVAL;
> +		dev_info(&pdev->dev,
> +			 "dmtimer pdata structure NULL, deferring probe\n");

This seems to be a bit verbose for EPROBE_DEFER case. Could we either remove
it as it is done later in pdata->request_by_node(timer) failure case or at
least make it dev_dbg? Otherwise thank you and with mentioned change
Acked-by: Ladislav Michl <ladis@linux-mips.org>

> +		ret = -EPROBE_DEFER;
>  		goto put;
>  	}
>  
> 
> base-commit: d72e90f33aa4709ebecc5005562f52335e106a60
> -- 
> 2.17.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
  2018-07-26 18:54   ` Ladislav Michl
@ 2018-07-26 20:37     ` David Rivshin
  -1 siblings, 0 replies; 12+ messages in thread
From: David Rivshin @ 2018-07-26 20:37 UTC (permalink / raw)
  To: Ladislav Michl, Thierry Reding, Keerthy, Neil Armstrong,
	Tony Lindgren, Thomas Gleixner, Pavel Machek
  Cc: linux-pwm, linux-omap, linux-arm-kernel

On Thu, 26 Jul 2018 20:54:26 +0200
Ladislav Michl <ladis@linux-mips.org> wrote:

> On Thu, Jul 26, 2018 at 09:36:58AM -0400, David Rivshin wrote:
> > From: David Rivshin <DRivshin@allworx.com>
> > 
> > If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
> > data won't be set yet.
> > 
> > Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
> > Cc: <stable@vger.kernel.org> # 4.17+
> > Signed-off-by: David Rivshin <drivshin@allworx.com>
> > Acked-by: Pavel Machek <pavel@ucw.cz>
> > Tested-by: Pavel Machek <pavel@ucw.cz>
> > ---
> > Changes in v2:
> > * Added Pavel's Acked-by/Tested-by [1]
> > 
> > [1] https://lkml.org/lkml/2018/7/16/346
> > 
> >  drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
> > index 665da3c8fbceb..d3d7ea7a53146 100644
> > --- a/drivers/pwm/pwm-omap-dmtimer.c
> > +++ b/drivers/pwm/pwm-omap-dmtimer.c
> > @@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
> >  
> >  	timer_pdata = dev_get_platdata(&timer_pdev->dev);
> >  	if (!timer_pdata) {
> > -		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> > -		ret = -EINVAL;
> > +		dev_info(&pdev->dev,
> > +			 "dmtimer pdata structure NULL, deferring probe\n");  
> 
> This seems to be a bit verbose for EPROBE_DEFER case. Could we either remove
> it as it is done later in pdata->request_by_node(timer) failure case or at
> least make it dev_dbg? Otherwise thank you and with mentioned change
> Acked-by: Ladislav Michl <ladis@linux-mips.org>

Hi Ladislav, thanks for the review.

I had grepped through other drivers and found no consistent pattern. Some
places used dev_err still, others reduced to one of dev_{warn,info,dbg}, 
and others no message at all. Some messages mentioned they are deferring 
the probe, other didn't. I was already getting a couple of dev_info from 
the pinctrl core code, so I went that way. I figured the message might be
useful to someone, but I don't feel strongly.

I personally would lean to dev_dbg if you think dev_info is too harsh, 
just in case someone's board suddenly isn't working after upgrade. But 
I'm certainly willing to remove the message entirely if you feel strongly,
or anyone else cares to weigh in.


> 
> > +		ret = -EPROBE_DEFER;
> >  		goto put;
> >  	}
> >  
> > 
> > base-commit: d72e90f33aa4709ebecc5005562f52335e106a60
> > -- 
> > 2.17.1  

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
@ 2018-07-26 20:37     ` David Rivshin
  0 siblings, 0 replies; 12+ messages in thread
From: David Rivshin @ 2018-07-26 20:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 26 Jul 2018 20:54:26 +0200
Ladislav Michl <ladis@linux-mips.org> wrote:

> On Thu, Jul 26, 2018 at 09:36:58AM -0400, David Rivshin wrote:
> > From: David Rivshin <DRivshin@allworx.com>
> > 
> > If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
> > data won't be set yet.
> > 
> > Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
> > Cc: <stable@vger.kernel.org> # 4.17+
> > Signed-off-by: David Rivshin <drivshin@allworx.com>
> > Acked-by: Pavel Machek <pavel@ucw.cz>
> > Tested-by: Pavel Machek <pavel@ucw.cz>
> > ---
> > Changes in v2:
> > * Added Pavel's Acked-by/Tested-by [1]
> > 
> > [1] https://lkml.org/lkml/2018/7/16/346
> > 
> >  drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
> > index 665da3c8fbceb..d3d7ea7a53146 100644
> > --- a/drivers/pwm/pwm-omap-dmtimer.c
> > +++ b/drivers/pwm/pwm-omap-dmtimer.c
> > @@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
> >  
> >  	timer_pdata = dev_get_platdata(&timer_pdev->dev);
> >  	if (!timer_pdata) {
> > -		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> > -		ret = -EINVAL;
> > +		dev_info(&pdev->dev,
> > +			 "dmtimer pdata structure NULL, deferring probe\n");  
> 
> This seems to be a bit verbose for EPROBE_DEFER case. Could we either remove
> it as it is done later in pdata->request_by_node(timer) failure case or at
> least make it dev_dbg? Otherwise thank you and with mentioned change
> Acked-by: Ladislav Michl <ladis@linux-mips.org>

Hi Ladislav, thanks for the review.

I had grepped through other drivers and found no consistent pattern. Some
places used dev_err still, others reduced to one of dev_{warn,info,dbg}, 
and others no message at all. Some messages mentioned they are deferring 
the probe, other didn't. I was already getting a couple of dev_info from 
the pinctrl core code, so I went that way. I figured the message might be
useful to someone, but I don't feel strongly.

I personally would lean to dev_dbg if you think dev_info is too harsh, 
just in case someone's board suddenly isn't working after upgrade. But 
I'm certainly willing to remove the message entirely if you feel strongly,
or anyone else cares to weigh in.


> 
> > +		ret = -EPROBE_DEFER;
> >  		goto put;
> >  	}
> >  
> > 
> > base-commit: d72e90f33aa4709ebecc5005562f52335e106a60
> > -- 
> > 2.17.1  

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
  2018-07-26 20:37     ` David Rivshin
@ 2018-07-27  7:19       ` Ladislav Michl
  -1 siblings, 0 replies; 12+ messages in thread
From: Ladislav Michl @ 2018-07-27  7:19 UTC (permalink / raw)
  To: David Rivshin
  Cc: linux-pwm, Neil Armstrong, Tony Lindgren, Keerthy,
	Thierry Reding, Pavel Machek, Thomas Gleixner, linux-omap,
	linux-arm-kernel

On Thu, Jul 26, 2018 at 04:37:05PM -0400, David Rivshin wrote:
> On Thu, 26 Jul 2018 20:54:26 +0200
> Ladislav Michl <ladis@linux-mips.org> wrote:
> 
> > On Thu, Jul 26, 2018 at 09:36:58AM -0400, David Rivshin wrote:
> > > From: David Rivshin <DRivshin@allworx.com>
> > > 
> > > If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
> > > data won't be set yet.
> > > 
> > > Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
> > > Cc: <stable@vger.kernel.org> # 4.17+
> > > Signed-off-by: David Rivshin <drivshin@allworx.com>
> > > Acked-by: Pavel Machek <pavel@ucw.cz>
> > > Tested-by: Pavel Machek <pavel@ucw.cz>
> > > ---
> > > Changes in v2:
> > > * Added Pavel's Acked-by/Tested-by [1]
> > > 
> > > [1] https://lkml.org/lkml/2018/7/16/346
> > > 
> > >  drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
> > > index 665da3c8fbceb..d3d7ea7a53146 100644
> > > --- a/drivers/pwm/pwm-omap-dmtimer.c
> > > +++ b/drivers/pwm/pwm-omap-dmtimer.c
> > > @@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
> > >  
> > >  	timer_pdata = dev_get_platdata(&timer_pdev->dev);
> > >  	if (!timer_pdata) {
> > > -		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> > > -		ret = -EINVAL;
> > > +		dev_info(&pdev->dev,
> > > +			 "dmtimer pdata structure NULL, deferring probe\n");  
> > 
> > This seems to be a bit verbose for EPROBE_DEFER case. Could we either remove
> > it as it is done later in pdata->request_by_node(timer) failure case or at
> > least make it dev_dbg? Otherwise thank you and with mentioned change
> > Acked-by: Ladislav Michl <ladis@linux-mips.org>
> 
> Hi Ladislav, thanks for the review.
> 
> I had grepped through other drivers and found no consistent pattern. Some
> places used dev_err still, others reduced to one of dev_{warn,info,dbg}, 
> and others no message at all. Some messages mentioned they are deferring 
> the probe, other didn't. I was already getting a couple of dev_info from 
> the pinctrl core code, so I went that way. I figured the message might be
> useful to someone, but I don't feel strongly.

Well, pinctrl probe deferal message is a bit annoying. It really does not
tell us much as long as pins are correctly configured and in case they are
not it is useless as well :)

> I personally would lean to dev_dbg if you think dev_info is too harsh, 
> just in case someone's board suddenly isn't working after upgrade. But 
> I'm certainly willing to remove the message entirely if you feel strongly,
> or anyone else cares to weigh in.

I'm fine with dev_dbg as well.

Thank you,
	ladis

> > 
> > > +		ret = -EPROBE_DEFER;
> > >  		goto put;
> > >  	}
> > >  
> > > 
> > > base-commit: d72e90f33aa4709ebecc5005562f52335e106a60
> > > -- 
> > > 2.17.1  

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
@ 2018-07-27  7:19       ` Ladislav Michl
  0 siblings, 0 replies; 12+ messages in thread
From: Ladislav Michl @ 2018-07-27  7:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jul 26, 2018 at 04:37:05PM -0400, David Rivshin wrote:
> On Thu, 26 Jul 2018 20:54:26 +0200
> Ladislav Michl <ladis@linux-mips.org> wrote:
> 
> > On Thu, Jul 26, 2018 at 09:36:58AM -0400, David Rivshin wrote:
> > > From: David Rivshin <DRivshin@allworx.com>
> > > 
> > > If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
> > > data won't be set yet.
> > > 
> > > Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
> > > Cc: <stable@vger.kernel.org> # 4.17+
> > > Signed-off-by: David Rivshin <drivshin@allworx.com>
> > > Acked-by: Pavel Machek <pavel@ucw.cz>
> > > Tested-by: Pavel Machek <pavel@ucw.cz>
> > > ---
> > > Changes in v2:
> > > * Added Pavel's Acked-by/Tested-by [1]
> > > 
> > > [1] https://lkml.org/lkml/2018/7/16/346
> > > 
> > >  drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
> > > index 665da3c8fbceb..d3d7ea7a53146 100644
> > > --- a/drivers/pwm/pwm-omap-dmtimer.c
> > > +++ b/drivers/pwm/pwm-omap-dmtimer.c
> > > @@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
> > >  
> > >  	timer_pdata = dev_get_platdata(&timer_pdev->dev);
> > >  	if (!timer_pdata) {
> > > -		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> > > -		ret = -EINVAL;
> > > +		dev_info(&pdev->dev,
> > > +			 "dmtimer pdata structure NULL, deferring probe\n");  
> > 
> > This seems to be a bit verbose for EPROBE_DEFER case. Could we either remove
> > it as it is done later in pdata->request_by_node(timer) failure case or at
> > least make it dev_dbg? Otherwise thank you and with mentioned change
> > Acked-by: Ladislav Michl <ladis@linux-mips.org>
> 
> Hi Ladislav, thanks for the review.
> 
> I had grepped through other drivers and found no consistent pattern. Some
> places used dev_err still, others reduced to one of dev_{warn,info,dbg}, 
> and others no message at all. Some messages mentioned they are deferring 
> the probe, other didn't. I was already getting a couple of dev_info from 
> the pinctrl core code, so I went that way. I figured the message might be
> useful to someone, but I don't feel strongly.

Well, pinctrl probe deferal message is a bit annoying. It really does not
tell us much as long as pins are correctly configured and in case they are
not it is useless as well :)

> I personally would lean to dev_dbg if you think dev_info is too harsh, 
> just in case someone's board suddenly isn't working after upgrade. But 
> I'm certainly willing to remove the message entirely if you feel strongly,
> or anyone else cares to weigh in.

I'm fine with dev_dbg as well.

Thank you,
	ladis

> > 
> > > +		ret = -EPROBE_DEFER;
> > >  		goto put;
> > >  	}
> > >  
> > > 
> > > base-commit: d72e90f33aa4709ebecc5005562f52335e106a60
> > > -- 
> > > 2.17.1  

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
  2018-07-27  7:19       ` Ladislav Michl
@ 2018-07-27 11:00         ` Pavel Machek
  -1 siblings, 0 replies; 12+ messages in thread
From: Pavel Machek @ 2018-07-27 11:00 UTC (permalink / raw)
  To: Ladislav Michl
  Cc: linux-pwm, David Rivshin, Neil Armstrong, Tony Lindgren, Keerthy,
	Thierry Reding, Thomas Gleixner, linux-omap, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 3139 bytes --]

On Fri 2018-07-27 09:19:01, Ladislav Michl wrote:
> On Thu, Jul 26, 2018 at 04:37:05PM -0400, David Rivshin wrote:
> > On Thu, 26 Jul 2018 20:54:26 +0200
> > Ladislav Michl <ladis@linux-mips.org> wrote:
> > 
> > > On Thu, Jul 26, 2018 at 09:36:58AM -0400, David Rivshin wrote:
> > > > From: David Rivshin <DRivshin@allworx.com>
> > > > 
> > > > If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
> > > > data won't be set yet.
> > > > 
> > > > Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
> > > > Cc: <stable@vger.kernel.org> # 4.17+
> > > > Signed-off-by: David Rivshin <drivshin@allworx.com>
> > > > Acked-by: Pavel Machek <pavel@ucw.cz>
> > > > Tested-by: Pavel Machek <pavel@ucw.cz>
> > > > ---
> > > > Changes in v2:
> > > > * Added Pavel's Acked-by/Tested-by [1]
> > > > 
> > > > [1] https://lkml.org/lkml/2018/7/16/346
> > > > 
> > > >  drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
> > > > index 665da3c8fbceb..d3d7ea7a53146 100644
> > > > --- a/drivers/pwm/pwm-omap-dmtimer.c
> > > > +++ b/drivers/pwm/pwm-omap-dmtimer.c
> > > > @@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
> > > >  
> > > >  	timer_pdata = dev_get_platdata(&timer_pdev->dev);
> > > >  	if (!timer_pdata) {
> > > > -		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> > > > -		ret = -EINVAL;
> > > > +		dev_info(&pdev->dev,
> > > > +			 "dmtimer pdata structure NULL, deferring probe\n");  
> > > 
> > > This seems to be a bit verbose for EPROBE_DEFER case. Could we either remove
> > > it as it is done later in pdata->request_by_node(timer) failure case or at
> > > least make it dev_dbg? Otherwise thank you and with mentioned change
> > > Acked-by: Ladislav Michl <ladis@linux-mips.org>
> > 
> > Hi Ladislav, thanks for the review.
> > 
> > I had grepped through other drivers and found no consistent pattern. Some
> > places used dev_err still, others reduced to one of dev_{warn,info,dbg}, 
> > and others no message at all. Some messages mentioned they are deferring 
> > the probe, other didn't. I was already getting a couple of dev_info from 
> > the pinctrl core code, so I went that way. I figured the message might be
> > useful to someone, but I don't feel strongly.
> 
> Well, pinctrl probe deferal message is a bit annoying. It really does not
> tell us much as long as pins are correctly configured and in case they are
> not it is useless as well :)
> 
> > I personally would lean to dev_dbg if you think dev_info is too harsh, 
> > just in case someone's board suddenly isn't working after upgrade. But 
> > I'm certainly willing to remove the message entirely if you feel strongly,
> > or anyone else cares to weigh in.
> 
> I'm fine with dev_dbg as well.

Looks good to me, too.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
@ 2018-07-27 11:00         ` Pavel Machek
  0 siblings, 0 replies; 12+ messages in thread
From: Pavel Machek @ 2018-07-27 11:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri 2018-07-27 09:19:01, Ladislav Michl wrote:
> On Thu, Jul 26, 2018 at 04:37:05PM -0400, David Rivshin wrote:
> > On Thu, 26 Jul 2018 20:54:26 +0200
> > Ladislav Michl <ladis@linux-mips.org> wrote:
> > 
> > > On Thu, Jul 26, 2018 at 09:36:58AM -0400, David Rivshin wrote:
> > > > From: David Rivshin <DRivshin@allworx.com>
> > > > 
> > > > If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
> > > > data won't be set yet.
> > > > 
> > > > Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
> > > > Cc: <stable@vger.kernel.org> # 4.17+
> > > > Signed-off-by: David Rivshin <drivshin@allworx.com>
> > > > Acked-by: Pavel Machek <pavel@ucw.cz>
> > > > Tested-by: Pavel Machek <pavel@ucw.cz>
> > > > ---
> > > > Changes in v2:
> > > > * Added Pavel's Acked-by/Tested-by [1]
> > > > 
> > > > [1] https://lkml.org/lkml/2018/7/16/346
> > > > 
> > > >  drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
> > > > index 665da3c8fbceb..d3d7ea7a53146 100644
> > > > --- a/drivers/pwm/pwm-omap-dmtimer.c
> > > > +++ b/drivers/pwm/pwm-omap-dmtimer.c
> > > > @@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
> > > >  
> > > >  	timer_pdata = dev_get_platdata(&timer_pdev->dev);
> > > >  	if (!timer_pdata) {
> > > > -		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> > > > -		ret = -EINVAL;
> > > > +		dev_info(&pdev->dev,
> > > > +			 "dmtimer pdata structure NULL, deferring probe\n");  
> > > 
> > > This seems to be a bit verbose for EPROBE_DEFER case. Could we either remove
> > > it as it is done later in pdata->request_by_node(timer) failure case or at
> > > least make it dev_dbg? Otherwise thank you and with mentioned change
> > > Acked-by: Ladislav Michl <ladis@linux-mips.org>
> > 
> > Hi Ladislav, thanks for the review.
> > 
> > I had grepped through other drivers and found no consistent pattern. Some
> > places used dev_err still, others reduced to one of dev_{warn,info,dbg}, 
> > and others no message at all. Some messages mentioned they are deferring 
> > the probe, other didn't. I was already getting a couple of dev_info from 
> > the pinctrl core code, so I went that way. I figured the message might be
> > useful to someone, but I don't feel strongly.
> 
> Well, pinctrl probe deferal message is a bit annoying. It really does not
> tell us much as long as pins are correctly configured and in case they are
> not it is useless as well :)
> 
> > I personally would lean to dev_dbg if you think dev_info is too harsh, 
> > just in case someone's board suddenly isn't working after upgrade. But 
> > I'm certainly willing to remove the message entirely if you feel strongly,
> > or anyone else cares to weigh in.
> 
> I'm fine with dev_dbg as well.

Looks good to me, too.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180727/8e98238b/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
  2018-07-27 11:00         ` Pavel Machek
@ 2018-07-27 13:42           ` David Rivshin
  -1 siblings, 0 replies; 12+ messages in thread
From: David Rivshin @ 2018-07-27 13:42 UTC (permalink / raw)
  To: Pavel Machek, Ladislav Michl
  Cc: linux-pwm, Neil Armstrong, Tony Lindgren, Keerthy,
	Thierry Reding, Thomas Gleixner, linux-omap, linux-arm-kernel

On Fri, 27 Jul 2018 13:00:06 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> On Fri 2018-07-27 09:19:01, Ladislav Michl wrote:
> > On Thu, Jul 26, 2018 at 04:37:05PM -0400, David Rivshin wrote:  
> > > On Thu, 26 Jul 2018 20:54:26 +0200
> > > Ladislav Michl <ladis@linux-mips.org> wrote:
> > >   
> > > > On Thu, Jul 26, 2018 at 09:36:58AM -0400, David Rivshin wrote:  
> > > > > From: David Rivshin <DRivshin@allworx.com>
> > > > > 
> > > > > If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
> > > > > data won't be set yet.
> > > > > 
> > > > > Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
> > > > > Cc: <stable@vger.kernel.org> # 4.17+
> > > > > Signed-off-by: David Rivshin <drivshin@allworx.com>
> > > > > Acked-by: Pavel Machek <pavel@ucw.cz>
> > > > > Tested-by: Pavel Machek <pavel@ucw.cz>
> > > > > ---
> > > > > Changes in v2:
> > > > > * Added Pavel's Acked-by/Tested-by [1]
> > > > > 
> > > > > [1] https://lkml.org/lkml/2018/7/16/346
> > > > > 
> > > > >  drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
> > > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
> > > > > index 665da3c8fbceb..d3d7ea7a53146 100644
> > > > > --- a/drivers/pwm/pwm-omap-dmtimer.c
> > > > > +++ b/drivers/pwm/pwm-omap-dmtimer.c
> > > > > @@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
> > > > >  
> > > > >  	timer_pdata = dev_get_platdata(&timer_pdev->dev);
> > > > >  	if (!timer_pdata) {
> > > > > -		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> > > > > -		ret = -EINVAL;
> > > > > +		dev_info(&pdev->dev,
> > > > > +			 "dmtimer pdata structure NULL, deferring probe\n");    
> > > > 
> > > > This seems to be a bit verbose for EPROBE_DEFER case. Could we either remove
> > > > it as it is done later in pdata->request_by_node(timer) failure case or at
> > > > least make it dev_dbg? Otherwise thank you and with mentioned change
> > > > Acked-by: Ladislav Michl <ladis@linux-mips.org>  
> > > 
> > > Hi Ladislav, thanks for the review.
> > > 
> > > I had grepped through other drivers and found no consistent pattern. Some
> > > places used dev_err still, others reduced to one of dev_{warn,info,dbg}, 
> > > and others no message at all. Some messages mentioned they are deferring 
> > > the probe, other didn't. I was already getting a couple of dev_info from 
> > > the pinctrl core code, so I went that way. I figured the message might be
> > > useful to someone, but I don't feel strongly.  
> > 
> > Well, pinctrl probe deferal message is a bit annoying. It really does not
> > tell us much as long as pins are correctly configured and in case they are
> > not it is useless as well :)
> >   
> > > I personally would lean to dev_dbg if you think dev_info is too harsh, 
> > > just in case someone's board suddenly isn't working after upgrade. But 
> > > I'm certainly willing to remove the message entirely if you feel strongly,
> > > or anyone else cares to weigh in.  
> > 
> > I'm fine with dev_dbg as well.  
> 
> Looks good to me, too.

OK, I'll spin a v3 with dev_dbg.

Pavel, I'm not sure of the normal etiquette: since I'm making a minor 
change change to the code, should I keep your Tested-by, or drop it 
until/if you test again? I assume, based on your response, that you'd
still be good with the Acked-by?


> 									Pavel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
@ 2018-07-27 13:42           ` David Rivshin
  0 siblings, 0 replies; 12+ messages in thread
From: David Rivshin @ 2018-07-27 13:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 27 Jul 2018 13:00:06 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> On Fri 2018-07-27 09:19:01, Ladislav Michl wrote:
> > On Thu, Jul 26, 2018 at 04:37:05PM -0400, David Rivshin wrote:  
> > > On Thu, 26 Jul 2018 20:54:26 +0200
> > > Ladislav Michl <ladis@linux-mips.org> wrote:
> > >   
> > > > On Thu, Jul 26, 2018 at 09:36:58AM -0400, David Rivshin wrote:  
> > > > > From: David Rivshin <DRivshin@allworx.com>
> > > > > 
> > > > > If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform
> > > > > data won't be set yet.
> > > > > 
> > > > > Fixes: ac30751df953 ("ARM: OMAP: pdata-quirks: Remove unused timer pdata")
> > > > > Cc: <stable@vger.kernel.org> # 4.17+
> > > > > Signed-off-by: David Rivshin <drivshin@allworx.com>
> > > > > Acked-by: Pavel Machek <pavel@ucw.cz>
> > > > > Tested-by: Pavel Machek <pavel@ucw.cz>
> > > > > ---
> > > > > Changes in v2:
> > > > > * Added Pavel's Acked-by/Tested-by [1]
> > > > > 
> > > > > [1] https://lkml.org/lkml/2018/7/16/346
> > > > > 
> > > > >  drivers/pwm/pwm-omap-dmtimer.c | 5 +++--
> > > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
> > > > > index 665da3c8fbceb..d3d7ea7a53146 100644
> > > > > --- a/drivers/pwm/pwm-omap-dmtimer.c
> > > > > +++ b/drivers/pwm/pwm-omap-dmtimer.c
> > > > > @@ -264,8 +264,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
> > > > >  
> > > > >  	timer_pdata = dev_get_platdata(&timer_pdev->dev);
> > > > >  	if (!timer_pdata) {
> > > > > -		dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> > > > > -		ret = -EINVAL;
> > > > > +		dev_info(&pdev->dev,
> > > > > +			 "dmtimer pdata structure NULL, deferring probe\n");    
> > > > 
> > > > This seems to be a bit verbose for EPROBE_DEFER case. Could we either remove
> > > > it as it is done later in pdata->request_by_node(timer) failure case or at
> > > > least make it dev_dbg? Otherwise thank you and with mentioned change
> > > > Acked-by: Ladislav Michl <ladis@linux-mips.org>  
> > > 
> > > Hi Ladislav, thanks for the review.
> > > 
> > > I had grepped through other drivers and found no consistent pattern. Some
> > > places used dev_err still, others reduced to one of dev_{warn,info,dbg}, 
> > > and others no message at all. Some messages mentioned they are deferring 
> > > the probe, other didn't. I was already getting a couple of dev_info from 
> > > the pinctrl core code, so I went that way. I figured the message might be
> > > useful to someone, but I don't feel strongly.  
> > 
> > Well, pinctrl probe deferal message is a bit annoying. It really does not
> > tell us much as long as pins are correctly configured and in case they are
> > not it is useless as well :)
> >   
> > > I personally would lean to dev_dbg if you think dev_info is too harsh, 
> > > just in case someone's board suddenly isn't working after upgrade. But 
> > > I'm certainly willing to remove the message entirely if you feel strongly,
> > > or anyone else cares to weigh in.  
> > 
> > I'm fine with dev_dbg as well.  
> 
> Looks good to me, too.

OK, I'll spin a v3 with dev_dbg.

Pavel, I'm not sure of the normal etiquette: since I'm making a minor 
change change to the code, should I keep your Tested-by, or drop it 
until/if you test again? I assume, based on your response, that you'd
still be good with the Acked-by?


> 									Pavel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-07-27 13:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-26 13:36 [PATCH v2] pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data David Rivshin
2018-07-26 13:36 ` David Rivshin
2018-07-26 18:54 ` Ladislav Michl
2018-07-26 18:54   ` Ladislav Michl
2018-07-26 20:37   ` David Rivshin
2018-07-26 20:37     ` David Rivshin
2018-07-27  7:19     ` Ladislav Michl
2018-07-27  7:19       ` Ladislav Michl
2018-07-27 11:00       ` Pavel Machek
2018-07-27 11:00         ` Pavel Machek
2018-07-27 13:42         ` David Rivshin
2018-07-27 13:42           ` David Rivshin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.