-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add RTC Support: No-OS API, Platform Ops, MAX32690 Driver & Example #2657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
/AzurePipelines run |
Azure Pipelines successfully started running 2 pipeline(s). |
e8354f1
to
0ded89f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other maxim MCUs also has RTC support. You may also want to add support for those MCUs here.
drivers/api/no_os_rtc.c
Outdated
|
||
if (!init_param->platform_ops->init) | ||
return -ENOSYS; | ||
// Initilize SPI descriptor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling. Also, don't use double slashes for comments. Use /* Initialize SPI descriptor */
// Initilize SPI descriptor | ||
ret = init_param->platform_ops->init(device, init_param); | ||
if (ret) | ||
return ret; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newline
drivers/api/no_os_rtc.c
Outdated
return -ENOSYS; | ||
return device->platform_ops->remove(device); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto: newline
* @param param - The structure that contains the RTC parameters. | ||
* @return 0 in case of success, -ERROR otherwise. | ||
*/ | ||
int32_t no_os_rtc_init(struct no_os_rtc_desc **device, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some platform APIs use int32_t
and some use int
. For consistency with recent drivers, return int
instead for all APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I retained usage of int32_t since the existing no_os_rtc.h in the main branch uses int32_t. I hope this is okay
return -EINVAL; | ||
if (!device->platform_ops->stop) | ||
return -ENOSYS; | ||
ret = device->platform_ops->stop(device); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto: newline
// Initialize the RTC | ||
ret = no_os_rtc_init(&rtc_desc, &rtc_ip); | ||
if (ret) { | ||
printf("RTC initialization failed.\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use pr_err
or pr_info
from no_os_print_log
. Do for all printf
instances.
printf("RTC IRQ controller initialization failed.\n"); | ||
return ret; | ||
} | ||
ret = no_os_irq_enable(rtc_irq_desc, 3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix indentation.
if (irq_flag){ | ||
no_os_gpio_set_value(led, NO_OS_GPIO_LOW); // Turn on the LED | ||
no_os_mdelay(5000); // Delay for 5 seconds | ||
ret = no_os_irq_register_callback(rtc_irq_desc, 3, &rtc_callback); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why re-register irq callback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed re-registration of callback function as it is unnecessary. Also, I moved the resetting of time count onto from the callback function to the main loop. Thanks for this.
return ret; | ||
} | ||
} | ||
else{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove braces
// print time | ||
ret = no_os_rtc_get_cnt(rtc_desc, &timer_cnt); | ||
if (ret) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inline brace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by running astyle. Thanks
Signed-off-by: Francisroi-Manabat_adi <[email protected]>
Signed-off-by: Francisroi-Manabat_adi <[email protected]>
V1 -> V2
|
Can the project be a generic RTC example project with multiple platforms supported instead? |
/AzurePipelines run |
Azure Pipelines successfully started running 2 pipeline(s). |
I believe that’s definitely feasible. I recently reviewed the RTC Alarm example in the STM32 IDE, and its structure closely aligns with this example project. Implementing CFLAGS in this example project will conveniently help it handle multiple platforms. However, I’ve noticed that platform drivers for RTC are currently unavailable for the other target platforms, which may require additional effort to implement. As for other Maxim MCUs, adding support for RTC should be relatively straightforward, considering the API naming conventions are quite similar. Ultimately, it will come down to testing on the actual hardware to ensure everything functions as expected. Thanks! |
Pull Request Description
This pull request introduces RTC (Real-Time Clock) support into the no-OS framework. The update includes the no-OS API implementation, platform operation hooks, MAX32690-specific RTC driver, and an RTC example to demonstrate usage of RTC alarm and integration.
PR Type
PR Checklist