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

SVD clusters should be used for register groupings #33

Open
jgrivera67 opened this issue Jul 31, 2016 · 1 comment
Open

SVD clusters should be used for register groupings #33

jgrivera67 opened this issue Jul 31, 2016 · 1 comment

Comments

@jgrivera67
Copy link

jgrivera67 commented Jul 31, 2016

Currently, in Freescale Kinetis SVD files (such as https://github.com/posborne/cmsis-svd/blob/master/data/Freescale/MKL25Z4.svd), what logically is an array of a grouping of registers is being represented in the SVD file as separate "dim" arrays of registers. For example:

        <register>
          <dim>6</dim>
          <dimIncrement>0x8</dimIncrement>
          <dimIndex>0,1,2,3,4,5</dimIndex>
          <name>C%sSC</name>
          <description>Channel (n) Status and Control</description>
          **<addressOffset>0xC</addressOffset>**
          <size>32</size>
          <access>read-write</access>
          <resetValue>0</resetValue>
          <resetMask>0xFFFFFFFF</resetMask>
          <fields>
            ...
          </fields>
        </register>
        <register>
          <dim>6</dim>
          <dimIncrement>0x8</dimIncrement>
          <dimIndex>0,1,2,3,4,5</dimIndex>
          <name>C%sV</name>
          <description>Channel (n) Value</description>
          **<addressOffset>0x10</addressOffset>**
          <size>32</size>
          <access>read-write</access>
          <resetValue>0</resetValue>
          <resetMask>0xFFFFFFFF</resetMask>
          <fields>
          ...
          </fields>
        </register>

This does not take advantage of the cluster construct in SVD syntax, which would be
both more readable and would allow tools like SVDConv generate code that is easier to work with. For the example above, the code generated by the ARM SVDConv tool looks like this:

typedef struct {                                    /*!< TPM0 Structure                                                        */
     ...
     __IO uint32_t  C0SC;                              /*!< Channel (n) Status and Control                                        */
     __IO uint32_t  C0V;                               /*!< Channel (n) Value                                                     */
     __IO uint32_t  C1SC;                              /*!< Channel (n) Status and Control                                        */
     __IO uint32_t  C1V;                               /*!< Channel (n) Value                                                     */
     __IO uint32_t  C2SC;                              /*!< Channel (n) Status and Control                                        */
     __IO uint32_t  C2V;                               /*!< Channel (n) Value                                                     */
     __IO uint32_t  C3SC;                              /*!< Channel (n) Status and Control                                        */
     __IO uint32_t  C3V;                               /*!< Channel (n) Value                                                     */
     __IO uint32_t  C4SC;                              /*!< Channel (n) Status and Control                                        */
     __IO uint32_t  C4V;                               /*!< Channel (n) Value                                                     */
     __IO uint32_t  C5SC;                              /*!< Channel (n) Status and Control                                        */
     __IO uint32_t  C5V;                               /*!< Channel (n) Value                                                     */
     ...
} TPM0_Type;`

In contrast, if we rewrite the SVD fragment shown above, using a * cluster*, as shown below:

        <cluster>
            <dim>6</dim>
            <dimIncrement>8</dimIncrement>
            <dimIndex>0-5</dimIndex>
            <name>CHANNELS[%s]</name>
            <description>Grouping of Channels</description>
            <addressOffset>0xC</addressOffset>
            <register>
              <name>CnSC</name>
              <description>Channel (n) Status and Control</description>
              <addressOffset>0x0</addressOffset>
              <size>32</size>
              <access>read-write</access>
              <resetValue>0</resetValue>
              <resetMask>0xFFFFFFFF</resetMask>
              <fields>
               ...
              </fields>
            </register>
            <register>
              <name>CnV</name>
              <description>Channel (n) Value</description>
              <addressOffset>0x4</addressOffset>
              <size>32</size>
              <access>read-write</access>
              <resetValue>0</resetValue>
              <resetMask>0xFFFFFFFF</resetMask>
              <fields>
              ...
              </fields>
            </register>
        </cluster>

the corresponding C code generated by the ARM SVDConv tool would look like this:

typedef struct {
  __IO uint32_t  CnSC;                              /*!< Channel (n) Status and Control                                        */
  __IO uint32_t  CnV;                               /*!< Channel (n) Value                                                     */
} TPM0_CHANNELS_Type;

typedef struct {                                    /*!< TPM0 Structure                                                        */
  ...
  TPM0_CHANNELS_Type CHANNELS[6];                   /*!< Grouping of Channels                                                  */
  ...
} TPM0_Type;

which is both more readable and easier to work with in code that uses it.
So, All the Freescale Kinetis SVD files should be updated to use clusters where
appropriate.

@posborne
Copy link
Collaborator

Unfortunately, due to the license on the SVD files, we don't directly change SVD files as part of this project. We might be able to convince the vendor (in this case Freescale/NXP) to make the change.

CC @flit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants