| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e1602065-2817-d380-7f7a-3961070c384f@kernel.org>
Date: Sun, 19 Feb 2017 15:08:50 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Benjamin Gaignard <benjamin.gaignard@...aro.org>,
linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
knaack.h@....de, lars@...afoo.de, pmeerw@...erw.net
Cc: fabrice.gasnier@...com, linaro-kernel@...ts.linaro.org,
Benjamin Gaignard <benjamin.gaignard@...com>
Subject: Re: [PATCH v2 1/2] iio: Allow triggers to be used as parent of others
triggers
On 16/02/17 14:23, Benjamin Gaignard wrote:
> Add "parent_trigger" sysfs attribute to iio trigger to be able
> to set a parent to the current trigger.
> Parent trigger edges or levels could be used to control current
> trigger status for example to start, stop or reset it.
Reset needs a description as well. In this case I think it boils
down to syncing a periodic timer driven trigger.
>
> Introduce validate_trigger function in iio_trigger_ops which does
> the same than validate_device but with a trigger as second parameter.
> Driver must implement this function and add dev_attr_parent_trigger
> in their trigger attribute group to be able to use parent trigger
> feature.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...com>
I think I'm fine with this but it's 'unusual' enough that I want
to get to the point where we have some more people involved in
the discussion.
Jonathan
>
> version 2:
> - add comment about parent trigger usage
> - parent trigger attribute must be set the driver no more by IIO core
> ---
> .../ABI/testing/sysfs-bus-iio-trigger-sysfs | 10 ++++
> drivers/iio/industrialio-trigger.c | 68 ++++++++++++++++++++++
> include/linux/iio/trigger.h | 7 ++-
> 3 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
> index 04ac623..9862562 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
> @@ -40,3 +40,13 @@ Description:
> associated file, representing the id of the trigger that needs
> to be removed. If the trigger can't be found, an invalid
> argument message will be returned to the user.
> +
> +What: /sys/bus/iio/devices/triggerX/parent_trigger
> +KernelVersion: 4.12
> +Contact: linux-iio@...r.kernel.org
> +Description:
> + This attribute is used to set a trigger as parent for the
> + current trigger. If the trigger can't use the parent an
> + invalid argument message will be returned.
> + Parent trigger edges or levels could be used to control current
> + trigger for example to start, stop or reset it.
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 978729f..238aa1a 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -58,6 +58,74 @@ static ssize_t iio_trigger_read_name(struct device *dev,
>
> static DEVICE_ATTR(name, S_IRUGO, iio_trigger_read_name, NULL);
>
> +/**
> + * iio_trigger_read_parent() - trigger consumer sysfs query parent trigger
> + * @dev: device associated with an industrial I/O trigger
> + * @attr: pointer to the device_attribute structure that
> + * is being processed
> + * @buf: buffer where the current trigger name will be printed into
> + *
> + * Return: a negative number on failure, the number of characters written
> + * on success or 0 if no trigger is available
> + */
> +static ssize_t iio_trigger_read_parent(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_trigger *trig = to_iio_trigger(dev);
> +
> + if (trig->parent_trigger)
> + return sprintf(buf, "%s\n", trig->parent_trigger->name);
> +
> + return 0;
> +}
> +
> +static struct iio_trigger *iio_trigger_find_by_name(const char *name,
> + size_t len);
> +
> +/**
> + * iio_trigger_write_parent() - trigger consumer sysfs set parent trigger
> + * @dev: device associated with an industrial I/O trigger
> + * @attr: device attribute that is being processed
> + * @buf: string buffer that holds the name of the trigger
> + * @len: length of the trigger name held by buf
> + *
> + * Return: negative error code on failure or length of the buffer
> + * on success
> + */
> +static ssize_t iio_trigger_write_parent(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t len)
> +{
> + struct iio_trigger *parent;
> + struct iio_trigger *child = to_iio_trigger(dev);
> + int ret;
> +
> + if (!child->ops->validate_trigger)
> + return -EINVAL;
> +
> + if (atomic_read(&child->use_count))
> + return -EBUSY;
> +
> + parent = iio_trigger_find_by_name(buf, len);
> +
> + if (parent == child->parent_trigger)
> + return len;
> +
> + ret = child->ops->validate_trigger(child, parent);
> + if (ret)
> + return ret;
> +
> + child->parent_trigger = parent;
> +
> + return len;
> +}
> +
> +DEVICE_ATTR(parent_trigger, S_IRUGO | S_IWUSR,
> + iio_trigger_read_parent, iio_trigger_write_parent);
> +EXPORT_SYMBOL_GPL(dev_attr_parent_trigger);
> +
> static struct attribute *iio_trig_dev_attrs[] = {
> &dev_attr_name.attr,
> NULL,
> diff --git a/include/linux/iio/trigger.h b/include/linux/iio/trigger.h
> index ea08302..efa2983 100644
> --- a/include/linux/iio/trigger.h
> +++ b/include/linux/iio/trigger.h
> @@ -20,6 +20,7 @@ struct iio_subirq {
>
> struct iio_dev;
> struct iio_trigger;
> +extern struct device_attribute dev_attr_parent_trigger;
>
> /**
> * struct iio_trigger_ops - operations structure for an iio_trigger.
> @@ -29,6 +30,7 @@ struct iio_subirq {
> * use count is zero (may be NULL)
> * @validate_device: function to validate the device when the
> * current trigger gets changed.
> + * @validate_trigger: function to validate the parent trigger (may be NULL)
> *
> * This is typically static const within a driver and shared by
> * instances of a given device.
> @@ -39,9 +41,10 @@ struct iio_trigger_ops {
> int (*try_reenable)(struct iio_trigger *trig);
> int (*validate_device)(struct iio_trigger *trig,
> struct iio_dev *indio_dev);
> + int (*validate_trigger)(struct iio_trigger *trig,
> + struct iio_trigger *parent);
> };
>
> -
> /**
> * struct iio_trigger - industrial I/O trigger device
> * @ops: [DRIVER] operations structure
> @@ -59,6 +62,7 @@ struct iio_trigger_ops {
> * @attached_own_device:[INTERN] if we are using our own device as trigger,
> * i.e. if we registered a poll function to the same
> * device as the one providing the trigger.
> + * @parent_trigger: [INTERN] parent trigger
> **/
> struct iio_trigger {
> const struct iio_trigger_ops *ops;
> @@ -77,6 +81,7 @@ struct iio_trigger {
> unsigned long pool[BITS_TO_LONGS(CONFIG_IIO_CONSUMERS_PER_TRIGGER)];
> struct mutex pool_lock;
> bool attached_own_device;
> + struct iio_trigger *parent_trigger;
> };
>
>
>
Powered by blists - more mailing lists