[PATCH 22/34] iio: inkern: only return error codes in iio_channel_get_*() APIs

[Andy Shevchenko]

On Fri, Jun 10, 2022 at 10:48 AM Nuno Sá <nuno.sa at analog.com> wrote:
>
> APIs like of_iio_channel_get_by_name() and of_iio_channel_get_all() were
> returning a mix of NULL and error pointers being NULL the way to
> "notify" that we should do a "system" lookup for channels. This make
> it very confusing and prone to errors as commit dbbccf7c20bf
> ("iio: inkern: fix return value in devm_of_iio_channel_get_by_name()")
> proves. On top of this, patterns like 'if (channel != NULL) return channel'
> were being used where channel could actually be an error code which
> makes the code hard to read.

...

>                 np = np->parent;
>                 if (np && !of_get_property(np, "io-channel-ranges", NULL))
> -                       return NULL;
> +                       return chan;

Shouldn't it return a dedicated error code and not some arbitrary one?
It may be I missed something and chan has a correct error code in this
case...

...

> +       if (nummaps == 0)       /* return -ENODEV to search map table */

Comment is superfluous, the next line is self-explaining.

> +               return ERR_PTR(-ENODEV);

...

> -               if (channel != NULL)
> +               if (!IS_ERR(channel) || PTR_ERR(channel) == -EPROBE_DEFER)

Btw, in the GPIO library we have a macro or inliner (don't remember)
that represents such a conditional.
Perhaps make it (if it's a macro) global, or introduce an inline in IIO?

Okay, it's here:
https://elixir.bootlin.com/linux/v5.19-rc1/source/drivers/gpio/gpiolib.h#L179

It's similar, but not the same, so just play with an idea to introduce
something in this file, maybe it's worth doing this, maybe not.

-- 
With Best Regards,
Andy Shevchenko