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) <[email protected]> >> --- >> 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
signature.asc
Description: PGP signature
