* [PATCH 0/2] staging:iio: trigger owner field confusion cleanup.
@ 2011-12-10 15:39 Jonathan Cameron
2011-12-10 15:39 ` [PATCH 1/2] staging:iio: Make sure all triggers have a trigger_ops for the owner field Jonathan Cameron
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jonathan Cameron @ 2011-12-10 15:39 UTC (permalink / raw)
To: linux-iio; +Cc: lars, Michael.Hennerich, Jonathan Cameron
This issue came to my attention whilst reviewing Maximes at91 patches.
Anyhow, we have iio_trigger->owner and iio_trigger->ops->owner.
The first was meant to have gone away.
A couple of drivers don't provide iio_trigger->ops causing nastiness
to happen when the trigger get or put occurs. Not seen in the wild
as I don't have either of the two devices in question.
Simple fix. Add the struture where missing and get rid of the owner field
from the iio_trigger structure.
Any comments?
Jonathan
Jonathan Cameron (2):
staging:iio: Make sure all triggers have a trigger_ops for the owner
field.
staging:iio: iio_trigger contains defunct owner field. Remove it.
drivers/staging/iio/adc/ad7192.c | 7 +++++--
drivers/staging/iio/adc/ad7793.c | 6 +++++-
drivers/staging/iio/trigger.h | 2 --
.../staging/iio/trigger/iio-trig-periodic-rtc.c | 1 -
4 files changed, 10 insertions(+), 6 deletions(-)
--
1.7.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] staging:iio: Make sure all triggers have a trigger_ops for the owner field.
2011-12-10 15:39 [PATCH 0/2] staging:iio: trigger owner field confusion cleanup Jonathan Cameron
@ 2011-12-10 15:39 ` Jonathan Cameron
2011-12-10 16:28 ` Jonathan Cameron
2011-12-10 15:39 ` [PATCH 2/2] staging:iio: iio_trigger contains defunct owner field. Remove it Jonathan Cameron
2011-12-11 12:00 ` [PATCH 0/2] staging:iio: trigger owner field confusion cleanup Lars-Peter Clausen
2 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2011-12-10 15:39 UTC (permalink / raw)
To: linux-iio; +Cc: lars, Michael.Hennerich, Jonathan Cameron
The core needs the owner field to prevent module removal whilst in use and
uses it without confirming that the trigger_ops structure actually exists.
Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
drivers/staging/iio/adc/ad7192.c | 6 +++++-
drivers/staging/iio/adc/ad7793.c | 5 +++++
2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 66cc507..6114601 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -609,6 +609,10 @@ static irqreturn_t ad7192_data_rdy_trig_poll(int irq, void *private)
return IRQ_HANDLED;
}
+static struct iio_trigger_ops ad7192_trigger_ops = {
+ .owner = THIS_MODULE,
+};
+
static int ad7192_probe_trigger(struct iio_dev *indio_dev)
{
struct ad7192_state *st = iio_priv(indio_dev);
@@ -621,7 +625,7 @@ static int ad7192_probe_trigger(struct iio_dev *indio_dev)
ret = -ENOMEM;
goto error_ret;
}
-
+ st->trig->ops = &ad7192_trigger_ops;
ret = request_irq(st->spi->irq,
ad7192_data_rdy_trig_poll,
IRQF_TRIGGER_LOW,
diff --git a/drivers/staging/iio/adc/ad7793.c b/drivers/staging/iio/adc/ad7793.c
index 4047c5d..f33feee 100644
--- a/drivers/staging/iio/adc/ad7793.c
+++ b/drivers/staging/iio/adc/ad7793.c
@@ -475,6 +475,10 @@ static irqreturn_t ad7793_data_rdy_trig_poll(int irq, void *private)
return IRQ_HANDLED;
}
+static struct iio_trigger_ops = {
+ .owner = THIS_MODULE,
+};
+
static int ad7793_probe_trigger(struct iio_dev *indio_dev)
{
struct ad7793_state *st = iio_priv(indio_dev);
@@ -487,6 +491,7 @@ static int ad7793_probe_trigger(struct iio_dev *indio_dev)
ret = -ENOMEM;
goto error_ret;
}
+ st->trig->ops = &ad7793_trigger_ops;
ret = request_irq(st->spi->irq,
ad7793_data_rdy_trig_poll,
--
1.7.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] staging:iio: Make sure all triggers have a trigger_ops for the owner field.
2011-12-10 15:39 ` [PATCH 1/2] staging:iio: Make sure all triggers have a trigger_ops for the owner field Jonathan Cameron
@ 2011-12-10 16:28 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2011-12-10 16:28 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, lars, Michael.Hennerich
On 12/10/2011 03:39 PM, Jonathan Cameron wrote:
> The core needs the owner field to prevent module removal whilst in use and
> uses it without confirming that the trigger_ops structure actually exists.
>
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
> ---
> drivers/staging/iio/adc/ad7192.c | 6 +++++-
> drivers/staging/iio/adc/ad7793.c | 5 +++++
> 2 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index 66cc507..6114601 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -609,6 +609,10 @@ static irqreturn_t ad7192_data_rdy_trig_poll(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static struct iio_trigger_ops ad7192_trigger_ops = {
> + .owner = THIS_MODULE,
> +};
> +
> static int ad7192_probe_trigger(struct iio_dev *indio_dev)
> {
> struct ad7192_state *st = iio_priv(indio_dev);
> @@ -621,7 +625,7 @@ static int ad7192_probe_trigger(struct iio_dev *indio_dev)
> ret = -ENOMEM;
> goto error_ret;
> }
> -
> + st->trig->ops = &ad7192_trigger_ops;
> ret = request_irq(st->spi->irq,
> ad7192_data_rdy_trig_poll,
> IRQF_TRIGGER_LOW,
> diff --git a/drivers/staging/iio/adc/ad7793.c b/drivers/staging/iio/adc/ad7793.c
> index 4047c5d..f33feee 100644
> --- a/drivers/staging/iio/adc/ad7793.c
> +++ b/drivers/staging/iio/adc/ad7793.c
> @@ -475,6 +475,10 @@ static irqreturn_t ad7793_data_rdy_trig_poll(int irq, void *private)
> return IRQ_HANDLED;
> }
>
Lets just pretend for the point of view of review that this variable
actually had the appropriate name. Sorry about that..
> +static struct iio_trigger_ops = {
> + .owner = THIS_MODULE,
> +};
> +
> static int ad7793_probe_trigger(struct iio_dev *indio_dev)
> {
> struct ad7793_state *st = iio_priv(indio_dev);
> @@ -487,6 +491,7 @@ static int ad7793_probe_trigger(struct iio_dev *indio_dev)
> ret = -ENOMEM;
> goto error_ret;
> }
> + st->trig->ops = &ad7793_trigger_ops;
>
> ret = request_irq(st->spi->irq,
> ad7793_data_rdy_trig_poll,
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] staging:iio: iio_trigger contains defunct owner field. Remove it.
2011-12-10 15:39 [PATCH 0/2] staging:iio: trigger owner field confusion cleanup Jonathan Cameron
2011-12-10 15:39 ` [PATCH 1/2] staging:iio: Make sure all triggers have a trigger_ops for the owner field Jonathan Cameron
@ 2011-12-10 15:39 ` Jonathan Cameron
2011-12-11 12:00 ` [PATCH 0/2] staging:iio: trigger owner field confusion cleanup Lars-Peter Clausen
2 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2011-12-10 15:39 UTC (permalink / raw)
To: linux-iio; +Cc: lars, Michael.Hennerich, Jonathan Cameron
This field moved into the trigger_ops structure a while back, but somehow
never quite got cleared up. This clears the last few drivers to set it
(nothing uses it) and gets rid of it entirely.
Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
drivers/staging/iio/adc/ad7192.c | 1 -
drivers/staging/iio/adc/ad7793.c | 1 -
drivers/staging/iio/trigger.h | 2 --
.../staging/iio/trigger/iio-trig-periodic-rtc.c | 1 -
4 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 6114601..dfeb4ba 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -637,7 +637,6 @@ static int ad7192_probe_trigger(struct iio_dev *indio_dev)
disable_irq_nosync(st->spi->irq);
st->irq_dis = true;
st->trig->dev.parent = &st->spi->dev;
- st->trig->owner = THIS_MODULE;
st->trig->private_data = indio_dev;
ret = iio_trigger_register(st->trig);
diff --git a/drivers/staging/iio/adc/ad7793.c b/drivers/staging/iio/adc/ad7793.c
index f33feee..1d4ffbb 100644
--- a/drivers/staging/iio/adc/ad7793.c
+++ b/drivers/staging/iio/adc/ad7793.c
@@ -504,7 +504,6 @@ static int ad7793_probe_trigger(struct iio_dev *indio_dev)
disable_irq_nosync(st->spi->irq);
st->irq_dis = true;
st->trig->dev.parent = &st->spi->dev;
- st->trig->owner = THIS_MODULE;
st->trig->private_data = indio_dev;
ret = iio_trigger_register(st->trig);
diff --git a/drivers/staging/iio/trigger.h b/drivers/staging/iio/trigger.h
index 5cc42a6..1cfca23 100644
--- a/drivers/staging/iio/trigger.h
+++ b/drivers/staging/iio/trigger.h
@@ -46,7 +46,6 @@ struct iio_trigger_ops {
* @private_data: [DRIVER] device specific data
* @list: [INTERN] used in maintenance of global trigger list
* @alloc_list: [DRIVER] used for driver specific trigger list
- * @owner: [DRIVER] used to monitor usage count of the trigger.
* @use_count: use count for the trigger
* @subirq_chip: [INTERN] associate 'virtual' irq chip.
* @subirq_base: [INTERN] base number for irqs provided by trigger.
@@ -63,7 +62,6 @@ struct iio_trigger {
void *private_data;
struct list_head list;
struct list_head alloc_list;
- struct module *owner;
int use_count;
struct irq_chip subirq_chip;
diff --git a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
index d35d085..bd7416b 100644
--- a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
+++ b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c
@@ -125,7 +125,6 @@ static int iio_trig_periodic_rtc_probe(struct platform_device *dev)
goto error_put_trigger_and_remove_from_list;
}
trig->private_data = trig_info;
- trig->owner = THIS_MODULE;
trig->ops = &iio_prtc_trigger_ops;
/* RTC access */
trig_info->rtc
--
1.7.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] staging:iio: trigger owner field confusion cleanup.
2011-12-10 15:39 [PATCH 0/2] staging:iio: trigger owner field confusion cleanup Jonathan Cameron
2011-12-10 15:39 ` [PATCH 1/2] staging:iio: Make sure all triggers have a trigger_ops for the owner field Jonathan Cameron
2011-12-10 15:39 ` [PATCH 2/2] staging:iio: iio_trigger contains defunct owner field. Remove it Jonathan Cameron
@ 2011-12-11 12:00 ` Lars-Peter Clausen
2 siblings, 0 replies; 6+ messages in thread
From: Lars-Peter Clausen @ 2011-12-11 12:00 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Michael.Hennerich
On 12/10/2011 04:39 PM, Jonathan Cameron wrote:
> This issue came to my attention whilst reviewing Maximes at91 patches.
> Anyhow, we have iio_trigger->owner and iio_trigger->ops->owner.
>
> The first was meant to have gone away.
> A couple of drivers don't provide iio_trigger->ops causing nastiness
> to happen when the trigger get or put occurs. Not seen in the wild
> as I don't have either of the two devices in question.
>
> Simple fix. Add the struture where missing and get rid of the owner field
> from the iio_trigger structure.
>
> Any comments?
>
> Jonathan
Looks good to me.
Both patches
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>
> Jonathan Cameron (2):
> staging:iio: Make sure all triggers have a trigger_ops for the owner
> field.
> staging:iio: iio_trigger contains defunct owner field. Remove it.
>
> drivers/staging/iio/adc/ad7192.c | 7 +++++--
> drivers/staging/iio/adc/ad7793.c | 6 +++++-
> drivers/staging/iio/trigger.h | 2 --
> .../staging/iio/trigger/iio-trig-periodic-rtc.c | 1 -
> 4 files changed, 10 insertions(+), 6 deletions(-)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/2] staging:iio: trigger owner field confusion cleanup.
@ 2011-12-14 20:49 Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2011-12-14 20:49 UTC (permalink / raw)
To: linux-iio, greg; +Cc: Jonathan Cameron
Hi Greg,
Lifting from original posting...
This issue came to my attention whilst reviewing Maximes at91 patches.
Anyhow, we have iio_trigger->owner and iio_trigger->ops->owner.
The first was meant to have gone away.
A couple of drivers don't provide iio_trigger->ops causing nastiness
to happen when the trigger get or put occurs. Not seen in the wild
as I don't have either of the two devices in question.
Simple fix. Add the struture where missing and get rid of the owner field
from the iio_trigger structure.
Thanks,
Jonathan
Jonathan Cameron (2):
staging:iio: Make sure all triggers have a trigger_ops for the owner
field.
staging:iio: iio_trigger contains defunct owner field. Remove it.
drivers/staging/iio/adc/ad7192.c | 7 +++++--
drivers/staging/iio/adc/ad7793.c | 6 +++++-
drivers/staging/iio/trigger.h | 2 --
.../staging/iio/trigger/iio-trig-periodic-rtc.c | 1 -
4 files changed, 10 insertions(+), 6 deletions(-)
--
1.7.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-14 20:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-10 15:39 [PATCH 0/2] staging:iio: trigger owner field confusion cleanup Jonathan Cameron
2011-12-10 15:39 ` [PATCH 1/2] staging:iio: Make sure all triggers have a trigger_ops for the owner field Jonathan Cameron
2011-12-10 16:28 ` Jonathan Cameron
2011-12-10 15:39 ` [PATCH 2/2] staging:iio: iio_trigger contains defunct owner field. Remove it Jonathan Cameron
2011-12-11 12:00 ` [PATCH 0/2] staging:iio: trigger owner field confusion cleanup Lars-Peter Clausen
-- strict thread matches above, loose matches on Subject: below --
2011-12-14 20:49 Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).