am243x_evm/am2434/r50_0: configure timer clock source#101283
Conversation
Write the protected MMR region during early init to select the correct clock source for the specified timer. To get the correct offset, recognize the timer currently in use via the DT alias system-timer. Then we do some soc-specific calculations to configure this source. This can be removed once there are clock control APIs to set parent sources in place. Signed-off-by: Amneesh Singh <amneesh@ti.com>
zephyrbot
added
area: Boards/SoCs
platform: TI K3
| #define MAIN_PADCFG_BASE (0xf0000) | ||
| #define MAIN_MMR_BASE (0x43000000) | ||
| #define MAIN_MMR_TIMER_CLKSEL_PART CTRL_PARTITION(MAIN_MMR_BASE, 2) | ||
| #define MAIN_MMR_TIMER_CLKSEL_VAL (0x0) /* HFOSC0_CLKOUT */ |
There was a problem hiding this comment.
how about r50_1? Do we need to have separate the handling for it?
also the question here is, the parent is same for all timers, i think timer source macro can be irrespective of config.
because there is same set of parents for all timers.
There was a problem hiding this comment.
how about r50_1? Do we need to have separate the handling for it?
When R50_1 support will be added, we will have a common option for all cores - something along the lines of CONFIG_SOC_AM2434_R5F, which would apply for all cores. That can be used here then.
also the question here is, the parent is same for all timers
The parent is same but the register offset for every timer is different, which is why we are getting data from DT to see which timer instance is being used.
There was a problem hiding this comment.
Understood, i think i was worried about so many custom conf macros to handle combinations, this is fine for now, may be a path ahead to use a syscon driver.
There was a problem hiding this comment.
It might be a good time to add the other R5F cores for AM243x, just to make sure we are not making any bad assumptions that only apply to R5F0_0 (even if that might not be the case this time). AM64x has all the R5F cores, no reason to not have them for AM243x.
There was a problem hiding this comment.
That is true, will do in another patchset
| aliases { | ||
| led0 = &ld26; | ||
| adc0 = &main_adc0; | ||
| system-timer = &main_timer8; |
There was a problem hiding this comment.
The way the system timer is selected currently is by being the 0th enabled timer instance. So as ugly as it would be you could also match this logic by doing DT_INST(inst, ti_am654_timer) to find the timer instance used as the system timer without adding this alias.
That said, we need to fix how we select the system timer, and it was probably going to be by adding an alias like this above. So this is fine.
There was a problem hiding this comment.
Hi, apologies, I was on vacation - yes, we can also use instance 0 to get the selected timer, and I 100% agree that this is ugly. As a note, we need two things in the future -
- A syscon mechanism that can be used for modifying control partitions and get rid of this file. Pinmuxing would also utilize this very driver.
- A set_parent clock API in the driver subsystem
As for the alias name I thought system-timer would fit. If there are any other suggestions, please tell.
|
can you rebase please? |
Hi, apologies, but I will be closing this in favor of a solution based on - #103330 Thanks |
5 participants
Write the protected MMR region during early init to select the correct clock source for the specified timer. To get the correct offset, recognize the timer currently in use via the DT alias system-timer. Then we do some soc-specific calculations to configure this source. This can be removed once there are clock control APIs to set parent sources in place.