Patchwork [Rebase/PATCHv2] drivers: mtd: m25p80: Add quad read support.

login
register
mail settings
Submitter Poddar, Sourav
Date Oct. 30, 2013, 9:20 a.m.
Message ID <1383124802-25079-1-git-send-email-sourav.poddar@ti.com>
Download mbox | patch
Permalink /patch/287171/
State New
Headers show

Comments

Poddar, Sourav - Oct. 30, 2013, 9:20 a.m.
Some flash also support quad read mode.
Adding support for adding quad mode in m25p80
for spansion and macronix flash.

Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
Rebase this on latest l2mtd.
v1->v2:
Small dev_err message fix to make it
mode appropriate.
v1: http://patchwork.ozlabs.org/patch/286109/

There is one cleanup suggestion from Marek Vasut on read_sr
value. I will take that up as a seperate patch, once this 
patch gets done.


 drivers/mtd/devices/m25p80.c |  160 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 155 insertions(+), 5 deletions(-)
Huang Shijie - Oct. 30, 2013, 9:29 a.m.
于 2013年10月30日 17:20, Sourav Poddar 写道:
> @@ -1077,6 +1221,12 @@ static int m25p_probe(struct spi_device *spi)
>   		flash->addr_width = 4;
>   		if (JEDEC_MFR(info->jedec_id) == CFI_MFR_AMD) {
>   			/* Dedicated 4-byte command set */
> +			if (flash->quad_read)
> +				flash->read_opcode = OPCODE_QUAD_READ_4B;
> +			else
> +				flash->read_opcode = flash->fast_read ?
> +					OPCODE_FAST_READ_4B :
> +					OPCODE_NORM_READ_4B;
>   			flash->read_opcode = flash->fast_read ?
>   				OPCODE_FAST_READ_4B :
>   				OPCODE_NORM_READ_4B;
you missed to remove these three lines above?

thanks
Huang Shijie
Poddar, Sourav - Oct. 30, 2013, 9:32 a.m.
Hi Huang,
On Wednesday 30 October 2013 02:59 PM, Huang Shijie wrote:
> 于 2013年10月30日 17:20, Sourav Poddar 写道:
>> @@ -1077,6 +1221,12 @@ static int m25p_probe(struct spi_device *spi)
>>           flash->addr_width = 4;
>>           if (JEDEC_MFR(info->jedec_id) == CFI_MFR_AMD) {
>>               /* Dedicated 4-byte command set */
>> +            if (flash->quad_read)
>> +                flash->read_opcode = OPCODE_QUAD_READ_4B;
>> +            else
>> +                flash->read_opcode = flash->fast_read ?
>> +                    OPCODE_FAST_READ_4B :
>> +                    OPCODE_NORM_READ_4B;
>>               flash->read_opcode = flash->fast_read ?
>>                   OPCODE_FAST_READ_4B :
>>                   OPCODE_NORM_READ_4B;
> you missed to remove these three lines above?
My bad. its removed in my internal testing branch. While rebasing,
I made some error. I will update the patch and send v3.

Thanks Huang for your review. if you have any other comments, it will be 
great to
know, so that I can consolidate all in v3 and send.

Thanks!
>
> thanks
> Huang Shijie
>
Huang Shijie - Oct. 30, 2013, 10:18 a.m.
于 2013年10月30日 17:20, Sourav Poddar 写道:
> @@ -1051,6 +1184,15 @@ static int m25p_probe(struct spi_device *spi)
>   	flash->page_size = info->page_size;
>   	flash->mtd.writebufsize = flash->page_size;
>
> +	if (spi->mode&  SPI_RX_QUAD&&  info->flags&  M25P80_QUAD_READ) {
> +		ret = set_quad_mode(flash, info->jedec_id);
> +		if (ret) {
> +			dev_err(&flash->spi->dev, "quad mode not supported\n");
> +			return ret;
> +		}
> +		flash->quad_read = true;
> +	}
Do we really need the M25P80_QUAD_READ?

Some NOR flash can supports both the QUAD read and DDR QUAD read, while 
some NOR flash only supports the
QUAD read.

when we try to add the DDR QUAD READ support, should we add another 
flag, such as M25P80_DDR_QUAD_READ?

Add Mark for this email.

I think you'd better CC this patch to the SPI maillist & Mark,.


thanks
Huang Shijie
Poddar, Sourav - Oct. 30, 2013, 10:19 a.m.
On Wednesday 30 October 2013 03:48 PM, Huang Shijie wrote:
> 于 2013年10月30日 17:20, Sourav Poddar 写道:
>> @@ -1051,6 +1184,15 @@ static int m25p_probe(struct spi_device *spi)
>>       flash->page_size = info->page_size;
>>       flash->mtd.writebufsize = flash->page_size;
>>
>> +    if (spi->mode&  SPI_RX_QUAD&&  info->flags&  M25P80_QUAD_READ) {
>> +        ret = set_quad_mode(flash, info->jedec_id);
>> +        if (ret) {
>> +            dev_err(&flash->spi->dev, "quad mode not supported\n");
>> +            return ret;
>> +        }
>> +        flash->quad_read = true;
>> +    }
> Do we really need the M25P80_QUAD_READ?
>
This is what we allign initially that its correct to  get this 
information through flash itself rather than dt.
> Some NOR flash can supports both the QUAD read and DDR QUAD read, 
> while some NOR flash only supports the
> QUAD read.
>
> when we try to add the DDR QUAD READ support, should we add another 
> flag, such as M25P80_DDR_QUAD_READ?
>
I suppose we need to.
> Add Mark for this email.
>
> I think you'd better CC this patch to the SPI maillist & Mark,.
>
>
> thanks
> Huang Shijie
>
>
>
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Huang Shijie - Oct. 30, 2013, 10:25 a.m.
于 2013年10月30日 18:19, Sourav Poddar 写道:
> This is what we allign initially that its correct to  get this 
> information through flash itself rather than dt. 
okay. got it.



Huang Shijie
Marek Vasut - Oct. 30, 2013, 1:38 p.m.
Dear Huang Shijie,

> 于 2013年10月30日 18:19, Sourav Poddar 写道:
> > This is what we allign initially that its correct to  get this
> > information through flash itself rather than dt.
> 
> okay. got it.

We certainly need this to operate as such:

- The SPI NOR driver knows what the SPI NOR flash can do (if it can do fast 
read, quad read, ddr read ...)
- The SPI bus driver knows what types of transfer it can support

The SPI NOR driver shall then negotiate the best transfer option and use it. So 
yes, we need flags to specify what each SPI NOR can do.

Best regards,
Marek Vasut
Brian Norris - Oct. 30, 2013, 11:19 p.m.
On Wed, Oct 30, 2013 at 02:50:02PM +0530, Sourav Poddar wrote:
> Some flash also support quad read mode.
> Adding support for adding quad mode in m25p80
> for spansion and macronix flash.
> 
> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> ---
> Rebase this on latest l2mtd.
> v1->v2:
> Small dev_err message fix to make it
> mode appropriate.
> v1: http://patchwork.ozlabs.org/patch/286109/
> 
> There is one cleanup suggestion from Marek Vasut on read_sr
> value. I will take that up as a seperate patch, once this 
> patch gets done.
> 
> 
>  drivers/mtd/devices/m25p80.c |  160 ++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 155 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 5897889..ba4dd8b 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c

[...]

> @@ -76,6 +79,10 @@
>  #define	SR_BP2			0x10	/* Block protect 2 */
>  #define	SR_SRWD			0x80	/* SR write protect */
>  
> +/* Configuration Register bits. */
> +#define CR_QUAD_EN_SPAN		0x2     /* Spansion Quad I/O */
> +#define SR_QUAD_EN_MX		0x40    /* Macronix Quad I/O */

Two little bits of style here:
 - The other bitfields have a tab after #define (although this comes out
   to 1 character-width of whitespace, so it looks like a space)
 - We already have a listing for several Status Register (SR_*)
   bitfields; I think it makes some sense to group the SR_QUAD_EN_MX
   with the other SR_* bits and keep the CR_* macro separate. That could
   help with keeping the difference between CR and SR clearer.

> +
>  /* Define max times to check status register before we give up. */
>  #define	MAX_READY_WAIT_JIFFIES	(40 * HZ)	/* M25P16 specs 40s max chip erase */
>  #define	MAX_CMD_SIZE		6
> @@ -95,6 +102,7 @@ struct m25p {
>  	u8			program_opcode;
>  	u8			*command;
>  	bool			fast_read;
> +	bool                    quad_read;

Did you have a response to my earlier suggestion that the fast_read and
quad_read fields be combined to a single field? This could easily be an
enum, and I think it could help some of the other code. It also wouldn't
require us to remember that quad_read takes precedence over fast_read
(which you do implicitly in this patch). And we can already foresee
additional switches needed if we add the DDR command types (Huang was
looking at this?), so we should just get it right now.

You could, perhaps, make this two patches: one for converting the bool
to an enum, and the other for supporting quad-read.

Or if you're having trouble, I could cook the first patch up.

>  };
>  
>  static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd)

[...]

> @@ -1077,6 +1221,12 @@ static int m25p_probe(struct spi_device *spi)
>  		flash->addr_width = 4;
>  		if (JEDEC_MFR(info->jedec_id) == CFI_MFR_AMD) {
>  			/* Dedicated 4-byte command set */
> +			if (flash->quad_read)
> +				flash->read_opcode = OPCODE_QUAD_READ_4B;
> +			else
> +				flash->read_opcode = flash->fast_read ?
> +					OPCODE_FAST_READ_4B :
> +					OPCODE_NORM_READ_4B;

Besides the repetition below (already mentioned by Huang), the above
assignment is kind of ugly again, mixing ?: and if/else. If we're
keeping the fast_read/quad_read bools, it should be:

	if (flash->quad_read)
		flash->read_opcode = OPCODE_QUAD_READ_4B;
	else if (flash->fast_read)
		flash->read_opcode = OPCODE_FAST_READ_4B;
	else
		flash->read_opcode = OPCODE_NORM_READ_4B;

(or if we use an enum, it's just a switch statement...)

>  			flash->read_opcode = flash->fast_read ?
>  				OPCODE_FAST_READ_4B :
>  				OPCODE_NORM_READ_4B;

Brian
Poddar, Sourav - Oct. 31, 2013, 5:01 a.m.
On Thursday 31 October 2013 04:49 AM, Brian Norris wrote:
> On Wed, Oct 30, 2013 at 02:50:02PM +0530, Sourav Poddar wrote:
>> Some flash also support quad read mode.
>> Adding support for adding quad mode in m25p80
>> for spansion and macronix flash.
>>
>> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com>
>> ---
>> Rebase this on latest l2mtd.
>> v1->v2:
>> Small dev_err message fix to make it
>> mode appropriate.
>> v1: http://patchwork.ozlabs.org/patch/286109/
>>
>> There is one cleanup suggestion from Marek Vasut on read_sr
>> value. I will take that up as a seperate patch, once this
>> patch gets done.
>>
>>
>>   drivers/mtd/devices/m25p80.c |  160 ++++++++++++++++++++++++++++++++++++++++--
>>   1 files changed, 155 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index 5897889..ba4dd8b 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
> [...]
>
>> @@ -76,6 +79,10 @@
>>   #define	SR_BP2			0x10	/* Block protect 2 */
>>   #define	SR_SRWD			0x80	/* SR write protect */
>>
>> +/* Configuration Register bits. */
>> +#define CR_QUAD_EN_SPAN		0x2     /* Spansion Quad I/O */
>> +#define SR_QUAD_EN_MX		0x40    /* Macronix Quad I/O */
> Two little bits of style here:
>   - The other bitfields have a tab after #define (although this comes out
>     to 1 character-width of whitespace, so it looks like a space)
>   - We already have a listing for several Status Register (SR_*)
>     bitfields; I think it makes some sense to group the SR_QUAD_EN_MX
>     with the other SR_* bits and keep the CR_* macro separate. That could
>     help with keeping the difference between CR and SR clearer.
>
Ok. Will change.
>> +
>>   /* Define max times to check status register before we give up. */
>>   #define	MAX_READY_WAIT_JIFFIES	(40 * HZ)	/* M25P16 specs 40s max chip erase */
>>   #define	MAX_CMD_SIZE		6
>> @@ -95,6 +102,7 @@ struct m25p {
>>   	u8			program_opcode;
>>   	u8			*command;
>>   	bool			fast_read;
>> +	bool                    quad_read;
> Did you have a response to my earlier suggestion that the fast_read and
> quad_read fields be combined to a single field? This could easily be an
> enum, and I think it could help some of the other code. It also wouldn't
> require us to remember that quad_read takes precedence over fast_read
> (which you do implicitly in this patch). And we can already foresee
> additional switches needed if we add the DDR command types (Huang was
> looking at this?), so we should just get it right now.
>
> You could, perhaps, make this two patches: one for converting the bool
> to an enum, and the other for supporting quad-read.
>
I read that, and I was planning to take that as a seperate excercise, 
but yes I
will cook this into two independent patches.
> Or if you're having trouble, I could cook the first patch up.
>
>>   };
>>
>>   static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd)
> [...]
>
>> @@ -1077,6 +1221,12 @@ static int m25p_probe(struct spi_device *spi)
>>   		flash->addr_width = 4;
>>   		if (JEDEC_MFR(info->jedec_id) == CFI_MFR_AMD) {
>>   			/* Dedicated 4-byte command set */
>> +			if (flash->quad_read)
>> +				flash->read_opcode = OPCODE_QUAD_READ_4B;
>> +			else
>> +				flash->read_opcode = flash->fast_read ?
>> +					OPCODE_FAST_READ_4B :
>> +					OPCODE_NORM_READ_4B;
> Besides the repetition below (already mentioned by Huang), the above
> assignment is kind of ugly again, mixing ?: and if/else. If we're
> keeping the fast_read/quad_read bools, it should be:
>
> 	if (flash->quad_read)
> 		flash->read_opcode = OPCODE_QUAD_READ_4B;
> 	else if (flash->fast_read)
> 		flash->read_opcode = OPCODE_FAST_READ_4B;
> 	else
> 		flash->read_opcode = OPCODE_NORM_READ_4B;
>
> (or if we use an enum, it's just a switch statement...)
>
Ok.
>>   			flash->read_opcode = flash->fast_read ?
>>   				OPCODE_FAST_READ_4B :
>>   				OPCODE_NORM_READ_4B;
> Brian
Brian Norris - Oct. 31, 2013, 4:05 p.m.
On Thu, Oct 31, 2013 at 10:31:23AM +0530, Sourav Poddar wrote:
> On Thursday 31 October 2013 04:49 AM, Brian Norris wrote:
> >On Wed, Oct 30, 2013 at 02:50:02PM +0530, Sourav Poddar wrote:
> >>@@ -95,6 +102,7 @@ struct m25p {
> >>  	u8			program_opcode;
> >>  	u8			*command;
> >>  	bool			fast_read;
> >>+	bool                    quad_read;
> >Did you have a response to my earlier suggestion that the fast_read and
> >quad_read fields be combined to a single field? This could easily be an
> >enum, and I think it could help some of the other code. It also wouldn't
> >require us to remember that quad_read takes precedence over fast_read
> >(which you do implicitly in this patch). And we can already foresee
> >additional switches needed if we add the DDR command types (Huang was
> >looking at this?), so we should just get it right now.
> >
> >You could, perhaps, make this two patches: one for converting the bool
> >to an enum, and the other for supporting quad-read.
> >
> I read that, and I was planning to take that as a seperate
> excercise, but yes I
> will cook this into two independent patches.

I think it is good to require the correct design principle (and
appropriate cleanup) before adding new features.

Brian
Poddar, Sourav - Nov. 2, 2013, 5:48 a.m.
Hi Brian,
On Thursday 31 October 2013 09:35 PM, Brian Norris wrote:
> On Thu, Oct 31, 2013 at 10:31:23AM +0530, Sourav Poddar wrote:
>> On Thursday 31 October 2013 04:49 AM, Brian Norris wrote:
>>> On Wed, Oct 30, 2013 at 02:50:02PM +0530, Sourav Poddar wrote:
>>>> @@ -95,6 +102,7 @@ struct m25p {
>>>>   	u8			program_opcode;
>>>>   	u8			*command;
>>>>   	bool			fast_read;
>>>> +	bool                    quad_read;
>>> Did you have a response to my earlier suggestion that the fast_read and
>>> quad_read fields be combined to a single field? This could easily be an
>>> enum, and I think it could help some of the other code. It also wouldn't
>>> require us to remember that quad_read takes precedence over fast_read
>>> (which you do implicitly in this patch). And we can already foresee
>>> additional switches needed if we add the DDR command types (Huang was
>>> looking at this?), so we should just get it right now.
>>>
>>> You could, perhaps, make this two patches: one for converting the bool
>>> to an enum, and the other for supporting quad-read.
>>>
>> I read that, and I was planning to take that as a seperate
>> excercise, but yes I
>> will cook this into two independent patches.
> I think it is good to require the correct design principle (and
> appropriate cleanup) before adding new features.
>
True, as per your suggestion, I will cook this up into two seperate patches.
One for converting bools into enum.
second for adding quad support.

Few clarifications here:
1, I hope I can use l2-mtd to rebase my patch, I was seeing a mail about
reverting certain patch, which I suppose is not required now.
2. Your patches were refactoring of the current code, where you were 
defaulting the
   read opcode to fast read. If you carefully see my quad read patch, 
based on conditions,
   I am assigning read opcode to quad or fast read, and by default, I am 
keeping it at
  NORMAL READ. The reason behind this is that I think fast/dual/quad are 
special cases, which needs to
  be set explicitly based on certain dt parameters and by default, we 
should stick to NORMAL read.
I hope you are fine with this approach too?
> Brian
Brian Norris - Nov. 5, 2013, 3:49 a.m.
On Sat, Nov 02, 2013 at 11:18:18AM +0530, Sourav Poddar wrote:
> Hi Brian,
> On Thursday 31 October 2013 09:35 PM, Brian Norris wrote:
> >On Thu, Oct 31, 2013 at 10:31:23AM +0530, Sourav Poddar wrote:
> >>On Thursday 31 October 2013 04:49 AM, Brian Norris wrote:
> >>>On Wed, Oct 30, 2013 at 02:50:02PM +0530, Sourav Poddar wrote:
> >>>>@@ -95,6 +102,7 @@ struct m25p {
> >>>>  	u8			program_opcode;
> >>>>  	u8			*command;
> >>>>  	bool			fast_read;
> >>>>+	bool                    quad_read;
> >>>Did you have a response to my earlier suggestion that the fast_read and
> >>>quad_read fields be combined to a single field? This could easily be an
> >>>enum, and I think it could help some of the other code. It also wouldn't
> >>>require us to remember that quad_read takes precedence over fast_read
> >>>(which you do implicitly in this patch). And we can already foresee
> >>>additional switches needed if we add the DDR command types (Huang was
> >>>looking at this?), so we should just get it right now.
> >>>
> >>>You could, perhaps, make this two patches: one for converting the bool
> >>>to an enum, and the other for supporting quad-read.
> >>>
> >>I read that, and I was planning to take that as a seperate
> >>excercise, but yes I
> >>will cook this into two independent patches.
> >I think it is good to require the correct design principle (and
> >appropriate cleanup) before adding new features.
> >
> True, as per your suggestion, I will cook this up into two seperate patches.
> One for converting bools into enum.
> second for adding quad support.

Sounds good. Thanks.

> Few clarifications here:
> 1, I hope I can use l2-mtd to rebase my patch, I was seeing a mail about
> reverting certain patch, which I suppose is not required now.

I don't believe we are planning to revert any of the m25p80 patches
right now. If we make any further changes, I can reconcile conflicts.

> 2. Your patches were refactoring of the current code, where you were
> defaulting the
>   read opcode to fast read. If you carefully see my quad read patch,
> based on conditions,
>   I am assigning read opcode to quad or fast read, and by default, I
> am keeping it at
>  NORMAL READ. The reason behind this is that I think fast/dual/quad
> are special cases, which needs to
>  be set explicitly based on certain dt parameters and by default, we
> should stick to NORMAL read.

As I read your current patch, it does not actually change any of the
fast-read decisions that Marek and I were discussing.

> I hope you are fine with this approach too?

AFAICT, your current patch was fine (w.r.t. the fast-read problem).

Brian

Patch

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 5897889..ba4dd8b 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -41,6 +41,7 @@ 
 #define	OPCODE_WRSR		0x01	/* Write status register 1 byte */
 #define	OPCODE_NORM_READ	0x03	/* Read data bytes (low frequency) */
 #define	OPCODE_FAST_READ	0x0b	/* Read data bytes (high frequency) */
+#define	OPCODE_QUAD_READ        0x6b    /* Read data bytes */
 #define	OPCODE_PP		0x02	/* Page program (up to 256 bytes) */
 #define	OPCODE_BE_4K		0x20	/* Erase 4KiB block */
 #define	OPCODE_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
@@ -48,10 +49,12 @@ 
 #define	OPCODE_CHIP_ERASE	0xc7	/* Erase whole flash chip */
 #define	OPCODE_SE		0xd8	/* Sector erase (usually 64KiB) */
 #define	OPCODE_RDID		0x9f	/* Read JEDEC ID */
+#define	OPCODE_RDCR             0x35    /* Read configuration register */
 
 /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
 #define	OPCODE_NORM_READ_4B	0x13	/* Read data bytes (low frequency) */
 #define	OPCODE_FAST_READ_4B	0x0c	/* Read data bytes (high frequency) */
+#define	OPCODE_QUAD_READ_4B	0x6c    /* Read data bytes */
 #define	OPCODE_PP_4B		0x12	/* Page program (up to 256 bytes) */
 #define	OPCODE_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
 
@@ -76,6 +79,10 @@ 
 #define	SR_BP2			0x10	/* Block protect 2 */
 #define	SR_SRWD			0x80	/* SR write protect */
 
+/* Configuration Register bits. */
+#define CR_QUAD_EN_SPAN		0x2     /* Spansion Quad I/O */
+#define SR_QUAD_EN_MX		0x40    /* Macronix Quad I/O */
+
 /* Define max times to check status register before we give up. */
 #define	MAX_READY_WAIT_JIFFIES	(40 * HZ)	/* M25P16 specs 40s max chip erase */
 #define	MAX_CMD_SIZE		6
@@ -95,6 +102,7 @@  struct m25p {
 	u8			program_opcode;
 	u8			*command;
 	bool			fast_read;
+	bool                    quad_read;
 };
 
 static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd)
@@ -131,6 +139,26 @@  static int read_sr(struct m25p *flash)
 }
 
 /*
+ * Read configuration register, returning its value in the
+ * location. Return the configuration register value.
+ * Returns negative if error occured.
+ */
+static int read_cr(struct m25p *flash)
+{
+	u8 code = OPCODE_RDCR;
+	int ret;
+	u8 val;
+
+	ret = spi_write_then_read(flash->spi, &code, 1, &val, 1);
+	if (ret < 0) {
+		dev_err(&flash->spi->dev, "error %d reading CR\n", ret);
+		return ret;
+	}
+
+	return val;
+}
+
+/*
  * Write status register 1 byte
  * Returns negative if error occurred.
  */
@@ -220,6 +248,95 @@  static int wait_till_ready(struct m25p *flash)
 }
 
 /*
+ * Write status Register and configuration register with 2 bytes
+ * The first byte will be written to the status register, while the
+ * second byte will be written to the configuration register.
+ * Return negative if error occured.
+ */
+static int write_sr_cr(struct m25p *flash, u16 val)
+{
+	flash->command[0] = OPCODE_WRSR;
+	flash->command[1] = val & 0xff;
+	flash->command[2] = (val >> 8);
+
+	return spi_write(flash->spi, flash->command, 3);
+}
+
+static int macronix_quad_enable(struct m25p *flash)
+{
+	int ret, val;
+	u8 cmd[2];
+	cmd[0] = OPCODE_WRSR;
+
+	val = read_sr(flash);
+	cmd[1] = val | SR_QUAD_EN_MX;
+	write_enable(flash);
+
+	spi_write(flash->spi, &cmd, 2);
+
+	if (wait_till_ready(flash))
+		return 1;
+
+	ret = read_sr(flash);
+	if (!(ret > 0 && (ret & SR_QUAD_EN_MX))) {
+		dev_err(&flash->spi->dev,
+			"Macronix Quad bit not set");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int spansion_quad_enable(struct m25p *flash)
+{
+	int ret;
+	int quad_en = CR_QUAD_EN_SPAN << 8;
+
+	write_enable(flash);
+
+	ret = write_sr_cr(flash, quad_en);
+	if (ret < 0) {
+		dev_err(&flash->spi->dev,
+			"error while writing configuration register");
+		return -EINVAL;
+	}
+
+	/* read back and check it */
+	ret = read_cr(flash);
+	if (!(ret > 0 && (ret & CR_QUAD_EN_SPAN))) {
+		dev_err(&flash->spi->dev,
+			"Spansion Quad bit not set");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static inline int set_quad_mode(struct m25p *flash, u32 jedec_id)
+{
+	int status;
+
+	switch (JEDEC_MFR(jedec_id)) {
+	case CFI_MFR_MACRONIX:
+		status = macronix_quad_enable(flash);
+		if (status) {
+			dev_err(&flash->spi->dev,
+				"Macronix quad-read not enabled");
+			return -EINVAL;
+		}
+		return status;
+	default:
+		status = spansion_quad_enable(flash);
+		if (status) {
+			dev_err(&flash->spi->dev,
+				"Spansion quad-read not enabled");
+			return -EINVAL;
+		}
+		return status;
+	}
+}
+
+/*
  * Erase the whole flash memory
  *
  * Returns 0 if successful, non-zero otherwise.
@@ -253,11 +370,24 @@  static void m25p_addr2cmd(struct m25p *flash, unsigned int addr, u8 *cmd)
 	cmd[4] = addr >> (flash->addr_width * 8 - 32);
 }
 
+/*
+ * Dummy Cycle calculation for fast and quad read.
+ * It can be used to support more commands with
+ * different dummy cycle requirement.
+ */
 static int m25p_cmdsz(struct m25p *flash)
 {
 	return 1 + flash->addr_width;
 }
 
+static inline int m25p80_dummy_cycles_read(struct m25p *flash)
+{
+	if (flash->quad_read || flash->fast_read)
+		return 1;
+
+	return 0;
+}
+
 /*
  * Erase one sector of flash memory at offset ``offset'' which is any
  * address within the sector which should be erased.
@@ -368,9 +498,10 @@  static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
 	memset(t, 0, (sizeof t));
 
 	t[0].tx_buf = flash->command;
-	t[0].len = m25p_cmdsz(flash) + (flash->fast_read ? 1 : 0);
+	t[0].len = m25p_cmdsz(flash) + m25p80_dummy_cycles_read(flash);
 	spi_message_add_tail(&t[0], &m);
 
+	t[1].rx_nbits = flash->quad_read ? SPI_NBITS_QUAD : 1;
 	t[1].rx_buf = buf;
 	t[1].len = len;
 	spi_message_add_tail(&t[1], &m);
@@ -392,7 +523,7 @@  static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
 	spi_sync(flash->spi, &m);
 
 	*retlen = m.actual_length - m25p_cmdsz(flash) -
-			(flash->fast_read ? 1 : 0);
+			m25p80_dummy_cycles_read(flash);
 
 	mutex_unlock(&flash->lock);
 
@@ -698,6 +829,7 @@  struct flash_info {
 #define	SST_WRITE	0x04		/* use SST byte programming */
 #define	M25P_NO_FR	0x08		/* Can't do fastread */
 #define	SECT_4K_PMC	0x10		/* OPCODE_BE_4K_PMC works uniformly */
+#define	M25P80_QUAD_READ	0x20    /* Flash supports Quad Read */
 };
 
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
@@ -775,7 +907,7 @@  static const struct spi_device_id m25p_ids[] = {
 	{ "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
 	{ "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, 0) },
 	{ "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) },
-	{ "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, 0) },
+	{ "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, M25P80_QUAD_READ) },
 
 	/* Micron */
 	{ "n25q064",     INFO(0x20ba17, 0, 64 * 1024,  128, 0) },
@@ -795,7 +927,7 @@  static const struct spi_device_id m25p_ids[] = {
 	{ "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,  64, 0) },
 	{ "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, 0) },
 	{ "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
-	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, 0) },
+	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, M25P80_QUAD_READ) },
 	{ "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, 0) },
 	{ "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
 	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
@@ -937,6 +1069,7 @@  static int m25p_probe(struct spi_device *spi)
 	unsigned			i;
 	struct mtd_part_parser_data	ppdata;
 	struct device_node *np = spi->dev.of_node;
+	int ret;
 
 	/* Platform data helps sort out which chip type we have, as
 	 * well as how this board partitions it.  If we don't have
@@ -1051,6 +1184,15 @@  static int m25p_probe(struct spi_device *spi)
 	flash->page_size = info->page_size;
 	flash->mtd.writebufsize = flash->page_size;
 
+	if (spi->mode & SPI_RX_QUAD && info->flags & M25P80_QUAD_READ) {
+		ret = set_quad_mode(flash, info->jedec_id);
+		if (ret) {
+			dev_err(&flash->spi->dev, "quad mode not supported\n");
+			return ret;
+		}
+		flash->quad_read = true;
+	}
+
 	if (np)
 		/* If we were instantiated by DT, use it */
 		flash->fast_read = of_property_read_bool(np, "m25p,fast-read");
@@ -1063,7 +1205,9 @@  static int m25p_probe(struct spi_device *spi)
 		flash->fast_read = false;
 
 	/* Default commands */
-	if (flash->fast_read)
+	if (flash->quad_read)
+		flash->read_opcode = OPCODE_QUAD_READ;
+	else if (flash->fast_read)
 		flash->read_opcode = OPCODE_FAST_READ;
 	else
 		flash->read_opcode = OPCODE_NORM_READ;
@@ -1077,6 +1221,12 @@  static int m25p_probe(struct spi_device *spi)
 		flash->addr_width = 4;
 		if (JEDEC_MFR(info->jedec_id) == CFI_MFR_AMD) {
 			/* Dedicated 4-byte command set */
+			if (flash->quad_read)
+				flash->read_opcode = OPCODE_QUAD_READ_4B;
+			else
+				flash->read_opcode = flash->fast_read ?
+					OPCODE_FAST_READ_4B :
+					OPCODE_NORM_READ_4B;
 			flash->read_opcode = flash->fast_read ?
 				OPCODE_FAST_READ_4B :
 				OPCODE_NORM_READ_4B;