Seeking advice on API return type inconsistency

[Andrew Goodbody]

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