Patchwork SPI: DUAL/QUAD support

login
register
mail settings
Submitter 王宇航
Date July 4, 2013, 11:36 a.m.
Message ID <CAHSAbzPNDSr_6N79fh0DTWSEg8buRRPyiw8SPodtKLPjzWXJsg@mail.gmail.com>
Download mbox | patch
Permalink /patch/256879/
State New
Headers show

Comments

王宇航 - July 4, 2013, 11:36 a.m.
Hi Mark,

Thanks for your reply.
I have added the kerneldoc into the patch below to explain the "DUAL"
and "QUAD" modes.
Hope for your suggestions.

Documentation/spi/spi-dual_quad |  102 +++++++++++++++++++++++++++++++++++++++
 drivers/mtd/devices/m25p80.c    |    2 +
 drivers/spi/spi.c               |    2 +
 include/linux/spi/spi.h         |    8 +++
 4 files changed, 114 insertions(+)
 create mode 100644 Documentation/spi/spi-dual_quad


--
1.7.9.5

Best regards
Johannes Stezenbach - July 4, 2013, 1 p.m.
On Thu, Jul 04, 2013 at 07:36:48PM +0800, yuhang wang wrote:
> 
> I have added the kerneldoc into the patch below to explain the "DUAL"
> and "QUAD" modes.
> Hope for your suggestions.
> 
> Documentation/spi/spi-dual_quad |  102 +++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/devices/m25p80.c    |    2 +
>  drivers/spi/spi.c               |    2 +
>  include/linux/spi/spi.h         |    8 +++

I think what Mark meant is to add new fields to the /** comments
in include/linux/spi/spi.h, then documentation could be generated
(but it seems only outdated copies are online, everyone prefers
to look at the code):
https://www.kernel.org/doc/htmldocs/device-drivers/

> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -354,6 +354,7 @@ static int m25p80_read(struct mtd_info *mtd,
> loff_t from, size_t len,
> 
>         t[1].rx_buf = buf;
>         t[1].len = len;
> +       t[1].bitwidth = flash->spi->rx_bitwidth;
>         spi_message_add_tail(&t[1], &m);
> 
>         mutex_lock(&flash->lock);
> @@ -409,6 +410,7 @@ static int m25p80_write(struct mtd_info *mtd,
> loff_t to, size_t len,
>         spi_message_add_tail(&t[0], &m);
> 
>         t[1].tx_buf = buf;
> +       t[1].bitwidth = flash->spi->tx_bitwidth;
>         spi_message_add_tail(&t[1], &m);
> 
>         mutex_lock(&flash->lock);

Doesn't m25p80 need to use different commands (OPCODEs)
for 2x and 4x speed?  It seems this change is incomplete.

Thanks,
Johannes
Mark Brown - July 4, 2013, 2:36 p.m.
On Thu, Jul 04, 2013 at 07:36:48PM +0800, yuhang wang wrote:
> Hi Mark,
> 
> Thanks for your reply.
> I have added the kerneldoc into the patch below to explain the "DUAL"
> and "QUAD" modes.
> Hope for your suggestions.

Please submit things in the form covered in SubmittingPatches -
especially you need to sign off anything you're submitting to the
standard kernel.

> +Description
> +----------------------
> +DUAL/QUAD means spi can transfer in 2bits/4bits at the same time.
> +These spi controllers provide 8 data lines(4-tx and 4-rx). User can
> +choose tranfer mode(SINGLE/DUAL/QUAD) by setting the certain register.
> +Though SPI is a serial interface, some spi controllers can support
> +transmitting and receiving in DUAL and QUAD modes aimed to improve
> +the performance. Also as spi slave lots of flashes do support this attribute,
> +such as serial-norflash in spansion company.

OK, so all this is about is devices that have extra data lines.  Please
don't invent terms like "DUAL" and "QUAD", it makes things much less
clear.  Just describe it as support for multiple data lines.

> +struct spi_transfer {
> ++       u8     bitwidth;
> ++#define        SPI_BITWIDTH_SINGLE     0x01; /* 1bit transfer */
> ++#define        SPI_BITWIDTH_DUAL       0x02; /* 2bits transfer */
> ++#define        SPI_BITWIDTH_QUAD       0x03; /* 4bits transfer */

Calling this "bandwidth" is really unclear - I would expect a bandwidth
to be expressed in bits per second or similar.  This would be much
clearer if it was just the number of data signals.

> +       t[0].rx_buf = buf;
> +       t[0].len = len;
> +       t[0].bitwidth = spi->rx_bitwidth/spi->tx_bitwidth;
> +       spi_message_add_tail(&t[0], &m);

This interface won't work for bidirectional transfers with asymmetric
numbers of data lines - we need separate fields for rx and rx.

> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -354,6 +354,7 @@ static int m25p80_read(struct mtd_info *mtd,
> loff_t from, size_t len,
> 
>         t[1].rx_buf = buf;
>         t[1].len = len;
> +       t[1].bitwidth = flash->spi->rx_bitwidth;
>         spi_message_add_tail(&t[1], &m);
> 
>         mutex_lock(&flash->lock);

This is going to fail in cases where the extra data lines aren't
available.
Thomas.Betker@rohde-schwarz.com - July 4, 2013, 2:58 p.m.
Hello Johannes:

> Doesn't m25p80 need to use different commands (OPCODEs)
> for 2x and 4x speed?  It seems this change is incomplete.

Yes, there are usually different opcodes, and as far as I know, they are 
not supported by the m25p80 driver. It seems we are missing some 
background info here.

My guess is that the flash chip in question is configured to treat single 
mode opcodes as dual or quad mode commands (some chips support this), and 
that the SPI controller needs to be told, for each transfer, which mode to 
use.

E.g., you would send a READ command with the command bytes in single mode 
(t[0].bitwidth is missing in the patch) and the data bytes in dual or quad 
mode. No dummy bytes are used -- the SPI is running at slower speed.

This situation is certainly a bit special. To me, the patch looks like a 
short-time workaround; I would rather see the flash driver support actual 
dual or quad mode commands. Is there any work in progress?

Best regards,
Thomas
Mark Brown - July 4, 2013, 3:49 p.m.
On Thu, Jul 04, 2013 at 04:58:52PM +0200, Thomas.Betker@rohde-schwarz.com wrote:

> This situation is certainly a bit special. To me, the patch looks like a 
> short-time workaround; I would rather see the flash driver support actual 
> dual or quad mode commands. Is there any work in progress?

Well, there's two bits here.  One is the support in the client drivers
for using the extra data lines and the other is the support in the SPI
core for telling controllers to do this.  We need that second bit in
place before the first one can be implemented.
Thomas.Betker@rohde-schwarz.com - July 4, 2013, 4:04 p.m.
Hello Mark:

> Well, there's two bits here.  One is the support in the client drivers
> for using the extra data lines and the other is the support in the SPI
> core for telling controllers to do this.  We need that second bit in
> place before the first one can be implemented.

I am not sure if we do. In the system I am working on, the QSPI controller 
recognizes the quad mode commands from the Tx data and automatically 
switches modes; no manual intervention by the flash driver is needed.

I agree that this may not be the case for all SPI controllers. Yuhang, can 
you tell us what your controller will do?

Best regards,
Thomas
Johannes Stezenbach - July 4, 2013, 6:06 p.m.
On Thu, Jul 04, 2013 at 03:36:45PM +0100, Mark Brown wrote:
> On Thu, Jul 04, 2013 at 07:36:48PM +0800, yuhang wang wrote:
> > +Description
> > +----------------------
> > +DUAL/QUAD means spi can transfer in 2bits/4bits at the same time.
> > +These spi controllers provide 8 data lines(4-tx and 4-rx). User can
> > +choose tranfer mode(SINGLE/DUAL/QUAD) by setting the certain register.
> > +Though SPI is a serial interface, some spi controllers can support
> > +transmitting and receiving in DUAL and QUAD modes aimed to improve
> > +the performance. Also as spi slave lots of flashes do support this attribute,
> > +such as serial-norflash in spansion company.
> 
> OK, so all this is about is devices that have extra data lines.  Please
> don't invent terms like "DUAL" and "QUAD", it makes things much less
> clear.  Just describe it as support for multiple data lines.

Flash data sheets use the terms Dual and Quad I/O Mode,
e.g. for MX25L25635E
http://www.macronix.com/QuickPlace/hq/PageLibrary4825740B00298A3B.nsf/h_Index/6F878CF760C559BD482576E00022E6CC/?OpenDocument&EPN=MX25L25635E

> > +struct spi_transfer {
> > ++       u8     bitwidth;
> > ++#define        SPI_BITWIDTH_SINGLE     0x01; /* 1bit transfer */
> > ++#define        SPI_BITWIDTH_DUAL       0x02; /* 2bits transfer */
> > ++#define        SPI_BITWIDTH_QUAD       0x03; /* 4bits transfer */
> 
> Calling this "bandwidth" is really unclear - I would expect a bandwidth
> to be expressed in bits per second or similar.  This would be much
> clearer if it was just the number of data signals.

"bitwidth" != "bandwidth", but maybe the name could be improved,
how about xfer_width?  The mmc susbsysem has something similar
and calls it bus_width.

> > +       t[0].rx_buf = buf;
> > +       t[0].len = len;
> > +       t[0].bitwidth = spi->rx_bitwidth/spi->tx_bitwidth;
> > +       spi_message_add_tail(&t[0], &m);
> 
> This interface won't work for bidirectional transfers with asymmetric
> numbers of data lines - we need separate fields for rx and rx.

I'm not aware that bidirectional transfers are supported in
dual or quad IO mode.  I think only flash supports it and
uses all available IO lines as either input (flash write
command) or output (flash read command).  I.e. for MX25L25635E flash
it means at least the command, but usually also the address,
are transferred in single IO mode, only then do the flash
and controller switch to parallel IO for the data transfer.


Johannes
Mark Brown - July 4, 2013, 7:12 p.m.
On Thu, Jul 04, 2013 at 08:06:56PM +0200, Johannes Stezenbach wrote:
> On Thu, Jul 04, 2013 at 03:36:45PM +0100, Mark Brown wrote:

> > OK, so all this is about is devices that have extra data lines.  Please
> > don't invent terms like "DUAL" and "QUAD", it makes things much less
> > clear.  Just describe it as support for multiple data lines.

> Flash data sheets use the terms Dual and Quad I/O Mode,
> e.g. for MX25L25635E
> http://www.macronix.com/QuickPlace/hq/PageLibrary4825740B00298A3B.nsf/h_Index/6F878CF760C559BD482576E00022E6CC/?OpenDocument&EPN=MX25L25635E

OK, but this is not a flash specific feature but rather something at the
SPI level and even with flash are they typically written in all caps?
This really needs to be described at the SPI level for the SPI subsystem,
not in terms of the specific application.

> > Calling this "bandwidth" is really unclear - I would expect a bandwidth
> > to be expressed in bits per second or similar.  This would be much
> > clearer if it was just the number of data signals.

> "bitwidth" != "bandwidth", but maybe the name could be improved,
> how about xfer_width?  The mmc susbsysem has something similar
> and calls it bus_width.

Bus width is better but I think half the issue here is the use of
"width" and the fact that this isn't being done separately for RX and
TX, it's really not clear what's being talked about.  xfer_width sounds
to me like it's something to do with the word size so I don't think
that's a good idea.  n_mosi and n_miso perhaps?

> > > +       t[0].rx_buf = buf;
> > > +       t[0].len = len;
> > > +       t[0].bitwidth = spi->rx_bitwidth/spi->tx_bitwidth;
> > > +       spi_message_add_tail(&t[0], &m);

> > This interface won't work for bidirectional transfers with asymmetric
> > numbers of data lines - we need separate fields for rx and rx.

> I'm not aware that bidirectional transfers are supported in
> dual or quad IO mode.  I think only flash supports it and

As far as I can tell this is just because you guys are only thinking in
terms of flash chips here - I can't believe that only flash manufacturers
came up with the idea of adding extra data signals to SPI, it's not that
big a leap (and MMC obviously has some overlap here...).  If nothing
else I'd expect some FPGAs would use this, and I wouldn't be surprised
if there were DSPs that could use this.
王宇航 - July 5, 2013, 6:25 a.m.
Hi Thomas:

>> Well, there's two bits here.  One is the support in the client drivers
>> for using the extra data lines and the other is the support in the SPI
>> core for telling controllers to do this.  We need that second bit in
>> place before the first one can be implemented.
>
> I am not sure if we do. In the system I am working on, the QSPI controller
> recognizes the quad mode commands from the Tx data and automatically
> switches modes; no manual intervention by the flash driver is needed.
>
> I agree that this may not be the case for all SPI controllers. Yuhang, can
> you tell us what your controller will do?

OK, well my spi controller just sets the certain transfer mode in the register
to make it in that mode(single,dual,quad). Then it will send and receive datas
with the specific lines. As master, it should be told the transfer lines by the
slave. Thus I need some members to deliver the information.

But what you said "QSPI controller recognizes the quad mode commands
from the Tx data and automatically switches modes;"  I can not figure out.
The communication is launched by SPI master, so how can the controller
recognizes the Tx data in the first transmission.

>Yes, there are usually different opcodes, and as far as I know, they are
>not supported by the m25p80 driver. It seems we are missing some
>background info here.
>My guess is that the flash chip in question is configured to treat single
>mode opcodes as dual or quad mode commands (some chips support this), and
>that the SPI controller needs to be told, for each transfer, which mode to
>use.
Just as Johannes said. In my patch m25p80's changes is incomplete.
Because my inicial aim is to add the transfer width member to tell controller
the mode flash in. And I will attach new patch about m25p80 later.
Also you are right. There are really series of flashes do not support dual/quad
transfer. So I don't know whether there are any standard for serial-flash just
like the CFI for parallel-flash. Personally, to make a general standard and
a general code for serial-flash is necessary. So that we do not need to add
special function in m25p80.c.

Thanks,
yuhang
pekon gupta - July 5, 2013, 6:45 a.m.
> Just as Johannes said. In my patch m25p80's changes is incomplete.
> Because my inicial aim is to add the transfer width member to tell controller
> the mode flash in. And I will attach new patch about m25p80 later.
> Also you are right. There are really series of flashes do not support dual/quad
> transfer. So I don't know whether there are any standard for serial-flash just
> like the CFI for parallel-flash. Personally, to make a general standard and
> a general code for serial-flash is necessary. So that we do not need to add
> special function in m25p80.c.
> 
[Pekon]: Does below generic framework for spianand, suit your driver?
http://lists.infradead.org/pipermail/linux-mtd/2013-July/047434.html

you should be able to extend the generic spinand API for all
serial modes (SPI, Dual-SPI, Quad-SPI).
As QSPI and Dual-SPI devices should implicitly support SPI (MISO/MOSI)
So, you can do device probing using default SPI mode. And then based on
DT inputs and device support upgrade to QSPI / Dual-SPI mode.
And going forward extend it for direct memory-mapped device for XIP.


with regards, pekon
Johannes Stezenbach - July 5, 2013, 7:35 a.m.
On Fri, Jul 05, 2013 at 06:45:01AM +0000, Gupta, Pekon wrote:
> 
> > Just as Johannes said. In my patch m25p80's changes is incomplete.
> > Because my inicial aim is to add the transfer width member to tell controller
> > the mode flash in. And I will attach new patch about m25p80 later.
> > Also you are right. There are really series of flashes do not support dual/quad
> > transfer. So I don't know whether there are any standard for serial-flash just
> > like the CFI for parallel-flash. Personally, to make a general standard and
> > a general code for serial-flash is necessary. So that we do not need to add
> > special function in m25p80.c.
> > 
> [Pekon]: Does below generic framework for spianand, suit your driver?
> http://lists.infradead.org/pipermail/linux-mtd/2013-July/047434.html
> 
> you should be able to extend the generic spinand API for all
> serial modes (SPI, Dual-SPI, Quad-SPI).
> As QSPI and Dual-SPI devices should implicitly support SPI (MISO/MOSI)
> So, you can do device probing using default SPI mode. And then based on
> DT inputs and device support upgrade to QSPI / Dual-SPI mode.
> And going forward extend it for direct memory-mapped device for XIP.

Not sure what you have in mind since NAND and NOR flash work very
differently, and SPI vs. memory-mapped is also very different.

But to add to the spinand review comments two things caught
my eye on quick glance over the code:

> +#define mu_spi_nand_driver_version "Beagle-MTD_01.00_Linux2.6.33_20100507"

seems like an unused leftover?  if the version number is
important then maybe better put it in commit message?

> +/bin/bash: 4: command not found

how did this get in there?


Johannes
Poddar, Sourav - July 5, 2013, 7:40 a.m.
On Friday 05 July 2013 11:55 AM, yuhang wang wrote:
> Hi Thomas:
>
>>> Well, there's two bits here.  One is the support in the client drivers
>>> for using the extra data lines and the other is the support in the SPI
>>> core for telling controllers to do this.  We need that second bit in
>>> place before the first one can be implemented.
>> I am not sure if we do. In the system I am working on, the QSPI controller
>> recognizes the quad mode commands from the Tx data and automatically
>> switches modes; no manual intervention by the flash driver is needed.
>>
>> I agree that this may not be the case for all SPI controllers. Yuhang, can
>> you tell us what your controller will do?
> OK, well my spi controller just sets the certain transfer mode in the register
> to make it in that mode(single,dual,quad). Then it will send and receive datas
> with the specific lines. As master, it should be told the transfer lines by the
> slave. Thus I need some members to deliver the information.
>

To add to all the discussion going on,
I am using a qspi controller with a spansion flash device.
I tried using quad read mode bit, and what is required to change in
my case is the following:

1. enable quad mode in flash configuration register.
2. change my read opcode to QUAD READ opcode according to flash datasheet.
3. Configure my qspi controller cmd reg to use 6 PIN whenever quad mode 
is used.

So,  I don't need to change things in spi framework to get
quad working. Though, as thomas also pointed out, it might
vary from controller to controller.

> But what you said "QSPI controller recognizes the quad mode commands
> from the Tx data and automatically switches modes;"  I can not figure out.
> The communication is launched by SPI master, so how can the controller
> recognizes the Tx data in the first transmission.
>
>> Yes, there are usually different opcodes, and as far as I know, they are
>> not supported by the m25p80 driver. It seems we are missing some
>> background info here.
>> My guess is that the flash chip in question is configured to treat single
>> mode opcodes as dual or quad mode commands (some chips support this), and
>> that the SPI controller needs to be told, for each transfer, which mode to
>> use.
> Just as Johannes said. In my patch m25p80's changes is incomplete.
> Because my inicial aim is to add the transfer width member to tell controller
> the mode flash in. And I will attach new patch about m25p80 later.
> Also you are right. There are really series of flashes do not support dual/quad
> transfer. So I don't know whether there are any standard for serial-flash just
> like the CFI for parallel-flash. Personally, to make a general standard and
> a general code for serial-flash is necessary. So that we do not need to add
> special function in m25p80.c.
>
> Thanks,
> yuhang
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Poddar, Sourav - July 5, 2013, 7:41 a.m.
On Friday 05 July 2013 01:05 PM, Johannes Stezenbach wrote:
> On Fri, Jul 05, 2013 at 06:45:01AM +0000, Gupta, Pekon wrote:
>>> Just as Johannes said. In my patch m25p80's changes is incomplete.
>>> Because my inicial aim is to add the transfer width member to tell controller
>>> the mode flash in. And I will attach new patch about m25p80 later.
>>> Also you are right. There are really series of flashes do not support dual/quad
>>> transfer. So I don't know whether there are any standard for serial-flash just
>>> like the CFI for parallel-flash. Personally, to make a general standard and
>>> a general code for serial-flash is necessary. So that we do not need to add
>>> special function in m25p80.c.
>>>
>> [Pekon]: Does below generic framework for spianand, suit your driver?
>> http://lists.infradead.org/pipermail/linux-mtd/2013-July/047434.html
>>
>> you should be able to extend the generic spinand API for all
>> serial modes (SPI, Dual-SPI, Quad-SPI).
>> As QSPI and Dual-SPI devices should implicitly support SPI (MISO/MOSI)
>> So, you can do device probing using default SPI mode. And then based on
>> DT inputs and device support upgrade to QSPI / Dual-SPI mode.
>> And going forward extend it for direct memory-mapped device for XIP.
> Not sure what you have in mind since NAND and NOR flash work very
> differently, and SPI vs. memory-mapped is also very different.
>
> But to add to the spinand review comments two things caught
> my eye on quick glance over the code:
>
>> +#define mu_spi_nand_driver_version "Beagle-MTD_01.00_Linux2.6.33_20100507"
> seems like an unused leftover?  if the version number is
> important then maybe better put it in commit message?
>
Yes, actually its not required, should be removed.
>> +/bin/bash: 4: command not found
> how did this get in there?
>
>
Yes, this is useless, will  removed in the next version.
> Johannes
pekon gupta - July 5, 2013, 8:04 a.m.
> > [Pekon]: Does below generic framework for spianand, suit your driver?
> > http://lists.infradead.org/pipermail/linux-mtd/2013-July/047434.html
> >
> > you should be able to extend the generic spinand API for all
> > serial modes (SPI, Dual-SPI, Quad-SPI).
> > As QSPI and Dual-SPI devices should implicitly support SPI (MISO/MOSI)
> > So, you can do device probing using default SPI mode. And then based on
> > DT inputs and device support upgrade to QSPI / Dual-SPI mode.
> > And going forward extend it for direct memory-mapped device for XIP.
> 
> Not sure what you have in mind since NAND and NOR flash work very
> differently, and SPI vs. memory-mapped is also very different.
> 
[Pekon]:  Opps.. Sorry for NAND v/s NOR confusion,
But I found generic framework for SPINAND (not in mainline) helpful
for us, so I pointed that..
I'm not sure about your end use-case, but I our QSPI controller supports
direct memory-addressable cpu-side bus-interface which can be enabled
for XIP support. So pointed out that here as-well..


> But to add to the spinand review comments two things caught
> my eye on quick glance over the code:
> 
> > +#define mu_spi_nand_driver_version "Beagle-
> MTD_01.00_Linux2.6.33_20100507"
> 
> seems like an unused leftover?  if the version number is
> important then maybe better put it in commit message?
> 
> > +/bin/bash: 4: command not found
> 
> how did this get in there?
> 
[Pekon]: Thanks.. Sourav needs to clean this up lot more.

with regards, pekon
王宇航 - July 5, 2013, 8:48 a.m.
> To add to all the discussion going on,
> I am using a qspi controller with a spansion flash device.
> I tried using quad read mode bit, and what is required to change in
> my case is the following:
>
> 1. enable quad mode in flash configuration register.
> 2. change my read opcode to QUAD READ opcode according to flash datasheet.
> 3. Configure my qspi controller cmd reg to use 6 PIN whenever quad mode is
> used.
>

Sorry, to the third point I am still not very clear. The first time,
spi controller should send
QUAD READ opcode to your spansion flash with 1 line, then receive datas from
flash with 4 lines. So how does your controller master this process
automatically.

Please give me some details.

Thanks
Poddar, Sourav - July 5, 2013, 8:55 a.m.
On Friday 05 July 2013 02:18 PM, yuhang wang wrote:
>> To add to all the discussion going on,
>> I am using a qspi controller with a spansion flash device.
>> I tried using quad read mode bit, and what is required to change in
>> my case is the following:
>>
>> 1. enable quad mode in flash configuration register.
>> 2. change my read opcode to QUAD READ opcode according to flash datasheet.
>> 3. Configure my qspi controller cmd reg to use 6 PIN whenever quad mode is
>> used.
>>
> Sorry, to the third point I am still not very clear. The first time,
> spi controller should send
> QUAD READ opcode to your spansion flash with 1 line, then receive datas from
> flash with 4 lines. So how does your controller master this process
> automatically.
>
> Please give me some details.
>
Its like, you pass txbuf and rxbuf from the spi core to your controller
driver. So, if your txbuf is not null then configure the controller
cmd register with WRITE_SINGLE and if rxbuf is not null then
configure the controller cmd register with READ_QUAD.

Something like this,
                 if (txbuf) {
                         dra7xxx_writel_data(qspi, *txbuf++,
                                 QSPI_SPI_DATA_REG, wlen);
                         dra7xxx_writel(qspi, qspi->dc, QSPI_SPI_DC_REG);
                         dra7xxx_writel(qspi, qspi->cmd | QSPI_WR_SNGL,
                                         QSPI_SPI_CMD_REG);
                         wait_for_completion(&qspi->word_complete);
                 }
                 if (rxbuf) {
                         printk("rx cmd %08x dc %08x\n",
                                 qspi->cmd | QSPI_RD_QUAD, qspi->dc);
                         dra7xxx_writel(qspi, INT_EN, 
QSPI_INTR_ENABLE_SET_REG);
                         dra7xxx_writel(qspi, qspi->dc, QSPI_SPI_DC_REG);
                     if (!quad_mode)
                             dra7xxx_writel(qspi, qspi->cmd | QSPI_RD_SNGL,
                                             QSPI_SPI_CMD_REG);
                     else
                             dra7xxx_writel(qspi, qspi->cmd | QSPI_RD_QUAD,
                                             QSPI_SPI_CMD_REG);
                  .......
                 }
> Thanks
王宇航 - July 5, 2013, 9:07 a.m.
>                     if (!quad_mode)
>                             dra7xxx_writel(qspi, qspi->cmd | QSPI_RD_SNGL,
>                                             QSPI_SPI_CMD_REG);
>                     else
>                             dra7xxx_writel(qspi, qspi->cmd | QSPI_RD_QUAD,
>                                             QSPI_SPI_CMD_REG);
>                  .......
>                 }

So what do you based on to set variable quad_mode.
Poddar, Sourav - July 5, 2013, 9:08 a.m.
On Friday 05 July 2013 02:37 PM, yuhang wang wrote:
>>                      if (!quad_mode)
>>                              dra7xxx_writel(qspi, qspi->cmd | QSPI_RD_SNGL,
>>                                              QSPI_SPI_CMD_REG);
>>                      else
>>                              dra7xxx_writel(qspi, qspi->cmd | QSPI_RD_QUAD,
>>                                              QSPI_SPI_CMD_REG);
>>                   .......
>>                  }
> So what do you based on to set variable quad_mode.
Best way should be to check for flash device id and manufacture data. 
Based on which you
can decide whether your flash supports quad bits or not.
王宇航 - July 5, 2013, 9:17 a.m.
2013/7/5 Sourav Poddar <sourav.poddar@ti.com>:
> On Friday 05 July 2013 02:37 PM, yuhang wang wrote:
>>>
>>>                      if (!quad_mode)
>>>                              dra7xxx_writel(qspi, qspi->cmd |
>>> QSPI_RD_SNGL,
>>>                                              QSPI_SPI_CMD_REG);
>>>                      else
>>>                              dra7xxx_writel(qspi, qspi->cmd |
>>> QSPI_RD_QUAD,
>>>                                              QSPI_SPI_CMD_REG);
>>>                   .......
>>>                  }
>>
>> So what do you based on to set variable quad_mode.
>
> Best way should be to check for flash device id and manufacture data. Based
> on which you
> can decide whether your flash supports quad bits or not.

Perhaps I did not said it clearly. Your flash supports quad and you
set it into quad
mode. But how can your controller driver notice that. In other word,
In which way
provide this information from flash to spi controller.
Poddar, Sourav - July 5, 2013, 9:27 a.m.
On Friday 05 July 2013 02:47 PM, yuhang wang wrote:
> 2013/7/5 Sourav Poddar<sourav.poddar@ti.com>:
>> On Friday 05 July 2013 02:37 PM, yuhang wang wrote:
>>>>                       if (!quad_mode)
>>>>                               dra7xxx_writel(qspi, qspi->cmd |
>>>> QSPI_RD_SNGL,
>>>>                                               QSPI_SPI_CMD_REG);
>>>>                       else
>>>>                               dra7xxx_writel(qspi, qspi->cmd |
>>>> QSPI_RD_QUAD,
>>>>                                               QSPI_SPI_CMD_REG);
>>>>                    .......
>>>>                   }
>>> So what do you based on to set variable quad_mode.
>> Best way should be to check for flash device id and manufacture data. Based
>> on which you
>> can decide whether your flash supports quad bits or not.
> Perhaps I did not said it clearly. Your flash supports quad and you
> set it into quad
> mode. But how can your controller driver notice that. In other word,
> In which way
> provide this information from flash to spi controller.
Perhaps, I understood you initial patch wrong. :(
Yes, this is little confusing, and I am trying to figure out that.
As of now, I have tested it with some extern variable, though
we need to see how it can be done cleanly.
I am trying to see if your patch can be used in some way now.
王宇航 - July 5, 2013, 9:41 a.m.
>> Flash data sheets use the terms Dual and Quad I/O Mode,
>> e.g. for MX25L25635E
>> http://www.macronix.com/QuickPlace/hq/PageLibrary4825740B00298A3B.nsf/h_Index/6F878CF760C559BD482576E00022E6CC/?OpenDocument&EPN=MX25L25635E
>
> OK, but this is not a flash specific feature but rather something at the
> SPI level and even with flash are they typically written in all caps?
> This really needs to be described at the SPI level for the SPI subsystem,
> not in terms of the specific application.
>

Yes. I agree with you. This should be described in SPI level.
So to SPI controller, if it has the ability to transfer in 2x or 4x, but
what a pity no device can tell this info to it. No matter what the slave
is and no matter whether they will use the spi transfer width member,
firstly SPI controller should provide these members. If the slave do
not set the member, that means the slave wants to communicate
in 1 line traditionally.
Mark Brown - July 5, 2013, 10:12 a.m.
On Fri, Jul 05, 2013 at 05:41:57PM +0800, yuhang wang wrote:

> Yes. I agree with you. This should be described in SPI level.
> So to SPI controller, if it has the ability to transfer in 2x or 4x, but
> what a pity no device can tell this info to it. No matter what the slave
> is and no matter whether they will use the spi transfer width member,
> firstly SPI controller should provide these members. If the slave do
> not set the member, that means the slave wants to communicate
> in 1 line traditionally.

Yes, it definitely makes sense to have this as a feature in the SPI
core.  We just need a good API for it, which I think is mostly just
about working out what to call things.

Patch

diff --git a/Documentation/spi/spi-dual_quad b/Documentation/spi/spi-dual_quad
new file mode 100644
index 0000000..06a2f9e
--- /dev/null
+++ b/Documentation/spi/spi-dual_quad
@@ -0,0 +1,102 @@ 
+spi-dual_quad: make spi support DUAL/QUAD
+============================================
+
+Description
+----------------------
+DUAL/QUAD means spi can transfer in 2bits/4bits at the same time.
+These spi controllers provide 8 data lines(4-tx and 4-rx). User can
+choose tranfer mode(SINGLE/DUAL/QUAD) by setting the certain register.
+Though SPI is a serial interface, some spi controllers can support
+transmitting and receiving in DUAL and QUAD modes aimed to improve
+the performance. Also as spi slave lots of flashes do support this attribute,
+such as serial-norflash in spansion company.
+
+Serial-flash such as s25fl129p in spansion company.
+In common way, the flash has two data pins(IO0,IO1) supporting SINGLE/DUAL.
+The flash also can work in QUAD mode, there are still other two
pins(HOLD,W#)which
+in other usage will be regarded as data pins.
+
+The members added below is used to provide the transfer information from slave
+to master. Thus spi controller driver can distinguish the transfer mode(SINGLE
+/DUAL/QUAD) and set the certain controlling register.
+
+
+Members added
+----------------------
+struct spi_device {
++       u8     rx_bitwidth;
++       u8     tx_bitwidth;
+}
+
+struct spi_transfer {
++       u8     bitwidth;
++#define        SPI_BITWIDTH_SINGLE     0x01; /* 1bit transfer */
++#define        SPI_BITWIDTH_DUAL       0x02; /* 2bits transfer */
++#define        SPI_BITWIDTH_QUAD       0x03; /* 4bits transfer */
+}
+
+struct spi_board_info {
++       u8     rx_bitwidth;
++       u8     tx_bitwidth;
+}
+
+
+How to use the added members
+----------------------
+
+DECLARE SLAVE DEVICES
+
+Normally your arch/.../mach-*/board-*.c files would provide a small table
+listing the SPI devices on each board.  Details refered to "spi-summary".
+At the same time, if your slave device support DUAL/QUAD transfer, you can
+set as below:
+
+       static struct spi_board_info spi_board_info[] __initdata = {
+       {
+               .modalias       = "xxxxx",
+               .......
+               .chip_select    = 0,
+               .rx_bitwidth = SPI_BITWIDTH_DUAL,
+               .tx_bitwidth = SPI_BITWIDTH_QUAD,
+       },
+       };
+
+ORGANISE SPI PACKAGE
+
+When your slave is registered by:
+
+       spi_register_board_info(spi_board_info, ARRAY_SIZE(spi_board_info));
+
+rx_bitwidth and tx_bitwidth members will be delivered into "struct spi_device".
+Thus the flash driver can notice the tranfer mode that user has appointed.
+Flash driver receives the data from the uplayer and organise the spi_transfer
+package as below:
+
+       ......
+       struct spi_transfer t[2];
+       struct spi_message m;
+       spi_message_init(&m);
+       memset(t, 0, (sizeof t));
+       ......
+       t[0].rx_buf = buf;
+       t[0].len = len;
+       t[0].bitwidth = spi->rx_bitwidth/spi->tx_bitwidth;
+       spi_message_add_tail(&t[0], &m);
+       ......
+       spi_sync(spi, &m);
+       ......
+
+finally, spi controller driver will deal with the spi_tranfer package.
+Controller driver will pick the bitwidth member out due to which set
the transfer
+mode register.
+
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 5b6b072..1411678 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -354,6 +354,7 @@  static int m25p80_read(struct mtd_info *mtd,
loff_t from, size_t len,

        t[1].rx_buf = buf;
        t[1].len = len;
+       t[1].bitwidth = flash->spi->rx_bitwidth;
        spi_message_add_tail(&t[1], &m);

        mutex_lock(&flash->lock);
@@ -409,6 +410,7 @@  static int m25p80_write(struct mtd_info *mtd,
loff_t to, size_t len,
        spi_message_add_tail(&t[0], &m);

        t[1].tx_buf = buf;
+       t[1].bitwidth = flash->spi->tx_bitwidth;
        spi_message_add_tail(&t[1], &m);

        mutex_lock(&flash->lock);
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 004b10f..cd99022 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -452,6 +452,8 @@  struct spi_device *spi_new_device(struct spi_master *master,
        proxy->irq = chip->irq;
        strlcpy(proxy->modalias, chip->modalias, sizeof(proxy->modalias));
        proxy->dev.platform_data = (void *) chip->platform_data;
+       proxy->rx_bitwidth = chip->rx_bitwidth;
+       proxy->tx_bitwidth = chip->tx_bitwidth;
        proxy->controller_data = chip->controller_data;
        proxy->controller_state = NULL;

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 38c2b92..ddcf308 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -93,6 +93,8 @@  struct spi_device {
        void                    *controller_data;
        char                    modalias[SPI_NAME_SIZE];
        int                     cs_gpio;        /* chip select gpio */
+       u8                      rx_bitwidth;
+       u8                      tx_bitwidth;

        /*
         * likely need more hooks for more protocol options affecting how
@@ -511,6 +513,10 @@  struct spi_transfer {
        dma_addr_t      rx_dma;

        unsigned        cs_change:1;
+       u8              bitwidth;
+#define        SPI_BITWIDTH_SINGLE     0x01; /* 1bit transfer */
+#define        SPI_BITWIDTH_DUAL       0x02; /* 2bits transfer */
+#define        SPI_BITWIDTH_QUAD       0x03; /* 4bits transfer */
        u8              bits_per_word;
        u16             delay_usecs;
        u32             speed_hz;
@@ -859,6 +865,8 @@  struct spi_board_info {
         * where the default of SPI_CS_HIGH = 0 is wrong.
         */
        u8              mode;
+       u8              rx_bitwidth;
+       u8              tx_bitwidth;

        /* ... may need additional spi_device chip config data here.
         * avoid stuff protocol drivers can set; but include stuff