linux-kernel - Re: [PATCH v3 7/9] iio: amplifiers: ad8366: add device tree 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: <aYKp4UrEhLInNNZZ@smile.fi.intel.com>
Date: Wed, 4 Feb 2026 04:07:29 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: rodrigo.alencar@...log.com
Cc: linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org,
	Michael Hennerich <Michael.Hennerich@...log.com>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Jonathan Cameron <jic23@...nel.org>,
	David Lechner <dlechner@...libre.com>,
	Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>
Subject: Re: [PATCH v3 7/9] iio: amplifiers: ad8366: add device tree support

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