Skip to content

SPI: Add API support for DDR, Dual, Quad and Octal modes #39991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

tbursztyka
Copy link
Collaborator

@tbursztyka tbursztyka commented Nov 2, 2021

(#20564 could not be reopened, so creating a new one.)

SPI extended modes

DDR: Double Data Rate
Dual: Dual MOSI lines
Quad: Quad MOSI lines
Octal: Octal MOSI lines
DQO: Dual Quand Octal

These modes comprise: DDR, Dual, Quad, Octal and the
specific command/address/data format

Uses cases

  • single MOSI line: cmd / addr / data ddr
  • single MOSI line: cmd / addr ddr / data ddr
  • single MOSI line: cmd ddr / addr ddr / data ddr
  • single MOSI line: cmd ddr
  • single MOSI line: data ddr
  • DQO MOSI lines: cmd single/addr single/data
  • DQO MOSI lines: cmd single/addr/data
  • DQO MOSI lines: cmd/addr/data
  • DQO MOSI lines: cmd
  • DQO MOSI lines: data
  • DBO uses cases with DDR

Design

Besides the operation attribute of struct spi_config, all goes through
a newly added u16_t attribute in struct spi_buf: flags.

Firts 9 bits are containing config flags, next 2 bits are the command
length (set to 0 if the cmd/addr format is not in use) and the last 5 bits
are the address length (set to 0 if the addr is not present or if the cmd/addr
format is not in use). Command length is a multiple of 4 (4, 8, 12, 16, ...)
and address length is a multiple of 2 factor 4 (4, 8, 16 ...)

The good side of this approach: it does not require changes on any existing SPI controller
drivers nor SPI device drivers.

The drawback: if only one SPI controller can support these modes, it adds up
16bits to each and every struct spi_buf in the system. Hopefully this is still
a relatively small amount of data (looks like uncommon for a driver to use more
than 2 struct spi_buf, and if does it will usually be due to a lack of factorization).

SPI configuration

spi_config {
	   ...
	   .operation = SPI_LINES_<SINGLE/DUAL/QUAD/OCTAL> | SPI_EXTENDED_MODES,
	   ...
}

Basic cmd/addr format transaction usage

Note: 8bits length cmd/addr are used for these examples, but it's trivial to
use other size.

TX only:

u8_t tx_buf[] = { CMD, ADDR };

struct spi_buf buf[2] =  {
	{
		.buf = tx_buf,
       		.len = 2,
		.flags = SPI_EM_CMD_<mode> | SPI_EM_ADDR_<mode> |
       	      	       SPI_EM_CMD_LENGTH_SET(SPI_EM_CMD_LEN_8_BITS) |
		       SPI_EM_ADR_LENGTH_SET(SPI_EM_ADR_LEN_8_BITS)
	},
	{
		.buf = data,
 		.len = data_length
	}
};

struct spi_buf_set tx {
       .buffers = buf,
       .count = 2
};

spi_transceive(spi_dev, spi_cfg, &tx, NULL)

RX added

u8_t tx_buf[] = { CMD, ADDR };

struct spi_buf buf_tx[1] =  {
	{
		.buf = tx_buf,
       		.len = 2,
		.flags = SPI_EM_CMD_<mode> | SPI_EM_ADDR_<mode> |
       	      	       SPI_EM_CMD_LENGTH_SET(SPI_EM_CMD_LEN_8_BITS) |
		       SPI_EM_ADR_LENGTH_SET(SPI_EM_ADR_LEN_8_BITS)
	}
};

struct spi_buf_set tx {
       .buffers = buf_tx,
       .count = 1
};

struct spi_buf buf_rx[1] =  {
	{
		.buf = data,
 		.len = data_length
	}
};

struct spi_buf_set rx {
       .buffers = buf_rx,
       .count = 1
};

spi_transceive(spi_dev, spi_cfg, &tx, &rx)

In case no address is given

u8_t tx_buf[] = { CMD };

struct spi_buf buf_tx[1] =  {
	{
		.buf = tx_buf,
       		.len = 2,
		.flags = SPI_EM_CMD_<mode> | SPI_EM_ADDR_<mode> |
       	      	       SPI_EM_CMD_LENGTH_SET(SPI_EM_CMD_LEN_8_BITS) |
		       SPI_EM_ADR_LENGTH_SET(SPI_EM_ADR_NONE)
	}
};

etc ...

@tbursztyka tbursztyka added area: SPI SPI bus RFC Request For Comments: want input from the community area: API Changes to public APIs DNM This PR should not be merged (Do Not Merge) labels Nov 2, 2021
@tbursztyka tbursztyka force-pushed the spi_enhanced_modes branch 2 times, most recently from 46cfc21 to c25095e Compare November 2, 2021 13:43
@tbursztyka
Copy link
Collaborator Author

@teburd interesting, I wrongly assumed that d/q/o modes were somewhat standardized across manufacturers and thus took only one hardware spec as a reference. I fail to see any use case that would require changing modes within the transaction but better be prepared (after all, devices using these modes - spi flash aside - are yet to become a common thing).

@tbursztyka tbursztyka force-pushed the spi_enhanced_modes branch from 81f9cf7 to 97270b5 Compare March 8, 2023 09:00
@scottwcpg
Copy link
Collaborator

I did not see a flag for data phase number of pin. Is the data phase using the same number of pins as the address phase?
Also, should we discuss a common approach for SPI flash "dummy" clocks and mode bytes. Most SPI flash devices recommend the only the first 3 or 4 bits of the mode byte are driven as data, the remaining bits are tri-state. Byte oriented SPI controllers can only drive a whole byte so a split mode byte can be done. Our QMSPI controller has a bit mode that can split a mode byte. Is it worth the effort for one controller? Probably not.

@teburd
Copy link
Collaborator

teburd commented Mar 8, 2023

I looked briefly at the SAM QSPI and it seems the flags match better there.

For SAM QSPI a seperate QSPI_IAR (Instruction Address Register) is available which can be used to setup an optional instruction frame before reading/writing data to the QSPI lines.

It offers optional fields that may be included for an instruction (8 bits), address (24 or 32 bit), and option (4-8 bits) written using 1 to 4 datalines each followed by reading/writing data in single/dual/quad spi.

I'm not entirely sure this supports all the flags but I think at least some subset of the flags here would work.

Actually the register set for it looks like a small extension of the normal sam spi IP.

@tbursztyka
Copy link
Collaborator Author

tbursztyka commented Mar 13, 2023

I'm not entirely sure this supports all the flags but I think at least some subset of the flags here would work.

As per-controller, it does (edit: no it does NOT... ) not have to support all the flags, it's fine. My concern is that the API should enable (most of) all the controllers (to a certain generalization extent).

I did not see a flag for data phase number of pin. Is the data phase using the same number of pins as the address phase?

There are flags for this. See SPI_EM_CMD_STD or SPI_EM_ADDR_STD for instance. It is either one line, or the whole set of lines.
I am not sure there is a need for more flexibility (2 lines for address, and 4 for data for instance). I lacked of verifiable use-cases when designing this API.

Also, should we discuss a common approach for SPI flash "dummy" clocks and mode bytes. Most SPI flash devices recommend the only the first 3 or 4 bits of the mode byte are driven as data, the remaining bits are tri-state. Byte oriented SPI controllers can only drive a whole byte so a split mode byte can be done. Our QMSPI controller has a bit mode that can split a mode byte. Is it worth the effort for one controller? Probably not.

The effort with this PR is to provide dual/quad/octal API exposed by generic SPI controllers, i.e. ones that can be wired to any type of SPI devices. Flash included BUT: I originally added flags for flash specific optimizations but it was not as good as doing a qspi flash driver from performance/flexibility point of view. I am not so sure it's worth the generalization effort to include flash idiosyncrasies, moreover that SPI controllers offers optimizations at different level compared to others. We could perhaps expose something as useful as XIP, but fine tuning options for flash... not sure.

@MaureenHelm
Copy link
Member

@tbursztyka can you implement this in a driver and demonstrate its use in an application?

@tbursztyka
Copy link
Collaborator Author

@tbursztyka can you implement this in a driver and demonstrate its use in an application?

Sure, I would need to find some time for it. But first, I need to find a platform that has a generic SPI controller capable of supporting dual/quad/octal as well, and a device to make transactions with.

@MaureenHelm
Copy link
Member

@tbursztyka can you implement this in a driver and demonstrate its use in an application?

Sure, I would need to find some time for it. But first, I need to find a platform that has a generic SPI controller capable of supporting dual/quad/octal as well, and a device to make transactions with.

Can you move this into the draft state until then?

@cfriedt
Copy link
Member

cfriedt commented Apr 28, 2023

Sure, I would need to find some time for it. But first, I need to find a platform that has a generic SPI controller capable of supporting dual/quad/octal as well, and a device to make transactions with.

@tbursztyka - this is a really interesting topic. I wonder if it might even be possible to list some relatively simple SPI peripherals that support each mode, and then maybe we could look into spi_emul and peripheral emulation for the time being. Tagging @jgl-meta because I know how much he loves QSPI 😁

@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jun 28, 2023
@jgl-meta jgl-meta removed the Stale label Jun 28, 2023
@cfriedt
Copy link
Member

cfriedt commented Aug 3, 2023

dev-review: @tbursztyka - do you have plans to move forward with this?

@MaureenHelm
Copy link
Member

dev-review: @tbursztyka - do you have plans to move forward with this?

@tbursztyka ping

Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@tbursztyka
Copy link
Collaborator Author

Reopening for further investigation related to #65659

@tbursztyka tbursztyka reopened this Feb 1, 2024
These modes are present on more advanced and recent SPI controller.
Though dual, quand and octal were present on configuration side already,
it could not work with the standard command/address/data format which is
now addressed via these changes.

Also adding a way to make use of hardware optimizations for SPI flash
JEDEC devices which seem to be largely present in various SPI
controllers.

Note, however, that these new SPI API modes are not meant to be used by
QSPI flash controllers: for these, a dedicated flash controller driver
will still be the preferred solution.

Fixes zephyrproject-rtos#17902

Signed-off-by: Tomasz Bursztyka <[email protected]>
@github-actions github-actions bot removed the Stale label Feb 2, 2024
Copy link

github-actions bot commented Apr 2, 2024

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.