Fix handling of derived peripherals and interrupt names#5168
Open
amken3d wants to merge 1 commit intotinygo-org:devfrom
Open
Fix handling of derived peripherals and interrupt names#5168amken3d wants to merge 1 commit intotinygo-org:devfrom
amken3d wants to merge 1 commit intotinygo-org:devfrom
Conversation
Ensure proper resolution of `DerivedFrom` peripherals with error checks for missing references. Clean up interrupt names to remove extra spaces and ensure consistency. This improves the reliability of the SVD parsing tool.
Member
|
What other platforms are affected by this? (It seems like this a breaking change, even if it's a fix.) |
Author
|
The tool ran cleanly on all chip familiies. The nil check on derivedFrom catches SVD bugs rather than introducing new failures. Tracking by peripheral name (not group name) is correct per SVD spec since derivedFrom references peripheral names. The generated output matches existing files semantically (same number of definitions), with only minor whitespace formatting differences. I checked this by diffing the outputs for rp2040,rp2350 and stm32g0b1. |
Member
|
Sorry, I thought this was a fix for #5154 which would have different generated code. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
The lib/stm32-svd submodule was updated from e6db8e3 to 5b13c6d during tinygo-org/stm32-svd#11. This update includes new device families and updated register definitions.
Issue 1: orderPeripherals tracking by wrong key
Problem: The orderPeripherals function in gen-device-svd.go is designed to reorder peripherals so that base peripherals are processed before peripherals that derive from them (via SVD's derivedFrom attribute). However, it was tracking known peripherals by their groupName, not their actual name.
Example from stm32u595.svd:
I2C1
I2C
...
I2C5
...
SEC_I2C5
...
The old code would:
Fix: Track peripherals by their name field since that's what derivedFrom references.
Issue 2: Whitespace in interrupt names
Problem: Some SVD files in the updated stm32-svd contain interrupt names with leading whitespace (e.g., FLASH in stm32l0x1.svd). The generator was using these names directly without sanitization, producing invalid Go code:
func interrupt FLASH() { // Invalid: space in function name
callHandlers(IRQ_ FLASH) // Invalid: space in identifier
}
Fix: Apply strings.TrimSpace and cleanName to interrupt names before use.
These issues only surfaced with the newer SVD files because: