rust-embedded / svd2rust

Generate Rust register maps (`struct`s) from SVD files
Apache License 2.0
707 stars 151 forks source link

support <alternateGroup> in registers? #97

Closed japaric closed 1 year ago

japaric commented 7 years ago

The TM4C1230C3PM.svd file defines the MCS register in the I2C0 peripheral like this:

        <register>
          <name>MCS</name>
          <description>I2C Master Control/Status</description>
          <addressOffset>0x00000004</addressOffset>
          <fields>
            <field>
              <name>I2C_MCS_RUN</name>
              <description>I2C Master Enable</description>
              <bitRange>[0:0]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_START</name>
              <description>Generate START</description>
              <bitRange>[1:1]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_ADRACK</name>
              <description>Acknowledge Address</description>
              <bitRange>[2:2]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_ACK</name>
              <description>Data Acknowledge Enable</description>
              <bitRange>[3:3]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_ARBLST</name>
              <description>Arbitration Lost</description>
              <bitRange>[4:4]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_IDLE</name>
              <description>I2C Idle</description>
              <bitRange>[5:5]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_CLKTO</name>
              <description>Clock Timeout Error</description>
              <bitRange>[7:7]</bitRange>
            </field>
          </fields>
        </register>
        <register>
          <name>MCS</name>
          <description>I2C Master Control/Status</description>
          <alternateGroup>I2C0_ALT</alternateGroup>
          <addressOffset>0x00000004</addressOffset>
          <fields>
            <field>
              <name>I2C_MCS_BUSY</name>
              <description>I2C Busy</description>
              <bitRange>[0:0]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_ERROR</name>
              <description>Error</description>
              <bitRange>[1:1]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_STOP</name>
              <description>Generate STOP</description>
              <bitRange>[2:2]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_DATACK</name>
              <description>Acknowledge Data</description>
              <bitRange>[3:3]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_HS</name>
              <description>High-Speed Enable</description>
              <bitRange>[4:4]</bitRange>
            </field>
            <field>
              <name>I2C_MCS_BUSBSY</name>
              <description>Bus Busy</description>
              <bitRange>[6:6]</bitRange>
            </field>
          </fields>
        </register>

This makes svd2rust generate two sets of MCS registers which makes the generated code not compilable. For some reason this doesn't the "overlapping" code that emits a warning about the overlap and only produces code for the first set of bitfields. Perhaps that's because the two overlapping registers have the same name?

According to the SVD specification having these two registers with the same name is valid because one of the has a <alternateGroup> element. This what the specification says about <alternateGroup>:

alternateGroup Specifies a group name associated with all alternate register that have the same name. At the same time, it indicates that there is a register definition allocating the same absolute address in the address space.

What actually this SVD file meant to convey is that when writing to the MCS register only the fields HS, ACK, STOP, START, RUN can be used, but when reading the MCS regsiter then it contains the fields CLKTO, BUSBSY, IDLE, ARBLST, DATACK, ADRACK, ERROR, BUSY. Of course the SVD file doesn't do this right because it says nothing about read vs write mode; also the bit ranges are wrong ([6:6] = 0-bit field).

I think these TM4C fiels may be busted. @whitequark you have worked with these micros using dslite2svd. Do the SVD files generated from dslite ones make use of this alternateGroup feature? Also how do those SVD files deal with these registers with different read / write modes?

whitequark commented 7 years ago

Do the SVD files generated from dslite ones make use of this alternateGroup feature?

No. There are a few (very few) overlaps in the dslite files, specifically...

for TM4C123x:

WARNING ecr overlaps with another register at offset 4. Ignoring.

for TM4C129x:

WARNING ecr overlaps with another register at offset 4. Ignoring.
WARNING gpcfg overlaps with another register at offset 16. Ignoring.
WARNING sdramcfg overlaps with another register at offset 16. Ignoring.
WARNING hb8cfg overlaps with another register at offset 16. Ignoring.
WARNING hb16cfg2 overlaps with another register at offset 20. Ignoring.
WARNING hb16cfg3 overlaps with another register at offset 776. Ignoring.
WARNING hb8cfg4 overlaps with another register at offset 780. Ignoring.
WARNING hb16time overlaps with another register at offset 784. Ignoring.
WARNING hb16time2 overlaps with another register at offset 788. Ignoring.
WARNING hb8time3 overlaps with another register at offset 792. Ignoring.
WARNING hb16time4 overlaps with another register at offset 796. Ignoring.

Analysis:

It would be an one-line change to add <alternateGroup> generation to dslite2svd but I have not bothered because of the relative obscurity of all these registers and lack of svd2rust support. On the part of svd2rust...

  1. In case of two alternateGroup registers describing read-only and write-only parts, just merge them into the same register, and join the names using an _.
  2. In case of two alternateGroup registers that are completely unrelated, the only reasonable choice is to add unsafe methods to the peripheral that cast the untyped reserved address where the registers reside into the appropriate register struct.

It also seems fine to skip the special code generation behavior (but not unsafe qualifier placement) for case 1 and just go with case 2 code structure for everything.

Also how do those SVD files deal with these registers with different read / write modes?

In general SVD files generated by dslite2svd specify the register as read-write and individual fields as read-only, read-write, write-only or what else you have. This is through no effort on part of dslite2svd because DSLite files already contain register-level as well as field-level access modes already.

whitequark commented 7 years ago

@posborne Where do the TM4C files originate from? The official position of Texas Instruments, stated multiple times by their representatives on TI E2E (e.g. this thread dragging from 2014 to 2017), is that there are no available SVD files for TM4C series. The ones in your repository seem completely worthless to me as they do not specify register or field access modes. I suggest removing them from your repository.

posborne commented 7 years ago

Those files were added by @thejpster with this commit: https://github.com/posborne/cmsis-svd/commit/4eb54d1955af6dcad3cb053c444f258c3979ccff which references another thread on the TI forums for the source: https://e2e.ti.com/support/microcontrollers/tiva_arm/f/908/p/261081/966053

burrbull commented 1 year ago

By default we add alternateGroup to register name. This can be disabled by ignore_groups