diff mbox series

[v2,06/26] interconnect: icc-clk: add support for scaling using OPP

Message ID 20230625202547.174647-7-dmitry.baryshkov@linaro.org
State Superseded
Headers show
Series ARM: qcom: apq8064: support CPU frequency scaling | expand

Commit Message

Dmitry Baryshkov June 25, 2023, 8:25 p.m. UTC
Sometimes it might be required to scale the clock using the OPP
framework (e.g. to scale regulators following the required clock rate).
Extend the interconnec-clk framework to handle OPP case in addition to
scaling the clock.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/interconnect/icc-clk.c   | 13 +++++++++++--
 include/linux/interconnect-clk.h |  1 +
 2 files changed, 12 insertions(+), 2 deletions(-)

Comments

Dmitry Baryshkov June 26, 2023, 1:44 p.m. UTC | #1
On 26/06/2023 14:28, Konrad Dybcio wrote:
> On 25.06.2023 22:25, Dmitry Baryshkov wrote:
>> Sometimes it might be required to scale the clock using the OPP
>> framework (e.g. to scale regulators following the required clock rate).
>> Extend the interconnec
> 't'
> 
>> -clk framework to handle OPP case in addition to
>> scaling the clock.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
> I think we should check for OPP at the icc-clk registration time,
> instead of passing it as a parameter, e.g.:
> 
> qn.opp = IS_ERR(dev_pm_opp_get_opp_count)
> 
> Not sure if there's a more idiomatic way.

No. icc-clk is not limited to a single clock->icc conversion. So it is 
possible to create several interconnect links, only one of which should 
be the OPP-based one.

> 
> Konrad
>>   drivers/interconnect/icc-clk.c   | 13 +++++++++++--
>>   include/linux/interconnect-clk.h |  1 +
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c
>> index 4d43ebff4257..c7962acdcee7 100644
>> --- a/drivers/interconnect/icc-clk.c
>> +++ b/drivers/interconnect/icc-clk.c
>> @@ -7,10 +7,13 @@
>>   #include <linux/device.h>
>>   #include <linux/interconnect-clk.h>
>>   #include <linux/interconnect-provider.h>
>> +#include <linux/pm_opp.h>
>>   
>>   struct icc_clk_node {
>> +	struct device *dev;
>>   	struct clk *clk;
>>   	bool enabled;
>> +	bool opp;
>>   };
>>   
>>   struct icc_clk_provider {
>> @@ -25,12 +28,16 @@ struct icc_clk_provider {
>>   static int icc_clk_set(struct icc_node *src, struct icc_node *dst)
>>   {
>>   	struct icc_clk_node *qn = src->data;
>> +	unsigned long rate = icc_units_to_bps(src->peak_bw);
>>   	int ret;
>>   
>>   	if (!qn || !qn->clk)
>>   		return 0;
>>   
>> -	if (!src->peak_bw) {
>> +	if (qn->opp)
>> +		return dev_pm_opp_set_rate(qn->dev, rate);
>> +
>> +	if (!rate) {
>>   		if (qn->enabled)
>>   			clk_disable_unprepare(qn->clk);
>>   		qn->enabled = false;
>> @@ -45,7 +52,7 @@ static int icc_clk_set(struct icc_node *src, struct icc_node *dst)
>>   		qn->enabled = true;
>>   	}
>>   
>> -	return clk_set_rate(qn->clk, icc_units_to_bps(src->peak_bw));
>> +	return clk_set_rate(qn->clk, rate);
>>   }
>>   
>>   static int icc_clk_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
>> @@ -106,7 +113,9 @@ struct icc_provider *icc_clk_register(struct device *dev,
>>   	icc_provider_init(provider);
>>   
>>   	for (i = 0, j = 0; i < num_clocks; i++) {
>> +		qp->clocks[i].dev = dev;
>>   		qp->clocks[i].clk = data[i].clk;
>> +		qp->clocks[i].opp = data[i].opp;
>>   
>>   		node = icc_node_create(first_id + j);
>>   		if (IS_ERR(node)) {
>> diff --git a/include/linux/interconnect-clk.h b/include/linux/interconnect-clk.h
>> index 0cd80112bea5..c695e5099901 100644
>> --- a/include/linux/interconnect-clk.h
>> +++ b/include/linux/interconnect-clk.h
>> @@ -11,6 +11,7 @@ struct device;
>>   struct icc_clk_data {
>>   	struct clk *clk;
>>   	const char *name;
>> +	bool opp;
>>   };
>>   
>>   struct icc_provider *icc_clk_register(struct device *dev,
Konrad Dybcio June 26, 2023, 4:47 p.m. UTC | #2
On 26.06.2023 15:44, Dmitry Baryshkov wrote:
> On 26/06/2023 14:28, Konrad Dybcio wrote:
>> On 25.06.2023 22:25, Dmitry Baryshkov wrote:
>>> Sometimes it might be required to scale the clock using the OPP
>>> framework (e.g. to scale regulators following the required clock rate).
>>> Extend the interconnec
>> 't'
>>
>>> -clk framework to handle OPP case in addition to
>>> scaling the clock.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>> I think we should check for OPP at the icc-clk registration time,
>> instead of passing it as a parameter, e.g.:
>>
>> qn.opp = IS_ERR(dev_pm_opp_get_opp_count)
>>
>> Not sure if there's a more idiomatic way.
> 
> No. icc-clk is not limited to a single clock->icc conversion. So it is possible to create several interconnect links, only one of which should be the OPP-based one.
Ugh. Right.

Konrad
> 
>>
>> Konrad
>>>   drivers/interconnect/icc-clk.c   | 13 +++++++++++--
>>>   include/linux/interconnect-clk.h |  1 +
>>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c
>>> index 4d43ebff4257..c7962acdcee7 100644
>>> --- a/drivers/interconnect/icc-clk.c
>>> +++ b/drivers/interconnect/icc-clk.c
>>> @@ -7,10 +7,13 @@
>>>   #include <linux/device.h>
>>>   #include <linux/interconnect-clk.h>
>>>   #include <linux/interconnect-provider.h>
>>> +#include <linux/pm_opp.h>
>>>     struct icc_clk_node {
>>> +    struct device *dev;
>>>       struct clk *clk;
>>>       bool enabled;
>>> +    bool opp;
>>>   };
>>>     struct icc_clk_provider {
>>> @@ -25,12 +28,16 @@ struct icc_clk_provider {
>>>   static int icc_clk_set(struct icc_node *src, struct icc_node *dst)
>>>   {
>>>       struct icc_clk_node *qn = src->data;
>>> +    unsigned long rate = icc_units_to_bps(src->peak_bw);
>>>       int ret;
>>>         if (!qn || !qn->clk)
>>>           return 0;
>>>   -    if (!src->peak_bw) {
>>> +    if (qn->opp)
>>> +        return dev_pm_opp_set_rate(qn->dev, rate);
>>> +
>>> +    if (!rate) {
>>>           if (qn->enabled)
>>>               clk_disable_unprepare(qn->clk);
>>>           qn->enabled = false;
>>> @@ -45,7 +52,7 @@ static int icc_clk_set(struct icc_node *src, struct icc_node *dst)
>>>           qn->enabled = true;
>>>       }
>>>   -    return clk_set_rate(qn->clk, icc_units_to_bps(src->peak_bw));
>>> +    return clk_set_rate(qn->clk, rate);
>>>   }
>>>     static int icc_clk_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
>>> @@ -106,7 +113,9 @@ struct icc_provider *icc_clk_register(struct device *dev,
>>>       icc_provider_init(provider);
>>>         for (i = 0, j = 0; i < num_clocks; i++) {
>>> +        qp->clocks[i].dev = dev;
>>>           qp->clocks[i].clk = data[i].clk;
>>> +        qp->clocks[i].opp = data[i].opp;
>>>             node = icc_node_create(first_id + j);
>>>           if (IS_ERR(node)) {
>>> diff --git a/include/linux/interconnect-clk.h b/include/linux/interconnect-clk.h
>>> index 0cd80112bea5..c695e5099901 100644
>>> --- a/include/linux/interconnect-clk.h
>>> +++ b/include/linux/interconnect-clk.h
>>> @@ -11,6 +11,7 @@ struct device;
>>>   struct icc_clk_data {
>>>       struct clk *clk;
>>>       const char *name;
>>> +    bool opp;
>>>   };
>>>     struct icc_provider *icc_clk_register(struct device *dev,
>
diff mbox series

Patch

diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c
index 4d43ebff4257..c7962acdcee7 100644
--- a/drivers/interconnect/icc-clk.c
+++ b/drivers/interconnect/icc-clk.c
@@ -7,10 +7,13 @@ 
 #include <linux/device.h>
 #include <linux/interconnect-clk.h>
 #include <linux/interconnect-provider.h>
+#include <linux/pm_opp.h>
 
 struct icc_clk_node {
+	struct device *dev;
 	struct clk *clk;
 	bool enabled;
+	bool opp;
 };
 
 struct icc_clk_provider {
@@ -25,12 +28,16 @@  struct icc_clk_provider {
 static int icc_clk_set(struct icc_node *src, struct icc_node *dst)
 {
 	struct icc_clk_node *qn = src->data;
+	unsigned long rate = icc_units_to_bps(src->peak_bw);
 	int ret;
 
 	if (!qn || !qn->clk)
 		return 0;
 
-	if (!src->peak_bw) {
+	if (qn->opp)
+		return dev_pm_opp_set_rate(qn->dev, rate);
+
+	if (!rate) {
 		if (qn->enabled)
 			clk_disable_unprepare(qn->clk);
 		qn->enabled = false;
@@ -45,7 +52,7 @@  static int icc_clk_set(struct icc_node *src, struct icc_node *dst)
 		qn->enabled = true;
 	}
 
-	return clk_set_rate(qn->clk, icc_units_to_bps(src->peak_bw));
+	return clk_set_rate(qn->clk, rate);
 }
 
 static int icc_clk_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
@@ -106,7 +113,9 @@  struct icc_provider *icc_clk_register(struct device *dev,
 	icc_provider_init(provider);
 
 	for (i = 0, j = 0; i < num_clocks; i++) {
+		qp->clocks[i].dev = dev;
 		qp->clocks[i].clk = data[i].clk;
+		qp->clocks[i].opp = data[i].opp;
 
 		node = icc_node_create(first_id + j);
 		if (IS_ERR(node)) {
diff --git a/include/linux/interconnect-clk.h b/include/linux/interconnect-clk.h
index 0cd80112bea5..c695e5099901 100644
--- a/include/linux/interconnect-clk.h
+++ b/include/linux/interconnect-clk.h
@@ -11,6 +11,7 @@  struct device;
 struct icc_clk_data {
 	struct clk *clk;
 	const char *name;
+	bool opp;
 };
 
 struct icc_provider *icc_clk_register(struct device *dev,