Feedback on the ESP32 SVD file

[aykevl]

I've tried switching TinyGo to use the new official SVD file, and of course there are differences with the community version :) Here is what I found:

• The registers FUNC%s_IN_SEL_CFG have the fields FUNC_IN_SEL, FUNC_IN_INV_SEL, and SIG_IN_SEL. That's a bit of repetition. Maybe that's entirely intended, but this repetition wasn't there in the community version of the SVD file (which had fields named SEL, IN_INV_SEL, and IN_SEL). (Feel free to ignore this as "works as intended").
• I think the IO_MUX pins are incorrect. Right now there is the GPIO%s array of registers address 0x3FF49044, which is correct for GPIO0, but is already incorrect for GPIO1 (aka U0TXD) which lives at address 0x3FF49088. I wish these registers were in some sort of logical order, but for the ESP32 this does not appear to be the case.
• The UART has the FIFO_AHB register according to the SVD file, which would be the UART_FIFO_REG according to the reference manual. I wonder why there is a _AHB suffix? It isn't there in the SVD file for the ESP32-C3.
• There is a peripheral named RTCCNTL with a group name of RTC_CNTL? That seems odd. In the ESP32-C3 it's just called RTC_CNTL.
• The SOC_CLK_SEL field of the RTC_CNTL register does not have names for the various clock sources (XTAL, PLL, CK8M, APLL). Maybe this is missing from the data? It was there in the community version of the SVD, which was nice. If it's not in the data, it's not a big loss to write these constants manually. (The same holds true for many other register fields).

Not all of these are necessarily bugs, but some seem to be.

[aykevl]

Actually, the UART FIFO is an odd one. It appears that it would normally be accessed through 0x3ff40000, but because of issues with the DPORT address space should be accessed via address 0x60000000:

• Peripherals accessed by the CPU via 0x3FF40000 ~ 0x3FF7FFFF address space (DPORT address) can also be accessed via 0x60000000 ~ 0x6003FFFF (AHB address). (0x3FF40000 + n) address and (0x60000000 + n) address access the same content, where n = 0 ~ 0x3FFFF.
• The CPU can access peripherals via DPORT address more efficiently than via AHB address. However, DPORT address is characterized by speculative reads, which means it cannot guarantee that each read is valid. In addition, DPORT address will upset the order of r/w operations on the bus to improve performance, which may cause programs that have strict requirements on the r/w order to crash. On the other hand, using AHB address to read FIFO registers will cause unpredictable errors. To address above issues please strictly follow the instructions documented in ESP32 ECO and Workarounds for Bugs, specifically sections 3.3, 3.10, 3.16, and 3.17.

Right now the datasheet has the registers FIFO_AHB and FIFO, both at offset 0 from the start of the UART peripheral. So that seems wrong anyway: I guess one of those is supposed to in the AHB address space instead. I'm not sure how that would be best encoded in the SVD file, maybe just by leaving out the _AHB version and hoping applications will realize they shouldn't read from the regular (DPORT) version.

[jessebraham]

Thank you so much for all the feedback!

Just as a quick preface (sorry if this has already been said), the ESP32 is going to be a bit of an exception when compared to the newer devices. For the newer chips I have a serialized format describing peripherals which I'm able to parse; these aren't perfect but they tend to be fairly high quality. Unfortunately, due to its age I do not have these files for the ESP32. Because of this the ESP32's SVD is going to be a bit flakey in the beginning, as I'm having to parse the data from various sources and sometimes even hand-write the files representing peripherals.

Some of your points will take me a bit longer to investigate and fully understand, so I will comment again at a later time.

---

The registers FUNC%s_IN_SEL_CFG have the fields FUNC_IN_SEL, FUNC_IN_INV_SEL, and SIG_IN_SEL. That's a bit of repetition. Maybe that's entirely intended, but this repetition wasn't there in the community version of the SVD file (which had fields named SEL, IN_INV_SEL, and IN_SEL). (Feel free to ignore this as "works as intended").

I don't have any strong feelings about this and it's trivial to change, so we will adopt the shorter field names in the next release. I will do the same for the FUNC%s_OUT_SEL_CFG registers.

I think the IO_MUX pins are incorrect. Right now there is the GPIO%s array of registers address 0x3FF49044, which is correct for GPIO0, but is already incorrect for GPIO1 (aka U0TXD) which lives at address 0x3FF49088. I wish these registers were in some sort of logical order, but for the ESP32 this does not appear to be the case.

This was my bad, I had incorrectly assumed that the pins were at regular, sequential addresses. I have regenerated the IO_MUX peripheral file to split these out into separate pin definitions with the correct offsets.

There is a peripheral named RTCCNTL with a group name of RTC_CNTL? That seems odd. In the ESP32-C3 it's just called RTC_CNTL.

That does seem odd! 😁 The peripheral will be renamed to RTC_CNTL in the next release.

The SOC_CLK_SEL field of the RTC_CNTL register does not have names for the various clock sources (XTAL, PLL, CK8M, APLL). Maybe this is missing from the data? It was there in the community version of the SVD, which was nice. If it's not in the data, it's not a big loss to write these constants manually. (The same holds true for many other register fields).

This should be possible to do. I'm not sure if this data is available to me in a convenient form or not, but if it is not then I should be able to inject it. I will investigate this but it will likely take some time. I think this would be very nice to have in all of our SVDs though.

[jessebraham]

I have be preoccupied with some other projects the last few weeks here, so not a lot of progress on this yet. Sorry about that.

I did push a new release of the SVD which includes a number of small fixes. Relevant to this issue:

• the FUNC_IN_SEL, FUNC_IN_INV_SEL, and SIG_IN_SEL registers have been renamed to SEL, IN_INV_SEL, and SIG_SEL respectively
• the GPIO%s pins have been broken out in to separate registers rather than being collected in an array
• the RTCCNTL peripheral has been renamed to RTC_CNTL

I will hopefully get to the remaining issues in the coming weeks.

[jessebraham]

Apologies for taking so long to get back to this (again). With the exception of the SOC_CLK_SEL field's enums, I believe everything else in here should be resolved. I do plan on adding field enum support to the SVD generator at some point, I just haven't had a chance yet unfortunately.

If you feel this has not been resolved please feel free to re-open this issue, or to open a new one.

[jessebraham]

I actually managed to get enumeratedValues support added to our SVD generator today, so the next release will include these for at least some registers.