On 11/08/2025 17:36, Quentin Schulz wrote:
Hi Andrew,
On 8/11/25 5:24 PM, Andrew Goodbody wrote:
Hi,
I was wondering what people's thoughts were on API return types. In
particular there is this and other examples in include/clk-uclass.h
/**
* get_rate() - Get current clock rate.
* @clk: The clock to query.
*
* This returns the current rate of a clock. If the clock is
disabled, it
* returns the rate at which the clock would run if it was enabled. The
* following pseudo-code should hold::
*
* disable(clk)
* rate = get_rate(clk)
* enable(clk)
* assert(get_rate(clk) == rate)
*
* Return:
* * The rate of @clk
* * -%ENOSYS if this function is not implemented for @clk
* * -%ENOENT if @clk->id is invalid. Prefer using an assert instead,
and doing
* this check in request().
* * Another negative error value (such as %EIO or %ECOMM) if the
rate could
* not be determined due to a bus error.
*/
ulong get_rate(struct clk *clk);
get_rate is declared as returning a ulong but the description says
that it can return negative errors. A simple test of the return value
for being less than 0 will always fail so errors can go undetected.
Casting to a signed type seems less than ideal.
What is the best way to deal with this? Cast to a signed or update the
API to be signed or...?
Note that clk_get_rate() in the kernel has the same function signature
so I would refrain from changing the type otherwise we'll have some
"funny" bugs to handle considering it isn't that uncommon to import
drivers almost as-is from the Linux kernel.
Ah yes. The difference being that the kernel does not seem to attempt to
push an error code through this API, you get a rate or you get 0.
You have get_rate, clk_get_rate and clk_ops.get_rate that should I
believe share the same signature?
This for sure isn't helping you but I also couldn't come up with
something the 5min I thought about it :/
Thanks for looking anyway.
Andrew
Cheers,
Quentin
