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? -- Tom
signature.asc
Description: PGP signature
