Skip to content

am243x_evm/am2434/r50_0: configure timer clock source #101283

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.

Sign up for GitHub

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

Closed
natto1784 wants to merge 1 commit into from
Closed

am243x_evm/am2434/r50_0: configure timer clock source #101283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions boards/ti/am243x_evm/am243x_evm_am2434_r5f0_0.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
aliases {
led0 = &ld26;
adc0 = &main_adc0;
system-timer = &main_timer8;
Copy link
Copy Markdown
Contributor

@glneo glneo Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@natto1784 natto1784 Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 -

  1. A syscon mechanism that can be used for modifying control partitions and get rid of this file. Pinmuxing would also utilize this very driver.
  2. 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.

};

leds: leds {
Expand Down
48 changes: 39 additions & 9 deletions soc/ti/k3/am6x/common/ctrl_partitions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/devicetree.h>
#include <zephyr/sys/util.h>
#include <zephyr/sys/device_mmio.h>
#include <zephyr/arch/common/sys_io.h>
Expand All @@ -22,8 +23,14 @@
#elif defined CONFIG_SOC_AM6234_M4 | defined CONFIG_SOC_AM6232_M4
#define WKUP_PADCFG_BASE (0x4080000)
#elif defined CONFIG_SOC_AM2434_R5F0_0
#define MCU_PADCFG_BASE (0x4080000)
#define MAIN_PADCFG_BASE (0xf0000)
#define DT_TIMER_INST \
((DT_REG_ADDR(DT_ALIAS(system_timer)) - DT_REG_ADDR(DT_NODELABEL(main_timer0))) / 0x10000)
#define MCU_PADCFG_BASE (0x4080000)
#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 */
Copy link
Copy Markdown
Contributor

@soumya-TI soumya-TI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@natto1784 natto1784 Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@soumya-TI soumya-TI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@glneo glneo Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@natto1784 natto1784 Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true, will do in another patchset

#define MAIN_MMR_TIMER_CLKSEL (MAIN_MMR_BASE + 0x81B0 + (DT_TIMER_INST * 4))
#endif

static const uintptr_t ctrl_partitions[] = {
Expand All @@ -41,16 +48,39 @@ static const uintptr_t ctrl_partitions[] = {
#endif
};

void k3_unlock_all_ctrl_partitions(void)
void k3_write_mmr(uint32_t value, mm_reg_t base_addr)
{
ARRAY_FOR_EACH(ctrl_partitions, i) {
mm_reg_t base_addr = ctrl_partitions[i];

#ifdef DEVICE_MMIO_IS_IN_RAM
device_map(&base_addr, base_addr, sizeof(base_addr), K_MEM_CACHE_NONE);
device_map(&base_addr, base_addr, sizeof(base_addr), K_MEM_CACHE_NONE);
#endif

sys_write32(KICK0_UNLOCK_VAL, base_addr + KICK0_OFFSET);
sys_write32(KICK1_UNLOCK_VAL, base_addr + KICK1_OFFSET);
sys_write32(value, base_addr);
}

void k3_unlock_partition(mm_reg_t base_addr)
{
k3_write_mmr(KICK0_UNLOCK_VAL, base_addr + KICK0_OFFSET);
k3_write_mmr(KICK1_UNLOCK_VAL, base_addr + KICK1_OFFSET);
}

void k3_lock_partition(mm_reg_t base_addr)
{
k3_write_mmr(KICK_LOCK_VAL, base_addr + KICK0_OFFSET);
k3_write_mmr(KICK_LOCK_VAL, base_addr + KICK1_OFFSET);
}

void k3_unlock_all_ctrl_partitions(void)
{
ARRAY_FOR_EACH(ctrl_partitions, i) {
k3_unlock_partition(ctrl_partitions[i]);
}

/* Currently setting clock parent is not possible via standard clock control API,
* hence do this for now
*/
#ifdef MAIN_MMR_TIMER_CLKSEL
k3_unlock_partition(MAIN_MMR_TIMER_CLKSEL_PART);
k3_write_mmr(MAIN_MMR_TIMER_CLKSEL_VAL, MAIN_MMR_TIMER_CLKSEL);
k3_lock_partition(MAIN_MMR_TIMER_CLKSEL_PART);
#endif
}
Loading