[PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed I/O PPS generator

subramanian.mohan@intel.com posted 3 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed I/O PPS generator
Posted by subramanian.mohan@intel.com 1 year, 3 months ago
From: Subramanian Mohan <subramanian.mohan@intel.com>

Add Intel Timed I/O PPS usage instructions.

Co-developed-by: Pandith N <pandith.n@intel.com>
Signed-off-by: Pandith N <pandith.n@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Rodolfo Giometti <giometti@enneenne.com>
Signed-off-by: Subramanian Mohan <subramanian.mohan@intel.com>
---
 Documentation/driver-api/pps.rst | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/driver-api/pps.rst b/Documentation/driver-api/pps.rst
index 78dded03e5d8..75f7b094f963 100644
--- a/Documentation/driver-api/pps.rst
+++ b/Documentation/driver-api/pps.rst
@@ -246,3 +246,27 @@ delay between assert and clear edge as small as possible to reduce system
 latencies. But if it is too small slave won't be able to capture clear edge
 transition. The default of 30us should be good enough in most situations.
 The delay can be selected using 'delay' pps_gen_parport module parameter.
+
+
+Intel Timed I/O PPS signal generator
+------------------------------------
+
+Intel Timed I/O is a high precision device, present on 2019 and newer Intel
+CPUs, that can generate PPS signals.
+
+Timed I/O and system time are both driven by same hardware clock. The signal
+is generated with a precision of ~20 nanoseconds. The generated PPS signal
+is used to synchronize an external device with system clock. For example,
+it can be used to share your clock with a device that receives PPS signal,
+generated by Timed I/O device. There are dedicated Timed I/O pins to deliver
+the PPS signal to an external device.
+
+Usage of Intel Timed I/O as PPS generator:
+
+Start generating PPS signal::
+
+        $echo 1 > /sys/devices/platform/INTCxxxx\:00/enable
+
+Stop generating PPS signal::
+
+        $echo 0 > /sys/devices/platform/INTCxxxx\:00/enable
-- 
2.35.3
Re: [PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed I/O PPS generator
Posted by Greg KH 1 year, 3 months ago
On Fri, Aug 23, 2024 at 12:31:07PM +0530, subramanian.mohan@intel.com wrote:
> From: Subramanian Mohan <subramanian.mohan@intel.com>
> 
> Add Intel Timed I/O PPS usage instructions.
> 
> Co-developed-by: Pandith N <pandith.n@intel.com>
> Signed-off-by: Pandith N <pandith.n@intel.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Rodolfo Giometti <giometti@enneenne.com>
> Signed-off-by: Subramanian Mohan <subramanian.mohan@intel.com>
> ---
>  Documentation/driver-api/pps.rst | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/Documentation/driver-api/pps.rst b/Documentation/driver-api/pps.rst
> index 78dded03e5d8..75f7b094f963 100644
> --- a/Documentation/driver-api/pps.rst
> +++ b/Documentation/driver-api/pps.rst
> @@ -246,3 +246,27 @@ delay between assert and clear edge as small as possible to reduce system
>  latencies. But if it is too small slave won't be able to capture clear edge
>  transition. The default of 30us should be good enough in most situations.
>  The delay can be selected using 'delay' pps_gen_parport module parameter.
> +
> +
> +Intel Timed I/O PPS signal generator
> +------------------------------------
> +
> +Intel Timed I/O is a high precision device, present on 2019 and newer Intel
> +CPUs, that can generate PPS signals.
> +
> +Timed I/O and system time are both driven by same hardware clock. The signal
> +is generated with a precision of ~20 nanoseconds. The generated PPS signal
> +is used to synchronize an external device with system clock. For example,
> +it can be used to share your clock with a device that receives PPS signal,
> +generated by Timed I/O device. There are dedicated Timed I/O pins to deliver
> +the PPS signal to an external device.
> +
> +Usage of Intel Timed I/O as PPS generator:
> +
> +Start generating PPS signal::
> +
> +        $echo 1 > /sys/devices/platform/INTCxxxx\:00/enable
> +
> +Stop generating PPS signal::
> +
> +        $echo 0 > /sys/devices/platform/INTCxxxx\:00/enable

As I mentioned on the sysfs documentation, why isn't this just a generic
pps class attribute instead?  Why did you make it
only-this-one-special-driver type of thing?

thanks,

greg k-h
Re: [PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed I/O PPS generator
Posted by Rodolfo Giometti 1 year, 3 months ago
On 24/08/24 04:21, Greg KH wrote:
> On Fri, Aug 23, 2024 at 12:31:07PM +0530, subramanian.mohan@intel.com wrote:
>> From: Subramanian Mohan <subramanian.mohan@intel.com>
>>
>> Add Intel Timed I/O PPS usage instructions.
>>
>> Co-developed-by: Pandith N <pandith.n@intel.com>
>> Signed-off-by: Pandith N <pandith.n@intel.com>
>> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Acked-by: Rodolfo Giometti <giometti@enneenne.com>
>> Signed-off-by: Subramanian Mohan <subramanian.mohan@intel.com>
>> ---
>>   Documentation/driver-api/pps.rst | 24 ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/Documentation/driver-api/pps.rst b/Documentation/driver-api/pps.rst
>> index 78dded03e5d8..75f7b094f963 100644
>> --- a/Documentation/driver-api/pps.rst
>> +++ b/Documentation/driver-api/pps.rst
>> @@ -246,3 +246,27 @@ delay between assert and clear edge as small as possible to reduce system
>>   latencies. But if it is too small slave won't be able to capture clear edge
>>   transition. The default of 30us should be good enough in most situations.
>>   The delay can be selected using 'delay' pps_gen_parport module parameter.
>> +
>> +
>> +Intel Timed I/O PPS signal generator
>> +------------------------------------
>> +
>> +Intel Timed I/O is a high precision device, present on 2019 and newer Intel
>> +CPUs, that can generate PPS signals.
>> +
>> +Timed I/O and system time are both driven by same hardware clock. The signal
>> +is generated with a precision of ~20 nanoseconds. The generated PPS signal
>> +is used to synchronize an external device with system clock. For example,
>> +it can be used to share your clock with a device that receives PPS signal,
>> +generated by Timed I/O device. There are dedicated Timed I/O pins to deliver
>> +the PPS signal to an external device.
>> +
>> +Usage of Intel Timed I/O as PPS generator:
>> +
>> +Start generating PPS signal::
>> +
>> +        $echo 1 > /sys/devices/platform/INTCxxxx\:00/enable
>> +
>> +Stop generating PPS signal::
>> +
>> +        $echo 0 > /sys/devices/platform/INTCxxxx\:00/enable
> 
> As I mentioned on the sysfs documentation, why isn't this just a generic
> pps class attribute instead?  Why did you make it
> only-this-one-special-driver type of thing?

This is an Original Sin when PPS generators were introduced. :-(

In 2011 a patch from Alexander Gordeev <lasaine@lvk.cs.msu.su> (which introduced 
the "parallel port PPS signal generator") was committed in the main kernel.

At the time it was something exotic and doing a PPS generator interface for it 
was not considered (since it actually has no controlling inputs), but now 
several Ethernet cards have such PPS generator functionalities, and they are 
enabled in a per-driver way or via the PTP API.

This code is a pure PPS generator and it cannot use any other way to enable such 
functionality than the one above since the PPS layer misses a proper 
implementation for PPS generator.

If you are willing to stop the inclusion due this fact maybe its time to add 
such PPS generators interface... on the other hand, if you agree for inclusion 
we can do this job as soon as the code has been included, in order to fix this 
anomalous status.

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming
Re: [PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed I/O PPS generator
Posted by Greg KH 1 year, 3 months ago
On Tue, Aug 27, 2024 at 03:09:15PM +0200, Rodolfo Giometti wrote:
> On 24/08/24 04:21, Greg KH wrote:
> > On Fri, Aug 23, 2024 at 12:31:07PM +0530, subramanian.mohan@intel.com wrote:
> > > From: Subramanian Mohan <subramanian.mohan@intel.com>
> > > 
> > > Add Intel Timed I/O PPS usage instructions.
> > > 
> > > Co-developed-by: Pandith N <pandith.n@intel.com>
> > > Signed-off-by: Pandith N <pandith.n@intel.com>
> > > Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Acked-by: Rodolfo Giometti <giometti@enneenne.com>
> > > Signed-off-by: Subramanian Mohan <subramanian.mohan@intel.com>
> > > ---
> > >   Documentation/driver-api/pps.rst | 24 ++++++++++++++++++++++++
> > >   1 file changed, 24 insertions(+)
> > > 
> > > diff --git a/Documentation/driver-api/pps.rst b/Documentation/driver-api/pps.rst
> > > index 78dded03e5d8..75f7b094f963 100644
> > > --- a/Documentation/driver-api/pps.rst
> > > +++ b/Documentation/driver-api/pps.rst
> > > @@ -246,3 +246,27 @@ delay between assert and clear edge as small as possible to reduce system
> > >   latencies. But if it is too small slave won't be able to capture clear edge
> > >   transition. The default of 30us should be good enough in most situations.
> > >   The delay can be selected using 'delay' pps_gen_parport module parameter.
> > > +
> > > +
> > > +Intel Timed I/O PPS signal generator
> > > +------------------------------------
> > > +
> > > +Intel Timed I/O is a high precision device, present on 2019 and newer Intel
> > > +CPUs, that can generate PPS signals.
> > > +
> > > +Timed I/O and system time are both driven by same hardware clock. The signal
> > > +is generated with a precision of ~20 nanoseconds. The generated PPS signal
> > > +is used to synchronize an external device with system clock. For example,
> > > +it can be used to share your clock with a device that receives PPS signal,
> > > +generated by Timed I/O device. There are dedicated Timed I/O pins to deliver
> > > +the PPS signal to an external device.
> > > +
> > > +Usage of Intel Timed I/O as PPS generator:
> > > +
> > > +Start generating PPS signal::
> > > +
> > > +        $echo 1 > /sys/devices/platform/INTCxxxx\:00/enable
> > > +
> > > +Stop generating PPS signal::
> > > +
> > > +        $echo 0 > /sys/devices/platform/INTCxxxx\:00/enable
> > 
> > As I mentioned on the sysfs documentation, why isn't this just a generic
> > pps class attribute instead?  Why did you make it
> > only-this-one-special-driver type of thing?
> 
> This is an Original Sin when PPS generators were introduced. :-(
> 
> In 2011 a patch from Alexander Gordeev <lasaine@lvk.cs.msu.su> (which
> introduced the "parallel port PPS signal generator") was committed in the
> main kernel.
> 
> At the time it was something exotic and doing a PPS generator interface for
> it was not considered (since it actually has no controlling inputs), but now
> several Ethernet cards have such PPS generator functionalities, and they are
> enabled in a per-driver way or via the PTP API.
> 
> This code is a pure PPS generator and it cannot use any other way to enable
> such functionality than the one above since the PPS layer misses a proper
> implementation for PPS generator.
> 
> If you are willing to stop the inclusion due this fact maybe its time to add
> such PPS generators interface... on the other hand, if you agree for
> inclusion we can do this job as soon as the code has been included, in order
> to fix this anomalous status.

Please make a generic pps subsystem for this, it would make it simpler
for everyone.

thanks,

greg k-h
RE: [PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed I/O PPS generator
Posted by Hall, Christopher S 1 year, 2 months ago
Hi Rodolfo,

> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Tuesday, September 03, 2024 3:25 AM
> To: Rodolfo Giometti <giometti@enneenne.com>

> Subject: Re: [PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed
> I/O PPS generator
> 

> > If you are willing to stop the inclusion due this fact maybe its time to add
> > such PPS generators interface... on the other hand, if you agree for
> > inclusion we can do this job as soon as the code has been included, in order
> > to fix this anomalous status.
> 
> Please make a generic pps subsystem for this, it would make it simpler
> for everyone.

What is required to move this? We can certainly test the code and do some
of this work, but I would look to you to define the interface.

Thanks,
Chris
Re: [PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed I/O PPS generator
Posted by Rodolfo Giometti 1 year, 2 months ago
On 25/09/24 23:55, Hall, Christopher S wrote:
> Hi Rodolfo,

Hello.

>> From: Greg KH <gregkh@linuxfoundation.org>
>> Sent: Tuesday, September 03, 2024 3:25 AM
>> To: Rodolfo Giometti <giometti@enneenne.com>
> 
>> Subject: Re: [PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed
>> I/O PPS generator
>>
> 
>>> If you are willing to stop the inclusion due this fact maybe its time to add
>>> such PPS generators interface... on the other hand, if you agree for
>>> inclusion we can do this job as soon as the code has been included, in order
>>> to fix this anomalous status.
>>
>> Please make a generic pps subsystem for this, it would make it simpler
>> for everyone.
> 
> What is required to move this? We can certainly test the code and do some
> of this work, but I would look to you to define the interface.

The problem is that a pps-generator has no related device, then no sysfs entries.

I think the right-thing(TM) to do is adding a new class named "pps-generator", 
so we will get the directory /sys/class/pps-generator with several devices as 
pps-generator0, pps-generator1, etc. For each device we should add at least 
these sysfs entries:

- system       : This file return "1" if the generator takes the timing from
                  the system clock, while it returns "0" if not (i.e. from a
                  peripheral device clock).

- time         : This file contains the current time stored into the generator
                  clock as two integers representing the current time seconds and
                  nanoseconds.

- enable       : This write-only file enables or disables generation of the
                  PPS signal.

- period       : This file defines the period for the generator signal. When
                  read, by default, it returns "1 0" which represent the period
                  second and nanoseconds (i.e. a PPS signal). When written, it
                  sets the period accordingly or ENOTSUPP if not supported.

- start_time   : This file defines the starting time for the the generator
                  signal. When read, by default, it returns "0 0" which means
                  "now". When written, it sets the starting time accordingly or
                  ENOTSUPP if not supported.

Please, let me know what do you think about this proposal. I'm going to provide 
a draft in the next weeks since I'm stuck on another project right now. :(

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming
Re: [PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed I/O PPS generator
Posted by Greg KH 1 year, 2 months ago
On Thu, Sep 26, 2024 at 10:46:54AM +0200, Rodolfo Giometti wrote:
> On 25/09/24 23:55, Hall, Christopher S wrote:
> > Hi Rodolfo,
> 
> Hello.
> 
> > > From: Greg KH <gregkh@linuxfoundation.org>
> > > Sent: Tuesday, September 03, 2024 3:25 AM
> > > To: Rodolfo Giometti <giometti@enneenne.com>
> > 
> > > Subject: Re: [PATCH v12 2/3] Documentation: driver-api: pps: Add Intel Timed
> > > I/O PPS generator
> > > 
> > 
> > > > If you are willing to stop the inclusion due this fact maybe its time to add
> > > > such PPS generators interface... on the other hand, if you agree for
> > > > inclusion we can do this job as soon as the code has been included, in order
> > > > to fix this anomalous status.
> > > 
> > > Please make a generic pps subsystem for this, it would make it simpler
> > > for everyone.
> > 
> > What is required to move this? We can certainly test the code and do some
> > of this work, but I would look to you to define the interface.
> 
> The problem is that a pps-generator has no related device, then no sysfs entries.
> 
> I think the right-thing(TM) to do is adding a new class named
> "pps-generator", so we will get the directory /sys/class/pps-generator with
> several devices as pps-generator0, pps-generator1, etc. For each device we
> should add at least these sysfs entries:
> 
> - system       : This file return "1" if the generator takes the timing from
>                  the system clock, while it returns "0" if not (i.e. from a
>                  peripheral device clock).
> 
> - time         : This file contains the current time stored into the generator
>                  clock as two integers representing the current time seconds and
>                  nanoseconds.
> 
> - enable       : This write-only file enables or disables generation of the
>                  PPS signal.
> 
> - period       : This file defines the period for the generator signal. When
>                  read, by default, it returns "1 0" which represent the period
>                  second and nanoseconds (i.e. a PPS signal). When written, it
>                  sets the period accordingly or ENOTSUPP if not supported.
> 
> - start_time   : This file defines the starting time for the the generator
>                  signal. When read, by default, it returns "0 0" which means
>                  "now". When written, it sets the starting time accordingly or
>                  ENOTSUPP if not supported.

This seems sane to me, thanks for writing it up!

greg k-h