Skip to content

Handle missing dimIndex on registers #168

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

Merged
merged 1 commit into from
Feb 24, 2018

Conversation

kevinmehall
Copy link
Contributor

@kevinmehall kevinmehall commented Jan 19, 2018

This supersedes @lucasbrendel's similar PR #153 and fixes #150.

Per the SVD spec, dimIndex is not required:

By default, <dimIndex> is a value starting with 0.
Remark: Do not define <dimIndex> when using the placeholder [%s] in <name> or <displayName>.

I interpret this as the element being optional with "%s" and not allowed with "[%s]" (this change does not reject the latter). Atmel SAMD21 SVDs use "%s" with no dimIndex for 0-indexed arrays.

This also tests that dimIndex is sequential and numeric in a single pass without allocating temporary vectors, and fixes the typo adresses -> addresses.

Per the SVD spec, dimIndex is not required:

> By default, <dimIndex> is a value starting with 0.
> Remark: Do not define <dimIndex> when using the placeholder [%s] in
<name> or <displayName>.

I interpret this as it being optional whether the name uses "%s" or
"[%s]". Atmel SAMD21 SVDs use "%s" with no dimIndex for 0-indexed
arrays.

This also tests that dimIndex is sequential and numeric in a single pass
without allocating temporary vectors, and fixes the typo
adresses -> addresses.
@jamesmunns
Copy link
Member

Not sure how I missed this, but this PR also addresses the issues I had mentioned in #177.

👍 from me!

@jamesmunns
Copy link
Member

@therealprof you may want to test your svds with this branch.

@therealprof
Copy link
Contributor

@jamesmunns For the ATSAMD the result is pretty much the same (as far as I can tell from memory it's indentical) as with your PR. The result in this particular case is still broken but I blame Atmel here.

@kevinmehall
Copy link
Contributor Author

kevinmehall commented Feb 23, 2018

@therealprof Yeah, the SAMD SVDs will still need some more features implemented including derivedFrom on registers, register clusters, and building unions for registers at the same address. None of which is actually in violation of the SVD spec as far as I've seen, but yes, they definitely could have done it more elegantly.

@japaric
Copy link
Member

japaric commented Feb 24, 2018

@homunkulus r+

@japaric
Copy link
Member

japaric commented Feb 24, 2018

bors r+

bors bot added a commit that referenced this pull request Feb 24, 2018
168: Handle missing dimIndex on registers r=japaric a=kevinmehall

This supersedes @lucasbrendel's similar PR #153 and fixes #150.

Per the [SVD spec](http://arm-software.github.io/CMSIS_5/SVD/html/elem_special.html#dimElementGroup_gr), dimIndex is not required:

> By default, \<dimIndex\> is a value starting with 0.
> Remark: Do not define \<dimIndex\> when using the placeholder [%s] in \<name\> or \<displayName\>.

I interpret this as the element being optional with "%s" and not allowed with "[%s]" (this change does not reject the latter). Atmel SAMD21 SVDs use "%s" with no dimIndex for 0-indexed arrays.

This also tests that dimIndex is sequential and numeric in a single pass without allocating temporary vectors, and fixes the typo adresses -> addresses.
@bors
Copy link
Contributor

bors bot commented Feb 24, 2018

Build failed

@japaric
Copy link
Member

japaric commented Feb 24, 2018

bors retry

@japaric
Copy link
Member

japaric commented Feb 24, 2018

bors-ng doesn't seem to have a retry command implemented, then ...

bors r+

bors bot added a commit that referenced this pull request Feb 24, 2018
168: Handle missing dimIndex on registers r=japaric a=kevinmehall

This supersedes @lucasbrendel's similar PR #153 and fixes #150.

Per the [SVD spec](http://arm-software.github.io/CMSIS_5/SVD/html/elem_special.html#dimElementGroup_gr), dimIndex is not required:

> By default, \<dimIndex\> is a value starting with 0.
> Remark: Do not define \<dimIndex\> when using the placeholder [%s] in \<name\> or \<displayName\>.

I interpret this as the element being optional with "%s" and not allowed with "[%s]" (this change does not reject the latter). Atmel SAMD21 SVDs use "%s" with no dimIndex for 0-indexed arrays.

This also tests that dimIndex is sequential and numeric in a single pass without allocating temporary vectors, and fixes the typo adresses -> addresses.
@bors
Copy link
Contributor

bors bot commented Feb 24, 2018

Timed out

@japaric
Copy link
Member

japaric commented Feb 24, 2018

All tests passed. Merging manually.

@japaric japaric merged commit 65482c2 into rust-embedded:master Feb 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SVD Registers with no dimIndex
4 participants