Samsung / TizenRT

TizenRT is a lightweight RTOS-based platform to support low-end IoT devices
Apache License 2.0
562 stars 561 forks source link

os/arch/arm/src/amebasmart: Amebasmart rtc fix #6214

Closed kaikaikaime closed 1 month ago

kaikaikaime commented 1 month ago
abhishek-samsung commented 1 month ago

I applied your patch on my test code and still the values are same as before and it seems to not work. Could you also please explain how changing the name would fix this issue?

branch : https://github.com/abhishek-samsung/TizenRT/tree/rtc_issue_updated config : flat_dev command : rtc

TASH>>rtc
TASH>>=== RTC Accuracy Test ===
amebasmart_rdtime: value fo timer : 4294967295 <--- same value is repeated
rtc_ioctl: tm_sec : 15, tm_min : 28
[Total 0m passed] Time Difference : 0s
amebasmart_rdtime: value fo timer : 4294967295
rtc_ioctl: tm_sec : 15, tm_min : 28
[Total 0m passed] Time Difference : 0s
amebasmart_rdtime: value fo timer : 4294967295
rtc_ioctl: tm_sec : 15, tm_min : 28
[Total 0m passed] Time Difference : 0s
amebasmart_rdtime: value fo timer : 4294967295
rtc_ioctl: tm_sec : 15, tm_min : 28
[Total 0m passed] Time Difference : 0s
kaikaikaime commented 1 month ago

Dear Abhishek,

Upon further inspection, this is found that the problem lies in the usage parameter for the example of rtc_main. Without the changes of this PR:

The lowest date allowed is : 1970-1-1 reference: rtc man pages

Therefore the parameters at minimum should be:

prev_time.tm_year = 70; // 1970 - 1900
prev_time.tm_mday = 1; //1~31
prev_time.tm_mon = 0;  //0~11 

instead of memset everything to 0.

I can revert the changes of 'rtk_' as this does not fix the current issue. Instead shall we add a warning or an assert if the dates set is below this set of values?

Chuen Kai

abhishek-samsung commented 1 month ago

Dear Abhishek,

Upon further inspection, this is found that the problem lies in the usage parameter for the example of rtc_main. Without the changes of this PR:

The lowest date allowed is : 1970-1-1 reference: rtc man pages

Therefore the parameters at minimum should be:

prev_time.tm_year = 70; // 1970 - 1900
prev_time.tm_mday = 1; //1~31
prev_time.tm_mon = 0;  //0~11 

instead of memset everything to 0.

I can revert the changes of 'rtk_' as this does not fix the current issue. Instead shall we add a warning or an assert if the dates set is below this set of values?

Chuen Kai

Thanks for checking with the test code. After applying the initialization that you provided for prev_time, rtc test seems to run fine. However, with rtl8721csm, the example works fine without the prev_time init. Could you let us know why there is a difference?

branch : https://github.com/abhishek-samsung/TizenRT/tree/rtc_issue_updated config : rtl8721csm/hello

TASH>>rtc
TASH>>=== RTC Accuracy Test ===
rtc_ioctl: tm_sec : 1, tm_min : 0
[Total 0m passed] Time Difference : 1s
rtc_ioctl: tm_sec : 2, tm_min : 0
[Total 0m passed] Time Difference : 1s
rtc_ioctl: tm_sec : 3, tm_min : 0
[Total 0m passed] Time Difference : 1s
rtc_ioctl: tm_sec : 4, tm_min : 0
[Total 0m passed] Time Difference : 1s
rtc_ioctl: tm_sec : 5, tm_min : 0
kaikaikaime commented 1 month ago

rtl8721csm does not have a year (date) input, therefore 0 will be acceptable. Since rtl8730e has added the year (date) parameter in the rtc, it is required to conform to 1970-1-1 restriction.

sunghan-chang commented 1 month ago
config RTC_DATETIME
    bool "Date/Time RTC Support"
    default n
    ---help---
        There are two general types of RTC:  (1) A simple battery backed
        counter that keeps the time when power is down, and (2) a full
        date / time RTC the provides the date and time information, often in
        BCD format.  If RTC_DATETIME is selected, it specifies this second kind
        of RTC. 

if !RTC_DATETIME

config RTC_HIRES
    bool "Hi-Res RTC Support"

We can use two types of RTCs like above. @kaikaikaime You mean RTL8730E is RTC_DATETIME and RTL8721CSM is RTC_HIRES ?

kaikaikaime commented 1 month ago

Dear Chang & Abhishek,

Both RTL8730E & RTL8721CSM used datetime format only. The difference is that RTL8730E has a additional field for year thus imposing additional restriction. Both RTL8730E & RTL8721CSM are not RTC_HIRES.

Regards, Chuen Kai

sunghan-chang commented 1 month ago

@abhishek-samsung @kaikaikaime We can't get the board config at app side which uses RTC. Then

  1. introduce new common config for year. But it looks strange.
  2. just add year value when we call the RTC api for all boards. Then RTL8721CSM is ok? It just ignore year value?
  3. Any other suggestion?
kaikaikaime commented 1 month ago

Dear Chang & Abhishek,

This is the conclusion I gathered:

  1. After Turning on Device Drivers->"RTC Device Support". mktime API used to process rtc_time struct in lib_mktime.c value of 99 (1999) tm_year automatically will be added. Both RTL8730E & RTL8721CSM can run.

  2. In addition of using built-in libraries-> "Localtime API call support" mktime API used to process rtc_time struct in lib_localtime.c tm_year is 0, due to memset 0 from rtc_main.c Both RTL8730E & RTL8721CSM will not able to start. Only by adding 1970-1-1 in rtc_main.c, both RTL8730E & RTL8721CSM will be able to run normally.

  3. RTL8721CSM does not ignore the year value, as it still counts value of leap year etc or lib_mktime was used previously (99 was added).

Regards, Chuen Kai

sunghan-chang commented 1 month ago

@r-prabu Please check this.

sunghan-chang commented 1 month ago

@kaikaikaime Will merge @abhishek-samsung 's PR #6242 . Let's close this. But rtc read without local time config looks wrong. @abhishek-samsung will send you a mail. Please check it.