MSPM0: RTC Support #90844
MSPM0: RTC Support #90844
Conversation
github-actions
bot
added
platform: Texas Instruments MSPM0
platform: TI SimpleLink
github-actions
bot
requested review from
bjarki-andreasen,
ssekar15,
vaishnavachath and
vanti
There was a problem hiding this comment.
Please add an overlay to and test out the driver with https://docs.zephyrproject.org/latest/hardware/peripherals/rtc.html#rtc-device-driver-test-suite :)
dts/bindings/rtc/ti,mspm0-rtc.yaml
Outdated
| compatible: "ti,mspm0-rtc" | ||
|
|
||
| include: | ||
| - name: rtc.yaml |
There was a problem hiding this comment.
only rtc-device.yaml should be included, rtc.yaml is an old relic :)
drivers/rtc/rtc_ti_mspm0.c
Outdated
|
|
||
| #define DT_DRV_COMPAT ti_mspm0_rtc | ||
|
|
||
| #include <errno.h> |
There was a problem hiding this comment.
not needed since its part of zephyr/kernel.h :)
| timeptr->tm_year = DL_RTC_Common_getCalendarYearBinary(cfg->base); | ||
| timeptr->tm_wday = DL_RTC_Common_getCalendarDayOfWeekBinary(cfg->base); | ||
| } | ||
|
|
There was a problem hiding this comment.
remember to initialize remaining fields tm_nsec = 0, and tm_isdst = -1
drivers/rtc/rtc_ti_mspm0.c
Outdated
| __disable_irq(); | ||
| ret = alarm->is_pending ? 1 : 0; | ||
| alarm->is_pending = false; | ||
| __enable_irq(); |
There was a problem hiding this comment.
disable IRQ is not needed here as that is what a spinlock does, so you are already in a critical section here because of line 349 :)
There was a problem hiding this comment.
also, this is not tracking (they key part), so it seems it could break the kernel by not restoring interrupt mask to correct state :) I have not seen __disable_irq() before
drivers/rtc/rtc_ti_mspm0.c
Outdated
| { | ||
| #if defined(CONFIG_RTC_ALARM) | ||
| uint8_t id; | ||
| uint32_t key = irq_lock(); |
There was a problem hiding this comment.
Use spinlock if needed :)
drivers/rtc/rtc_ti_mspm0.c
Outdated
| alarm->is_pending = true; | ||
| if (alarm->callback) { | ||
| alarm->callback(dev, id, alarm->user_data); | ||
| } |
There was a problem hiding this comment.
Its safer to store the callback and user data in a local variable, reenable irqs as critical section is no longer needed, then call the callback. This way the callback is not called with irqs disabled (or more appropriately spinlock held) :)
drivers/rtc/rtc_ti_mspm0.c
Outdated
| .base = (RTC_Regs *)DT_INST_REG_ADDR(n), \ | ||
| .rtc_x = DT_INST_PROP(n, ti_rtc_x), \ | ||
| .irq_config_func = ti_mspm0_config_irq_##n, \ |
There was a problem hiding this comment.
alingment here should be
static type sym = {
.member = foo,
};
dts/arm/ti/mspm0/mspm0.dtsi
Outdated
| reg = <0x40094000 0x2000>; | ||
| interrupts = <30 0>; | ||
| prescaler = <1>; | ||
| clock-frequency = <32768>; |
There was a problem hiding this comment.
missing alarms-count = <2>; property inhereted from rtc-device.yaml :)
add Real-Time Clock binding for Texas Instruments MSPM0 Family Signed-off-by: Karthikeyan Krishnasamy <karthikeyan@linumiz.com>
added Real-Time Clock driver support for Texas Instruments MSPM0 Family Signed-off-by: Karthikeyan Krishnasamy <karthikeyan@linumiz.com>
Add a rtc support for mspm0 Signed-off-by: Karthikeyan Krishnasamy <karthikeyan@linumiz.com>
Add rtc overlay for lp_mspm0g3507 to enable test coverage Signed-off-by: Karthikeyan Krishnasamy <karthikeyan@linumiz.com>
karthi012
force-pushed
the
upstream/ti/mspm0-rtc
branch
from
4e49499 to
bb57eaa
Compare
|
|
@bjarki-andreasen Updated the changes. Please have a look |
This PR add real time clock support for Texas Instruments MSPM0 Family Series