Re: [PATCH v2 2/4] iio: temperature: ltc2983: convert to dev_err_probe()

Andy Shevchenko
2024-04-23T15:43:00Z

On Tue, Apr 23, 2024 at 05:20:31PM +0200, Nuno Sa via B4 Relay 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.

...

> /* Check space on the table. */
> if (st->custom_table_size + new_custom->size >
> - (LTC2983_CUST_SENS_TBL_END_REG -
> - LTC2983_CUST_SENS_TBL_START_REG) + 1) {

> + (LTC2983_CUST_SENS_TBL_END_REG - LTC2983_CUST_SENS_TBL_START_REG) + 1)

Semi-unrelated change?

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for differential thermocouple",

While at it, add missing \n.

> + sensor->chan);

...

> + return dev_err_cast_probe(dev, ref,
> + "Property adi,rsense-handle missing or invalid");

Ditto.

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid number of wires:%u\n",
> + n_wires);

Can be compressed in terms of LoCs?

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Rotation not allowed for 2/3 Wire RTDs");

\n

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid rsense chann:%d to use in kelvin rsense",
> + rtd->r_sense_chan);

Ditto.

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for the rtd config",

Ditto.

> + sensor->chan);

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for RTD",

Ditto.

> + sensor->chan);

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid value for excitation current(%u)",

Ditto.

> + excitation_current);

...

> + if (IS_ERR(ref))
> + return dev_err_cast_probe(dev, ref,
> + "Property adi,rsense-handle missing or invalid");

Ditto.

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for differential thermistor",
> + sensor->chan);

Ditto.

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid value for excitation current(%u)",
> + excitation_current);

Ditto.

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for differential thermistor",
> + sensor->chan);

Ditto.

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid value for excitation current(%u)",
> + excitation_current);

Ditto.

...

> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for r_sense",
> + sensor->chan);

Ditto.

...

> + if (!st->num_channels)
> + return dev_err_probe(dev, -EINVAL,
> + "At least one channel must be given!");

Ditto.

...

> + return dev_err_probe(dev, -EINVAL,
> + "EEPROM command failed: 0x%02X\n", val);

One line?

...

> + if (IS_ERR(st->regmap))
> + return dev_err_probe(dev, PTR_ERR(st->regmap),
> + "Failed to initialize regmap\n");

Wondering about Andi's proposal in conjunction with %pe to be in use

return dev_???(dev, st->regmap, "Failed to initialize regmap\n");

where it returns an int and uses const void * as an error pointer for %pe.

> - st->iio_chan = devm_kzalloc(&spi->dev,
> + st->iio_chan = devm_kzalloc(dev,
> st->iio_channels * sizeof(*st->iio_chan),
> GFP_KERNEL);

Separate change to devm_kzalloc() before this patch?
In that patch you may also introduce a temporary struct device *dev.

--
With Best Regards,
Andy Shevchenko