[PATCH] clk: Add clk_determine_rate function call

[PATCH] clk: Add clk_determine_rate function call

[Jayesh Choudhary]

Add a function to determine if a particular rate can be set for a clock
with its argument being the clock and the desired rate so that it could
be exposed to other peripherals.
For example, the display controllers typically has to perform multiple
checks for supported display resolutions including those related to
clock rates. The controller has to check this way before it actually
enables the clock and has to do it multiple times (typically for each
mode), and therefore using the clk_set_rate when its not needed, does
not make sense.

The driver does have "__clk_determine_rate()" but this cannot be used
by other subsystems because of the function arguments used.
"clk_hw" is not accessible to other peripherals due to clk and clk_core
structure definition in driver instead of include file, so we cannot use
already exisiting "__clk_determine_rate()" in other drivers.

Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
 drivers/clk/clk.c   | 22 ++++++++++++++++++++++
 include/linux/clk.h | 15 +++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 0565c87656cf..f72d638cc211 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2566,6 +2566,28 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
 	return ret;
 }
 
+/**
+ * clk_determine_rate - determine if the rate for clk can be set or not
+ * @clk: the clk whose rate is being changed
+ * @rate: the new rate for clk
+ *
+ * Check if there is a best match frequency for the desired rate that
+ * can be set for clk.
+ * Returns 0 on success, -EERROR otherwise.
+ */
+int clk_determine_rate(struct clk *clk, unsigned long rate)
+{
+	struct clk_rate_request req;
+
+	if (!clk)
+		return 0;
+
+	clk_hw_init_rate_request(clk->core->hw, &req, rate);
+
+	return __clk_determine_rate(clk->core->hw, &req);
+}
+EXPORT_SYMBOL_GPL(clk_determine_rate);
+
 /**
  * clk_set_rate - specify a new rate for clk
  * @clk: the clk whose rate is being changed
diff --git a/include/linux/clk.h b/include/linux/clk.h
index b607482ca77e..fd81ba738e50 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -829,6 +829,16 @@ void devm_clk_put(struct device *dev, struct clk *clk);
  */
 long clk_round_rate(struct clk *clk, unsigned long rate);
 
+/**
+ * clk_determine_rate - determine if the clock rate for a clock source
+ * can be set or not
+ * @clk: clock source
+ * @rate: desired clock rate in Hz
+ *
+ * Returns success (0) or negative errno.
+ */
+int clk_determine_rate(struct clk *clk, unsigned long rate);
+
 /**
  * clk_set_rate - set the clock rate for a clock source
  * @clk: clock source
@@ -1078,6 +1088,11 @@ static inline unsigned long clk_get_rate(struct clk *clk)
 	return 0;
 }
 
+static inline int clk_determine_rate(struct clk *clk, unsigned long rate)
+{
+	return 0;
+}
+
 static inline int clk_set_rate(struct clk *clk, unsigned long rate)
 {
 	return 0;
-- 
2.34.1

Re: [PATCH] clk: Add clk_determine_rate function call

[Brian Masney]

On Mon, Jun 16, 2025 at 04:05:27PM +0530, Jayesh Choudhary wrote:
> Add a function to determine if a particular rate can be set for a clock
> with its argument being the clock and the desired rate so that it could
> be exposed to other peripherals.
> For example, the display controllers typically has to perform multiple
> checks for supported display resolutions including those related to
> clock rates. The controller has to check this way before it actually
> enables the clock and has to do it multiple times (typically for each
> mode), and therefore using the clk_set_rate when its not needed, does
> not make sense.
> 
> The driver does have "__clk_determine_rate()" but this cannot be used
> by other subsystems because of the function arguments used.
> "clk_hw" is not accessible to other peripherals due to clk and clk_core
> structure definition in driver instead of include file, so we cannot use
> already exisiting "__clk_determine_rate()" in other drivers.
> 
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>

Do you have a link to how this will be used within the DRM subsystem? If
not, could you post a new series to include the user of this new API so
that we can see specifically how it will be used.

Thanks,

Brian

Re: [PATCH] clk: Add clk_determine_rate function call

[Jayesh Choudhary]

Hello Brian,

On 18/06/25 01:33, Brian Masney wrote:
> On Mon, Jun 16, 2025 at 04:05:27PM +0530, Jayesh Choudhary wrote:
>> Add a function to determine if a particular rate can be set for a clock
>> with its argument being the clock and the desired rate so that it could
>> be exposed to other peripherals.
>> For example, the display controllers typically has to perform multiple
>> checks for supported display resolutions including those related to
>> clock rates. The controller has to check this way before it actually
>> enables the clock and has to do it multiple times (typically for each
>> mode), and therefore using the clk_set_rate when its not needed, does
>> not make sense.
>>
>> The driver does have "__clk_determine_rate()" but this cannot be used
>> by other subsystems because of the function arguments used.
>> "clk_hw" is not accessible to other peripherals due to clk and clk_core
>> structure definition in driver instead of include file, so we cannot use
>> already exisiting "__clk_determine_rate()" in other drivers.
>>
>> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> 
> Do you have a link to how this will be used within the DRM subsystem? If
> not, could you post a new series to include the user of this new API so
> that we can see specifically how it will be used.
> 
> Thanks,
> 
> Brian
> 

Based on the conversation in [1], this patch is no longer required.
This can be handled with preexisting functions.

[1]: 
https://lore.kernel.org/all/f2e54128-f7c1-4193-a511-13775559e261@ti.com/

Thanks and Warm Regards,
Jayesh

Re: [PATCH] clk: Add clk_determine_rate function call

[Russell King (Oracle)]

On Mon, Jun 16, 2025 at 04:05:27PM +0530, Jayesh Choudhary wrote:
> Add a function to determine if a particular rate can be set for a clock
> with its argument being the clock and the desired rate so that it could
> be exposed to other peripherals.
> For example, the display controllers typically has to perform multiple
> checks for supported display resolutions including those related to
> clock rates. The controller has to check this way before it actually
> enables the clock and has to do it multiple times (typically for each
> mode), and therefore using the clk_set_rate when its not needed, does
> not make sense.

So what's up with using clk_round_rate(), which returns the clock rate
that one would actually get if one calls clk_set_rate() with the the
value passed into clk_round_rate() ?

Why is clk_round_rate() not sufficient?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] clk: Add clk_determine_rate function call

[Jayesh Choudhary]

Hello Russell,

Thank you for the review.

On 18/06/25 03:58, Russell King (Oracle) wrote:
> On Mon, Jun 16, 2025 at 04:05:27PM +0530, Jayesh Choudhary wrote:
>> Add a function to determine if a particular rate can be set for a clock
>> with its argument being the clock and the desired rate so that it could
>> be exposed to other peripherals.
>> For example, the display controllers typically has to perform multiple
>> checks for supported display resolutions including those related to
>> clock rates. The controller has to check this way before it actually
>> enables the clock and has to do it multiple times (typically for each
>> mode), and therefore using the clk_set_rate when its not needed, does
>> not make sense.
> 
> So what's up with using clk_round_rate(), which returns the clock rate
> that one would actually get if one calls clk_set_rate() with the the
> value passed into clk_round_rate() ?
> 
> Why is clk_round_rate() not sufficient?
> 

I missed this. Sorry about that. My bad.

In the driver I see that clk_core_determine_round_nolock() will call
determine_rate() op if it is defined. And since clk_round_rate() also
calls this, it is sufficient!!!

I have tested this with my display controller TIDSS on TI's SoC and
have posted that patch upstream:
https://lore.kernel.org/all/20250618075804.139844-1-j-choudhary@ti.com/

This patch is no longer required.

Thanks and Warm Regards,
Jayesh