On 12/08/2025 15:33, Tom Rini wrote:
On Tue, Aug 12, 2025 at 10:17:47AM +0100, Andrew Goodbody wrote:
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.
How is the error code pushed? Or is it up to the caller to decide that 0
means on a case by case basis?
In the Linux kernel almost no code checks the return of clk_get_rate at
all. Some code will check the value is sensible and 0 is obviously not
sensible. But pretty much the call to clk_get_rate is not expected to fail.
Andrew
