Re: [PATCH v3 1/3] iio: temperature: ltc2983: convert to dev_err_probe()

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 01 Mar 2024 18:14:50 +0100
Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> From: Nuno Sa <nuno.sa@analog.com>
> 
> Use dev_err_probe() in the probe() path. While at it, made some simple
> improvements:
>  * Declare a struct device *dev helper. This also makes the style more
>    consistent (some places the helper was used and not in other places);
>  * Explicitly included the err.h and errno.h headers;
>  * Removed an useless else if();
>  * Removed some unnecessary line breaks.
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>

I'd rather see a proposal for dev_errp_probe() than a local solution.
We can fallback to local solution if it doesn't meet with approval!

Not that many uses in IIO so far, but seems some potential elsewhere.
There are already a few return ERR_PTR(dev_err_probe()) cases in tree
for low hanging fruit + looks like scmi_iio could benefit as another case
in IIO.

> ---
>  drivers/iio/temperature/ltc2983.c | 262 ++++++++++++++++++--------------------
>  1 file changed, 122 insertions(+), 140 deletions(-)
> 
> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> index 3c4524d57b8e..592887097117 100644
> --- a/drivers/iio/temperature/ltc2983.c
> +++ b/drivers/iio/temperature/ltc2983.c
> @@ -8,6 +8,8 @@
>  #include <linux/bitfield.h>
>  #include <linux/completion.h>
>  #include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
>  #include <linux/kernel.h>
>  #include <linux/iio/iio.h>
>  #include <linux/interrupt.h>
> @@ -206,6 +208,13 @@ enum {
>  #define to_temp(_sensor) \
>  		container_of(_sensor, struct ltc2983_temp, sensor)
>  
> +/* Helper macro for returning error pointers in probe() paths. */
> +#define ltc2983_errp_probe(dev, ret, fmt, ...)	({			\
> +	int ___ret = dev_err_probe(dev, ret, fmt, ##__VA_ARGS__);	\
> +									\
> +	ERR_PTR(___ret);						\

Given it's all wrapped up in a macro, I'd avoid the need for the ___ to try
and prevent shadowing an existing variable by just going with
	ERR_PTR(dev_err_probe(dev, ret, fmt, ##__VA_ARGS__))
and no ({ complexities.

If you really need to use a local variable, just through the hoops to make it unique
like the cleanup.h macros do.

> +})
> +