[v7,3/4] mtd: spi-nor: introduce Double Transfer Rate (DTR) SPI protocols

Submitted by Cyrille Pitchen on April 18, 2017, 10:51 p.m.

Details

Message ID be585167c61f5c057041b2079d477e0106e9be64.1492554665.git.cyrille.pitchen@atmel.com
State Superseded
Headers show

Commit Message

Cyrille Pitchen April 18, 2017, 10:51 p.m.
This patch introduces support to Double Transfer Rate (DTR) SPI protocols.
DTR is used only for Fast Read operations.

According to manufacturer datasheets, whatever the number of I/O lines
used during instruction (x) and address/mode/dummy (y) clock cycles, DTR
is used only during data (z) clock cycles of SPI x-y-z protocols.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 10 +++++++++
 include/linux/mtd/spi-nor.h   | 48 +++++++++++++++++++++++++++++++++----------
 2 files changed, 47 insertions(+), 11 deletions(-)

Comments

Marek Vasut April 18, 2017, 11:05 p.m.
On 04/19/2017 12:51 AM, Cyrille Pitchen wrote:
> This patch introduces support to Double Transfer Rate (DTR) SPI protocols.
> DTR is used only for Fast Read operations.
> 
> According to manufacturer datasheets, whatever the number of I/O lines
> used during instruction (x) and address/mode/dummy (y) clock cycles, DTR
> is used only during data (z) clock cycles of SPI x-y-z protocols.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

[...]

> @@ -282,19 +305,22 @@ struct spi_nor_hwcaps {
>   * As a matter of performances, it is relevant to use Quad SPI protocols first,
>   * then Dual SPI protocols before Fast Read and lastly (Slow) Read.
>   */
> -#define SNOR_HWCAPS_READ_MASK		GENMASK(7, 0)
> +#define SNOR_HWCAPS_READ_MASK		GENMASK(10, 0)
>  #define SNOR_HWCAPS_READ		BIT(0)
>  #define SNOR_HWCAPS_READ_FAST		BIT(1)
> -
> -#define SNOR_HWCAPS_READ_DUAL		GENMASK(4, 2)
> -#define SNOR_HWCAPS_READ_1_1_2		BIT(2)
> -#define SNOR_HWCAPS_READ_1_2_2		BIT(3)
> -#define SNOR_HWCAPS_READ_2_2_2		BIT(4)
> -
> -#define SNOR_HWCAPS_READ_QUAD		GENMASK(7, 5)
> -#define SNOR_HWCAPS_READ_1_1_4		BIT(5)
> -#define SNOR_HWCAPS_READ_1_4_4		BIT(6)
> -#define SNOR_HWCAPS_READ_4_4_4		BIT(7)
> +#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
> +
> +#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
> +#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
> +#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
> +#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
> +#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
> +
> +#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
> +#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
> +#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
> +#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
> +#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)

I can't say I'm a big fan on this re-numeration when you add a new
entry. But unless you have a better idea, we'll have to live with this ...
Cyrille Pitchen April 19, 2017, 8:20 p.m.
Hi Marek,

Le 19/04/2017 à 01:05, Marek Vasut a écrit :
> On 04/19/2017 12:51 AM, Cyrille Pitchen wrote:
>> This patch introduces support to Double Transfer Rate (DTR) SPI protocols.
>> DTR is used only for Fast Read operations.
>>
>> According to manufacturer datasheets, whatever the number of I/O lines
>> used during instruction (x) and address/mode/dummy (y) clock cycles, DTR
>> is used only during data (z) clock cycles of SPI x-y-z protocols.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> 
> [...]
> 
>> @@ -282,19 +305,22 @@ struct spi_nor_hwcaps {
>>   * As a matter of performances, it is relevant to use Quad SPI protocols first,
>>   * then Dual SPI protocols before Fast Read and lastly (Slow) Read.
>>   */
>> -#define SNOR_HWCAPS_READ_MASK		GENMASK(7, 0)
>> +#define SNOR_HWCAPS_READ_MASK		GENMASK(10, 0)
>>  #define SNOR_HWCAPS_READ		BIT(0)
>>  #define SNOR_HWCAPS_READ_FAST		BIT(1)
>> -
>> -#define SNOR_HWCAPS_READ_DUAL		GENMASK(4, 2)
>> -#define SNOR_HWCAPS_READ_1_1_2		BIT(2)
>> -#define SNOR_HWCAPS_READ_1_2_2		BIT(3)
>> -#define SNOR_HWCAPS_READ_2_2_2		BIT(4)
>> -
>> -#define SNOR_HWCAPS_READ_QUAD		GENMASK(7, 5)
>> -#define SNOR_HWCAPS_READ_1_1_4		BIT(5)
>> -#define SNOR_HWCAPS_READ_1_4_4		BIT(6)
>> -#define SNOR_HWCAPS_READ_4_4_4		BIT(7)
>> +#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
>> +
>> +#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
>> +#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
>> +#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
>> +#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
>> +#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
>> +
>> +#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
>> +#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
>> +#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
>> +#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
>> +#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
> 
> I can't say I'm a big fan on this re-numeration when you add a new
> entry. But unless you have a better idea, we'll have to live with this ...
> 

Well, the other solution would be to reserve unused bit position in
patch 1 but would look odd too, wouldn't it?

As explained in the comments just above those definitions, the order of
the bits *does* matter. So maybe in the future, those bits would have to
be reordered again depending on the new features we would add then.

Thanks for your review!

Best regards,

Cyrille
Marek Vasut April 19, 2017, 9:35 p.m.
On 04/19/2017 10:20 PM, Cyrille Pitchen wrote:
> Hi Marek,
> 
> Le 19/04/2017 à 01:05, Marek Vasut a écrit :
>> On 04/19/2017 12:51 AM, Cyrille Pitchen wrote:
>>> This patch introduces support to Double Transfer Rate (DTR) SPI protocols.
>>> DTR is used only for Fast Read operations.
>>>
>>> According to manufacturer datasheets, whatever the number of I/O lines
>>> used during instruction (x) and address/mode/dummy (y) clock cycles, DTR
>>> is used only during data (z) clock cycles of SPI x-y-z protocols.
>>>
>>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>>
>> [...]
>>
>>> @@ -282,19 +305,22 @@ struct spi_nor_hwcaps {
>>>   * As a matter of performances, it is relevant to use Quad SPI protocols first,
>>>   * then Dual SPI protocols before Fast Read and lastly (Slow) Read.
>>>   */
>>> -#define SNOR_HWCAPS_READ_MASK		GENMASK(7, 0)
>>> +#define SNOR_HWCAPS_READ_MASK		GENMASK(10, 0)
>>>  #define SNOR_HWCAPS_READ		BIT(0)
>>>  #define SNOR_HWCAPS_READ_FAST		BIT(1)
>>> -
>>> -#define SNOR_HWCAPS_READ_DUAL		GENMASK(4, 2)
>>> -#define SNOR_HWCAPS_READ_1_1_2		BIT(2)
>>> -#define SNOR_HWCAPS_READ_1_2_2		BIT(3)
>>> -#define SNOR_HWCAPS_READ_2_2_2		BIT(4)
>>> -
>>> -#define SNOR_HWCAPS_READ_QUAD		GENMASK(7, 5)
>>> -#define SNOR_HWCAPS_READ_1_1_4		BIT(5)
>>> -#define SNOR_HWCAPS_READ_1_4_4		BIT(6)
>>> -#define SNOR_HWCAPS_READ_4_4_4		BIT(7)
>>> +#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
>>> +
>>> +#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
>>> +#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
>>> +#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
>>> +#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
>>> +#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
>>> +
>>> +#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
>>> +#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
>>> +#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
>>> +#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
>>> +#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
>>
>> I can't say I'm a big fan on this re-numeration when you add a new
>> entry. But unless you have a better idea, we'll have to live with this ...
>>
> 
> Well, the other solution would be to reserve unused bit position in
> patch 1 but would look odd too, wouldn't it?

Yeah, that's not super either ... I was pondering if there might be some
less error-prone way to autogenerate this maybe.

> As explained in the comments just above those definitions, the order of
> the bits *does* matter. So maybe in the future, those bits would have to
> be reordered again depending on the new features we would add then.
> 
> Thanks for your review!
> 
> Best regards,
> 
> Cyrille
>

Patch hide | download patch | download mbox

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 3421a42b4120..9f0956d56dc9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -203,6 +203,10 @@  static inline u8 spi_nor_convert_3to4_read(u8 opcode)
 		{ SPINOR_OP_READ_1_2_2,	SPINOR_OP_READ_1_2_2_4B },
 		{ SPINOR_OP_READ_1_1_4,	SPINOR_OP_READ_1_1_4_4B },
 		{ SPINOR_OP_READ_1_4_4,	SPINOR_OP_READ_1_4_4_4B },
+
+		{ SPINOR_OP_READ_1_1_1_DTR,	SPINOR_OP_READ_1_1_1_DTR_4B },
+		{ SPINOR_OP_READ_1_2_2_DTR,	SPINOR_OP_READ_1_2_2_DTR_4B },
+		{ SPINOR_OP_READ_1_4_4_DTR,	SPINOR_OP_READ_1_4_4_DTR_4B },
 	};
 
 	return spi_nor_convert_opcode(opcode, spi_nor_3to4_read,
@@ -1509,16 +1513,19 @@  struct spi_nor_pp_command {
 enum spi_nor_read_command_index {
 	SNOR_CMD_READ,
 	SNOR_CMD_READ_FAST,
+	SNOR_CMD_READ_1_1_1_DTR,
 
 	/* Dual SPI */
 	SNOR_CMD_READ_1_1_2,
 	SNOR_CMD_READ_1_2_2,
 	SNOR_CMD_READ_2_2_2,
+	SNOR_CMD_READ_1_2_2_DTR,
 
 	/* Quad SPI */
 	SNOR_CMD_READ_1_1_4,
 	SNOR_CMD_READ_1_4_4,
 	SNOR_CMD_READ_4_4_4,
+	SNOR_CMD_READ_1_4_4_DTR,
 
 	SNOR_CMD_READ_MAX
 };
@@ -1646,12 +1653,15 @@  static int spi_nor_hwcaps_read2cmd(u32 hwcaps)
 	static const int hwcaps_read2cmd[][2] = {
 		{ SNOR_HWCAPS_READ,		SNOR_CMD_READ },
 		{ SNOR_HWCAPS_READ_FAST,	SNOR_CMD_READ_FAST },
+		{ SNOR_HWCAPS_READ_1_1_1_DTR,	SNOR_CMD_READ_1_1_1_DTR },
 		{ SNOR_HWCAPS_READ_1_1_2,	SNOR_CMD_READ_1_1_2 },
 		{ SNOR_HWCAPS_READ_1_2_2,	SNOR_CMD_READ_1_2_2 },
 		{ SNOR_HWCAPS_READ_2_2_2,	SNOR_CMD_READ_2_2_2 },
+		{ SNOR_HWCAPS_READ_1_2_2_DTR,	SNOR_CMD_READ_1_2_2_DTR },
 		{ SNOR_HWCAPS_READ_1_1_4,	SNOR_CMD_READ_1_1_4 },
 		{ SNOR_HWCAPS_READ_1_4_4,	SNOR_CMD_READ_1_4_4 },
 		{ SNOR_HWCAPS_READ_4_4_4,	SNOR_CMD_READ_4_4_4 },
+		{ SNOR_HWCAPS_READ_1_4_4_DTR,	SNOR_CMD_READ_1_4_4_DTR },
 	};
 
 	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index f5e0db94e545..5ed2872fe61a 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -73,6 +73,15 @@ 
 #define SPINOR_OP_BE_32K_4B	0x5c	/* Erase 32KiB block */
 #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
 
+/* Double Transfer Rate opcodes - defined in JEDEC JESD216B. */
+#define SPINOR_OP_READ_1_1_1_DTR	0x0d
+#define SPINOR_OP_READ_1_2_2_DTR	0xbd
+#define SPINOR_OP_READ_1_4_4_DTR	0xed
+
+#define SPINOR_OP_READ_1_1_1_DTR_4B	0x0e
+#define SPINOR_OP_READ_1_2_2_DTR_4B	0xbe
+#define SPINOR_OP_READ_1_4_4_DTR_4B	0xee
+
 /* Used for SST flashes only. */
 #define SPINOR_OP_BP		0x02	/* Byte program */
 #define SPINOR_OP_WRDI		0x04	/* Write disable */
@@ -135,10 +144,15 @@ 
 #define SNOR_PROTO_DATA(_nbits)	\
 	((((u32)(_nbits)) << SNOR_PROTO_DATA_SHIFT) & SNOR_PROTO_DATA_MASK)
 
+#define SNOR_PROTO_IS_DTR	BIT(24)	/* Double Transfer Rate */
+
 #define SNOR_PROTO_STR(_inst_nbits, _addr_nbits, _data_nbits)	\
 	(SNOR_PROTO_INST(_inst_nbits) |				\
 	 SNOR_PROTO_ADDR(_addr_nbits) |				\
 	 SNOR_PROTO_DATA(_data_nbits))
+#define SNOR_PROTO_DTR(_inst_nbits, _addr_nbits, _data_nbits)	\
+	(SNOR_PROTO_IS_DTR |					\
+	 SNOR_PROTO_STR(_inst_nbits, _addr_nbits, _data_nbits))
 
 enum spi_nor_protocol {
 	SNOR_PROTO_1_1_1 = SNOR_PROTO_STR(1, 1, 1),
@@ -148,8 +162,17 @@  enum spi_nor_protocol {
 	SNOR_PROTO_1_4_4 = SNOR_PROTO_STR(1, 4, 4),
 	SNOR_PROTO_2_2_2 = SNOR_PROTO_STR(2, 2, 2),
 	SNOR_PROTO_4_4_4 = SNOR_PROTO_STR(4, 4, 4),
+
+	SNOR_PROTO_1_1_1_DTR = SNOR_PROTO_DTR(1, 1, 1),
+	SNOR_PROTO_1_2_2_DTR = SNOR_PROTO_DTR(1, 2, 2),
+	SNOR_PROTO_1_4_4_DTR = SNOR_PROTO_DTR(1, 4, 4),
 };
 
+static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
+{
+	return (proto & SNOR_PROTO_IS_DTR) == SNOR_PROTO_IS_DTR;
+}
+
 static inline u8 spi_nor_get_protocol_inst_nbits(enum spi_nor_protocol proto)
 {
 	return ((u32)(proto & SNOR_PROTO_INST_MASK)) >> SNOR_PROTO_INST_SHIFT;
@@ -282,19 +305,22 @@  struct spi_nor_hwcaps {
  * As a matter of performances, it is relevant to use Quad SPI protocols first,
  * then Dual SPI protocols before Fast Read and lastly (Slow) Read.
  */
-#define SNOR_HWCAPS_READ_MASK		GENMASK(7, 0)
+#define SNOR_HWCAPS_READ_MASK		GENMASK(10, 0)
 #define SNOR_HWCAPS_READ		BIT(0)
 #define SNOR_HWCAPS_READ_FAST		BIT(1)
-
-#define SNOR_HWCAPS_READ_DUAL		GENMASK(4, 2)
-#define SNOR_HWCAPS_READ_1_1_2		BIT(2)
-#define SNOR_HWCAPS_READ_1_2_2		BIT(3)
-#define SNOR_HWCAPS_READ_2_2_2		BIT(4)
-
-#define SNOR_HWCAPS_READ_QUAD		GENMASK(7, 5)
-#define SNOR_HWCAPS_READ_1_1_4		BIT(5)
-#define SNOR_HWCAPS_READ_1_4_4		BIT(6)
-#define SNOR_HWCAPS_READ_4_4_4		BIT(7)
+#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
+
+#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
+#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
+#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
+#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
+#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
+
+#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
+#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
+#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
+#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
+#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
 
 /*
  * Page Program capabilities.