[PATCH 6/7] score: Always validate ticks in _TOD_Validate()

[Sebastian Huber]

On 07/09/2021 05:11, Chris Johns wrote:
> On 6/9/21 6:11 pm, Sebastian Huber wrote:
>> On 06/09/2021 09:55, Chris Johns wrote:
>>> On 6/9/21 3:49 pm, Sebastian Huber wrote:
>>>> On 04/09/2021 06:20, Joel Sherrill wrote:
>>>>>        > -    sc = _TOD_Validate(&temp_tod, TOD_ENABLE_TICKS_VALIDATION);
>>>>>        > +    sc = _TOD_Validate(&temp_tod);
>>>>>
>>>>>       This has leaked out of the internal implementation interface. Should it?
>>>>>
>>>>>       I prefer this does not happen and BSPs use an API.
>>>>>
>>>>>
>>>>> I noticed this also. It needs an API.
>>>> There is no RTEMS API to support writing realtime clock drivers currently.
>>> Where did the RTC API connection comes from? I was only thinking of a call in
>>> `rtems/clock.h` to validate time. I would not expect an RTC API to result in the
>>> validate call being moved from where it is.
>>
>> This is not the only _TOD_* stuff used by RTC drivers.
> 
> A check shows there are 2 calls used and the bfin bsp can be ignored. Maybe I am
> missing something here. Am I?

Yes, you missed the other uses of _TOD_* stuff:

bsps/arm/lpc32xx/rtc/rtc-config.c:  lpc32xx_rtc_set(_TOD_To_seconds(tod));
bsps/shared/dev/rtc/ds1375.c:  secs = _TOD_To_seconds( time );
bsps/shared/dev/rtc/ds1375.c:  secs = _TOD_To_seconds( &rtod );
bsps/shared/dev/rtc/rtc-support.c:  rtems_time = _TOD_To_seconds( 
&rtems_tod );
bsps/shared/dev/rtc/rtc-support.c:  rtc_time = _TOD_To_seconds( &rtc_tod );
bsps/bfin/shared/dev/rtc.c: 
_TOD_Days_to_date[1][tod_temp.month] + tod_temp.day - 1;
bsps/bfin/shared/dev/rtc.c:    if (days <= 
_TOD_Days_to_date[Leap_year][n+1]) {
bsps/bfin/shared/dev/rtc.c:      tod_temp.day = days - 
_TOD_Days_to_date[Leap_year][n];
bsps/bfin/shared/dev/rtc.c:  rtems_time = _TOD_To_seconds( &rtems_tod );
bsps/bfin/shared/dev/rtc.c:  rtc_time = _TOD_To_seconds( &rtc_tod );

> 
>>>> I am not that sure if this is really a high priority issue.
>>> If you mean an RTC API, sure I agree with that. If this is about dipping
>>> directly into the score, I do not agree, it is always a priority.
>>
>> The _TOD_* stuff is probably used for decades in RTC drivers. Adding RTEMS API
>> functions is a bit of work and all core developers are busy. I think it is
>> unrealistic that someone will have time to work on this topic.
> 
> I think our goals are not aligned. I am only asking the call be protected by an
> API signature placed in rtems/clock.h. Is there a problem with doing this?
> 
> /**
>   * @brief Validates the time of day.
>   *
>   * @param tod is the reference to the time of day structure to validate or
>   *   NULL.
>   *
>   * @retval RTEMS_SUCCESSFUL @a tod references a valid time of day.
>   * @retval RTEMS_INVALID_CLOCK @a tod references an invalid time of day.
>   * @retval RTEMS_INVALID_ADDRESS @a tod reference is @c NULL.
>   */
> rtems_status rtems_time_of_day_validate(rtems_time_of_day* tod);
> 
> And the implementation is:
> 
> rtems_status rtems_time_of_day_validate(
>    rtems_time_of_day* tod
> )
> {
>    return _TOD_Validate(tod);
> }

The RTEMS API should be documented in the Classic API Guide. There 
should be also separate tests for API functions. API functions should be 
stable and cover important use cases of applications. I am really not 
sure if a function used only by two device drivers should be moved to 
the API.

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