| Message ID | 1588507469-31889-3-git-send-email-rnayak@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 5/3/2020 5:34 PM, Rajendra Nayak wrote: > geni spi needs to express a perforamnce state requirement on CX > depending on the frequency of the clock rates. Use OPP table from > DT to register with OPP framework and use dev_pm_opp_set_rate() to > set the clk/perf state. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > Reviewed-by: Matthias Kaehlcke <mka@chromium.org> > Cc: Mark Brown <broonie@kernel.org> > Cc: Alok Chauhan <alokc@codeaurora.org> > Cc: Akash Asthana <akashast@codeaurora.org> > Cc: linux-spi@vger.kernel.org > --- > This patch has a dependency on the 'PATCH 1/6' in this series, > due to the changes in include/linux/qcom-geni-se.h > Its ideal if this and the previous patch gets merged via the > msm tree (once reviewed and ack'ed) > Greg has already responded he is fine with it for serial. Mark, would you be able to review/ack this patch so it can be taken in via the msm tree? 'PATCH 1/6' is Ack'd by Greg, and its going to land via the msm tree as well. > > drivers/spi/spi-geni-qcom.c | 26 +++++++++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c > index c397242..bc2916f 100644 > --- a/drivers/spi/spi-geni-qcom.c > +++ b/drivers/spi/spi-geni-qcom.c > @@ -7,6 +7,7 @@ > #include <linux/log2.h> > #include <linux/module.h> > #include <linux/platform_device.h> > +#include <linux/pm_opp.h> > #include <linux/pm_runtime.h> > #include <linux/qcom-geni-se.h> > #include <linux/spi/spi.h> > @@ -95,7 +96,6 @@ static int get_spi_clk_cfg(unsigned int speed_hz, > { > unsigned long sclk_freq; > unsigned int actual_hz; > - struct geni_se *se = &mas->se; > int ret; > > ret = geni_se_clk_freq_match(&mas->se, > @@ -112,9 +112,9 @@ static int get_spi_clk_cfg(unsigned int speed_hz, > > dev_dbg(mas->dev, "req %u=>%u sclk %lu, idx %d, div %d\n", speed_hz, > actual_hz, sclk_freq, *clk_idx, *clk_div); > - ret = clk_set_rate(se->clk, sclk_freq); > + ret = dev_pm_opp_set_rate(mas->dev, sclk_freq); > if (ret) > - dev_err(mas->dev, "clk_set_rate failed %d\n", ret); > + dev_err(mas->dev, "dev_pm_opp_set_rate failed %d\n", ret); > return ret; > } > > @@ -561,6 +561,17 @@ static int spi_geni_probe(struct platform_device *pdev) > mas->se.wrapper = dev_get_drvdata(dev->parent); > mas->se.base = base; > mas->se.clk = clk; > + mas->se.opp_table = dev_pm_opp_set_clkname(&pdev->dev, "se"); > + if (IS_ERR(mas->se.opp_table)) > + return PTR_ERR(mas->se.opp_table); > + /* OPP table is optional */ > + ret = dev_pm_opp_of_add_table(&pdev->dev); > + if (!ret) { > + mas->se.has_opp_table = true; > + } else if (ret != -ENODEV) { > + dev_err(&pdev->dev, "invalid OPP table in device tree\n"); > + return ret; > + } > > spi->bus_num = -1; > spi->dev.of_node = dev->of_node; > @@ -596,6 +607,9 @@ static int spi_geni_probe(struct platform_device *pdev) > spi_geni_probe_runtime_disable: > pm_runtime_disable(dev); > spi_master_put(spi); > + if (mas->se.has_opp_table) > + dev_pm_opp_of_remove_table(&pdev->dev); > + dev_pm_opp_put_clkname(mas->se.opp_table); > return ret; > } > > @@ -604,6 +618,9 @@ static int spi_geni_remove(struct platform_device *pdev) > struct spi_master *spi = platform_get_drvdata(pdev); > struct spi_geni_master *mas = spi_master_get_devdata(spi); > > + if (mas->se.has_opp_table) > + dev_pm_opp_of_remove_table(&pdev->dev); > + dev_pm_opp_put_clkname(mas->se.opp_table); > /* Unregister _before_ disabling pm_runtime() so we stop transfers */ > spi_unregister_master(spi); > > @@ -617,6 +634,9 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev) > struct spi_master *spi = dev_get_drvdata(dev); > struct spi_geni_master *mas = spi_master_get_devdata(spi); > > + /* Drop the performance state vote */ > + dev_pm_opp_set_rate(dev, 0); > + > return geni_se_resources_off(&mas->se); > } > >
On Thu, May 07, 2020 at 10:30:43PM +0530, Rajendra Nayak wrote: > Mark, would you be able to review/ack this patch so it can be > taken in via the msm tree? 'PATCH 1/6' is Ack'd by Greg, and its > going to land via the msm tree as well. Please don't send content free pings and please allow a reasonable time for review. People get busy, go on holiday, attend conferences and so on so unless there is some reason for urgency (like critical bug fixes) please allow at least a couple of weeks for review. If there have been review comments then people may be waiting for those to be addressed. Sending content free pings adds to the mail volume (if they are seen at all) which is often the problem and since they can't be reviewed directly if something has gone wrong you'll have to resend the patches anyway, so sending again is generally a better approach though there are some other maintainers who like them - if in doubt look at how patches for the subsystem are normally handled.
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index c397242..bc2916f 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -7,6 +7,7 @@ #include <linux/log2.h> #include <linux/module.h> #include <linux/platform_device.h> +#include <linux/pm_opp.h> #include <linux/pm_runtime.h> #include <linux/qcom-geni-se.h> #include <linux/spi/spi.h> @@ -95,7 +96,6 @@ static int get_spi_clk_cfg(unsigned int speed_hz, { unsigned long sclk_freq; unsigned int actual_hz; - struct geni_se *se = &mas->se; int ret; ret = geni_se_clk_freq_match(&mas->se, @@ -112,9 +112,9 @@ static int get_spi_clk_cfg(unsigned int speed_hz, dev_dbg(mas->dev, "req %u=>%u sclk %lu, idx %d, div %d\n", speed_hz, actual_hz, sclk_freq, *clk_idx, *clk_div); - ret = clk_set_rate(se->clk, sclk_freq); + ret = dev_pm_opp_set_rate(mas->dev, sclk_freq); if (ret) - dev_err(mas->dev, "clk_set_rate failed %d\n", ret); + dev_err(mas->dev, "dev_pm_opp_set_rate failed %d\n", ret); return ret; } @@ -561,6 +561,17 @@ static int spi_geni_probe(struct platform_device *pdev) mas->se.wrapper = dev_get_drvdata(dev->parent); mas->se.base = base; mas->se.clk = clk; + mas->se.opp_table = dev_pm_opp_set_clkname(&pdev->dev, "se"); + if (IS_ERR(mas->se.opp_table)) + return PTR_ERR(mas->se.opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(&pdev->dev); + if (!ret) { + mas->se.has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(&pdev->dev, "invalid OPP table in device tree\n"); + return ret; + } spi->bus_num = -1; spi->dev.of_node = dev->of_node; @@ -596,6 +607,9 @@ static int spi_geni_probe(struct platform_device *pdev) spi_geni_probe_runtime_disable: pm_runtime_disable(dev); spi_master_put(spi); + if (mas->se.has_opp_table) + dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_put_clkname(mas->se.opp_table); return ret; } @@ -604,6 +618,9 @@ static int spi_geni_remove(struct platform_device *pdev) struct spi_master *spi = platform_get_drvdata(pdev); struct spi_geni_master *mas = spi_master_get_devdata(spi); + if (mas->se.has_opp_table) + dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_put_clkname(mas->se.opp_table); /* Unregister _before_ disabling pm_runtime() so we stop transfers */ spi_unregister_master(spi); @@ -617,6 +634,9 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev) struct spi_master *spi = dev_get_drvdata(dev); struct spi_geni_master *mas = spi_master_get_devdata(spi); + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); + return geni_se_resources_off(&mas->se); }