-
Notifications
You must be signed in to change notification settings - Fork 8.3k
boards: sifli: add sf32lb52_devkit_lcd revisions #98914
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
base: main
Are you sure you want to change the base?
Conversation
|
Hello @HalfSweet, and thank you very much for your first pull request to the Zephyr project! |
gmarull
left a comment
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.
please split changed into independent commits
| if(NOT DEFINED BOARD_REVISION) | ||
| set(BOARD_REVISION "n16r8") | ||
| else() | ||
| if(NOT BOARD_REVISION IN_LIST BOARD_REVISIONS) | ||
| message(FATAL_ERROR "${BOARD_REVISION} is not a valid revision for sf32lb52_devkit_lcd. Accepted revisions: ${BOARD_REVISIONS}") | ||
| endif() | ||
| endif() |
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.
@nordicjm looks like this should not be needed?
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.
is needed for custom board revisions
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.
doesn't build system complain if one provides an invalid board revision?
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.
not with custom, custom basically means the revision.cmake file defines the revisions, the ones in board.yml will just be listed by the list boards python script
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.
Is there anything else I need to modify here? I've noticed similar wording in some other board variants.
For example: https://github.com/zephyrproject-rtos/zephyr/blob/main/boards/arduino/uno_r4/revision.cmake
8956afb to
841093e
Compare
| if(NOT DEFINED BOARD_REVISION) | ||
| set(BOARD_REVISION "n16r8") | ||
| else() | ||
| if(NOT BOARD_REVISION IN_LIST BOARD_REVISIONS) | ||
| message(FATAL_ERROR "${BOARD_REVISION} is not a valid revision for sf32lb52_devkit_lcd. Accepted revisions: ${BOARD_REVISIONS}") | ||
| endif() | ||
| endif() |
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.
is needed for custom board revisions
| default: "n16r8" | ||
| revisions: | ||
| - name: "n16r8" | ||
| - name: "a128r8" |
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.
does this variant boot now?
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.
Yes, currently the bootrom transfers code to SRAM for execution.
Of course, this isn't a perfect solution. In future updates, I'll add memc drivers to support PSRAM and integrate MCUBoot adaptation to enable running larger code sizes from NAND storage
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.
does this variant have the NOR defined in board dts files?
- declare n16r8 (default) and a128r8 revisions in board.yml - add revision.cmake to validate BOARD_REVISION and keep n16r8 as fallback Signed-off-by: Haoran Jiang <[email protected]>
841093e to
c4c990d
Compare
|



Uh oh!
There was an error while loading. Please reload this page.