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

[Sebastian Huber]

On 07/04/2022 11:57, Gabriel.Moyano at dlr.de wrote:
>> 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?

We should avoid new CPU options in general.

> 
>> 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.

Is something from PPS_SYNC used in the normal timekeeping?

> 
>> 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.

A file descriptor should be optional. It should be possible to add a PPS 
driver just using struct pps_state.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/