Drivers: RTC: Initial implementation of RTC for IFX cyw920829m2evk_02 platform #79343
Conversation
mcatee-infineon
commented
Oct 2, 2024
mcatee-infineon
commented
zephyrbot
added
area: RTC
mcatee-infineon
force-pushed
the
ifx-cat1-rtc-driver
branch
from
526d612 to
6a929ec
Compare
drivers/rtc/rtc_ifx_cat1.c
Outdated
| if (Cy_RTC_IsExternalResetOccurred()) { | ||
| /* Reset to default time */ | ||
| Cy_RTC_SetDateAndTime(&default_time); | ||
| _ifx_cat1_rtc_set_century(_IFX_CAT1_RTC_INIT_CENTURY); | ||
| } |
There was a problem hiding this comment.
The expected behavior of the RTC is to return -ENODATA until the time has actually been set to a valid value, rather than defaulting to a pseudo random value.
The init function should just do the minimal amount of init, like configuring the IRQ. Setting the time is done with rtc_set_time() at which point the RTC's time will be initialized to a valid value
|
Just wanted to say thanks to @bjarki-andreasen for the detailed review! |
happy to help :) |
|
@bjarki-andreasen and @npal-cy, thank you for your feedback. I have pushed up updates that should address your concerns. I have done so in the form of a second commit so that changes are clear. If you both find the new version acceptable, I will then be sure to squash the commits together. Once again, thank you for your feedback. |
mcatee-infineon
force-pushed
the
ifx-cat1-rtc-driver
branch
4 times, most recently
from
a963e4a to
2a3ffc5
Compare
drivers/rtc/rtc_ifx_cat1.c
Outdated
| @@ -87,7 +93,8 @@ struct ifx_cat1_rtc_config { | |||
| }; | |||
|
|
|||
| struct ifx_cat1_rtc_data { | |||
| struct k_mutex lock; | |||
| struct k_spinlock lock; | |||
| bool valid_time_set; | |||
There was a problem hiding this comment.
I believe this var should be replaced by checking the state of the RTC by looking at its registers, a quick search for a HAL for the RTC https://infineon.github.io/mtb-hal-cat1/html/group__group__hal__rtc.html#gac6891d0e8b9de6c32f46f179aa32b800 (I could not find a reference manual) does indicate the RTC can survive a system reset, and it is possible to check if the time has been set and RTC is counting. By reading the RTC, if the time was set before the system was reset, rtc_get_time() would work after reset without setting the time, which is not possible if the valid_time_set is stored in RAM, since it is reset every time
There was a problem hiding this comment.
@bjarki-andreasen, I have squashed the old verification commit and replaced it with a new one for review. It should resolve this concern.
mcatee-infineon
force-pushed
the
ifx-cat1-rtc-driver
branch
from
2a3ffc5 to
5d210c4
Compare
mcatee-infineon
force-pushed
the
ifx-cat1-rtc-driver
branch
2 times, most recently
from
82e872d to
186e9ce
Compare
drivers/rtc/rtc_ifx_cat1.c
Outdated
| struct ifx_cat1_rtc_config { | ||
| uint32_t irqn; | ||
| }; |
There was a problem hiding this comment.
this not not actually used, you can remove it, along with its instance in the CAT1_RTC_INIT macro :) save a whole 4 bytes of memory :D
There was a problem hiding this comment.
Great! Thank you @bjarki-andreasen. This final version of the "verification commit" should resolve this. As for calibration, the test has been ran and the results were as expected.
- Initial driver implementation - Overlay for rtc_api test - dtsi updates. Signed-off-by: McAtee Maxwell (CSS ICW SW MTO INT 2) <[email protected]>
- verify code changes from PR review Signed-off-by: McAtee Maxwell <[email protected]>
mcatee-infineon
force-pushed
the
ifx-cat1-rtc-driver
branch
from
186e9ce to
a11b489
Compare
