Hi Manuel,
More explanation would be good.
This falls into the debate of using a single sysfs param to list
available options with the selected one highlighted in the list
(what you have here)
For every other element of iio we have gone with the separate attribute
listing what is available.
I'm also a little dubious about this as it massively
crosses over between different drivers (basically it gives an alternative
way of listing all triggers currently on the system). Obviously this is
to a certain extent true of led triggers as well, but there are relatively
few of them (as far as I know?)
We could easily have hundreds of triggers on a complex sensor rig. That
makes for a very bit attribute indeed.
So this is a nice idea, but in practice I'm not yet convinced it is actually
a good idea in practice. This may be one to bounce up to lkml for a wider
set of opinions.
Jonathan
Signed-off-by: Manuel Stahl <manuel.stahl@xxxxxxxxxxxxxxxxx> ---
drivers/staging/iio/industrialio-trigger.c | 27
++++++++++++++++++++------- 1 files changed, 20 insertions(+), 7
deletions(-)
diff --git a/drivers/staging/iio/industrialio-trigger.c
b/drivers/staging/iio/industrialio-trigger.c index 57dd923..25ee00f
100644 --- a/drivers/staging/iio/industrialio-trigger.c +++
b/drivers/staging/iio/industrialio-trigger.c @@ -293,11 +293,25 @@ static
ssize_t iio_trigger_read_current(struct device *dev,
char *buf)
{
struct iio_dev *dev_info = dev_get_drvdata(dev);
+ struct iio_trigger *trig;
int len = 0;
- if (dev_info->trig)
- len = sprintf(buf,
- "%s\n",
- dev_info->trig->name);
+
+ if (!dev_info->trig)
+ len += sprintf(buf+len, "[none]");
spaces round the +? I have a vague feeling checkpatch is fussy about that.
+ else
+ len += sprintf(buf+len, "none ");
+
+ mutex_lock(&iio_trigger_list_lock);
+ list_for_each_entry(trig, &iio_trigger_list, list) {
what is the point of this?
+ if (dev_info->trig && !strcmp(dev_info->trig->name,
+ trig->name))
+ len += sprintf(buf+len, " [%s]", trig->name);
+ else
+ len += sprintf(buf+len, " %s", trig->name);
At very least these need some protection to prevent a buffer overrun.
+ }
+ mutex_unlock(&iio_trigger_list_lock);
+
+ len += sprintf(len+buf, "\n");
return len;
}
@@ -331,17 +345,16 @@ static ssize_t iio_trigger_write_current(struct
device *dev,
return len;
}
-static DEVICE_ATTR(current_trigger, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(trigger, S_IRUGO | S_IWUSR,
iio_trigger_read_current,
iio_trigger_write_current);
static struct attribute *iio_trigger_consumer_attrs[] = {
- &dev_attr_current_trigger.attr,
+ &dev_attr_trigger.attr,
NULL,
};
static const struct attribute_group iio_trigger_consumer_attr_group = {
- .name = "trigger",
.attrs = iio_trigger_consumer_attrs,
};
--
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
