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

Fix parsing of nested register groups #4

Open
Rahix opened this issue Jul 25, 2020 · 8 comments
Open

Fix parsing of nested register groups #4

Rahix opened this issue Jul 25, 2020 · 8 comments
Labels
help wanted Extra attention is needed

Comments

@Rahix
Copy link
Owner

Rahix commented Jul 25, 2020

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
Copy link
Owner Author

Rahix commented Jul 25, 2020

I've pushed commit 59bf4b3 ("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.

@trembel
Copy link
Contributor

trembel commented Nov 29, 2020

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
Copy link
Contributor

trembel commented Nov 29, 2020

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
Copy link
Contributor

trembel commented Nov 30, 2020

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

@chris-ricketts
Copy link

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

@trembel
Copy link
Contributor

trembel commented Dec 8, 2020

@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..

@Rahix
Copy link
Owner Author

Rahix commented Dec 8, 2020

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 ...

@xoriath
Copy link
Contributor

xoriath commented Mar 18, 2021

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;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants