[v4,3/5] mtd: devices: m25p80: add support for mmap read request
diff mbox

Message ID 1448860515-28336-4-git-send-email-vigneshr@ti.com
State New
Headers show

Commit Message

Vignesh Raghavendra Nov. 30, 2015, 5:15 a.m. UTC
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@ti.com>
---

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(+)

Comments

Cyrille Pitchen Dec. 3, 2015, 9:42 a.m. UTC | #1
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@ti.com>
> ---
> 
> 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-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vignesh Raghavendra Dec. 3, 2015, 11:23 a.m. UTC | #2
Hi,

On 12/03/2015 03:12 PM, Cyrille Pitchen wrote:
> 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@ti.com>
>> ---
>>
>> 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.
> 

Well, I based my patches on linux-next as per Brian's suggestion. If
patches to support other flash protocol modes are accepted, I would be
happy to rebase and make use of other modes. It would just be the matter
of populating msg.{opcode/addr}_n_bits correctly.


> 
> 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.
> 

Above feature can be added incrementally over this series, (as and when
m25p80 is updated) moreover ti-qspi controller does not support
option/mode cycles in memory-mapped mode, so I wont be able to test this
feature anyway.

Patch
diff mbox

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);
+
+		ret = spi_flash_read(spi, &msg);
+		*retlen = msg.retlen;
+		return ret;
+	}
+
 	spi_message_init(&m);
 	memset(t, 0, (sizeof t));