On 12.08.25 16: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?
Using macro IS_ERR_VALUE() in the caller should do the needed.
See include/linux/err.h:21:
#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
Best regards
Heinrich
