On 06/07/2011 10:48 AM, Jonathan Cameron wrote:
On 06/06/11 20:52, michael.hennerich@xxxxxxxxxx wrote:
From: Michael Hennerich<michael.hennerich@xxxxxxxxxx>
Allow devices to reject triggers and vice versa.
Changes since V1:
Added kernel-doc
Moved callback into iio_info
Changed function naming
Revised return value passing
Included trigger.h to avoid warnings
Moved definition of struct iio_poll_func to avoid warnings
This last change should be a separate patch. If nothing else
it ought to go into mainline asap, whereas the rest of this
isn't quite so obviously a fix. Also, what is the warning?
(nought showing up here)
The warnings started with putting the callback into iio_info while it
didn't warn in iio_dev.
drivers/staging/iio/adc/../iio.h:263: warning: ‘struct iio_trigger’
declared inside parameter list
drivers/staging/iio/adc/../iio.h:263: warning: its scope is only this
definition or declaration, which is probably not what you want
In file included from drivers/staging/iio/industrialio-trigger.c:19:
So I included trigger.g in iio.h
Then these warnings appeared -
The question is not why those appear - this is quite obvious..
However why haven't we seen these before???
In file included from drivers/staging/iio/iio.h:18,
from drivers/staging/iio/industrialio-core.c:24:
drivers/staging/iio/trigger.h:101: warning: ‘struct iio_poll_func’
declared inside parameter list
drivers/staging/iio/trigger.h:101: warning: its scope is only this
definition or declaration, which is probably not what you want
drivers/staging/iio/trigger.h:110: warning: ‘struct iio_poll_func’
declared inside parameter list
In file included from drivers/staging/iio/iio.h:18,
from drivers/staging/iio/industrialio-trigger.c:19:
drivers/staging/iio/trigger.h:101: warning: ‘struct iio_poll_func’
declared inside parameter list
drivers/staging/iio/trigger.h:101: warning: its scope is only this
definition or declaration, which is probably not what you want
drivers/staging/iio/trigger.h:110: warning: ‘struct iio_poll_func’
declared inside parameter list
drivers/staging/iio/industrialio-trigger.c:226: error: conflicting types
for ‘iio_trigger_attach_poll_func’
drivers/staging/iio/trigger.h:100: error: previous declaration of
‘iio_trigger_attach_poll_func’ was here
drivers/staging/iio/industrialio-trigger.c:242: error: conflicting types
for ‘iio_trigger_attach_poll_func’
drivers/staging/iio/trigger.h:100: error: previous declaration of
‘iio_trigger_attach_poll_func’ was here
drivers/staging/iio/industrialio-trigger.c:244: error: conflicting types
for ‘iio_trigger_dettach_poll_func’
drivers/staging/iio/trigger.h:109: error: previous declaration of
‘iio_trigger_dettach_poll_func’ was here
drivers/staging/iio/industrialio-trigger.c:263: error: conflicting types
for ‘iio_trigger_dettach_poll_func’
drivers/staging/iio/trigger.h:109: error: previous declaration of
‘iio_trigger_dettach_poll_func’ was here
make[4]: *** [drivers/staging/iio/industrialio-trigger.o] Error 1
Hence, if you have time please break that bit out. Ack is
for both patches if you do this.
Otherwise, all looks good to me - I'll pull into my local
tree to rebase the trigger cleanups on top, but feel free
to push to Greg as well. I'm waiting on a few tests of non
ADI parts before pushing out the dev_data removal set.
The addition of these functions would also be slightly nicer
if in a set with a patch that actually uses them.
Well - AD7793 driver I'm going to send out for another round of review
will use them.
Thanks,
Jonathan
Signed-off-by: Michael Hennerich<michael.hennerich@xxxxxxxxxx>
Acked-by: Jonathan Cameron<jic23@xxxxxxxxx>
---
drivers/staging/iio/iio.h | 6 +++
drivers/staging/iio/industrialio-trigger.c | 20 ++++++++++-
drivers/staging/iio/trigger.h | 52 +++++++++++++++-------------
3 files changed, 53 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
index 78a0927..9bf8b11 100644
--- a/drivers/staging/iio/iio.h
+++ b/drivers/staging/iio/iio.h
@@ -15,6 +15,7 @@
#include<linux/irq.h>
#include "sysfs.h"
#include "chrdev.h"
+#include "trigger.h"
/* IIO TODO LIST */
/*
@@ -224,6 +225,8 @@ static inline s64 iio_get_time_ns(void)
* is event dependant. event_code specifies which event.
* @write_event_value: write the value associate with the event.
* Meaning is event dependent.
+ * @validate_trigger: function to validate the trigger when the
+ * current trigger gets changed.
**/
struct iio_info {
struct module *driver_module;
@@ -256,6 +259,9 @@ struct iio_info {
int (*write_event_value)(struct iio_dev *indio_dev,
int event_code,
int val);
+ int (*validate_trigger)(struct iio_dev *indio_dev,
+ struct iio_trigger *trig);
+
};
/**
diff --git a/drivers/staging/iio/industrialio-trigger.c b/drivers/staging/iio/industrialio-trigger.c
index d504aa2..90ca2df 100644
--- a/drivers/staging/iio/industrialio-trigger.c
+++ b/drivers/staging/iio/industrialio-trigger.c
@@ -340,6 +340,9 @@ static ssize_t iio_trigger_write_current(struct device *dev,
{
struct iio_dev *dev_info = dev_get_drvdata(dev);
struct iio_trigger *oldtrig = dev_info->trig;
+ struct iio_trigger *trig;
+ int ret;
+
mutex_lock(&dev_info->mlock);
if (dev_info->currentmode == INDIO_RING_TRIGGERED) {
mutex_unlock(&dev_info->mlock);
@@ -347,7 +350,22 @@ static ssize_t iio_trigger_write_current(struct device *dev,
}
mutex_unlock(&dev_info->mlock);
- dev_info->trig = iio_trigger_find_by_name(buf, len);
+ trig = iio_trigger_find_by_name(buf, len);
+
+ if (trig&& dev_info->info->validate_trigger) {
+ ret = dev_info->info->validate_trigger(dev_info, trig);
+ if (ret)
+ return ret;
+ }
+
+ if (trig&& trig->validate_device) {
+ ret = trig->validate_device(trig, dev_info);
+ if (ret)
+ return ret;
+ }
+
+ dev_info->trig = trig;
+
if (oldtrig&& dev_info->trig != oldtrig)
iio_put_trigger(oldtrig);
if (dev_info->trig)
diff --git a/drivers/staging/iio/trigger.h b/drivers/staging/iio/trigger.h
index f329fe1..e0b58ed 100644
--- a/drivers/staging/iio/trigger.h
+++ b/drivers/staging/iio/trigger.h
@@ -29,6 +29,8 @@ struct iio_subirq {
* @set_trigger_state: [DRIVER] switch on/off the trigger on demand
* @try_reenable: function to reenable the trigger when the
* use count is zero (may be NULL)
+ * @validate_device: function to validate the device when the
+ * current trigger gets changed.
* @subirq_chip: [INTERN] associate 'virtual' irq chip.
* @subirq_base: [INTERN] base number for irqs provided by trigger.
* @subirqs: [INTERN] information about the 'child' irqs.
@@ -48,6 +50,8 @@ struct iio_trigger {
int (*set_trigger_state)(struct iio_trigger *trig, bool state);
int (*try_reenable)(struct iio_trigger *trig);
+ int (*validate_device)(struct iio_trigger *trig,
+ struct iio_dev *indio_dev);
struct irq_chip subirq_chip;
int subirq_base;
@@ -57,6 +61,30 @@ struct iio_trigger {
struct mutex pool_lock;
};
+/**
+ * struct iio_poll_func - poll function pair
+ *
+ * @private_data: data specific to device (passed into poll func)
+ * @h: the function that is actually run on trigger
+ * @thread: threaded interrupt part
+ * @type: the type of interrupt (basically if oneshot)
+ * @name: name used to identify the trigger consumer.
+ * @irq: the corresponding irq as allocated from the
+ * trigger pool
+ * @timestamp: some devices need a timestamp grabbed as soon
+ * as possible after the trigger - hence handler
+ * passes it via here.
+ **/
+struct iio_poll_func {
+ void *private_data;
+ irqreturn_t (*h)(int irq, void *p);
+ irqreturn_t (*thread)(int irq, void *p);
+ int type;
+ char *name;
+ int irq;
+ s64 timestamp;
+};
+
static inline struct iio_trigger *to_iio_trigger(struct device *d)
{
return container_of(d, struct iio_trigger, dev);
@@ -136,30 +164,6 @@ static inline void iio_trigger_put_irq(struct iio_trigger *trig, int irq)
mutex_unlock(&trig->pool_lock);
};
-/**
- * struct iio_poll_func - poll function pair
- *
- * @private_data: data specific to device (passed into poll func)
- * @h: the function that is actually run on trigger
- * @thread: threaded interrupt part
- * @type: the type of interrupt (basically if oneshot)
- * @name: name used to identify the trigger consumer.
- * @irq: the corresponding irq as allocated from the
- * trigger pool
- * @timestamp: some devices need a timestamp grabbed as soon
- * as possible after the trigger - hence handler
- * passes it via here.
- **/
-struct iio_poll_func {
- void *private_data;
- irqreturn_t (*h)(int irq, void *p);
- irqreturn_t (*thread)(int irq, void *p);
- int type;
- char *name;
- int irq;
- s64 timestamp;
-};
-
struct iio_poll_func
*iio_alloc_pollfunc(irqreturn_t (*h)(int irq, void *p),
irqreturn_t (*thread)(int irq, void *p),
--
Greetings,
Michael
--
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
