This actually works locally, being sure to use the same tool versions. Not sure what's up with CI.

Need to decide how to separate the new device family parts.

Also I remember when diffing the mmaps there were some regressions. Probably have to add some of the patches back but with pieces removed.

I've started reviewing this for the existing sub-families, and the new SVDs are certainly an improvement.

RM0399: A few improvements and I only spotted one regression (extra fields in DBGMCU.CR)

RM0433: More extensive changes, but always improvements I think. Doesn't seem to affect anything in the HAL.

RM0455: Again more extensive changes. There are many renames in RCC/PWR like D3 -> SRD and D1/D2 -> CD, and whilst they now match the RM this will cause a lot of noise in the HAL to try to support both naming schemes. I think we just have to accept that unfortunately. The CRC peripheral enable has moved from AHB4ENR to AHB1ENR which is pretty wild, but it does now match the RM.

One of the SVD changes for RM0455 adds some enumeratedValues with names like B_0x0, B_0x1... rather than something useful.

This PR from @burrbull is very useful for removing those enumeratedValues and allowing peripherals/rcc/rcc_h7.yaml to apply cleanly.

@richardeoin

Yes, I was wondering what to do about the D3->SRD rename. I could tolerate it a bit more if all their newer parts started using that nomenclature but this is unfortunate.

I hadn't paid attention to the enumeratedValues and was happy that ST was finally improving their SVDs but apparently we can't have nice things. It's unbelievable to me they don't use a single source of truth to generate the SVDs, their headers and the reference manual. These things should not have typos. There should not be engineer hours paid to manually update a 100k line XML file.

Sorry, I had to rant.

It is really strange that they've made the D3->SRD rename for the RM0455 sub-family but not for the (presumably newer) H73X/H72X parts. Just one of those mysteries I guess.. I completely understand the rant, I guess someone at ST had the bright idea to add enumeratedValues in there but didn't have any useful names to use.

The CI is failing because the Compare mmaps job only uses the ./devices and ./peripherals directories from the PR, and everything else is from master. However the mmap comparisons would be quite useful for this PR, despite the change in the SVDs

Unfortunately the restriction is a security one: the CI job for comparing mmaps needs access to repo secrets to know the key to push to the mmaps repo and to leave a comment on the PR, and so we can't allow it to execute arbitrary code from a PR. However, to extract the SVDs requires running the extract script (which is changed in this PR, for instance). That's why it just copies the YAML files and not the Makefile or SVDs from the PR.

Changing the workflow file in the PR won't help either: PRs with pull_request_target targets like this one always use the workflow file from the base repository (for the same security reason).

I think it could be possible to split the workflow in two steps; the first runs as a normal pull_request target without any secret access or permissions, but builds the mmaps from the PR and packages them into a zip which it uploads as an artifact and then triggers a second CI job which runs with access to the secrets and downloads the artifact and uses the zip files (without executing any code they might contain). But, that was much more complicated to set up, which is why I went for this approach originally, which works unless the SVDs have been changed.

Of course, it's still possible to run the mmap comparison locally too, following the same steps as the CI job.

That makes complete sense, and I was half expecting that modifying the workflow file in the PR would be disallowed.

Running the mmap comparison locally works well of course, but where possible having them automatically linked on the PR is rather nice

Here are the current mmap diffs as of c8436d9:

https://gist.github.com/mattico/378af521ca39ebf41a324a5c8c40613c

I can now successfully build the HAL against this PR, so I'm removing the draft status.

I'm not sure how best to review this, since even going through the mmap diffs by hand is not really feasible. Being able to build the HAL gives some confidence, but I'm sure there are still errors. Perhaps it's worth focusing on the core peripherals (RCC, PWR, SYSCFG) and accepting that errors elsewhere can be fixed by additional PRs.

Yea, agreed. It's not like we were sure there were zero errors previously anyway.

I finished reviewing the RCC, PWR and SYSCFG peripherals on a sample of existing parts:

• stm32h743v No changes
• stm32h747cm7 Changes to SYSCFG only, now matches RM0399 Rev 3
• stm32h7b3 Checked against RM0455 Rev 6. RCC and PWR match, except for a couple of cases we have extra registers (for example here). I think they're all justifiable. Added a couple of commits to fix SYSCFG.

Therefore I think this PR is ready to merge!

Build succeeded:

• Check (stm32f0)
• Check (stm32f1)
• Check (stm32f2)
• Check (stm32f3)
• Check (stm32f4)
• Check (stm32f7)
• Check (stm32g0)
• Check (stm32g4)
• Check (stm32h7)
• Check (stm32l0)
• Check (stm32l1)
• Check (stm32l4)
• Check (stm32l5)
• Check (stm32mp1)
• Check (stm32wb)
• Check (stm32wl)