Fix detection of some registers

[MabezDev]

The current output from master shows the failed parsing of some registers.

Failed to match reg info at esp-idf/components/soc/esp32/include/soc/uhci_reg.h:1007
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/i2c_reg.h:944
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/frc_timer_reg.h:34
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/frc_timer_reg.h:45
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/frc_timer_reg.h:48
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rtc_cntl_reg.h:1833
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:18
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:20
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:22
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:24
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:26
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:28
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:30
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:32
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:35
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:37
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:39
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:41
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:43
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:45
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:47
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:49
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:51
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:53
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:55
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:57
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:59
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:61
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:63
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:65
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:67
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/hwcrypto_reg.h:27
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/hwcrypto_reg.h:29
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/hwcrypto_reg.h:31
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/hwcrypto_reg.h:33
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/hwcrypto_reg.h:35
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/spi_reg.h:135
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rmt_reg.h:18
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rmt_reg.h:20
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rmt_reg.h:22
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rmt_reg.h:24
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rmt_reg.h:26
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rmt_reg.h:28
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rmt_reg.h:30
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rmt_reg.h:32
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/host_reg.h:973
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rtc_i2c_reg.h:155
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rtc_i2c_reg.h:231
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rtc_i2c_reg.h:237
Failed to match reg info at esp-idf/components/soc/esp32/include/soc/uart_reg.h:22

[jessebraham]

I haven't had a chance to dig into this too deep yet, but I think I may have found the source of the problem. For certain definitions, the transition from State::FindReg to State::FindBitFieldInfo seems to be consuming an additional line, and so re_reg_bit_info is not matching on the line we want it to.

I printed out a bit of debug info to show this. I've included a couple of examples below:

Failed to match reg info at esp-idf/components/soc/esp32/include/soc/uart_reg.h:22
Register {
    name: "UART_FIFO_AHB",
    address: 0,
    width: 0,
    description: "UART_FIFO_AHB",
    reset_value: 0,
    detailed_description: None,
    bit_fields: [],
}
PrevLine: #define UART_FIFO_AHB_REG(i)  (REG_UART_AHB_BASE(i) + 0x0)
Line:     #define UART_FIFO_REG(i)          (REG_UART_BASE(i) + 0x0)

Failed to match reg info at esp-idf/components/soc/esp32/include/soc/uhci_reg.h:1007
Register {
    name: "UHCI_ACK_NUM",
    address: 108,
    width: 0,
    description: "UHCI_ACK_NUM",
    reset_value: 0,
    detailed_description: None,
    bit_fields: [],
}
PrevLine: #define UHCI_ACK_NUM_REG(i)          (REG_UHCI_BASE(i) + 0x6C)
Line:

As you can see in both these cases, the previous line is the one we want to be matching on. The line number printed in the error message corresponds with the previous line.

[MabezDev]

Thanks for looking into this! I've just poked around in those files, and in those two instances, I think the parser it behaving normally.

The way idf2svd works is by finding defines that end in _REG as a starting point. Then the next line(s) will be bitfield information.

The full snippet from uart_reg.h looks like this

#define UART_FIFO_AHB_REG(i)  (REG_UART_AHB_BASE(i) + 0x0) // idf2svd expects a comment with bitfield info below here
#define UART_FIFO_REG(i)          (REG_UART_BASE(i) + 0x0) // and below here

/* UART_RXFIFO_RD_BYTE : RO ;bitpos:[7:0] ;default: 8'b0 ; */
/*description: This register stores one byte data  read by rx fifo.*/
#define UART_RXFIFO_RD_BYTE  0x000000FF
#define UART_RXFIFO_RD_BYTE_M  ((UART_RXFIFO_RD_BYTE_V)<<(UART_RXFIFO_RD_BYTE_S))
#define UART_RXFIFO_RD_BYTE_V  0xFF
#define UART_RXFIFO_RD_BYTE_S  0

I think this more of an issue with the consistency of the documentation of the idf; though with that in mind, what is the best course of action to take.

I only looked at uart and uhci so far but its possible that there are bugs in the parsing for other peripherals.

[MabezDev]

From page 351 in the reference manual UART_FIFO_REG is a valid reg with UART_RXFIFO_RD_BYTE as its only bitfield, it seems there has been a newline mistakenly added.

Ideally the parser should really be able to cope with this but as far as I can tell UART_FIFO_AHB_REG doesn't make sense in that position

[MabezDev]

In RTC_CNTL, the read only info uses a '0' instead of a 'O'.

/* RTC_CNTL_RDY_FOR_WAKEUP : R/0; bitpos:[19]; default: 0 */

should be

/* RTC_CNTL_RDY_FOR_WAKEUP : R/O; bitpos:[19]; default: 0 */

[jessebraham]

Gotcha, thanks for clearing that up. The output just seemed off to me initially.

The error The following registers failed to parse ["FRC_TIMER_LOAD", "FRC_TIMER_COUNT"] is printed as well. The lines these items are defined on both end with comments, which in turn end with ). The comments were being captured along with the offsets in my testing. I think we just need to lazily match on the last group to fix this:

r"\#define[\s*]+([^\s*]+)_REG\(i\)[\s*]+\(REG_([0-9A-Za-z_]+)_BASE[\s*]*\(i\) \+ (.*?)\)"

(Note the addition of ? in the last capture group)

[MabezDev]

Ah good spot! Would you mind PR'ing that change to the regex?

For the items where the docs are wrong (or missing), I guess we'll have to fix the docs in our own fork (unless you have any other ideas?) but I think its better to worry about that later.

[jessebraham]

I've submitted a PR to fix the register parsing issue. The items outlined in the first comment still need to be resolved at this point.

I'm not against maintaining our own fork, as that's probably easier and more reliable than trying to be clever and coding around the inconsistencies. But as you said that can probably be addressed at another time, it's documented here at least for the time being.

[bschwind]

Hi there, I recently commented on your reddit thread about helping out. Still reading through the code, but here are my assumptions so far, please correct me if I'm wrong:

• The SVD file is a standardized XML file which describes all peripherals for the chip, along with their registers, memory offsets, etc.
• This project is reading the defines in the esp-idf code, and generating an SVD file
• Currently the program outputs Failed to match reg info at... on registers it couldn't parse
• If we can match every register properly, is the work for this repo considered "done" ?
• Are we essentially trying to do the work of a C preprocessor by expanding the DEFINEs to the point where we have a mapping of registers to memory offsets? I might be grossly simplifying it...

Just trying to figure out the end state here.

Also, it seems even if not every register got matched, the SVD file is still usable for the registers that did get matched, right?

[jessebraham]

• The SVD file is a standardized XML file which describes all peripherals for the chip, along with their registers, memory offsets, etc.

Essentially, yes. atsamd-rs has some SVDs from Microchip if you'd like to see some complete examples.

• This project is reading the defines in the esp-idf code, and generating an SVD file
• Currently the program outputs Failed to match reg info at... on registers it couldn't parse

Correct on both. A good chunk of the registers failing to parse seem to be due to inconsistencies in the esp-idf source as far as I can tell.

• If we can match every register properly, is the work for this repo considered "done" ?

As done as anything ever is :wink: esp-idf will continue to be updated I'd imagine so there will likely be tweaks required here and there, but in theory once all peripherals are documented by the SVD file then we have nothing left to do.

• Are we essentially trying to do the work of a C preprocessor by expanding the DEFINEs to the point where we have a mapping of registers to memory offsets? I might be grossly simplifying it...

That seems like a reasonable simplification, though we'd like to get information on interrupts as well (and I'm sure there are further details we can extract).

Also, it seems even if not every register got matched, the SVD file is still usable for the registers that did get matched, right?

The registers which are being matched have PAC code generated for them currently; this is handled by the esp32 repo. In theory if the register is being parsed you should be able to manipulate it programmatically.

Hopefully that helps, anything else I'll leave to @MabezDev

[igrr]

(ESP-IDF maintainer here) We have an issue filed for providing SVD files: https://github.com/espressif/esp-idf/issues/2214. Probably we will solve this for the upcoming ESP32-S2 chip first, because we have started using an internal register description format which can be converted to SVD (some code needs to be written...). For the ESP32, this may take more time because we didn't have that format in place when ESP32 was developed.

I know this doesn't solve the present day issue, just wanted to say that we are aware of this need and sorry for making you jump through hoops to generate SVDs.

[bschwind]

@jessebraham Thanks for the details. Originally I thought maybe using a C preprocessor would be easier, but probably a hand-rolled solution is easy enough so I'll try to work through the code here.

@igrr That's great to hear espressif will have official SVDs at some point!

[MabezDev]

@bschwind

@jessebraham has summed it up quite nicely. It's a two step process, idf2rust capatures the raw documentation data from the idf docs, esp32 tries to format, modify and improve the raw data before it is passed to svd2rust for codegen. I will try and open some issues tonight on the esp32 repo.

@igrr

sorry for making you jump through hoops to generate SVDs

No problem! It was actually pretty fun getting it all working, but I'd be much nicer to have a verified SVD from yourselves. Hopefully generating a esp32-s2 PAC will be a lot less work :)

[bschwind]

So given the unknown timeline, I suppose we'll continue with this solution until espressif puts out an official SVD?

I'm still a little unfamiliar with what we want to do when we run into particular registers.

As an example, I'm on the current master, and this is the first register matching error that occurs:

Failed to match reg info at esp-idf/components/soc/esp32/include/soc/rtc_i2c_reg.h:155

And the contents of those lines:

150 #define RTC_I2C_SLAVE_ADDR_S  0
151 
152 /* Result of last read operation. Not used directly as the data will be
153 * returned to the ULP. Listed for debugging purposes.
154  */
155 #define RTC_I2C_DATA_REG         (DR_REG_RTC_I2C_BASE + 0x01c)
156 
157 /* Interrupt registers; since the interrupt from RTC_I2C is not connected,
158 * these registers are only listed for debugging purposes.
159  */
160 
161 /* Interrupt raw status register */
162 #define RTC_I2C_INT_RAW_REG         (DR_REG_RTC_I2C_BASE + 0x020)

So that register is only listed for debugging purposes - do we still want to document or is it okay to leave it absent?

A lot of the other failed registers are ones that are indexed, what do we want to do with those?

Sorry for all the questions, I think I understand the overall goal but I'm not sure what actions we want to take when we find these unusual register definitions.

[igrr]

RTC_I2C is mostly useable from the ULP coprocessor of the ESP32, so it shouldn't matter for writing Rust programs running on Xtensa cores. I would suggest skipping this file entirely.

Sorry for all the questions, I think I understand the overall goal but I'm not sure what actions we want to take when we find these unusual register definitions.

I can keep an eye on this topic and answer your questions, if it helps.

[MabezDev]

So given the unknown timeline, I suppose we'll continue with this solution until espressif puts out an official SVD?

Yeah seeing as the esp32-s2 isn't even out yet I suspect it will be quite a while for the esp32 svd to show up.

I'm still a little unfamiliar with what we want to do when we run into particular registers.

Me too to be honest; earlier on in the thread I suggested forking the idf and fixing the doc comments, which we we eventually fold back into upstream idf.

Perhaps it might be better to just create the peripherals manually in esp32. https://github.com/esp-rs/esp32/pull/14/files already added some missing peripherals it might just be easier to create them there.

I can keep an eye on this topic and answer your questions, if it helps.

I think that would be very helpful.

[bschwind]

If it makes sense, I'll go ahead and run on the latest master and put together a table of all the registers that failed to match, and link to their source in the esp-idf.

[bschwind]

Kinda ugly but maybe this is useful?

Ran on idf2svd @ commit f370ddb8b6fb9e05f1011529bfef059321ec4ae1

Can Skip	File/Line	esp-idf link	Notes
✅	esp-idf/components/soc/esp32/include/soc/rtc_i2c_reg.h:155	link	RTC_I2C is mostly useable from the ULP coprocessor of the ESP32, so it shouldn't matter for writing Rust programs running on Xtensa cores.
✅	esp-idf/components/soc/esp32/include/soc/rtc_i2c_reg.h:231	link	"
✅	esp-idf/components/soc/esp32/include/soc/rtc_i2c_reg.h:237	link	"
✅	esp-idf/components/soc/esp32/include/soc/frc_timer_reg.h:26	link	It is unlikely that you will need to use this timer in Rust code, since there are 4 64-bit timers in the Timer Group peripheral which are much easier to use. We do use this timer in IDF for the esp_timer implementation
✅	esp-idf/components/soc/esp32/include/soc/frc_timer_reg.h:30	link	"
✅	esp-idf/components/soc/esp32/include/soc/frc_timer_reg.h:34	link	"
✅	esp-idf/components/soc/esp32/include/soc/frc_timer_reg.h:45	link	"
✅	esp-idf/components/soc/esp32/include/soc/frc_timer_reg.h:48	link	"
❌	esp-idf/components/soc/esp32/include/soc/spi_reg.h:135	link	This register should have a 32-bit r/w field inside
❌	esp-idf/components/soc/esp32/include/soc/i2c_reg.h:944	link	This is the address of a 32-byte hardware FIFO
❌	esp-idf/components/soc/esp32/include/soc/uart_reg.h:22	link	For UART FIFO, we use different base address than for the rest of the register. On the ESP32, the peripherals are mapped into two address ranges, 0x3ffxxxxxx and 0x600xxxxxx. The former allows for faster access, but the latter guarantees that the CPU will not issue speculative reads. This distinction is important when accessing the FIFOs.
❌	esp-idf/components/soc/esp32/include/soc/hwcrypto_reg.h:27	link	All registers (ending with _REG) are 32-bit, and there are some register banks, their names end with _BASE.
❌	esp-idf/components/soc/esp32/include/soc/hwcrypto_reg.h:29	link	"
❌	esp-idf/components/soc/esp32/include/soc/hwcrypto_reg.h:31	link	"
❌	esp-idf/components/soc/esp32/include/soc/hwcrypto_reg.h:33	link	"
❌	esp-idf/components/soc/esp32/include/soc/hwcrypto_reg.h:35	link	"
❌	esp-idf/components/soc/esp32/include/soc/rmt_reg.h:18	link	These data registers are 32-bit registers, contain a single r/w field
❌	esp-idf/components/soc/esp32/include/soc/rmt_reg.h:20	link	"
❌	esp-idf/components/soc/esp32/include/soc/rmt_reg.h:22	link	"
❌	esp-idf/components/soc/esp32/include/soc/rmt_reg.h:24	link	"
❌	esp-idf/components/soc/esp32/include/soc/rmt_reg.h:26	link	"
❌	esp-idf/components/soc/esp32/include/soc/rmt_reg.h:28	link	"
❌	esp-idf/components/soc/esp32/include/soc/rmt_reg.h:30	link	"
❌	esp-idf/components/soc/esp32/include/soc/rmt_reg.h:32	link	"
✅	esp-idf/components/soc/esp32/include/soc/uhci_reg.h:1007	link	Not entirely sure what this one does, we haven't implemented support for UHCI peripheral in IDF. (This is basically a UART DMA engine.)
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:18	link	There is a sdmmc_struct.h file, which does contain field definitions. If I recall correctly, it was written by hand, as we didn't have a machine-readable description of the SDMMC registers.
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:20	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:22	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:24	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:26	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:28	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:30	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:32	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:35	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:37	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:39	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:41	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:43	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:45	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:47	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:49	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:51	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:53	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:55	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:57	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:59	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:61	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:63	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:65	link	"
❌	esp-idf/components/soc/esp32/include/soc/sdmmc_reg.h:67	link	"
❌	esp-idf/components/soc/esp32/include/soc/rtc_cntl_reg.h:1833	link	This one fails to parse because of R/0 instead of R/O
❌	esp-idf/components/soc/esp32/include/soc/host_reg.h:973	link	a R/W register, has a single 32-bit field.

[igrr]

frc_timer_reg.h: this file was written by hand, not generated by a tool, hence the different format. I think it is unlikely that you will need to use this timer in Rust code, since there are 4 64-bit timers in the Timer Group peripheral which are much easier to use. We do use this timer in IDF for the esp_timer implementation, just to not reserve one of the Timer Group timers for system purposes.

spi_reg.h:135: looks like some manual fixup was made. This register should have a 32-bit r/w field inside.

i2c_reg.h:944: this is the address of a 32-byte hardware FIFO

uart_reg.h:22: For UART FIFO, we use different base address than for the rest of the register. On the ESP32, the peripherals are mapped into two address ranges, 0x3ffxxxxxx and 0x600xxxxxx. The former allows for faster access, but the latter guarantees that the CPU will not issue speculative reads. This distinction is important when accessing the FIFOs.

hwcrypto_reg.h: Also not generated by a tool. All registers (ending with _REG) are 32-bit, and there are some register banks, their names end with _BASE.

rmt_reg.h: These data registers are 32-bit registers, contain a single r/w field.

UHCI_ACK_NUM_REG: Not entirely sure what this one does, we haven't implemented support for UHCI peripheral in IDF. (This is basically a UART DMA engine.)

sdmmc_reg.h: Not generated by a tool, so there is no breakdown of registers into fields for this peripheral. There is a sdmmc_struct.h file, which does contain field definitions. If I recall correctly, it was written by hand, as we didn't have a machine-readable description of the SDMMC registers.

rtc_cntl_reg.h#L1833: not sure why this was flagged, definition looks okay to me.

HOST_SLCHOST_WIN_CMD_REG: a R/W register, has a single 32-bit field.

[bschwind]

@igrr Thanks so much! I'll update the table in a bit.

Regarding trc_cntl_reg.h, I think it had to do with there being a R/0 instead of R/O

[bschwind]

@MabezDev I updated the table in my comment above, hopefully it helps. It feels like we could go forward with just manually creating the missing registers, there aren't a huge amount of them.

[MabezDev]

@bschwind Thank you for taking the time to format this nicely! I would tend to agree, I think we've got 99~>% coverage so I think it would be best to focus on other aspects in esp32.

I've been pretty busy these last few days, but I will eventually put up some issues on the esp32 repo. I'll be sure to ping this thread when they are up.

[MabezDev]

So I've started creating some issues:

idf2svd:

• [x] https://github.com/MabezDev/idf2svd/issues/11 - this one is pretty important as we wont be able to use any peripherals that use GPIO's without the io mux

xtensa-lx6-rt:

• [ ] https://github.com/esp-rs/xtensa-lx6-rt/issues/5 - again pretty crucial, xtensa-lx6-rt is really primitive at the moment, the hard part is finding the relevant documentation, and in particular the format of the vector table etc. Being able to set interrupt & exception handlers is a must.

esp32:

• [ ] https://github.com/esp-rs/esp32/issues/15