linux-kernel - Re: [PATCH v4 02/11] iio: adc: Add basic support for AD4170

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: <aD3XQfUfxIiz62ZU@debian-BULLSEYE-live-builder-AMD64>
Date: Mon, 2 Jun 2025 13:54:25 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: Andy Shevchenko <andy@...nel.org>
Cc: Marcelo Schmitt <marcelo.schmitt@...log.com>, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-gpio@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Ana-Maria Cusco <ana-maria.cusco@...log.com>, jic23@...nel.org,
	lars@...afoo.de, Michael.Hennerich@...log.com,
	dlechner@...libre.com, nuno.sa@...log.com, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org, linus.walleij@...aro.org,
	brgl@...ev.pl
Subject: Re: [PATCH v4 02/11] iio: adc: Add basic support for AD4170

...
> > +static bool ad4170_setup_eq(struct ad4170_setup *a, struct ad4170_setup *b)
> > +{
> > +	/*
> > +	 * The use of static_assert() here is to make sure that the comparison
> > +	 * is adapted whenever struct ad4170_setup is changed.
> > +	 */
> > +	static_assert(sizeof(*a) ==
> > +		      sizeof(struct {
> > +				     u16 misc;
> > +				     u16 afe;
> > +				     u16 filter;
> > +				     u16 filter_fs;
> > +				     u32 offset;
> > +				     u32 gain;
> > +			     }));
> 
> I think it doesn't make much sense unless one uses memcpy().

memcpy() is used to update the setups after reg write succeeds.
Also, previously, memcmp() was used to compare setups.
Since struct ad4170_setup has only unsigned integers (no floating point fields
like ad7124 had [1]), ad4170 works properly when comparing setups with memcmp().
Though, it was asked to do explicit field matching on previous reviews [2] so
that's how it had been since then. Well, both ways work for ad4170. We can
compare setup with memcmp(), or do the comparison field by field. I don't mind
changing it again if requested. I guess we only need to reach an agreement about
what to go with.

[1]: https://lore.kernel.org/all/20250303114659.1672695-13-u.kleine-koenig@baylibre.com/
[2]: https://lore.kernel.org/linux-iio/20250504192117.5e19f44b@jic23-huawei/

> 
> > +	if (a->misc != b->misc ||
> > +	    a->afe != b->afe ||
> > +	    a->filter != b->filter ||
> > +	    a->filter_fs != b->filter_fs ||
> > +	    a->offset != b->offset ||
> > +	    a->gain != b->gain)
> > +		return false;
> > +
> > +	return true;
> > +}
> 
> ...
> 

Thanks,
Marcelo