Re: [PATCH v4 3/5] mtd: devices: m25p80: add support for mmap read request

[Cyrille Pitchen <cyrille.pitchen@xxxxxxxxx>]

Hi Vignesh,

Le 30/11/2015 06:15, Vignesh R a écrit :
> Certain spi controllers may provide accelerated interface to read from
> m25p80 type flash devices. This interface provides better read
> performance than regular SPI interface.
> Call spi_flash_read(), if supported, to make use of such interface.
> 
> Signed-off-by: Vignesh R <vigneshr@xxxxxx>
> ---
> 
> v4: 
>  * Use spi_flash_read_message struct to pass args.
>  * support passing of opcode/addr/data nbits.
> 
>  drivers/mtd/devices/m25p80.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index fe9ceb7b5405..00094a668c62 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -131,6 +131,26 @@ static int m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>  	/* convert the dummy cycles to the number of bytes */
>  	dummy /= 8;
>  
> +	if (spi_flash_read_supported(spi)) {
> +		struct spi_flash_read_message msg;
> +		int ret;
> +
> +		msg.buf = buf;
> +		msg.from = from;
> +		msg.len = len;
> +		msg.read_opcode = nor->read_opcode;
> +		msg.addr_width = nor->addr_width;
> +		msg.dummy_bytes = dummy;
> +		/* TODO: Support other combinations */
> +		msg.opcode_nbits = SPI_NBITS_SINGLE;
> +		msg.addr_nbits = SPI_NBITS_SINGLE;
> +		msg.data_nbits = m25p80_rx_nbits(nor);

I wanted to let you know that the support of other SPI protocols has already
been implemented by this series:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/371170.html

patches 2 & 3 handle the probing of the (Q)SPI memory in spi_nor_scan() and
select the right protocols for register read/write, memory read, memory write,
and memory erase operations. The choice is done according to both the memory
and SPI controller capabilities.

patch 4 updates the m25p80 driver to take advantage of the bandwidth increase
allowed by QSPI protocols. For instance, the Atmel QSPI controller, like TI
one, maps the SPI NOR memory to the system bus. Yesterday I ran mtd_speedtest
to compare Fast Read 1-1-1 (0x0b) and Fast Read 1-1-4 (0x6b). The SPI clock
frequency was limited to 83MHz. The QSPI memory is a Micron n25q128a13.

I only had to change the mode argument of spi_nor_scan() from SPI_NOR_QUAD to
SPI_NOR_FAST in the atmel_quadspi driver (based on fsl-quadspi driver from
Freescale since Atmel's driver also uses the system bus mapping for memory
write operations) to force the spi-nor framework to choose the SPI 1-1-1
protocol instead of the SPI-1-1-4.

1 - Fast Read 1-1-1

mtd_speedtest: testing eraseblock write speed
mtd_speedtest: eraseblock read speed is 9319 KiB/s
[...]
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 6649 KiB/s
[...]
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 7757 KiB/s

2 - Fast Read 1-1-4

mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 30117 KiB/s
[...]
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 13096 KiB/s
[...]
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 18224 KiB/s

So the performance improvements are:
eraseblock read speed (65536 bytes) : +223%
page read speed (512 bytes)         :  +97%
2 page read speed (1024 bytes)      : +135%


That's why I believe that you could take advantage of these performance
improvements in the TI (Q)SPI controller driver.


Also please note that you may have to add in the struct spi_flash_read_message
two other fields for the number of option/mode cycles and their value.
Option/mode cycles are sent between the address and dummy cycles. They are
used by some memory manufacturers such as Spansion, Micron and Macronix to
enter/leave their continuous read mode.
So if uninitialized dummy cycles are interpreted by the QSPI memory as
option/mode cycles, depending on the actual value, the memory may enter by
mistake in continuous mode. Once in continuous mode, the command op code byte
must not be sent and is not expected by the memory: the memory implicitly uses
the read op code sent in the SPI message which triggered the memory to enter
continuous read mode. Next SPI messages start from the address cycles until
the right option/mode cycles are sent to leave the continuous read mode.

Currently the mtd layer doesn't use this feature but it should be aware of it.
That's why I believe we'll have to address this point in spi_nor_scan() and the
"regular" m25p80() sooner or later.

> +
> +		ret = spi_flash_read(spi, &msg);
> +		*retlen = msg.retlen;
> +		return ret;
> +	}
> +
>  	spi_message_init(&m);
>  	memset(t, 0, (sizeof t));
>  
> 

Best regards,

Cyrille
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html