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

boards/stm32l4: Fix all boards to support proper stm32_bringup.c #15817

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

acassis
Copy link
Contributor

@acassis acassis commented Feb 11, 2025

Summary

Many STM32L4 boards are missing stm32_bringup.c. That confusion
was created when stm32_appinit.c was created. It introduced a new
way to do the board initialization without depending on NSH arch
specific initialization.

Impact

Fix boards bringup process.

Testing

Building

Many STM32L4 boards are missing stm32_bringup.c. That confusion
was created when stm32_appinit.c was created. It introduced a new
way to do the board initialization without depending on NSH arch
specific initialization.

Signed-off-by: Alan C. Assis <[email protected]>
Many STM32L4 boards are missing stm32_bringup.c. That confusion
was created when stm32_appinit.c was created. It introduced a new
way to do the board initialization without depending on NSH arch
specific initialization.

Signed-off-by: Alan C. Assis <[email protected]>
Many STM32L4 boards are missing stm32_bringup.c. That confusion
was created when stm32_appinit.c was created. It introduced a new
way to do the board initialization without depending on NSH arch
specific initialization.

Signed-off-by: Alan C. Assis <[email protected]>
@github-actions github-actions bot added Board: arm Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. labels Feb 11, 2025
@nuttxpr
Copy link

nuttxpr commented Feb 11, 2025

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides a summary of the what and why, it lacks crucial details.

Here's a breakdown of what's missing:

  • Summary:
    • How does the change exactly work? It mentions removing the dependency on NSH arch initialization, but doesn't explain how it achieves that. Does it involve adding stm32_bringup.c files? Modifying existing files? What specific changes are made within those files?
  • Impact: The impact section is far too brief. While it mentions fixing the bringup process, it needs to explicitly address all the points in the template. For example:
    • Impact on user: Even if the answer is NO, it should be stated explicitly.
    • Impact on build: While it mentions "Building," this isn't descriptive. Does the build process change in any way due to the addition/modification of files?
    • Impact on hardware: Which specific STM32L4 boards are affected?
    • Impact on documentation: Does this change necessitate documentation updates?
    • Impact on security, compatibility, etc.: All of these need to be addressed, even if the answer is NO.
  • Testing:
    • Build Host(s): Missing operating system, CPU architecture, and compiler information.
    • Target(s): Missing specific architectures and board configurations tested.
    • Testing logs: Completely empty. Needs to include logs demonstrating the issue before the change and the improved behavior after the change. "Building" is not a testing log. Show output demonstrating successful bringup.

In short, the PR needs to be significantly more detailed to meet the requirements. It needs to clearly explain the technical implementation of the change, thoroughly address all aspects of its impact, and provide concrete evidence of testing on specific platforms.

Many STM32L4 boards are missing stm32_bringup.c. That confusion
was created when stm32_appinit.c was created. It introduced a new
way to do the board initialization without depending on NSH arch
specific initialization.

Signed-off-by: Alan C. Assis <[email protected]>
Many STM32L4 boards are missing stm32_bringup.c. That confusion
was created when stm32_appinit.c was created. It introduced a new
way to do the board initialization without depending on NSH arch
specific initialization.

Signed-off-by: Alan C. Assis <[email protected]>
@cederom
Copy link
Contributor

cederom commented Feb 12, 2025

Thank you @acassis !! I played a bit some time ago with STM32 boards and noticed the inconsistencies, thank you for the cleanup!! I have some of these boards, will test on hardware and report back :-)

Copy link
Contributor

@jerpelea jerpelea left a comment

Choose a reason for hiding this comment

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

please update the title to include the modified area
ex:
boards/stm32l4r9ai-disco: Add support to bringup

thanks

@acassis acassis changed the title STM32L4: Fix all boards to support proper stm32_bringup.c boards/stm32l4: Fix all boards to support proper stm32_bringup.c Feb 14, 2025
@acassis
Copy link
Contributor Author

acassis commented Feb 14, 2025

please update the title to include the modified area ex: boards/stm32l4r9ai-disco: Add support to bringup

thanks

Done! Since it update many boards I used boards/stm32l4

@jerpelea
Copy link
Contributor

please update the title to include the modified area ex: boards/stm32l4r9ai-disco: Add support to bringup
thanks

Done! Since it update many boards I used boards/stm32l4

in the future please specify boards also on the commit title
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Board: arm Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants