On Wed, 26 Nov 2025 12:40:00 +0100
Jorge Marques <gastmaier@gmail.com> wrote:

> On Mon, Nov 24, 2025 at 12:20:57PM +0200, Andy Shevchenko wrote:
> > On Mon, Nov 24, 2025 at 10:18:02AM +0100, Jorge Marques wrote:  
> > > The AD4060/AD4062 are versatile, 16-bit/12-bit, successive approximation
> > > register (SAR) analog-to-digital converter (ADC) with low-power and
> > > threshold monitoring modes.  
> > 
> > ...
> > 
> Hi Andy,
> > > +#define AD4062_SOFT_RESET	0x81  
> > 
> > The grouping seems a bit strange. Haven't you forgotten a blank line here?
> > Ditto for other similar cases.
> > 
> Ack.

Side note. For efficiency, if you agree with something just delete that
block of the thread and don't say so.  Reviewers should be safe to assume
that anything the author agrees with will just be fixed in the next version.

That lets us focus in very fast on the key discussions.

> > 
> > > +struct ad4062_state {
> > > +	const struct ad4062_chip_info *chip;
> > > +	const struct ad4062_bus_ops *ops;
> > > +	enum ad4062_operation_mode mode;
> > > +	struct completion completion;
> > > +	struct iio_trigger *trigger;
> > > +	struct iio_dev *indio_dev;
> > > +	struct i3c_device *i3cdev;
> > > +	struct regmap *regmap;
> > > +	u16 sampling_frequency;
> > > +	int vref_uv;
> > > +	int samp_freqs[ARRAY_SIZE(ad4062_conversion_freqs)];
> > > +	u8 oversamp_ratio;
> > > +	union {
> > > +		__be32 be32;
> > > +		__be16 be16;
> > > +		u8 bytes[4];
> > > +	} buf __aligned(IIO_DMA_MINALIGN);
> > > +	u8 reg_addr_conv;  
> > 
> > Can't we group u8:s to save a few bytes of memory?
> > 
> Sure
> 
>   struct ad4062_state {
>   	// ...
>   	union {
>   		__be32 be32;
>   		__be16 be16;
>   		u8 bytes[4];
>   	} buf __aligned(IIO_DMA_MINALIGN);
>   	u16 sampling_frequency;
>   	u8 oversamp_ratio;
>   	u8 reg_addr_conv;

Unless my assumption is wrong and those 3 values are passed
as buffers for DMA (or otherwise very carefully protected
against access racing with the DMA buffers) then this is very wrong.
Refresh your memory on why we do the __aligned(IIO_DMA_MINALIGN) and
exactly how that works.

Short answer, nothing must come after it in the structure as it
only forces the start of the buffer, not it's end and hence the
following data ends up in the same cacheline and fun data corruption
is the result.

>   };