MEGA65 / mega65-core

MEGA65 FPGA core
Other
237 stars 84 forks source link

QSPI: Busy flag ```sdio_busy``` is unreliable #763

Closed 0xa000 closed 5 days ago

0xa000 commented 6 months ago

Test Environment (required)

Describe the bug The flag that indicates (amongst other things) if a hardware assisted QSPI operation is being executed is unreliable, that is, it can be set to '0' (false) even though a QSPI operation is in fact being executed.

Although all relevant QSPI hardware assisted operations update the sd_state signal and assign '1' to the sdio_busy signal, these changes only take effect after the VHDL process ends. Often, the value of sd_state at the time a hardware assisted operation is started is Idle.

The state machine that handles sd_state is part of the same VHDL process that handles starting QSPI hardware assisted operations. The handler for sd_state equal to Idle assigns '0' to the sdio_busy signal. Since this VHDL code comes after the code that assigns '1' to the sdio_busy signal in the same process, the first assignment is effectively lost.

To Reproduce Create a test application that executes a long running QSPI operation (for example, a 64K sector erase) and observe that the sdio_busy flag is never set to 1. The busy flag can be observed by PEEK-ing $D680, bit 0.

Expected behavior The value of the busy flag should be equal to '1' as long as a QSPI hardware assisted operation is being executed.

Additional context In sdcardio.vhdl, there is another piece of code that could reset the value of the sdcard_busy signal to '0'. At the moment I'm not certain if it will ever pose a problem, though. It's on lines 3791 -- 3795:

if last_sd_error /= x"0000" then
    sdio_error <= '1';
    sdio_busy <= '0';
    sd_state <= Idle;
end if;
gardners commented 1 month ago

Consider addressing together with work on #766

lydon42 commented 1 month ago

The fix should be quite simple:

lydon42 commented 1 month ago

Handling in 782-mflash-lowlevel, as we need this for MEGAFLASH refactoring.

lydon42 commented 1 month ago

Test with mflash.prg, replacing the delay loop in erase_sector with a sdio_busy wait loop, now results in a stable flash (r6, with attic cache).

gardners commented 1 month ago

Let me know when done, so I can merge the change into my sdcard cache work on #766 as well. Thanks for tackling this.

lydon42 commented 5 days ago

I deem this fixed.