Intel Timed-IO driver in IIO/Counter subsystem

Intel Timed-IO driver in IIO/Counter subsystem

[N, Pandith]

Hi,

We have a Intel Timed IO peripheral with following functionalities :

1. Event capture capability - Captures event count and timestamp.
2. Pulse generation - periodic or single event generation.
3. Return cross-timestamp on request.

Timed IO device is being used in various Industrial use cases such as : time capture, synchronization, fan speed calculation etc.

IIO or counter subsystem seems to be suitable for timed-io driver.

Is it favourable to implement as part of IIO or counter subsystem ? Wanted to know your feedback.

We may need to use custom ABI for sysfs based user interaction OR
Can we enhance ioctl interface to accommodate our use case (counter-chardev.c) ?
Since timed-io works in nano second precision, ioctl is more suitable.

Regards,
Pandith and Sowjanya

ps : resent the mail with plain text as delivery to linux-iio@vger.kernel.org failed.

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

[Lars-Peter Clausen]

On 6/17/22 08:37, N, Pandith wrote:
> Hi,
>
> We have a Intel Timed IO peripheral with following functionalities :
>
> 1. Event capture capability - Captures event count and timestamp.
> 2. Pulse generation - periodic or single event generation.
> 3. Return cross-timestamp on request.
>
> Timed IO device is being used in various Industrial use cases such as : time capture, synchronization, fan speed calculation etc.
>
> IIO or counter subsystem seems to be suitable for timed-io driver.
>
> Is it favourable to implement as part of IIO or counter subsystem ? Wanted to know your feedback.

That sounds like a mix of a counter and PPS device. Have you looked at 
the PPS subsystem[1]?

[1] https://www.kernel.org/doc/html/latest/driver-api/pps.html

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

[Shevchenko, Andriy]

On Fri, Jun 17, 2022 at 09:22:09AM +0200, Lars-Peter Clausen wrote:
> On 6/17/22 08:37, N, Pandith wrote:
> > Hi,
> > 
> > We have a Intel Timed IO peripheral with following functionalities :
> > 
> > 1. Event capture capability - Captures event count and timestamp.
> > 2. Pulse generation - periodic or single event generation.
> > 3. Return cross-timestamp on request.
> > 
> > Timed IO device is being used in various Industrial use cases such as : time capture, synchronization, fan speed calculation etc.
> > 
> > IIO or counter subsystem seems to be suitable for timed-io driver.
> > 
> > Is it favourable to implement as part of IIO or counter subsystem ? Wanted to know your feedback.
> 
> That sounds like a mix of a counter and PPS device. Have you looked at the
> PPS subsystem[1]?

The newly HTE subsystem was proposed by Linus W. to be used for this case.

> [1] https://www.kernel.org/doc/html/latest/driver-api/pps.html

-- 
With Best Regards,
Andy Shevchenko

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

[Linus Walleij]

On Fri, Jun 17, 2022 at 11:51 AM Shevchenko, Andriy
<andriy.shevchenko@intel.com> wrote:
> On Fri, Jun 17, 2022 at 09:22:09AM +0200, Lars-Peter Clausen wrote:
> > On 6/17/22 08:37, N, Pandith wrote:
> > > Hi,
> > >
> > > We have a Intel Timed IO peripheral with following functionalities :
> > >
> > > 1. Event capture capability - Captures event count and timestamp.
> > > 2. Pulse generation - periodic or single event generation.
> > > 3. Return cross-timestamp on request.
> > >
> > > Timed IO device is being used in various Industrial use cases such as : time capture, synchronization, fan speed calculation etc.
> > >
> > > IIO or counter subsystem seems to be suitable for timed-io driver.
> > >
> > > Is it favourable to implement as part of IIO or counter subsystem ? Wanted to know your feedback.
> >
> > That sounds like a mix of a counter and PPS device. Have you looked at the
> > PPS subsystem[1]?
>
> The newly HTE subsystem was proposed by Linus W. to be used for this case.

Indeed, the subsystem was proposed exactly because both IIO and GPIO would
be consumers of it (possibly more). I think it covers case 1 & 3.

Dipen (the HTE subsystem maintainer) worked very hard on it and it can be
used with Tegra194 GPIO events flawlessly all the way to userspace.

You need to make an drivers/hte driver for the Intel hardware and jig
it to the IIO subsystem the same way we connected it to GPIO.

For 2. I am uncertain. Periodic events sound like PWM to me.

If a "single event" is something
like pulling a GPIO line high/low at a specific (wall clock) time in the
future, it should probably be in the GPIO subsystem, like a triggered
GPIO event or so, that sounds a bit hard but certainly doable with some
thinking and tinkering.

Yours,
Linus Walleij

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

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 1687 bytes --]

On Fri, Jun 17, 2022 at 06:37:14AM +0000, N, Pandith wrote:
> Hi,
> 
> We have a Intel Timed IO peripheral with following functionalities :
> 
> 1. Event capture capability - Captures event count and timestamp.
> 2. Pulse generation - periodic or single event generation.
> 3. Return cross-timestamp on request.
> 
> Timed IO device is being used in various Industrial use cases such as : time capture, synchronization, fan speed calculation etc.
> 
> IIO or counter subsystem seems to be suitable for timed-io driver.
> 
> Is it favourable to implement as part of IIO or counter subsystem ? Wanted to know your feedback.
> 
> We may need to use custom ABI for sysfs based user interaction OR
> Can we enhance ioctl interface to accommodate our use case (counter-chardev.c) ?
> Since timed-io works in nano second precision, ioctl is more suitable.
> 
> Regards,
> Pandith and Sowjanya
> 
> ps : resent the mail with plain text as delivery to linux-iio@vger.kernel.org failed.

Hello Pandith and Sowjanya,

What you are describing sounds similar to what counter-chardev.c tries
to solve (i.e. Counter events with timestamps). Would you elaborate more
on how this device works and what you are trying to accomplish with it?

For example, when you refer to an "event count and timestamp", does
count here mean the internal device hardware timestamp or is this the
Linux system timestamp? Does "pulse generation" refer to capturing the
count on some physical line signal, or is this a device-internal timer
countdown trigger event? Is "cross-timestamp" referring to a difference
calculation between two count events?

Thanks,

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Jonathan Cameron]

On Fri, 17 Jun 2022 10:03:30 -0400
William Breathitt Gray <william.gray@linaro.org> wrote:

> On Fri, Jun 17, 2022 at 06:37:14AM +0000, N, Pandith wrote:
> > Hi,
> > 
> > We have a Intel Timed IO peripheral with following functionalities :
> > 
> > 1. Event capture capability - Captures event count and timestamp.
> > 2. Pulse generation - periodic or single event generation.
> > 3. Return cross-timestamp on request.
> > 
> > Timed IO device is being used in various Industrial use cases such as : time capture, synchronization, fan speed calculation etc.
> > 
> > IIO or counter subsystem seems to be suitable for timed-io driver.
> > 
> > Is it favourable to implement as part of IIO or counter subsystem ? Wanted to know your feedback.
> > 
> > We may need to use custom ABI for sysfs based user interaction OR
> > Can we enhance ioctl interface to accommodate our use case (counter-chardev.c) ?
> > Since timed-io works in nano second precision, ioctl is more suitable.
> > 
> > Regards,
> > Pandith and Sowjanya
> > 
> > ps : resent the mail with plain text as delivery to linux-iio@vger.kernel.org failed.  
> 
> Hello Pandith and Sowjanya,
> 
> What you are describing sounds similar to what counter-chardev.c tries
> to solve (i.e. Counter events with timestamps). Would you elaborate more
> on how this device works and what you are trying to accomplish with it?
> 
> For example, when you refer to an "event count and timestamp", does
> count here mean the internal device hardware timestamp or is this the
> Linux system timestamp? Does "pulse generation" refer to capturing the
> count on some physical line signal, or is this a device-internal timer
> countdown trigger event? Is "cross-timestamp" referring to a difference
> calculation between two count events?
> 
> Thanks,
> 
> William Breathitt Gray
> 

If there is any chance of some docs access it might cut down people trying to
interpret what "Captures event count and timestamp" means! :) That could be
read as encoder type cases or it could be read as single event capture.

As you've discovered, the boundaries can get rather blurred!

Jonathan

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

[Hall, Christopher S]

N, Pandith <pandith.n@intel.com> wrote:
> Hi,
> 
> We have a Intel Timed IO peripheral with following functionalities :
> 
> 1. Event capture capability - Captures event count and timestamp.

An event is an edge on the input or output signal. Rising, falling,
or both edges can be selected for counting / timestamping. The
timestamp and count are captured synchronously.

If, for example, the hardware is configured to capture both types of
input edges, each input edge causes the count to increment by one and
the ART value (see below) to be captured atomically.

To see how this may be useful, consider the case where two event
occur before software is able to read the first. A count delta of >1
indicates that an event timestamp was missed.

For another example: If the input signal is periodic, the frequency
offset between the input clock and the local clock can be computed:

 (delta ART / delta count) * (nom. input freq / 1e9 ns) *
	(1e9 / ART frequency)

It is not necessary to read each event to determine the average
frequency for multiple events.

> 2. Pulse generation - periodic or single event generation.
> 3. Return cross-timestamp on request.

This may imply that the Timed I/O logic is driven by a separate device clock.
It is not. It is driven by the platform clock - the Always Running
Timer (ART). ART is directly related to the CPU timestamp counter (TSC).
ART and TSC can be converted to one another using the following equation:

TSC_Value = (ART_Value * CPUID.15H:EBX[31:0] )/ CPUID.15H:EAX[31:0] + K

ART ticks at the core crystal frequency. Typically this is 38.4 MHz. The
frequency can be discovered by reading CPUID.15H:ECX[31:0].

More information can be found in the Intel Software Developer's
Manual (SDM) in the Invariant Timekeeping section
17.17.4 and Determining the Processor Base Frequency section 18.7.3
(https://cdrdv2.intel.com/v1/dl/getContent/671200)

K is typically zero. A virtualized guest is an example where K != 0. In
this case, K is offset by the value of the VMCS TSC offset.

An example of how ART can be directly converted to system time is in the
e1000e driver:

drivers/net/ethernet/intel/e1000e/ptp.c:e1000e_phc_getcrosststamp()

using the following functions:

- arch/x86/kernel/tsc.c:convert_art_to_tsc() [ART->TSC]
- kernel/time/timekeeping.c:get_device_system_crosststamp() [TSC->System Time]

These are dependent upon TSC being selected as the clocksource. The
attempted conversion results in an error otherwise.

A PHC driver implementation of Timed I/O was proposed:
- https://lkml.org/lkml/2020/1/31/25

that included a cross-timestamp function. This crosstimestamp - in the sense
of determining the relationship between two independent clocks - is a
software fiction because system time is based on ART. The cross-timestamp
value enabled conversion of an event timestamp from ART -> System Time in
the application in the usual way when using the PHC API.

In my opinion, given a (more) greenfield API implementation. ART timestamps
should not be exposed at the application level at all. All timestamps
returned to the application should be in terms of system time.

There is a chapter (21.3.5) in the Atom(r) x6000E datasheet for Timed I/O:

https://cdrdv2.intel.com/v1/dl/getContent/636112?explicitVersion=true&wapkw=EHL%20datasheet

Note that the hardware function is called TGPIO.

There is also Timed I/O example code using the PHC driver referenced above:

https://www.intel.com/content/www/us/en/develop/documentation/tcc-tools-2021-2-developer-guide/top/time-synchronization-and-communication-tools/time-aware-gpio-tgpio-samples.html

Thanks,
Christopher

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

[Hall, Christopher S]

Hi Linus,

Friday, June 17, 2022 4:40 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> For 2. I am uncertain. Periodic events sound like PWM to me.

I do not think TGPIO periodic output is useful for PWM. There are two output
modes: edge output and pulse output. In edge mode output, where the an edge
is produced periodically based on the programmed period the duty cycle is
always 50%. In pulse mode output where a pulse is produced each output
period, the width of the pulse is two ART ticks which on current Intel
client platforms is about 50 ns. The pulse width is not adjustable.

We want to be able to output a clock from 1 Hz (1 PPS) up to 1 KHz that is
synchronized with the system clock.

It is possible to represent the periodic output function as a PWM device,
but the PWM subsystem output - without modification - is not aligned to
any clock which breaks the timing application.

> If a "single event" is something
> like pulling a GPIO line high/low at a specific (wall clock) time in the
> future, it should probably be in the GPIO subsystem, like a triggered
> GPIO event or so, that sounds a bit hard but certainly doable with some
> thinking and tinkering.

Earlier, we proposed a linereq_write() method in addition to the already
existing linereq_read().

https://lkml.org/lkml/2021/8/24/807

This is for "single shot" scheduled output only. This is fairly easy to
implement using the Timed I/O hardware because the clock used to schedule
output events is directly related to TSC and system time.

The difficult part is implementing this for devices that are not time
aware.

> Yours,
> Linus Walleij

Thanks,
Christopher

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

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 5589 bytes --]

On Sat, Jun 18, 2022 at 01:01:47AM +0000, Hall, Christopher S wrote:
> N, Pandith <pandith.n@intel.com> wrote:
> > Hi,
> > 
> > We have a Intel Timed IO peripheral with following functionalities :
> > 
> > 1. Event capture capability - Captures event count and timestamp.
> 
> An event is an edge on the input or output signal. Rising, falling,
> or both edges can be selected for counting / timestamping. The
> timestamp and count are captured synchronously.
> 
> If, for example, the hardware is configured to capture both types of
> input edges, each input edge causes the count to increment by one and
> the ART value (see below) to be captured atomically.
> 
> To see how this may be useful, consider the case where two event
> occur before software is able to read the first. A count delta of >1
> indicates that an event timestamp was missed.
> 
> For another example: If the input signal is periodic, the frequency
> offset between the input clock and the local clock can be computed:
> 
>  (delta ART / delta count) * (nom. input freq / 1e9 ns) *
> 	(1e9 / ART frequency)
> 
> It is not necessary to read each event to determine the average
> frequency for multiple events.
> 
> > 2. Pulse generation - periodic or single event generation.
> > 3. Return cross-timestamp on request.
> 
> This may imply that the Timed I/O logic is driven by a separate device clock.
> It is not. It is driven by the platform clock - the Always Running
> Timer (ART). ART is directly related to the CPU timestamp counter (TSC).
> ART and TSC can be converted to one another using the following equation:
> 
> TSC_Value = (ART_Value * CPUID.15H:EBX[31:0] )/ CPUID.15H:EAX[31:0] + K
> 
> ART ticks at the core crystal frequency. Typically this is 38.4 MHz. The
> frequency can be discovered by reading CPUID.15H:ECX[31:0].
> 
> More information can be found in the Intel Software Developer's
> Manual (SDM) in the Invariant Timekeeping section
> 17.17.4 and Determining the Processor Base Frequency section 18.7.3
> (https://cdrdv2.intel.com/v1/dl/getContent/671200)
> 
> K is typically zero. A virtualized guest is an example where K != 0. In
> this case, K is offset by the value of the VMCS TSC offset.
> 
> An example of how ART can be directly converted to system time is in the
> e1000e driver:
> 
> drivers/net/ethernet/intel/e1000e/ptp.c:e1000e_phc_getcrosststamp()
> 
> using the following functions:
> 
> - arch/x86/kernel/tsc.c:convert_art_to_tsc() [ART->TSC]
> - kernel/time/timekeeping.c:get_device_system_crosststamp() [TSC->System Time]
> 
> These are dependent upon TSC being selected as the clocksource. The
> attempted conversion results in an error otherwise.
> 
> A PHC driver implementation of Timed I/O was proposed:
> - https://lkml.org/lkml/2020/1/31/25
> 
> that included a cross-timestamp function. This crosstimestamp - in the sense
> of determining the relationship between two independent clocks - is a
> software fiction because system time is based on ART. The cross-timestamp
> value enabled conversion of an event timestamp from ART -> System Time in
> the application in the usual way when using the PHC API.
> 
> In my opinion, given a (more) greenfield API implementation. ART timestamps
> should not be exposed at the application level at all. All timestamps
> returned to the application should be in terms of system time.
> 
> There is a chapter (21.3.5) in the Atom(r) x6000E datasheet for Timed I/O:
> 
> https://cdrdv2.intel.com/v1/dl/getContent/636112?explicitVersion=true&wapkw=EHL%20datasheet
> 
> Note that the hardware function is called TGPIO.
> 
> There is also Timed I/O example code using the PHC driver referenced above:
> 
> https://www.intel.com/content/www/us/en/develop/documentation/tcc-tools-2021-2-developer-guide/top/time-synchronization-and-communication-tools/time-aware-gpio-tgpio-samples.html
> 
> Thanks,
> Christopher

Hi Christopher,

I'm not familiar with the recent HTE code, so I don't know whether it
can handle arbitrary data values; does the HTE code only track GPIO line
states (whether high or low) with respective timestamps, or can it also
track events count values with a respective timestamps?

Despite the events being triggered by GPIO, it seems from your
description that what you're actually concerned with is getting the
count of "events" and a "timestamp" derived from the ART ticks. If those
two values are what you're trying to handle, then the Counter subsystem
character device should be suitable for your needs:
https://www.kernel.org/doc/html/latest/driver-api/generic-counter.html#counter-character-device

This could be implemented by creating a Counter driver that treats the
events count as its "Count", and the respective ART-derived timestamp as
a its "Count Extension". When an event is triggered, the driver can push
that event to the respective Count character device via a
counter_push_event() call; userspace can add a "Counter Watch" via the
ioctl COUNTER_ADD_WATCH_IOCTL code to watch changes to these two values
(see tools/counter/counter_example.c).

As a side note, the "timestamp" member of struct counter_event serves
more as an event ID for your particular case here because your actual
timestamp is provided by your ART-derived Counter extension component
(whose value can be in terms of system time if you so wish); it'll come
as the "value" member of another struct counter_event following the
struct counter_event containing your events count.

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Linus Walleij]

On Sat, Jun 18, 2022 at 4:01 AM Hall, Christopher S
<christopher.s.hall@intel.com> wrote:
> Friday, June 17, 2022 4:40 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> > For 2. I am uncertain. Periodic events sound like PWM to me.
>
> I do not think TGPIO periodic output is useful for PWM. There are two output
> modes: edge output and pulse output. In edge mode output, where the an edge
> is produced periodically based on the programmed period the duty cycle is
> always 50%. In pulse mode output where a pulse is produced each output
> period, the width of the pulse is two ART ticks which on current Intel
> client platforms is about 50 ns. The pulse width is not adjustable.
>
> We want to be able to output a clock from 1 Hz (1 PPS) up to 1 KHz that is
> synchronized with the system clock.
>
> It is possible to represent the periodic output function as a PWM device,
> but the PWM subsystem output - without modification - is not aligned to
> any clock which breaks the timing application.

Is it "just" a clock then? As in drivers/clk?

It's of course annoying that the functionality of a certain hardware falls
between the subsystems so we end up using pieces of a subsystem in
another one, but there are several precedents, like network switch chips
and USB UART chips with GPIO inside them, or graphic chips with
clock dividers inside.

> > If a "single event" is something
> > like pulling a GPIO line high/low at a specific (wall clock) time in the
> > future, it should probably be in the GPIO subsystem, like a triggered
> > GPIO event or so, that sounds a bit hard but certainly doable with some
> > thinking and tinkering.
>
> Earlier, we proposed a linereq_write() method in addition to the already
> existing linereq_read().
>
> https://lkml.org/lkml/2021/8/24/807

This might be a good approach for this part of the hardware, as long
as we can make sure we get the userspace API abstract enough
that other hardware can make use of it and userspace does not
need to know what provides the timed output, just that there is
some hardware that does.

The ABI in the patch looks a bit dangerous, what happens if
you set an event like that and something decides to shake the
line by setting the output while the scheduled event is pending?

The direction seems sound however, but Bartosz and Kent need
to look at it in detail, their effort on the userspace ABI has been
tremendous.

Yours,
Linus Walleij

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

[Kent Gibson]

On Thu, Jun 23, 2022 at 02:21:30PM +0200, Linus Walleij wrote:
> On Sat, Jun 18, 2022 at 4:01 AM Hall, Christopher S
> <christopher.s.hall@intel.com> wrote:
> > Friday, June 17, 2022 4:40 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > > For 2. I am uncertain. Periodic events sound like PWM to me.
> >
> > I do not think TGPIO periodic output is useful for PWM. There are two output
> > modes: edge output and pulse output. In edge mode output, where the an edge
> > is produced periodically based on the programmed period the duty cycle is
> > always 50%. In pulse mode output where a pulse is produced each output
> > period, the width of the pulse is two ART ticks which on current Intel
> > client platforms is about 50 ns. The pulse width is not adjustable.
> >
> > We want to be able to output a clock from 1 Hz (1 PPS) up to 1 KHz that is
> > synchronized with the system clock.
> >
> > It is possible to represent the periodic output function as a PWM device,
> > but the PWM subsystem output - without modification - is not aligned to
> > any clock which breaks the timing application.
> 
> Is it "just" a clock then? As in drivers/clk?
> 
> It's of course annoying that the functionality of a certain hardware falls
> between the subsystems so we end up using pieces of a subsystem in
> another one, but there are several precedents, like network switch chips
> and USB UART chips with GPIO inside them, or graphic chips with
> clock dividers inside.
> 
> > > If a "single event" is something
> > > like pulling a GPIO line high/low at a specific (wall clock) time in the
> > > future, it should probably be in the GPIO subsystem, like a triggered
> > > GPIO event or so, that sounds a bit hard but certainly doable with some
> > > thinking and tinkering.
> >
> > Earlier, we proposed a linereq_write() method in addition to the already
> > existing linereq_read().
> >
> > https://lkml.org/lkml/2021/8/24/807
> 
> This might be a good approach for this part of the hardware, as long
> as we can make sure we get the userspace API abstract enough
> that other hardware can make use of it and userspace does not
> need to know what provides the timed output, just that there is
> some hardware that does.
> 
> The ABI in the patch looks a bit dangerous, what happens if
> you set an event like that and something decides to shake the
> line by setting the output while the scheduled event is pending?
> 
> The direction seems sound however, but Bartosz and Kent need
> to look at it in detail, their effort on the userspace ABI has been
> tremendous.
> 

Extending the GPIO uAPI seems like a reasonable approach for timed
sets, but as Linus mentioned, you need to consider the semantics of
your operation for the general case.  What if the set time has already
passed?  What happens with subsequent sets when one is already pending?
Can they be cancelled?

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 don't see the linereq_write() approach flying - an ioctl is more
appropriate.

Cheers,
Kent.

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

[Hall, Christopher S]

Hi Kent, William, and Linus:

Thank you for your input.

Kent Gibson <warthog618@gmail.com> wrote on Monday, July 04, 2022 8:17 PM:
> To: Hall, Christopher S <christopher.s.hall@intel.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>; Bartosz Golaszewski <brgl@bgdev.pl>; Shevchenko, Andriy
> <andriy.shevchenko@intel.com>; Dipen Patel <dipenp@nvidia.com>; N, Pandith <pandith.n@intel.com>; D,
> Lakshmi Sowjanya <lakshmi.sowjanya.d@intel.com>; Lars-Peter Clausen <lars@metafoo.de>; linux-
> iio@vger.kernel.org; vilhelm.gray@gmail.com; jic23@kernel.org; Sangannavar, Mallikarjunappa
> <mallikarjunappa.sangannavar@intel.com>; T R, Thejesh Reddy <thejesh.reddy.t.r@intel.com>
> Subject: Re: Intel Timed-IO driver in IIO/Counter subsystem
> 
> On Thu, Jun 23, 2022 at 02:21:30PM +0200, Linus Walleij wrote:
> > On Sat, Jun 18, 2022 at 4:01 AM Hall, Christopher S
> > <christopher.s.hall@intel.com> wrote:
> > > Friday, June 17, 2022 4:40 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> > > We want to be able to output a clock from 1 Hz (1 PPS) up to 1 KHz that is
> > > synchronized with the system clock.
> > >
> > > It is possible to represent the periodic output function as a PWM device,
> > > but the PWM subsystem output - without modification - is not aligned to
> > > any clock which breaks the timing application.
> >
> > Is it "just" a clock then? As in drivers/clk?
> >
> > It's of course annoying that the functionality of a certain hardware falls
> > between the subsystems so we end up using pieces of a subsystem in
> > another one, but there are several precedents, like network switch chips
> > and USB UART chips with GPIO inside them, or graphic chips with
> > clock dividers inside.
> >
> > > > If a "single event" is something
> > > > like pulling a GPIO line high/low at a specific (wall clock) time in the
> > > > future, it should probably be in the GPIO subsystem, like a triggered
> > > > GPIO event or so, that sounds a bit hard but certainly doable with some
> > > > thinking and tinkering.
> > >
> > > Earlier, we proposed a linereq_write() method in addition to the already
> > > existing linereq_read().
> > >
> > > https://lkml.org/lkml/2021/8/24/807
> >
> > This might be a good approach for this part of the hardware, as long
> > as we can make sure we get the userspace API abstract enough
> > that other hardware can make use of it and userspace does not
> > need to know what provides the timed output, just that there is
> > some hardware that does.
> >
> > The ABI in the patch looks a bit dangerous, what happens if
> > you set an event like that and something decides to shake the
> > line by setting the output while the scheduled event is pending?
> >
> > The direction seems sound however, but Bartosz and Kent need
> > to look at it in detail, their effort on the userspace ABI has been
> > tremendous.
> >
> 
> Extending the GPIO uAPI seems like a reasonable approach for timed
> sets, but as Linus mentioned, you need to consider the semantics of
> your operation for the general case.  What if the set time has already
> passed?  What happens with subsequent sets when one is already pending?
> Can they be cancelled?
> 
> 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. There is not a set method in the Timed IO hardware
logic, but one could be simulated by reading the current TSC clock value
and writing that to the COMPV (trigger time) register. This necessarily
cancels any trigger in progress unless the COMPV value is cached in
software and rewritten after the level is set. Having said that, I
cannot think of a good reason to mix the two trigger methods. For
example, if timed IO is used to produce a PPS output it is not very
useful to insert an untimed edge.

If we implement the set method, I think we should not allow setting the
output and return the error if an edge is already scheduled. The 'ban'
is lifted after the timer expires or is canceled.

> I don't see the linereq_write() approach flying - an ioctl is more
> appropriate.

This is OK. I am not overly attached to the write() interface

I think we could do this with a two ioctls:

struct lineevent_trigger {
	clockid_t clkid; /* We may want to select another clock */
	struct timespec trigger;
} ltrig_spec;

ioctl(fd, GPIOHANDLE_SET_LINE_EVENT_TRIGGER_ENABLE_IOCTL, &ltrig_spec) /* Set trigger time */
ioctl(fd, GPIOHANDLE_SET_LINE_EVENT_TRIGGER_CANCEL_IOCTL, &ltrig_spec) /* Cancel a trigger */

I do not see a need for a querying trigger status. The software can set
a timer or poll with clock_gettime()

I think this covers a single event. Periodic output is more of a challenge.
As noted above, the PWM interface is not a great fit mainly because the
PWM pulse is not aligned to a clock. We can augment the interface above

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

A period of 0 indicates a one-shot timer. This is consistent with the
timer_settime() API.

The trick here is that we can specify the nominal *TSC* output period, but
we need to adjust the period to align the output with a local clock - or
since this feature is likely to be used with TSN - a PTP network clock.

The hardware can count the number of events. Using this, we compute
the *actual* output period with respect to the system clock:

delta(system clock) / delta(event count)

We can nudge the output period to match whichever clock we are tracking
as long as we know its relation to the system clock.

The counter interface can be used find the deltas with the system/ART
timestamp extension suggested by William.

> Cheers,
> Kent.

Thanks,
Christopher

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

[Linus Walleij]

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

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

[Kent Gibson]

On Thu, Jul 07, 2022 at 01:05:16AM +0200, Linus Walleij wrote:
> 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.
> 

Strongly agree with Linus on this - the ABI needs to be general, not
in any way specific to your hardware - but something that can be
implemented using your hardware.  Ideally you have other example
hardware, or two.

GPIO or Timed may not be used simultaneously, but might it be selectable
at runtime?  If so the user would specify an output mode as part of the
line request, which could be an addition to the existing drive modes.

It still isn't clear to me what happens if I request a set for a past
time.  Does it set immediately, do nothing, or return an error?

Same applies for while one is pending, particularly how it is determined
that it is pending.  I have a bad feeling about race conditions here.

Also be aware that the GPIO uAPI line requests operate on a set of lines,
so your ioctl needs to identify which of the lines in the request it is
operating on. e.g. the mask in gpio_v2_line_values.

And you should use fixed sized fields in ioctl structs [1].
For timestamps the GPIO uAPI uses u64.  How that is interpreted depends
on the selected clock.

Also agree with Linus on the periodic trigger - that seems to have
jumped outside of GPIO scope.

Cheers,
Kent.

[1] https://www.kernel.org/doc/html/latest/driver-api/ioctl.html