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

From: Greg KH
Date: Thu Sep 26 2024 - 05:35:25 EST


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@xxxxxxxxxxxxxxxxxxx>
> > > Sent: Tuesday, September 03, 2024 3:25 AM
> > > To: Rodolfo Giometti <giometti@xxxxxxxxxxxx>
> >
> > > 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