On 2/10/2026 11:58 AM, Marek Vasut wrote:
> On 2/9/26 11:53 PM, Tom Rini wrote:
>> On Mon, Feb 09, 2026 at 10:46:03PM +0100, Marek Vasut wrote:
>>> On 2/9/26 9:51 PM, Tom Rini wrote:
>>>> On Mon, Feb 09, 2026 at 09:35:54PM +0100, Marek Vasut wrote:
>>>>> On 2/9/26 9:14 PM, Jonas Karlman wrote:
>>>>>> clk_get_rate() in U-Boot is documented to return clock rate on success,
>>>>>> 0 for invalid clock and -ve error code for other errors. This differ
>>>>>> slightly from Linux where only >= 0 is returned from clk_get_rate().
>>>>>>
>>>>>> Some clock drivers take advantage of this difference and may return -ve
>>>>>> error code for clocks not fully supported in U-Boot.
>>>>>>
>>>>>> Use IS_ERR_VALUE() to check for an error code in addition to current
>>>>>> invalid clock check to fix broken and unpredicted behavior when clock
>>>>>> driver returns a -ve error code for the ref_clk.
>>>>>>
>>>>>> Signed-off-by: Jonas Karlman <[email protected]>
>>>>> I wonder if it would be better to fix the API discrepancy . Could you try
>>>>> that instead ?
>>>>
>>>> Which API discrepancy? clk_get_rate behaves the way it behaves
>>>> intentionally and there's some fairly long and recent threads about that
>>>> resulting from attepmts to change that.
>>> Align clk_get_rate() between U-Boot and Linux kind of discrepancy .
>>
>> That's not in the current list of options. Please look at the various
>> threads in the past 6-8 months (and some reverts about series that tried
>> this). That was my initial opinion too, FWIW.
> Can you please elaborate WHY clk_get_rate() cannot behave the same way
> between U-Boot and Linux ? It is confusing to users and it is a source
> of bugs, this should be fixed.
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.
Other clk_*() related functions also differs from U-Boot and Linux, some
only exists in U-Boot, others behave differently.
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;
+ 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)
Regards,
Jonas
