[PATCH 01/24] spi: spi-mem: Extend spi-mem operations with a per-operation maximum frequency
Han Xu
han.xu at nxp.com
Wed Oct 30 13:52:52 PDT 2024
On 24/10/25 06:14PM, Miquel Raynal wrote:
> In the spi subsystem, the bus frequency is derived as follows:
> - the controller may expose a minimum and maximum operating frequency
> - the hardware description, through the spi peripheral properties,
> advise what is the maximum acceptable frequency from a device/wiring
> point of view.
> Transfers must be observed at a frequency which fits both (so in
> practice, the lowest maximum).
>
> Actually, this second point mixes two information and already takes the
> lowest frequency among:
> - what the spi device is capable of (what is written in the component
> datasheet)
> - what the wiring allows (electromagnetic sensibility, crossovers,
> terminations, antenna effect, etc).
>
> This logic works until spi devices are no longer capable of sustaining
> their highest frequency regardless of the operation. Spi memories are
> typically subject to such variation. Some devices are capable of
> spitting their internally stored data (essentially in read mode) at a
> very fast rate, typically up to 166MHz on Winbond SPI-NAND chips, using
> "fast" commands. However, some of the low-end operations, such as
> regular page read-from-cache commands, are more limited and can only be
> executed at 54MHz at most. This is currently a problem in the SPI-NAND
> subsystem. Another situation, even if not yet supported, will be with
> DTR commands, when the data is latched on both edges of the clock. The
> same chips as mentioned previously are in this case limited to
> 80MHz. Yet another example might be continuous reads, which, under
> certain circumstances, can also run at most at 104 or 120MHz.
>
> As a matter of fact, the "one frequency per chip" policy is outdated and
> more fine grain configuration is needed: we need to allow per-operation
> frequency limitations. So far, all datasheets I encountered advertise a
> maximum default frequency, which need to be lowered for certain specific
> operations. So based on the current infrastructure, we can still expect
> firmware (device trees in general) to continued advertising the same
> maximum speed which is a mix between the PCB limitations and the chip
> maximum capability, and expect per-operation lower frequencies when this
> is relevant.
Hi Miquel,
I met the similar problem when working on the Micron MT35XU01GBBA SPI NOR chip.
The chip can work at 166MHz in SDR mode but 200MHz in DDR mode. I found Read ID
failed on some platforms when using 200MHz as maximum frequency, so I have to
lower the maximum frequency with some performance loss.
I think the patch is useful but the SPI NOR doesn't have such vendor-specific
predefined SPI_MEM_OPS like SPI NAND. Do you have any suggestion on how to handle
this case? Thanks.
>
> Add a `struct spi_mem_op` member to carry this information. Not
> providing this field explicitly from upper layers means that there is no
> further constraint and the default spi device maximum speed will be
> carried instead. The SPI_MEM_OP() macro is also expanded with an
> optional frequency argument, because virtually all operations can be
> subject to such a limitation, and this will allow for a smooth and
> discrete transition.
>
> For controller drivers which do not implement the spi-mem interface, the
> per-transfer speed is also set acordingly to a lower (than the maximum
> default) speed, or 0, to comply with the current API.
>
> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
> ---
> drivers/spi/spi-mem.c | 8 ++++++++
> include/linux/spi/spi-mem.h | 11 ++++++++++-
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index 17b8baf749e6..ab650ae953bb 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -356,6 +356,7 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
> {
> unsigned int tmpbufsize, xferpos = 0, totalxferlen = 0;
> struct spi_controller *ctlr = mem->spi->controller;
> + unsigned int xfer_speed = op->max_freq;
> struct spi_transfer xfers[4] = { };
> struct spi_message msg;
> u8 *tmpbuf;
> @@ -368,6 +369,9 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
> if (!spi_mem_internal_supports_op(mem, op))
> return -EOPNOTSUPP;
>
> + if (!op->max_freq || op->max_freq > mem->spi->max_speed_hz)
> + ((struct spi_mem_op *)op)->max_freq = mem->spi->max_speed_hz;
> +
> if (ctlr->mem_ops && ctlr->mem_ops->exec_op && !spi_get_csgpiod(mem->spi, 0)) {
> ret = spi_mem_access_start(mem);
> if (ret)
> @@ -407,6 +411,7 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
> xfers[xferpos].tx_buf = tmpbuf;
> xfers[xferpos].len = op->cmd.nbytes;
> xfers[xferpos].tx_nbits = op->cmd.buswidth;
> + xfers[xferpos].speed_hz = xfer_speed;
> spi_message_add_tail(&xfers[xferpos], &msg);
> xferpos++;
> totalxferlen++;
> @@ -421,6 +426,7 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
> xfers[xferpos].tx_buf = tmpbuf + 1;
> xfers[xferpos].len = op->addr.nbytes;
> xfers[xferpos].tx_nbits = op->addr.buswidth;
> + xfers[xferpos].speed_hz = xfer_speed;
> spi_message_add_tail(&xfers[xferpos], &msg);
> xferpos++;
> totalxferlen += op->addr.nbytes;
> @@ -432,6 +438,7 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
> xfers[xferpos].len = op->dummy.nbytes;
> xfers[xferpos].tx_nbits = op->dummy.buswidth;
> xfers[xferpos].dummy_data = 1;
> + xfers[xferpos].speed_hz = xfer_speed;
> spi_message_add_tail(&xfers[xferpos], &msg);
> xferpos++;
> totalxferlen += op->dummy.nbytes;
> @@ -447,6 +454,7 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
> }
>
> xfers[xferpos].len = op->data.nbytes;
> + xfers[xferpos].speed_hz = xfer_speed;
> spi_message_add_tail(&xfers[xferpos], &msg);
> xferpos++;
> totalxferlen += op->data.nbytes;
> diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
> index f866d5c8ed32..8963f236911b 100644
> --- a/include/linux/spi/spi-mem.h
> +++ b/include/linux/spi/spi-mem.h
> @@ -68,6 +68,9 @@ enum spi_mem_data_dir {
> SPI_MEM_DATA_OUT,
> };
>
> +#define SPI_MEM_OP_MAX_FREQ(__freq) \
> + .max_freq = __freq
> +
> /**
> * struct spi_mem_op - describes a SPI memory operation
> * @cmd.nbytes: number of opcode bytes (only 1 or 2 are valid). The opcode is
> @@ -95,6 +98,9 @@ enum spi_mem_data_dir {
> * operation does not involve transferring data
> * @data.buf.in: input buffer (must be DMA-able)
> * @data.buf.out: output buffer (must be DMA-able)
> + * @max_freq: frequency limitation wrt this operation. 0 means there is no
> + * specific constraint and the highest achievable frequency can be
> + * attempted).
> */
> struct spi_mem_op {
> struct {
> @@ -132,14 +138,17 @@ struct spi_mem_op {
> const void *out;
> } buf;
> } data;
> +
> + unsigned int max_freq;
> };
>
> -#define SPI_MEM_OP(__cmd, __addr, __dummy, __data) \
> +#define SPI_MEM_OP(__cmd, __addr, __dummy, __data, ...) \
> { \
> .cmd = __cmd, \
> .addr = __addr, \
> .dummy = __dummy, \
> .data = __data, \
> + __VA_ARGS__ \
> }
>
> /**
> --
> 2.43.0
>
More information about the linux-mtd
mailing list