linux-kernel - Re: [PATCH v2 4/6] 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 for Android: free password hash cracker in your pocket

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

Message-ID: <aXksSjsyNn6if3eQ@smile.fi.intel.com>
Date: Tue, 27 Jan 2026 23:21:14 +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 v2 4/6] iio: amplifiers: ad8366: add device tree support

On Mon, Jan 26, 2026 at 01:51:05PM +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;

> - mutex_lock()/mutex_unlock() replaced for guard(mutex)() to allow
>   moving to early returns;

Shouldn't this be in a separate change? I dunno. Let Jonathan to decide.

...

> +static size_t ad8366_pack_code(struct ad8366_state *st)
> +{
> +	u8 ch_a = bitrev8(st->ch[0] & 0x3F);
> +	u8 ch_b = bitrev8(st->ch[1] & 0x3F);

GENMASK() in both cases? But I don't see why ch_a needs this at all,
isn't the 2 LSBs are not used anyway?

Also missed header inclusion for this? And also perhaps sorting headers first
to see what's there and what needs to be updated (ideally another patch to move
to IWYU principle).

> +	st->data[0] = ch_b >> 4;
> +	st->data[1] = (ch_b << 4) | (ch_a >> 2);
> +	return 2;
> +}

...

>  static int ad8366_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val,
>  			   int *val2,
> -			   long m)
> +			   long mask)

Seems like unrelated change.

...

> -	/* Values in dB */

Do you think this comment is useless? To me looks like a stray change.

>  	if (val < 0)
>  		gain = (val * 1000) - (val2 / 1000);

...

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

> +	if (!st->info)
> +		return dev_err_probe(dev, -EINVAL, "Invalid device info\n");

Only useful for the developer, dead code in production.

-- 
With Best Regards,
Andy Shevchenko