pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
PATCH v2 1/1 pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
From: David Rivshin <[email protected]>
If a pwm-omap-dmtimer is probed before the dmtimer it uses, the platform data won't be set yet.
---
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(-)
drivers/pwm/pwm-omap-dmtimer.c +3 -2
@@ -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
Signature · 1 line
2.17.1
PATCH v2 1/1 pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
Quoted 24 lines · David Rivshin
From: David Rivshin <[email protected]>
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 <[email protected]> # 4.17+Signed-off-by David Rivshin <[email protected]>Acked-by Pavel Machek <[email protected]>Tested-by Pavel Machek <[email protected]>---
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(-)drivers/pwm/pwm-omap-dmtimer.c +0
-0
drivers/pwm/pwm-omap-dmtimer.c static int pwm_omap_dmtimer_probe(struct platform_device *pdev) · line 268
Quoted 7 lines
drivers/pwm/pwm-omap-dmtimer.c +2
-2@@ -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
Quoted 8 lines
drivers/pwm/pwm-omap-dmtimer.c +1
-0+ ret = -EPROBE_DEFER ; goto put; }base-commit:
d72e90f33aa4709ebecc5005562f52335e106a60Signature · 1 line
2.17.1
PATCH v2 1/1 pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
Quoted 36 lines · On Thu, 26 Jul 2018 20:54:26 +0200 Ladislav Michl <[email protected]> wrote:
Quoted 24 lines · David Rivshin
From: David Rivshin <[email protected]>
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 <[email protected]> # 4.17+Signed-off-by David Rivshin <[email protected]>Acked-by Pavel Machek <[email protected]>Tested-by Pavel Machek <[email protected]>---
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(-)drivers/pwm/pwm-omap-dmtimer.c +0
-0drivers/pwm/pwm-omap-dmtimer.c static int pwm_omap_dmtimer_probe(struct platform_device *pdev) · line 268
Quoted 7 lines
drivers/pwm/pwm-omap-dmtimer.c +2
-2@@ -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_DEFERcase. Could we either remove it as it is done later inpdata->request_by_node(timer)failure case or at least make itdev_dbg? Otherwise thank you and with mentioned change
Quoted 1 line
Acked-by Ladislav Michl <[email protected]>
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.
Quoted 9 lines
Quoted 9 lines
drivers/pwm/pwm-omap-dmtimer.c +1
-0+ ret = -EPROBE_DEFER ; goto put; }base-commit:
d72e90f33aa4709ebecc5005562f52335e106a60Signature · 1 line
2.17.1
PATCH v2 1/1 pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
Quoted 41 lines · David Rivshin
Quoted 36 lines · On Thu, 26 Jul 2018 20:54:26 +0200 Ladislav Michl <[email protected]> wrote:
Quoted 24 lines · David Rivshin
From: David Rivshin <[email protected]>
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 <[email protected]> # 4.17+Signed-off-by David Rivshin <[email protected]>Acked-by Pavel Machek <[email protected]>Tested-by Pavel Machek <[email protected]>---
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(-)drivers/pwm/pwm-omap-dmtimer.c +0
-0drivers/pwm/pwm-omap-dmtimer.c static int pwm_omap_dmtimer_probe(struct platform_device *pdev) · line 268
Quoted 7 lines
drivers/pwm/pwm-omap-dmtimer.c +2
-2@@ -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_DEFERcase. Could we either remove it as it is done later inpdata->request_by_node(timer)failure case or at least make itdev_dbg? Otherwise thank you and with mentioned changeQuoted 1 line
Acked-by Ladislav Michl <[email protected]>
Quoted 8 lines
Hi Ladislav, thanks for the review.
I had grepped through other drivers and found no consistent pattern. Some places used
dev_errstill, 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 ofdev_infofrom 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 :)
Quoted 4 lines
I personally would lean to
dev_dbgif you thinkdev_infois 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
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
drivers/pwm/pwm-omap-dmtimer.c +1
-0+ ret = -EPROBE_DEFER ; goto put; }base-commit:
d72e90f33aa4709ebecc5005562f52335e106a60Signature · 1 line
2.17.1
PATCH v2 1/1 pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
Quoted 55 lines · Ladislav Michl
Quoted 41 lines · David Rivshin
Quoted 36 lines · On Thu, 26 Jul 2018 20:54:26 +0200 Ladislav Michl <[email protected]> wrote:
Quoted 24 lines · David Rivshin
From: David Rivshin <[email protected]>
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 <[email protected]> # 4.17+Signed-off-by David Rivshin <[email protected]>Acked-by Pavel Machek <[email protected]>Tested-by Pavel Machek <[email protected]>---
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(-)drivers/pwm/pwm-omap-dmtimer.c +0
-0drivers/pwm/pwm-omap-dmtimer.c static int pwm_omap_dmtimer_probe(struct platform_device *pdev) · line 268
Quoted 7 lines
drivers/pwm/pwm-omap-dmtimer.c +2
-2@@ -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_DEFERcase. Could we either remove it as it is done later inpdata->request_by_node(timer)failure case or at least make itdev_dbg? Otherwise thank you and with mentioned changeQuoted 1 line
Acked-by Ladislav Michl <[email protected]>Quoted 8 lines
Hi Ladislav, thanks for the review.
I had grepped through other drivers and found no consistent pattern. Some places used
dev_errstill, 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 ofdev_infofrom 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 :)
Quoted 6 lines
Quoted 4 lines
I personally would lean to
dev_dbgif you thinkdev_infois 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_dbgas well.
Looks good to me, too.
Pavel
Signature · 9 lines
(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>
PATCH v2 1/1 pwm: pwm-omap-dmtimer: return -EPROBE_DEFER if no dmtimer platform data
Quoted 56 lines · On Fri, 27 Jul 2018 13:00:06 +0200 Pavel Machek <[email protected]> wrote:
Quoted 55 lines · Ladislav Michl
Quoted 41 lines · David Rivshin
Quoted 36 lines · On Thu, 26 Jul 2018 20:54:26 +0200 Ladislav Michl <[email protected]> wrote:
Quoted 24 lines · David Rivshin
From: David Rivshin <[email protected]>
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 <[email protected]> # 4.17+Signed-off-by David Rivshin <[email protected]>Acked-by Pavel Machek <[email protected]>Tested-by Pavel Machek <[email protected]>---
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(-)drivers/pwm/pwm-omap-dmtimer.c +0
-0drivers/pwm/pwm-omap-dmtimer.c static int pwm_omap_dmtimer_probe(struct platform_device *pdev) · line 268
Quoted 7 lines
drivers/pwm/pwm-omap-dmtimer.c +2
-2@@ -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_DEFERcase. Could we either remove it as it is done later inpdata->request_by_node(timer)failure case or at least make itdev_dbg? Otherwise thank you and with mentioned changeQuoted 1 line
Acked-by Ladislav Michl <[email protected]>Quoted 8 lines
Hi Ladislav, thanks for the review.
I had grepped through other drivers and found no consistent pattern. Some places used
dev_errstill, 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 ofdev_infofrom 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 :)
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
I personally would lean to
dev_dbgif you thinkdev_infois 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_dbgas 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?
Quoted 1 line
Pavel