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

Enable flash driver for STM32G4 #838

Merged
merged 2 commits into from Mar 17, 2022
Merged

Conversation

rasmuskleist
Copy link
Contributor

The flash implementation has been tested on STM32g474ret6. It does not implement bank selection for dual bank modes. Also it resolves and issue with g0 devices where the EOP flag should be cleared after programming.

Copy link
Member

@rleh rleh left a comment

Choose a reason for hiding this comment

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

Hi and welcome to modm!

Thanks for your pull request.
I have a few comments, see below.

src/modm/platform/flash/stm32/flash.cpp.in Show resolved Hide resolved
src/modm/platform/flash/stm32/flash.hpp.in Show resolved Hide resolved
@rleh
Copy link
Member

rleh commented Mar 15, 2022

Could you add an example for on of our STM32G4 dev boards (Nucleo-G431KB, Nucleo-G431RB, Nucleo-G474RE; if you have access to one of them?), so that the CI actually compiles the code? I suggest copying the nucleo_g071rb/flash example.

@rleh rleh changed the title Feature flash Enable flash driver for STM32G4 Mar 15, 2022
@rleh rleh added the ci:hal Triggers the exhaustive HAL compile CI jobs label Mar 16, 2022
Copy link
Member

@rleh rleh left a comment

Choose a reason for hiding this comment

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

Tested on my Nucleo-G431KB:

Reboot␊
Erasing sectors [32, 64)␊
Erasing done in 704757us with errors: 0␊
Erasing with 95kiB/s␊
Programming done in 700045us with errors: 0␊
Programming with 95kiB/s␊

Works ✅

Thanks!

@rleh
Copy link
Member

rleh commented Mar 16, 2022

I have now triggered the extensive HAL tests.
If they run successfully, the commits just need to be squashed into two commits (e.g. Adapt and enable flash driver for STM32G4 + [example] Add flash examples for STM32G4) and then we can merge.
Do you want to squash the commits or should I do it?

@rasmuskleist
Copy link
Contributor Author

Feel free to do it!

Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

Nice!

@rleh rleh removed the ci:hal Triggers the exhaustive HAL compile CI jobs label Mar 16, 2022
@rleh
Copy link
Member

rleh commented Mar 16, 2022

Feel free to do it!

Sorry @rasmuskleist, but I am not able to push to the branch of this Pull Request, probably because the [ ] Allow edits and access to secrets by maintainers checkbox is not ticked:
image
(see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork for more information)

Could you tick the checkbox for me to push the squashed commits?
If that doesn't work (there could be an issue with the permissions because your modm repo is in the https://github.com/DanSTAR-DTU namespace and not in a personal namespace) it would be great if you put the squashed commits I pushed here on a branch in my repo on your branch.
Let me know if you need support in using Git for this!

@rasmuskleist
Copy link
Contributor Author

It does not seem like I can let others work on the pull request unfortunately. Instead I added your repo as a remote and rebased my origin/feature-flash to rleh/feature-flash. I hope this is what your meant otherwise I would like to get some Git support.

@rleh rleh merged commit f7dff92 into modm-io:develop Mar 17, 2022
@rleh
Copy link
Member

rleh commented Mar 17, 2022

Perfekt, thanks!

@twast92 twast92 deleted the feature-flash branch May 5, 2022 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants