atdf2svd icon indicating copy to clipboard operation
atdf2svd copied to clipboard

Fix parsing of nested register groups

Open Rahix opened this issue 5 years ago • 8 comments

Example: ATxmega128A1

Some ATDF files contain nested <register-group /> elements, for example:

<register-group caption="DMA Controller" name="DMA" size="80">
  <register caption="Control" name="CTRL" offset="0x00" size="1">
    ...
  </register>
  ...
  <register-group caption="DMA Channel 0" name="CH0" offset="0x10" name-in-module="DMA_CH"/>
  <register-group caption="DMA Channel 1" name="CH1" offset="0x20" name-in-module="DMA_CH"/>
  <register-group caption="DMA Channel 2" name="CH2" offset="0x30" name-in-module="DMA_CH"/>
  <register-group caption="DMA Channel 3" name="CH3" offset="0x40" name-in-module="DMA_CH"/>
</register-group>

These register groups are defined right next to this one, in the same <module>:

<register-group caption="DMA Channel" name="DMA_CH" size="16">
  <register caption="Channel Control" name="CTRLA" offset="0x00" size="1">
    ...
  </register>
</register-group>

The parser needs to expand those sub-register groups into registers in the peripheral, probably by appending the names. E.g. the above example would create a CH0_CTRLA register.

Rahix avatar Jul 25 '20 16:07 Rahix

I've pushed commit 59bf4b3d660a ("peripheral: Make sure child nodes have the correct name") to at least make files affected by this generate SVD output for all the other stuff.

Rahix avatar Jul 25 '20 19:07 Rahix

I wanted to tackle that one. However, it does not seem to be that simple (not the implementation, but the way how to handle it).

On the attiny412 the TCA0 peripheral can be in one of two modes: Normal mode and Split mode. Similar to # , depending on the ... register all other register can have different "meanings". E.g.:

<module caption="16-bit Timer/Counter Type A" id="I2117" name="TCA">
      <register-group caption="16-bit Timer/Counter Type A - Single Mode" name="TCA_SINGLE" size="0x40">
        ...
        <register caption="Control A" initval="0x00" name="CTRLA" offset="0x00" rw="RW" size="1">
          <bitfield caption="Clock Selection" mask="0xe" name="CLKSEL" rw="RW" values="TCA_SINGLE_CLKSEL"/>
          <bitfield caption="Module Enable" mask="0x1" name="ENABLE" rw="RW"/>
        </register>
        ...
      </register-group>
      <register-group caption="16-bit Timer/Counter Type A - Split Mode" name="TCA_SPLIT" size="0x40">
        <register caption="Control A" initval="0x00" name="CTRLA" offset="0x00" rw="RW" size="1">
          <bitfield caption="Clock Selection" mask="0xe" name="CLKSEL" rw="RW" values="TCA_SPLIT_CLKSEL"/>
          <bitfield caption="Module Enable" mask="0x1" name="ENABLE" rw="RW"/>
        </register>
        ...
      </register-group

How should those two groups - the register in the above case have per coincidence the same meaning (others don't) - involving the same register addresses (but with different meaning) be combined? Or shouldn't they be combined (which in my opinion would violate the ownership/soundness of the generated code)?

trembel avatar Nov 29 '20 21:11 trembel

If the register have the same name in both cases, we could just add redundant bitfields with different meanings. But I'm not 100% sure if the register always have the same name.

trembel avatar Nov 29 '20 21:11 trembel

In the attached files are all occurrences of the nested register-groups. For all MCUs of the newer generation (except ATxmega), its only the TCA making problems. For the ATxmega also other peripherals have nested groups.

errors.txt

trembel avatar Nov 30 '20 17:11 trembel

@trembel is there anything I can do to assist in this? I'm trying to add support for the ATtiny1607.

chris-ricketts avatar Dec 08 '20 15:12 chris-ricketts

@trembel is there anything I can do to assist in this? I'm trying to add support for the ATtiny1607.

First it must be decided how to handle it. Afterwards feel free to tackle this issue, I'll probably can't find any time this year..

trembel avatar Dec 08 '20 16:12 trembel

Just to make sure we're all on the same page: This "feature" is not a blocker for supporting the devices; any peripheral that is not using nested register-groups will work fine with the current atdf2svd. So for basic support for an MCU, this is most likely not necessary. It will only become an issue later, when trying to write HAL drivers for peripherals that use those nested register-groups ...

Rahix avatar Dec 08 '20 18:12 Rahix

In the c header files, modes are modelled as a union of structs:

/* 16-bit Timer/Counter Type A - Single Mode */
typedef struct TCA_SINGLE_struct
{
    register8_t CTRLA;  /* Control A */
    ...
} TCA_SINGLE_t;

/* 16-bit Timer/Counter Type A - Split Mode */
typedef struct TCA_SPLIT_struct
{
    register8_t CTRLA;  /* Control A */
   ...
} TCA_SPLIT_t;

/* 16-bit Timer/Counter Type A */
typedef union TCA_union
{
    TCA_SINGLE_t SINGLE;  /* 16-bit Timer/Counter Type A - Single Mode */
    TCA_SPLIT_t SPLIT;  /* 16-bit Timer/Counter Type A - Split Mode */
} TCA_t;

Nested register-groups are modelled as structs of structs:

/* DMA Channel */
typedef struct DMA_CH_struct
{
    register8_t CTRLA;  /* Channel Control */
    register8_t CTRLB;  /* Channel Control */
    register8_t ADDRCTRL;  /* Address Control */
    ...
} DMA_CH_t;

/*
--------------------------------------------------------------------------
DMA - DMA Controller
--------------------------------------------------------------------------
*/

/* DMA Controller */
typedef struct DMA_struct
{
    register8_t CTRL;  /* Control */
    ...
    DMA_CH_t CH0;  /* DMA Channel 0 */
    DMA_CH_t CH1;  /* DMA Channel 1 */
    DMA_CH_t CH2;  /* DMA Channel 2 */
    DMA_CH_t CH3;  /* DMA Channel 3 */
} DMA_t;

xoriath avatar Mar 18 '21 16:03 xoriath