Fix HSEM Implementation

[Legorooj]

This PR fixes the HSEM implementation for STM32 MCUs. The current implementation does not appear to work on wb55 boards at minimum, and as such needs testing where possible.

List of chips that the current implementation is confirmed to not function correctly on:

• [x] WB55
• [x] WL5x
• [ ] WLEx
• [ ] h723,h725,h730,h733,h735
• [x] h742,h743,h753,h750
• [x] h745,h755,h747,h757
• [ ] h7a3,h7b3,h7b0

Chips that the new implementation is confirmed to work on:

• [x] WB55
• [x] WL5x
• [ ] WLEx
• [ ] h723,h725,h730,h733,h735
• [ ] h742,h743,h753,h750
• [ ] h745,h755,h747,h757
• [ ] h7a3,h7b3,h7b0

This implementation should provide full functionality on all STM MCUs which have a Hardware Semaphore feature.

[Dirbaio]

thanks for working on this! :)

With the trusted label CI will run HIL tests. I can also give you a token so you can cargo run in the HIL test farm locally so you don't have to wait for CI to build stuff. Ping me on Matrix if you want it.

[Dirbaio]

Have you seen #4106 ? seems you guys are working on the same thing.

[Legorooj]

@Dirbaio ha, no, I hadn't, though looking now, that PR will only fix the WB55 where as this one aims to fix the implementation for all of the MCUs supported.

[ckrenslehner]

I did not see it, sorry. I am fine with any implementation :-)

[ckrenslehner]

Is there a reason why you moved your hsem code out into a separate file and rename the mod.rs into old.rs?

[ckrenslehner]

@Dirbaio ha, no, I hadn't, though looking now, that PR will only fix the WB55 where as this one aims to fix the implementation for all of the MCUs supported.

This was not my intention. I kept the values for the h7 mcus as they were, but I moved the RLR values into a separate enum. Looking at your code, you added more h7 variants, though. :-) https://github.com/embassy-rs/embassy/pull/4106/commits/e3034a7cda20865378d0f25d83727716c413c49d#diff-e2b9c0e56c77ee17673e3aa131d3a98e608d3fd12a0c57eed4108b3a86107bcbR73

This is out of inspiration of the stm32-data crate, but I am not sure if this is worth the hassle to really move such things into the data crate and add mcu specific hsem yaml files

[Legorooj]

Is there a reason why you moved your hsem code out into a separate file and rename the mod.rs into old.rs?

HSEM doesn't need multiple files, therefore hsem/mod.rs is slightly more confusing than hsem.rs in my opinion. Additionally, there's no consistency within Embassy for one or the other of these, as there are multiple modules in both patterns. If @Dirbaio has a preference, I am happy to make it match, but if not, then a single hsem.rs file is clearer imo.

[Legorooj]

I'd like input regarding the H7 variants for HSEM @Dirbaio and @ckrenslehner. I cannot seem to get them working, no matter what I try the results do not match what the datasheets say they should. I've even checked against STM's official HAL & my implementation semantically matches. As such, which do you think is the better option:

1. Commit current implementation (a little housekeeping is needed before this happens). H7 MCUs with HSEM should just fail, I could also maybe add a note in the docs for h7 that it is known to have issues.
2. Remove support for H7 CPUs entirely, via features.
3. If you have suggestions?

[ckrenslehner]

Can you explain more about the datasheet mismatch or what exactly is not working? I probably have no access to a H7 so unfortunately I cannot test myself

[ckrenslehner]

Looking through the manual of the h7 and your code, it should work. As commented, the onestep should IMO check the uppermost bit for the lock status.

Have you tested running some cube code on a h7 target? Unfortunately, I do not have a h7 to test with

[Legorooj]

Apologies for the delay on response.

Have you tested running some cube code on a h7 target? Unfortunately, I do not have a h7 to test with

Yes, see the latest ci/build. I've got a bunch of debug statements in there, you should be able to read those and track my confusion!

Looking through the manual of the h7 and your code, it should work. As commented, the onestep should IMO check the uppermost bit for the lock status.

To which comment are you referring?

[ckrenslehner]

To which comment are you referring?

Sorry, I thought I submitted the review. To this comment: https://github.com/embassy-rs/embassy/pull/4092#discussion_r2050044949

My bad, sorry again.

[ckrenslehner]

Yes, see the latest ci/build. I've got a bunch of debug statements in there, you should be able to read those and track my confusion!

Ok, so the actual core ID value is 0 even though it should be 3. That's odd. Can you print the whole RLR and R register?

I just wonder if there is anything non zero inside