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