| Message ID | 20190707223303.6755-4-digetx@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | More improvements for Tegra30 devfreq driver | expand |
On 19. 7. 8. 오전 7:32, Dmitry Osipenko wrote: > The EMC clock rate rounding technically could fail, hence let's handle > the error cases properly. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/devfreq/tegra30-devfreq.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c > index 5e2b133babdd..5e606ae3f238 100644 > --- a/drivers/devfreq/tegra30-devfreq.c > +++ b/drivers/devfreq/tegra30-devfreq.c > @@ -592,8 +592,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev) > struct tegra_devfreq_device *dev; > struct tegra_devfreq *tegra; > struct devfreq *devfreq; > - unsigned long rate; > unsigned int i; > + long rate; > int err; > > tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL); > @@ -650,8 +650,14 @@ static int tegra_devfreq_probe(struct platform_device *pdev) > > reset_control_deassert(tegra->reset); > > - tegra->max_freq = clk_round_rate(tegra->emc_clock, ULONG_MAX) / KHZ; > + rate = clk_round_rate(tegra->emc_clock, ULONG_MAX); > + if (rate < 0) { > + dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate); > + return rate; > + } > + > tegra->cur_freq = clk_get_rate(tegra->emc_clock) / KHZ; > + tegra->max_freq = rate / KHZ; > > for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) { > dev = tegra->devices + i; > @@ -662,6 +668,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev) > for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) { > rate = clk_round_rate(tegra->emc_clock, rate); > Please remove unneeded blank line. > + if (rate < 0) { > + dev_err(&pdev->dev, > + "Failed to round clock rate: %ld\n", rate); > + err = rate; > + goto remove_opps; > + } Also, this patch doesn't contain code which restore the previous tegra->cur_freq/max_freq when error happen. > + > err = dev_pm_opp_add(&pdev->dev, rate, 0); > if (err) { > dev_err(&pdev->dev, "Failed to add OPP: %d\n", err); >
16.07.2019 14:50, Chanwoo Choi пишет: > On 19. 7. 8. 오전 7:32, Dmitry Osipenko wrote: >> The EMC clock rate rounding technically could fail, hence let's handle >> the error cases properly. >> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/devfreq/tegra30-devfreq.c | 17 +++++++++++++++-- >> 1 file changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c >> index 5e2b133babdd..5e606ae3f238 100644 >> --- a/drivers/devfreq/tegra30-devfreq.c >> +++ b/drivers/devfreq/tegra30-devfreq.c >> @@ -592,8 +592,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev) >> struct tegra_devfreq_device *dev; >> struct tegra_devfreq *tegra; >> struct devfreq *devfreq; >> - unsigned long rate; >> unsigned int i; >> + long rate; >> int err; >> >> tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL); >> @@ -650,8 +650,14 @@ static int tegra_devfreq_probe(struct platform_device *pdev) >> >> reset_control_deassert(tegra->reset); >> >> - tegra->max_freq = clk_round_rate(tegra->emc_clock, ULONG_MAX) / KHZ; >> + rate = clk_round_rate(tegra->emc_clock, ULONG_MAX); >> + if (rate < 0) { >> + dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate); >> + return rate; >> + } >> + >> tegra->cur_freq = clk_get_rate(tegra->emc_clock) / KHZ; >> + tegra->max_freq = rate / KHZ; >> >> for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) { >> dev = tegra->devices + i; >> @@ -662,6 +668,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev) >> for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) { >> rate = clk_round_rate(tegra->emc_clock, rate); >> > > Please remove unneeded blank line. I can remove it, but it was added specifically to ease reading of the code. >> + if (rate < 0) { >> + dev_err(&pdev->dev, >> + "Failed to round clock rate: %ld\n", rate); >> + err = rate; >> + goto remove_opps; >> + } > > Also, this patch doesn't contain code which restore the previous > tegra->cur_freq/max_freq when error happen. The error here results in abortion of the driver's probing, hence nothing need to be restored in that case because nothing was changed at this point yet.
On 19. 7. 16. 오후 10:09, Dmitry Osipenko wrote: > 16.07.2019 14:50, Chanwoo Choi пишет: >> On 19. 7. 8. 오전 7:32, Dmitry Osipenko wrote: >>> The EMC clock rate rounding technically could fail, hence let's handle >>> the error cases properly. >>> >>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>> --- >>> drivers/devfreq/tegra30-devfreq.c | 17 +++++++++++++++-- >>> 1 file changed, 15 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c >>> index 5e2b133babdd..5e606ae3f238 100644 >>> --- a/drivers/devfreq/tegra30-devfreq.c >>> +++ b/drivers/devfreq/tegra30-devfreq.c >>> @@ -592,8 +592,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev) >>> struct tegra_devfreq_device *dev; >>> struct tegra_devfreq *tegra; >>> struct devfreq *devfreq; >>> - unsigned long rate; >>> unsigned int i; >>> + long rate; >>> int err; >>> >>> tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL); >>> @@ -650,8 +650,14 @@ static int tegra_devfreq_probe(struct platform_device *pdev) >>> >>> reset_control_deassert(tegra->reset); >>> >>> - tegra->max_freq = clk_round_rate(tegra->emc_clock, ULONG_MAX) / KHZ; >>> + rate = clk_round_rate(tegra->emc_clock, ULONG_MAX); >>> + if (rate < 0) { >>> + dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate); >>> + return rate; >>> + } >>> + >>> tegra->cur_freq = clk_get_rate(tegra->emc_clock) / KHZ; >>> + tegra->max_freq = rate / KHZ; >>> >>> for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) { >>> dev = tegra->devices + i; >>> @@ -662,6 +668,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev) >>> for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) { >>> rate = clk_round_rate(tegra->emc_clock, rate); >>> >> >> Please remove unneeded blank line. > > I can remove it, but it was added specifically to ease reading of the code. > >>> + if (rate < 0) { >>> + dev_err(&pdev->dev, >>> + "Failed to round clock rate: %ld\n", rate); >>> + err = rate; >>> + goto remove_opps; >>> + } >> >> Also, this patch doesn't contain code which restore the previous >> tegra->cur_freq/max_freq when error happen. > > The error here results in abortion of the driver's probing, hence > nothing need to be restored in that case because nothing was changed at > this point yet. > > OK.
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c index 5e2b133babdd..5e606ae3f238 100644 --- a/drivers/devfreq/tegra30-devfreq.c +++ b/drivers/devfreq/tegra30-devfreq.c @@ -592,8 +592,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev) struct tegra_devfreq_device *dev; struct tegra_devfreq *tegra; struct devfreq *devfreq; - unsigned long rate; unsigned int i; + long rate; int err; tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL); @@ -650,8 +650,14 @@ static int tegra_devfreq_probe(struct platform_device *pdev) reset_control_deassert(tegra->reset); - tegra->max_freq = clk_round_rate(tegra->emc_clock, ULONG_MAX) / KHZ; + rate = clk_round_rate(tegra->emc_clock, ULONG_MAX); + if (rate < 0) { + dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate); + return rate; + } + tegra->cur_freq = clk_get_rate(tegra->emc_clock) / KHZ; + tegra->max_freq = rate / KHZ; for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) { dev = tegra->devices + i; @@ -662,6 +668,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev) for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) { rate = clk_round_rate(tegra->emc_clock, rate); + if (rate < 0) { + dev_err(&pdev->dev, + "Failed to round clock rate: %ld\n", rate); + err = rate; + goto remove_opps; + } + err = dev_pm_opp_add(&pdev->dev, rate, 0); if (err) { dev_err(&pdev->dev, "Failed to add OPP: %d\n", err);
The EMC clock rate rounding technically could fail, hence let's handle the error cases properly. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/devfreq/tegra30-devfreq.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)