pulp-platform / riscv-dbg

RISC-V Debug Support for our PULP RISC-V Cores
Other
198 stars 70 forks source link

debug_rom/gen_rom.py: fixes #108

Closed flaviens closed 1 year ago

flaviens commented 3 years ago

Some fixes in the debug_rom/gen_rom.py script.

Importantly, map has no len, therefore it seems necessary to cast it to a list and this script does not run as it was, under Python 3.8.5.

bluewww commented 3 years ago

I don't think this patch is entirely correct. I get this

# for python3.8
bluew:[debug_rom]: PYTHON=python3.8 make clean all
rm -f *.img *.dump *.bin *.sv
riscv64-unknown-elf-objcopy -O binary debug_rom.elf debug_rom.bin
dd if=debug_rom.bin of=debug_rom.img bs=256 count=1
0+1 records in
0+1 records out
148 bytes copied, 5.378e-05 s, 2.8 MB/s
python3.8 gen_rom.py debug_rom.img
Traceback (most recent call last):
  File "gen_rom.py", line 102, in <module>
    rom = read_bin()
  File "gen_rom.py", line 91, in read_bin
    rom = list(map(''.join, zip(rom[::2], rom[1::2])))
TypeError: sequence item 0: expected str instance, int found
make: *** [Makefile:13: debug_rom.sv] Error 1
rm debug_rom.bin

bluew:[debug_rom]: python3.8 --version
Python 3.8.7

# same for python3.6
bluew PYTHON=python3.6 make clean all
rm -f *.img *.dump *.bin *.sv
riscv64-unknown-elf-objcopy -O binary debug_rom.elf debug_rom.bin
dd if=debug_rom.bin of=debug_rom.img bs=256 count=1
0+1 records in
0+1 records out
148 bytes copied, 5.2845e-05 s, 2.8 MB/s
python3.6 gen_rom.py debug_rom.img
Traceback (most recent call last):
  File "gen_rom.py", line 102, in <module>
    rom = read_bin()
  File "gen_rom.py", line 91, in read_bin
    rom = list(map(''.join, zip(rom[::2], rom[1::2])))
TypeError: sequence item 0: expected str instance, int found
make: *** [Makefile:13: debug_rom.sv] Error 1
rm debug_rom.bin

bluew:[debug_rom]: python3.6 --version
Python 3.6.12
flaviens commented 3 years ago

Hi bluewww,

Thank you very much for your feedback! Indeed there was another issue in the script that I had not solved. Should be fixed now.

msfschaffner commented 1 year ago

It would be nice to get this fixed indeed. We keep a similar patch in the OpenTitan repo to fix this, but ideally this would be fixed upstream: https://github.com/lowRISC/opentitan/blob/master/hw/vendor/patches/pulp_riscv_dbg/0002-Update-debug-ROM-generator-script-to-work-with-Pytho.patch

CC @andreaskurth

andreaskurth commented 1 year ago

@bluewww: I think this PR can be closed, now that #151 is merged