On Thu, Oct 10, 2024 at 02:32:49PM +0000, Nechita, Ramona wrote:
...
> >> +struct ad7779_state {
> >> + struct spi_device *spi;
> >> + const struct ad7779_chip_info *chip_info;
> >> + struct clk *mclk;
> >> + struct iio_trigger *trig;
> >> + struct completion completion;
> >> + unsigned int sampling_freq;
> >> + enum ad7779_filter filter_enabled;
> >> + /*
> >> + * DMA (thus cache coherency maintenance) requires the
> >> + * transfer buffers to live in their own cache lines.
> >> + */
> >> + struct {
> >> + u32 chans[8];
> >> + s64 timestamp;
> >
> > aligned_s64 timestamp;
> >
> >while it makes no difference in this case, this makes code aligned inside
> >the IIO subsystem.
>
> I might be missing something but I can't find the aligned_s64 data type,
> should I define it myself in the driver?
Definitely, basically the rule of thumb is to create your patches based on
the respective subsystem tree. In such a case this is IIO tree togreg branch
(in some cases testing should be used).
There is the mentioned type been introduced.
> >> + } data __aligned(IIO_DMA_MINALIGN);
> >
> >Note, this is different alignment to the above. And isn't the buffer below should have it instead?
> >
> >> + u32 spidata_tx[8];
> >> + u8 reg_rx_buf[3];
> >> + u8 reg_tx_buf[3];
> >> + u8 reset_buf[8];
> >> +};
...
> >> +static int ad7779_write_raw(struct iio_dev *indio_dev,
> >> + struct iio_chan_spec const *chan, int val, int val2,
> >> + long mask)
> >
> >long? Not unsigned long?
>
> I copied the function header directly from iio.h, shouldn't it be left as such?
Oh, this is unfortunate. It should be fixed there, indeed.
--
With Best Regards,
Andy Shevchenko
