Re: Intel Timed-IO driver in IIO/Counter subsystem

Linus Walleij
2022-07-06T23:05:31Z

On Wed, Jul 6, 2022 at 7:52 AM Hall, Christopher S
<christopher.s.hall@intel.com> wrote:

> > My first thought was that you could extend the SET_VALUES ioctl but,
> > while we have reserved space for future use in most of the ioctls it
> > turns out we overlooked sets and gets, so you would be looking at a new
> > ioctl. And you need to keep in mind how the SET_VALUES ioctl would
> > interact with it (Linus' point).
>
> I think we worked around this in a previous patchset by disallowing
> (return error) the 'set' method. The pin/pad is assigned (by mux
> configured in BIOS) to either GPIO or Timed I/O logic and both cannot be
> used simultaneously.

And we know it will always be like that? What about other systems
that are not your specific x86 box and that go and implement this
API? I don't think this should be handled in the driver but in the
gpiolib.

> I think we could do this with a two ioctls:
>
> struct lineevent_trigger {
> clockid_t clkid; /* We may want to select another clock */

Now you also need an ioctl to list the available clock IDs and
possibly their characteristics. Userspace can't just assume things
here.

> struct timespec trigger;
> } ltrig_spec;

You also need to specify which event you are triggering, right?
If it is a rising edge or a falling edge. I suspect you can just reuse
the event IDs from the existing incoming events:

/**
* enum gpio_v2_line_event_id - &struct gpio_v2_line_event.id values
* @GPIO_V2_LINE_EVENT_RISING_EDGE: event triggered by a rising edge
* @GPIO_V2_LINE_EVENT_FALLING_EDGE: event triggered by a falling edge
*/
enum gpio_v2_line_event_id {
GPIO_V2_LINE_EVENT_RISING_EDGE = 1,
GPIO_V2_LINE_EVENT_FALLING_EDGE = 2,
};

This is a simple __u32 id; in the ioctl struct.

> struct lineevent_trigger_periodic {
> clockid_t clkid;
> struct timespec trigger;
> struct timespec period;
> } ltrig_spec;

Needs the same things as above, but I'm sceptical about this one.
To me it seems like some fancy clock divider and then it should just
use the clk framework, I think I already remarked on this.

How is a periodic trigger with 50/50 duty cycle that cannot be changed
and derived from a certain clock different from a good old clock divider?
Externally routed clocks and clock dividers are not new.

I don't see what business this has in the GPIO library other than that
some hardware designer put these things together.

This would be relevant if you could set the duration of the pulse train,
such as "send 50 pulses then stop", so it can be used for things
such as stepper motors but apparently it can't? Especially not with
this ABI.

Yours,
Linus Walleij