Skip to content

boards: arm: add QEMU support for Cortex-M0 #19479

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

Merged
merged 7 commits into from
Oct 3, 2019

Conversation

ioannisg
Copy link
Member

@ioannisg ioannisg commented Sep 30, 2019

QEMU for Cortex-M0, based on BBC Microbit

Fixes #2686

@zephyrbot
Copy link
Collaborator

zephyrbot commented Sep 30, 2019

All checks are passing now.

checkpatch (informational only, not a failure)

-:452: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "nordic,nrf51822-qfaa" appears un-documented -- check ./dts/bindings/
#452: FILE: boards/arm/qemu_cortex_m0/qemu_cortex_m0.dts:13:
+	compatible = "nordic,nrf51822-qfaa", "nordic,nrf51822";

-:452: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "nordic,nrf51822" appears un-documented -- check ./dts/bindings/
#452: FILE: boards/arm/qemu_cortex_m0/qemu_cortex_m0.dts:13:
+	compatible = "nordic,nrf51822-qfaa", "nordic,nrf51822";

-:483: WARNING:LONG_LINE_COMMENT: line over 80 characters
#483: FILE: boards/arm/qemu_cortex_m0/qemu_cortex_m0.dts:44:
+	 * http://docs.zephyrproject.org/latest/guides/dts/index.html#flash-partitions

- total: 0 errors, 3 warnings, 544 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@ioannisg ioannisg added the Release Notes To be mentioned in the release notes label Sep 30, 2019
@ioannisg ioannisg added this to the v2.1.0 milestone Sep 30, 2019
@ioannisg ioannisg requested a review from galak September 30, 2019 13:36
@ioannisg ioannisg added the area: ARM ARM (32-bit) Architecture label Sep 30, 2019
@ioannisg ioannisg force-pushed the qemu_cortex_m0 branch 2 times, most recently from 6818cbd to e27e531 Compare September 30, 2019 14:13
Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

can we not just use the microbit board? do we need to introduce a different board port?

@nashif
Copy link
Member

nashif commented Sep 30, 2019

can we not just use the microbit board? do we need to introduce a different board port?

agree here, similar to what we did with mps2_an385

@nashif
Copy link
Member

nashif commented Sep 30, 2019

also, I would make this a default platform and remove the nrf51 board which was covering for cortex-m0 as a default platform.

@nashif
Copy link
Member

nashif commented Sep 30, 2019

can we not just use the microbit board? do we need to introduce a different board port?

agree here, similar to what we did with mps2_an385

although, one issue we have with microbit is the low RAM which makes it less interesting when it comes to coverage...

@ioannisg
Copy link
Member Author

can we not just use the microbit board? do we need to introduce a different board port?

agree here, similar to what we did with mps2_an385

although, one issue we have with microbit is the low RAM which makes it less interesting when it comes to coverage...

Yes, and besides that, @nashif @galak 👍
My goal was to have a generic Cortex-M0 platform to get kernel/arch coverage
BBC Microbit is an actual IoT platform used by lots of developers, so I don't want to confuse with adding qemu support on that one. Also bbc microbit uses peripheral devices and has additional hardware that I had no intention of testing via the QEMU platform.

that's why I was happy to add an additional platform, similarly to qemu_cortex_m3.

@ioannisg
Copy link
Member Author

ioannisg commented Oct 1, 2019

@nashif @galak there seems to be one additional reason I want the separate board for Qemu Cortex-M0:
QEMU emulation for Microbit does not seem to support thr RTC peripheral - only the TIMER. Since nRF51 has no SysTuck, this means, we need to have a timer driver for system clock, based on TIMER. And this should be specific to qemu_cortex_m0.

@ioannisg ioannisg requested a review from galak October 1, 2019 16:33
@ioannisg
Copy link
Member Author

ioannisg commented Oct 1, 2019

I added a timer driver for the qemu_cortex_m0 platform, based on the nRF TIMER peripheral. The driver looks very similar to nrf_rtc_timer driver in drivers/timer. With this timer driver, the emulated microbit platform is able to generate timer interrupts, and effectively, run almost all the kernel test suite successfully.

As our goal is to get coverage on kernel and arm code on Cortex-M0, I believe we can live with the filtering-out the failing tests for this platform.

@zephyrbot zephyrbot added area: Timer Timer area: Tests Issues related to a particular existing or missing test area: Kernel labels Oct 1, 2019
@andrewboie
Copy link
Contributor

Can we give this new target significantly more RAM & flash so we can run all tests? From browsing this PR seems like we only have 16K of RAM.

@ioannisg ioannisg marked this pull request as ready for review October 1, 2019 18:24
@@ -0,0 +1,188 @@
/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did the whole driver have to be copied to the board directory?

Copy link
Member Author

@ioannisg ioannisg Oct 1, 2019

Choose a reason for hiding this comment

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

That's because there is no point having a second system clock driver based on Timer peripheral , besides the basic RTC driver. I only added the driver because the qemu does not emulate the RTC :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I'm confused. Where does the emulation come from then? I mean, it looks like nrf_timer_cc_read() is returning your counter value from somewhere, right? So the HAL is doing the emulation? If that's true, why won't the default driver work?

Copy link
Member Author

@ioannisg ioannisg Oct 1, 2019

Choose a reason for hiding this comment

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

@andyross the default nRF driver, used in Zephyr, is based on the NRF RTC peripheral. This peripheral is, however, not supported in the QEMU emulation of the Microbit platform (nRF51). Instead, the QEMU guys have added support for the NRF TIMER peripheral. So, I had to write a system timer driver, based on the NRF TIMER peripheral.

But there is no need to have this driver in the /drivers/timer directory as a generic timer driver; cause there is no use case for that; nRF SoCs use the nRF RTC timer that can utilize the low-frequency low-power clock source. This driver is not, even, specific to Microbit; actual microbit HW boards can use the default nRF RTC driver. The new driver is specific for the qemu_cortex_m0 platform. Its sole purpose is to add system timer support for qemu cortex-m0, so we get coverage on ARMv6-M architecture.

Copy link
Member Author

Choose a reason for hiding this comment

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

nrf_timer_cc_read() is returning your counter value from somewhere, right? So the HAL is doing the emulation? If that's true, why won't the default driver work?

Correct. The default driver is a different peripheral. The "corresponding function, there, is called nrf_rtc_cc_read(). But as I said, this peripheral does not yet work in qemu.

@ioannisg
Copy link
Member Author

ioannisg commented Oct 1, 2019

Can we give this new target significantly more RAM & flash so we can run all tests? From browsing this PR seems like we only have 16K of RAM.

I'll check this, @andrewboie

This commit defines qemu_cortex_m0 board, adding
support for Cortex-M0 in QEMU. The added platform
is based on the (nRF51) bbc_microbit board.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
This commit adds some documentation for the
newly introduced qemu_cortex_m0 platform.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
Adding code owner for ARM boards
- qemu_cortex_m0
- qemu_cortex_m3

Signed-off-by: Ioannis Glaropoulos <[email protected]>
This commit configures the qemu_cortex_m0 board to
build with it's custom timer driver, instead of the
default nrf_rtc_timer driver for nRF51x SoCs. It,
additionally, configures a default system clock
frequency to 1MHz, as well as 10 Hz tick frequency.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
Fix an inline comment in nrf_rtc_timer.c correcting the
path to the mentioned test.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
For the qemu_cortex_m0 we implement a custom system clock
driver based on the nRF51 TIMER peripheral. The system
clock is configured to run at 1 MHz frequency.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg
Copy link
Member Author

ioannisg commented Oct 2, 2019

Can we give this new target significantly more RAM & flash so we can run all tests? From browsing this PR seems like we only have 16K of RAM.

I'll check this, @andrewboie

@andrewboie I have looked into modifying DTS for the board, so it adds more ram and flash, however, several of the additional tests that are now attempted, are resulting in QEMU core dump; perhaps QEMU forbids using non-existing memory areas on this particular board. So, I would leave this out for now, to get basic cortex-M0 qemu support and look at it again in the future

We filter out the following kernel tests
- tickless_concept
- timer_api
from the set of tests running on QEMU Cortex-M0 platform,
as the tests consistently fail on QEMU. In addition, we
add a workaround for kernel/interrupt test, so it can
successfully execute on QEMU Cortex-M0.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
Copy link
Member

@nashif nashif left a comment

Choose a reason for hiding this comment

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

Should use this PR to remove nrf52_pca10028 as default, it was representing the cortex-m0 family:

--- a/boards/arm/nrf51_pca10028/nrf51_pca10028.yaml
+++ b/boards/arm/nrf51_pca10028/nrf51_pca10028.yaml
@@ -15,6 +15,5 @@ supported:
   - watchdog
   - counter
 testing:
-  default: true
   ignore_tags:
     - net

@ioannisg
Copy link
Member Author

ioannisg commented Oct 2, 2019

is PR to remove nrf52_pca10028 as default, it was represe

Alright, I can send a patch upstream after this PR is merged.

@ioannisg
Copy link
Member Author

ioannisg commented Oct 2, 2019

FYI: we currently build/run 104 tests for qemu_cortex_m0 - this looks like good coverage, IMO ;)

@ioannisg
Copy link
Member Author

ioannisg commented Oct 2, 2019

@andyross @galak @andrewboie could you re-review?
@dbkinder could you check the docs here?

@nashif nashif merged commit 85afd0f into zephyrproject-rtos:master Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture area: Boards area: Code Coverage area: Kernel area: Tests Issues related to a particular existing or missing test area: Timer Timer Release Notes To be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add qemu_cortex_m0/m0+ board.
7 participants