Skip to content
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

Peripherials missing addressBlock element #25

Closed
evilmav opened this issue Jun 7, 2021 · 8 comments · Fixed by #26
Closed

Peripherials missing addressBlock element #25

evilmav opened this issue Jun 7, 2021 · 8 comments · Fixed by #26
Labels
bug Something isn't working

Comments

@evilmav
Copy link

evilmav commented Jun 7, 2021

According to CMSIS-SVD docs:

addressBlock:| Specify an address range uniquely mapped to this peripheral. A peripheral must have at least one
address block, but can allocate multiple distinct address ranges. If a peripheral is derived from another peripheral, the
addressBlock is not mandatory.

It seems that generated SVDs are generally missing this element. This breaks e.g. PlatformIO Debugger.

Rahix added a commit that referenced this issue Jun 9, 2021
As mandated by the SVD specification, create <addressBlock> elements for
each continuous memory range which uniquely belongs to this periperal.

Ref: #25
@Rahix Rahix linked a pull request Jun 9, 2021 that will close this issue
@Rahix Rahix added the bug Something isn't working label Jun 9, 2021
@Rahix
Copy link
Owner

Rahix commented Jun 9, 2021

You're right, atdf2svd is violating the spec here... Can you please try out whether #26 yields SVD files that fix usage with the PlatformIO Debugger?

@evilmav
Copy link
Author

evilmav commented Jun 10, 2021

Thank you for your effort! Vscode parser does not crash any more, but does not show peripherials either... I would not bet it's the svd at fault though: I've cut out reduced a sample from the generated SVD for testing:

Chunk of XML
<?xml version="1.0" encoding="UTF-8"?>
<device>
  <vendor>Atmel</vendor>
  <name>ATmega1284P</name>
  <addressUnitBits>8</addressUnitBits>
  <size>8</size>
  <access>read-write</access>
  <resetValue>0</resetValue>
  <resetMask>0xff</resetMask>
  <peripherals>
    <peripheral>
      <name>AC</name>
      <description>Analog Comparator</description>
      <baseAddress>0x50</baseAddress>
      <addressBlock>
        <offset>0x0</offset>
        <size>0x1</size>
        <usage>registers</usage>
      </addressBlock>
      <registers>
        <register>
          <name>ACSR</name>
          <description>Analog Comparator Control And Status Register</description>
          <addressOffset>0x0</addressOffset>
          <access>read-write</access>
          <fields>
            <field>
              <name>ACIS</name>
              <description>Analog Comparator Interrupt Mode Select bits</description>
              <bitRange>[1:0]</bitRange>
              <access>read-write</access>
              <writeConstraint>
                <useEnumeratedValues>true</useEnumeratedValues>
              </writeConstraint>
              <enumeratedValues>
                <enumeratedValue>
                  <name>VAL_0x00</name>
                  <description>Interrupt on Toggle</description>
                  <value>0</value>
                </enumeratedValue>
                <enumeratedValue>
                  <name>VAL_0x01</name>
                  <description>Reserved</description>
                  <value>1</value>
                </enumeratedValue>
                <enumeratedValue>
                  <name>VAL_0x02</name>
                  <description>Interrupt on Falling Edge</description>
                  <value>2</value>
                </enumeratedValue>
                <enumeratedValue>
                  <name>VAL_0x03</name>
                  <description>Interrupt on Rising Edge</description>
                  <value>3</value>
                </enumeratedValue>
              </enumeratedValues>
            </field>
            <field>
              <name>ACIC</name>
              <description>Analog Comparator Input Capture Enable</description>
              <bitRange>[2:2]</bitRange>
              <access>read-write</access>
            </field>
            <field>
              <name>ACIE</name>
              <description>Analog Comparator Interrupt Enable</description>
              <bitRange>[3:3]</bitRange>
              <access>read-write</access>
            </field>
            <field>
              <name>ACI</name>
              <description>Analog Comparator Interrupt Flag</description>
              <bitRange>[4:4]</bitRange>
              <access>read-write</access>
            </field>
            <field>
              <name>ACO</name>
              <description>Analog Compare Output</description>
              <bitRange>[5:5]</bitRange>
              <access>read-write</access>
            </field>
            <field>
              <name>ACBG</name>
              <description>Analog Comparator Bandgap Select</description>
              <bitRange>[6:6]</bitRange>
              <access>read-write</access>
            </field>
            <field>
              <name>ACD</name>
              <description>Analog Comparator Disable</description>
              <bitRange>[7:7]</bitRange>
              <access>read-write</access>
            </field>
          </fields>
        </register>
      </registers>
    </peripheral>
  </peripherals>
</device>

Peripherials view stays empty, but it starts working if I set addressBlock.size to 0x4 or above. Unless there is some metadata missing and defaulting to 32bits, this seems more like vscode bug then SVD - chances are it was never tested with anything but ARM... Unfortunately, I can't figure out where their parser code even is...

@Rahix
Copy link
Owner

Rahix commented Jun 10, 2021

but it starts working if I set addressBlock.size to 0x4 or above.

Yeah that very much sounds like they hard coded a minimum register size of 32 bits or something similar. Not sure if there's anything we can do about it short of a fix on their side...

@evilmav
Copy link
Author

evilmav commented Jun 10, 2021

Agree, just would have been nice to validate it in some way. The issue is in platformio-vscode-debug, for which I can not find the source although it's claimed to be open-sourced. I've created an Issue.

BTW, CMSIS provided SVDConv is claimed to be able to verify SVD files. Unfortunately, I couldn't get it to run without segfault on linux so far.

@Rahix
Copy link
Owner

Rahix commented Jun 10, 2021

BTW, CMSIS provided SVDConv is claimed to be able to verify SVD files. Unfortunately, I couldn't get it to run without segfault on linux so far.

Yeah it might be a good idea to validate the output of atdf2svd, I'm sure this isn't the only spec violation... To be honest we built it to barely emit enough output to make svd2rust work, with little regard for the actual SVD spec... Maybe there is a machine-readable schema available somewhere which we could compare against a generated file, if SVDConv really fails to run?

@evilmav
Copy link
Author

evilmav commented Jun 10, 2021

There is this: CMSIS-SVD Schema File.

I've fed a generated SVD to it, the output is:

ATmega1284P.svd:2: element device: Schemas validity error : Element 'device': The attribute 'schemaVersion' is required but missing.
ATmega1284P.svd:5: element addressUnitBits: Schemas validity error : Element 'addressUnitBits': This element is not expected. Expected is one of ( series, version ).
ATmega1284P.svd fails to validate

Though the latter error seems to be due to sequence, which is unlikely to confuse a real parser.

PS Tried to run SVDConv from latest CMSIS release on windows. No luck either. Doesn't seem like anyone actually uses this thing ^^

@maxgerhardt
Copy link

If I can chirp in on this for a brief moment, I think there are good reference SVD files at https://github.com/gicking/STM8_headers for STM8 devices which also have lots of 8-bit registers as the AVR, and these ones were explicitly validated using SVDConv by the author to find all errors -- maybe you can find an error in this converter / source files easier with that.

@Rahix
Copy link
Owner

Rahix commented Jun 10, 2021

Interesting. @maxgerhardt, can you confirm that even the registers in register blocks with size less than 4 are showing up from those SVD files?

@Rahix Rahix closed this as completed in #26 Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants