linux-kernel - Re: [PATCH v3 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aLsVJraM57okkP1o@smile.fi.intel.com>
Date: Fri, 5 Sep 2025 19:51:50 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Jonathan Santos <Jonathan.Santos@...log.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, lars@...afoo.de, jic23@...nel.org,
	dlechner@...libre.com, nuno.sa@...log.com, andy@...nel.org,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	marcelo.schmitt1@...il.com, jonath4nns@...il.com
Subject: Re: [PATCH v3 2/4] iio: adc: ad7768-1: introduce chip info for
 future multidevice support

On Fri, Sep 05, 2025 at 06:49:21AM -0300, Jonathan Santos wrote:
> Add Chip info struct in SPI device to store channel information for
> each supported part.

...

> +#define AD7768_CHAN(_idx, _msk_avail) { \

I consider slightly better to read

#define AD7768_CHAN(_idx, _msk_avail) \
{ \


> +	.type = IIO_VOLTAGE, \
> +	.info_mask_separate_available = _msk_avail, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \

> +			BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \

In the original code below indentation was not broken.


> +	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.ext_info = ad7768_ext_info, \
> +	.indexed = 1, \
> +	.channel = _idx, \
> +	.scan_index = _idx, \
> +	.has_ext_scan_type = 1, \
> +	.ext_scan_type = ad7768_scan_type, \
> +	.num_ext_scan_type = ARRAY_SIZE(ad7768_scan_type), \
> +}

...

> +	st->chip = spi_get_device_match_data(spi);

> +	if (!st->chip)
> +		return dev_err_probe(&spi->dev, -ENODEV,
> +				     "Could not find chip info data\n");

Might be useful in some cases, but I don't see its value. Just always provide
a chip_info and no need to care about this. Esp. this just shows that it's
mandatory, but if absent, it will crash on the following line. Since it's about
probe, one will notice it immediately, otherwise it will mean a submission of
the code that has never been tested.

TL;DR: just drop this check.

> +	indio_dev->channels = st->chip->channel_spec;


-- 
With Best Regards,
Andy Shevchenko