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

[Marcelo Schmitt <marcelo.schmitt1@gmail.com>]

On 06/03, Andy Shevchenko wrote:
> On Mon, Jun 02, 2025 at 01:54:25PM -0300, Marcelo Schmitt wrote:
> 
> ...
> 
> > > > +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.
> > > > +	 */
Does the reason given in the comment justify the use of static_assert?

> > > > +	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.
> 
> The question was "why do you need the static_assert() now?"

To ensure that the comparison function gets updated if struct ad4170_setup is
ever modified? This intends to be similar to what was implemented in ad7124
driver as the chips have similar channel configuration mechanisms. We also
have ad7173 and ad4130 using static_assert for analogous purpose. There was
also a comment about static_assert above.

Best regards,
Marcelo

> 
> > [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;
> > > > +}
> 
> -- 
> With Best Regards,
> Andy Shevchenko