Off-by-one error in spiFlash prevents writing to the last byte of the flash

[rwhitby]

openFPGALoader/src/spiFlash.cpp

Line 261 in 0283f18

if ((unsigned int)(base_addr + len) > (_flash_model->nr_sector * 0x10000) - 1) {

There seems to be an off-by-one error on this line, which prevents writing to the very last byte of the flash.

[trabucayre]

For instance for the W25Q16 device we have:

{0xef4015, {
    .manufacturer = "Winbond",
    .model = "W25Q16",
    .nr_sector = 32,
    [...]
}, 

• so the storage capacity is 32 * 0x10000 = 0x200000 or 2MB
• as for any array allowed address are from 0 to n-1 (see figure 2 of W25Q16's datasheet), so last block (or sector depending on doc) is 31 (again -1 is required) and the last valid address is 0x200000 - 1 = 0x1FFFFF.

So If you try to write in 0x200000, it's the first Byte of the block 32 (not allowed for W25Q16 but correct for W25Q32).

This is why a -1 is used to convert capacity for upper address to check if address is not strictly greater than last Byte (it's a > not a =>.

[rwhitby]

If I try and write a 0x10000 byte file (1 sector) to location 0x1F0000, then I'm writing all the way up to and including the last byte of the flash, but I am not writing past the last byte.

if ((unsigned int)(base_addr + len) > (_flash_model->nr_sector * 0x10000) - 1) {

base_addr = 0x1F0000, len = 0x10000, nr_sector = 32

(0x1F0000 + 0x10000) > ((32 * 0x10000) - 1)

Therefore I am prevented from writing the last sector.

[trabucayre]

I'm dumb it's true! I've focalized my observation to the right side of the comparison but not to the left one.
This test compare the last allowed address (right) with the write size (left) so It's true the -1 has to be removed.

[rwhitby]

Thanks for accepting the issue - really appreciate this work.

[trabucayre]

Done!

I have to find a way to create unitest to check subtilities like this one!
Thanks!

[trabucayre]

It's okay for you?
Can I close this issue ?
Thanks

[rwhitby]

Working well, thanks.