atdf2svd icon indicating copy to clipboard operation
atdf2svd copied to clipboard

Implement union peripherals with clustering (TCA single mode / split mode on attiny's)

Open hammerlink opened this issue 8 months ago • 3 comments

This PR addresses part of issue #4, specifically adding support for union types.

The TCA0 modes were already implemented at some point, but the functionality was lost in the commit history. I reintroduced them to peripherals with both modes clustered.

I noticed that the svd2rust crate allows peripherals to use clustered registers instead of direct registers. This feature allows a good handling for the TCA Peripheral as it has 2 different modes, Single or Split.

Example solution


        dp.tca0.tca_single().ctrla().write(|w| w.clksel().div1());
        dp.tca0.tca_split().ctrla().write(|w| w.enable().set_bit());

Clustering example in svd

    <peripheral>
      <name>TCA0</name>
      <description>16-bit Timer/Counter Type A</description>
      <baseAddress>0x00000A00</baseAddress>
      <registers>
        <!-- default registers come here <register>...</register> -->
        
        <!--clusterered registers -->
        <cluster>
          <name>TCA_SPLIT</name>
          <description>16-bit Timer/Counter Type A - Split Mode</description>
          <addressOffset>0x0</addressOffset>

          <!-- clustered registers <register>...</register> -->
        </cluster>
      </registers>
    </peripheral>

Example ATDF:

        <!-- Sub register group referenced by the union, contains the actual registers  -->
        <register-group caption="16-bit Timer/Counter Type A - Single Mode"
                         name="TCA_SINGLE"
                         size="0x40">
            <register caption="Compare 0" name="CMP0" offset="0x28" rw="RW" size="2"/>
            <!-- multiple <register>...</register> -->
         </register-group>

        <!-- Module union register group, targets the sub register groups -->
        <register-group caption="16-bit Timer/Counter Type A" class="union" name="TCA" size="0x40" union-tag="TCA.SINGLE.CTRLD.SPLITM">
            <register-group name="SINGLE" name-in-module="TCA_SINGLE" offset="0" union-tag-value="0"/>
            <register-group name="SPLIT" name-in-module="TCA_SPLIT" offset="0" union-tag-value="1"/>
        </register-group>

Testing: Tests passed locally.

Please review and let me know if additional changes or tests are needed!

notes

This only partially fixes the #4, as there are sub register-groups without the module register group being a union class (f.i. avr64du28 - "USB" peripheral). This was out of scope for my changes. This would need further analyses of how to include the registers of these sub groups. I already prepared the atdf peripherals to include these references, but without further implementation. It looks like these sub register-groups can be recursive as well.

hammerlink avatar Apr 29 '25 08:04 hammerlink

Thanks @Rahix for the thorough review, it is much appreciated!

I've rebased the branch to include the ATxmega128A1 regression test as requested.

Implementation Updates

  • I've restructured the register group handling with your suggestions in mind, which provides a much cleaner representation of the data and eliminates the potential for unused/ignored data issues.

  • While implementing these changes, I encountered cases in actual ATDF files (such as avr64du28.atdf) that demonstrate nested register groups requiring recursive handling:

<register-group caption="USB Device Controller EP_TABLE" name="USB_EP_TABLE" size="0x122">
   <!-- containing multiple registers -->
   <register-group caption="USB Device Controller EP"
                   count="0x10"
                   name="EP"
                   name-in-module="USB_EP_PAIR"
                   offset="0x20"
                   size="0x100"/>
</register-group>
<register-group caption="USB Device Controller EP_PAIR" name="USB_EP_PAIR" size="0x10">
   <register-group caption="USB Device Controller OUT"
                   name="OUT"
                   name-in-module="USB_EP"
                   offset="0x00"
                   size="0x08"/>
   <register-group caption="USB Device Controller IN"
                   name="IN"
                   name-in-module="USB_EP"
                   offset="0x08"
                   size="0x08"/>
</register-group>
<register-group caption="USB Device Controller EP" name="USB_EP" size="0x8">
   <!-- containing multiple registers -->
</register-group>
  • This led to a full recursive implementation that handles both the union and non-union cases, unintentionally implementing issue #4 completely. But to keep the changes simple I did use the is_union bool. So for the moment it only works for the class=union. Though refactoring this boolean will fully enable the nested register group support for all atdf. There are actually quite a few impacted mcus.

Address & Offset Handling

I noticed some complexities with the current address/offset calculation approach:

  • The original implementation stored absolute addresses on registers and calculated base addresses afterward
  • This approach becomes problematic with nested register groups
  • The calculated base addresses often differ from what's defined in the ATDF instance

I've modified this to more closely align with how ATDF and SVD both use relative offsets between parents and children, which simplifies the nested register-group handling.

hammerlink avatar May 01 '25 06:05 hammerlink

Hi @Rahix , just checking in to see if you’ve had a chance to take a look at the updated PR. Let me know if there’s anything else I can do to move this forward. Thanks!

hammerlink avatar May 12 '25 19:05 hammerlink

Didn't get to it yet, but your PR is on my list :) Will ping you once I have reviewed your updated changeset!

Rahix avatar May 13 '25 10:05 Rahix