doc/guides: add guide for porting new MCUs#22173
doc/guides: add guide for porting new MCUs#22173fjrdev wants to merge 11 commits intoRIOT-OS:masterfrom
Conversation
|
There is a compile error when trying to build the documentation: |
crasbe
left a comment
There was a problem hiding this comment.
Thank you for adding this much needed guide!
The remarks about rearranging the Typical Responsibilities lists is valid for all files, I did not want to add "Same" comments for every occurance.
Once the documentation is buildable, we can perhaps think about using actual Subheadings for the Example: etc "headings" that are currently just in text style.
| - documentation and tests that make the port maintainable. | ||
|
|
||
| This document focuses on the CPU side. For the board side, also refer to the | ||
| existing RIOT guide on porting boards. |
There was a problem hiding this comment.
It would be nice if you could add a link to said guide.
| └── cpu.c | ||
| ``` | ||
|
|
||
| Not every subdirectory is mandatory, but most non-trivial ports will need at |
There was a problem hiding this comment.
Maybe you could add an asterisk to the mandatory or non-mandatory files?
| Typical responsibilities: | ||
|
|
||
| - define `MODULE = cpu`, | ||
| - include `$(RIOTBASE)/Makefile.base`, | ||
| - optionally add subdirectories such as `periph/`, `vectors/`, or a common CPU | ||
| family directory. |
There was a problem hiding this comment.
This feels a bit redundant because that's exactly what's in the code below. I'd probably remove it tbh.
| Typical responsibilities: | ||
|
|
||
| - pull in common CPU-family dependency files, | ||
| - select required helper modules |
| `cpu.c` contains `cpu_init()`, which performs the minimum hardware and runtime | ||
| initialization needed before RIOT starts normal execution. | ||
|
|
||
| Typical responsibilities: | ||
|
|
||
| - call generic core initialization for the architecture, | ||
| - set interrupt priorities and interrupt grouping if required, | ||
| - configure clocks or clock assumptions used by the CPU port, | ||
| - perform any CPU-local setup that must happen before peripherals are used. |
| corresponding peripheral implementation. | ||
|
|
||
| :::note | ||
| Refer to the Technical Reference Manual of the target CPU for details on implementing the boot process. |
There was a problem hiding this comment.
| Refer to the Technical Reference Manual of the target CPU for details on implementing the boot process. | |
| Refer to the Technical Reference Manual of the target CPU for details on | |
| implementing the boot process. |
Long line.
| ### Reuse common code | ||
|
|
||
| If the MCU belongs to an already supported architecture or family, reuse the | ||
| existing common code where possible. For this case, a base directory (e.g. `cpu/<CPU_FAM>_common`) should be created that contains all shared/common configurations. This drastically reduces maintenance and lowers the |
|
|
||
| ### Refer to the manual of the target CPU | ||
|
|
||
| Many CPUs are shipped with detailed manuals containing register-level information about peripherals, clock trees, the boot sequence, interrupt handling, and more. This information should be read, understood, and then mapped to the RIOT framework. |
|
I think the last two commits were not meant to be added here? 👀 |
Oh sorry. you are right! Will fix it + your suggestions asap! |
|
To avoid this kind of issue, it is usually a good idea to create a separate branch for a pull request and not use the Unfortunately it is not possible to change the branch after opening the PR AFAIK. |
|
@crasbe Thanks a lot for reviewing! I have added your feedback in the latest version. I will add subheading to the corresponding sections. Can we use # levels here as well? |
|
@AnnsAnns: as our recently graduated CPU-porting expert, would you like to take a look at this too? 👁️👁️ |
|
You'll have to add the guide to the Astro configuration file, otherwise it won't show up in the nagivation side bar: diff --git a/doc/starlight/astro.config.ts b/doc/starlight/astro.config.ts
index 1d6bf1f3d4..06a48f592b 100644
--- a/doc/starlight/astro.config.ts
+++ b/doc/starlight/astro.config.ts
@@ -142,6 +142,7 @@ export default defineConfig({
"advanced_tutorials/unittests",
"advanced_tutorials/device_drivers",
"advanced_tutorials/porting_boards",
+ "advanced_tutorials/porting_cpus",
"advanced_tutorials/event_queue",
],
}, |
|
For sure will take a look, I was really happy to hear that somebody was working on this when crasbe mentioned this as I wanted to also do something like this, thank you for this PR 🫡 |
crasbe
left a comment
There was a problem hiding this comment.
Now that the documentation can be built in Starlight, I gave this a more thorough review. Nothing major, just some minor improvements here and there :)
| A practical CPU porting workflow usually looks like this: | ||
|
|
||
| 1. create the CPU directory and build-system files, | ||
| 2. add startup support and a working `cpu_init()`, |
There was a problem hiding this comment.
| 2. add startup support and a working `cpu_init()`, | |
| 2. add startup support and a working `cpu_init()` function, |
|
|
||
| ## Build System Files | ||
|
|
||
| ### `Makefile` |
There was a problem hiding this comment.
With the Code formatting, our current style file does not respect the sublevel headings:
@AnnsAnns @LasseRosenow is that something that you'd want to fix in the style file or should the use of code formatting just be avoided for sublevel headlines?
There was a problem hiding this comment.
I'd personally say just avoid formatting in titles
There was a problem hiding this comment.
I'd personally say just avoid formatting in titles
There was a problem hiding this comment.
I'd personally say just avoid formatting in titles
There was a problem hiding this comment.
You'd personally say just avoid formatting in titles? Did I understand that correctly? 🤪
There was a problem hiding this comment.
Most normal Github behavior, how is this the default git forge but it can't handle weird internet connections, I was legit just in an ICE on train wifi 😭
| ### `Makefile` | ||
|
|
||
| The top-level CPU `Makefile` defines the CPU module and may include common | ||
| subdirectories that are shared with other CPUs. |
There was a problem hiding this comment.
| subdirectories that are shared with other CPUs. | |
| subdirectories that are shared with other CPUs. `$(RIOTCPU)` is the `cpu/` subdirectory | |
| of the RIOT basefolder `$(RIOTBASE)`. |
|
|
||
| This file declares what the CPU provides to the rest of RIOT by defining the | ||
| CPU architecture, including common family feature files as well as defining | ||
| the implemented peripherals with `FEATURES_PROVIDED`. |
There was a problem hiding this comment.
| the implemented peripherals with `FEATURES_PROVIDED`. | |
| the implemented peripherals with `FEATURES_PROVIDED` in alphabetical order. |
|
|
||
| ### `Makefile.include` | ||
|
|
||
| This file contains low-level build configuration for the CPU. |
There was a problem hiding this comment.
| This file contains low-level build configuration for the CPU. | |
| This file contains low-level build configuration and additional compiler and | |
| linker search paths for the CPU. |
| ### Generic high-level shape | ||
|
|
||
| ```c | ||
| spi_init(bus); | ||
| spi_init_cs(bus, cs); | ||
|
|
||
| spi_acquire(bus, cs, SPI_MODE_0, SPI_CLK_1MHZ); | ||
| spi_transfer_bytes(bus, cs, false, tx_buf, rx_buf, len); | ||
| spi_release(bus); | ||
| ``` |
There was a problem hiding this comment.
This would be application/device driver level though.
There was a problem hiding this comment.
Is it too specific? Thought this would be nice as an addition to the bullet points above this snippet
There was a problem hiding this comment.
Perhaps you could specify that this is not part of the CPU code but just given as an example to clarify how the functions will be called on the application/driver level.
|
|
||
| - boot to `cpu_init()`, | ||
| - confirm the vector table works, | ||
| - get `examples/basic/hello-world` running over UART, |
There was a problem hiding this comment.
I'd probably include something like "light up an LED at different stages of the code if you don't observe output on the UART". That often helped me in the past.
| ### Reuse common code | ||
|
|
||
| If the MCU belongs to an already supported architecture or family, reuse the | ||
| existing common code where possible. For this case, a base directory |
There was a problem hiding this comment.
| existing common code where possible. For this case, a base directory | |
| existing common code where possible. In this case, a base directory |
| existing common code where possible. For this case, a base directory | ||
| (e.g. `cpu/<CPU_FAM>_common`) should be created that contains all shared/common | ||
| configurations. This drastically reduces maintenance and lowers the risk of | ||
| subtle startup or interrupt bugs. |
There was a problem hiding this comment.
| subtle startup or interrupt bugs. | |
| subtle startup or interrupt bugs, as well as code divergation issues in the future. |
| Correct handling of repeated starts, ACK/NACK conditions, and bus-busy recovery | ||
| is more important than adding optional optimizations early. | ||
|
|
||
| ## General Recommendations |
There was a problem hiding this comment.
I wonder if this chapter wouldn't have a better home further on top. Currently this is a repetition of many things that were already explained in greater detail 🤔
There was a problem hiding this comment.
I put this chapter at the end to wrap-up the sections explaining the different directories/files. Moving it at the top would put the mentioning of some concepts/files above the explanation wouldn't it? maybe im missing your point
AnnsAnns
left a comment
There was a problem hiding this comment.
Either the wifi is too spotty or github is having problems so I'll have to halt my review for now since I'm getting a lot of errors, so far this was a nice read though 👍
|
|
||
| ## Build System Files | ||
|
|
||
| ### `Makefile` |
There was a problem hiding this comment.
I'd personally say just avoid formatting in titles
|
|
||
| ## Build System Files | ||
|
|
||
| ### `Makefile` |
There was a problem hiding this comment.
I'd personally say just avoid formatting in titles
AnnsAnns
left a comment
There was a problem hiding this comment.
Read through it again, looks good for the most part, just one minor nitpick from my side left
| {% filetree %} | ||
| - cpu/foo | ||
| - include | ||
| - cpu_conf.h | ||
| - periph_cpu.h | ||
| - vendor* | ||
| - ldscripts* | ||
| - periph* | ||
| - gpio.c | ||
| - i2c.c | ||
| - spi.c | ||
| - … | ||
| - vectors | ||
| - vectors_<model>.c | ||
| - Makefile | ||
| - Makefile.dep* | ||
| - Makefile.features* | ||
| - Makefile.include | ||
| - cpu.c | ||
| {% /filetree %} |
There was a problem hiding this comment.
| {% filetree %} | |
| - cpu/foo | |
| - include | |
| - cpu_conf.h | |
| - periph_cpu.h | |
| - vendor* | |
| - ldscripts* | |
| - periph* | |
| - gpio.c | |
| - i2c.c | |
| - spi.c | |
| - … | |
| - vectors | |
| - vectors_<model>.c | |
| - Makefile | |
| - Makefile.dep* | |
| - Makefile.features* | |
| - Makefile.include | |
| - cpu.c | |
| {% /filetree %} | |
| <FileTree> | |
| - cpu/foo | |
| - include | |
| - cpu_conf.h | |
| - periph_cpu.h | |
| - vendor* | |
| - ldscripts* | |
| - periph* | |
| - gpio.c | |
| - i2c.c | |
| - spi.c | |
| - … | |
| - vectors | |
| - vectors_<model>.c | |
| - Makefile | |
| - Makefile.dep* | |
| - Makefile.features* | |
| - Makefile.include | |
| - cpu.c | |
| </FileTree> |
That syntax is for Markdoc, we use MDX. To use this component rename the file to "porting_cpus.mdx" and also import import { FileTree } from '@astrojs/starlight/components'; at the top of the file. See: https://guide.riot-os.org/misc/how_to_doc/ and https://starlight.astro.build/components/file-tree/ for the component itself
There was a problem hiding this comment.
Thank you for pointing this out! I updated to mdx in the last commit.
crasbe
left a comment
There was a problem hiding this comment.
You know that you can run make doc-starlight locally to check if it works, right? 👀
| - architecture-common code such as `cpu/cortexm_common/` when porting | ||
| Cortex-M-based MCUs | ||
| - merged RIOT PRs adding CPU support, for reference and pattern matching: | ||
| <https://github.com/RIOT-OS/RIOT/pulls?q=is%3Apr+is%3Amerged+%22cpu%2F%22+%22boards%2F%22> |
There was a problem hiding this comment.
| <https://github.com/RIOT-OS/RIOT/pulls?q=is%3Apr+is%3Amerged+%22cpu%2F%22+%22boards%2F%22> | |
| [see GitHub](https://github.com/RIOT-OS/RIOT/pulls?q=is%3Apr+is%3Amerged+%22cpu%2F%22+%22boards%2F%22) |
To fix the build error:
11:07:49 [ERROR] Unexpected character `/` (U+002F) before local name, expected a character that can start a name, such as a letter, `$`, or `_` (note: to create a link in MDX, use `[text](url)`)
Stack trace:
at /home/cbuec/RIOTstuff/riot-vanillaice/RIOT/doc/guides/advanced_tutorials/porting_cpus.mdx:650:10
[...] See full stack trace in the browser, or rerun with --verbose.
| description: Guide on how to port new CPU families to RIOT-OS | ||
| --- | ||
|
|
||
| import { FileTree } from '@astrojs/starlight/components'; |
There was a problem hiding this comment.
| import { FileTree } from '@astrojs/starlight/components'; | |
| import FileTree from '@components/FileTree.astro'; |
To fix the next occurring issue:
11:11:12 [ERROR] Cannot find module '@astrojs/starlight/components' imported from '/home/cbuec/RIOTstuff/riot-vanillaice/RIOT/doc/guides/advanced_tutorials/porting_cpus.mdx'
Stack trace:
at fetchModule (file:///home/cbuec/RIOTstuff/riot-vanillaice/RIOT/doc/starlight/node_modules/vite/dist/node/chunks/config.js:33980:34)
[...] See full stack trace in the browser, or rerun with --verbose.
There was a problem hiding this comment.
I just saw that I recommended this line. Sorry for suggesting something incorrect 🫨
| - spi.c | ||
| - … | ||
| - vectors | ||
| - vectors_<model>.c |
There was a problem hiding this comment.
| - vectors_<model>.c | |
| - vectors_\<model\>.c |
To fix the next error:
11:10:05 [ERROR] Expected a closing tag for `<model>` (50:15-50:22) before the end of `paragraph`
Stack trace:
at /home/cbuec/RIOTstuff/riot-vanillaice/RIOT/doc/guides/advanced_tutorials/porting_cpus.mdx:50:7
[...] See full stack trace in the browser, or rerun with --verbose.
There was a problem hiding this comment.
Thanks for the suggestions! I have resolved the build issues in the newest commit
|
I'd recommend looking at https://guide.riot-os.org/misc/how_to_doc/#how-to-run-the-site-locally if something is not working, otherwise feel free to reach out for help |
Didn't know that, thanks for pointing out! :D |
Contribution description
This PR features a markdown file describing the process of porting new CPUs into the RIOT ecosystem, since there is currently only a porting guide for boards but not for CPUs. The guide first gives a general overview of the process and then dives into to process of creating the file-structure for a new CPU. A description + code examples are given for the required files. Furthermore, a roadmap is given for the implementing the most important peripherals. This guide was created during the port of the Infineon PSOC-6, which is a new CPU for RIOT. A PR for this will follow in a bit.
This PR adds a new Markdown guide at
doc/guides/advanced_tutorials/RIOT_CPU_PORTING.mdthat documents the process of porting a new CPU to RIOT.At the moment, RIOT provides guidance for board porting, but there is no CPU porting guide. This contribution is intended to fill that gap by describing the overall CPU porting workflow, the expected file structure, the role of the main CPU-specific files and high-level descriptions for implementing core peripherals (GPIO, UART, Timer, SPI and I2C).
This guide was written during the ongoing port of the Infineon PSoC-6 CPU family to RIOT. The corresponding CPU port itself will be submitted in a separate PR in the near future. It is intended to make future CPU ports easier to structure and understand.
Scope:
Testing procedure
Issues/PRs references
None
Declaration of AI-Tools / LLMs usage:
AI-Tools / LLMs that were used are: