Skip to content

Commit

Permalink
Fix translation of peripheral register addresses
Browse files Browse the repository at this point in the history
This was already incorrect in the original ESP32 implementation
but was discovered while testing the new S2/S3 implementation.

This was also wrong within the ESP-IDF, that we based the translation
logic on. Espressif fixed the issue in this pull request:
espressif/esp-idf#11652

We now also have unit tests and compat (integration) tests, that
compare our binary output against that of binutils-gdb/esp32-ulp-as,
which already did this translation correctly, but we didnt have a
test for the specific cases we handled incorrectly, so we didn't
notice this bug.

This fix has also been tested on a real device, because S2/S3 devices
need the IOMUX clock enabled in order to be able to read GPIO input
from the ULP, and enabling that clock required writing to a register
in the SENS address range, which didnt work correctly before this fix.
  • Loading branch information
wnienhaus committed Aug 31, 2023
1 parent a157ebe commit ed28d27
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 5 deletions.
4 changes: 2 additions & 2 deletions esp32_ulp/opcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ def i_reg_wr(reg, high_bit, low_bit, val):
_wr_reg.addr = reg & 0xff
_wr_reg.periph_sel = (reg & 0x300) >> 8
else:
_wr_reg.addr = (reg & 0xff) >> 2
_wr_reg.addr = (reg >> 2) & 0xff
_wr_reg.periph_sel = _soc_reg_to_ulp_periph_sel(reg)
_wr_reg.data = get_imm(val)
_wr_reg.low = get_imm(low_bit)
Expand All @@ -394,7 +394,7 @@ def i_reg_rd(reg, high_bit, low_bit):
_rd_reg.addr = reg & 0xff
_rd_reg.periph_sel = (reg & 0x300) >> 8
else:
_rd_reg.addr = (reg & 0xff) >> 2
_rd_reg.addr = (reg >> 2) & 0xff
_rd_reg.periph_sel = _soc_reg_to_ulp_periph_sel(reg)
_rd_reg.unused = 0
_rd_reg.low = get_imm(low_bit)
Expand Down
4 changes: 2 additions & 2 deletions esp32_ulp/opcodes_s2.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ def i_reg_wr(reg, high_bit, low_bit, val):
_wr_reg.addr = reg & 0xff
_wr_reg.periph_sel = (reg & 0x300) >> 8
else:
_wr_reg.addr = (reg & 0xff) >> 2
_wr_reg.addr = (reg >> 2) & 0xff
_wr_reg.periph_sel = _soc_reg_to_ulp_periph_sel(reg)
_wr_reg.data = get_imm(val)
_wr_reg.low = get_imm(low_bit)
Expand All @@ -437,7 +437,7 @@ def i_reg_rd(reg, high_bit, low_bit):
_rd_reg.addr = reg & 0xff
_rd_reg.periph_sel = (reg & 0x300) >> 8
else:
_rd_reg.addr = (reg & 0xff) >> 2
_rd_reg.addr = (reg >> 2) & 0xff
_rd_reg.periph_sel = _soc_reg_to_ulp_periph_sel(reg)
_rd_reg.unused = 0
_rd_reg.low = get_imm(low_bit)
Expand Down
19 changes: 18 additions & 1 deletion tests/01_compat_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,19 @@ calc_file_hash() {
shasum < $1 | cut -d' ' -f1
}

make_log_dir() {
mkdir -p log
}

fetch_esp_idf() {
[ -d esp-idf ] && return

echo "Fetching esp-idf"
log_file=log/fetch-esp-idf.log
git clone --depth 1 \
https://github.com/espressif/esp-idf.git 1>$log_file 2>&1
}

run_tests_for_cpu() {
local cpu=$1
echo "Testing for CPU: $cpu"
Expand All @@ -33,7 +46,9 @@ run_tests_for_cpu() {
bin_file="${src_name}.bin"

echo -e "\tBuilding using binutils ($cpu)"
gcc -E -o ${pre_file} $src_file
gcc -I esp-idf/components/soc/$cpu/include -I esp-idf/components/esp_common/include \
-x assembler-with-cpp \
-E -o ${pre_file} $src_file
esp32ulp-elf-as --mcpu=$cpu -o $obj_file ${pre_file}
esp32ulp-elf-ld -T esp32.ulp.ld -o $elf_file $obj_file
esp32ulp-elf-objcopy -O binary $elf_file $bin_file
Expand All @@ -56,5 +71,7 @@ run_tests_for_cpu() {
echo ""
}

make_log_dir
fetch_esp_idf
run_tests_for_cpu esp32
run_tests_for_cpu esp32s2
15 changes: 15 additions & 0 deletions tests/compat/reg.esp32.S
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#include "soc/rtc_cntl_reg.h"
#include "soc/soc_ulp.h"

reg_rd 0x012, 1, 2
reg_rd 0x234, 3, 4
reg_rd 0x345, 5, 6

reg_wr 0x012, 1, 2, 1
reg_wr 0x234, 3, 4, 1
reg_wr 0x345, 5, 6, 1

WRITE_RTC_REG(0x3ff484a8, 1, 2, 3)
READ_RTC_REG(0x3ff484a8, 1, 2)
WRITE_RTC_REG(0x3ff48904, 1, 2, 3)
READ_RTC_REG(0x3ff48904, 1, 2)
15 changes: 15 additions & 0 deletions tests/compat/reg.esp32s2.S
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#include "soc/rtc_cntl_reg.h"
#include "soc/soc_ulp.h"

reg_rd 0x012, 1, 2
reg_rd 0x234, 3, 4
reg_rd 0x345, 5, 6

reg_wr 0x012, 1, 2, 1
reg_wr 0x234, 3, 4, 1
reg_wr 0x345, 5, 6, 1

WRITE_RTC_REG(0x3f4084a8, 1, 2, 3)
READ_RTC_REG(0x3f4084a8, 1, 2)
WRITE_RTC_REG(0x3f408904, 1, 2, 3)
READ_RTC_REG(0x3f408904, 1, 2)
26 changes: 26 additions & 0 deletions tests/opcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,31 @@ def test_reg_address_translations():
assert ins.addr == 0x2a # low 8 bits of 0x12a


def test_reg_address_translations_sens():
"""
Test addressing of peripheral registers using full DPORT bus addresses
"""

ins = make_ins("""
addr : 8 # Address within either RTC_CNTL, RTC_IO, or SARADC
periph_sel : 2 # Select peripheral: RTC_CNTL (0), RTC_IO(1), SARADC(2)
unused : 8 # Unused
low : 5 # Low bit
high : 5 # High bit
opcode : 4 # Opcode (OPCODE_RD_REG)
""")

# direct ULP address is derived from full address as follows:
# full:0x3ff48904 == ulp:(0x3ff48904-DR_REG_RTCCNTL_BASE) / 4
# full:0x3ff48904 == ulp:(0x3ff48904-0x3f408000) / 4
# full:0x3ff48904 == ulp:0x904 / 4
# full:0x3ff48904 == ulp:0x241
# see: https://github.com/espressif/binutils-esp32ulp/blob/249ec34/gas/config/tc-esp32ulp_esp32s2.c#L78
ins.all = opcodes.i_reg_rd("0x3ff48904", "0", "0")
assert ins.periph_sel == 2 # high 2 bits of 0x241
assert ins.addr == 0x41 # low 8 bits of 0x241


test_make_ins_struct_def()
test_make_ins()
test_arg_qualify()
Expand All @@ -183,3 +208,4 @@ def test_reg_address_translations():
test_eval_arg()
test_reg_direct_ulp_addressing()
test_reg_address_translations()
test_reg_address_translations_sens()
52 changes: 52 additions & 0 deletions tests/opcodes_s2.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,31 @@ def test_reg_address_translations_s2():
assert ins.addr == 0x2a # low 8 bits of 0x12a


def test_reg_address_translations_s2_sens():
"""
Test addressing of ESP32-S2 peripheral registers using full DPORT bus addresses
"""

ins = make_ins("""
addr : 8 # Address within either RTC_CNTL, RTC_IO, or SARADC
periph_sel : 2 # Select peripheral: RTC_CNTL (0), RTC_IO(1), SARADC(2)
unused : 8 # Unused
low : 5 # Low bit
high : 5 # High bit
opcode : 4 # Opcode (OPCODE_RD_REG)
""")

# direct ULP address is derived from full address as follows:
# full:0x3f408904 == ulp:(0x3f408904-DR_REG_RTCCNTL_BASE) / 4
# full:0x3f408904 == ulp:(0x3f408904-0x3f408000) / 4
# full:0x3f408904 == ulp:0x904 / 4
# full:0x3f408904 == ulp:0x241
# see: https://github.com/espressif/binutils-esp32ulp/blob/249ec34/gas/config/tc-esp32ulp_esp32s2.c#L78
ins.all = opcodes.i_reg_rd("0x3f408904", "0", "0")
assert ins.periph_sel == 2 # high 2 bits of 0x241
assert ins.addr == 0x41 # low 8 bits of 0x241


def test_reg_address_translations_s3():
"""
Test addressing of ESP32-S3 peripheral registers using full DPORT bus addresses
Expand All @@ -199,6 +224,31 @@ def test_reg_address_translations_s3():
assert ins.addr == 0x2a # low 8 bits of 0x12a


def test_reg_address_translations_s3_sens():
"""
Test addressing of ESP32-S3 peripheral registers using full DPORT bus addresses
"""

ins = make_ins("""
addr : 8 # Address within either RTC_CNTL, RTC_IO, or SARADC
periph_sel : 2 # Select peripheral: RTC_CNTL (0), RTC_IO(1), SARADC(2)
unused : 8 # Unused
low : 5 # Low bit
high : 5 # High bit
opcode : 4 # Opcode (OPCODE_RD_REG)
""")

# direct ULP address is derived from full address as follows:
# full:0x60008904 == ulp:(0x60008904-DR_REG_RTCCNTL_BASE) / 4
# full:0x60008904 == ulp:(0x60008904-0x60008000) / 4
# full:0x60008904 == ulp:0x904 / 4
# full:0x60008904 == ulp:0x241
# see: https://github.com/espressif/binutils-esp32ulp/blob/249ec34/gas/config/tc-esp32ulp_esp32s2.c#L78
ins.all = opcodes.i_reg_rd("0x60008904", "0", "0")
assert ins.periph_sel == 2 # high 2 bits of 0x241
assert ins.addr == 0x41 # low 8 bits of 0x241


test_make_ins_struct_def()
test_make_ins()
test_arg_qualify()
Expand All @@ -209,3 +259,5 @@ def test_reg_address_translations_s3():
test_reg_direct_ulp_addressing()
test_reg_address_translations_s2()
test_reg_address_translations_s3()
test_reg_address_translations_s2_sens()
test_reg_address_translations_s3_sens()

0 comments on commit ed28d27

Please sign in to comment.