Feature qspi

[kl-cruz]

Description

Initial QSPI support for mbed. This pull request come with hal and driver prepositions and QSPI hal implementation for nRF52840. I will add simple application which show how to use it (similar to SPI or other serial drivers). There is also layer called memspec. C++ driver uses memspec to configure memory in proper state (SPI or QUAD, 24bit od 32bit addressing mode etc). This layer was separated from QSPI to allow creating solution based on reading sfdp (serial flash discoverable parameters).
Doxygen was added to every function.
There are a lot of changes (github counts more than 4k). Lot of them are related to removing spaces from end of the lines in sdk_config.h file. QSPI related changes count less than 2k lines of code.

Status

READY

Migrations

NO

Steps to test or reproduce

Simple application will be add as a external example

[kl-cruz]

@0xc0170 @nvlsianpu
Added qspi support (reference to last sync)

[kl-cruz]

Example app: https://github.com/kl-cruz/mbed-os-example-qspi
Compile with: mbed compile -t GCC_ARM -m NRF52840_dk

[theotherjimmy]

@kl-cruz How can we test this?

[adbridge]

retest uvisor

[adbridge]

/morph test

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 131

All builds and test passed!

[screamerbg]

@kl-cruz Could you please advise on testing QSPI? Perhaps a greentea test?

[adbridge]

retest uvisor

[kl-cruz]

The base test could just erase, write and read data to check consistency and prove that everything works well. For more complex tests I think we should implement (for example) software emulated memory device worked as simple SPI slave device in 1-1-1 mode (It might be hard to simulate 4 data wires QSPI mode device, but could be possible). 25xxx memory devices family is good choice to do it. It is generic, well known and widely spread.

[theotherjimmy]

@kl-cruz Could you write a test and place it with the others? (mbed-os/TESTS/mbed_drivers/qspi)

[0xc0170]

Please resolve conflicts

[kl-cruz]

Conflicts resolved. Sources aligned to master (prior to #4245)

@theotherjimmy I will do it if I'll find a while

[kl-cruz]

Sources rebased

[adbridge]

/morph test

[adbridge]

@kl-cruz Thanks for rebasing. Just a successful re-run of the CI and a test added, then we can take this.

[geky]

This is great! Having QSPI support would be a very welcome addition.

However I think this is a bit misaligned with the other storage apis. Have you had a chance to look at the SPIFBlockDevice and the BlockDevice interface?

The is how the current storage stack fits together:

FATFilesystem
implements FileSystem                            
uses BlockDevice
 |
 v
SPIFBlockDevice
implements BlockDevice
uses SPI
 |
 v
SPI
uses spi_api hal

From looking at the QSPI class, it looks like QSPI is pretty bound to use with flash devices. Could I use the QSPI class to communicate to another device I have rigged up as a QSPI slave?

If you are only interested in implementing a memory api for QSPI, I would suggest extending the BlockDevice class (maybe as the QSPIFBlockDevice?). This will give you filesystem support for free.

[geky]

Sorry for putting a red X on this, but I want to make sure what's coming in is aligned with where the storage APIs are going. It would be a shame for QSPI support to come in and not be picked up since it doesn't fit into the filesystem work we are doing.

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 252

All builds and test passed!

[kl-cruz]

@geky I implemented generic QSPI driver which can work across different MCUs and keep driver ecosystem clean and consistent. I also implemented (internally, as a POC) simple block device and connect it to filesystem. I had some troubles with memory alignment and filesystem (when I just read whole image from memory and tried to mount it under Windows; header looked like aligned to 4k data). I think those issues were connected with aligning filesystem sources to SD card. I worked on it some time ago, but as I remember this issue show me that every operation on SD card uses 4k data, but QSPI memory uses smaller/different ones for every operation (read 1B, write 1B or 256B, erase 4kB). A lot of operations in https://github.com/ARMmbed/mbed-os/blob/master/features/filesystem/fat/FATFileSystem.cpp file use erase size data, maybe it is place to dig into. Block device layer looks to be easy to prepare, but currently I have to change priorities a little. I will comment my pull requests, don't worry!

[kl-cruz]

In addition. Yes, QSPI is a complex peripheral and was created to fit to for example block devices. This driver implements only master side of QSPI peripheral. I have never seen QSPI slave in MCUs (Did You think about QSPI slave driver or using QSPI master with different types of memories?).

[geky]

Hi @kl-cruz, sorry for wording my question poorly.

The SPI driver in mbed OS is designed to be general purpose:
https://github.com/ARMmbed/mbed-os/blob/master/drivers/SPI.h#L75

Is it possible (or reasonable) to also make QSPI general purpose? If it's possible to provide a general purpose QSPI class, we want to reserve the class QSPI for future work.

We can still adopt a QSPI block device without the general purpose QSPI class, but this would need to follow the block device naming convention (QSPIFBlockDevice) and inherit from BlockDevice.

As a component, it would also not need to be merged into mbed OS. You can look at the SPIFBlockDevice as an example:
https://github.com/armmbed/spiflash-driver

[kl-cruz]

As I understand general purpose means to isolate from QSPI driver part look-like SPI (for raw transfers on four lines) and layer to add command and address bytes on higher layer. Am I right?
If it is true then I do not know if it is good solution. It would introduce extra layer and overcomplicate peripheral driver. QSPI peripheral is powerful when You use it in huge and fast transmissions with its own protocol.

I spent some time to find devices which use four data lines for data exchange to compare how the communication look like across these devices. I found small black&white LCDs which use four lines of data for communication and does not use QSPI as a protocol, but in my opinion it is not the device on which we should focus. I think our targets should be more powerful devices controlled by QSPI protocol like memory devices from family 25xxx, 26xxx, nvsram devices for example from Cypress or LCD driver from FTDI:
http://www.ftdichip.com/Support/Documents/DataSheets/ICs/DS_FT81x.pdf (see 4.1.1 chapter, below the table)

QSPI is more than peripheral. It is also some kind of protocol to control device (data: command+address+data, settings: command+options).
So I think we should focus on QSPI driver supporting QSPI protocol with memory-like devices. I think this API fits this.

[sg-]

What about something like WLAN devices? http://www.cypress.com/file/298076/download From a HAL perspective it seems best to expose read/write as SPI would and then further up the stack drive this as a WLAN device, memory device, LCD, etc. May need a bit more digging into but from a quick glance QSPI, SDIO, gSPI and a few others all seem compatible with different names but I'm sure there are corner cases.

[theotherjimmy]

@kl-cruz Could you respond to @sg-'s comments? Could you rebase to resolve conflicts?

[theotherjimmy]

@kl-cruz bump. Could you address my prior comment?

[kl-cruz]

Sorry Sam, I have some holiday now. I'll back to this at the beginning of July.
I look through Cypress's doc of this WLAN chip and looks like this chip supports SDIO and generic SPI. Those protocol are different compared to QSPI. QSPI peripheral has embed its own protocol. SDIO uses different hw protocol. SPI is protocol agnostic peripheral.
For example even STM32 has different peripheral for QSPI and SDIO (at least doc shows it in this way).
As I wrote, I'll back to this PR in near future.

[theotherjimmy]

@kl-cruz Bump. Any news?

[geky]

@sg-, @screamerbg, I'm still new to licensing things, is this copyright notice correct for mbed OS?

[theotherjimmy]

@kl-cruz Bump. Any news?

[kl-cruz]

Hi, sure. At last I have a moment to response to some comments. Copyright notice has to be checked by proper person.
API is similar to linux's spi-nor API (see https://github.com/torvalds/linux/blob/master/include/linux/mtd/spi-nor.h#L279 )

Coming back to code and driver. Unfortunately I have a lot of other work now and can spend only quick moment per week on this feature. It would be good to find someone to take over this code and publish it as it is or align to your concept.

[theotherjimmy]

@AnotherButler Another new API.

@kl-cruz How have you been? Is there anything we can do to help you along?

[mbed-ci]

Build : ABORTED

Build number : 88
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/4229/

[adbridge]

@kl-cruz looks like this needs a rebase ?

[adbridge]

@kl-cruz bump..

[kl-cruz]

Sorry, I have no time now. I will back to this code this or next month

[0xc0170]

Hi @kl-cruz,

thanks for this proposal, great effort with HAL and also high level API.
I am going to focus on HAL mainly, as that one is the starting point and I am currently looking at it.

How would nordic SDK map to more generic QSPI HAL API? This could be a proposal:

qspi_status_t qspi_write(qspi_t *obj, qspi_command_t *command, void *data, size_t length);
qspi_status_t qspi_read(qspi_t *obj, qspi_command_t *command, void *data, size_t length);
qspi_status_t qspi_write_command(qspi_t *obj, qspi_command_t *command);

Providing write/read and write only command phase. The command structure would provide the frame format configuration as follows:

----------------------------------------------
| Instruction | Address | Alt | Dummy | Data |
----------------------------------------------

To define a complete command object, this could be used

typedef structure qspi_command {
    struct instruction {
        qspi_bus_width_t bus_width; /**< Bus width for the instruction >*/
        uint8_t value;  /**< Instruction, 0 - disabled, non-zero defined value used >*/
    }
    struct address {
        qspi_bus_width_t bus_width; /**< Bus width for the address >*/
        qspi_address_size_t size; /**< Address size >*/
        uint32_t value; /**< Address, 0 - disabled, non-zero defined value used >*/
    };
    struct alt {
        qspi_bus_width_t bus_width; /**< Bus width for alternative  >*/
        qspi_alt_size_t size; /**< Alternative size >*/
        uint32_t value; /**< Alternative, 0 - disabled, non-zero defined value used >*/
    };
    uint8_t dummy_count; /**< Dummy cycles count >*/
    struct data {
        qspi_bus_width_t bus_width; /**< Bus width for data >*/
    }
} qspi_command_t;

On top of these, there could be higher level API that is filesystem like, read/write/erase type. You have something similar covered in custom instruction function in HAL (both read/write covered there).

I find qspi modes hard to understand as it is in this patch (mode 1_1_1, or 1_1_2), being separated for write/read, 3 numbers that decode I assume single/dual/quad lines based on the nordic SDK docs - intruction, addres + data phase - how many lines are used (easily understood from nor linux driver). I find it easier how for instance at91bootstrap does it or having the above, where you define bus width configuration. This is a code snippet from at91bootstrap, or isn't it? Same is in the linux nor drivers, where they decode if its addr or instr or data.

typedef enum qspi_protocol {
    QSPI_PROTO_1_1_1,   /* SINGLE BIT SPI */
    QSPI_PROTO_1_1_2,   /* DUAL OUTPUT */
    QSPI_PROTO_1_1_4,   /* QUAD OUTPUT */
    QSPI_PROTO_1_2_2,   /* DUAL IO */
    QSPI_PROTO_1_4_4,   /* QUAD IO */
    QSPI_PROTO_2_2_2,   /* DUAL CMD */
    QSPI_PROTO_4_4_4,   /* QUAD CMD */
} qspi_protocol_t;

This could be potentionaly used to define a protocol, and have one function send_command that would do either read/write based on these along with other configuration bits. This could be potentionally similar API to what I proposed above.

I find the former easier to use and implement. On top of this one, high level API would be implemented for instance: write_program_page would invoke write() with a command configured accordingly, erase_sector similar write() with a command configured for erasing, etc. As I read through the discussion here, I think we are all on the same page (having low level API that is easy to read/implement), just need to align on how should low level API look like. Thus I am proposing something a bit diferent than in this patch.

I'll set up the proposal PR for QSPI HAL where we can have a deeper look.

Update: please see the below PR with a proposal that has some updates since this comment has been posted

[adbridge]

@kl-cruz Any update on this ?

[0xc0170]

We can close this PR.

Please any further questions target the QSPI feature branch. Thanks @kl-cruz for all the work 👍 , it helped us a lot (more to see on the feature-qspi branch that is still in the development)