| Message ID | 20190415145505.18397-7-digetx@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | NVIDIA Tegra devfreq improvements and Tegra20/30 support | expand |
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 <digetx@gmail.com> > --- > 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) > &tegra_devfreq_profile, > "tegra_actmon", > NULL); > + if (IS_ERR(tegra->devfreq)) { > + err = PTR_ERR(tegra->devfreq); > + return err; > + } > > return 0; > } >
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.
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. > >
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) &tegra_devfreq_profile, "tegra_actmon", NULL); + if (IS_ERR(tegra->devfreq)) { + err = PTR_ERR(tegra->devfreq); + return err; + } return 0; }
The code doesn't check for devfreq initialization failure, fix it. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/devfreq/tegra-devfreq.c | 4 ++++ 1 file changed, 4 insertions(+)