Re: [PATCH v2 06/19] PM / devfreq: tegra: Fix missed error checking on devfreq initialization failure
Hi, On 19. 4. 16. 오후 11:29, Dmitry Osipenko wrote: > 16.04.2019 5:32, Chanwoo Choi пишет: >> Hi, >> >> patch6/7/8/9 are for handling of exception handling in probe() function. >> Actually, I'm not sure that there are special reason to split out >> the patches. I think that you can squash patch6/7/8/9 to only one patch. > > Indeed, I was rebasing and reordering patches multiple times and looks like > there is no reason not to squash these patches now. > >> Also, even if patch6/7/8/9 handle the exception handling in probe(), >> the tegra_devfreq_probe() doesn't support the restoring sequence >> when fail happen. I think that if you want to fix the fail case of probe(), >> please add the restoring sequence about following function. >> - clk_disable_unprepare() >> - clk_notifier_unregister() >> - dev_pm_opp_remove() > > When all of 6/7/8/9 patches are applied, the clk_notifier_register() becomes > the last invocation of the probe function and clk_enable() is kept at the > first place of the probe order. Hence the sequence you're suggesting is > already incorrect because error-unwinding order usually should be opposite to > the probe order. It looks to me that the current final result of these > patches is already correct. You're right. When I replied it, I have not considered the order. Sorry, it made you some confusion. > > -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH v2 06/19] PM / devfreq: tegra: Fix missed error checking on devfreq initialization failure
16.04.2019 5:32, Chanwoo Choi пишет: > Hi, > > patch6/7/8/9 are for handling of exception handling in probe() function. > Actually, I'm not sure that there are special reason to split out > the patches. I think that you can squash patch6/7/8/9 to only one patch. Indeed, I was rebasing and reordering patches multiple times and looks like there is no reason not to squash these patches now. > Also, even if patch6/7/8/9 handle the exception handling in probe(), > the tegra_devfreq_probe() doesn't support the restoring sequence > when fail happen. I think that if you want to fix the fail case of probe(), > please add the restoring sequence about following function. > - clk_disable_unprepare() > - clk_notifier_unregister() > - dev_pm_opp_remove() When all of 6/7/8/9 patches are applied, the clk_notifier_register() becomes the last invocation of the probe function and clk_enable() is kept at the first place of the probe order. Hence the sequence you're suggesting is already incorrect because error-unwinding order usually should be opposite to the probe order. It looks to me that the current final result of these patches is already correct.
Re: [PATCH v2 06/19] PM / devfreq: tegra: Fix missed error checking on devfreq initialization failure
Hi,
patch6/7/8/9 are for handling of exception handling in probe() function.
Actually, I'm not sure that there are special reason to split out
the patches. I think that you can squash patch6/7/8/9 to only one patch.
Also, even if patch6/7/8/9 handle the exception handling in probe(),
the tegra_devfreq_probe() doesn't support the restoring sequence
when fail happen. I think that if you want to fix the fail case of probe(),
please add the restoring sequence about following function.
- clk_disable_unprepare()
- clk_notifier_unregister()
- dev_pm_opp_remove()
On 19. 4. 15. 오후 11:54, Dmitry Osipenko wrote:
> The code doesn't check for devfreq initialization failure, fix it.
>
> Signed-off-by: Dmitry Osipenko
> ---
> drivers/devfreq/tegra-devfreq.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/devfreq/tegra-devfreq.c b/drivers/devfreq/tegra-devfreq.c
> index f0f0d78f6cbf..aa0478464b35 100644
> --- a/drivers/devfreq/tegra-devfreq.c
> +++ b/drivers/devfreq/tegra-devfreq.c
> @@ -707,6 +707,10 @@ static int tegra_devfreq_probe(struct platform_device
> *pdev)
>_devfreq_profile,
>"tegra_actmon",
>NULL);
> + if (IS_ERR(tegra->devfreq)) {
> + err = PTR_ERR(tegra->devfreq);
> + return err;
> + }
>
> return 0;
> }
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
[PATCH v2 06/19] PM / devfreq: tegra: Fix missed error checking on devfreq initialization failure
The code doesn't check for devfreq initialization failure, fix it.
Signed-off-by: Dmitry Osipenko
---
drivers/devfreq/tegra-devfreq.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/devfreq/tegra-devfreq.c b/drivers/devfreq/tegra-devfreq.c
index f0f0d78f6cbf..aa0478464b35 100644
--- a/drivers/devfreq/tegra-devfreq.c
+++ b/drivers/devfreq/tegra-devfreq.c
@@ -707,6 +707,10 @@ static int tegra_devfreq_probe(struct platform_device
*pdev)
_devfreq_profile,
"tegra_actmon",
NULL);
+ if (IS_ERR(tegra->devfreq)) {
+ err = PTR_ERR(tegra->devfreq);
+ return err;
+ }
return 0;
}
--
2.21.0
