Re: [PATCH 1/3] iio: adc: add max11410 adc driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 7 Jul 2022 08:28:11 +0000
Ibrahim Tilki <Ibrahim.Tilki@xxxxxxxxxx> wrote:

> > > +static irqreturn_t max11410_trigger_handler(int irq, void *p)
> > > +{
> > > +       struct iio_poll_func *pf = p;
> > > +       struct iio_dev *indio_dev = pf->indio_dev;
> > > +       struct max11410_state *st = iio_priv(indio_dev);
> > > +       struct {
> > > +               int data;
> > > +               s64 ts __aligned(8);
> > > +       } scan = {0};  
> > 
> > Why do you need an assignment here?
> > Even memcpy() in IRQ context is a burden.  
> 
> Because the buffer gets populated with meaningless data in the absence of assignment due to timestamp alignment.
> I've removed the assignment and addressed your other comments and sent a new series of patches.

I'll look at v2 in more depth, but as this got highlighted...
a) A non fixed size type for a scan element is going to potentially cause
problems.
b) There are holes in that structure, so you can't get away with just assigning
it as the compiler isn't guaranteed to zero out the holes.  An explicit memset()
is needed (as IIRC that is guaranteed to do so).

Jonathan

> 
> Best regards,
> Ibrahim Tilki




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux