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

[collab-hwm] Port GD32 to HWMv2 (ARM only) #67124

Merged
merged 18 commits into from
Jan 11, 2024

Conversation

gmarull
Copy link
Member

@gmarull gmarull commented Jan 2, 2024

Note: RISC-V part skipped, it looks like RISC-V soc/ folder will need a bit of love first.

Contains commits from
#67147
#67355

Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

SoC and board changes look good, not looked at python file

@gmarull gmarull force-pushed the gd32-hwmv2 branch 3 times, most recently from a98348c to c2b5dbc Compare January 3, 2024 10:05
@gmarull gmarull mentioned this pull request Jan 3, 2024
@gmarull gmarull force-pushed the gd32-hwmv2 branch 2 times, most recently from b453cb7 to 9468661 Compare January 3, 2024 10:18
@gmarull gmarull marked this pull request as ready for review January 3, 2024 10:18
@gmarull
Copy link
Member Author

gmarull commented Jan 3, 2024

Note: compliance failure is a false positive

 Found references to undefined Kconfig symbols. If any of these are false
positives, then add them to UNDEF_KCONFIG_WHITELIST in /home/runner/work/zephyr/zephyr/./scripts/ci/check_compliance.py.

If the reference is for a comment like /* CONFIG_FOO_* */ (or
/* CONFIG_FOO_*_... */), then please use exactly that form (with the '*'). The
CI check knows not to flag it.

More generally, a reference followed by $, @, {, *, or ## will never be
flagged.

CONFIG_BOARD_                              scripts/utils/board_v1_to_v2.py:107

Copy link
Member

@fabiobaltieri fabiobaltieri left a comment

Choose a reason for hiding this comment

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

Cool, wondering if the documentation include files could be generated automatically by aggregating the board.yaml information and grouping by vendor there, but that's probably a small project in itself

@nandojve
Copy link
Member

nandojve commented Jan 3, 2024

Hi @gmarull ,

Not sure if compliance is a false positive here.

@gmarull
Copy link
Member Author

gmarull commented Jan 3, 2024

Hi @gmarull ,

Not sure if compliance is a false positive here.

It is, CONFIG_BOARD_ is part of a regex in the migration script.

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

lgtm ❤️

@tejlmand
Copy link
Collaborator

tejlmand commented Jan 4, 2024

It is, CONFIG_BOARD_ is part of a regex in the migration script.

so should be added to the list here:

# Many of these are symbols used as examples. Note that the list is sorted
# alphabetically, and skips the CONFIG_ prefix.
UNDEF_KCONFIG_WHITELIST = {
"ALSO_MISSING",
"APP_LINK_WITH_",
"APP_LOG_LEVEL", # Application log level is not detected correctly as

@gmarull gmarull added the DNM This PR should not be merged (Do Not Merge) label Jan 8, 2024
@@ -0,0 +1,10 @@
.. _boards-arm-holdings:

ARM Holdings Boards
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ARM Holdings Boards
Arm Holdings Boards

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, can we just get rid of the " Boards" suffix? It shouldn't be necessary to repeat as the TOC hierarchy already conveys that information and this clutters the navigation tree quite a bit (see v1 below)

image

Copy link
Member Author

Choose a reason for hiding this comment

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

done, but used "ARM Ltd." as in vendor-prefixes.txt

@@ -0,0 +1,10 @@
.. _boards-gigadevice:

Gigadevice Boards
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it time we start looking at Sphinx/python tooling to automate some of the things around boards' documentation, in particular the generation of the toctree?

Reason I'm thinking this is that here you have GigaDevice spelled Gigadevice (lowercase d), and the boards' vendor is set as "GigaDevice Semiconductor". Looks like the kind of inconsistencies that, while certainly not the end of the world, can easily spread to many places.

If board.yml starts adopting the vendor prefix, a first step could be that we automatically add entries to substititions.txt so that something like |vendor-gd| gets replaced by the associated entry in vendor-prefixes.txt

Copy link
Member Author

Choose a reason for hiding this comment

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

@kartben It would be nice to implement a new extension to display an interactive list of boards, where you can filter by vendor, soc, etc. It should now be easier with the new board.yml file. Regarding vendor, I think board.yml should contain a vendor: $prefix, and use a database (like we do for DT), cc: @tejlmand

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think board.yml should contain a vendor: $prefix, and use a database (like we do for DT), cc: @tejlmand

Not against the idea, but we need to also remember that it should be easy for oot users to just create their own board.
Of course such users can provide a custom vendor-prefixes.txt file, but then it starts to require a bit more integration in multiple places.

I think we should have an enhancement issue, and then link that issue in here: #51831

Copy link
Member Author

Choose a reason for hiding this comment

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

for now: using text found in vendor-prefixes, so at least we're consistent everywhere.

boards/v2/gigadevice/index.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

latest changes also looks good 👍

Port all the Gigadevice SoCs to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Port the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Convert the board to HWMv2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Some SoC/board files have been moved, use new paths.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Add an entry for Gigadevice boards.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Add a new entry for ARM Holdings plc boards.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Add a new entry for Renesas boards.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
SoC has been converted to v2.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
@gmarull gmarull removed the DNM This PR should not be merged (Do Not Merge) label Jan 11, 2024
@carlescufi carlescufi merged commit a5543dc into zephyrproject-rtos:collab-hwm Jan 11, 2024
20 of 21 checks passed
@gmarull gmarull deleted the gd32-hwmv2 branch January 11, 2024 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants