On 2/10/26 1:52 PM, Jonas Karlman wrote:

[...]

I think main issue is that U-Boot != Linux, and in U-Boot we can disable
subsystems such as CLK, and the normal U-Boot way to signal that is to
return -ENOSYS from functions, what clk_get_rate() does when disabled.

Sure, but if the function is called the same, people will make the assumption that the function return value handling is the same. So either the function has to be renamed and maybe a wrapper added that makes it linux-compatible, or simply make the function linux-compatible.

Other clk_*() related functions also differs from U-Boot and Linux, some
only exists in U-Boot, others behave differently.

Which is really not good.

Last attempt at changing clk_get_rate() behavior in U-Boot I gave a NAK
on the Rockchip driver side. I am not fully against changing behavior
of clk_get_rate(), but the driver ops should continue to work the same.
There is a difference between an unsupported and an invalid clock.

Something like below, and similar for other clk ops, could possible be
fine if someone really want to align clk_*() funcs with Linux.

Back to this patch, I will send other patches to implement the missing/
unsupported dwc3 ref clocks on Rockchip, this patch is still valid and
matches current and documented behavior for clk_get_rate() in U-Boot
and prevent lockups or crashes when ref clk is missing/unsupported.


diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 748359de287b..4603ecef82b5 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -485,6 +485,7 @@ int clk_request(struct udevice *dev, struct clk *clk)
  ulong clk_get_rate(struct clk *clk)
  {
        const struct clk_ops *ops;
+       ulong rate;
debug("%s(clk=%p)\n", __func__, clk);
        if (!clk_valid(clk))
@@ -492,9 +493,17 @@ ulong clk_get_rate(struct clk *clk)
        ops = clk_dev_ops(clk->dev);
if (!ops->get_rate)
-               return -ENOSYS;
+               rate = -ENOSYS;

This would have to be PTR_ERR() , right ?

+       else
+               rate = ops->get_rate(clk);
+
+       if (IS_ERR_VALUE(rate)) {
+               dm_warn("clk_get_rate(clk=%p, id=%lu) failed: %ld\n",
+                       clk, clk->id, (long)rate);
+               return 0;
+       }
- return ops->get_rate(clk);
+       return rate;
  }
struct clk *clk_get_parent(struct clk *clk)
[...]

Reply via email to