Hi Michael,
-----Original Message----- From: Michael Nazzareno Trimarchi michael@amarulasolutions.com Sent: Thursday, July 25, 2024 6:21 PM To: Z.Q. Hou zhiqiang.hou@nxp.com Cc: Simon Glass sjg@chromium.org; u-boot@lists.denx.de; trini@konsulko.com; lukma@denx.de; seanga2@gmail.com Subject: Re: [PATCH] clk: fix ccf_clk_get_rate
Hi
On Thu, Jul 25, 2024 at 11:58 AM Z.Q. Hou zhiqiang.hou@nxp.com wrote:
Hi Michael,
-----Original Message----- From: Michael Nazzareno Trimarchi michael@amarulasolutions.com Sent: Thursday, July 25, 2024 5:26 PM To: Z.Q. Hou zhiqiang.hou@nxp.com; Simon Glass sjg@chromium.org Cc: u-boot@lists.denx.de; trini@konsulko.com; lukma@denx.de; seanga2@gmail.com Subject: Re: [PATCH] clk: fix ccf_clk_get_rate
Hi
On Thu, Jul 25, 2024 at 10:53 AM Z.Q. Hou zhiqiang.hou@nxp.com
wrote:
Hi Michael,
-----Original Message----- From: Michael Nazzareno Trimarchi
Sent: Thursday, July 25, 2024 3:32 PM To: Z.Q. Hou zhiqiang.hou@nxp.com Cc: u-boot@lists.denx.de; trini@konsulko.com; lukma@denx.de; seanga2@gmail.com Subject: Re: [PATCH] clk: fix ccf_clk_get_rate
Hi
On Thu, Jul 25, 2024 at 9:16 AM Zhiqiang Hou Zhiqiang.Hou@nxp.com wrote:
From: Hou Zhiqiang Zhiqiang.Hou@nxp.com
As the type of return value is 'ulong', when clk_get_by_id() failed, it should return 0 to indicate the get_rate operation doesn't
succeed.
I understand your point here but the clk_get_rate that can call the ccf clk_get_rate can already return -ENOSYS.
will also fix it.
Is there any usage of the error set on the latest bit of the clock? We need to be sure that this is correct use accross the uboot. The clk-uclass define the error that can return
The problem is this function's return value is 'ulong', if use the error set, the
caller will treat it as a good value instead of a error number. There are several APIs have the same problem.
2 methods to fix it:
- keep the API's prototype, and use '0' to indicate error condition, but a real
'0' return value is also considered as error.
- Change the return type to like 'long long int', and use a negative error
number to indicate error condition. Need to update the check of return value for all the callers.
Any suggestion?
unit test suggest that error condition are evaluated so we have 32 bit on arm of unsigned long so we can not map all the positive clock
rate = clk_set_rate(clk, 80000000); ut_asserteq(rate, -ENOSYS);
Now, can we have a clock greater then 2Ghz if yes this not work always on 32 bit, then the macro IS_ERR_VALUE should help on clk return
You mean keep it as it's and let the callers evaluate the error condition using IS_ERR_VALUE, right? Current implement also make 'return 0' as error condition, and many callers treat all non-zero return value as good, these need to fix.
ulong clk_get_rate(struct clk *clk) { ... if (!clk_valid(clk)) return 0; ... }
Contrast to reserve 4K errnos, I prefer to 'return 0' as error condition, 0 Hz is almost never a correct value, and all the positive value can be used especially on 32-bit platform, does the caller really cares about the exact error number?
Thanks, Zhiqiang