linux-kernel - Re: [RFC PATCH 1/2] iio: adc: ad7476: Simplify chip type detection

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <e91711f6-c943-402a-8502-52d8ed4c05a9@gmail.com>
Date: Mon, 4 Aug 2025 08:57:37 +0300
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
 Lars-Peter Clausen <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Jonathan Cameron <jic23@...nel.org>, David Lechner <dlechner@...libre.com>,
 Nuno Sá <nuno.sa@...log.com>,
 Andy Shevchenko <andy@...nel.org>, Liam Girdwood <lgirdwood@...il.com>,
 Mark Brown <broonie@...nel.org>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/2] iio: adc: ad7476: Simplify chip type detection

On 01/08/2025 14:09, Jonathan Cameron wrote:
> On Fri, 1 Aug 2025 13:07:13 +0300
> Matti Vaittinen <mazziesaccount@...il.com> wrote:
> 
>> The ad7476 driver uses a table of structures for defining the IC variant
>> specific data. Table is indexed using enum values, which are picked by
>> SPI ID.
>>
>> Having the table and an enum adds extra complexity. It is potentially
>> unsafe if someone alters the enumeration values, or size of the IC data
>> table.
>>
>> Simplify this by dropping the table and using individual structures for
>> the IC specific data, and storing the IC specific structure's address
>> directly in the SPI ID data.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>
>>
>> ---
>> 100% Untested.
>> No functional changes intended
> 
> One tiny thing inline, otherwise looks good to me.  This aligns with
> how we prefer to do things these days.  Tends to end up easier to read
> than the enum array thing and best of all removes any temptation to use
> the enum for anything else.
> 
>>   
>>   static const struct iio_info ad7476_info = {
>> @@ -312,7 +306,7 @@ static int ad7476_probe(struct spi_device *spi)
>>   
>>   	st = iio_priv(indio_dev);
>>   	st->chip_info =
>> -		&ad7476_chip_info_tbl[spi_get_device_id(spi)->driver_data];
>> +		(struct ad7476_chip_info *)spi_get_device_id(spi)->driver_data;
> 
> Switch to spi_get_device_match_data()
> which checks via generic firmware paths first (so DT here) and then the
> old school tables.  Also returns a void * so gets rid of need to cast.

Ah. Right! Thanks!

> Only works with all pointers (or a lot of care) because a value 0 is a
> fail to match.  So kind of enabled by your patch.

Yours,
	-- Matti