cnlohr / rv003usb

CH32V003 RISC-V Pure Software USB Controller
MIT License
436 stars 43 forks source link

Fix Bootloader not Flashing #57

Closed BogdanTheGeek closed 5 days ago

BogdanTheGeek commented 4 months ago

Issue:

Found B003Fun Bootloader
Halting Boot Countdown
Interface Setup
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
ioctl (SFEATURE): Protocol error
Warning: Issue with hid_send_feature_report. Retrying
Error: Flash operation error
Error: Failed to erase sector at 08000000
Error writing block at memory 08000000 (error = -9)
Error: Fault writing image.
make: *** [ch32v003fun/ch32v003fun/ch32v003fun.mk:188: cv_flash] Error 243
xsrf commented 4 months ago

This fix adds 20 bytes to the bootloader, rendering even more of the boot configurations unusable. Can you explain when you encountered the issue and why this fix fixes it? I've never had issues with the bootloader not flashing on windows and linux.

BogdanTheGeek commented 4 months ago

Can you explain when you encountered the issue and why this fix fixes it?

@cnlohr troubleshooted the issue on discord last Saturday and wrote the fix. From what I understood and remember, because flash writes are blocking, the device sent the response at the wrong time. He mentioned that most usb hubs are not as fussy, but that this fix is the "correct" way of handling the blocking writes. I hope I am not misrepresenting his view. I can share the usb traces if that helps.

This fix adds 20 bytes to the bootloader

My compiler seems to be a bit more aggressive with optimisations. Its only 16 bytes larger maste:

riscv64-unknown-elf-size bootloader.elf
   text    data     bss     dec     hex filename
   1884       0      92    1976     7b8 bootloader.elf

fix:

riscv64-unknown-elf-size bootloader.elf
   text    data     bss     dec     hex filename
   1896       0      96    1992     7c8 bootloader.elf
xsrf commented 4 months ago

Ah okay, thx. Guess that's a pill we have to take then. Yours is only +12 Bytes btw ;) For me (on windows) master actually compiles 8 Bytes less but with the PR it's also 1896.

> make build
riscv64-unknown-elf-gcc -o bootloader.elf  bootloader.c ../rv003usb/rv003usb.S  -g -Os -flto -ffunction-sections -fdata-sections -fmessage-length=0 -msmall-data-limit=8 -march=rv32ec 
-mabi=ilp32e -DCH32V003=1 -static-libgcc -I/usr/include/newlib -I../ch32v003fun/ch32v003fun/../extralibs -I../ch32v003fun/ch32v003fun -nostdlib -I. -Wall -I. -I../lib -DUSE_TINY_BOOT 
-I../rv003usb -L../ch32v003fun/ch32v003fun/../misc -lgcc -T ch32v003fun-usb-bootloader.ld -Wl,--gc-sections
riscv64-unknown-elf-size bootloader.elf
   text    data     bss     dec     hex filename
   1876       0      92    1968     7b0 bootloader.elf
BogdanTheGeek commented 4 months ago

Yours is only +12 Bytes btw ;)

I was looking at dec not text. bss also increase by 4 bytes because of the new global.

EDIT: an no, making it a u8 or u16 didn't reduce it, i guess its pretty well optimised.

xsrf commented 4 months ago

The bss is not relevant. The text needs to be 1920 or less.

cnlohr commented 2 months ago

Circling back, @xsrf is this something that should be merged?

cnlohr commented 5 days ago

Tested, good now! Just was weird because of some slightly incorrect macro logic.