| Message ID | CAHSAbzPNDSr_6N79fh0DTWSEg8buRRPyiw8SPodtKLPjzWXJsg@mail.gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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
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.
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
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
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.
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
> 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
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
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/
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]: 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
> 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
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
> 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.
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.
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.
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.
>> 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.
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.
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