[PATCH 1/2] iio: mma8452: Fix trigger reference couting

[PATCH 1/2] iio: mma8452: Fix trigger reference couting

[Lars-Peter Clausen]

The mma8452 driver directly assigns a trigger to the struct iio_dev. The
IIO core when done using this trigger will call `iio_trigger_put()` to drop
the reference count by 1.

Without the matching `iio_trigger_get()` in the driver the reference count
can reach 0 too early, the trigger gets freed while still in use and a
use-after-free occurs.

Fix this by getting a reference to the trigger before assigning it to the
IIO device.

Fixes: ae6d9ce05691 ("iio: mma8452: Add support for interrupt driven triggers.")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/accel/mma8452.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 715b8138fb71..09c7f10fefb6 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1470,7 +1470,7 @@ static int mma8452_trigger_setup(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
-	indio_dev->trig = trig;
+	indio_dev->trig = iio_trigger_get(trig);
 
 	return 0;
 }
-- 
2.20.1

[PATCH 2/2] iio: trigger: Fix reference counting

[Lars-Peter Clausen]

In viio_trigger_alloc() device_initialize() is used to set the initial
reference count of the trigger to 1. Then another get_device() is called on
trigger. This sets the reference count to 2 before the trigger is returned.

iio_trigger_free(), which is the matching API to viio_trigger_alloc(),
calls put_device() which decreases the reference count by 1. But the second
reference count acquired in viio_trigger_alloc() is never dropped.

As a result the iio_trigger_release() function is never called and the
memory associated with the trigger is never freed.

Since there is no reason for the trigger to start its lifetime with two
reference counts just remove the extra get_device() in
viio_trigger_alloc().

Fixes: 5f9c035cae18 ("staging:iio:triggers. Add a reference get to the core for triggers.")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
I'm a bit unsure about the fixes tag. I've looked at the IIO tree at the
point when this was introduced and I believe it was incorrect even back
then.

But we also had a few drivers that directly assigned the indio_dev->trig
without getting an extra reference. So these two bugs, one in the core, one
in the drivers sort of even out. Except that iio_trigger_get() also gets a
reference to the drivers module and iio_trigger_put() releases it again. So
with the missing iio_trigger_get() there is still the problem that, even
though the device references balance out, there is a module reference count
imbalance.
---
 drivers/iio/industrialio-trigger.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index b23caa2f2aa1..93990ff1dfe3 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -556,7 +556,6 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
 		irq_modify_status(trig->subirq_base + i,
 				  IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
 	}
-	get_device(&trig->dev);
 
 	return trig;
 
-- 
2.20.1

RE: [PATCH 2/2] iio: trigger: Fix reference counting

[Sa, Nuno]

> -----Original Message-----
> From: Lars-Peter Clausen <lars@metafoo.de>
> Sent: Sunday, October 24, 2021 11:27 AM
> To: Jonathan Cameron <jic23@kernel.org>
> Cc: Martin Fuzzey <mfuzzey@parkeon.com>; Peter Meerwald-Stadler
> <pmeerw@pmeerw.net>; linux-iio@vger.kernel.org; Lars-Peter
> Clausen <lars@metafoo.de>
> Subject: [PATCH 2/2] iio: trigger: Fix reference counting
> 
> [External]
> 
> In viio_trigger_alloc() device_initialize() is used to set the initial
> reference count of the trigger to 1. Then another get_device() is called
> on
> trigger. This sets the reference count to 2 before the trigger is
> returned.
> 
> iio_trigger_free(), which is the matching API to viio_trigger_alloc(),
> calls put_device() which decreases the reference count by 1. But the
> second
> reference count acquired in viio_trigger_alloc() is never dropped.
> 
> As a result the iio_trigger_release() function is never called and the
> memory associated with the trigger is never freed.
> 
> Since there is no reason for the trigger to start its lifetime with two
> reference counts just remove the extra get_device() in
> viio_trigger_alloc().
> 
> Fixes: 5f9c035cae18 ("staging:iio:triggers. Add a reference get to the
> core for triggers.")
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Acked-by: Nuno Sá <nuno.sa@analog.com>

> ---
> I'm a bit unsure about the fixes tag. I've looked at the IIO tree at the
> point when this was introduced and I believe it was incorrect even
> back
> then.
> 
> But we also had a few drivers that directly assigned the indio_dev->trig
> without getting an extra reference. So these two bugs, one in the
> core, one
> in the drivers sort of even out. Except that iio_trigger_get() also gets a
> reference to the drivers module and iio_trigger_put() releases it again.
> So
> with the missing iio_trigger_get() there is still the problem that, even
> though the device references balance out, there is a module reference
> count
> imbalance.
> ---
>  drivers/iio/industrialio-trigger.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-
> trigger.c
> index b23caa2f2aa1..93990ff1dfe3 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -556,7 +556,6 @@ struct iio_trigger *viio_trigger_alloc(struct
> device *parent,
>  		irq_modify_status(trig->subirq_base + i,
>  				  IRQ_NOREQUEST | IRQ_NOAUTOEN,
> IRQ_NOPROBE);
>  	}
> -	get_device(&trig->dev);
> 
>  	return trig;
> 
> --
> 2.20.1

Re: [PATCH 2/2] iio: trigger: Fix reference counting

[Jonathan Cameron]

On Sun, 24 Oct 2021 11:27:00 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> In viio_trigger_alloc() device_initialize() is used to set the initial
> reference count of the trigger to 1. Then another get_device() is called on
> trigger. This sets the reference count to 2 before the trigger is returned.
> 
> iio_trigger_free(), which is the matching API to viio_trigger_alloc(),
> calls put_device() which decreases the reference count by 1. But the second
> reference count acquired in viio_trigger_alloc() is never dropped.
> 
> As a result the iio_trigger_release() function is never called and the
> memory associated with the trigger is never freed.
> 
> Since there is no reason for the trigger to start its lifetime with two
> reference counts just remove the extra get_device() in
> viio_trigger_alloc().
> 
> Fixes: 5f9c035cae18 ("staging:iio:triggers. Add a reference get to the core for triggers.")
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

I fully agree the current code is wrong, but we really should be using
device_put() in the error path after device_initialize() has been called.

There are multiple places where we currently do this wrong in IIO but this particular
one looks like a local fix would be safe.
Worth doing that in the same patch at this one given it's all about reference
counting logic being wrong?  If not, we can do it as a separate follow up patch.

Jonathan


> ---
> I'm a bit unsure about the fixes tag. I've looked at the IIO tree at the
> point when this was introduced and I believe it was incorrect even back
> then.
> 
> But we also had a few drivers that directly assigned the indio_dev->trig
> without getting an extra reference. So these two bugs, one in the core, one
> in the drivers sort of even out. Except that iio_trigger_get() also gets a
> reference to the drivers module and iio_trigger_put() releases it again. So
> with the missing iio_trigger_get() there is still the problem that, even
> though the device references balance out, there is a module reference count
> imbalance.
> ---
>  drivers/iio/industrialio-trigger.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index b23caa2f2aa1..93990ff1dfe3 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -556,7 +556,6 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
>  		irq_modify_status(trig->subirq_base + i,
>  				  IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
>  	}
> -	get_device(&trig->dev);
>  
>  	return trig;
>  

Re: [PATCH 2/2] iio: trigger: Fix reference counting

[Lars-Peter Clausen]

On 10/28/21 4:16 PM, Jonathan Cameron wrote:
> On Sun, 24 Oct 2021 11:27:00 +0200
> Lars-Peter Clausen <lars@metafoo.de> wrote:
>
>> In viio_trigger_alloc() device_initialize() is used to set the initial
>> reference count of the trigger to 1. Then another get_device() is called on
>> trigger. This sets the reference count to 2 before the trigger is returned.
>>
>> iio_trigger_free(), which is the matching API to viio_trigger_alloc(),
>> calls put_device() which decreases the reference count by 1. But the second
>> reference count acquired in viio_trigger_alloc() is never dropped.
>>
>> As a result the iio_trigger_release() function is never called and the
>> memory associated with the trigger is never freed.
>>
>> Since there is no reason for the trigger to start its lifetime with two
>> reference counts just remove the extra get_device() in
>> viio_trigger_alloc().
>>
>> Fixes: 5f9c035cae18 ("staging:iio:triggers. Add a reference get to the core for triggers.")
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> I fully agree the current code is wrong, but we really should be using
> device_put() in the error path after device_initialize() has been called.
>
> There are multiple places where we currently do this wrong in IIO but this particular
> one looks like a local fix would be safe.
> Worth doing that in the same patch at this one given it's all about reference
> counting logic being wrong?  If not, we can do it as a separate follow up patch.
I already have that patch. Just waiting for this to be applied since it 
has a dependency.
>
> Jonathan
>
>
>> ---
>> I'm a bit unsure about the fixes tag. I've looked at the IIO tree at the
>> point when this was introduced and I believe it was incorrect even back
>> then.
>>
>> But we also had a few drivers that directly assigned the indio_dev->trig
>> without getting an extra reference. So these two bugs, one in the core, one
>> in the drivers sort of even out. Except that iio_trigger_get() also gets a
>> reference to the drivers module and iio_trigger_put() releases it again. So
>> with the missing iio_trigger_get() there is still the problem that, even
>> though the device references balance out, there is a module reference count
>> imbalance.
>> ---
>>   drivers/iio/industrialio-trigger.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
>> index b23caa2f2aa1..93990ff1dfe3 100644
>> --- a/drivers/iio/industrialio-trigger.c
>> +++ b/drivers/iio/industrialio-trigger.c
>> @@ -556,7 +556,6 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
>>   		irq_modify_status(trig->subirq_base + i,
>>   				  IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
>>   	}
>> -	get_device(&trig->dev);
>>   
>>   	return trig;
>>   

Re: [PATCH 2/2] iio: trigger: Fix reference counting

[Jonathan Cameron]

On Thu, 28 Oct 2021 18:04:22 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 10/28/21 4:16 PM, Jonathan Cameron wrote:
> > On Sun, 24 Oct 2021 11:27:00 +0200
> > Lars-Peter Clausen <lars@metafoo.de> wrote:
> >  
> >> In viio_trigger_alloc() device_initialize() is used to set the initial
> >> reference count of the trigger to 1. Then another get_device() is called on
> >> trigger. This sets the reference count to 2 before the trigger is returned.
> >>
> >> iio_trigger_free(), which is the matching API to viio_trigger_alloc(),
> >> calls put_device() which decreases the reference count by 1. But the second
> >> reference count acquired in viio_trigger_alloc() is never dropped.
> >>
> >> As a result the iio_trigger_release() function is never called and the
> >> memory associated with the trigger is never freed.
> >>
> >> Since there is no reason for the trigger to start its lifetime with two
> >> reference counts just remove the extra get_device() in
> >> viio_trigger_alloc().
> >>
> >> Fixes: 5f9c035cae18 ("staging:iio:triggers. Add a reference get to the core for triggers.")
> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>  
> > I fully agree the current code is wrong, but we really should be using
> > device_put() in the error path after device_initialize() has been called.
> >
> > There are multiple places where we currently do this wrong in IIO but this particular
> > one looks like a local fix would be safe.
> > Worth doing that in the same patch at this one given it's all about reference
> > counting logic being wrong?  If not, we can do it as a separate follow up patch.  
> I already have that patch. Just waiting for this to be applied since it 
> has a dependency.

In that case, applied for this one to the fixes-togreg branch of iio.git.

Thanks,

Jonathan

> >
> > Jonathan
> >
> >  
> >> ---
> >> I'm a bit unsure about the fixes tag. I've looked at the IIO tree at the
> >> point when this was introduced and I believe it was incorrect even back
> >> then.
> >>
> >> But we also had a few drivers that directly assigned the indio_dev->trig
> >> without getting an extra reference. So these two bugs, one in the core, one
> >> in the drivers sort of even out. Except that iio_trigger_get() also gets a
> >> reference to the drivers module and iio_trigger_put() releases it again. So
> >> with the missing iio_trigger_get() there is still the problem that, even
> >> though the device references balance out, there is a module reference count
> >> imbalance.
> >> ---
> >>   drivers/iio/industrialio-trigger.c | 1 -
> >>   1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> >> index b23caa2f2aa1..93990ff1dfe3 100644
> >> --- a/drivers/iio/industrialio-trigger.c
> >> +++ b/drivers/iio/industrialio-trigger.c
> >> @@ -556,7 +556,6 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
> >>   		irq_modify_status(trig->subirq_base + i,
> >>   				  IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
> >>   	}
> >> -	get_device(&trig->dev);
> >>   
> >>   	return trig;
> >>     
> 
> 

Re: [PATCH 1/2] iio: mma8452: Fix trigger reference couting

[Jonathan Cameron]

On Sun, 24 Oct 2021 11:26:59 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> The mma8452 driver directly assigns a trigger to the struct iio_dev. The
> IIO core when done using this trigger will call `iio_trigger_put()` to drop
> the reference count by 1.
> 
> Without the matching `iio_trigger_get()` in the driver the reference count
> can reach 0 too early, the trigger gets freed while still in use and a
> use-after-free occurs.
> 
> Fix this by getting a reference to the trigger before assigning it to the
> IIO device.
> 
> Fixes: ae6d9ce05691 ("iio: mma8452: Add support for interrupt driven triggers.")
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Gah. I thought we'd gotten all these years ago. I guess this one slipped through
the net.

Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> ---
>  drivers/iio/accel/mma8452.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 715b8138fb71..09c7f10fefb6 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1470,7 +1470,7 @@ static int mma8452_trigger_setup(struct iio_dev *indio_dev)
>  	if (ret)
>  		return ret;
>  
> -	indio_dev->trig = trig;
> +	indio_dev->trig = iio_trigger_get(trig);
>  
>  	return 0;
>  }

Re: [PATCH 1/2] iio: mma8452: Fix trigger reference couting

[Lars-Peter Clausen]

On 10/28/21 4:07 PM, Jonathan Cameron wrote:
> On Sun, 24 Oct 2021 11:26:59 +0200
> Lars-Peter Clausen <lars@metafoo.de> wrote:
>
>> The mma8452 driver directly assigns a trigger to the struct iio_dev. The
>> IIO core when done using this trigger will call `iio_trigger_put()` to drop
>> the reference count by 1.
>>
>> Without the matching `iio_trigger_get()` in the driver the reference count
>> can reach 0 too early, the trigger gets freed while still in use and a
>> use-after-free occurs.
>>
>> Fix this by getting a reference to the trigger before assigning it to the
>> IIO device.
>>
>> Fixes: ae6d9ce05691 ("iio: mma8452: Add support for interrupt driven triggers.")
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Gah. I thought we'd gotten all these years ago. I guess this one slipped through
> the net.
Btw. we already have iio_trigger_set_immutable(), which handles the 
reference counting. I was think of adding a iio(_device)_trigger_set() 
that does the same except not setting the trig_readonly flag. And then 
eventually move the trigger to iio_dev_opaque. Any concerns with this?

Re: [PATCH 1/2] iio: mma8452: Fix trigger reference couting

[Jonathan Cameron]

On Thu, 28 Oct 2021 21:52:46 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 10/28/21 4:07 PM, Jonathan Cameron wrote:
> > On Sun, 24 Oct 2021 11:26:59 +0200
> > Lars-Peter Clausen <lars@metafoo.de> wrote:
> >  
> >> The mma8452 driver directly assigns a trigger to the struct iio_dev. The
> >> IIO core when done using this trigger will call `iio_trigger_put()` to drop
> >> the reference count by 1.
> >>
> >> Without the matching `iio_trigger_get()` in the driver the reference count
> >> can reach 0 too early, the trigger gets freed while still in use and a
> >> use-after-free occurs.
> >>
> >> Fix this by getting a reference to the trigger before assigning it to the
> >> IIO device.
> >>
> >> Fixes: ae6d9ce05691 ("iio: mma8452: Add support for interrupt driven triggers.")
> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>  
> > Gah. I thought we'd gotten all these years ago. I guess this one slipped through
> > the net.  
> Btw. we already have iio_trigger_set_immutable(), which handles the 
> reference counting. I was think of adding a iio(_device)_trigger_set() 
> that does the same except not setting the trig_readonly flag. And then 
> eventually move the trigger to iio_dev_opaque. Any concerns with this?

No concerns, seems like as sensible change given how things are evolving.
Obviously some other stuff that would need changing before we can
actually move trig.

One early step would be to modify iio_trigger_notify_done() to take
a struct iio_dev rather than a struct iio_trigger.  A job for a coccinelle
script I think!  That function name might need a rethink along with the
parameter change.

Hmm. Looks like we have a few drivers passing indio_dev->trig to iio_trigger_poll
as well which is a little odd.  mma8452 is one of them and it's not using
an immutable trigger or validate_trigger() so unexpected results if anyone
changes the trigger...  Possibly not fatal as the interrupt will probably
not occur but not correct either...

Jonathan

Re: [PATCH 1/2] iio: mma8452: Fix trigger reference couting

[Lars-Peter Clausen]

On 10/30/21 5:03 PM, Jonathan Cameron wrote:
> On Thu, 28 Oct 2021 21:52:46 +0200
> Lars-Peter Clausen <lars@metafoo.de> wrote:
>
>> On 10/28/21 4:07 PM, Jonathan Cameron wrote:
>>> On Sun, 24 Oct 2021 11:26:59 +0200
>>> Lars-Peter Clausen <lars@metafoo.de> wrote:
>>>   
>>>> The mma8452 driver directly assigns a trigger to the struct iio_dev. The
>>>> IIO core when done using this trigger will call `iio_trigger_put()` to drop
>>>> the reference count by 1.
>>>>
>>>> Without the matching `iio_trigger_get()` in the driver the reference count
>>>> can reach 0 too early, the trigger gets freed while still in use and a
>>>> use-after-free occurs.
>>>>
>>>> Fix this by getting a reference to the trigger before assigning it to the
>>>> IIO device.
>>>>
>>>> Fixes: ae6d9ce05691 ("iio: mma8452: Add support for interrupt driven triggers.")
>>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>> Gah. I thought we'd gotten all these years ago. I guess this one slipped through
>>> the net.
>> Btw. we already have iio_trigger_set_immutable(), which handles the
>> reference counting. I was think of adding a iio(_device)_trigger_set()
>> that does the same except not setting the trig_readonly flag. And then
>> eventually move the trigger to iio_dev_opaque. Any concerns with this?
> No concerns, seems like as sensible change given how things are evolving.
> Obviously some other stuff that would need changing before we can
> actually move trig.
>
> One early step would be to modify iio_trigger_notify_done() to take
> a struct iio_dev rather than a struct iio_trigger.  A job for a coccinelle
> script I think!  That function name might need a rethink along with the
> parameter change.

That was my first idea, but then I was like why do we even have to call 
notify_done()? Can't we automate this, given all the bugs we had around 
this over the years.

Sill work in progress: 
https://github.com/larsclausen/linux/commit/d6ed694c9e512e1f7f3b40ad06b153feca8d7bb1 
but I think this will work.

>
> Hmm. Looks like we have a few drivers passing indio_dev->trig to iio_trigger_poll
> as well which is a little odd.  mma8452 is one of them and it's not using
> an immutable trigger or validate_trigger() so unexpected results if anyone
> changes the trigger...  Possibly not fatal as the interrupt will probably
> not occur but not correct either...
Yep, that's on my radar too. And one of the reasons to move the trigger 
to the opaque structure so this type of error can not happen.

Re: [PATCH 1/2] iio: mma8452: Fix trigger reference couting

[Jonathan Cameron]

On Sat, 30 Oct 2021 17:12:02 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 10/30/21 5:03 PM, Jonathan Cameron wrote:
> > On Thu, 28 Oct 2021 21:52:46 +0200
> > Lars-Peter Clausen <lars@metafoo.de> wrote:
> >  
> >> On 10/28/21 4:07 PM, Jonathan Cameron wrote:  
> >>> On Sun, 24 Oct 2021 11:26:59 +0200
> >>> Lars-Peter Clausen <lars@metafoo.de> wrote:
> >>>     
> >>>> The mma8452 driver directly assigns a trigger to the struct iio_dev. The
> >>>> IIO core when done using this trigger will call `iio_trigger_put()` to drop
> >>>> the reference count by 1.
> >>>>
> >>>> Without the matching `iio_trigger_get()` in the driver the reference count
> >>>> can reach 0 too early, the trigger gets freed while still in use and a
> >>>> use-after-free occurs.
> >>>>
> >>>> Fix this by getting a reference to the trigger before assigning it to the
> >>>> IIO device.
> >>>>
> >>>> Fixes: ae6d9ce05691 ("iio: mma8452: Add support for interrupt driven triggers.")
> >>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>  
> >>> Gah. I thought we'd gotten all these years ago. I guess this one slipped through
> >>> the net.  
> >> Btw. we already have iio_trigger_set_immutable(), which handles the
> >> reference counting. I was think of adding a iio(_device)_trigger_set()
> >> that does the same except not setting the trig_readonly flag. And then
> >> eventually move the trigger to iio_dev_opaque. Any concerns with this?  
> > No concerns, seems like as sensible change given how things are evolving.
> > Obviously some other stuff that would need changing before we can
> > actually move trig.
> >
> > One early step would be to modify iio_trigger_notify_done() to take
> > a struct iio_dev rather than a struct iio_trigger.  A job for a coccinelle
> > script I think!  That function name might need a rethink along with the
> > parameter change.  
> 
> That was my first idea, but then I was like why do we even have to call 
> notify_done()? Can't we automate this, given all the bugs we had around 
> this over the years.

Maybe.   Originally thinking was that some devices would schedule work to
complete the read so it might not correspond to IRQ_HANDLED.

I have no idea if there are any drivers still doing that though.

> 
> Sill work in progress: 
> https://github.com/larsclausen/linux/commit/d6ed694c9e512e1f7f3b40ad06b153feca8d7bb1 
> but I think this will work.
> 
> >
> > Hmm. Looks like we have a few drivers passing indio_dev->trig to iio_trigger_poll
> > as well which is a little odd.  mma8452 is one of them and it's not using
> > an immutable trigger or validate_trigger() so unexpected results if anyone
> > changes the trigger...  Possibly not fatal as the interrupt will probably
> > not occur but not correct either...  
> Yep, that's on my radar too. And one of the reasons to move the trigger 
> to the opaque structure so this type of error can not happen.
Indeed - the goal is good, but might take some doing to get there!

Jonathan