im-tomu / foboot

Bootloader for Fomu
Apache License 2.0
99 stars 32 forks source link

Potential typo bug in foboot/main.c #333

Closed neuschaefer closed 1 year ago

neuschaefer commented 1 year ago

In the reboot function we have this piece of code:

    if (boot_config & 0x00000002) // DDR_EN
        picorvspi_cfg3_write(picorvspi_cfg3_read() | 0x40);
    if (boot_config & 0x00000002) // CFM_EN
        picorvspi_cfg3_write(picorvspi_cfg3_read() | 0x10);

The comments document different bit names, but the bit is the same (0x2 in both if conditions). This looks like a bug.

neuschaefer commented 1 year ago

According to BOOT-SEQUENCE.md, CFM_EN is 0x4.

xobs commented 1 year ago

Thanks for pointing this out!

I've removed the foboot-main directory and moved it to a branch.

The general thought was that there would be a simple foboot ROM that gets shipped as part of the default SPI image, but a larger and more complex "Foboot Main" could live on SPI flash. If the boot ROM detected a magic number on the main SPI, it would jump to that.

Foboot Main would present a UF2 filesystem or similar interface. The exact specifics of which were never ironed out.

Early on in the life of Fomu, it used the PicoRvSpi block for doing XIP spi. This was deprecated in favour of the litex SPI block which was more resource-efficient (if a bit slower).

Foboot Main had rotted to the point where it was no longer usable. As you noted, it wasn't even using the correct bit-bang SPI driver.

Since it still may be of academic interest to someone, I'll keep it as a branch (https://github.com/im-tomu/foboot/tree/foboot-main/foboot-main) but I'll remove it from the main branch to prevent future confusion.

neuschaefer commented 1 year ago

Thanks for clearing that up, @xobs!

neuschaefer commented 1 year ago

FWIW, the same typo also exists in sw/src/main.c

xobs commented 1 year ago

Thanks for pointing that out! I corrected that in dfa09fc84b0acb4d9a2d409f3a381274a1551e9f