linux-kernel - Re: [PATCH v2 2/4] iio: temperature: ltc2983: convert to dev_err

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZifW_fUVcdIpfOWO@smile.fi.intel.com>
Date: Tue, 23 Apr 2024 18:42:53 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: nuno.sa@...log.com
Cc: Petr Mladek <pmladek@...e.com>, Chris Down <chris@...isdown.name>,
	John Ogness <john.ogness@...utronix.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jonathan Cameron <jic23@...nel.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Olivier Moysan <olivier.moysan@...s.st.com>,
	Andi Shyti <andi.shyti@...nel.org>,
	Jyoti Bhayana <jbhayana@...gle.com>, linux-kernel@...r.kernel.org,
	linux-iio@...r.kernel.org
Subject: Re: [PATCH v2 2/4] iio: temperature: ltc2983: convert to
 dev_err_probe()

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