diff mbox series

[1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba

Message ID 20181003165603.2579-2-vigneshr@ti.com
State Superseded
Headers show
Series spi-nor: Add Octal SPI support | expand

Commit Message

Raghavendra, Vignesh Oct. 3, 2018, 4:56 p.m. UTC 
Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It
supports read/write over 8 IO lines simulatenously. Add support for
Octal read mode for Micron mt35xu512aba.
Unfortunately, this flash is only complaint to SFDP JESD216B and does not
seem to support newer JESD216C standard that provides auto detection of
Octal mode capabilities and opcodes. Therefore, this capability is
manually added using new SPI_NOR_OCTAL_READ flag.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
 include/linux/mtd/spi-nor.h   |  2 ++
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Yogesh Narayan Gaur Oct. 4, 2018, 6:51 a.m. UTC | #1 
Hi Vignesh,

> -----Original Message-----
> From: Vignesh R [mailto:vigneshr@ti.com]
> Sent: Wednesday, October 3, 2018 10:26 PM
> To: Boris Brezillon <boris.brezillon@bootlin.com>; Marek Vasut
> <marek.vasut@gmail.com>; Rob Herring <robh+dt@kernel.org>
> Cc: Brian Norris <computersforpeace@gmail.com>; Yogesh Narayan Gaur
> <yogeshnarayan.gaur@nxp.com>; Linux ARM Mailing List <linux-arm-
> kernel@lists.infradead.org>; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Vignesh R
> <vigneshr@ti.com>
> Subject: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba
> 
> Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It supports
> read/write over 8 IO lines simulatenously. Add support for Octal read mode for
> Micron mt35xu512aba.
> Unfortunately, this flash is only complaint to SFDP JESD216B and does not seem
> to support newer JESD216C standard that provides auto detection of Octal
> mode capabilities and opcodes. Therefore, this capability is manually added
> using new SPI_NOR_OCTAL_READ flag.
> 

Thanks for sending the patch-set of adding octal support.
If possible, can you share the MT35x datasheet?

I also have the patch ready in which I have added support for Read (1-1-8 and 1-8-8) protocol and Write (1-1-8 and 1-8-8).
Also have added support of Octal in driver/spi/spi.c framework.

IMO, we would collaborate our patches.
--
Regards
Yogesh Gaur

> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
>  include/linux/mtd/spi-nor.h   |  2 ++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index
> aff5e6ff0b2c..4926e805a8cb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -90,6 +90,7 @@ struct flash_info {
>  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip
> erase */
>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>  #define USE_CLSR		BIT(14)	/* use CLSR command */
> +#define SPI_NOR_OCTAL_READ	BIT(15)	/* Flash supports Octal Read */
> 
>  	int	(*quad_enable)(struct spi_nor *nor);
>  };
> @@ -209,6 +210,7 @@ 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_8,	SPINOR_OP_READ_1_1_8_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 },
> @@ -1406,7 +1408,7 @@ static const struct flash_info spi_nor_ids[] = {
>  	{ "mt25qu02g",   INFO(0x20bb22, 0, 64 * 1024, 4096, SECT_4K |
> USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
> 
>  	/* Micron */
> -	{ "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K |
> USE_FSR | SPI_NOR_4B_OPCODES) },
> +	{ "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K |
> USE_FSR
> +| SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
> 
>  	/* PMC */
>  	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
> @@ -3199,6 +3201,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  					  SNOR_PROTO_1_1_4);
>  	}
> 
> +	if (info->flags & SPI_NOR_OCTAL_READ) {
> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> +		spi_nor_set_read_settings(&params-
> >reads[SNOR_CMD_READ_1_1_8],
> +					  0, 8, SPINOR_OP_READ_1_1_8,
> +					  SNOR_PROTO_1_1_8);
> +	}
> +
>  	/* Page Program settings. */
>  	params->hwcaps.mask |= SNOR_HWCAPS_PP;
>  	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index
> 8b1acf68b7ac..ae9861ed7e0f 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -50,6 +50,7 @@
>  #define SPINOR_OP_READ_1_2_2	0xbb	/* Read data bytes (Dual I/O
> SPI) */
>  #define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad
> Output SPI) */
>  #define SPINOR_OP_READ_1_4_4	0xeb	/* Read data bytes (Quad I/O
> SPI) */
> +#define SPINOR_OP_READ_1_1_8	0x8b	/* Read data bytes (Octal
> Output SPI) */
>  #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
>  #define SPINOR_OP_PP_1_1_4	0x32	/* Quad page program */
>  #define SPINOR_OP_PP_1_4_4	0x38	/* Quad page program */
> @@ -73,6 +74,7 @@
>  #define SPINOR_OP_READ_1_2_2_4B	0xbc	/* Read data bytes (Dual I/O
> SPI) */
>  #define SPINOR_OP_READ_1_1_4_4B	0x6c	/* Read data bytes (Quad
> Output SPI) */
>  #define SPINOR_OP_READ_1_4_4_4B	0xec	/* Read data bytes (Quad I/O
> SPI) */
> +#define SPINOR_OP_READ_1_1_8_4B	0x7c	/* Read data bytes (Octal
> Output SPI) */
>  #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256
> bytes) */
>  #define SPINOR_OP_PP_1_1_4_4B	0x34	/* Quad page program */
>  #define SPINOR_OP_PP_1_4_4_4B	0x3e	/* Quad page program */
> --
> 2.19.0
Boris Brezillon Oct. 4, 2018, 7:39 a.m. UTC | #2 
+Julien, Zhengxunli and Mason from Macronix

Hi Yogesh,

On Thu, 4 Oct 2018 06:51:41 +0000
Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:

> Hi Vignesh,
> 
> > -----Original Message-----
> > From: Vignesh R [mailto:vigneshr@ti.com]
> > Sent: Wednesday, October 3, 2018 10:26 PM
> > To: Boris Brezillon <boris.brezillon@bootlin.com>; Marek Vasut
> > <marek.vasut@gmail.com>; Rob Herring <robh+dt@kernel.org>
> > Cc: Brian Norris <computersforpeace@gmail.com>; Yogesh Narayan Gaur
> > <yogeshnarayan.gaur@nxp.com>; Linux ARM Mailing List <linux-arm-  
> > kernel@lists.infradead.org>; linux-mtd@lists.infradead.org;  
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Vignesh R
> > <vigneshr@ti.com>
> > Subject: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba
> > 
> > Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It supports
> > read/write over 8 IO lines simulatenously. Add support for Octal read mode for
> > Micron mt35xu512aba.
> > Unfortunately, this flash is only complaint to SFDP JESD216B and does not seem
> > to support newer JESD216C standard that provides auto detection of Octal
> > mode capabilities and opcodes. Therefore, this capability is manually added
> > using new SPI_NOR_OCTAL_READ flag.
> >   
> 
> Thanks for sending the patch-set of adding octal support.
> If possible, can you share the MT35x datasheet?
> 
> I also have the patch ready in which I have added support for Read (1-1-8 and 1-8-8) protocol and Write (1-1-8 and 1-8-8).
> Also have added support of Octal in driver/spi/spi.c framework.
> 
> IMO, we would collaborate our patches.

Looks like we are of stepping on each others toes here (see this branch
[1]). I guess it's not a problem if we agree on who is working on what.

Yogesh, you already sent "spi: add flags for octal I/O data
transfer" [3] which is only adding the new OCTAL flags but is not
patching spi.c and spi-mem.c to take those new flags into account. Here
is my version of this patch [2] (it's still missing an update of
SPI_MEM_MAX_BUSWIDTH). Let me know what you want to do (rework your
version to address the problem or take mine).

Regarding other patches in [2], they're mainly here to add support for
X-X-X and DTR modes and get the m25p80 logic integrated in spi-nor.c so
that we can really check which NOR operations are supported by the SPI
controller.

Regards,

Boris

[1]https://github.com/bbrezillon/linux/commits/spi-nor/octo
[2]https://github.com/bbrezillon/linux/commit/9854a8fdd23f64e79859fd07a71d4a1c57b812f2
[3]https://patchwork.ozlabs.org/patch/894916/
Yogesh Narayan Gaur Oct. 4, 2018, 8:47 a.m. UTC | #3 
Hi Boris,

> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> Sent: Thursday, October 4, 2018 1:09 PM
> To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
> Cc: Vignesh R <vigneshr@ti.com>; Marek Vasut <marek.vasut@gmail.com>; Rob
> Herring <robh+dt@kernel.org>; Brian Norris <computersforpeace@gmail.com>;
> Linux ARM Mailing List <linux-arm-kernel@lists.infradead.org>; linux-
> mtd@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; juliensu@mxic.com.tw; zhengxunli@mxic.com.tw;
> masonccyang@mxic.com.tw
> Subject: Re: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for
> mt35xu512aba
> 
> +Julien, Zhengxunli and Mason from Macronix
> 
> Hi Yogesh,
> 
> On Thu, 4 Oct 2018 06:51:41 +0000
> Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> 
> > Hi Vignesh,
> >
> > > -----Original Message-----
> > > From: Vignesh R [mailto:vigneshr@ti.com]
> > > Sent: Wednesday, October 3, 2018 10:26 PM
> > > To: Boris Brezillon <boris.brezillon@bootlin.com>; Marek Vasut
> > > <marek.vasut@gmail.com>; Rob Herring <robh+dt@kernel.org>
> > > Cc: Brian Norris <computersforpeace@gmail.com>; Yogesh Narayan Gaur
> > > <yogeshnarayan.gaur@nxp.com>; Linux ARM Mailing List <linux-arm-
> > > kernel@lists.infradead.org>; linux-mtd@lists.infradead.org;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Vignesh R
> > > <vigneshr@ti.com>
> > > Subject: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for
> > > mt35xu512aba
> > >
> > > Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines.
> > > It supports read/write over 8 IO lines simulatenously. Add support
> > > for Octal read mode for Micron mt35xu512aba.
> > > Unfortunately, this flash is only complaint to SFDP JESD216B and
> > > does not seem to support newer JESD216C standard that provides auto
> > > detection of Octal mode capabilities and opcodes. Therefore, this
> > > capability is manually added using new SPI_NOR_OCTAL_READ flag.
> > >
> >
> > Thanks for sending the patch-set of adding octal support.
> > If possible, can you share the MT35x datasheet?
> >
> > I also have the patch ready in which I have added support for Read (1-1-8 and
> 1-8-8) protocol and Write (1-1-8 and 1-8-8).
> > Also have added support of Octal in driver/spi/spi.c framework.
> >
> > IMO, we would collaborate our patches.
> 
> Looks like we are of stepping on each others toes here (see this branch [1]). I
> guess it's not a problem if we agree on who is working on what.
> 
> Yogesh, you already sent "spi: add flags for octal I/O data transfer" [3] which is
> only adding the new OCTAL flags but is not patching spi.c and spi-mem.c to take
> those new flags into account. Here is my version of this patch [2] (it's still
> missing an update of SPI_MEM_MAX_BUSWIDTH). Let me know what you want
> to do (rework your version to address the problem or take mine).
> 
I haven't said that I have sent the patches. My patches are ready and needed to be sent for review.
In these patches,  I am adding support for X-X-X protocol for octal support and integrating them with spi  and flash m25p80 interface.

Would send the patches by today evening for review.

Regards
Yogesh Gaur

> Regarding other patches in [2], they're mainly here to add support for X-X-X and
> DTR modes and get the m25p80 logic integrated in spi-nor.c so that we can
> really check which NOR operations are supported by the SPI controller.
> 
> Regards,
> 
> Boris
> 
> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fbbrezillon%2Flinux%2Fcommits%2Fspi-
> nor%2Focto&amp;data=02%7C01%7Cyogeshnarayan.gaur%40nxp.com%7C3ad
> 4b6f2a4c9470e98f308d629cc8362%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C636742355682081158&amp;sdata=vXg7nwZ6dLrUxoNw9t41GYMp
> oPdNWUWhLM6wZPmrIec%3D&amp;reserved=0
> [2]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fbbrezillon%2Flinux%2Fcommit%2F9854a8fdd23f64e79859fd07a71d4
> a1c57b812f2&amp;data=02%7C01%7Cyogeshnarayan.gaur%40nxp.com%7C3ad
> 4b6f2a4c9470e98f308d629cc8362%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C636742355682081158&amp;sdata=yEZqGYprHS8q2wuisFLpHg8bMj
> uuWDrl3ggWlA8LRoc%3D&amp;reserved=0
> [3]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> chwork.ozlabs.org%2Fpatch%2F894916%2F&amp;data=02%7C01%7Cyogeshnar
> ayan.gaur%40nxp.com%7C3ad4b6f2a4c9470e98f308d629cc8362%7C686ea1d3
> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636742355682081158&amp;sdata=t
> 5DFvXgDOlyLn7pMieue9BB5q24DNiJMywGvNrWAYOo%3D&amp;reserved=0
Boris Brezillon Oct. 4, 2018, 9:10 a.m. UTC | #4 
On Thu, 4 Oct 2018 08:47:33 +0000
Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:

> > 
> > Yogesh, you already sent "spi: add flags for octal I/O data
> > transfer" [3] which is only adding the new OCTAL flags but is not
> > patching spi.c and spi-mem.c to take those new flags into account.
> > Here is my version of this patch [2] (it's still missing an update
> > of SPI_MEM_MAX_BUSWIDTH). Let me know what you want to do (rework
> > your version to address the problem or take mine). 
> I haven't said that I have sent the patches.

Well, you at least sent one of them, I know it because I was in
CC :P.

> My patches are ready and needed to be sent for review.

I initially told you to drop those patches because we didn't have a
user yet, that's about to change, so I guess we're all good.

> In these patches,  I am adding support for X-X-X protocol for octal
> support and integrating them with spi  and flash m25p80 interface.

From what I've seen you're not adding support of 8-8-8 but for 1-8-8 an
1-1-8, which is a lot easier to deal with (it's completely stateless,
which is what the spi-nor expects). Unless you're talking about other
patches you haven't sent yet...
Boris Brezillon Oct. 4, 2018, 9:45 a.m. UTC | #5 
On Wed, 3 Oct 2018 22:26:01 +0530
Vignesh R <vigneshr@ti.com> wrote:

> Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It
> supports read/write over 8 IO lines simulatenously. Add support for
> Octal read mode for Micron mt35xu512aba.
> Unfortunately, this flash is only complaint to SFDP JESD216B and does not
> seem to support newer JESD216C standard that provides auto detection of
> Octal mode capabilities and opcodes. Therefore, this capability is
> manually added using new SPI_NOR_OCTAL_READ flag.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
>  include/linux/mtd/spi-nor.h   |  2 ++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index aff5e6ff0b2c..4926e805a8cb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -90,6 +90,7 @@ struct flash_info {
>  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip erase */
>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>  #define USE_CLSR		BIT(14)	/* use CLSR command */
> +#define SPI_NOR_OCTAL_READ	BIT(15)	/* Flash supports Octal Read */

Hm, we'll need to clarify what OCTAL means. I see at least 3 different
modes using 8 IO lines (1-1-8, 1-8-8 and 8-8-8) and all of them could
be qualified as "octal" modes.
So how about renaming this macro SPI_NOR_1_1_8_READ.

Also, I fear we'll soon run out of bits in ->flags if we keep adding
one flag per mode which is why I proposed a solution to let flash
chips tweak the flash parameters as they wish [1][2]. I'm not saying we
should do it now, but we should definitely plan for something like that.

[1]https://github.com/bbrezillon/linux/commit/9c672e4c85a91f1b0803c9c6e4b8f3aae5d79ffb
[2]https://github.com/bbrezillon/linux/commit/3a5515c8821314c06a3d84f9861aefe476bb711e
Raghavendra, Vignesh Oct. 4, 2018, 10:38 a.m. UTC | #6 
Hi,

On Thursday 04 October 2018 12:21 PM, Yogesh Narayan Gaur wrote:
> Hi Vignesh,
> 
>> -----Original Message-----
>> From: Vignesh R [mailto:vigneshr@ti.com]
>> Sent: Wednesday, October 3, 2018 10:26 PM
>> To: Boris Brezillon <boris.brezillon@bootlin.com>; Marek Vasut
>> <marek.vasut@gmail.com>; Rob Herring <robh+dt@kernel.org>
>> Cc: Brian Norris <computersforpeace@gmail.com>; Yogesh Narayan Gaur
>> <yogeshnarayan.gaur@nxp.com>; Linux ARM Mailing List <linux-arm-
>> kernel@lists.infradead.org>; linux-mtd@lists.infradead.org;
>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Vignesh R
>> <vigneshr@ti.com>
>> Subject: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba
>>
>> Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It supports
>> read/write over 8 IO lines simulatenously. Add support for Octal read mode for
>> Micron mt35xu512aba.
>> Unfortunately, this flash is only complaint to SFDP JESD216B and does not seem
>> to support newer JESD216C standard that provides auto detection of Octal
>> mode capabilities and opcodes. Therefore, this capability is manually added
>> using new SPI_NOR_OCTAL_READ flag.
>>
> 
> Thanks for sending the patch-set of adding octal support.
> If possible, can you share the MT35x datasheet?
> 

I dont have datasheet, this patch is based on prior patches from here:
http://patchwork.ozlabs.org/patch/740942/

> I also have the patch ready in which I have added support for Read (1-1-8 and 1-8-8) protocol and Write (1-1-8 and 1-8-8).
> Also have added support of Octal in driver/spi/spi.c framework.
> 
> IMO, we would collaborate our patches.

That would be great, thanks!

Regards
Vignesh

> --
> Regards
> Yogesh Gaur
> 
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
>>  include/linux/mtd/spi-nor.h   |  2 ++
>>  2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index
>> aff5e6ff0b2c..4926e805a8cb 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -90,6 +90,7 @@ struct flash_info {
>>  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip
>> erase */
>>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>>  #define USE_CLSR		BIT(14)	/* use CLSR command */
>> +#define SPI_NOR_OCTAL_READ	BIT(15)	/* Flash supports Octal Read */
>>
>>  	int	(*quad_enable)(struct spi_nor *nor);
>>  };
>> @@ -209,6 +210,7 @@ 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_8,	SPINOR_OP_READ_1_1_8_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 },
>> @@ -1406,7 +1408,7 @@ static const struct flash_info spi_nor_ids[] = {
>>  	{ "mt25qu02g",   INFO(0x20bb22, 0, 64 * 1024, 4096, SECT_4K |
>> USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
>>
>>  	/* Micron */
>> -	{ "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K |
>> USE_FSR | SPI_NOR_4B_OPCODES) },
>> +	{ "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K |
>> USE_FSR
>> +| SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
>>
>>  	/* PMC */
>>  	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
>> @@ -3199,6 +3201,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
>>  					  SNOR_PROTO_1_1_4);
>>  	}
>>
>> +	if (info->flags & SPI_NOR_OCTAL_READ) {
>> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
>> +		spi_nor_set_read_settings(&params-
>>> reads[SNOR_CMD_READ_1_1_8],
>> +					  0, 8, SPINOR_OP_READ_1_1_8,
>> +					  SNOR_PROTO_1_1_8);
>> +	}
>> +
>>  	/* Page Program settings. */
>>  	params->hwcaps.mask |= SNOR_HWCAPS_PP;
>>  	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index
>> 8b1acf68b7ac..ae9861ed7e0f 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -50,6 +50,7 @@
>>  #define SPINOR_OP_READ_1_2_2	0xbb	/* Read data bytes (Dual I/O
>> SPI) */
>>  #define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad
>> Output SPI) */
>>  #define SPINOR_OP_READ_1_4_4	0xeb	/* Read data bytes (Quad I/O
>> SPI) */
>> +#define SPINOR_OP_READ_1_1_8	0x8b	/* Read data bytes (Octal
>> Output SPI) */
>>  #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
>>  #define SPINOR_OP_PP_1_1_4	0x32	/* Quad page program */
>>  #define SPINOR_OP_PP_1_4_4	0x38	/* Quad page program */
>> @@ -73,6 +74,7 @@
>>  #define SPINOR_OP_READ_1_2_2_4B	0xbc	/* Read data bytes (Dual I/O
>> SPI) */
>>  #define SPINOR_OP_READ_1_1_4_4B	0x6c	/* Read data bytes (Quad
>> Output SPI) */
>>  #define SPINOR_OP_READ_1_4_4_4B	0xec	/* Read data bytes (Quad I/O
>> SPI) */
>> +#define SPINOR_OP_READ_1_1_8_4B	0x7c	/* Read data bytes (Octal
>> Output SPI) */
>>  #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256
>> bytes) */
>>  #define SPINOR_OP_PP_1_1_4_4B	0x34	/* Quad page program */
>>  #define SPINOR_OP_PP_1_4_4_4B	0x3e	/* Quad page program */
>> --
>> 2.19.0
>
Raghavendra, Vignesh Oct. 4, 2018, 11:12 a.m. UTC | #7 
On Thursday 04 October 2018 03:15 PM, Boris Brezillon wrote:
> On Wed, 3 Oct 2018 22:26:01 +0530
> Vignesh R <vigneshr@ti.com> wrote:
> 
>> Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It
>> supports read/write over 8 IO lines simulatenously. Add support for
>> Octal read mode for Micron mt35xu512aba.
>> Unfortunately, this flash is only complaint to SFDP JESD216B and does not
>> seem to support newer JESD216C standard that provides auto detection of
>> Octal mode capabilities and opcodes. Therefore, this capability is
>> manually added using new SPI_NOR_OCTAL_READ flag.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
>>  include/linux/mtd/spi-nor.h   |  2 ++
>>  2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index aff5e6ff0b2c..4926e805a8cb 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -90,6 +90,7 @@ struct flash_info {
>>  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip erase */
>>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>>  #define USE_CLSR		BIT(14)	/* use CLSR command */
>> +#define SPI_NOR_OCTAL_READ	BIT(15)	/* Flash supports Octal Read */
> 
> Hm, we'll need to clarify what OCTAL means. I see at least 3 different
> modes using 8 IO lines (1-1-8, 1-8-8 and 8-8-8) and all of them could
> be qualified as "octal" modes.
> So how about renaming this macro SPI_NOR_1_1_8_READ.
> 

My understanding is that, if a flash is Octal IO capable, then it
supports all mode of Octal IO (1-1-8, 1-8-8 and 8-8-8). At least in the
current code, I see SPI_NOR_QUAD_READ seems to imply 1-1-4, 1-4-4 and
4-4-4 modes.
Moreover, these capabilities should be auto discoverable once flash
start populating SFDP JESD216C tables (IIRC, JESD216B already takes care
of QUAD IO modes). So, I don't think we need different flags for 1-1-8,
1-8-8 and 8-8-8. Please let me if that's not the case.

For this patch, I wanted to start small and support only 1-1-8 mode
which is the easiest.


> Also, I fear we'll soon run out of bits in ->flags if we keep adding
> one flag per mode which is why I proposed a solution to let flash
> chips tweak the flash parameters as they wish [1][2]. I'm not saying we
> should do it now, but we should definitely plan for something like that.
> 
> [1]https://github.com/bbrezillon/linux/commit/9c672e4c85a91f1b0803c9c6e4b8f3aae5d79ffb
> [2]https://github.com/bbrezillon/linux/commit/3a5515c8821314c06a3d84f9861aefe476bb711e
> 

Hmm, it almost seems like Macronix flash warrants its own driver now.
Boris Brezillon Oct. 4, 2018, 11:27 a.m. UTC | #8 
On Thu, 4 Oct 2018 16:42:22 +0530
Vignesh R <vigneshr@ti.com> wrote:

> On Thursday 04 October 2018 03:15 PM, Boris Brezillon wrote:
> > On Wed, 3 Oct 2018 22:26:01 +0530
> > Vignesh R <vigneshr@ti.com> wrote:
> >   
> >> Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It
> >> supports read/write over 8 IO lines simulatenously. Add support for
> >> Octal read mode for Micron mt35xu512aba.
> >> Unfortunately, this flash is only complaint to SFDP JESD216B and does not
> >> seem to support newer JESD216C standard that provides auto detection of
> >> Octal mode capabilities and opcodes. Therefore, this capability is
> >> manually added using new SPI_NOR_OCTAL_READ flag.
> >>
> >> Signed-off-by: Vignesh R <vigneshr@ti.com>
> >> ---
> >>  drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
> >>  include/linux/mtd/spi-nor.h   |  2 ++
> >>  2 files changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> >> index aff5e6ff0b2c..4926e805a8cb 100644
> >> --- a/drivers/mtd/spi-nor/spi-nor.c
> >> +++ b/drivers/mtd/spi-nor/spi-nor.c
> >> @@ -90,6 +90,7 @@ struct flash_info {
> >>  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip erase */
> >>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
> >>  #define USE_CLSR		BIT(14)	/* use CLSR command */
> >> +#define SPI_NOR_OCTAL_READ	BIT(15)	/* Flash supports Octal Read */  
> > 
> > Hm, we'll need to clarify what OCTAL means. I see at least 3 different
> > modes using 8 IO lines (1-1-8, 1-8-8 and 8-8-8) and all of them could
> > be qualified as "octal" modes.
> > So how about renaming this macro SPI_NOR_1_1_8_READ.
> >   
> 
> My understanding is that, if a flash is Octal IO capable, then it
> supports all mode of Octal IO (1-1-8, 1-8-8 and 8-8-8). At least in the
> current code, I see SPI_NOR_QUAD_READ seems to imply 1-1-4, 1-4-4 and
> 4-4-4 modes.

I already have one NOR which is not following this rule (mx25uw51245g)
and is only supporting 1-1-1 and 8-8-8, nothing in between.

> Moreover, these capabilities should be auto discoverable once flash
> start populating SFDP JESD216C tables (IIRC, JESD216B already takes care
> of QUAD IO modes). So, I don't think we need different flags for 1-1-8,
> 1-8-8 and 8-8-8. Please let me if that's not the case.

Except we already have NORs out there that do not have a valid SFDP
table but still support a subset of the octal modes (mx25uw51245g is in
this case, but I'm pretty sure there are others).

> 
> For this patch, I wanted to start small and support only 1-1-8 mode
> which is the easiest.

Which is fine, just wanted to know what SPI_NOR_OCTAL_READ meant.

> 
> 
> > Also, I fear we'll soon run out of bits in ->flags if we keep adding
> > one flag per mode which is why I proposed a solution to let flash
> > chips tweak the flash parameters as they wish [1][2]. I'm not saying we
> > should do it now, but we should definitely plan for something like that.
> > 
> > [1]https://github.com/bbrezillon/linux/commit/9c672e4c85a91f1b0803c9c6e4b8f3aae5d79ffb
> > [2]https://github.com/bbrezillon/linux/commit/3a5515c8821314c06a3d84f9861aefe476bb711e
> >   
> 
> Hmm, it almost seems like Macronix flash warrants its own driver now.
> 

Actually, we already have quite a bit of code that is vendor (and even
chip) specific in spi-nor.c, and I indeed think it would be beneficial
to have one driver per manufacturer so that the core is not polluted by
those chip/manufacturer specific hacks. The thing is, the spi_nor
interface (and the way hwcaps selection works) is not ready for that.
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index aff5e6ff0b2c..4926e805a8cb 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -90,6 +90,7 @@  struct flash_info {
 #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip erase */
 #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
 #define USE_CLSR		BIT(14)	/* use CLSR command */
+#define SPI_NOR_OCTAL_READ	BIT(15)	/* Flash supports Octal Read */
 
 	int	(*quad_enable)(struct spi_nor *nor);
 };
@@ -209,6 +210,7 @@  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_8,	SPINOR_OP_READ_1_1_8_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 },
@@ -1406,7 +1408,7 @@  static const struct flash_info spi_nor_ids[] = {
 	{ "mt25qu02g",   INFO(0x20bb22, 0, 64 * 1024, 4096, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
 
 	/* Micron */
-	{ "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K | USE_FSR | SPI_NOR_4B_OPCODES) },
+	{ "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
 
 	/* PMC */
 	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
@@ -3199,6 +3201,13 @@  static int spi_nor_init_params(struct spi_nor *nor,
 					  SNOR_PROTO_1_1_4);
 	}
 
+	if (info->flags & SPI_NOR_OCTAL_READ) {
+		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
+		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_8],
+					  0, 8, SPINOR_OP_READ_1_1_8,
+					  SNOR_PROTO_1_1_8);
+	}
+
 	/* Page Program settings. */
 	params->hwcaps.mask |= SNOR_HWCAPS_PP;
 	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 8b1acf68b7ac..ae9861ed7e0f 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -50,6 +50,7 @@ 
 #define SPINOR_OP_READ_1_2_2	0xbb	/* Read data bytes (Dual I/O SPI) */
 #define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad Output SPI) */
 #define SPINOR_OP_READ_1_4_4	0xeb	/* Read data bytes (Quad I/O SPI) */
+#define SPINOR_OP_READ_1_1_8	0x8b	/* Read data bytes (Octal Output SPI) */
 #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_PP_1_1_4	0x32	/* Quad page program */
 #define SPINOR_OP_PP_1_4_4	0x38	/* Quad page program */
@@ -73,6 +74,7 @@ 
 #define SPINOR_OP_READ_1_2_2_4B	0xbc	/* Read data bytes (Dual I/O SPI) */
 #define SPINOR_OP_READ_1_1_4_4B	0x6c	/* Read data bytes (Quad Output SPI) */
 #define SPINOR_OP_READ_1_4_4_4B	0xec	/* Read data bytes (Quad I/O SPI) */
+#define SPINOR_OP_READ_1_1_8_4B	0x7c	/* Read data bytes (Octal Output SPI) */
 #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_PP_1_1_4_4B	0x34	/* Quad page program */
 #define SPINOR_OP_PP_1_4_4_4B	0x3e	/* Quad page program */