[PATCH 03/21] spi: spi-geni-qcom: Use OPP API to set clk/perf state

[PATCH 03/21] spi: spi-geni-qcom: Use OPP API to set clk/perf state

[Rajendra Nayak]

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>
Cc: Alok Chauhan <alokc@codeaurora.org>
Cc: Akash Asthana <akashast@codeaurora.org>
Cc: linux-spi@vger.kernel.org
---
 drivers/spi/spi-geni-qcom.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index c397242..ce387dc 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;
 }
 
@@ -553,6 +553,7 @@ static int spi_geni_probe(struct platform_device *pdev)
 	if (!spi)
 		return -ENOMEM;
 
+
 	platform_set_drvdata(pdev, spi);
 	mas = spi_master_get_devdata(spi);
 	mas->irq = irq;
@@ -561,6 +562,8 @@ 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 = dev_pm_opp_set_clkname(&pdev->dev, "se");
+	dev_pm_opp_of_add_table(&pdev->dev);
 
 	spi->bus_num = -1;
 	spi->dev.of_node = dev->of_node;
@@ -596,6 +599,7 @@ static int spi_geni_probe(struct platform_device *pdev)
 spi_geni_probe_runtime_disable:
 	pm_runtime_disable(dev);
 	spi_master_put(spi);
+	dev_pm_opp_of_remove_table(&pdev->dev);
 	return ret;
 }
 
@@ -604,6 +608,7 @@ 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);
 
+	dev_pm_opp_of_remove_table(&pdev->dev);
 	/* Unregister _before_ disabling pm_runtime() so we stop transfers */
 	spi_unregister_master(spi);
 
@@ -617,6 +622,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);
 }
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 19/21] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Rajendra Nayak]

QSPI needs to vote on a performance state of a power domain depending on
the clock rate. Add support for it by specifying the perf state/clock rate
as an OPP table in device tree.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Cc: Alok Chauhan <alokc@codeaurora.org>
Cc: Akash Asthana <akashast@codeaurora.org>
Cc: linux-spi@vger.kernel.org
---
 drivers/spi/spi-qcom-qspi.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c
index 3c4f83b..42a9511 100644
--- a/drivers/spi/spi-qcom-qspi.c
+++ b/drivers/spi/spi-qcom-qspi.c
@@ -8,6 +8,7 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/pm_runtime.h>
+#include <linux/pm_opp.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/spi-mem.h>
 
@@ -235,7 +236,7 @@ static int qcom_qspi_transfer_one(struct spi_master *master,
 		speed_hz = xfer->speed_hz;
 
 	/* In regular operation (SBL_EN=1) core must be 4x transfer clock */
-	ret = clk_set_rate(ctrl->clks[QSPI_CLK_CORE].clk, speed_hz * 4);
+	ret = dev_pm_opp_set_rate(ctrl->dev, speed_hz * 4);
 	if (ret) {
 		dev_err(ctrl->dev, "Failed to set core clk %d\n", ret);
 		return ret;
@@ -481,6 +482,9 @@ static int qcom_qspi_probe(struct platform_device *pdev)
 	master->handle_err = qcom_qspi_handle_err;
 	master->auto_runtime_pm = true;
 
+	dev_pm_opp_set_clkname(&pdev->dev, "core");
+	dev_pm_opp_of_add_table(&pdev->dev);
+
 	pm_runtime_enable(dev);
 
 	ret = spi_register_master(master);
@@ -490,6 +494,7 @@ static int qcom_qspi_probe(struct platform_device *pdev)
 	pm_runtime_disable(dev);
 
 exit_probe_master_put:
+	dev_pm_opp_of_remove_table(&pdev->dev);
 	spi_master_put(master);
 
 	return ret;
@@ -499,6 +504,8 @@ static int qcom_qspi_remove(struct platform_device *pdev)
 {
 	struct spi_master *master = platform_get_drvdata(pdev);
 
+	dev_pm_opp_of_remove_table(&pdev->dev);
+
 	/* Unregister _before_ disabling pm_runtime() so we stop transfers */
 	spi_unregister_master(master);
 
@@ -512,6 +519,7 @@ static int __maybe_unused qcom_qspi_runtime_suspend(struct device *dev)
 	struct spi_master *master = dev_get_drvdata(dev);
 	struct qcom_qspi *ctrl = spi_master_get_devdata(master);
 
+	dev_pm_opp_set_rate(dev, 0);
 	clk_bulk_disable_unprepare(QSPI_NUM_CLKS, ctrl->clks);
 
 	return 0;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 03/21] spi: spi-geni-qcom: Use OPP API to set clk/perf state

[Matthias Kaehlcke]

Hi Rajendra,

On Wed, Apr 08, 2020 at 07:16:29PM +0530, 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>
> Cc: Alok Chauhan <alokc@codeaurora.org>
> Cc: Akash Asthana <akashast@codeaurora.org>
> Cc: linux-spi@vger.kernel.org
> ---
>  drivers/spi/spi-geni-qcom.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index c397242..ce387dc 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;
>  }
>  
> @@ -553,6 +553,7 @@ static int spi_geni_probe(struct platform_device *pdev)
>  	if (!spi)
>  		return -ENOMEM;
>  
> +
>  	platform_set_drvdata(pdev, spi);
>  	mas = spi_master_get_devdata(spi);
>  	mas->irq = irq;
> @@ -561,6 +562,8 @@ 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 = dev_pm_opp_set_clkname(&pdev->dev, "se");

As commented on the serial patch, it seems an error check is needed
and the OPP table saved in 'struct geni_se' is never used.

> +	dev_pm_opp_of_add_table(&pdev->dev);

This function could also fail for multiple reasons, so the return value
should be checked.

From patch "[01/21] opp: Manage empty OPP tables with clk handle" it seems
ignoring errors is intended to be able to operate when no OPP table is
specified. But even with that you want to return in case of certain errors,
like an invalid OPP table, out of memory or -EPROBE_DEFER.

Re: [PATCH 03/21] spi: spi-geni-qcom: Use OPP API to set clk/perf state

[Rajendra Nayak]

On 4/9/2020 11:50 PM, Matthias Kaehlcke wrote:
> Hi Rajendra,
> 
> On Wed, Apr 08, 2020 at 07:16:29PM +0530, 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>
>> Cc: Alok Chauhan <alokc@codeaurora.org>
>> Cc: Akash Asthana <akashast@codeaurora.org>
>> Cc: linux-spi@vger.kernel.org
>> ---
>>   drivers/spi/spi-geni-qcom.c | 14 +++++++++++---
>>   1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
>> index c397242..ce387dc 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;
>>   }
>>   
>> @@ -553,6 +553,7 @@ static int spi_geni_probe(struct platform_device *pdev)
>>   	if (!spi)
>>   		return -ENOMEM;
>>   
>> +
>>   	platform_set_drvdata(pdev, spi);
>>   	mas = spi_master_get_devdata(spi);
>>   	mas->irq = irq;
>> @@ -561,6 +562,8 @@ 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 = dev_pm_opp_set_clkname(&pdev->dev, "se");
> 
> As commented on the serial patch, it seems an error check is needed
> and the OPP table saved in 'struct geni_se' is never used.

right, I explained about the OPP table in the other patch.

> 
>> +	dev_pm_opp_of_add_table(&pdev->dev);
> 
> This function could also fail for multiple reasons, so the return value
> should be checked.
> 
>  From patch "[01/21] opp: Manage empty OPP tables with clk handle" it seems
> ignoring errors is intended to be able to operate when no OPP table is
> specified. But even with that you want to return in case of certain errors,
> like an invalid OPP table, out of memory or -EPROBE_DEFER.

Thats correct, my intention of not checking the errors was in cases where its
optional, and I did not want to break anything with existing DT. I will go back and
check if its indeed possible for it to return a -EPROBE_DEFER and others which
I should handle.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation