SPIFBlockDevice has memory alignment issues with SST26VF016 on k64f

[Spinnaker-design]

Description of defect

Erase calls a "no region found for address error"

program operations appear to be offset by 1

Running the spif example (with some additional printfs) from https://os.mbed.com/docs/mbed-os/v6.2/apis/spi-flash-block-device.html

produces the following output:

spif size: 2097152
spif read size: 1
spif program size: 1
spif erase size: 4096
Erase Failed: -4005
Contents: ello World!

Target(s) affected by this defect ?

K64F

Toolchain(s) (name and version) displaying this defect ?

ARM6

What version of Mbed-os are you using (tag or sha) ?

6.2.0
SHA: a2ada74

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed studio

How is this defect reproduced ?

Run the example from https://os.mbed.com/docs/mbed-os/v6.2/apis/spi-flash-block-device.html on a board with a k64f and a SST26VF016

[Spinnaker-design]

Some additional information:
If I do the following:

char *buffer = (char *)malloc(spif.get_erase_size());
    sprintf(buffer, "Hello World!!!\n");
    
    rc = spif.erase(32768 , spif.get_erase_size());
    if(rc != 0){
        printf("Erase Failed: %d\n", rc);
    }

    rc = spif.program(buffer, 32768 , spif.get_erase_size());
    if(rc != 0){
        printf("Program Failed: %d\n", rc);
    }

    sprintf(buffer, "NOPE\n");
    // Read back what was stored
    rc = spif.read(buffer, 32767, spif.get_erase_size());

    if(rc != 0){
        printf("Read Failed: %d\n", rc);
    }
    printf("Contents: %s\n", buffer);

    // Deinitialize the device
    spif.deinit();
}

I actually get the expected output of:
spif read size: 1
spif program size: 1
spif erase size: 4096
Contents: Hello World!!!

So it appears that there are two issues:

1. The erase function will not work below the address of 32768
2. Either the program command is writing to addr - 1 or the read command is reading from addr + 1

[0xc0170]

cc @ARMmbed/mbed-os-storage

[Spinnaker-design]

Any update on this, it's causing us a significant delay?

[boraozgen]

I am having the same issue with the same chip. Another flash chip works fine (Winbond W25Q32JV). Wondering if this issue is chip or driver related?

[sklatham69]

OK ... we seem to have verified that the read command issues a dummy byte when not necessary. If we set "dummy_bytes = 0" for instruction 0x03 it seems to fix the problem of losing the first byte on the read. We still can't erase from address 0 and that looks like a bug in the driver. We are having to set the SPIF block device at 32768 and apply the fix above. Testing this solution today so we'll provide more detail later today.

[sklatham69]

@boraozgen It looks like it's an Mbed driver issue. We have been on with Microchip for some time chasing this issue.

[boraozgen]

Are you sure that the problem is on the read but not the write? My application was fixed when I added +1 to the write address. It obviously does not work for APIs that use the BD, such as KVStore...

I believe the erase issue is a separate bug, caused by the SFDP parsing of the driver.

[sklatham69]

We do think it may be related to the SFDP parsing but only because it doesn't seem to look at the dummy byte parameter on INIT.
We don't have a write problem (that we know of) since we see the data coming back on the logic analyzer. We need to use the SPIFblockdevice because of the file system we are using (at least it makes it much easier). We'll have more later today.

[boraozgen]

After @sklatham69 mentioned that the issue is related to the dummy bytes, I had some time to dig into this and found that this line might be causing the issue:

mbed-os/storage/blockdevice/COMPONENT_SPIF/source/SPIFBlockDevice.cpp

Line 500 in fbe0409

_read_dummy_and_mode_cycles = 8;

Actually the usage of _read_dummy_and_mode_cycles and _dummy_and_mode_cycles seems to be a bit inconsistent and probably needs refactoring/explanation. For this particular case, as _dummy_and_mode_cycles is set in the next line and this variable is used in the following _spi_send_read_command(), setting _read_dummy_and_mode_cycles is unnecessary. Furthermore, this causes the issue with SST26 since after _sfdp_detect_best_bus_read_mode() sets this value to 0, which is the default value for SPI chips AFAIK, the _spi_send_read_sfdp_command is called again, and it is set to 8, which is never again set back to 0.

When I remove this line, the driver works properly for both of the SPI chips I have, the SST26VF016B and W25Q32JV. I do not know the requirements for other chips and cannot test them. I can still issue a PR for this if needed. Nevertheless, I believe the whole module should be refactored with this in mind.

I have yet to look into the erase address 0 issue, which is mentioned in the forum topic https://forums.mbed.com/t/spifblockdevice-not-erasing-or-writing-to-the-sst26vf016-on-mk64f/9489

[boraozgen]

I believe this function causes the address 0 issue, since it returns -1 if the searched address is 0 and the region count is more than 1 (i.e. the sector table has been parsed):

mbed-os/drivers/source/SFDP.cpp

Lines 376 to 394 in 2165297

	int sfdp_find_addr_region(bd_size_t offset, const sfdp_hdr_info &sfdp_info)
	{
	if ((offset > sfdp_info.bptbl.device_size_bytes) || (sfdp_info.smptbl.region_cnt == 0)) {
	return -1;
	}
	if (sfdp_info.smptbl.region_cnt == 1) {
	return 0;
	}
	for (int idx = sfdp_info.smptbl.region_cnt - 2; idx >= 0; idx--) {
	if (offset > sfdp_info.smptbl.region_high_boundary[idx]) {
	return (idx + 1);
	}
	}
	return -1;
	}

I will open another issue for this as it is a separate bug.

[sklatham69]

We did a work around that essentially bypasses the table. @Spinnaker-design can elaborate when he comes online in a couple hours. We currently have the SST26VF016B in use with littlefs/Mbed6.3 and our two workarounds (dummy byte removed for our read 0x3 and table bypassed). Again, I'll have @Spinnaker-design elaborate in case that helps the process. You all seem to have everything in order already but the modifications may provide more insight.