Hi Santosh,
This series contains a few fixes for the TI SCI clock driver.
- Patch #1 is a clear bug fix, where we missed to parse assigned-clock
data properly to detect which clocks are in use on the SoC.
- Patch #2 is a performance improvement patch which avoids some
unnecessary round trips to firmware side while setting clock
frequency.
- Patch #3 fixes some issues with set_rate passed to firmware, where the
parameters are too strict; namely, firmware fails to handle some cases
properly if min,tgt,max values for a clock rate are exactly the same
value. Yeah, the firmware is quite weird here but nothing much else we
can do from kernel side other than this....
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Currently, we request exact clock rates from the firmware to be set with
set_rate. Due to some rounding errors and internal functionality of the
firmware itself, this can fail. Thus, add some slack to the set_rate
functionality so that we are always guaranteed to pass. The firmware
always attempts to use frequency as close to the target freq as
possible despite the slack given here.
Signed-off-by: Tero Kristo <[email protected]>
---
drivers/clk/keystone/sci-clk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
index b6477b08af04..aaf31abe1c8f 100644
--- a/drivers/clk/keystone/sci-clk.c
+++ b/drivers/clk/keystone/sci-clk.c
@@ -221,7 +221,8 @@ static int sci_clk_set_rate(struct clk_hw *hw, unsigned long rate,
struct sci_clk *clk = to_sci_clk(hw);
return clk->provider->ops->set_freq(clk->provider->sci, clk->dev_id,
- clk->clk_id, rate, rate, rate);
+ clk->clk_id, rate / 10 * 9, rate,
+ rate / 10 * 11);
}
/**
--
2.17.1
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
The DT clock probe loop incorrectly terminates after processing "clocks"
only, fix this by re-starting the loop when all entries for current
DT property have been parsed.
Fixes: 8e48b33f9def ("clk: keystone: sci-clk: probe clocks from DT instead of firmware")
Reported-by: Peter Ujfalusi <[email protected]>
Signed-off-by: Tero Kristo <[email protected]>
---
drivers/clk/keystone/sci-clk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
index 2ad26cb927fd..f126b6045afa 100644
--- a/drivers/clk/keystone/sci-clk.c
+++ b/drivers/clk/keystone/sci-clk.c
@@ -522,7 +522,7 @@ static int ti_sci_scan_clocks_from_dt(struct sci_clk_provider *provider)
np = of_find_node_with_property(np, *clk_name);
if (!np) {
clk_name++;
- break;
+ continue;
}
if (!of_device_is_available(np))
--
2.17.1
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Cache results of the latest query rate operation. This optimizes the
firmware interface a bit, avoiding unnecessary calls to firmware if we
know the result already; the firmware interface is pretty expensive
to use for query rate functionality.
Signed-off-by: Tero Kristo <[email protected]>
---
drivers/clk/keystone/sci-clk.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
index f126b6045afa..b6477b08af04 100644
--- a/drivers/clk/keystone/sci-clk.c
+++ b/drivers/clk/keystone/sci-clk.c
@@ -54,6 +54,8 @@ struct sci_clk_provider {
* @provider: Master clock provider
* @flags: Flags for the clock
* @node: Link for handling clocks probed via DT
+ * @cached_req: Cached requested freq for determine rate calls
+ * @cached_res: Cached result freq for determine rate calls
*/
struct sci_clk {
struct clk_hw hw;
@@ -63,6 +65,8 @@ struct sci_clk {
struct sci_clk_provider *provider;
u8 flags;
struct list_head node;
+ unsigned long cached_req;
+ unsigned long cached_res;
};
#define to_sci_clk(_hw) container_of(_hw, struct sci_clk, hw)
@@ -175,6 +179,11 @@ static int sci_clk_determine_rate(struct clk_hw *hw,
int ret;
u64 new_rate;
+ if (clk->cached_req && clk->cached_req == req->rate) {
+ req->rate = clk->cached_res;
+ return 0;
+ }
+
ret = clk->provider->ops->get_best_match_freq(clk->provider->sci,
clk->dev_id,
clk->clk_id,
@@ -189,6 +198,9 @@ static int sci_clk_determine_rate(struct clk_hw *hw,
return ret;
}
+ clk->cached_req = req->rate;
+ clk->cached_res = new_rate;
+
req->rate = new_rate;
return 0;
@@ -249,6 +261,8 @@ static int sci_clk_set_parent(struct clk_hw *hw, u8 index)
{
struct sci_clk *clk = to_sci_clk(hw);
+ clk->cached_req = 0;
+
return clk->provider->ops->set_parent(clk->provider->sci, clk->dev_id,
clk->clk_id,
index + 1 + clk->clk_id);
--
2.17.1
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 9/7/20 1:57 AM, Tero Kristo wrote:
> Hi Santosh,
>
> This series contains a few fixes for the TI SCI clock driver.
> - Patch #1 is a clear bug fix, where we missed to parse assigned-clock
> data properly to detect which clocks are in use on the SoC.
> - Patch #2 is a performance improvement patch which avoids some
> unnecessary round trips to firmware side while setting clock
> frequency.
> - Patch #3 fixes some issues with set_rate passed to firmware, where the
> parameters are too strict; namely, firmware fails to handle some cases
> properly if min,tgt,max values for a clock rate are exactly the same
> value. Yeah, the firmware is quite weird here but nothing much else we
> can do from kernel side other than this....
>
Looks fine to me Tero.
Acked-by: Santosh Shilimkar <[email protected]>
Hi Stephen, Mike,
Can you please pick these fixes via clk tree ?
Quoting [email protected] (2020-09-08 10:19:32)
>
>
> On 9/7/20 1:57 AM, Tero Kristo wrote:
> > Hi Santosh,
> >
> > This series contains a few fixes for the TI SCI clock driver.
> > - Patch #1 is a clear bug fix, where we missed to parse assigned-clock
> > data properly to detect which clocks are in use on the SoC.
> > - Patch #2 is a performance improvement patch which avoids some
> > unnecessary round trips to firmware side while setting clock
> > frequency.
> > - Patch #3 fixes some issues with set_rate passed to firmware, where the
> > parameters are too strict; namely, firmware fails to handle some cases
> > properly if min,tgt,max values for a clock rate are exactly the same
> > value. Yeah, the firmware is quite weird here but nothing much else we
> > can do from kernel side other than this....
> >
> Looks fine to me Tero.
>
> Acked-by: Santosh Shilimkar <[email protected]>
>
>
> Hi Stephen, Mike,
> Can you please pick these fixes via clk tree ?
Sure. I assume this is -next material and not critical fixes.
On 9/10/20 2:29 PM, Stephen Boyd wrote:
> Quoting [email protected] (2020-09-08 10:19:32)
>>
>>
>> On 9/7/20 1:57 AM, Tero Kristo wrote:
>>> Hi Santosh,
>>>
>>> This series contains a few fixes for the TI SCI clock driver.
>>> - Patch #1 is a clear bug fix, where we missed to parse assigned-clock
>>> data properly to detect which clocks are in use on the SoC.
>>> - Patch #2 is a performance improvement patch which avoids some
>>> unnecessary round trips to firmware side while setting clock
>>> frequency.
>>> - Patch #3 fixes some issues with set_rate passed to firmware, where the
>>> parameters are too strict; namely, firmware fails to handle some cases
>>> properly if min,tgt,max values for a clock rate are exactly the same
>>> value. Yeah, the firmware is quite weird here but nothing much else we
>>> can do from kernel side other than this....
>>>
>> Looks fine to me Tero.
>>
>> Acked-by: Santosh Shilimkar <[email protected]>
>>
>>
>> Hi Stephen, Mike,
>> Can you please pick these fixes via clk tree ?
>
> Sure. I assume this is -next material and not critical fixes.
>
Yep.
Quoting Tero Kristo (2020-09-07 01:57:38)
> The DT clock probe loop incorrectly terminates after processing "clocks"
> only, fix this by re-starting the loop when all entries for current
> DT property have been parsed.
>
> Fixes: 8e48b33f9def ("clk: keystone: sci-clk: probe clocks from DT instead of firmware")
> Reported-by: Peter Ujfalusi <[email protected]>
> Signed-off-by: Tero Kristo <[email protected]>
> ---
Applied to clk-next
Quoting Tero Kristo (2020-09-07 01:57:40)
> Currently, we request exact clock rates from the firmware to be set with
> set_rate. Due to some rounding errors and internal functionality of the
> firmware itself, this can fail. Thus, add some slack to the set_rate
> functionality so that we are always guaranteed to pass. The firmware
> always attempts to use frequency as close to the target freq as
> possible despite the slack given here.
>
> Signed-off-by: Tero Kristo <[email protected]>
> ---
Applied to clk-next
Quoting Tero Kristo (2020-09-07 01:57:39)
> Cache results of the latest query rate operation. This optimizes the
> firmware interface a bit, avoiding unnecessary calls to firmware if we
> know the result already; the firmware interface is pretty expensive
> to use for query rate functionality.
>
> Signed-off-by: Tero Kristo <[email protected]>
> ---
Applied to clk-next