-
Notifications
You must be signed in to change notification settings - Fork 7.7k
boards: weact: blackpill_h523ce: Add board support #92353
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?
boards: weact: blackpill_h523ce: Add board support #92353
Conversation
db0b9a8
to
ebca689
Compare
ebca689
to
6a23213
Compare
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.
LGTM but please solve compliance checks and documentation build issues
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 make required changes to enable the support for the actual SoC you're using, otherwise LGTM!
#include <st/h5/stm32h533Xe.dtsi> | ||
#include <st/h5/stm32h533retx-pinctrl.dtsi> |
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 add the h523Xe.dtsi file you need this will avoid unexpected issues if some later changes are made to the h533Xe.dtsi file.
Same thing for the -pinctrl.dtsi file. You should use stm32h523cetx-pinctrl.dtsi
and you don't even need to create it.
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.
Do you mean that I need to add stm32h523Xe.dtsi file under dts/arm/st/ht/ and it can be a copy of stm32h533Xe.dtsi, that is already supported as a SOC?
And I can just include stm32h523Xe-pinctrl.dtsi without creating it?
I added stm32h533 as a SOC because it is almost the same chip as a stm32h523, I mean, they share same documentation: STM32H523/533
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.
Do you mean that I need to add stm32h523Xe.dtsi file under dts/arm/st/ht/ and it can be a copy of stm32h533Xe.dtsi, that is already supported as a SOC?
Yes, you need to add it (at least someone needs to do it and that would be nice that it is you as you're the first one needing it). Not as a copy though, you need to follow the inclusion scheme:
Diff between 523 and 533 is (S)AES and OTFDEC, which are not supported today. So basically, you can:
- rename existing stm32h533.dtsi to stm32h523.dtsi
- create new stm32h533.dtsi that include stm32h523.dtsi (as indeed they are equivalent today)
- add the stm32h523Xe.dtsi (which is indeed a mere copy of stm32h533Xe.dtsi)
Please note that this work has to be done in a separate commit in this same PR.
I agree that today, functionally, this is exactly the same as what you have done. But if (S)AES and OTFDEC support is added, this may require follow up support in your board.
And I can just include stm32h523Xe-pinctrl.dtsi without creating it?
Yes, this one is already available (we generate them automatically).
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.
Thank you for a such detailed answer! I understand what I have to do now.
6a23213
to
0b58b33
Compare
0b58b33
to
334b6ea
Compare
ce720dc
to
5b17134
Compare
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.
Thanks for having added the SoC support.
Few changes for consistency and we'll be good to go:
- Put the SoC commit first
- Group the changes on the board in a single commit
Provide support for STM32H523XE. Signed-off-by: Filip Stojanovic <[email protected]>
This adds support for the WeAct blackpill_h523ce board. Signed-off-by: Filip Stojanovic <[email protected]>
5b17134
to
c259b2e
Compare
|
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.
Here we go. Good job!
Thank you Erwan :) |
looks like you have quite a few CI issues to address tho :) |
This adds initial support for a new WeAct STM32H523CE board - blackpill_h523ce.