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

[Andy Shevchenko]

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.
> > > +	 */
> > > +	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?"

> [1]: https://lore.kernel.org/all/20250303114659.1672695-13-u.kleine-koenig@xxxxxxxxxxxx/
> [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