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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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