Revise some API docs due to aggressive combining #35
Conversation
This file contains 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
mciantyre
force-pushed
the
combine-config-never-combine
branch
from
c7de028 to
672e255
Compare
The goal is to correct the documentation generated for the CCM CCGR
fields. Based on how the combiner works, these fields are generally
considered combine-able across all chips. So when we generate code for
the 1061 CCM, we're permitted to use the fields from the 1011 CCM. The
1011 CCM has many more reserved gates than the 1062 CCM, so those end up
leaking into the documentation.
This commit drops the fields that are marked "reserved" in their
human-readable description. It's not clear why these are provided in the
SVD. By dropping these reserved fields, we change the "field plurality"
heuristic that the combiner uses to understand equivalent fieldsets.
Notice how the CCGR documentation corrects itself in the 1061 CCM. We
also generate less code, and less chance for humans to poke at reserved
registers.
I'm not a fan of hard-coding this behavior into the SVD importer. But it
was easy to prototype. I might consider a transform that does this
across all document-able items.
I didn't use a 'contains("reserved")' predicate for the field skip.
There are some fields that are documented with "all other values are
reserved," so that larger-reaching predicate would incorrectly remove
those fields. I briefly scanned these changes to make sure removals
seemed appropriate.
Supply a regex that matches an IR path. If the combiner matches that path, it always signals "not equivalent." This prevents the combiner from combining the element. Apply the configuration to CCM CCGR registers. This fixes the documentation that would not be fixed by the previous commit (it's a superficial documentation change, but nevertheless a good test that the "never combine" behavior is working).
Similar to strict enum names, this configuration instructs the combiner to compare enum variant descriptions, the human-readable strings associated with items. If they're not equal, then the enums cannot be combined. When applied to IOMUXC peripherals, this ensures that the documentation associated with each enum variant is correct. The next commit checks in the updates.
Briefly spot-checked the 1061 IOMUXC to make sure the diff of some registers are correct. See the parent commit for context.
mciantyre
force-pushed
the
combine-config-never-combine
branch
from
672e255 to
7a20156
Compare
mciantyre
force-pushed
the
combine-config-strict-enums
branch
from
1e5e196 to
51f6f70
Compare
The 1020, 1050, and 1060 SVDs qualify these fields as reserved, so the previous commits scrubbed the fields. Patch the SVDs to re-introduce the field.
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.
By aggressively combining IR elements, the combiner could create peripherals that have incorrect documentation. This multi-faceted PR addresses some invalid documentation in the RAL.
First, we remove specific cases of reserved registers. Users shouldn't touch them anyway, and it's nice to remove code. It also forces the combiner to treat fieldsets for CCM clock gates differently. This corrects most of the documentation issues presented in #32, but not all.
To fix all of the clock gate documentation, we build on #34 and introduce a new combiner configuration. A "never combine" configuration should prevent any kind of IR element from combining. It's tested with the CCM clock gate fieldsets.
But there's still documentation inconsistencies in IOMUXC, and I couldn't find a way to express the corrections in terms of "never combine" configurations. So there's another combiner configuration that requires the documentation of an enum variant to match across chips. This fixes IOMUXC documentation.