Issues building and testing new bootloader for OrangeCrab r0.2

[iostat]

Hi there!

I'm trying to modify the bootloader to also look at one of the IO pins when determining whether or not to enter DFU mode, and I can't seem to produce a bitstream that actually /works/. Particularly, I can run the entire compilation process, create a bitstream, upload it with dfu-util, but once it loads, the RGB LED seems to just get stuck at green (which doesn't seem to actually correspond to any state of the bootloader), regardless if the IOs are tied high, low, or left floating. I'm not sure if I'm doing something wrong, or if that's an expected behavior of the bootloader when running from the "user" section. I'm hoping to avoid having to actually flash it as the real bootloader to test it. I have a Segger J-LINK if it turns out to be necessary to use JTAG to recover the bootloader (i.e., not super worried about bricking the device), but I'm similarly hoping to avoid having to go down that road if possible.

Below are all the changes I've made to in my attempts to get this working. Any help would be appreciated! These are divided into two sections, system-specific tweaks I had to make to the Makefile and linker script to get it building on my machine, and then the changes I attempted to make to the bitstream to try and get it to look at IO pins besides the onboard button

Build environment

parallels@parallels-Parallels-Virtual-Platform:~$ uname -a
Linux parallels-Parallels-Virtual-Platform 4.15.0-34-generic #37-Ubuntu SMP Mon Aug 27 15:21:48 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

parallels@parallels-Parallels-Virtual-Platform:~$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=18.04
DISTRIB_CODENAME=bionic
DISTRIB_DESCRIPTION="Ubuntu 18.04.5 LTS"

parallels@parallels-Parallels-Virtual-Platform:~$ yosys --version
Yosys 0.9+3521 (git sha1 4f2b78e1, clang 6.0.0-1ubuntu2 -fPIC -Os)

parallels@parallels-Parallels-Virtual-Platform:~$ nextpnr-ecp5 --version
nextpnr-ecp5 -- Next Generation Place and Route (Version f6d436d5)

parallels@parallels-Parallels-Virtual-Platform:~$ ecppack --version
Project Trellis ecppack Version 1.0-182-g8c0a638

parallels@parallels-Parallels-Virtual-Platform:~$ riscv32-unknown-elf-gcc --version
riscv32-unknown-elf-gcc (GCC) 10.1.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

parallels@parallels-Parallels-Virtual-Platform:~$ riscv32-unknown-elf-ld --version
GNU ld (GNU Binutils) 2.35
Copyright (C) 2020 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.

My build-and-flash command is:

python3 foboot-bitstream.py --platform orangecrab --revision 0.2 --device 25F && \
cp build/gateware/orangecrab.bit build/gateware/orangecrab.dfu && \
dfu-suffix -v 1209 -p 5af0 -a build/gateware/orangecrab.dfu && \
dfu-util -D build/gateware/orangecrab.dfu 

System-specific changes

Modified sw/Makefile to support my system:

diff --git a/sw/Makefile b/sw/Makefile
index a08bf1a..04d478e 100644
--- a/sw/Makefile
+++ b/sw/Makefile
@@ -1,5 +1,5 @@
 # Earlier versions of the Raspberry Pi image only had riscv32-gcc
-ifneq (,$(wildcard /usr/bin/riscv32-unknown-elf-gcc))
+ifneq (,$(wildcard /opt/riscv/bin/riscv32-unknown-elf-gcc))
 TRGT       ?= riscv32-unknown-elf-
 else
 TRGT       ?= riscv64-unknown-elf-

Modified the linker script so that it actually links

Not sure if this is because I'm using the most recent riscv-gnu-toolchain, but ld was complaining about .srodata.landing_url_descriptor overflowing into .data. I had to tweak the rodata section to look like:

diff --git a/sw/ld/linker.ld b/sw/ld/linker.ld
index 5796f16..7282043 100644
--- a/sw/ld/linker.ld
+++ b/sw/ld/linker.ld
@@ -21,7 +21,7 @@ SECTIONS
                _frodata = .;
                *(.rodata .rodata.* .gnu.linkonce.r.*)
                *(.rodata1)
-               *(.srodata)
+               *(.srodata .srodata.*)
                _erodata = .;
        } > rom

Changes to the actual bitstream

Then I made a few changes to the platform scripts to support treating IO0 and IO1 as readable peripherals via a CSR much like the usr_btn is.

Added two IOs to hw/deps/litex-boards/litex_boards/platforms/orangecrab.py

Just adding these two pins to be able to reference them later in the stack

diff --git a/litex_boards/platforms/orangecrab.py b/litex_boards/platforms/orangecrab.py
index efdd103..4cae9d8 100644
--- a/litex_boards/platforms/orangecrab.py
+++ b/litex_boards/platforms/orangecrab.py
@@ -84,6 +84,8 @@ _io_r0_2 = [
     ("rst_n", 0, Pins("V17"), IOStandard("LVCMOS33")),

     ("usr_btn", 0, Pins("J17"), IOStandard("SSTL135_I")),
+    ("io_0", 0, Pins("GPIO:0"), IOStandard("LVCMOS33")),
+    ("io_1", 0, Pins("GPIO:1"), IOStandard("LVCMOS33")),

     ("rgb_led", 0,
         Subsignal("r", Pins("K4"), IOStandard("LVCMOS33")),

Added the two IOs to hw/rtl/plaform/orangecrab.py

Modified add_button to make two more "buttons" that read those two pins similar to how the real button is added

diff --git a/hw/rtl/platform/orangecrab.py b/hw/rtl/platform/orangecrab.py
index 40fb8e4..47cc86c 100644
--- a/hw/rtl/platform/orangecrab.py
+++ b/hw/rtl/platform/orangecrab.py
@@ -91,7 +91,12 @@ class Platform(LatticePlatform):
     def add_button(self, soc):
         try:
             btn = self.request("usr_btn")
+            io0 = self.request("io_0")
+            io1 = self.request("io_1")
+
             soc.submodules.button = Button(btn)
+            soc.submodules.io0 = Button(io0)
+            soc.submodules.io1 = Button(io1)
         except:
             ...

Added the two new "button" CSRs to hw/foboot-bitstream.py

diff --git a/hw/foboot-bitstream.py b/hw/foboot-bitstream.py
index d2c1512..241474c 100755
--- a/hw/foboot-bitstream.py
+++ b/hw/foboot-bitstream.py
@@ -75,6 +75,8 @@ class BaseSoC(SoCCore, AutoDoc):
         "lxspi":          15,
         "messible":       16,
         "button":         17,
+        "io0":            18,
+        "io1":            19,
     }

     SoCCore.mem_map = {

Finally, modified sw/src/main.c to consider the new IO pins when determining whether to enter DFU mode

diff --git a/sw/src/main.c b/sw/src/main.c
index ca77fd1..2f216a4 100644
--- a/sw/src/main.c
+++ b/sw/src/main.c
@@ -117,7 +117,14 @@ static int nerve_pinch(void) {

 static int button_pressed(void){
 #ifdef CSR_BUTTON_BASE
+#   ifdef CSR_IO0_BASE
+#       ifdef CSR_IO1_BASE
+          return button_i_read() != 1 || io0_i_read() != 1 || io1_i_read() != 1;
+#       else
+          return button_i_read() != 1 || io0_i_read() != 1;
+#       endif
     return button_i_read() != 1;
+#   endif
 #else
     return 1;
 #endif

Thanks for taking a look at this! Any insight would be appreciated, and hopefully this can go on to serve a power user in the future if they want to try this!

[gregdavill]

It's a bit of a unique use case running the bootloader from the user section, but should technically work.

I notice that in your build script you are using orangecrab.bit to create orangecrab.dfu. This likely results in a file that has no embedded firmware. Have you tried using the foboot.bit file instead?

In order to facilitate quick firmware iteration cycles the bootloader is currently setup to fill the embedded block ram with pesudo-random data, ecpbram can detect this pattern in the compiled bitstream and use it to replace the embedded blockram with firmware. When we do this I'm placing the resulting bitstream in foboot.bit This is the function that performs this task: https://github.com/gregdavill/foboot/blob/OrangeCrab/hw/rtl/platform/orangecrab.py#L129-L159

[iostat]

Thanks for the reply!

It's a bit of a unique use case running the bootloader from the user section, but should technically work.

Ha, absolutely! I'm only doing this to be confident that the new bootloader actually works as intended (i.e., I get a DFU device and I can trigger it with an I/O pin) before I go on to replace it as the primary bootloader.

I notice that in your build script you are using orangecrab.bit to create orangecrab.dfu. This likely results in a file that has no embedded firmware. Have you tried using the foboot.bit file instead?

Yeah, I wasn't sure which is the preferred image to use, so I went with orangecrab.bit as it was larger, and I assumed the size difference was from the embedded firmware. However I see the same final behavior in both images. The only difference is that with orangecrab.dfu there is a noticeably longer period where the LED is white before it becomes green. I just chalked that up to loading time for the bitstream (as orangecrab.bit is ~3x larger than foboot.bit)

Part of me suspects that this is some issue related to initializing the RiscV CPU (e.g., reset vector mismatch, or some kind of hard fault shortly upon starting), but I can't explain the LED being green (as opposed to say, the white it's set to when powered on) - unless that's actually the default state for the LED peripheral?

I'm curious if you've ran into anything like this during your development process -- might give a lead to narrow down the root cause.

[gregdavill]

Green is the expected colour from the LED when the FPGA is in an un-configured state.

foboot.bit has bitstream compression enabled, so that is why it's considerably smaller.

The next thing to try is to comment out the reboot when button is not enabled, this will mean the DFU will enumerate if the RISCV is running.

diff --git a/sw/src/main.c b/sw/src/main.c
index ca77fd1..1ed9b9d 100644
--- a/sw/src/main.c
+++ b/sw/src/main.c
@@ -267,10 +267,10 @@ static void init(void)
         lxspi_bitbang_en_write(1);
     }
 #elif defined(CONFIG_ORANGECRAB_REV_R0_1) | defined(CONFIG_ORANGECRAB_REV_R0_2)
-    if(!button_pressed()){
-        spiFree();
-        reboot();
-    }
+//    if(!button_pressed()){
+//        spiFree();
+//        reboot();
+//    }
 #endif

 #ifdef CSR_UART_BASE

There has also been some changes to the foboot repo. So it may be there is some other things broken. I'll be able to test on some hardware in ~8hrs.

[iostat]

Green is the expected colour from the LED when the FPGA is in an un-configured state.

The next thing to try is to comment out the reboot when button is not enabled, this will mean the DFU will enumerate if the RISCV is running.

Good call! Incidentally, I tried commenting out everything other than that which seemed Fomu related :P . It seems like I get the green LED even with that commented out (and no enumeration). So at least it's promising in the sense that it means my bitstream is likely malformed -- assuming that the longer "white" time is actually due to loading time. It seems a little odd to me though that the 256K-ish compressed image loads nearly instantly compared to the 650K uncompressed one, though.

[gregdavill]

There are a few things at play with the loading times. The compressed option only effects the file size.

The foboot.bit file also increases the clock speed from 2.4MHz (default) to 38.8MHz. As well as enabling quad-read of the bitstream. For an equivalent bitstream of the same size these two changes speed up booting by ~64x.

[iostat]

Ok, so, by some miracle, I actually got it to work! Turns out it was an issue in the linker script. I had to move the .srodata section just before .sdata, like so:

diff --git a/sw/ld/linker.ld b/sw/ld/linker.ld
index 5796f16..77741b6 100644
--- a/sw/ld/linker.ld
+++ b/sw/ld/linker.ld
@@ -21,7 +21,6 @@ SECTIONS
                _frodata = .;
                *(.rodata .rodata.* .gnu.linkonce.r.*)
                *(.rodata1)
-               *(.srodata)
                _erodata = .;
        } > rom

@@ -32,6 +31,7 @@ SECTIONS
                *(.data .data.* .gnu.linkonce.d.*)
                *(.data1)
                _gp = ALIGN(16);
+               *(.srodata .srodata.*)
                *(.sdata .sdata.* .gnu.linkonce.s.* .sdata2 .sdata2.*)
                _edata = ALIGN(16); /* Make sure _edata is >= _gp. */
        } > sram

Guess that still leaves the question of the green LED. When you say it's the intended behavior for the FPGA in an unconfigured state, I take it you mean "no bitstream loaded" as opposed to the default state of the RGB peripheral on wishbone. If it's the latter, then that makes sense -- the bitstream /was/ loaded, just the riscv binary was messed up and the CPU was hung. If it's the former, I'm really curious how that happened!

[iostat]

Strangely enough, running the toolchain with -mno-relax didn't affect anything being placed in .srodata. So this solution might only be due to my version of the RISC-V toolchain

[boxfire]

I looked into this last week. Fof the .srodata you certainly want the alignment sorted .srodata. Also to allow the relaxation (though it doesnt seem to be used in dfu anyway) you also need an offset: Instead of

*(.srodata .srodata.*)

_gp = . + 0x800;
*(.srodata.cst16) *(.srodata.cst8) *(.srodata.cst4) *(.srodata.cst2) *(.srodata*)

[boxfire]

these are due to improvements in gcc riscv codegen, and all future ld scripts should have those if not using the default link script