| Message ID | 1586353607-32222-14-git-send-email-rnayak@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Wed, 8 Apr 2020 at 15:48, Rajendra Nayak <rnayak@codeaurora.org> wrote: > > On some qualcomm SoCs we need to vote on a performance state of a power > domain depending on the clock rates. Hence move to using OPP api to set > the clock rate and performance state specified in the OPP table. > On platforms without an OPP table, dev_pm_opp_set_rate() is eqvivalent to > clk_set_rate() > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: Pradeep P V K <ppvk@codeaurora.org> > Cc: Veerabhadrarao Badiganti <vbadigan@codeaurora.org> > Cc: Subhash Jadavani <subhashj@codeaurora.org> > Cc: linux-mmc@vger.kernel.org This looks good to me! However, are there any of the other patches in the series that $subject patch depends on - or can I apply this as a standalone mmc patch? Kind regards Uffe > --- > drivers/mmc/host/sdhci-msm.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > index 09ff731..d82075a 100644 > --- a/drivers/mmc/host/sdhci-msm.c > +++ b/drivers/mmc/host/sdhci-msm.c > @@ -10,6 +10,7 @@ > #include <linux/delay.h> > #include <linux/mmc/mmc.h> > #include <linux/pm_runtime.h> > +#include <linux/pm_opp.h> > #include <linux/slab.h> > #include <linux/iopoll.h> > #include <linux/regulator/consumer.h> > @@ -242,6 +243,7 @@ struct sdhci_msm_host { > struct clk *xo_clk; /* TCXO clk needed for FLL feature of cm_dll*/ > struct clk_bulk_data bulk_clks[4]; /* core, iface, cal, sleep clocks */ > unsigned long clk_rate; > + struct opp_table *opp; > struct mmc_host *mmc; > bool use_14lpp_dll_reset; > bool tuning_done; > @@ -332,7 +334,7 @@ static void msm_set_clock_rate_for_bus_mode(struct sdhci_host *host, > int rc; > > clock = msm_get_clock_rate_for_bus_mode(host, clock); > - rc = clk_set_rate(core_clk, clock); > + rc = dev_pm_opp_set_rate(mmc_dev(host->mmc), clock); > if (rc) { > pr_err("%s: Failed to set clock at rate %u at timing %d\n", > mmc_hostname(host->mmc), clock, > @@ -1963,7 +1965,7 @@ static int sdhci_msm_probe(struct platform_device *pdev) > msm_host->bulk_clks[0].clk = clk; > > /* Vote for maximum clock rate for maximum performance */ > - ret = clk_set_rate(clk, INT_MAX); > + ret = dev_pm_opp_set_rate(&pdev->dev, INT_MAX); > if (ret) > dev_warn(&pdev->dev, "core clock boost failed\n"); > > @@ -2087,6 +2089,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) > goto clk_disable; > } > > + msm_host->opp = dev_pm_opp_set_clkname(&pdev->dev, "core"); > + dev_pm_opp_of_add_table(&pdev->dev); > + > pm_runtime_get_noresume(&pdev->dev); > pm_runtime_set_active(&pdev->dev); > pm_runtime_enable(&pdev->dev); > @@ -2109,10 +2114,12 @@ static int sdhci_msm_probe(struct platform_device *pdev) > return 0; > > pm_runtime_disable: > + dev_pm_opp_of_remove_table(&pdev->dev); > pm_runtime_disable(&pdev->dev); > pm_runtime_set_suspended(&pdev->dev); > pm_runtime_put_noidle(&pdev->dev); > clk_disable: > + dev_pm_opp_set_rate(&pdev->dev, 0); > clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), > msm_host->bulk_clks); > bus_clk_disable: > @@ -2133,10 +2140,12 @@ static int sdhci_msm_remove(struct platform_device *pdev) > > sdhci_remove_host(host, dead); > > + dev_pm_opp_of_remove_table(&pdev->dev); > pm_runtime_get_sync(&pdev->dev); > pm_runtime_disable(&pdev->dev); > pm_runtime_put_noidle(&pdev->dev); > > + dev_pm_opp_set_rate(&pdev->dev, 0); > clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), > msm_host->bulk_clks); > if (!IS_ERR(msm_host->bus_clk)) > @@ -2151,6 +2160,7 @@ static __maybe_unused int sdhci_msm_runtime_suspend(struct device *dev) > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); > > + dev_pm_opp_set_rate(dev, 0); > clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), > msm_host->bulk_clks); > > @@ -2173,9 +2183,11 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) > * restore the SDR DLL settings when the clock is ungated. > */ > if (msm_host->restore_dll_config && msm_host->clk_rate) > - return sdhci_msm_restore_sdr_dll_config(host); > + ret = sdhci_msm_restore_sdr_dll_config(host); > > - return 0; > + dev_pm_opp_set_rate(dev, msm_host->clk_rate); > + > + return ret; > } > > static const struct dev_pm_ops sdhci_msm_pm_ops = { > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation
On 4/15/2020 7:22 PM, Ulf Hansson wrote: > On Wed, 8 Apr 2020 at 15:48, Rajendra Nayak <rnayak@codeaurora.org> wrote: >> >> On some qualcomm SoCs we need to vote on a performance state of a power >> domain depending on the clock rates. Hence move to using OPP api to set >> the clock rate and performance state specified in the OPP table. >> On platforms without an OPP table, dev_pm_opp_set_rate() is eqvivalent to >> clk_set_rate() >> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >> Cc: Ulf Hansson <ulf.hansson@linaro.org> >> Cc: Pradeep P V K <ppvk@codeaurora.org> >> Cc: Veerabhadrarao Badiganti <vbadigan@codeaurora.org> >> Cc: Subhash Jadavani <subhashj@codeaurora.org> >> Cc: linux-mmc@vger.kernel.org > > This looks good to me! > > However, are there any of the other patches in the series that > $subject patch depends on - or can I apply this as a standalone mmc > patch? Hey Ulf, thanks for the review. I'll just need to respin these to make sure I do not do a dev_pm_opp_of_remove_table() if dev_pm_opp_of_add_table() isn;t successful as discussed with Viresh on another thread [1] As for the dependencies, its only PATCH 01/21 in this series and that's already been queued by Viresh [2] [1] https://lkml.org/lkml/2020/4/15/18 [2] https://lkml.org/lkml/2020/4/14/98 > > Kind regards > Uffe > >> --- >> drivers/mmc/host/sdhci-msm.c | 20 ++++++++++++++++---- >> 1 file changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c >> index 09ff731..d82075a 100644 >> --- a/drivers/mmc/host/sdhci-msm.c >> +++ b/drivers/mmc/host/sdhci-msm.c >> @@ -10,6 +10,7 @@ >> #include <linux/delay.h> >> #include <linux/mmc/mmc.h> >> #include <linux/pm_runtime.h> >> +#include <linux/pm_opp.h> >> #include <linux/slab.h> >> #include <linux/iopoll.h> >> #include <linux/regulator/consumer.h> >> @@ -242,6 +243,7 @@ struct sdhci_msm_host { >> struct clk *xo_clk; /* TCXO clk needed for FLL feature of cm_dll*/ >> struct clk_bulk_data bulk_clks[4]; /* core, iface, cal, sleep clocks */ >> unsigned long clk_rate; >> + struct opp_table *opp; >> struct mmc_host *mmc; >> bool use_14lpp_dll_reset; >> bool tuning_done; >> @@ -332,7 +334,7 @@ static void msm_set_clock_rate_for_bus_mode(struct sdhci_host *host, >> int rc; >> >> clock = msm_get_clock_rate_for_bus_mode(host, clock); >> - rc = clk_set_rate(core_clk, clock); >> + rc = dev_pm_opp_set_rate(mmc_dev(host->mmc), clock); >> if (rc) { >> pr_err("%s: Failed to set clock at rate %u at timing %d\n", >> mmc_hostname(host->mmc), clock, >> @@ -1963,7 +1965,7 @@ static int sdhci_msm_probe(struct platform_device *pdev) >> msm_host->bulk_clks[0].clk = clk; >> >> /* Vote for maximum clock rate for maximum performance */ >> - ret = clk_set_rate(clk, INT_MAX); >> + ret = dev_pm_opp_set_rate(&pdev->dev, INT_MAX); >> if (ret) >> dev_warn(&pdev->dev, "core clock boost failed\n"); >> >> @@ -2087,6 +2089,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) >> goto clk_disable; >> } >> >> + msm_host->opp = dev_pm_opp_set_clkname(&pdev->dev, "core"); >> + dev_pm_opp_of_add_table(&pdev->dev); >> + >> pm_runtime_get_noresume(&pdev->dev); >> pm_runtime_set_active(&pdev->dev); >> pm_runtime_enable(&pdev->dev); >> @@ -2109,10 +2114,12 @@ static int sdhci_msm_probe(struct platform_device *pdev) >> return 0; >> >> pm_runtime_disable: >> + dev_pm_opp_of_remove_table(&pdev->dev); >> pm_runtime_disable(&pdev->dev); >> pm_runtime_set_suspended(&pdev->dev); >> pm_runtime_put_noidle(&pdev->dev); >> clk_disable: >> + dev_pm_opp_set_rate(&pdev->dev, 0); >> clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), >> msm_host->bulk_clks); >> bus_clk_disable: >> @@ -2133,10 +2140,12 @@ static int sdhci_msm_remove(struct platform_device *pdev) >> >> sdhci_remove_host(host, dead); >> >> + dev_pm_opp_of_remove_table(&pdev->dev); >> pm_runtime_get_sync(&pdev->dev); >> pm_runtime_disable(&pdev->dev); >> pm_runtime_put_noidle(&pdev->dev); >> >> + dev_pm_opp_set_rate(&pdev->dev, 0); >> clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), >> msm_host->bulk_clks); >> if (!IS_ERR(msm_host->bus_clk)) >> @@ -2151,6 +2160,7 @@ static __maybe_unused int sdhci_msm_runtime_suspend(struct device *dev) >> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); >> >> + dev_pm_opp_set_rate(dev, 0); >> clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), >> msm_host->bulk_clks); >> >> @@ -2173,9 +2183,11 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) >> * restore the SDR DLL settings when the clock is ungated. >> */ >> if (msm_host->restore_dll_config && msm_host->clk_rate) >> - return sdhci_msm_restore_sdr_dll_config(host); >> + ret = sdhci_msm_restore_sdr_dll_config(host); >> >> - return 0; >> + dev_pm_opp_set_rate(dev, msm_host->clk_rate); >> + >> + return ret; >> } >> >> static const struct dev_pm_ops sdhci_msm_pm_ops = { >> -- >> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member >> of Code Aurora Forum, hosted by The Linux Foundation
On 15-04-20, 22:13, Rajendra Nayak wrote: > As for the dependencies, its only PATCH 01/21 in this series and that's > already been queued by Viresh [2] It must be part of v5.7-rc2
On Thu, 16 Apr 2020 at 05:39, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 15-04-20, 22:13, Rajendra Nayak wrote: > > As for the dependencies, its only PATCH 01/21 in this series and that's > > already been queued by Viresh [2] > > It must be part of v5.7-rc2 Great, thanks! KInd regards Uffe
On Wed, 15 Apr 2020 at 18:43, Rajendra Nayak <rnayak@codeaurora.org> wrote: > > > > On 4/15/2020 7:22 PM, Ulf Hansson wrote: > > On Wed, 8 Apr 2020 at 15:48, Rajendra Nayak <rnayak@codeaurora.org> wrote: > >> > >> On some qualcomm SoCs we need to vote on a performance state of a power > >> domain depending on the clock rates. Hence move to using OPP api to set > >> the clock rate and performance state specified in the OPP table. > >> On platforms without an OPP table, dev_pm_opp_set_rate() is eqvivalent to > >> clk_set_rate() > >> > >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > >> Cc: Ulf Hansson <ulf.hansson@linaro.org> > >> Cc: Pradeep P V K <ppvk@codeaurora.org> > >> Cc: Veerabhadrarao Badiganti <vbadigan@codeaurora.org> > >> Cc: Subhash Jadavani <subhashj@codeaurora.org> > >> Cc: linux-mmc@vger.kernel.org > > > > This looks good to me! > > > > However, are there any of the other patches in the series that > > $subject patch depends on - or can I apply this as a standalone mmc > > patch? > > Hey Ulf, thanks for the review. I'll just need to respin these to make > sure I do not do a dev_pm_opp_of_remove_table() if dev_pm_opp_of_add_table() > isn;t successful as discussed with Viresh on another thread [1] > > As for the dependencies, its only PATCH 01/21 in this series and that's > already been queued by Viresh [2] I see, thanks! Looks like Viresh is sending it to be included for v5.7-rc2, so I can pick your new version of $subject patch next week. [...] Kind regards Uffe
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c index 09ff731..d82075a 100644 --- a/drivers/mmc/host/sdhci-msm.c +++ b/drivers/mmc/host/sdhci-msm.c @@ -10,6 +10,7 @@ #include <linux/delay.h> #include <linux/mmc/mmc.h> #include <linux/pm_runtime.h> +#include <linux/pm_opp.h> #include <linux/slab.h> #include <linux/iopoll.h> #include <linux/regulator/consumer.h> @@ -242,6 +243,7 @@ struct sdhci_msm_host { struct clk *xo_clk; /* TCXO clk needed for FLL feature of cm_dll*/ struct clk_bulk_data bulk_clks[4]; /* core, iface, cal, sleep clocks */ unsigned long clk_rate; + struct opp_table *opp; struct mmc_host *mmc; bool use_14lpp_dll_reset; bool tuning_done; @@ -332,7 +334,7 @@ static void msm_set_clock_rate_for_bus_mode(struct sdhci_host *host, int rc; clock = msm_get_clock_rate_for_bus_mode(host, clock); - rc = clk_set_rate(core_clk, clock); + rc = dev_pm_opp_set_rate(mmc_dev(host->mmc), clock); if (rc) { pr_err("%s: Failed to set clock at rate %u at timing %d\n", mmc_hostname(host->mmc), clock, @@ -1963,7 +1965,7 @@ static int sdhci_msm_probe(struct platform_device *pdev) msm_host->bulk_clks[0].clk = clk; /* Vote for maximum clock rate for maximum performance */ - ret = clk_set_rate(clk, INT_MAX); + ret = dev_pm_opp_set_rate(&pdev->dev, INT_MAX); if (ret) dev_warn(&pdev->dev, "core clock boost failed\n"); @@ -2087,6 +2089,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) goto clk_disable; } + msm_host->opp = dev_pm_opp_set_clkname(&pdev->dev, "core"); + dev_pm_opp_of_add_table(&pdev->dev); + pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev); @@ -2109,10 +2114,12 @@ static int sdhci_msm_probe(struct platform_device *pdev) return 0; pm_runtime_disable: + dev_pm_opp_of_remove_table(&pdev->dev); pm_runtime_disable(&pdev->dev); pm_runtime_set_suspended(&pdev->dev); pm_runtime_put_noidle(&pdev->dev); clk_disable: + dev_pm_opp_set_rate(&pdev->dev, 0); clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), msm_host->bulk_clks); bus_clk_disable: @@ -2133,10 +2140,12 @@ static int sdhci_msm_remove(struct platform_device *pdev) sdhci_remove_host(host, dead); + dev_pm_opp_of_remove_table(&pdev->dev); pm_runtime_get_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); pm_runtime_put_noidle(&pdev->dev); + dev_pm_opp_set_rate(&pdev->dev, 0); clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), msm_host->bulk_clks); if (!IS_ERR(msm_host->bus_clk)) @@ -2151,6 +2160,7 @@ static __maybe_unused int sdhci_msm_runtime_suspend(struct device *dev) struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); + dev_pm_opp_set_rate(dev, 0); clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), msm_host->bulk_clks); @@ -2173,9 +2183,11 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) * restore the SDR DLL settings when the clock is ungated. */ if (msm_host->restore_dll_config && msm_host->clk_rate) - return sdhci_msm_restore_sdr_dll_config(host); + ret = sdhci_msm_restore_sdr_dll_config(host); - return 0; + dev_pm_opp_set_rate(dev, msm_host->clk_rate); + + return ret; } static const struct dev_pm_ops sdhci_msm_pm_ops = {
On some qualcomm SoCs we need to vote on a performance state of a power domain depending on the clock rates. Hence move to using OPP api to set the clock rate and performance state specified in the OPP table. On platforms without an OPP table, dev_pm_opp_set_rate() is eqvivalent to clk_set_rate() Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> Cc: Ulf Hansson <ulf.hansson@linaro.org> Cc: Pradeep P V K <ppvk@codeaurora.org> Cc: Veerabhadrarao Badiganti <vbadigan@codeaurora.org> Cc: Subhash Jadavani <subhashj@codeaurora.org> Cc: linux-mmc@vger.kernel.org --- drivers/mmc/host/sdhci-msm.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)