Skip to content
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

cpu/qn908x: Add timer driver based on CTIMER. #15557

Merged
merged 2 commits into from Dec 5, 2020

Conversation

iosabi
Copy link
Contributor

@iosabi iosabi commented Dec 3, 2020

Contribution description

The QN908x CPU has several timer modules: one RTC (Real-Time Clock) that
can count from the 32kHz internal clock or 32.768 kHz external clock,
four CTIMER that use the APB clock and have four channels each and one
SCT timer with up to 10 channels running on the AHB clock.

This patch implements a timer driver for the CTIMER blocks only, which
is enough to make the xtimer module work. Future patches should improve
on this module to support using the RTC CNT2 32-bit free-running
counter unit and/or the SCT timer.

Testing procedure

./dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . qn9080dk --jobs=1 --with-test-only --applications 'tests/xtimer_*'

Issues/PRs references

This is part of issue #13852.

@benpicco benpicco added Area: cpu Area: CPU/MCU ports Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Dec 3, 2020
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

looks good - does this need any board config?
Ah does't seems so - the timers are always sourced by the same clock - reminds me of lpx23xx 😉

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 4, 2020
@benpicco
Copy link
Contributor

benpicco commented Dec 4, 2020

Ah this exposed some bugs/omissions in the watchdog & gpio driver since those tests were previously not build.

@iosabi
Copy link
Contributor Author

iosabi commented Dec 4, 2020

Ah this exposed some bugs/omissions in the watchdog & gpio driver since those tests were previously not build.

That seems to be the issue.

GPIO_BOTH (both rising and falling edges) doesn't seem to be supported by the hardware. I can either set rising, falling, low or high. I could try to emulate this by setting one edge or the other one based on the current level, but I think it might not be all that robust. I'll try.

UART_DATA_BITS_5 and UART_DATA_BITS_6, the MCU supports 7, 8 and 9 bits. How can I signal that those are not supported?

@benpicco
Copy link
Contributor

benpicco commented Dec 4, 2020

GPIO_BOTH (both rising and falling edges) doesn't seem to be supported by the hardware. I can either set rising, falling, low or high. I could try to emulate this by setting one edge or the other one based on the current level, but I think it might not be all that robust. I'll try.

you could also chose to not support it and return error when trying to configure it.

UART_DATA_BITS_5 and UART_DATA_BITS_6, the MCU supports 7, 8 and 9 bits. How can I signal that those are not supported?

stm32 just marks them as not supported and then fails uart_mode if those are set.

GPIO_BOTH gpio_flank_t; UART_PARTY_MARK and UART_PARTY_SPACE in
uart_parity_t; and UART_DATA_BITS_5 and UART_DATA_BITS_6
uart_data_bits_t enum values where missing from the periph_cpu.h header
since they are not supported by the CPU. This was causing some tests to
fail to compile, but only after adding the periph_timer module.

This patch adds those missing macros and makes the corresponding
functions fail when trying to use them.

A minor fix to the NWDT_TIME_LOWER_LIMIT value setting it to 1U to avoid
a -Werror=type-limits error in the tests/periph_wdt test. In theory 0
is a totally valid value although a bit useless since it will trigger
the WDT right away.
The QN908x CPU has several timer modules: one RTC (Real-Time Clock) that
can count from the 32kHz internal clock or 32.768 kHz external clock,
four CTIMER that use the APB clock and have four channels each and one
SCT timer with up to 10 channels running on the AHB clock.

This patch implements a timer driver for the CTIMER blocks only, which
is enough to make the xtimer module work. Future patches should improve
on this module to support using the RTC CNT2 32-bit free-running
counter unit and/or the SCT timer.
@iosabi
Copy link
Contributor Author

iosabi commented Dec 4, 2020

I added a second commit (first in the stack) to this pull request that should address gpio, uart and wdt.

I see most CPUs treat NWDT_TIME_LOWER_LIMIT as non-inclusive (assert(NWDT_TIME_LOWER_LIMIT < max_time) or similar). This is a bit more restrictive than it should be. We could actually set 0 if we wanted the WDT to trigger immediately. Right now the lowest max_time is 2, because that constant can't be 0.

@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 4, 2020
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 5, 2020
@benpicco benpicco merged commit f72e98d into RIOT-OS:master Dec 5, 2020
@iosabi iosabi deleted the qn908x_ctimer branch January 31, 2021 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants