Supported chip not found

[mzero]

flash_devices.h has device descirptor initializor lists for 20 different flash chips. But possible_devices, the list that begin() looks through after getting the jedec id, lists only 8 of these. (in Adafruit_SPIFLashBase.cpp).

I have a custom board with one of the non-listed flash chips, S25FL064L. I could create the descriptor myself, initialized from the macro in flash_devices.h, and pass it to begin()... but my code also runs on a standard Feather M0 Express, which uses a different chip, and is listed in possible_devices. And I'd like to do this from the same binary.

So I want the behavior of looking up the descriptor based on the jedec id... but I need my flash chip in the list.

I see three possible fixes for this, and will submit a pull request, but I'm interested to know which way you'd like to spin this:

1: Just add the missing 12 chip descriptors to possible_devices. This will add 12*16 = 192 bytes to the static image size of any user of the library.

2: Make a version of begin() that takes a list of additional possible devices, not just a single device. Have begin() search through the supplied list, and thepossible_devices.

3: Don't change the library, just call begin() and if that fails, call it again with the descriptor of the chip and see if that works. begin() would need to ensure that it is safe to call again if it fails. I'm not sure if the internal transport object supports having its begin() method called twice.

[hathach]

flash_devices.h has device descirptor initializor lists for 20 different flash chips. But possible_devices, the list that begin() looks through after getting the jedec id, lists only 8 of these. (in Adafruit_SPIFLashBase.cpp).

I have a custom board with one of the non-listed flash chips, S25FL064L. I could create the descriptor myself, initialized from the macro in flash_devices.h, and pass it to begin()... but my code also runs on a standard Feather M0 Express, which uses a different chip, and is listed in possible_devices. And I'd like to do this from the same binary.

So I want the behavior of looking up the descriptor based on the jedec id... but I need my flash chip in the list.

I see three possible fixes for this, and will submit a pull request, but I'm interested to know which way you'd like to spin this:

1: Just add the missing 12 chip descriptors to possible_devices. This will add 12*16 = 192 bytes to the static image size of any user of the library.

We cannot support everyone's flash device, this issue will come up again later. So I don't like this method

2: Make a version of begin() that takes a list of additional possible devices, not just a single device. Have begin() search through the supplied list, and thepossible_devices.

This is preferable, but I want to change it a bit, we still use the current begin(SPIFlash_Device_t*) but change its behavior from using exclusive input flash_dev to first try with input, if failed use the possible_devices as last resort

3: Don't change the library, just call begin() and if that fails, call it again with the descriptor of the chip and see if that works. begin() would need to ensure that it is safe to call again if it fails. I'm not sure if the internal transport object supports having its begin() method called twice.

This is also good, we can update the lib to implement the end() before calling the begin() again. However, modified method 2) is better since it need less modification. We can add this later on as well to make sure the library fail cleanly.

[mzero]

1- Agreed. It is very nice that flash_devices.h can have the parameters for all sorts of chips - without costing anything in memory for those that don't need them.

2- Your change will work for me... now.... but what happens when I rev the board and I use a 2nd chip? Now I've got two chips that are in flash-devices.h but just not in the search list. I really should be able to pass a list of chips.

So why not do both? How about have a signature of:

bool begin(begin(SPIFlash_Device_t const *flash_dev = NULL, int count = 1)

Behavior becomes:

• begin() -- exactly as it is now, just search the possible_devices list.
• begin(&my_device) -- as you proposed: check id against my_device, if no match, search possible_devices. This is safer than the current implementation which just uses the supplied device whether it matches or not.
• begin(&my_devices, num_my_devices) -- as I proposed: check id against supplied device list, if no match, search possible_devices

3- Agreed, option 2 (some form of it) is better.

[hathach]

yeah right, let's add the count to make it an array. and keep the current behavior to use either input array or possible_devices exclusively That will make the API less confusing. Thanks for the suggestion.

[mzero]

I'll prepare the pull request today

[hathach]

fixed by #55