Hi Michael,
> -----Original Message-----
> From: Michael Nazzareno Trimarchi <mich...@amarulasolutions.com>
> Sent: Thursday, July 25, 2024 6:21 PM
> To: Z.Q. Hou <zhiqiang....@nxp.com>
> Cc: Simon Glass <s...@chromium.org>; u-boot@lists.denx.de;
> tr...@konsulko.com; lu...@denx.de; sean...@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....@nxp.com> wrote:
> >
> > Hi Michael,
> >
> > > -----Original Message-----
> > > From: Michael Nazzareno Trimarchi <mich...@amarulasolutions.com>
> > > Sent: Thursday, July 25, 2024 5:26 PM
> > > To: Z.Q. Hou <zhiqiang....@nxp.com>; Simon Glass <s...@chromium.org>
> > > Cc: u-boot@lists.denx.de; tr...@konsulko.com; lu...@denx.de;
> > > sean...@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....@nxp.com>
> wrote:
> > > >
> > > > Hi Michael,
> > > >
> > > > > -----Original Message-----
> > > > > From: Michael Nazzareno Trimarchi
> <mich...@amarulasolutions.com>
> > > > > Sent: Thursday, July 25, 2024 3:32 PM
> > > > > To: Z.Q. Hou <zhiqiang....@nxp.com>
> > > > > Cc: u-boot@lists.denx.de; tr...@konsulko.com; lu...@denx.de;
> > > > > sean...@gmail.com
> > > > > Subject: Re: [PATCH] clk: fix ccf_clk_get_rate
> > > > >
> > > > > Hi
> > > > >
> > > > > On Thu, Jul 25, 2024 at 9:16 AM Zhiqiang Hou
> > > > > <zhiqiang....@nxp.com>
> > > > > wrote:
> > > > > >
> > > > > > From: Hou Zhiqiang <zhiqiang....@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:
> > 1. keep the API's prototype, and use '0' to indicate error condition, but a
> > real
> '0' return value is also considered as error.
> > 2. 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
