On Thu, 22 Feb 2024 13:55:54 +0100
Nuno Sa <nuno.sa@xxxxxxxxxx> 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@xxxxxxxxxx>
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);
> }
