On Mon, 19 May 2025 23:25:58 +0900
Gyeyoung Baek <gye976@gmail.com> wrote:
> The `trig_type` indicates whether the trigger calls poll() or poll_nested().
> The `early_timestamp` indicates whether the trigger grabs the timestamp at the trigger.
> We need this to prevent the consumer from overwriting the timestamp.
>
> To allow the trigger to directly write the timestamp into the consumer's poll_func,
> add poll_func pointer member to the iio_trigger structure.
>
> However, I'm not sure if having a poll_func pointer member
> in iio_trigger is a good approach.
> Would this approach be acceptable?
I need to think about that. My initial thought was that it crosses boundaries
that we really don't want to cross.
I'm not sure I yet understand why we ever want the trigger to do that
write of the timestamp. Why are the wrapping handlers not enough?
>
> Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
> ---
> include/linux/iio/trigger.h | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/iio/trigger.h b/include/linux/iio/trigger.h
> index bce3b1788199..f3b89a1e0318 100644
> --- a/include/linux/iio/trigger.h
> +++ b/include/linux/iio/trigger.h
> @@ -36,6 +36,10 @@ struct iio_trigger_ops {
> struct iio_dev *indio_dev);
> };
>
> +#define IIO_TRIG_TYPE_POLL BIT(0)
> +#define IIO_TRIG_TYPE_POLL_NESTED BIT(1)
> +#define IIO_TRIG_TYPE_BOTH (IIO_TRIG_TYPE_POLL | \
> + IIO_TRIG_TYPE_POLL_NESTED)
I'm lost. How does a given trigger do both?
Also didn't these get used in an earlier patch? Even for an RFC make
sure it bisects.
>
> /**
> * struct iio_trigger - industrial I/O trigger device
> @@ -56,7 +60,10 @@ struct iio_trigger_ops {
> * i.e. if we registered a poll function to the same
> * device as the one providing the trigger.
> * @reenable_work: [INTERN] work item used to ensure reenable can sleep.
> + * @trig_type: [DRIVER] specifies whether the trigger calls poll(), poll_nested(), or both.
> + * @early_timestamp: [DRIVER] set to true if the trigger supports grabbing timestamp.
> **/
> +
Stray blank line.
> struct iio_trigger {
> const struct iio_trigger_ops *ops;
> struct module *owner;
> @@ -76,8 +83,13 @@ struct iio_trigger {
> struct mutex pool_lock;
> bool attached_own_device;
> struct work_struct reenable_work;
> -};
>
> + /* RFC, exists to access the consumer device’s pollfunc. */
> + struct iio_poll_func *consumer_pf[CONFIG_IIO_CONSUMERS_PER_TRIGGER];
> +
> + int trig_type;
> + bool early_timestamp;
> +};
>
> static inline struct iio_trigger *to_iio_trigger(struct device *d)
> {
>