[PATCH 08/24] clk: rockchip: Remove negative error returns from clk_get_rate

[Andrew Goodbody]

On 19/10/2025 21:28, Jonas Karlman wrote:
> Hi Andrew,
> 
> On 10/15/2025 4:32 PM, Andrew Goodbody wrote:
>> clk_get_rate() returns a ulong so do not attempt to pass negative error
>> codes through it.
> 
> On Rockchip the clk drivers only implement support for a subset of all
> clocks supported by the hardware, mostly to save space for TPL/SPL and
> not having to implement logic for clocks not really needed by U-Boot.
> 
> Because support for all clocks is not implemented the existing working
> error handling actually help us catch clock issues on the Rockchip
> platform. E.g. when a clk is referenced by assigned-clock-rates in DT we
> get an error and can fix the issue, or decide if we can safely ignore
> setting the clk rate.
> 
> I do not understand why you need to change the way the clk ops work,
> just check with IS_ERR_VALUE() inside the clk-uclass or similar if you
> really must remove working error handling from the callers.

OK, so here we have a question and it is one that I wish had come up in 
the original discussion. IS_ERR_VALUE() and other related macros are 
from include/linux/err.h and are introduced with this comment

/*
  * Kernel pointers have redundant information, so we can use a
  * scheme where we can return either an error code or a dentry
  * pointer with the same return value.
  *
  * This should be a per-architecture thing, to allow different
  * error and pointer decisions.
  */

So they are clearly intended to be unambiguous and used to add 
information about errors into kernel pointers. It also suggests that 
there may be other implementations of these macros for other architectures.

Much of the error checking of the return value from clk_get_rate() is 
done with IS_ERR_VALUE() despite the return value not being a kernel 
pointer and with this default implementation it does happen to work just 
as long as your clock rate does not collide with an error. OK collisions 
are very unlikely but it does still feel like an abuse of the macro to me.

Is there any official guidance on this use of IS_ERR_VALUE()?

Andrew

> The current working clk error handling has really helped me time and
> time again on Rockchip platform during soc and board bring-up.
> 
> This is a NAK for me without first having a proper replacement for all
> existing working error handling that is just being left ignored after
> this patch/series.
> 
> Regards,
> Jonas
> 
>>
>> Signed-off-by: Andrew Goodbody <andrew.goodbody at linaro.org>
>> ---
>>   drivers/clk/rockchip/clk_px30.c   | 24 +++++++--------
>>   drivers/clk/rockchip/clk_rk3036.c |  2 +-
>>   drivers/clk/rockchip/clk_rk3066.c |  8 ++---
>>   drivers/clk/rockchip/clk_rk3128.c |  6 ++--
>>   drivers/clk/rockchip/clk_rk3188.c |  6 ++--
>>   drivers/clk/rockchip/clk_rk322x.c |  4 +--
>>   drivers/clk/rockchip/clk_rk3288.c |  6 ++--
>>   drivers/clk/rockchip/clk_rk3308.c | 26 ++++++++--------
>>   drivers/clk/rockchip/clk_rk3328.c |  6 ++--
>>   drivers/clk/rockchip/clk_rk3368.c |  8 ++---
>>   drivers/clk/rockchip/clk_rk3399.c | 12 ++++----
>>   drivers/clk/rockchip/clk_rk3528.c | 20 ++++++-------
>>   drivers/clk/rockchip/clk_rk3568.c | 62 +++++++++++++++++++--------------------
>>   drivers/clk/rockchip/clk_rk3576.c | 36 +++++++++++------------
>>   drivers/clk/rockchip/clk_rk3588.c | 32 ++++++++++----------
>>   drivers/clk/rockchip/clk_rv1108.c |  4 +--
>>   drivers/clk/rockchip/clk_rv1126.c | 52 ++++++++++++++++----------------
>>   17 files changed, 157 insertions(+), 157 deletions(-)