* [PATCH] soc: rockchip: power-domain: Handle errors from of_genpd_add_provider_onecell
@ 2016-09-15 10:43 ` Tomeu Vizoso
0 siblings, 0 replies; 8+ messages in thread
From: Tomeu Vizoso @ 2016-09-15 10:43 UTC (permalink / raw)
To: linux-kernel
Cc: Tomeu Vizoso, Caesar Wang, Heiko Stuebner, linux-arm-kernel,
linux-rockchip
It was a bit surprising that the device was reported to have probed just
fine, but the provider hadn't been registered.
So handle any errors when registering the provider and fail the probe
accordingly.
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Caesar Wang <wxt@rock-chips.com>
---
drivers/soc/rockchip/pm_domains.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
index 7acd1517dd37..57e920128cb2 100644
--- a/drivers/soc/rockchip/pm_domains.c
+++ b/drivers/soc/rockchip/pm_domains.c
@@ -627,7 +627,11 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
goto err_out;
}
- of_genpd_add_provider_onecell(np, &pmu->genpd_data);
+ error = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
+ if (error) {
+ dev_err(dev, "failed to add provider: %d\n", error);
+ goto err_out;
+ }
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH] soc: rockchip: power-domain: Handle errors from of_genpd_add_provider_onecell
@ 2016-09-15 10:43 ` Tomeu Vizoso
0 siblings, 0 replies; 8+ messages in thread
From: Tomeu Vizoso @ 2016-09-15 10:43 UTC (permalink / raw)
To: linux-arm-kernel
It was a bit surprising that the device was reported to have probed just
fine, but the provider hadn't been registered.
So handle any errors when registering the provider and fail the probe
accordingly.
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Caesar Wang <wxt@rock-chips.com>
---
drivers/soc/rockchip/pm_domains.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
index 7acd1517dd37..57e920128cb2 100644
--- a/drivers/soc/rockchip/pm_domains.c
+++ b/drivers/soc/rockchip/pm_domains.c
@@ -627,7 +627,11 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
goto err_out;
}
- of_genpd_add_provider_onecell(np, &pmu->genpd_data);
+ error = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
+ if (error) {
+ dev_err(dev, "failed to add provider: %d\n", error);
+ goto err_out;
+ }
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] soc: rockchip: power-domain: Handle errors from of_genpd_add_provider_onecell
2016-09-15 10:43 ` Tomeu Vizoso
@ 2016-09-15 14:39 ` Heiko Stübner
-1 siblings, 0 replies; 8+ messages in thread
From: Heiko Stübner @ 2016-09-15 14:39 UTC (permalink / raw)
To: Tomeu Vizoso; +Cc: linux-kernel, Caesar Wang, linux-arm-kernel, linux-rockchip
Am Donnerstag, 15. September 2016, 12:43:41 schrieb Tomeu Vizoso:
> It was a bit surprising that the device was reported to have probed just
> fine, but the provider hadn't been registered.
>
> So handle any errors when registering the provider and fail the probe
> accordingly.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Caesar Wang <wxt@rock-chips.com>
> ---
> drivers/soc/rockchip/pm_domains.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/rockchip/pm_domains.c
> b/drivers/soc/rockchip/pm_domains.c index 7acd1517dd37..57e920128cb2 100644
> --- a/drivers/soc/rockchip/pm_domains.c
> +++ b/drivers/soc/rockchip/pm_domains.c
> @@ -627,7 +627,11 @@ static int rockchip_pm_domain_probe(struct
> platform_device *pdev) goto err_out;
> }
>
> - of_genpd_add_provider_onecell(np, &pmu->genpd_data);
> + error = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
> + if (error) {
> + dev_err(dev, "failed to add provider: %d\n", error);
> + goto err_out;
> + }
>
> return 0;
Looks good in itself, but seems to trigger some issue in the genpd code
when applied alone on top of linux-next-20160915. Looks like genpd
is missing counter-initialization somewhere, as I'm seeing now:
[ 1.664744] genpd_poweroff_unused disabling (null)
[ 1.669553] ------------[ cut here ]------------
[ 1.674169] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440 __queue_work+0x2b8/0x3f8
[ 1.682337] Modules linked in:
[ 1.685401] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5
[ 1.694608] Hardware name: Rockchip (Device Tree)
[ 1.699312] [<c0310388>] (unwind_backtrace) from [<c030ba04>] (show_stack+0x10/0x14)
[ 1.707050] [<c030ba04>] (show_stack) from [<c0599c90>] (dump_stack+0x90/0xa4)
[ 1.714267] [<c0599c90>] (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100)
[ 1.721221] [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28)
[ 1.728785] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>] (__queue_work+0x2b8/0x3f8)
[ 1.737041] [<c0355b28>] (__queue_work) from [<c0355ca8>] (queue_work_on+0x40/0x4c)
[ 1.744692] [<c0355ca8>] (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c)
[ 1.753123] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>] (do_one_initcall+0x40/0x170)
[ 1.761815] [<c0301e78>] (do_one_initcall) from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc)
[ 1.770507] [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>] (kernel_init+0x8/0x114)
[ 1.778678] [<c0bb7ba0>] (kernel_init) from [<c0307df8>] (ret_from_fork+0x14/0x3c)
[ 1.786240] ---[ end trace b38c51ace1463add ]---
[ 1.790875] genpd_poweroff_unused disabling ������
[ 1.795856] genpd_poweroff_unused disabling ������
[ 1.800830] ------------[ cut here ]------------
[ 1.805443] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440 __queue_work+0x2b8/0x3f8
[ 1.813610] Modules linked in:
[ 1.816673] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5
[ 1.827094] Hardware name: Rockchip (Device Tree)
[ 1.831795] [<c0310388>] (unwind_backtrace) from [<c030ba04>] (show_stack+0x10/0x14)
[ 1.839532] [<c030ba04>] (show_stack) from [<c0599c90>] (dump_stack+0x90/0xa4)
[ 1.846747] [<c0599c90>] (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100)
[ 1.853700] [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28)
[ 1.861264] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>] (__queue_work+0x2b8/0x3f8)
[ 1.869521] [<c0355b28>] (__queue_work) from [<c0355ca8>] (queue_work_on+0x40/0x4c)
[ 1.877170] [<c0355ca8>] (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c)
[ 1.885600] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>] (do_one_initcall+0x40/0x170)
[ 1.894290] [<c0301e78>] (do_one_initcall) from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc)
[ 1.902981] [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>] (kernel_init+0x8/0x114)
[ 1.911152] [<c0bb7ba0>] (kernel_init) from [<c0307df8>] (ret_from_fork+0x14/0x3c)
[ 1.918713] ---[ end trace b38c51ace1463ade ]---
[ 1.923338] genpd_poweroff_unused disabling ������
[ 1.928325] genpd_poweroff_unused disabling ������
[+ millions more of those]
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] soc: rockchip: power-domain: Handle errors from of_genpd_add_provider_onecell
@ 2016-09-15 14:39 ` Heiko Stübner
0 siblings, 0 replies; 8+ messages in thread
From: Heiko Stübner @ 2016-09-15 14:39 UTC (permalink / raw)
To: linux-arm-kernel
Am Donnerstag, 15. September 2016, 12:43:41 schrieb Tomeu Vizoso:
> It was a bit surprising that the device was reported to have probed just
> fine, but the provider hadn't been registered.
>
> So handle any errors when registering the provider and fail the probe
> accordingly.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Caesar Wang <wxt@rock-chips.com>
> ---
> drivers/soc/rockchip/pm_domains.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/rockchip/pm_domains.c
> b/drivers/soc/rockchip/pm_domains.c index 7acd1517dd37..57e920128cb2 100644
> --- a/drivers/soc/rockchip/pm_domains.c
> +++ b/drivers/soc/rockchip/pm_domains.c
> @@ -627,7 +627,11 @@ static int rockchip_pm_domain_probe(struct
> platform_device *pdev) goto err_out;
> }
>
> - of_genpd_add_provider_onecell(np, &pmu->genpd_data);
> + error = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
> + if (error) {
> + dev_err(dev, "failed to add provider: %d\n", error);
> + goto err_out;
> + }
>
> return 0;
Looks good in itself, but seems to trigger some issue in the genpd code
when applied alone on top of linux-next-20160915. Looks like genpd
is missing counter-initialization somewhere, as I'm seeing now:
[ 1.664744] genpd_poweroff_unused disabling (null)
[ 1.669553] ------------[ cut here ]------------
[ 1.674169] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440 __queue_work+0x2b8/0x3f8
[ 1.682337] Modules linked in:
[ 1.685401] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5
[ 1.694608] Hardware name: Rockchip (Device Tree)
[ 1.699312] [<c0310388>] (unwind_backtrace) from [<c030ba04>] (show_stack+0x10/0x14)
[ 1.707050] [<c030ba04>] (show_stack) from [<c0599c90>] (dump_stack+0x90/0xa4)
[ 1.714267] [<c0599c90>] (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100)
[ 1.721221] [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28)
[ 1.728785] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>] (__queue_work+0x2b8/0x3f8)
[ 1.737041] [<c0355b28>] (__queue_work) from [<c0355ca8>] (queue_work_on+0x40/0x4c)
[ 1.744692] [<c0355ca8>] (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c)
[ 1.753123] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>] (do_one_initcall+0x40/0x170)
[ 1.761815] [<c0301e78>] (do_one_initcall) from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc)
[ 1.770507] [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>] (kernel_init+0x8/0x114)
[ 1.778678] [<c0bb7ba0>] (kernel_init) from [<c0307df8>] (ret_from_fork+0x14/0x3c)
[ 1.786240] ---[ end trace b38c51ace1463add ]---
[ 1.790875] genpd_poweroff_unused disabling ??????
[ 1.795856] genpd_poweroff_unused disabling ??????
[ 1.800830] ------------[ cut here ]------------
[ 1.805443] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440 __queue_work+0x2b8/0x3f8
[ 1.813610] Modules linked in:
[ 1.816673] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5
[ 1.827094] Hardware name: Rockchip (Device Tree)
[ 1.831795] [<c0310388>] (unwind_backtrace) from [<c030ba04>] (show_stack+0x10/0x14)
[ 1.839532] [<c030ba04>] (show_stack) from [<c0599c90>] (dump_stack+0x90/0xa4)
[ 1.846747] [<c0599c90>] (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100)
[ 1.853700] [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28)
[ 1.861264] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>] (__queue_work+0x2b8/0x3f8)
[ 1.869521] [<c0355b28>] (__queue_work) from [<c0355ca8>] (queue_work_on+0x40/0x4c)
[ 1.877170] [<c0355ca8>] (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c)
[ 1.885600] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>] (do_one_initcall+0x40/0x170)
[ 1.894290] [<c0301e78>] (do_one_initcall) from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc)
[ 1.902981] [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>] (kernel_init+0x8/0x114)
[ 1.911152] [<c0bb7ba0>] (kernel_init) from [<c0307df8>] (ret_from_fork+0x14/0x3c)
[ 1.918713] ---[ end trace b38c51ace1463ade ]---
[ 1.923338] genpd_poweroff_unused disabling ??????
[ 1.928325] genpd_poweroff_unused disabling ??????
[+ millions more of those]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] soc: rockchip: power-domain: Handle errors from of_genpd_add_provider_onecell
2016-09-15 14:39 ` Heiko Stübner
@ 2016-09-15 22:31 ` Heiko Stübner
-1 siblings, 0 replies; 8+ messages in thread
From: Heiko Stübner @ 2016-09-15 22:31 UTC (permalink / raw)
To: Tomeu Vizoso; +Cc: linux-kernel, Caesar Wang, linux-arm-kernel, linux-rockchip
Am Donnerstag, 15. September 2016, 16:39:34 schrieb Heiko Stübner:
> Am Donnerstag, 15. September 2016, 12:43:41 schrieb Tomeu Vizoso:
> > It was a bit surprising that the device was reported to have probed just
> > fine, but the provider hadn't been registered.
> >
> > So handle any errors when registering the provider and fail the probe
> > accordingly.
> >
> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > Cc: Caesar Wang <wxt@rock-chips.com>
> > ---
> >
> > drivers/soc/rockchip/pm_domains.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/rockchip/pm_domains.c
> > b/drivers/soc/rockchip/pm_domains.c index 7acd1517dd37..57e920128cb2
> > 100644
> > --- a/drivers/soc/rockchip/pm_domains.c
> > +++ b/drivers/soc/rockchip/pm_domains.c
> > @@ -627,7 +627,11 @@ static int rockchip_pm_domain_probe(struct
> > platform_device *pdev) goto err_out;
> >
> > }
> >
> > - of_genpd_add_provider_onecell(np, &pmu->genpd_data);
> > + error = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
> > + if (error) {
> > + dev_err(dev, "failed to add provider: %d\n", error);
> > + goto err_out;
> > + }
> >
> > return 0;
>
> Looks good in itself, but seems to trigger some issue in the genpd code
> when applied alone on top of linux-next-20160915. Looks like genpd
> is missing counter-initialization somewhere, as I'm seeing now:
>
> [ 1.664744] genpd_poweroff_unused disabling (null)
> [ 1.669553] ------------[ cut here ]------------
> [ 1.674169] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440
> __queue_work+0x2b8/0x3f8 [ 1.682337] Modules linked in:
> [ 1.685401] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5 [ 1.694608] Hardware
> name: Rockchip (Device Tree)
> [ 1.699312] [<c0310388>] (unwind_backtrace) from [<c030ba04>]
> (show_stack+0x10/0x14) [ 1.707050] [<c030ba04>] (show_stack) from
> [<c0599c90>] (dump_stack+0x90/0xa4) [ 1.714267] [<c0599c90>]
> (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100) [ 1.721221]
> [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28) [
> 1.728785] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>]
> (__queue_work+0x2b8/0x3f8) [ 1.737041] [<c0355b28>] (__queue_work) from
> [<c0355ca8>] (queue_work_on+0x40/0x4c) [ 1.744692] [<c0355ca8>]
> (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c) [
> 1.753123] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>]
> (do_one_initcall+0x40/0x170) [ 1.761815] [<c0301e78>] (do_one_initcall)
> from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc) [ 1.770507]
> [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>]
> (kernel_init+0x8/0x114) [ 1.778678] [<c0bb7ba0>] (kernel_init) from
> [<c0307df8>] (ret_from_fork+0x14/0x3c) [ 1.786240] ---[ end trace
> b38c51ace1463add ]---
> [ 1.790875] genpd_poweroff_unused disabling ������
> [ 1.795856] genpd_poweroff_unused disabling ������
> [ 1.800830] ------------[ cut here ]------------
> [ 1.805443] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440
> __queue_work+0x2b8/0x3f8 [ 1.813610] Modules linked in:
> [ 1.816673] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W
> 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5 [ 1.827094] Hardware
> name: Rockchip (Device Tree)
> [ 1.831795] [<c0310388>] (unwind_backtrace) from [<c030ba04>]
> (show_stack+0x10/0x14) [ 1.839532] [<c030ba04>] (show_stack) from
> [<c0599c90>] (dump_stack+0x90/0xa4) [ 1.846747] [<c0599c90>]
> (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100) [ 1.853700]
> [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28) [
> 1.861264] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>]
> (__queue_work+0x2b8/0x3f8) [ 1.869521] [<c0355b28>] (__queue_work) from
> [<c0355ca8>] (queue_work_on+0x40/0x4c) [ 1.877170] [<c0355ca8>]
> (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c) [
> 1.885600] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>]
> (do_one_initcall+0x40/0x170) [ 1.894290] [<c0301e78>] (do_one_initcall)
> from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc) [ 1.902981]
> [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>]
> (kernel_init+0x8/0x114) [ 1.911152] [<c0bb7ba0>] (kernel_init) from
> [<c0307df8>] (ret_from_fork+0x14/0x3c) [ 1.918713] ---[ end trace
> b38c51ace1463ade ]---
> [ 1.923338] genpd_poweroff_unused disabling ������
> [ 1.928325] genpd_poweroff_unused disabling ������
>
> [+ millions more of those]
just for completenes sake, I've included your patch in a series of my
own [0] which adds the necessary pm_genpd_remove prequisite to prevent
the errors shown above.
Heiko
[0] "[PATCH 0/2] soc: rockchip: fix probe error path in power-domain driver"
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] soc: rockchip: power-domain: Handle errors from of_genpd_add_provider_onecell
@ 2016-09-15 22:31 ` Heiko Stübner
0 siblings, 0 replies; 8+ messages in thread
From: Heiko Stübner @ 2016-09-15 22:31 UTC (permalink / raw)
To: linux-arm-kernel
Am Donnerstag, 15. September 2016, 16:39:34 schrieb Heiko St?bner:
> Am Donnerstag, 15. September 2016, 12:43:41 schrieb Tomeu Vizoso:
> > It was a bit surprising that the device was reported to have probed just
> > fine, but the provider hadn't been registered.
> >
> > So handle any errors when registering the provider and fail the probe
> > accordingly.
> >
> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > Cc: Caesar Wang <wxt@rock-chips.com>
> > ---
> >
> > drivers/soc/rockchip/pm_domains.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/rockchip/pm_domains.c
> > b/drivers/soc/rockchip/pm_domains.c index 7acd1517dd37..57e920128cb2
> > 100644
> > --- a/drivers/soc/rockchip/pm_domains.c
> > +++ b/drivers/soc/rockchip/pm_domains.c
> > @@ -627,7 +627,11 @@ static int rockchip_pm_domain_probe(struct
> > platform_device *pdev) goto err_out;
> >
> > }
> >
> > - of_genpd_add_provider_onecell(np, &pmu->genpd_data);
> > + error = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
> > + if (error) {
> > + dev_err(dev, "failed to add provider: %d\n", error);
> > + goto err_out;
> > + }
> >
> > return 0;
>
> Looks good in itself, but seems to trigger some issue in the genpd code
> when applied alone on top of linux-next-20160915. Looks like genpd
> is missing counter-initialization somewhere, as I'm seeing now:
>
> [ 1.664744] genpd_poweroff_unused disabling (null)
> [ 1.669553] ------------[ cut here ]------------
> [ 1.674169] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440
> __queue_work+0x2b8/0x3f8 [ 1.682337] Modules linked in:
> [ 1.685401] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5 [ 1.694608] Hardware
> name: Rockchip (Device Tree)
> [ 1.699312] [<c0310388>] (unwind_backtrace) from [<c030ba04>]
> (show_stack+0x10/0x14) [ 1.707050] [<c030ba04>] (show_stack) from
> [<c0599c90>] (dump_stack+0x90/0xa4) [ 1.714267] [<c0599c90>]
> (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100) [ 1.721221]
> [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28) [
> 1.728785] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>]
> (__queue_work+0x2b8/0x3f8) [ 1.737041] [<c0355b28>] (__queue_work) from
> [<c0355ca8>] (queue_work_on+0x40/0x4c) [ 1.744692] [<c0355ca8>]
> (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c) [
> 1.753123] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>]
> (do_one_initcall+0x40/0x170) [ 1.761815] [<c0301e78>] (do_one_initcall)
> from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc) [ 1.770507]
> [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>]
> (kernel_init+0x8/0x114) [ 1.778678] [<c0bb7ba0>] (kernel_init) from
> [<c0307df8>] (ret_from_fork+0x14/0x3c) [ 1.786240] ---[ end trace
> b38c51ace1463add ]---
> [ 1.790875] genpd_poweroff_unused disabling ??????
> [ 1.795856] genpd_poweroff_unused disabling ??????
> [ 1.800830] ------------[ cut here ]------------
> [ 1.805443] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440
> __queue_work+0x2b8/0x3f8 [ 1.813610] Modules linked in:
> [ 1.816673] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W
> 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5 [ 1.827094] Hardware
> name: Rockchip (Device Tree)
> [ 1.831795] [<c0310388>] (unwind_backtrace) from [<c030ba04>]
> (show_stack+0x10/0x14) [ 1.839532] [<c030ba04>] (show_stack) from
> [<c0599c90>] (dump_stack+0x90/0xa4) [ 1.846747] [<c0599c90>]
> (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100) [ 1.853700]
> [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28) [
> 1.861264] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>]
> (__queue_work+0x2b8/0x3f8) [ 1.869521] [<c0355b28>] (__queue_work) from
> [<c0355ca8>] (queue_work_on+0x40/0x4c) [ 1.877170] [<c0355ca8>]
> (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c) [
> 1.885600] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>]
> (do_one_initcall+0x40/0x170) [ 1.894290] [<c0301e78>] (do_one_initcall)
> from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc) [ 1.902981]
> [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>]
> (kernel_init+0x8/0x114) [ 1.911152] [<c0bb7ba0>] (kernel_init) from
> [<c0307df8>] (ret_from_fork+0x14/0x3c) [ 1.918713] ---[ end trace
> b38c51ace1463ade ]---
> [ 1.923338] genpd_poweroff_unused disabling ??????
> [ 1.928325] genpd_poweroff_unused disabling ??????
>
> [+ millions more of those]
just for completenes sake, I've included your patch in a series of my
own [0] which adds the necessary pm_genpd_remove prequisite to prevent
the errors shown above.
Heiko
[0] "[PATCH 0/2] soc: rockchip: fix probe error path in power-domain driver"
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] soc: rockchip: power-domain: Handle errors from of_genpd_add_provider_onecell
2016-09-15 22:31 ` Heiko Stübner
@ 2016-09-16 5:59 ` Tomeu Vizoso
-1 siblings, 0 replies; 8+ messages in thread
From: Tomeu Vizoso @ 2016-09-16 5:59 UTC (permalink / raw)
To: Heiko Stübner
Cc: linux-kernel, Caesar Wang, linux-arm-kernel, linux-rockchip
On 09/16/2016 12:31 AM, Heiko Stübner wrote:
> Am Donnerstag, 15. September 2016, 16:39:34 schrieb Heiko Stübner:
>> Am Donnerstag, 15. September 2016, 12:43:41 schrieb Tomeu Vizoso:
>>> It was a bit surprising that the device was reported to have probed just
>>> fine, but the provider hadn't been registered.
>>>
>>> So handle any errors when registering the provider and fail the probe
>>> accordingly.
>>>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>> Cc: Caesar Wang <wxt@rock-chips.com>
>>> ---
>>>
>>> drivers/soc/rockchip/pm_domains.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/soc/rockchip/pm_domains.c
>>> b/drivers/soc/rockchip/pm_domains.c index 7acd1517dd37..57e920128cb2
>>> 100644
>>> --- a/drivers/soc/rockchip/pm_domains.c
>>> +++ b/drivers/soc/rockchip/pm_domains.c
>>> @@ -627,7 +627,11 @@ static int rockchip_pm_domain_probe(struct
>>> platform_device *pdev) goto err_out;
>>>
>>> }
>>>
>>> - of_genpd_add_provider_onecell(np, &pmu->genpd_data);
>>> + error = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
>>> + if (error) {
>>> + dev_err(dev, "failed to add provider: %d\n", error);
>>> + goto err_out;
>>> + }
>>>
>>> return 0;
>>
>> Looks good in itself, but seems to trigger some issue in the genpd code
>> when applied alone on top of linux-next-20160915. Looks like genpd
>> is missing counter-initialization somewhere, as I'm seeing now:
>>
>> [ 1.664744] genpd_poweroff_unused disabling (null)
>> [ 1.669553] ------------[ cut here ]------------
>> [ 1.674169] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440
>> __queue_work+0x2b8/0x3f8 [ 1.682337] Modules linked in:
>> [ 1.685401] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
>> 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5 [ 1.694608] Hardware
>> name: Rockchip (Device Tree)
>> [ 1.699312] [<c0310388>] (unwind_backtrace) from [<c030ba04>]
>> (show_stack+0x10/0x14) [ 1.707050] [<c030ba04>] (show_stack) from
>> [<c0599c90>] (dump_stack+0x90/0xa4) [ 1.714267] [<c0599c90>]
>> (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100) [ 1.721221]
>> [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28) [
>> 1.728785] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>]
>> (__queue_work+0x2b8/0x3f8) [ 1.737041] [<c0355b28>] (__queue_work) from
>> [<c0355ca8>] (queue_work_on+0x40/0x4c) [ 1.744692] [<c0355ca8>]
>> (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c) [
>> 1.753123] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>]
>> (do_one_initcall+0x40/0x170) [ 1.761815] [<c0301e78>] (do_one_initcall)
>> from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc) [ 1.770507]
>> [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>]
>> (kernel_init+0x8/0x114) [ 1.778678] [<c0bb7ba0>] (kernel_init) from
>> [<c0307df8>] (ret_from_fork+0x14/0x3c) [ 1.786240] ---[ end trace
>> b38c51ace1463add ]---
>> [ 1.790875] genpd_poweroff_unused disabling ������
>> [ 1.795856] genpd_poweroff_unused disabling ������
>> [ 1.800830] ------------[ cut here ]------------
>> [ 1.805443] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440
>> __queue_work+0x2b8/0x3f8 [ 1.813610] Modules linked in:
>> [ 1.816673] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W
>> 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5 [ 1.827094] Hardware
>> name: Rockchip (Device Tree)
>> [ 1.831795] [<c0310388>] (unwind_backtrace) from [<c030ba04>]
>> (show_stack+0x10/0x14) [ 1.839532] [<c030ba04>] (show_stack) from
>> [<c0599c90>] (dump_stack+0x90/0xa4) [ 1.846747] [<c0599c90>]
>> (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100) [ 1.853700]
>> [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28) [
>> 1.861264] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>]
>> (__queue_work+0x2b8/0x3f8) [ 1.869521] [<c0355b28>] (__queue_work) from
>> [<c0355ca8>] (queue_work_on+0x40/0x4c) [ 1.877170] [<c0355ca8>]
>> (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c) [
>> 1.885600] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>]
>> (do_one_initcall+0x40/0x170) [ 1.894290] [<c0301e78>] (do_one_initcall)
>> from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc) [ 1.902981]
>> [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>]
>> (kernel_init+0x8/0x114) [ 1.911152] [<c0bb7ba0>] (kernel_init) from
>> [<c0307df8>] (ret_from_fork+0x14/0x3c) [ 1.918713] ---[ end trace
>> b38c51ace1463ade ]---
>> [ 1.923338] genpd_poweroff_unused disabling ������
>> [ 1.928325] genpd_poweroff_unused disabling ������
>>
>> [+ millions more of those]
>
> just for completenes sake, I've included your patch in a series of my
> own [0] which adds the necessary pm_genpd_remove prequisite to prevent
> the errors shown above.
That's awesome, thanks. For some reason I hadn't noticed those.
Regards,
Tomeu
>
> Heiko
>
> [0] "[PATCH 0/2] soc: rockchip: fix probe error path in power-domain driver"
>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] soc: rockchip: power-domain: Handle errors from of_genpd_add_provider_onecell
@ 2016-09-16 5:59 ` Tomeu Vizoso
0 siblings, 0 replies; 8+ messages in thread
From: Tomeu Vizoso @ 2016-09-16 5:59 UTC (permalink / raw)
To: linux-arm-kernel
On 09/16/2016 12:31 AM, Heiko St?bner wrote:
> Am Donnerstag, 15. September 2016, 16:39:34 schrieb Heiko St?bner:
>> Am Donnerstag, 15. September 2016, 12:43:41 schrieb Tomeu Vizoso:
>>> It was a bit surprising that the device was reported to have probed just
>>> fine, but the provider hadn't been registered.
>>>
>>> So handle any errors when registering the provider and fail the probe
>>> accordingly.
>>>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>> Cc: Caesar Wang <wxt@rock-chips.com>
>>> ---
>>>
>>> drivers/soc/rockchip/pm_domains.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/soc/rockchip/pm_domains.c
>>> b/drivers/soc/rockchip/pm_domains.c index 7acd1517dd37..57e920128cb2
>>> 100644
>>> --- a/drivers/soc/rockchip/pm_domains.c
>>> +++ b/drivers/soc/rockchip/pm_domains.c
>>> @@ -627,7 +627,11 @@ static int rockchip_pm_domain_probe(struct
>>> platform_device *pdev) goto err_out;
>>>
>>> }
>>>
>>> - of_genpd_add_provider_onecell(np, &pmu->genpd_data);
>>> + error = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
>>> + if (error) {
>>> + dev_err(dev, "failed to add provider: %d\n", error);
>>> + goto err_out;
>>> + }
>>>
>>> return 0;
>>
>> Looks good in itself, but seems to trigger some issue in the genpd code
>> when applied alone on top of linux-next-20160915. Looks like genpd
>> is missing counter-initialization somewhere, as I'm seeing now:
>>
>> [ 1.664744] genpd_poweroff_unused disabling (null)
>> [ 1.669553] ------------[ cut here ]------------
>> [ 1.674169] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440
>> __queue_work+0x2b8/0x3f8 [ 1.682337] Modules linked in:
>> [ 1.685401] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
>> 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5 [ 1.694608] Hardware
>> name: Rockchip (Device Tree)
>> [ 1.699312] [<c0310388>] (unwind_backtrace) from [<c030ba04>]
>> (show_stack+0x10/0x14) [ 1.707050] [<c030ba04>] (show_stack) from
>> [<c0599c90>] (dump_stack+0x90/0xa4) [ 1.714267] [<c0599c90>]
>> (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100) [ 1.721221]
>> [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28) [
>> 1.728785] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>]
>> (__queue_work+0x2b8/0x3f8) [ 1.737041] [<c0355b28>] (__queue_work) from
>> [<c0355ca8>] (queue_work_on+0x40/0x4c) [ 1.744692] [<c0355ca8>]
>> (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c) [
>> 1.753123] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>]
>> (do_one_initcall+0x40/0x170) [ 1.761815] [<c0301e78>] (do_one_initcall)
>> from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc) [ 1.770507]
>> [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>]
>> (kernel_init+0x8/0x114) [ 1.778678] [<c0bb7ba0>] (kernel_init) from
>> [<c0307df8>] (ret_from_fork+0x14/0x3c) [ 1.786240] ---[ end trace
>> b38c51ace1463add ]---
>> [ 1.790875] genpd_poweroff_unused disabling ??????
>> [ 1.795856] genpd_poweroff_unused disabling ??????
>> [ 1.800830] ------------[ cut here ]------------
>> [ 1.805443] WARNING: CPU: 0 PID: 1 at ../kernel/workqueue.c:1440
>> __queue_work+0x2b8/0x3f8 [ 1.813610] Modules linked in:
>> [ 1.816673] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W
>> 4.8.0-rc6-next-20160915-00001-g7432710-dirty #5 [ 1.827094] Hardware
>> name: Rockchip (Device Tree)
>> [ 1.831795] [<c0310388>] (unwind_backtrace) from [<c030ba04>]
>> (show_stack+0x10/0x14) [ 1.839532] [<c030ba04>] (show_stack) from
>> [<c0599c90>] (dump_stack+0x90/0xa4) [ 1.846747] [<c0599c90>]
>> (dump_stack) from [<c0341bc0>] (__warn+0xe8/0x100) [ 1.853700]
>> [<c0341bc0>] (__warn) from [<c0341c88>] (warn_slowpath_null+0x20/0x28) [
>> 1.861264] [<c0341c88>] (warn_slowpath_null) from [<c0355b28>]
>> (__queue_work+0x2b8/0x3f8) [ 1.869521] [<c0355b28>] (__queue_work) from
>> [<c0355ca8>] (queue_work_on+0x40/0x4c) [ 1.877170] [<c0355ca8>]
>> (queue_work_on) from [<c103e8b8>] (genpd_poweroff_unused+0x78/0x9c) [
>> 1.885600] [<c103e8b8>] (genpd_poweroff_unused) from [<c0301e78>]
>> (do_one_initcall+0x40/0x170) [ 1.894290] [<c0301e78>] (do_one_initcall)
>> from [<c1000dc8>] (kernel_init_freeable+0x15c/0x1fc) [ 1.902981]
>> [<c1000dc8>] (kernel_init_freeable) from [<c0bb7ba0>]
>> (kernel_init+0x8/0x114) [ 1.911152] [<c0bb7ba0>] (kernel_init) from
>> [<c0307df8>] (ret_from_fork+0x14/0x3c) [ 1.918713] ---[ end trace
>> b38c51ace1463ade ]---
>> [ 1.923338] genpd_poweroff_unused disabling ??????
>> [ 1.928325] genpd_poweroff_unused disabling ??????
>>
>> [+ millions more of those]
>
> just for completenes sake, I've included your patch in a series of my
> own [0] which adds the necessary pm_genpd_remove prequisite to prevent
> the errors shown above.
That's awesome, thanks. For some reason I hadn't noticed those.
Regards,
Tomeu
>
> Heiko
>
> [0] "[PATCH 0/2] soc: rockchip: fix probe error path in power-domain driver"
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-16 5:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-15 10:43 [PATCH] soc: rockchip: power-domain: Handle errors from of_genpd_add_provider_onecell Tomeu Vizoso
2016-09-15 10:43 ` Tomeu Vizoso
2016-09-15 14:39 ` Heiko Stübner
2016-09-15 14:39 ` Heiko Stübner
2016-09-15 22:31 ` Heiko Stübner
2016-09-15 22:31 ` Heiko Stübner
2016-09-16 5:59 ` Tomeu Vizoso
2016-09-16 5:59 ` Tomeu Vizoso
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.