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
Add STM CMSIS and HAL files as a submodule called "stm32lib" #3284
Conversation
Looks good generally. One concern with tagging HAL versions is whether you can mix and match from versions. I am using L4_V1.6.0 to support L451 and L496 but with the SD card driver from L4_V1.3.0. The SD card driver has changed and breaks MicroPython. Only alternative is to update MicroPython SD driver to latest HAL and also update F4 and F7 HALs at the same time. I don't know enough about git submodules. If I have a dev branch of MicroPython and a dev branch of stm32lib for developing a new port will the submodule branch change automatically when I switch MicroPython from master to dev and back? In other words can a project branch specify the submodule branch it wants? Is there a better git workflow for development of STM32HAL ports? |
Looks good. |
Yes I had this concern as well. One solution is to make separate repositories for each MCU variant, eg stm32f4lib, stm32f7lib, stm32f4lib, etc, and then manage any changes within those repos by creating new branches and applying patches (eg by holding back the SD driver in the L4 repo). Then you'd need to include multiple git submodules. I think that would be a big pain, having multiple submodules for each MCU variant, and anyway the idea of the STM Cube HAL is that at a given point in time (ie for a given set of versions for each MCU variant) the API should be equivalent across all MCUs. So that's the argument to keep everything together in one repo, ie the stm32lib repo.
As said above, the idea of the STM Cube HAL is to provide a consistent API across MCUs, so the right thing to do here is update all HALs at the same time to a consistent version, and update uPy to work with them all. It's easier to do that upgrade with a separate repo that tracks the vendor releases (ie STM releases). Anyway, the point is that putting all the HAL stuff in a separate repo doesn't make things any worse than they currently are. If you're maintaining new L4 HAL with a held-back SD driver from a previous version, then you should be able to do that with a submodule (probably easier).
It won't automatically, you need to do "git submodule update". But that's easy to do and it'll anyway warn you (if you type "git status") if the submodule is wrong.
Yes, very easily! That's one of the reason submodules are useful, they are just a pointer to a commit in a remote/separate repo, and you can change that at will and have different branches point to different commits. |
@dpgeorge Thanks for the explanation. I will wait for your commit and continue testing the HAL update to latest versions. There is now an L4_V1.8.1 patch. Could you please update vendor branch with it? |
@chrismas9 now done. |
In 8388ec4 this repo was updated to use the latest HAL library. The main changes were with the SD card driver. |
This upgrades the HAL to the versions: - F4 V1.16.0 - F7 V1.7.0 - L4 V1.8.1 The main changes were in the SD card driver. The vendor changed the SD read/write functions to accept block number intead of byte address, so there is no longer any need for a custom patch for this in stm32lib. The CardType values also changed, so pyb.SDCard().info() will return different values for the 3rd element of the tuple, but this function was never documented.
Remove event handler before setting it to NULL
As discussed in #3283, this PR moves all the STM CMSIS and HAL files to a submodule. The submodule is named "stm32lib" to emphasise that it's for the STM32 line of MCUs (as opposed to stmlib), and it's located at https://github.com/micropython/stm32lib.
There should be no regressions using this new submodule because it contains on one of its branches (the one that's linked here) an (almost) exact copy of the code that is removed.
This PR should be good to go but please voice any concerns.