MEGA65 / mega65-core

MEGA65 FPGA core
Other
240 stars 85 forks source link

MEGAFLASH: QSPI flash write-protect PPB bits are not checked on R3A #592

Open gardners opened 2 years ago

gardners commented 2 years ago

Test Environment (required) Any extant core.

Describe the bug The flash menu and jtagflash utilities do not query the PPB write-protect flags for flash sectors. But this information is required when diagnosing problems restoring core slot 0 in the flash when it has been somehow messed up.

To Reproduce Steps to reproduce the behavior:

  1. Lock a sector using the PPB flags
  2. Hold NO SCROLL and power cycle, or run jtagflash.prg via your favourite method.
  3. Attempt to flash a slot that includes the locked sector.
  4. Note that you will get an error about unable to write, but no explanation as to the root cause.

Expected behavior

  1. If a PPB bit is configured for a sector, this should be reported, and flashing stopped.

Screenshots N/A

Additional context N/A

lydon42 commented 1 year ago

the changes of 66c9f2b are no longer in development nor in 683-cartflash, as they made problems.

So we need to reinvestigate this, but as there is no thing that sets the write protect, I will need a testcase for this.

0xa000 commented 7 months ago

What is the use case for this? Normally the flash should unlocked, right? I did read somewhere that R3(A?) boards came with write protection enabled, is that true?

Checking write protection in a general way is a (little) bit tricky, because there is also block protection in addition to the PPB bits (and DYB bits, but these we can ignore). Also, for the 4K/64K mixed sector flash chips, determining the bit to check needs extra work. (I thought erasing also needed special care, but it turns out you can erase the 4K/64K chips as if they have a uniform 64K sector layout.)

lydon42 commented 1 month ago

I think this is more a function we need to add to mflash.prg, as it seems to me that this will come up during development and debugging, rather than on production systems. So we could opt to not add it right now.