Re: [PATCH v3 26/27] staging: iio: resolver: ad2s1210: implement fault events

[David Lechner]

On Sat, Sep 30, 2023 at 11:00 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Fri, 29 Sep 2023 12:23:31 -0500
> David Lechner <dlechner@xxxxxxxxxxxx> wrote:
>
> > From: David Lechner <david@xxxxxxxxxxxxxx>
> >
> > From: David Lechner <dlechner@xxxxxxxxxxxx>
> >
> > When reading the position and velocity on the AD2S1210, there is also a
> > 3rd byte following the two data bytes that contains the fault flag bits.
> > This patch adds support for reading this byte and generating events when
> > faults occur.
> >
> > The faults are mapped to various channels and event types in order to
> > have a unique event for each fault.
> >
> > Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>
>
> Use of x and y modifiers is a little odd.  What was your reasoning?
> Was it just that there was a X_OR_Y modifier?  If so, don't use that!
> It seemed like a good idea at the time, but it's not nice to deal with
> and requires a channel with that modifier to hang the controls off
> + make sure userspace expects that event code.


Regarding the point about "requires a channel with that modifier to
hang the controls off...". Although that comment was about modifiers,
does it also apply in general.

There are several fault events that don't have any configurable
parameters, namely _sine/cosine inputs clipping_ and _velocity exceeds
max tracking rate_. So there won't be any attributes that contain the
event specification for those (e.g. no `events/in_angl0_*`
attributes). It sounds like this would be a problem as well?

Should we consider a IIO_EV_INFO_LABEL so that we can have some sort
of attribute (namely `events/<dir>_<channel spec>_label`) so that
userspace can enumerate expected events for non-configurable events?