[PATCH] clk: clk-uclass: Ignore -ENOSYS in clk_set_default_rates()

[PATCH] clk: clk-uclass: Ignore -ENOSYS in clk_set_default_rates()

[Markus Schneider-Pargmann (TI)]

clk_set_default_parents() already ignores -ENOSYS from clk_set_parent().
Do the same for clk_set_default_rates() and rely on the earlier setup
clock rates.

On am437x the cpsw switch has assigned-clock-rates for the RGMII clock.
The divider delegates to a multiplier of the DPLL that has no set_rate
and no delegation, so it returns -ENOSYS.

Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
---
 drivers/clk/clk-uclass.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index c33f415917e9..0069ce9a8875 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -359,6 +359,15 @@ static int clk_set_default_rates(struct udevice *dev,
 
 		ret = clk_set_rate(c, rates[index]);
 
+		/*
+		 * Not all drivers/clocks support setting the rate (as of now).
+		 * Ignore that and keep the rate set by an earlier boot stage.
+		 */
+		if (ret == -ENOSYS) {
+			ret = 0;
+			continue;
+		}
+
 		if (IS_ERR_VALUE(ret)) {
 			dev_warn(dev,
 				 "failed to set rate on clock index %d (%ld) (error = %d)\n",

---
base-commit: d3e8597e46b635ec556a057bc42f0b0859654bdf
change-id: 20260827-v2026-10-next-topic-am45-ofupstream-clk-default-rates-enosys-1359deb9d80b

Best regards,
--  
Markus Schneider-Pargmann (TI) <msp@baylibre.com>

Re: [PATCH] clk: clk-uclass: Ignore -ENOSYS in clk_set_default_rates()

[Heinrich Schuchardt]

On 8/27/26 14:36, Markus Schneider-Pargmann (TI) wrote:
> clk_set_default_parents() already ignores -ENOSYS from clk_set_parent().
> Do the same for clk_set_default_rates() and rely on the earlier setup
> clock rates.
> 
> On am437x the cpsw switch has assigned-clock-rates for the RGMII clock.
> The divider delegates to a multiplier of the DPLL that has no set_rate
> and no delegation, so it returns -ENOSYS.
> 
> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
> ---
>   drivers/clk/clk-uclass.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index c33f415917e9..0069ce9a8875 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -359,6 +359,15 @@ static int clk_set_default_rates(struct udevice *dev,
>   
>   		ret = clk_set_rate(c, rates[index]);
>   
> +		/*
> +		 * Not all drivers/clocks support setting the rate (as of now).
> +		 * Ignore that and keep the rate set by an earlier boot stage.
> +		 */
> +		if (ret == -ENOSYS) {

In include struct clk_ops is incompletely documented.

It is undefined which error code might indicate what.
This might be handled differently in different drivers.

E.g. zynq_clk_set_rate() returns ENXIO if there is no set rate 
functionality.

Please, complete the description of struct clk_ops, check what all clock 
drivers do, and harmonize the return codes before adding this change.

The struct clk_ops documentation should explicitly mention how error 
codes are handled by the uclass.

Best regards

Heinrich

> +			ret = 0;
> +			continue;
> +		}
> +
>   		if (IS_ERR_VALUE(ret)) {
>   			dev_warn(dev,
>   				 "failed to set rate on clock index %d (%ld) (error = %d)\n",
> 
> ---
> base-commit: d3e8597e46b635ec556a057bc42f0b0859654bdf
> change-id: 20260827-v2026-10-next-topic-am45-ofupstream-clk-default-rates-enosys-1359deb9d80b
> 
> Best regards,
> --
> Markus Schneider-Pargmann (TI) <msp@baylibre.com>
> 

Re: [PATCH] clk: clk-uclass: Ignore -ENOSYS in clk_set_default_rates()

[Markus Schneider-Pargmann]

[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]

Hi Heinrich,

On Thu Aug 27, 2026 at 2:49 PM CEST, Heinrich Schuchardt wrote:
> On 8/27/26 14:36, Markus Schneider-Pargmann (TI) wrote:
>> clk_set_default_parents() already ignores -ENOSYS from clk_set_parent().
>> Do the same for clk_set_default_rates() and rely on the earlier setup
>> clock rates.
>> 
>> On am437x the cpsw switch has assigned-clock-rates for the RGMII clock.
>> The divider delegates to a multiplier of the DPLL that has no set_rate
>> and no delegation, so it returns -ENOSYS.
>> 
>> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
>> ---
>>   drivers/clk/clk-uclass.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>> 
>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>> index c33f415917e9..0069ce9a8875 100644
>> --- a/drivers/clk/clk-uclass.c
>> +++ b/drivers/clk/clk-uclass.c
>> @@ -359,6 +359,15 @@ static int clk_set_default_rates(struct udevice *dev,
>>   
>>   		ret = clk_set_rate(c, rates[index]);
>>   
>> +		/*
>> +		 * Not all drivers/clocks support setting the rate (as of now).
>> +		 * Ignore that and keep the rate set by an earlier boot stage.
>> +		 */
>> +		if (ret == -ENOSYS) {
>
> In include struct clk_ops is incompletely documented.
>
> It is undefined which error code might indicate what.
> This might be handled differently in different drivers.

Thanks for your feedback! This patch is more about handling what
clk_set_rate() returns than what the clk drivers return in set_rate().
clk_set_rate() itself has a check if CLK_SET_RATE_PARENT is unset and
returns -ENOSYS otherwise if it couldn't find a suitable set_rate:

	/* Try to find parents which can set rate */
	while (!ops->set_rate) {
		...
		if (!(clkp->flags & CLK_SET_RATE_PARENT))
			return -ENOSYS;
		...
	}

>
> E.g. zynq_clk_set_rate() returns ENXIO if there is no set rate 
> functionality.
>
> Please, complete the description of struct clk_ops, check what all clock 
> drivers do, and harmonize the return codes before adding this change.
>
> The struct clk_ops documentation should explicitly mention how error 
> codes are handled by the uclass.

set_rate is also already documented in clk-uclass.h:

  /**
   * set_rate() - Set current clock rate.
   * @clk:	The clock to manipulate.
   * @rate:	New clock rate in Hz.
   ...
   * Return:
   * * The new rate on success
   * * -%ENOSYS if this function is not implemented for @clk
   * * -%ENOENT if @clk->id is invalid. Prefer using an assert instead, and doing
   *   this check in request().
   * * -%EINVAL if @rate is not valid for @clk.
   * * Another negative error value (such as %EIO or %ECOMM) if the rate could
   *   not be set due to a bus error.
   */
  ulong set_rate(struct clk *clk, ulong rate);

Best
Markus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]

Re: [PATCH] clk: clk-uclass: Ignore -ENOSYS in clk_set_default_rates()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3230 bytes --]

On Thu, Aug 27, 2026 at 04:43:06PM +0200, Markus Schneider-Pargmann wrote:
> Hi Heinrich,
> 
> On Thu Aug 27, 2026 at 2:49 PM CEST, Heinrich Schuchardt wrote:
> > On 8/27/26 14:36, Markus Schneider-Pargmann (TI) wrote:
> >> clk_set_default_parents() already ignores -ENOSYS from clk_set_parent().
> >> Do the same for clk_set_default_rates() and rely on the earlier setup
> >> clock rates.
> >> 
> >> On am437x the cpsw switch has assigned-clock-rates for the RGMII clock.
> >> The divider delegates to a multiplier of the DPLL that has no set_rate
> >> and no delegation, so it returns -ENOSYS.
> >> 
> >> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
> >> ---
> >>   drivers/clk/clk-uclass.c | 9 +++++++++
> >>   1 file changed, 9 insertions(+)
> >> 
> >> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> >> index c33f415917e9..0069ce9a8875 100644
> >> --- a/drivers/clk/clk-uclass.c
> >> +++ b/drivers/clk/clk-uclass.c
> >> @@ -359,6 +359,15 @@ static int clk_set_default_rates(struct udevice *dev,
> >>   
> >>   		ret = clk_set_rate(c, rates[index]);
> >>   
> >> +		/*
> >> +		 * Not all drivers/clocks support setting the rate (as of now).
> >> +		 * Ignore that and keep the rate set by an earlier boot stage.
> >> +		 */
> >> +		if (ret == -ENOSYS) {
> >
> > In include struct clk_ops is incompletely documented.
> >
> > It is undefined which error code might indicate what.
> > This might be handled differently in different drivers.
> 
> Thanks for your feedback! This patch is more about handling what
> clk_set_rate() returns than what the clk drivers return in set_rate().
> clk_set_rate() itself has a check if CLK_SET_RATE_PARENT is unset and
> returns -ENOSYS otherwise if it couldn't find a suitable set_rate:
> 
> 	/* Try to find parents which can set rate */
> 	while (!ops->set_rate) {
> 		...
> 		if (!(clkp->flags & CLK_SET_RATE_PARENT))
> 			return -ENOSYS;
> 		...
> 	}
> 
> >
> > E.g. zynq_clk_set_rate() returns ENXIO if there is no set rate 
> > functionality.
> >
> > Please, complete the description of struct clk_ops, check what all clock 
> > drivers do, and harmonize the return codes before adding this change.
> >
> > The struct clk_ops documentation should explicitly mention how error 
> > codes are handled by the uclass.
> 
> set_rate is also already documented in clk-uclass.h:
> 
>   /**
>    * set_rate() - Set current clock rate.
>    * @clk:	The clock to manipulate.
>    * @rate:	New clock rate in Hz.
>    ...
>    * Return:
>    * * The new rate on success
>    * * -%ENOSYS if this function is not implemented for @clk
>    * * -%ENOENT if @clk->id is invalid. Prefer using an assert instead, and doing
>    *   this check in request().
>    * * -%EINVAL if @rate is not valid for @clk.
>    * * Another negative error value (such as %EIO or %ECOMM) if the rate could
>    *   not be set due to a bus error.
>    */
>   ulong set_rate(struct clk *clk, ulong rate);

Does this answer your concerns / questions, Heinrich? I believe this
patch is required for another series, which is in turn required to move
the am43xx platforms to OF_UPSTREAM. Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]