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

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 22 Feb 2024 13:55:54 +0100
Nuno Sa <nuno.sa@analog.com> wrote:

> 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().
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>

Hmm. Up to you whether you rebase on top of the device_for_each_child_node_scoped()
patch - mostly depends if you give the new version a reviewed by or not!

If they land in the other order I can fix it up whilst applying.
After that series is in place though the number of places this will do

	dev_err_probe(dev, ret, "message\n");
	return ERR_PTR(ret);

Makes me wonder whether
	return ERR_PTR(dev_err_probe(dev, ret, "message\n")); is
too ugly or not?

Maybe we need a dev_err_probe_ret_ptr() but that's also ugly.

One comment inline which is why I didn't just pick this up today and send
a new version of this patch in my series.

Jonathan

> ---
>  drivers/iio/temperature/ltc2983.c | 190 ++++++++++++++++++++------------------
>  1 file changed, 98 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> index 23f2d43fc040..4b096aa3fbd8 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>
> @@ -656,11 +658,12 @@ ltc2983_thermocouple_new(const struct fwnode_handle *child, struct ltc2983_data
>  			 const struct ltc2983_sensor *sensor)
>  {
>  	struct ltc2983_thermocouple *thermo;
> +	struct device *dev = &st->spi->dev;
>  	struct fwnode_handle *ref;
>  	u32 oc_current;
>  	int ret;
>  
> -	thermo = devm_kzalloc(&st->spi->dev, sizeof(*thermo), GFP_KERNEL);
> +	thermo = devm_kzalloc(dev, sizeof(*thermo), GFP_KERNEL);
>  	if (!thermo)
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -687,8 +690,9 @@ ltc2983_thermocouple_new(const struct fwnode_handle *child, struct ltc2983_data
>  					LTC2983_THERMOCOUPLE_OC_CURR(3);
>  			break;
>  		default:
> -			dev_err(&st->spi->dev,
> -				"Invalid open circuit current:%u", oc_current);
> +			dev_err_probe(dev, -EINVAL,
> +				      "Invalid open circuit current:%u",
> +				      oc_current);
Hmm. I'm in two minds on these. 
We don't get the advantage of return dev_error_probe() and I'm not seeing these
hitting EPROBE_DEFER so getting the debug advantages.


>  			return ERR_PTR(-EINVAL);
>  		}