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: st: Add support for STM32H745I-Disco #72510

Merged
merged 1 commit into from
May 14, 2024

Conversation

darkmoon32
Copy link
Contributor

This PR adds a new board STM32H745I-Discovery. Its definition is heavily inspired by nucleo_h745zi_q.

I have tested basic functionality by running:

  • samples/hello_world
  • samples/basic/blinky
  • samples/basic/blinky_pwm
  • samples/basic/button

I am using the board for developing application that uses Ethernet, async UART, RTC, QSPI, so these features are also working fine.

FMC and SDRAM I only tested by tests/drivers/memc/ram/.

Copy link

github-actions bot commented May 9, 2024

Hello @darkmoon32, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

Copy link
Collaborator

@ajarmouni-st ajarmouni-st left a comment

Choose a reason for hiding this comment

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

Thank you for contributing, here is an initial batch of comments.

boards/st/stm32h745i_disco/doc/index.rst Outdated Show resolved Hide resolved
boards/st/stm32h745i_disco/doc/index.rst Show resolved Hide resolved
boards/st/stm32h745i_disco/doc/index.rst Show resolved Hide resolved
boards/st/stm32h745i_disco/doc/index.rst Show resolved Hide resolved
@zephyrbot zephyrbot added the area: UART Universal Asynchronous Receiver-Transmitter label May 13, 2024
@zephyrbot zephyrbot requested a review from dcpleung May 13, 2024 10:17
boards/st/stm32h745i_disco/doc/index.rst Outdated Show resolved Hide resolved
boards/st/stm32h745i_disco/doc/index.rst Outdated Show resolved Hide resolved
boards/st/stm32h745i_disco/doc/index.rst Outdated Show resolved Hide resolved
boards/st/stm32h745i_disco/doc/index.rst Outdated Show resolved Hide resolved
boards/st/stm32h745i_disco/doc/index.rst Outdated Show resolved Hide resolved
boards/st/stm32h745i_disco/doc/index.rst Outdated Show resolved Hide resolved
boards/st/stm32h745i_disco/doc/index.rst Outdated Show resolved Hide resolved
@darkmoon32 darkmoon32 force-pushed the stm32h745i-disco branch 3 times, most recently from 46ea905 to ffd0d8f Compare May 13, 2024 12:51
Supported features:
- GPIO
- RTC
- PWM
- Ethernet
- UART
- FMC
- QSPI NOR Flash

Signed-off-by: Tomáš Juřena <jurenatomas@gmail.com>
@darkmoon32 darkmoon32 force-pushed the stm32h745i-disco branch 2 times, most recently from 9b6293b to eee95eb Compare May 13, 2024 15:11
ajarmouni-st
ajarmouni-st previously approved these changes May 14, 2024
Copy link
Collaborator

@ajarmouni-st ajarmouni-st left a comment

Choose a reason for hiding this comment

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

Great work !

erwango
erwango previously approved these changes May 14, 2024
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Approved, but I's suggest to drop the second commit (see comment above)

Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

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

Please add an entry in the release notes

@erwango
Copy link
Member

erwango commented May 14, 2024

Please add an entry in the release notes

Is that a new requirement ?
We've never asked this previously. I know what I'm saying as I usually update the release notes for STM32 related changes and this is quite unusual that new boards additions are documented directly in the release notes.
Also, considering this is first contribution from @darkmoon32 I would not make this blocking.

@aescolar
Copy link
Member

Please add an entry in the release notes

Is that a new requirement ?

We do have a section for that:
https://docs.zephyrproject.org/latest/releases/release-notes-3.7.html#boards-soc-support
And I think it is worthwhile adding a line mentioning it (it is easy enough). But as you are the maintainer of this area, if you don't think it is worthwhile doing for ST boards I remove the request.

@erwango
Copy link
Member

erwango commented May 14, 2024

We do have a section for that:
https://docs.zephyrproject.org/latest/releases/release-notes-3.7.html#boards-soc-support
And I think it is worthwhile adding a line mentioning it (it is easy enough). But as you are the maintainer of this area, if you don't think it is worthwhile doing for ST boards I remove the request.

I was just saying we're not usually making it blocking, specially for first time contributors and also because this is not documented in contribution guidelines (last time I've checked, IIC).
I totally agree this should be documented but I'm updating this section at each release, and in any case I'll end up verifying/fixing it (as, as a maintainer, I should check this is updated and complete what is missing if needed).

@aescolar aescolar merged commit c8faa1e into zephyrproject-rtos:main May 14, 2024
21 checks passed
Copy link

Hi @darkmoon32!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

@darkmoon32 darkmoon32 deleted the stm32h745i-disco branch May 14, 2024 16:21
@henrikbrixandersen
Copy link
Member

@darkmoon32 Not sure how you tested the added CAN functionality on this board, but the configuration is wrong and causes build warnings (fix provided here: #72787).

You added CAN controllers and CAN transceivers, but neglected to add can as supported in boards/st/stm32h745i_disco/stm32h745i_disco_stm32h745xx_m7.yaml? No CAN tests will be compiled/run for this board configuration (leading to issues like the one I have just solved).

@darkmoon32
Copy link
Contributor Author

Hi @henrikbrixandersen ,
I have run the test for the tests/drivers/can/api by hand. I guess I did not notice the warnings or I just ignored them because I wasn't sure if my change caused them or if were already present. I will me more careful next time.

In your PR, shouldn't can be listed in the boards/st/stm32h745i_disco/stm32h745i_disco_stm32h745xx_m7.yaml? As you suggested.

@henrikbrixandersen
Copy link
Member

henrikbrixandersen commented May 15, 2024

In your PR, shouldn't can be listed in the boards/st/stm32h745i_disco/stm32h745i_disco_stm32h745xx_m7.yaml? As you suggested.

No, I should not be the one adding that as I do not have access to the hardware. If you can verify, with "can" added to boards/st/stm32h745i_disco/stm32h745i_disco_stm32h745xx_m7.yaml, that the following tests all pass on hardware, please open a new PR for listing CAN as supported:

west twister -p stm32h745i_disco/stm32h745xx/m7 --device-testing --device-serial ... -v -t can

Please include the output of the command above in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: UART Universal Asynchronous Receiver-Transmitter platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants