Re: [PATCH v3 7/9] iio: amplifiers: ad8366: add device tree support

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

On Tue, Feb 03, 2026 at 11:24:13AM +0000, Rodrigo Alencar via B4 Relay wrote:

> Add device-tree support by dropping the enum ID in favor of extended
> chip info table, containing:
> - gain_step, indicating with sign the start of the code range;
> - num_channels, to indicate the number IIO channels;
> - pack_code() function to describe how SPI buffer is populated;
> 
> With this, switch cases on the device type were dropped:
> - probe() function adjusted accordingly;
> - Simplified read_raw() and write_raw() callbacks;

...

> +static size_t ad8366_pack_code(struct ad8366_state *st)
> +{
> +	u8 ch_a = bitrev8(st->ch[0]) >> 2;
> +	u8 ch_b = bitrev8(st->ch[1]) >> 2;
> +
> +	put_unaligned_be16((ch_b << 6) | ch_a, &st->data[0]);
> +	return 2;

With this return it will look better as

	return sizeof(__be16);

Alternatively it can be done via array:

	u8 ch[] = { bitrev8(st->ch[0]) >> 2, bitrev8(st->ch[1]) >> 2 };

but I find it uglier than the original approach.

> +}

...

> +	const struct ad8366_info *inf = st->info;
> +	size_t len = 1;
>  
> +	if (inf->pack_code)
> +		len = inf->pack_code(st);
> +	else
> +		st->data[0] = st->ch[0];
>  
> +	return spi_write(st->spi, st->data, len);

Hmm... What about

	const struct ad8366_info *inf = st->info;

	if (inf->pack_code)
		return spi_write(st->spi, st->data, inf->pack_code(st));

	st->data[0] = st->ch[0];
	return spi_write(st->spi, st->data, 1);

?

...

>  	struct ad8366_state *st = iio_priv(indio_dev);
> +	const struct ad8366_info *inf = st->info;
>  	int ret;
> +	int gain = inf->gain_step > 0 ? inf->gain_min : inf->gain_max;

Please, preserve reversed xmas tree order.

...

> -	{"ada4961", ID_ADA4961},
> -	{"adl5240", ID_ADL5240},
> -	{"hmc792a", ID_HMC792},
> -	{"hmc1119", ID_HMC1119},
> +	{"ad8366", (kernel_ulong_t)&ad8366_chip_info},
> +	{"ada4961", (kernel_ulong_t)&ada4961_chip_info},
> +	{"adl5240", (kernel_ulong_t)&adl5240_chip_info},
> +	{"hmc792a", (kernel_ulong_t)&hmc792_chip_info},
> +	{"hmc1119", (kernel_ulong_t)&hmc1119_chip_info},

The conversion to chip_info may be split to a separate patch.

-- 
With Best Regards,
Andy Shevchenko