Rework Xilinx TTC driver #23567
+267
−162
There are no files selected for viewing
Validating CODEOWNERS rules …
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,5 +28,5 @@ | |
|
|
||
| &ttc0 { | ||
| status = "okay"; | ||
| clock-frequency = <12000000>; | ||
| }; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,196 +1,195 @@ | ||
| /* | ||
| * Copyright (c) 2020 Stephanos Ioannidis <root@stephanos.io> | ||
| * Copyright (c) 2018 Xilinx, Inc. | ||
| * | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| #include <soc.h> | ||
| #include <drivers/timer/system_timer.h> | ||
| #include "xlnx_psttc_timer_priv.h" | ||
|
|
||
| #define TIMER_INDEX CONFIG_XLNX_PSTTC_TIMER_INDEX | ||
| #define TIMER_DT(v) UTIL_CAT(UTIL_CAT(DT_INST_, TIMER_INDEX), _##v) | ||
|
|
||
| #define TIMER_IRQ TIMER_DT(XLNX_TTCPS_IRQ_0) | ||
| #define TIMER_BASE_ADDR TIMER_DT(XLNX_TTCPS_BASE_ADDRESS) | ||
| #define TIMER_CLOCK_FREQUECY TIMER_DT(XLNX_TTCPS_CLOCK_FREQUENCY) | ||
|
|
||
| #define TICKS_PER_SEC CONFIG_SYS_CLOCK_TICKS_PER_SEC | ||
|
There was a problem hiding this comment. Style: unless this is needed to keep line lengths under control, I'd prefer to see the existing kconfig used directly instead of a local synonym. |
||
| #define CYCLES_PER_SEC TIMER_CLOCK_FREQUECY | ||
|
There was a problem hiding this comment. Ditto, though I guess DTS constants are really ugly, so I feel less strongly about this one. There was a problem hiding this comment. yes, they get very long and ugly soon after this. |
||
| #define CYCLES_PER_TICK (CYCLES_PER_SEC / TICKS_PER_SEC) | ||
|
|
||
| /* | ||
| * CYCLES_NEXT_MIN must be large enough to ensure that the timer does not miss | ||
| * interrupts. This value was conservatively set using the trial and error | ||
| * method, and there is room for improvement. | ||
| */ | ||
| #define CYCLES_NEXT_MIN (10000) | ||
stephanosio marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| #define CYCLES_NEXT_MAX (XTTC_MAX_INTERVAL_COUNT) | ||
|
|
||
| BUILD_ASSERT_MSG(TIMER_DT(XLNX_TTCPS_CLOCK_FREQUENCY) == | ||
| CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC, | ||
| "Configured system timer frequency does not match the TTC " | ||
| "clock frequency in the device tree"); | ||
|
|
||
| BUILD_ASSERT_MSG(CYCLES_PER_SEC >= TICKS_PER_SEC, | ||
| "Timer clock frequency must be greater than the system tick " | ||
| "frequency"); | ||
|
|
||
| BUILD_ASSERT_MSG((CYCLES_PER_SEC % TICKS_PER_SEC) == 0, | ||
| "Timer clock frequency is not divisible by the system tick " | ||
| "frequency"); | ||
|
|
||
| #ifdef CONFIG_TICKLESS_KERNEL | ||
| static u32_t last_cycles; | ||
| #endif | ||
|
|
||
| static u32_t read_count(void) | ||
| { | ||
| /* Read current counter value */ | ||
| return sys_read32(TIMER_BASE_ADDR + XTTCPS_COUNT_VALUE_OFFSET); | ||
| } | ||
|
|
||
| static void update_match(u32_t cycles, u32_t match) | ||
| { | ||
| u32_t delta = match - cycles; | ||
|
|
||
| /* Ensure that the match value meets the minimum timing requirements */ | ||
| if (delta < CYCLES_NEXT_MIN) { | ||
| match += CYCLES_NEXT_MIN - delta; | ||
| } | ||
|
|
||
| /* Write counter match value for interrupt generation */ | ||
| sys_write32(match, TIMER_BASE_ADDR + XTTCPS_MATCH_0_OFFSET); | ||
| } | ||
|
|
||
| static void ttc_isr(void *arg) | ||
| { | ||
| u32_t cycles; | ||
| u32_t ticks; | ||
|
|
||
| ARG_UNUSED(arg); | ||
|
|
||
| /* Acknowledge interrupt */ | ||
| sys_read32(TIMER_BASE_ADDR + XTTCPS_ISR_OFFSET); | ||
|
|
||
| /* Read counter value */ | ||
| cycles = read_count(); | ||
|
|
||
| #ifdef CONFIG_TICKLESS_KERNEL | ||
| /* Calculate the number of ticks since last announcement */ | ||
| ticks = (cycles - last_cycles) / CYCLES_PER_TICK; | ||
|
|
||
| /* Update last cycles count */ | ||
| last_cycles = cycles; | ||
| #else | ||
| /* Update counter match value for the next interrupt */ | ||
| update_match(cycles, cycles + CYCLES_PER_TICK); | ||
|
|
||
| /* Advance tick count by 1 */ | ||
| ticks = 1; | ||
| #endif | ||
|
|
||
| /* Announce to the kernel*/ | ||
| z_clock_announce(ticks); | ||
| } | ||
|
|
||
| int z_clock_driver_init(struct device *device) | ||
| { | ||
| u32_t reg_val; | ||
|
|
||
| /* Stop timer */ | ||
| sys_write32(XTTCPS_CNT_CNTRL_DIS_MASK, | ||
| TIMER_BASE_ADDR + XTTCPS_CNT_CNTRL_OFFSET); | ||
|
|
||
| #ifdef CONFIG_TICKLESS_KERNEL | ||
| /* Initialise internal states */ | ||
| last_cycles = 0; | ||
| #endif | ||
|
|
||
| /* Initialise timer registers */ | ||
| sys_write32(XTTCPS_CNT_CNTRL_RESET_VALUE, | ||
| TIMER_BASE_ADDR + XTTCPS_CNT_CNTRL_OFFSET); | ||
| sys_write32(0, TIMER_BASE_ADDR + XTTCPS_CLK_CNTRL_OFFSET); | ||
| sys_write32(0, TIMER_BASE_ADDR + XTTCPS_INTERVAL_VAL_OFFSET); | ||
| sys_write32(0, TIMER_BASE_ADDR + XTTCPS_MATCH_0_OFFSET); | ||
| sys_write32(0, TIMER_BASE_ADDR + XTTCPS_MATCH_1_OFFSET); | ||
| sys_write32(0, TIMER_BASE_ADDR + XTTCPS_MATCH_2_OFFSET); | ||
| sys_write32(0, TIMER_BASE_ADDR + XTTCPS_IER_OFFSET); | ||
| sys_write32(XTTCPS_IXR_ALL_MASK, TIMER_BASE_ADDR + XTTCPS_ISR_OFFSET); | ||
|
|
||
| /* Reset counter value */ | ||
| reg_val = sys_read32(TIMER_BASE_ADDR + XTTCPS_CNT_CNTRL_OFFSET); | ||
| reg_val |= XTTCPS_CNT_CNTRL_RST_MASK; | ||
| sys_write32(reg_val, TIMER_BASE_ADDR + XTTCPS_CNT_CNTRL_OFFSET); | ||
|
|
||
| /* Set match mode */ | ||
| reg_val = sys_read32(TIMER_BASE_ADDR + XTTCPS_CNT_CNTRL_OFFSET); | ||
| reg_val |= XTTCPS_CNT_CNTRL_MATCH_MASK; | ||
| sys_write32(reg_val, TIMER_BASE_ADDR + XTTCPS_CNT_CNTRL_OFFSET); | ||
|
|
||
| /* Set initial timeout */ | ||
| reg_val = IS_ENABLED(CONFIG_TICKLESS_KERNEL) ? | ||
| CYCLES_NEXT_MAX : CYCLES_PER_TICK; | ||
| sys_write32(reg_val, TIMER_BASE_ADDR + XTTCPS_MATCH_0_OFFSET); | ||
|
|
||
| /* Connect timer interrupt */ | ||
|
There was a problem hiding this comment. Style: "IRQ_CONNECT(TIMER_IRQ, ..." sufficiently implies that the timer interrupt is connected without the comment. :) There was a problem hiding this comment. Just wanted to provide a logical flow of this function in the comments (i.e. even without reading actual code, you will know what this function does just only reading comments). Though, I agree that this would often qualify as a "state-the-obvious" kind of comment :p |
||
| IRQ_CONNECT(TIMER_IRQ, 0, ttc_isr, 0, 0); | ||
| irq_enable(TIMER_IRQ); | ||
|
|
||
| /* Enable timer interrupt */ | ||
| reg_val = sys_read32(TIMER_BASE_ADDR + XTTCPS_IER_OFFSET); | ||
| reg_val |= XTTCPS_IXR_MATCH_0_MASK; | ||
| sys_write32(reg_val, TIMER_BASE_ADDR + XTTCPS_IER_OFFSET); | ||
|
|
||
| /* Start timer */ | ||
| reg_val = sys_read32(TIMER_BASE_ADDR + XTTCPS_CNT_CNTRL_OFFSET); | ||
| reg_val &= (~XTTCPS_CNT_CNTRL_DIS_MASK); | ||
| sys_write32(reg_val, TIMER_BASE_ADDR + XTTCPS_CNT_CNTRL_OFFSET); | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| void z_clock_set_timeout(s32_t ticks, bool idle) | ||
| { | ||
| #ifdef CONFIG_TICKLESS_KERNEL | ||
| u32_t cycles; | ||
| u32_t next_cycles; | ||
|
|
||
| /* Read counter value */ | ||
| cycles = read_count(); | ||
|
|
||
| /* Calculate timeout counter value */ | ||
| if (ticks == K_FOREVER) { | ||
| next_cycles = cycles + CYCLES_NEXT_MAX; | ||
| } else { | ||
| next_cycles = cycles + ((u32_t)ticks * CYCLES_PER_TICK); | ||
| } | ||
|
|
||
| /* Set match value for the next interrupt */ | ||
| update_match(cycles, next_cycles); | ||
| #endif | ||
| } | ||
|
|
||
| u32_t z_clock_elapsed(void) | ||
| { | ||
| #ifdef CONFIG_TICKLESS_KERNEL | ||
| u32_t cycles; | ||
|
|
||
| /* Read counter value */ | ||
| cycles = read_count(); | ||
|
|
||
| /* Return the number of ticks since last announcement */ | ||
| return (cycles - last_cycles) / CYCLES_PER_TICK; | ||
| #else | ||
| /* Always return 0 for tickful operation */ | ||
| return 0; | ||
| #endif | ||
| } | ||
|
|
||
| u32_t z_timer_cycle_get_32(void) | ||
| { | ||
| /* Return the current counter value */ | ||
| return read_count(); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
No complaints here, though IIRC there are still some issues in the main -icount patch. Does this work for everything?
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.
This change is actually from #22904.
There is an issue with the WFI instruction in the Xilinx QEMU when icount mode is used, but this only affects one Zephyr test (tests/kernel/context) and does not really have a real impact in terms of functionality.
#22904 (comment)