Patchwork [V1,3/5] mtd: m25p80: add the quad-read support

login
register
mail settings
Submitter Huang Shijie
Date Aug. 19, 2013, 4:10 a.m.
Message ID <1376885403-12156-4-git-send-email-b32955@freescale.com>
Download mbox | patch
Permalink /patch/268077/
State New
Headers show

Comments

Huang Shijie - Aug. 19, 2013, 4:10 a.m.
This patch adds the quad read support:

(1) Add the relative commands:
      OPCODE_QIOR, OPCODE_4QIOR, OPCODE_RDCR,

    also add the relative macro for the Configuartion Register.

(2) add the "m25p,quad-read" property for the m25p80 driver
    If the dts has the "m25p,quad-read" property, the kernel will
    set the Quad bit of the configuration register, and when the
    setting is suceeded, we set the read opcode with OPCODE_QIOR.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 Documentation/devicetree/bindings/mtd/m25p80.txt |    5 ++
 drivers/mtd/devices/m25p80.c                     |   51 ++++++++++++++++++++++
 include/linux/mtd/spi-nor.h                      |    6 +++
 3 files changed, 62 insertions(+), 0 deletions(-)
Brian Norris - Aug. 22, 2013, 7:34 p.m.
Adding others (keep devicetree@vger.kernel.org in the loop)

On Mon, Aug 19, 2013 at 12:10:01PM +0800, Huang Shijie wrote:
> This patch adds the quad read support:
> 
> (1) Add the relative commands:
>       OPCODE_QIOR, OPCODE_4QIOR, OPCODE_RDCR,
> 
>     also add the relative macro for the Configuartion Register.
> 
> (2) add the "m25p,quad-read" property for the m25p80 driver
>     If the dts has the "m25p,quad-read" property, the kernel will
>     set the Quad bit of the configuration register, and when the
>     setting is suceeded, we set the read opcode with OPCODE_QIOR.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  Documentation/devicetree/bindings/mtd/m25p80.txt |    5 ++
>  drivers/mtd/devices/m25p80.c                     |   51 ++++++++++++++++++++++
>  include/linux/mtd/spi-nor.h                      |    6 +++
>  3 files changed, 62 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
> index 6d3d576..b33313f 100644
> --- a/Documentation/devicetree/bindings/mtd/m25p80.txt
> +++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
> @@ -17,6 +17,11 @@ Optional properties:
>                     Refer to your chips' datasheet to check if this is supported
>                     by your chip.
>  
> +- m25p,quad-read : Use the "quad read" opcode to read data from the chip instead
> +                   of the usual "read" opcode. This opcode is not supported by
> +                   all chips and support for it can not be detected at runtime.
> +                   Refer to your chips' datasheet to check if this is supported
> +                   by your chip.

Why can't this be detected at runtime? We added a "no fast read" flag to
the device table, so why not "dual/quad mode supported"? And believe it
or not, not all m25p80 users have device tree. So it isn't very logical
to tie this support to device-tree only.

>  Example:
>  
>  	flash: m25p80@0 {
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index f3598c1..4bc9b1b 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -103,6 +103,40 @@ static int write_sr(struct m25p *flash, u8 val)
>  }
>  
>  /*
> + * Read the configuration register, returning its value in the location
> + * Return the configuration register value.
> + * Returns negative if error occurred.
> + */
> +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 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.
> + * Returns negative if error occurred.
> + */
> +static int write_sr_cr(struct m25p *flash, u16 val)
> +{
> +	flash->command[0] = OPCODE_WRSR;
> +	flash->command[1] = 0;
> +	flash->command[2] = (val >> 8);
> +
> +	return spi_write(flash->spi, flash->command, 3);
> +}
> +
> +/*
>   * Set write enable latch with Write Enable command.
>   * Returns negative if error occurred.
>   */
> @@ -880,6 +914,8 @@ static int m25p_probe(struct spi_device *spi)
>  	unsigned			i;
>  	struct mtd_part_parser_data	ppdata;
>  	struct device_node __maybe_unused *np = spi->dev.of_node;
> +	u16 sr_cr;
> +	int ret;
>  
>  #ifdef CONFIG_MTD_OF_PARTS
>  	if (!of_device_is_available(np))
> @@ -1014,6 +1050,21 @@ static int m25p_probe(struct spi_device *spi)
>  	else
>  		flash->read_opcode = OPCODE_NORM_READ;
>  
> +	/* Try to enable the Quad Read */
> +	if (np && of_property_read_bool(np, "m25p,quad-read")) {
> +		/* The configuration register is set by the second byte. */
> +		sr_cr = CR_QUAD << 8;
> +
> +		/* Write the QUAD bit to the Configuration Register. */
> +		write_enable(flash);
> +		if (write_sr_cr(flash, sr_cr) == 0) {
> +			/* read back and check it */
> +			ret = read_cr(flash);
> +			if (ret > 0 && (ret & CR_QUAD))
> +				flash->read_opcode = OPCODE_QIOR;

This is not correct. You are assuming that the SPI master knows to read
with 4 IO lines, instead of the traditional 1 line; IOW, you are
assuming that:
(1) if the slave DT node has "quad-read", then the whole system supports
    it (bad design; you're putting assumptions about the "parent" node
    in the child)
(2) the controller driver will act as your new driver does -- that it
    will decode these commands and recognize that the quad read command
    should be run on 4 IO lines, not just 1

What you're really missing from device-tree (and the SPI subystem in
general) is how to detect those SPI controllers which support dual and
quad mode transfers, and how to communicate that a particular SPI
transaction should be performed on 1 or 4 IO lines. We shouldn't have
this just hacked in via assumptions.

See Wang Yuhang's patch set for adding proper dual/quad support to the
SPI subsystem. It seems to be addressing (1) and (2) in a better way.

  http://thread.gmane.org/gmane.linux.kernel.spi.devel/14420

(I can't find patch 2/2)

> +		}
> +	}
> +
>  	flash->program_opcode = OPCODE_PP;
>  
>  	if (info->addr_width)
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index b420a5b..d5b189d 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -39,6 +39,9 @@
>  
>  /* Used for Spansion flashes only. */
>  #define	OPCODE_BRWR		0x17	/* Bank register write */
> +#define OPCODE_QIOR		0xeb	/* Quad read */
> +#define OPCODE_4QIOR		0xec	/* Quad read */

Use tab between 'define' and 'OPCODE...', like OPCODE_RDCR below.

> +#define	OPCODE_RDCR		0x35	/* Read configuration register */
>  
>  /* Status Register bits. */
>  #define	SR_WIP			1	/* Write in progress */
> @@ -49,4 +52,7 @@
>  #define	SR_BP2			0x10	/* Block protect 2 */
>  #define	SR_SRWD			0x80	/* SR write protect */
>  
> +/* Configuration Register bits. */
> +#define CR_QUAD			0x2	/* Quad I/O */
> +
>  #endif /* __LINUX_MTD_SPI_NOR_H */

Brian
Mark Brown - Aug. 22, 2013, 7:55 p.m.
On Thu, Aug 22, 2013 at 12:34:53PM -0700, Brian Norris wrote:
> On Mon, Aug 19, 2013 at 12:10:01PM +0800, Huang Shijie wrote:

> > +- m25p,quad-read : Use the "quad read" opcode to read data from the chip instead
> > +                   of the usual "read" opcode. This opcode is not supported by
> > +                   all chips and support for it can not be detected at runtime.
> > +                   Refer to your chips' datasheet to check if this is supported
> > +                   by your chip.

> Why can't this be detected at runtime? We added a "no fast read" flag to
> the device table, so why not "dual/quad mode supported"? And believe it
> or not, not all m25p80 users have device tree. So it isn't very logical
> to tie this support to device-tree only.

There needs to be some way of saying if the additional data lines are
actually wired up or not; it could be a negative property (flagging if
the lines are not present) but that runs the risk of breaking systems
if a driver acquires the ability to support extra data lines but a
system doesn't have it.

This should be a generic property for all quad devices to use, though,
since the same thing applies everywhere.

> This is not correct. You are assuming that the SPI master knows to read
> with 4 IO lines, instead of the traditional 1 line; IOW, you are
> assuming that:
> (1) if the slave DT node has "quad-read", then the whole system supports
>     it (bad design; you're putting assumptions about the "parent" node
>     in the child)

This is fine, the device tree is for the board as a whole not for the
individual chips - if the board doesn't support quad read the device
tree shouldn't configure the chip for quad mode.  It really needs to be
a slave property since a system could opt to connect some devices with
extra data lines and some without on the same SPI bus.

> What you're really missing from device-tree (and the SPI subystem in
> general) is how to detect those SPI controllers which support dual and
> quad mode transfers, and how to communicate that a particular SPI
> transaction should be performed on 1 or 4 IO lines. We shouldn't have
> this just hacked in via assumptions.

That bit does need to be fixed in this driver, yes.  The SPI core now
has quad mode support.
Marek Vasut - Aug. 22, 2013, 8:29 p.m.
Dear Mark Brown,

> On Thu, Aug 22, 2013 at 12:34:53PM -0700, Brian Norris wrote:
> > On Mon, Aug 19, 2013 at 12:10:01PM +0800, Huang Shijie wrote:
> > > +- m25p,quad-read : Use the "quad read" opcode to read data from the
> > > chip instead +                   of the usual "read" opcode. This
> > > opcode is not supported by +                   all chips and support
> > > for it can not be detected at runtime. +                   Refer to
> > > your chips' datasheet to check if this is supported +                 
> > >  by your chip.
> > 
> > Why can't this be detected at runtime? We added a "no fast read" flag to
> > the device table, so why not "dual/quad mode supported"? And believe it
> > or not, not all m25p80 users have device tree. So it isn't very logical
> > to tie this support to device-tree only.
> 
> There needs to be some way of saying if the additional data lines are
> actually wired up or not; it could be a negative property (flagging if
> the lines are not present) but that runs the risk of breaking systems
> if a driver acquires the ability to support extra data lines but a
> system doesn't have it.
> 
> This should be a generic property for all quad devices to use, though,
> since the same thing applies everywhere.

Full ACK, "m25p,dual-read" and "m25p,quad-read" sound like a good prop names?

Best regards,
Marek Vasut
Mark Brown - Aug. 22, 2013, 11:36 p.m.
On Thu, Aug 22, 2013 at 10:29:01PM +0200, Marek Vasut wrote:
> > On Thu, Aug 22, 2013 at 12:34:53PM -0700, Brian Norris wrote:

> > This should be a generic property for all quad devices to use, though,
> > since the same thing applies everywhere.

> Full ACK, "m25p,dual-read" and "m25p,quad-read" sound like a good prop names?

I was actually thinking something more generic than that - putting the
property at the SPI generic bindings level.  Though if all flashes with
this dual/quad read functionality have the prefix m25p the above would
work also, at the minute this does seem to be mostly used by flash (I
bet someone's got some DSPs or something though).
Marek Vasut - Aug. 22, 2013, 11:58 p.m.
Dear Mark Brown,

> On Thu, Aug 22, 2013 at 10:29:01PM +0200, Marek Vasut wrote:
> > > On Thu, Aug 22, 2013 at 12:34:53PM -0700, Brian Norris wrote:
> > > 
> > > This should be a generic property for all quad devices to use, though,
> > > since the same thing applies everywhere.
> > 
> > Full ACK, "m25p,dual-read" and "m25p,quad-read" sound like a good prop
> > names?
> 
> I was actually thinking something more generic than that - putting the
> property at the SPI generic bindings level.  Though if all flashes with
> this dual/quad read functionality have the prefix m25p the above would
> work also, at the minute this does seem to be mostly used by flash (I
> bet someone's got some DSPs or something though).

Ah! So you mean the SPI controller would provide information that it can do 
dual/quad transfers? But then, the additional pins can only be wired to certain 
chips (controller by certain CS lines).

Best regards,
Marek Vasut
Huang Shijie - Aug. 23, 2013, 6:26 a.m.
于 2013年08月23日 03:55, Mark Brown 写道:
> On Thu, Aug 22, 2013 at 12:34:53PM -0700, Brian Norris wrote:
>> On Mon, Aug 19, 2013 at 12:10:01PM +0800, Huang Shijie wrote:
>
>> Why can't this be detected at runtime? We added a "no fast read" flag to
>> the device table, so why not "dual/quad mode supported"? And believe it
>> or not, not all m25p80 users have device tree. So it isn't very logical
>> to tie this support to device-tree only.
>
Hi Brian:
My only question here is do we need to add some flags , such as "quad
read", to
the device table? If you think we need to, i will add a patch for it.

(Just as Brown said, when the DTS configurates the "m25p,quad-read"
property, it
means : Both the board and the NOR support the Quad read.)


>> What you're really missing from device-tree (and the SPI subystem in
>> general) is how to detect those SPI controllers which support dual and
>> quad mode transfers, and how to communicate that a particular SPI
>> transaction should be performed on 1 or 4 IO lines. We shouldn't have
>> this just hacked in via assumptions.
> That bit does need to be fixed in this driver, yes.  The SPI core now
> has quad mode support.
yes, i do not need these bits in this driver.


thanks
Huang Shijie
王宇航 - Aug. 23, 2013, 9:05 a.m.
Hi, Shijie

2013/8/19 Huang Shijie <b32955@freescale.com>:
> This patch adds the quad read support:
>
> (1) Add the relative commands:
>       OPCODE_QIOR, OPCODE_4QIOR, OPCODE_RDCR,
>
>     also add the relative macro for the Configuartion Register.
>
> (2) add the "m25p,quad-read" property for the m25p80 driver
>     If the dts has the "m25p,quad-read" property, the kernel will
>     set the Quad bit of the configuration register, and when the
>     setting is suceeded, we set the read opcode with OPCODE_QIOR.
>
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  Documentation/devicetree/bindings/mtd/m25p80.txt |    5 ++
>  drivers/mtd/devices/m25p80.c                     |   51 ++++++++++++++++++++++
>  include/linux/mtd/spi-nor.h                      |    6 +++
>  3 files changed, 62 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
> index 6d3d576..b33313f 100644
> --- a/Documentation/devicetree/bindings/mtd/m25p80.txt
> +++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
> @@ -17,6 +17,11 @@ Optional properties:
>                     Refer to your chips' datasheet to check if this is supported
>                     by your chip.
>
> +- m25p,quad-read : Use the "quad read" opcode to read data from the chip instead
> +                   of the usual "read" opcode. This opcode is not supported by
> +                   all chips and support for it can not be detected at runtime.
> +                   Refer to your chips' datasheet to check if this is supported
> +                   by your chip.
>  Example:
>
>         flash: m25p80@0 {
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index f3598c1..4bc9b1b 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -103,6 +103,40 @@ static int write_sr(struct m25p *flash, u8 val)
>  }
>
>  /*
> + * Read the configuration register, returning its value in the location
> + * Return the configuration register value.
> + * Returns negative if error occurred.
> + */
> +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 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.
> + * Returns negative if error occurred.
> + */
> +static int write_sr_cr(struct m25p *flash, u16 val)
> +{
> +       flash->command[0] = OPCODE_WRSR;
> +       flash->command[1] = 0;
> +       flash->command[2] = (val >> 8);
> +
> +       return spi_write(flash->spi, flash->command, 3);
> +}
> +
> +/*
>   * Set write enable latch with Write Enable command.
>   * Returns negative if error occurred.
>   */
> @@ -880,6 +914,8 @@ static int m25p_probe(struct spi_device *spi)
>         unsigned                        i;
>         struct mtd_part_parser_data     ppdata;
>         struct device_node __maybe_unused *np = spi->dev.of_node;
> +       u16 sr_cr;
> +       int ret;
>
>  #ifdef CONFIG_MTD_OF_PARTS
>         if (!of_device_is_available(np))
> @@ -1014,6 +1050,21 @@ static int m25p_probe(struct spi_device *spi)
>         else
>                 flash->read_opcode = OPCODE_NORM_READ;
>
> +       /* Try to enable the Quad Read */
> +       if (np && of_property_read_bool(np, "m25p,quad-read")) {
> +               /* The configuration register is set by the second byte. */
> +               sr_cr = CR_QUAD << 8;
> +
> +               /* Write the QUAD bit to the Configuration Register. */
> +               write_enable(flash);
> +               if (write_sr_cr(flash, sr_cr) == 0) {
> +                       /* read back and check it */
> +                       ret = read_cr(flash);
> +                       if (ret > 0 && (ret & CR_QUAD))
> +                               flash->read_opcode = OPCODE_QIOR;
> +               }
> +       }
> +

Well, M25p80.c support lots of flash devices, so driver should be as
general as possible. Firstly not all the devices m25p80 supports set
quad mode as your sequence, perhaps write_sr_cr can not match all the
m25p80 flash. Secondly, why you only support QIOR(high performance)
not QOR or DOR. Maybe QIOR seems too special, so what if user want to
use QOR if he set quad mode in DTS.

Another point, if command changed to OPCODE_QIOR, there should also
should be some correct in m25p_read. such as the number of dummy data.
QIOR can support read without read command if set the certain bit in
transfer, these aspects did not reflect in your patch.

>         flash->program_opcode = OPCODE_PP;
>
>         if (info->addr_width)
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index b420a5b..d5b189d 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -39,6 +39,9 @@
>
>  /* Used for Spansion flashes only. */
>  #define        OPCODE_BRWR             0x17    /* Bank register write */
> +#define OPCODE_QIOR            0xeb    /* Quad read */
> +#define OPCODE_4QIOR           0xec    /* Quad read */
> +#define        OPCODE_RDCR             0x35    /* Read configuration register */
>
>  /* Status Register bits. */
>  #define        SR_WIP                  1       /* Write in progress */
> @@ -49,4 +52,7 @@
>  #define        SR_BP2                  0x10    /* Block protect 2 */
>  #define        SR_SRWD                 0x80    /* SR write protect */
>
> +/* Configuration Register bits. */
> +#define CR_QUAD                        0x2     /* Quad I/O */
> +
>  #endif /* __LINUX_MTD_SPI_NOR_H */
> --
> 1.7.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Huang Shijie - Aug. 23, 2013, 9:25 a.m.
于 2013年08月23日 17:05, yuhang wang 写道:
>> +       u16 sr_cr;
>> >  +       int ret;
>> >
>> >    #ifdef CONFIG_MTD_OF_PARTS
>> >           if (!of_device_is_available(np))
>> >  @@ -1014,6 +1050,21 @@ static int m25p_probe(struct spi_device *spi)
>> >           else
>> >                   flash->read_opcode = OPCODE_NORM_READ;
>> >
>> >  +       /* Try to enable the Quad Read */
>> >  +       if (np&&  of_property_read_bool(np, "m25p,quad-read")) {
>> >  +               /* The configuration register is set by the second byte. */
>> >  +               sr_cr = CR_QUAD<<  8;
>> >  +
>> >  +               /* Write the QUAD bit to the Configuration Register. */
>> >  +               write_enable(flash);
>> >  +               if (write_sr_cr(flash, sr_cr) == 0) {
>> >  +                       /* read back and check it */
>> >  +                       ret = read_cr(flash);
>> >  +                       if (ret>  0&&  (ret&  CR_QUAD))
>> >  +                               flash->read_opcode = OPCODE_QIOR;
>> >  +               }
>> >  +       }
>> >  +
> Well, M25p80.c support lots of flash devices, so driver should be as
> general as possible. Firstly not all the devices m25p80 supports set
> quad mode as your sequence, perhaps write_sr_cr can not match all the
It does not matter the NOR flash supports the write_sr_cr() or not,
If the NOR flash does not support the write_sr_cr(), it may fails, and 
you will not set the OPCODE_QIOR for the
m25p80_read.

> m25p80 flash. Secondly, why you only support QIOR(high performance)
> not QOR or DOR. Maybe QIOR seems too special, so what if user want to
> use QOR if he set quad mode in DTS.
>
Frankly speaking, i am reluctant to support the QIOR, it is a little 
slow. :)

So the the QIOR is lowest speed for QUADSPI controller, and i do not 
want to support the DOR.

In my new version, i add the support for DDR QIOR read which is the 
double rate of the QIOR.

The user should knows if the NOR flash supports the quad-read or not, 
and set the proper DT.

> Another point, if command changed to OPCODE_QIOR, there should also
> should be some correct in m25p_read. such as the number of dummy data.
I only need to change the read opcode.
> QIOR can support read without read command if set the certain bit in
> transfer, these aspects did not reflect in your patch.
>
For the Quadspi, it will handle the dummy by the LUT sequence, such as 
DDR QUAD read, the LUT sequence will
set proper dummy (6 cycles for S25FL128S). I do not need the m25p_read 
to set the dummy.


thanks
Huang Shijie
Mark Brown - Aug. 23, 2013, 9:41 a.m.
On Fri, Aug 23, 2013 at 01:58:05AM +0200, Marek Vasut wrote:

> > I was actually thinking something more generic than that - putting the
> > property at the SPI generic bindings level.  Though if all flashes with
> > this dual/quad read functionality have the prefix m25p the above would
> > work also, at the minute this does seem to be mostly used by flash (I
> > bet someone's got some DSPs or something though).

> Ah! So you mean the SPI controller would provide information that it can do 
> dual/quad transfers? But then, the additional pins can only be wired to certain 
> chips (controller by certain CS lines).

No, not exactly - I just meant that the property on the child node
should be one that's consistent over all chips and could hopefully be
implemented in the SPI core as part of instantiating the device in DT.
Which probably just means stripping or changing the vendor prefix.
Poddar, Sourav - Aug. 23, 2013, 9:57 a.m.
On Friday 23 August 2013 02:55 PM, Huang Shijie wrote:
> 于 2013年08月23日 17:05, yuhang wang 写道:
>>> +       u16 sr_cr;
>>> >  +       int ret;
>>> >
>>> >    #ifdef CONFIG_MTD_OF_PARTS
>>> >           if (!of_device_is_available(np))
>>> >  @@ -1014,6 +1050,21 @@ static int m25p_probe(struct spi_device *spi)
>>> >           else
>>> >                   flash->read_opcode = OPCODE_NORM_READ;
>>> >
>>> >  +       /* Try to enable the Quad Read */
>>> >  +       if (np&&  of_property_read_bool(np, "m25p,quad-read")) {
>>> >  +               /* The configuration register is set by the 
>>> second byte. */
>>> >  +               sr_cr = CR_QUAD<<  8;
>>> >  +
>>> >  +               /* Write the QUAD bit to the Configuration 
>>> Register. */
>>> >  +               write_enable(flash);
>>> >  +               if (write_sr_cr(flash, sr_cr) == 0) {
>>> >  +                       /* read back and check it */
>>> >  +                       ret = read_cr(flash);
>>> >  +                       if (ret>  0&&  (ret&  CR_QUAD))
>>> >  +                               flash->read_opcode = OPCODE_QIOR;
>>> >  +               }
>>> >  +       }
>>> >  +
>> Well, M25p80.c support lots of flash devices, so driver should be as
>> general as possible. Firstly not all the devices m25p80 supports set
>> quad mode as your sequence, perhaps write_sr_cr can not match all the
> It does not matter the NOR flash supports the write_sr_cr() or not,
> If the NOR flash does not support the write_sr_cr(), it may fails, and 
> you will not set the OPCODE_QIOR for the
> m25p80_read.
>
>> m25p80 flash. Secondly, why you only support QIOR(high performance)
>> not QOR or DOR. Maybe QIOR seems too special, so what if user want to
>> use QOR if he set quad mode in DTS.
>>
> Frankly speaking, i am reluctant to support the QIOR, it is a little 
> slow. :)
>
You should add QOR opcodes also in your patch, so we have the complete set.
> So the the QIOR is lowest speed for QUADSPI controller, and i do not 
> want to support the DOR.
>
> In my new version, i add the support for DDR QIOR read which is the 
> double rate of the QIOR.
>
> The user should knows if the NOR flash supports the quad-read or not, 
> and set the proper DT.
>
>> Another point, if command changed to OPCODE_QIOR, there should also
>> should be some correct in m25p_read. such as the number of dummy data.
> I only need to change the read opcode.
>> QIOR can support read without read command if set the certain bit in
>> transfer, these aspects did not reflect in your patch.
>>
> For the Quadspi, it will handle the dummy by the LUT sequence, such as 
> DDR QUAD read, the LUT sequence will
> set proper dummy (6 cycles for S25FL128S). I do not need the m25p_read 
> to set the dummy.
>
>
> thanks
> Huang Shijie
>
>
>
>
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Marek Vasut - Aug. 23, 2013, 10:42 a.m.
Dear Mark Brown,

> On Fri, Aug 23, 2013 at 01:58:05AM +0200, Marek Vasut wrote:
> > > I was actually thinking something more generic than that - putting the
> > > property at the SPI generic bindings level.  Though if all flashes with
> > > this dual/quad read functionality have the prefix m25p the above would
> > > work also, at the minute this does seem to be mostly used by flash (I
> > > bet someone's got some DSPs or something though).
> > 
> > Ah! So you mean the SPI controller would provide information that it can
> > do dual/quad transfers? But then, the additional pins can only be wired
> > to certain chips (controller by certain CS lines).
> 
> No, not exactly - I just meant that the property on the child node
> should be one that's consistent over all chips and could hopefully be
> implemented in the SPI core as part of instantiating the device in DT.
> Which probably just means stripping or changing the vendor prefix.

Ah right, got you now. Thanks!

Best regards,
Marek Vasut
Brian Norris - Aug. 23, 2013, 11:23 a.m.
On 08/22/2013 12:55 PM, Mark Brown wrote:
> On Thu, Aug 22, 2013 at 12:34:53PM -0700, Brian Norris wrote:
>> What you're really missing from device-tree (and the SPI subystem in
>> general) is how to detect those SPI controllers which support dual and
>> quad mode transfers, and how to communicate that a particular SPI
>> transaction should be performed on 1 or 4 IO lines. We shouldn't have
>> this just hacked in via assumptions.
>
> That bit does need to be fixed in this driver, yes.  The SPI core now
> has quad mode support.

Where? Perhaps I'm missing the obvious, but I don't see SPI core quad 
support in linux-next or in Linus' tree.

Brian
Poddar, Sourav - Aug. 23, 2013, 11:27 a.m.
Hi,
On Friday 23 August 2013 04:53 PM, Brian Norris wrote:
> On 08/22/2013 12:55 PM, Mark Brown wrote:
>> On Thu, Aug 22, 2013 at 12:34:53PM -0700, Brian Norris wrote:
>>> What you're really missing from device-tree (and the SPI subystem in
>>> general) is how to detect those SPI controllers which support dual and
>>> quad mode transfers, and how to communicate that a particular SPI
>>> transaction should be performed on 1 or 4 IO lines. We shouldn't have
>>> this just hacked in via assumptions.
>>
>> That bit does need to be fixed in this driver, yes.  The SPI core now
>> has quad mode support.
>
> Where? Perhaps I'm missing the obvious, but I don't see SPI core quad 
> support in linux-next or in Linus' tree.
>
You can find it here in Mark's tree:
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
branch: remotes/brownspi/spi-next
> Brian
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Mark Brown - Aug. 23, 2013, 11:30 a.m.
On Fri, Aug 23, 2013 at 04:23:44AM -0700, Brian Norris wrote:
> On 08/22/2013 12:55 PM, Mark Brown wrote:

> >That bit does need to be fixed in this driver, yes.  The SPI core now
> >has quad mode support.

> Where? Perhaps I'm missing the obvious, but I don't see SPI core
> quad support in linux-next or in Linus' tree.

It went in yesterday, -next doesn't seem to have been updated today (I
suspect Stephen may be on vacation, I'd need to check his announcement
mail from yesterday to confirm).  The SPI tree is:

  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Brian Norris - Aug. 23, 2013, 11:46 a.m.
On 08/23/2013 02:41 AM, Mark Brown wrote:
> On Fri, Aug 23, 2013 at 01:58:05AM +0200, Marek Vasut wrote:
>>> I was actually thinking something more generic than that - putting the
>>> property at the SPI generic bindings level.  Though if all flashes with
>>> this dual/quad read functionality have the prefix m25p the above would
>>> work also, at the minute this does seem to be mostly used by flash (I
>>> bet someone's got some DSPs or something though).
> 
>> Ah! So you mean the SPI controller would provide information that it can do
>> dual/quad transfers? But then, the additional pins can only be wired to certain
>> chips (controller by certain CS lines).
> 
> No, not exactly - I just meant that the property on the child node
> should be one that's consistent over all chips and could hopefully be
> implemented in the SPI core as part of instantiating the device in DT.
> Which probably just means stripping or changing the vendor prefix.

(Now that I've been pointed to the support merged into the SPI tree...)

Aren't the following new DT properties (for the SPI slave) sufficient?

spi-rx-nbits
spi-tx-nbits

We can leave the detection of which flash chips support which modes to 
software (m25p80.c) where it belongs, IMO.

They're already in the following commit:

commit f477b7fb13df2b843997559ff34e87d054ba6538
Author: wangyuhang <wangyuhang2014@gmail.com>
Date:   Sun Aug 11 18:15:17 2013 +0800

    spi: DUAL and QUAD support
    
    fix the previous patch some mistake below:
    1. DT in slave node, use "spi-tx-nbits = <1/2/4>" in place of using
       "spi-tx-dual, spi-tx-quad" directly, same to rx. So correct the
       previous way to get the property in @of_register_spi_devices().
    2. Change the value of transfer bit macro(SPI_NBITS_SINGLE, SPI_NBITS_DUAL
       SPI_NBITS_QUAD) to 0x01, 0x02 and 0x04 to match the actual wires.
    3. Add the following check
       (1)keep the tx_nbits and rx_nbits in spi_transfer is not beyond the
          single, dual and quad.
       (2)keep tx_nbits and rx_nbits are contained by @spi_device->mode
          example: if @spi_device->mode = DUAL, then tx/rx_nbits can not be set
                   to QUAD(SPI_NBITS_QUAD)
       (3)if "@spi_device->mode & SPI_3WIRE", then tx/rx_nbits should be in
          single(SPI_NBITS_SINGLE)
    
    Signed-off-by: wangyuhang <wangyuhang2014@gmail.com>
    Signed-off-by: Mark Brown <broonie@linaro.org>

Brian
Brian Norris - Aug. 23, 2013, 11:53 a.m.
On 08/23/2013 04:46 AM, Brian Norris wrote:
> (Now that I've been pointed to the support merged into the SPI tree...)
>
> Aren't the following new DT properties (for the SPI slave) sufficient?
>
> spi-rx-nbits
> spi-tx-nbits
...
> They're already in the following commit:
>
> commit f477b7fb13df2b843997559ff34e87d054ba6538
> Author: wangyuhang <wangyuhang2014@gmail.com>
> Date:   Sun Aug 11 18:15:17 2013 +0800
>
>      spi: DUAL and QUAD support
>
>      fix the previous patch some mistake below:
>      1. DT in slave node, use "spi-tx-nbits = <1/2/4>" in place of using
>         "spi-tx-dual, spi-tx-quad" directly, same to rx. So correct the
>         previous way to get the property in @of_register_spi_devices().
>      2. Change the value of transfer bit macro(SPI_NBITS_SINGLE, SPI_NBITS_DUAL
>         SPI_NBITS_QUAD) to 0x01, 0x02 and 0x04 to match the actual wires.
>      3. Add the following check
>         (1)keep the tx_nbits and rx_nbits in spi_transfer is not beyond the
>            single, dual and quad.
>         (2)keep tx_nbits and rx_nbits are contained by @spi_device->mode
>            example: if @spi_device->mode = DUAL, then tx/rx_nbits can not be set
>                     to QUAD(SPI_NBITS_QUAD)
>         (3)if "@spi_device->mode & SPI_3WIRE", then tx/rx_nbits should be in
>            single(SPI_NBITS_SINGLE)
>
>      Signed-off-by: wangyuhang <wangyuhang2014@gmail.com>
>      Signed-off-by: Mark Brown <broonie@linaro.org>

Speaking of which, the new device-tree properties are not documented in 
Documentation/devicetree/bindings/spi/spi-bus.txt yet.

Brian
Mark Brown - Aug. 23, 2013, 12:01 p.m.
On Fri, Aug 23, 2013 at 04:53:09AM -0700, Brian Norris wrote:
> On 08/23/2013 04:46 AM, Brian Norris wrote:

> >     Signed-off-by: wangyuhang <wangyuhang2014@gmail.com>
> >     Signed-off-by: Mark Brown <broonie@linaro.org>

> Speaking of which, the new device-tree properties are not documented
> in Documentation/devicetree/bindings/spi/spi-bus.txt yet.

Yes, indeed - if you look at the review you'll see I asked for a
followup patch doing that.
王宇航 - Aug. 23, 2013, 1:20 p.m.
Hi,

2013/8/23 Brian Norris <computersforpeace@gmail.com>:
> On 08/23/2013 04:46 AM, Brian Norris wrote:
>>
>> (Now that I've been pointed to the support merged into the SPI tree...)
>>
>> Aren't the following new DT properties (for the SPI slave) sufficient?
>>
>> spi-rx-nbits
>> spi-tx-nbits
>
> ...
>
>> They're already in the following commit:
>>
>> commit f477b7fb13df2b843997559ff34e87d054ba6538
>> Author: wangyuhang <wangyuhang2014@gmail.com>
>> Date:   Sun Aug 11 18:15:17 2013 +0800
>>
>>      spi: DUAL and QUAD support
>>
>>      fix the previous patch some mistake below:
>>      1. DT in slave node, use "spi-tx-nbits = <1/2/4>" in place of using
>>         "spi-tx-dual, spi-tx-quad" directly, same to rx. So correct the
>>         previous way to get the property in @of_register_spi_devices().
>>      2. Change the value of transfer bit macro(SPI_NBITS_SINGLE,
>> SPI_NBITS_DUAL
>>         SPI_NBITS_QUAD) to 0x01, 0x02 and 0x04 to match the actual wires.
>>      3. Add the following check
>>         (1)keep the tx_nbits and rx_nbits in spi_transfer is not beyond
>> the
>>            single, dual and quad.
>>         (2)keep tx_nbits and rx_nbits are contained by @spi_device->mode
>>            example: if @spi_device->mode = DUAL, then tx/rx_nbits can not
>> be set
>>                     to QUAD(SPI_NBITS_QUAD)
>>         (3)if "@spi_device->mode & SPI_3WIRE", then tx/rx_nbits should be
>> in
>>            single(SPI_NBITS_SINGLE)
>>
>>      Signed-off-by: wangyuhang <wangyuhang2014@gmail.com>
>>      Signed-off-by: Mark Brown <broonie@linaro.org>
>
>
> Speaking of which, the new device-tree properties are not documented in
> Documentation/devicetree/bindings/spi/spi-bus.txt yet.
>
> Brian

Sorry, because I don't have the entire environment in my own PC, so I
will update the document patch as soon as possible when I go back to
company.
王宇航 - Aug. 23, 2013, 1:59 p.m.
Hi, Shijie

2013/8/23 Huang Shijie <b32955@freescale.com>:
> 于 2013年08月23日 17:05, yuhang wang 写道:
>>>
>>> +       u16 sr_cr;
>>> >  +       int ret;
>>> >
>>> >    #ifdef CONFIG_MTD_OF_PARTS
>>> >           if (!of_device_is_available(np))
>>> >  @@ -1014,6 +1050,21 @@ static int m25p_probe(struct spi_device *spi)
>>> >           else
>>> >                   flash->read_opcode = OPCODE_NORM_READ;
>>> >
>>> >  +       /* Try to enable the Quad Read */
>>> >  +       if (np&&  of_property_read_bool(np, "m25p,quad-read")) {
>>>
>>> >  +               /* The configuration register is set by the second
>>> > byte. */
>>> >  +               sr_cr = CR_QUAD<<  8;
>>> >  +
>>> >  +               /* Write the QUAD bit to the Configuration Register.
>>> > */
>>> >  +               write_enable(flash);
>>> >  +               if (write_sr_cr(flash, sr_cr) == 0) {
>>> >  +                       /* read back and check it */
>>> >  +                       ret = read_cr(flash);
>>> >  +                       if (ret>  0&&  (ret&  CR_QUAD))
>>>
>>> >  +                               flash->read_opcode = OPCODE_QIOR;
>>> >  +               }
>>> >  +       }
>>> >  +
>>
>> Well, M25p80.c support lots of flash devices, so driver should be as
>> general as possible. Firstly not all the devices m25p80 supports set
>> quad mode as your sequence, perhaps write_sr_cr can not match all the
>
> It does not matter the NOR flash supports the write_sr_cr() or not,
> If the NOR flash does not support the write_sr_cr(), it may fails, and you
> will not set the OPCODE_QIOR for the
> m25p80_read.
>
>
So your purpose of the patch is to make m25p80 support quad read or
just support QIOR? if it's the previous one, when set quad support in
DT, but it is possible that quad mode set failed and m25p80 driver
still read in single mode. In such case, user won't get any error
message, so user won't know  what transfer mode the flash works in. Or
you just aimed to support QIOR, so the name in DT(quad read) seems not
appropriate.

>> m25p80 flash. Secondly, why you only support QIOR(high performance)
>> not QOR or DOR. Maybe QIOR seems too special, so what if user want to
>> use QOR if he set quad mode in DTS.
>>
> Frankly speaking, i am reluctant to support the QIOR, it is a little slow.
> :)
>
> So the the QIOR is lowest speed for QUADSPI controller, and i do not want to
> support the DOR.
>
> In my new version, i add the support for DDR QIOR read which is the double
> rate of the QIOR.
>
> The user should knows if the NOR flash supports the quad-read or not, and
> set the proper DT.
>
>
It is slow in your spi system, but to m25p80 it should be general.
Maybe some others will use that function. So I think it is better to
supplement the other operations.

>> Another point, if command changed to OPCODE_QIOR, there should also
>> should be some correct in m25p_read. such as the number of dummy data.
>
> I only need to change the read opcode.
>
>> QIOR can support read without read command if set the certain bit in
>> transfer, these aspects did not reflect in your patch.
>>
> For the Quadspi, it will handle the dummy by the LUT sequence, such as DDR
> QUAD read, the LUT sequence will
> set proper dummy (6 cycles for S25FL128S). I do not need the m25p_read to
> set the dummy.
>
>
Also the same point to above, it is OK to your spi controller, but
your current m25p80 patch can not content others. If I don't have the
SPI controller which support LUT sequence, so my spi controller driver
rely on the info that m25p80 provides, then your patch won't work.

Best regards
Poddar, Sourav - Aug. 23, 2013, 3:59 p.m.
On Saturday 24 August 2013 08:15 AM, Huang Shijie wrote:
> On Fri, Aug 23, 2013 at 03:27:43PM +0530, Sourav Poddar wrote:
>> On Friday 23 August 2013 02:55 PM, Huang Shijie wrote:
>>> 于 2013年08月23日 17:05, yuhang wang 写道:
>>>
>> You should add QOR opcodes also in your patch, so we have the complete set.
> yes, i have already supported the QIOR opcode in my driver.
>
I meant opcode 0x6b Quad output read which spansion flash supports.
> thanks
> Huang Shijie
Huang Shijie - Aug. 24, 2013, 2:45 a.m.
On Fri, Aug 23, 2013 at 03:27:43PM +0530, Sourav Poddar wrote:
> On Friday 23 August 2013 02:55 PM, Huang Shijie wrote:
> >于 2013年08月23日 17:05, yuhang wang 写道:
> >
> You should add QOR opcodes also in your patch, so we have the complete set.

yes, i have already supported the QIOR opcode in my driver.

thanks
Huang Shijie
Huang Shijie - Aug. 24, 2013, 3:01 a.m.
On Fri, Aug 23, 2013 at 09:59:09PM +0800, yuhang wang wrote:
> Hi, Shijie
> 
> >>> >  +
> >>
> >> Well, M25p80.c support lots of flash devices, so driver should be as
> >> general as possible. Firstly not all the devices m25p80 supports set
> >> quad mode as your sequence, perhaps write_sr_cr can not match all the
> >
> > It does not matter the NOR flash supports the write_sr_cr() or not,
> > If the NOR flash does not support the write_sr_cr(), it may fails, and you
> > will not set the OPCODE_QIOR for the
> > m25p80_read.
> >
> >
> So your purpose of the patch is to make m25p80 support quad read or
> just support QIOR? if it's the previous one, when set quad support in

The patch makes the m25p80 could supports the Quad read.
it is okay if the quad-read mode set fails.

> DT, but it is possible that quad mode set failed and m25p80 driver
> still read in single mode. In such case, user won't get any error
For the Quadspi driver, if the quad read mode set failed, it will still
run in the Fast Read mode.

> message, so user won't know  what transfer mode the flash works in. Or

do you need a warning when the quad read set fails?

> you just aimed to support QIOR, so the name in DT(quad read) seems not
> appropriate.

sorry, could you explain a little more? how can make the DT seems more
appropriate ?

thanks

> 
> >> m25p80 flash. Secondly, why you only support QIOR(high performance)
> >> not QOR or DOR. Maybe QIOR seems too special, so what if user want to
> >> use QOR if he set quad mode in DTS.
> >>
> > Frankly speaking, i am reluctant to support the QIOR, it is a little slow.
> > :)
> >
> > So the the QIOR is lowest speed for QUADSPI controller, and i do not want to
> > support the DOR.
> >
> > In my new version, i add the support for DDR QIOR read which is the double
> > rate of the QIOR.
> >
> > The user should knows if the NOR flash supports the quad-read or not, and
> > set the proper DT.
> >
> >
> It is slow in your spi system, but to m25p80 it should be general.
> Maybe some others will use that function. So I think it is better to
> supplement the other operations.

it is the other driver's responsibility to add the bits info or the
dummy info to the m25p80 code.


> 
> >> Another point, if command changed to OPCODE_QIOR, there should also
> >> should be some correct in m25p_read. such as the number of dummy data.
> >
> > I only need to change the read opcode.
> >
> >> QIOR can support read without read command if set the certain bit in
> >> transfer, these aspects did not reflect in your patch.
> >>
> > For the Quadspi, it will handle the dummy by the LUT sequence, such as DDR
> > QUAD read, the LUT sequence will
> > set proper dummy (6 cycles for S25FL128S). I do not need the m25p_read to
> > set the dummy.
> >
> >
> Also the same point to above, it is OK to your spi controller, but
> your current m25p80 patch can not content others. If I don't have the
> SPI controller which support LUT sequence, so my spi controller driver
> rely on the info that m25p80 provides, then your patch won't work.

dido.

you can submit a patch to fix this issue if your SPI controller can not
works on this patch.

It is over-design for this patch set to add the bits info/dummy info to
the m25p80 code. 

thanks
Huang Shijie

Patch

diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
index 6d3d576..b33313f 100644
--- a/Documentation/devicetree/bindings/mtd/m25p80.txt
+++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
@@ -17,6 +17,11 @@  Optional properties:
                    Refer to your chips' datasheet to check if this is supported
                    by your chip.
 
+- m25p,quad-read : Use the "quad read" opcode to read data from the chip instead
+                   of the usual "read" opcode. This opcode is not supported by
+                   all chips and support for it can not be detected at runtime.
+                   Refer to your chips' datasheet to check if this is supported
+                   by your chip.
 Example:
 
 	flash: m25p80@0 {
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index f3598c1..4bc9b1b 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -103,6 +103,40 @@  static int write_sr(struct m25p *flash, u8 val)
 }
 
 /*
+ * Read the configuration register, returning its value in the location
+ * Return the configuration register value.
+ * Returns negative if error occurred.
+ */
+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 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.
+ * Returns negative if error occurred.
+ */
+static int write_sr_cr(struct m25p *flash, u16 val)
+{
+	flash->command[0] = OPCODE_WRSR;
+	flash->command[1] = 0;
+	flash->command[2] = (val >> 8);
+
+	return spi_write(flash->spi, flash->command, 3);
+}
+
+/*
  * Set write enable latch with Write Enable command.
  * Returns negative if error occurred.
  */
@@ -880,6 +914,8 @@  static int m25p_probe(struct spi_device *spi)
 	unsigned			i;
 	struct mtd_part_parser_data	ppdata;
 	struct device_node __maybe_unused *np = spi->dev.of_node;
+	u16 sr_cr;
+	int ret;
 
 #ifdef CONFIG_MTD_OF_PARTS
 	if (!of_device_is_available(np))
@@ -1014,6 +1050,21 @@  static int m25p_probe(struct spi_device *spi)
 	else
 		flash->read_opcode = OPCODE_NORM_READ;
 
+	/* Try to enable the Quad Read */
+	if (np && of_property_read_bool(np, "m25p,quad-read")) {
+		/* The configuration register is set by the second byte. */
+		sr_cr = CR_QUAD << 8;
+
+		/* Write the QUAD bit to the Configuration Register. */
+		write_enable(flash);
+		if (write_sr_cr(flash, sr_cr) == 0) {
+			/* read back and check it */
+			ret = read_cr(flash);
+			if (ret > 0 && (ret & CR_QUAD))
+				flash->read_opcode = OPCODE_QIOR;
+		}
+	}
+
 	flash->program_opcode = OPCODE_PP;
 
 	if (info->addr_width)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index b420a5b..d5b189d 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -39,6 +39,9 @@ 
 
 /* Used for Spansion flashes only. */
 #define	OPCODE_BRWR		0x17	/* Bank register write */
+#define OPCODE_QIOR		0xeb	/* Quad read */
+#define OPCODE_4QIOR		0xec	/* Quad read */
+#define	OPCODE_RDCR		0x35	/* Read configuration register */
 
 /* Status Register bits. */
 #define	SR_WIP			1	/* Write in progress */
@@ -49,4 +52,7 @@ 
 #define	SR_BP2			0x10	/* Block protect 2 */
 #define	SR_SRWD			0x80	/* SR write protect */
 
+/* Configuration Register bits. */
+#define CR_QUAD			0x2	/* Quad I/O */
+
 #endif /* __LINUX_MTD_SPI_NOR_H */