AW: [PATCH 12/12] spec/build/cpukit: Add option for enabling PPS synchronization

[Gabriel.Moyano at dlr.de]

> On 07/04/2022 10:36, Gabriel Moyano wrote:
> > ---
> >   spec/build/cpukit/cpuopts.yml    |  2 ++
> >   spec/build/cpukit/optppssync.yml | 16 ++++++++++++++++
> >   2 files changed, 18 insertions(+)
> >   create mode 100644 spec/build/cpukit/optppssync.yml
> >
> > diff --git a/spec/build/cpukit/cpuopts.yml
> > b/spec/build/cpukit/cpuopts.yml index 301d49ccea..db0e05395f 100644
> > --- a/spec/build/cpukit/cpuopts.yml
> > +++ b/spec/build/cpukit/cpuopts.yml
> > @@ -49,6 +49,8 @@ links:
> >     uid: optparavirt
> >   - role: build-dependency
> >     uid: optposix
> > +- role: build-dependency
> > +  uid: optppssync
> >   - role: build-dependency
> >     uid: optprofiling
> >   - role: build-dependency
> > diff --git a/spec/build/cpukit/optppssync.yml
> > b/spec/build/cpukit/optppssync.yml
> > new file mode 100644
> > index 0000000000..6766cf0d40
> > --- /dev/null
> > +++ b/spec/build/cpukit/optppssync.yml
> > @@ -0,0 +1,16 @@
> > +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> > +actions:
> > +- get-boolean: null
> > +- env-enable: null
> > +- define-condition: null
> > +build-type: option
> > +copyrights:
> > +- Copyright (C) 2022 German Aerospace Center (DLR)
> > +default: false
> > +default-by-variant: []
> > +description: |
> > +  Enable time synchronization using a Pulse Per Second signal
> > +enabled-by: true
> > +links: []
> > +name: PPS_SYNC
> > +type: build
> 
> If this should really be a CPU option, then it needs a proper RTEMS_* prefix. 

Do you mean that the name should be RTEMS_PPS_SYNC?

> Why do we need the option. What is the overhead if it is always enabled.

The PPS_SYNC option comes from FREEBSD. Without it, you can still use the API. For example a task can wait for an event calling time_pps_fetch().
But if it is not enabled, hardpps() isn't called and time_freq is not updated.

> In general, the patch set lacks test cases.

I was thinking that the next step could be to add a generic device which is required to use the API (a file descriptor is needed). This is something that wanted to ask in the mailing list first. When this device is added, also the test cases can be. Furthermore I wanted to split the submitted changes in order to make a review easier.