[RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ protocols

[Prabhakar Kushwaha]

Hi Cyrille,


> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen at wedev4u.fr]
> Sent: Wednesday, December 06, 2017 4:04 PM
> To: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>; linux-
> mtd at lists.infradead.org
> Cc: boris.brezillon at free-electrons.com; computersforpeace at gmail.com;
> dedekind1 at gmail.com
> Subject: Re: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ
> protocols
> 
> Hi Prabhakar,
> 
> Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> > IO READ protocols transfers both address and data on multiple
> > data bits. 1-2-2(DUAL IO), 1-4-4(QUAD IO) transfer address on 2
> > data bits or 4 bits per rising edge of SCK respectively.
> >
> > This patch update spi_nor_flash_parameter->spi_nor_read_command
> > array based on DUAL or QUAD IO flag enabled in flash_info for a flash.
> >
> > Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index 19c00072..7d3b52f 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -61,7 +61,7 @@ struct flash_info {
> >  	u16		page_size;
> >  	u16		addr_width;
> >
> > -	u16		flags;
> > +	u32		flags;
> >  #define SECT_4K			BIT(0)	/* SPINOR_OP_BE_4K works
> uniformly */
> >  #define SPI_NOR_NO_ERASE	BIT(1)	/* No erase command needed */
> >  #define SST_WRITE		BIT(2)	/* use SST byte programming */
> > @@ -89,6 +89,8 @@ 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_DUAL_IO_READ	BIT(15)	/* Flash supports Dual IO Read
> */
> > +#define SPI_NOR_QUAD_IO_READ	BIT(16)	/* Flash supports Quad IO
> Read */
> >  };
> >
> >  #define JEDEC_MFR(info)	((info)->id[0])
> > @@ -2399,6 +2401,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
> >  					  SNOR_PROTO_1_1_2);
> >  	}
> >
> > +	if (info->flags & SPI_NOR_DUAL_IO_READ) {
> > +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
> > +		spi_nor_set_read_settings(&params-
> >reads[SNOR_CMD_READ_1_2_2],
> > +					  0, 8, SPINOR_OP_READ_1_2_2,
> > +					  SNOR_PROTO_1_2_2);
> > +	}
> > +
> >  	if (info->flags & SPI_NOR_QUAD_READ) {
> >  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
> >  		spi_nor_set_read_settings(&params-
> >reads[SNOR_CMD_READ_1_1_4],
> > @@ -2406,6 +2415,14 @@ static int spi_nor_init_params(struct spi_nor *nor,
> >  					  SNOR_PROTO_1_1_4);
> >  	}
> >
> > +	if (info->flags & SPI_NOR_QUAD_IO_READ) {
> > +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4;
> > +		spi_nor_set_read_settings(&params-
> >reads[SNOR_CMD_READ_1_4_4],
> > +					  0, 10, SPINOR_OP_READ_1_4_4,
> 
> The actual number of mode and wait state clock cycles depend on the
> manufacturer and the memory part number.
> 
> Here are few examples of factory settings for Fast Read 1-4-4
> (mode/wait states):
> 
> Micron: 1/9
> Macronix: 2/4
> Spansion: 2/8
> 
> AFAIK, 0/10 is not used by any manufacturer and doesn't work with Macronix.
> 
> For Fast Read 1-1-1, 1-1-2 and 1-1-4, all memories I know use 0/8. I guess
> those numbers were chosen for 1-1-2 and 1-1-4 too to be backward compatible
> with older memories supporting only Fast Read 1-1-1, which always uses no
> node cycle and 8 wait-states.
> 
> However with the introduction of Fast Read 1-2-2 and 1-4-4, things became
> messy and every manufacturer used its own settings. That's why we didn't
> introduce SPI_NOR_DUAL_IO_READ of SPI_NOR_QUAD_IO_READ flags: because
> we
> can associate them settings that would apply to all manufacturers.
> 
> We can still use Fast Read 1-2-2 and 1-4-4 when supported by both the
> memory and the SPI controller by parsing the memory SFDP tables and filling
> the 'struct spi_nor_flash_parameter' accordingly.
> 
> We (MTD maintainers) don't want to add more hard-coded data in the
> 'struct flash_info' and in the spi_nor_ids[] array when those data can
> actually be retrieved from the SFDP tables.
> 
> Latest SPI NOR memories almost all support SFDP tables (JEDEC JESD216B).
> I had a quick overview of you series and it seems that you have been
> testing with both Micron and Spansion memory parts: both should support
> SFDP.
> 
> So sorry but I will reject this patch: as I said unlike Fast Read 1-1-z
> protocols which always use 0 mode cycle and 8 wait states, there are no
> settings for Fast Read 1-4-4 or 1-2-2 that could be valid for all
> memory manufacturers.
> 

I understand your point. Let me explore SFDP.

--pk