On 7/9/21 3:34 pm, Sebastian Huber wrote:
> 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 );
>
This is what I saw and it not much that needs to be added to the header file.
>>
>>>>> 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.
>
static void test_leap_year(void)
{
rtems_status_code test_status;
const rtems_time_of_day *problem = &problem_2100;
const rtems_time_of_day *problem2 = &problem_2100_2;
// 2100 is not a leap year, so it should have 28 days
test_status = rtems_time_of_day_validate(problem);
rtems_test_assert(test_status == RTEMS_SUCCESSFUL);
test_status = rtems_time_of_day_validate(problem2);
rtems_test_assert(test_status == RTEMS_INVALID_CLOCK);
}
?
Chris
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
