Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential typo bug in foboot/main.c #333

Closed
neuschaefer opened this issue Dec 31, 2022 · 5 comments
Closed

Potential typo bug in foboot/main.c #333

neuschaefer opened this issue Dec 31, 2022 · 5 comments

Comments

@neuschaefer
Copy link

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
Copy link
Author

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

@xobs
Copy link
Member

xobs commented Dec 31, 2022

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
Copy link
Author

Thanks for clearing that up, @xobs!

@neuschaefer
Copy link
Author

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

xobs added a commit that referenced this issue Dec 31, 2022
The previous picorvspi code used an incorrect constant when
examining the CFM_EN bit. As a result, this mode would never
work.

Use the correct constant, in case anyone builds foboot using
a picorvspi hardware block.

This addresses #333.

Signed-off-by: Sean Cross <sean@xobs.io>
@xobs
Copy link
Member

xobs commented Dec 31, 2022

Thanks for pointing that out! I corrected that in dfa09fc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants