| Message ID | 20201111011456.7875-3-digetx@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Chanwoo Choi |
| Headers | show |
| Series | Introduce memory interconnect for NVIDIA Tegra SoCs | expand |
On 11-11-20, 04:14, Dmitry Osipenko wrote: > The dev_pm_opp_get_opp_table() shouldn't be used by drivers, use > dev_pm_opp_set_clkname() instead. > > Suggested-by: Viresh Kumar <viresh.kumar@linaro.org> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/memory/tegra/tegra20-emc.c | 30 +++++++++++++++++++----------- > 1 file changed, 19 insertions(+), 11 deletions(-) > > diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c > index 5e10aa97809f..bb3f315c9587 100644 > --- a/drivers/memory/tegra/tegra20-emc.c > +++ b/drivers/memory/tegra/tegra20-emc.c > @@ -902,7 +902,7 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc) > > static int tegra_emc_opp_table_init(struct tegra_emc *emc) > { > - struct opp_table *opp_table; > + struct opp_table *reg_opp_table = NULL, *clk_opp_table; > const char *rname = "core"; > int err; > > @@ -917,19 +917,24 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc) > } > > /* voltage scaling is optional */ > - if (device_property_present(emc->dev, "core-supply")) > - opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1); > - else > - opp_table = dev_pm_opp_get_opp_table(emc->dev); > + if (device_property_present(emc->dev, "core-supply")) { > + reg_opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1); > + if (IS_ERR(reg_opp_table)) > + return dev_err_probe(emc->dev, PTR_ERR(reg_opp_table), > + "failed to set OPP regulator\n"); > + } > > - if (IS_ERR(opp_table)) > - return dev_err_probe(emc->dev, PTR_ERR(opp_table), > - "failed to prepare OPP table\n"); > + clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL); > + err = PTR_ERR_OR_ZERO(clk_opp_table); Don't check for NULL here. > + if (err) { > + dev_err(emc->dev, "failed to set OPP clk: %d\n", err); > + goto put_reg_table; > + } > > err = dev_pm_opp_of_add_table(emc->dev); > if (err) { > dev_err(emc->dev, "failed to add OPP table: %d\n", err); > - goto put_table; > + goto put_clk_table; > } > > dev_info(emc->dev, "current clock rate %lu MHz\n", > @@ -946,8 +951,11 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc) > > remove_table: > dev_pm_opp_of_remove_table(emc->dev); > -put_table: > - dev_pm_opp_put_regulators(opp_table); > +put_clk_table: > + dev_pm_opp_put_clkname(clk_opp_table); > +put_reg_table: > + if (reg_opp_table) This won't be required after my other patchset and yeah it is a classic chicken and egg problem we have here :) Maybe you can fix them separately in 5.11 after everything is applied. > + dev_pm_opp_put_regulators(reg_opp_table); > > return err; > } > -- > 2.29.2
On 11-11-20, 11:15, Viresh Kumar wrote: > On 11-11-20, 04:14, Dmitry Osipenko wrote: > > The dev_pm_opp_get_opp_table() shouldn't be used by drivers, use > > dev_pm_opp_set_clkname() instead. > > > > Suggested-by: Viresh Kumar <viresh.kumar@linaro.org> > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > > --- > > drivers/memory/tegra/tegra20-emc.c | 30 +++++++++++++++++++----------- > > 1 file changed, 19 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c > > index 5e10aa97809f..bb3f315c9587 100644 > > --- a/drivers/memory/tegra/tegra20-emc.c > > +++ b/drivers/memory/tegra/tegra20-emc.c > > @@ -902,7 +902,7 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc) > > > > static int tegra_emc_opp_table_init(struct tegra_emc *emc) > > { > > - struct opp_table *opp_table; > > + struct opp_table *reg_opp_table = NULL, *clk_opp_table; > > const char *rname = "core"; > > int err; > > > > @@ -917,19 +917,24 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc) > > } > > > > /* voltage scaling is optional */ > > - if (device_property_present(emc->dev, "core-supply")) > > - opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1); > > - else > > - opp_table = dev_pm_opp_get_opp_table(emc->dev); > > + if (device_property_present(emc->dev, "core-supply")) { > > + reg_opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1); > > + if (IS_ERR(reg_opp_table)) > > + return dev_err_probe(emc->dev, PTR_ERR(reg_opp_table), > > + "failed to set OPP regulator\n"); > > + } > > > > - if (IS_ERR(opp_table)) > > - return dev_err_probe(emc->dev, PTR_ERR(opp_table), > > - "failed to prepare OPP table\n"); > > + clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL); > > + err = PTR_ERR_OR_ZERO(clk_opp_table); > > Don't check for NULL here. My bad. You aren't checking but just converting to err. Its fine.
11.11.2020 08:45, Viresh Kumar пишет: >> +put_reg_table: >> + if (reg_opp_table) > This won't be required after my other patchset and yeah it is a > classic chicken and egg problem we have here :) > > Maybe you can fix them separately in 5.11 after everything is applied. > I already prepared patch in the "core voltage scaling" series that will remove this code.
On Wed, Nov 11, 2020 at 04:14:32AM +0300, Dmitry Osipenko wrote: > The dev_pm_opp_get_opp_table() shouldn't be used by drivers, use > dev_pm_opp_set_clkname() instead. > > Suggested-by: Viresh Kumar <viresh.kumar@linaro.org> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/memory/tegra/tegra20-emc.c | 30 +++++++++++++++++++----------- > 1 file changed, 19 insertions(+), 11 deletions(-) Thanks, applied. Best regards, Krzysztof
diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c index 5e10aa97809f..bb3f315c9587 100644 --- a/drivers/memory/tegra/tegra20-emc.c +++ b/drivers/memory/tegra/tegra20-emc.c @@ -902,7 +902,7 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc) static int tegra_emc_opp_table_init(struct tegra_emc *emc) { - struct opp_table *opp_table; + struct opp_table *reg_opp_table = NULL, *clk_opp_table; const char *rname = "core"; int err; @@ -917,19 +917,24 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc) } /* voltage scaling is optional */ - if (device_property_present(emc->dev, "core-supply")) - opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1); - else - opp_table = dev_pm_opp_get_opp_table(emc->dev); + if (device_property_present(emc->dev, "core-supply")) { + reg_opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1); + if (IS_ERR(reg_opp_table)) + return dev_err_probe(emc->dev, PTR_ERR(reg_opp_table), + "failed to set OPP regulator\n"); + } - if (IS_ERR(opp_table)) - return dev_err_probe(emc->dev, PTR_ERR(opp_table), - "failed to prepare OPP table\n"); + clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL); + err = PTR_ERR_OR_ZERO(clk_opp_table); + if (err) { + dev_err(emc->dev, "failed to set OPP clk: %d\n", err); + goto put_reg_table; + } err = dev_pm_opp_of_add_table(emc->dev); if (err) { dev_err(emc->dev, "failed to add OPP table: %d\n", err); - goto put_table; + goto put_clk_table; } dev_info(emc->dev, "current clock rate %lu MHz\n", @@ -946,8 +951,11 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc) remove_table: dev_pm_opp_of_remove_table(emc->dev); -put_table: - dev_pm_opp_put_regulators(opp_table); +put_clk_table: + dev_pm_opp_put_clkname(clk_opp_table); +put_reg_table: + if (reg_opp_table) + dev_pm_opp_put_regulators(reg_opp_table); return err; }
The dev_pm_opp_get_opp_table() shouldn't be used by drivers, use dev_pm_opp_set_clkname() instead. Suggested-by: Viresh Kumar <viresh.kumar@linaro.org> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/memory/tegra/tegra20-emc.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-)