* SPI: DUAL/QUAD support @ 2013-07-04 11:36 yuhang wang 2013-07-04 13:00 ` Johannes Stezenbach 2013-07-04 14:36 ` Mark Brown 0 siblings, 2 replies; 27+ messages in thread From: yuhang wang @ 2013-07-04 11:36 UTC (permalink / raw) To: linux-arm-kernel 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 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 -- 1.7.9.5 Best regards ^ permalink raw reply related [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-04 11:36 SPI: DUAL/QUAD support yuhang wang @ 2013-07-04 13:00 ` Johannes Stezenbach 2013-07-04 14:58 ` Thomas.Betker at rohde-schwarz.com 2013-07-04 14:36 ` Mark Brown 1 sibling, 1 reply; 27+ messages in thread From: Johannes Stezenbach @ 2013-07-04 13:00 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-04 13:00 ` Johannes Stezenbach @ 2013-07-04 14:58 ` Thomas.Betker at rohde-schwarz.com 2013-07-04 15:49 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: Thomas.Betker at rohde-schwarz.com @ 2013-07-04 14:58 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-04 14:58 ` Thomas.Betker at rohde-schwarz.com @ 2013-07-04 15:49 ` Mark Brown 2013-07-04 16:04 ` Thomas.Betker at rohde-schwarz.com 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2013-07-04 15:49 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 04, 2013 at 04:58:52PM +0200, Thomas.Betker at 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. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130704/5c865c14/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-04 15:49 ` Mark Brown @ 2013-07-04 16:04 ` Thomas.Betker at rohde-schwarz.com 2013-07-05 6:25 ` yuhang wang 0 siblings, 1 reply; 27+ messages in thread From: Thomas.Betker at rohde-schwarz.com @ 2013-07-04 16:04 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-04 16:04 ` Thomas.Betker at rohde-schwarz.com @ 2013-07-05 6:25 ` yuhang wang 2013-07-05 6:45 ` Gupta, Pekon 2013-07-05 7:40 ` Sourav Poddar 0 siblings, 2 replies; 27+ messages in thread From: yuhang wang @ 2013-07-05 6:25 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 6:25 ` yuhang wang @ 2013-07-05 6:45 ` Gupta, Pekon 2013-07-05 7:35 ` Johannes Stezenbach 2013-07-05 7:40 ` Sourav Poddar 1 sibling, 1 reply; 27+ messages in thread From: Gupta, Pekon @ 2013-07-05 6:45 UTC (permalink / raw) To: linux-arm-kernel > 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 6:45 ` Gupta, Pekon @ 2013-07-05 7:35 ` Johannes Stezenbach 2013-07-05 7:41 ` Sourav Poddar 2013-07-05 8:04 ` Gupta, Pekon 0 siblings, 2 replies; 27+ messages in thread From: Johannes Stezenbach @ 2013-07-05 7:35 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 7:35 ` Johannes Stezenbach @ 2013-07-05 7:41 ` Sourav Poddar 2013-07-05 8:04 ` Gupta, Pekon 1 sibling, 0 replies; 27+ messages in thread From: Sourav Poddar @ 2013-07-05 7:41 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 7:35 ` Johannes Stezenbach 2013-07-05 7:41 ` Sourav Poddar @ 2013-07-05 8:04 ` Gupta, Pekon 1 sibling, 0 replies; 27+ messages in thread From: Gupta, Pekon @ 2013-07-05 8:04 UTC (permalink / raw) To: linux-arm-kernel > > [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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 6:25 ` yuhang wang 2013-07-05 6:45 ` Gupta, Pekon @ 2013-07-05 7:40 ` Sourav Poddar 2013-07-05 8:48 ` yuhang wang 1 sibling, 1 reply; 27+ messages in thread From: Sourav Poddar @ 2013-07-05 7:40 UTC (permalink / raw) To: linux-arm-kernel 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/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 7:40 ` Sourav Poddar @ 2013-07-05 8:48 ` yuhang wang 2013-07-05 8:55 ` Sourav Poddar 0 siblings, 1 reply; 27+ messages in thread From: yuhang wang @ 2013-07-05 8:48 UTC (permalink / raw) To: linux-arm-kernel > 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 8:48 ` yuhang wang @ 2013-07-05 8:55 ` Sourav Poddar 2013-07-05 9:07 ` yuhang wang 0 siblings, 1 reply; 27+ messages in thread From: Sourav Poddar @ 2013-07-05 8:55 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 8:55 ` Sourav Poddar @ 2013-07-05 9:07 ` yuhang wang 2013-07-05 9:08 ` Sourav Poddar 0 siblings, 1 reply; 27+ messages in thread From: yuhang wang @ 2013-07-05 9:07 UTC (permalink / raw) To: linux-arm-kernel > 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. ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 9:07 ` yuhang wang @ 2013-07-05 9:08 ` Sourav Poddar 2013-07-05 9:17 ` yuhang wang 0 siblings, 1 reply; 27+ messages in thread From: Sourav Poddar @ 2013-07-05 9:08 UTC (permalink / raw) To: linux-arm-kernel 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. ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 9:08 ` Sourav Poddar @ 2013-07-05 9:17 ` yuhang wang 2013-07-05 9:27 ` Sourav Poddar 0 siblings, 1 reply; 27+ messages in thread From: yuhang wang @ 2013-07-05 9:17 UTC (permalink / raw) To: linux-arm-kernel 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. ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 9:17 ` yuhang wang @ 2013-07-05 9:27 ` Sourav Poddar 2013-07-05 10:24 ` yuhang wang 0 siblings, 1 reply; 27+ messages in thread From: Sourav Poddar @ 2013-07-05 9:27 UTC (permalink / raw) To: linux-arm-kernel 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. ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 9:27 ` Sourav Poddar @ 2013-07-05 10:24 ` yuhang wang 2013-07-05 14:34 ` Johannes Stezenbach 0 siblings, 1 reply; 27+ messages in thread From: yuhang wang @ 2013-07-05 10:24 UTC (permalink / raw) To: linux-arm-kernel 2013/7/5 Sourav Poddar <sourav.poddar@ti.com>: > 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. > Got it. In fact the kernel version I am working in is 3.0.77. And I modified m25p80.c in Ver 3.0.90 to fit to the dual and quad. But the driver is not general because I don't find any standard for serial-flash. Also to the member name, just as Mark said, the meaning is not suitable enough. Finally, do not support device tree. 2 Questions here: 1.In m25p80.c probe, the kmalloc below I feel very strange. flash->command = kmalloc(MAX_CMD_SIZE + (flash->fast_read ? 1 : 0), GFP_KERNEL); (flash->fast_read ? 1 : 0) must be 0! So why do it like that. 2.Is that any need to add wait_till_ready in the erase function to wait for the end of erase. But it will really take a long time. Signed-off-by: wangyuhang <wangyuhang2014@gmail.com> --- drivers/mtd/devices/m25p80.c | 93 ++++++++++++++++++++++++++++++++---------- drivers/spi/spi.c | 2 + include/linux/spi/spi.h | 8 ++++ 3 files changed, 82 insertions(+), 21 deletions(-) diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 5b6b072..267c8af 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -47,6 +47,10 @@ #define OPCODE_CHIP_ERASE 0xc7 /* Erase whole flash chip */ #define OPCODE_SE 0xd8 /* Sector erase (usually 64KiB) */ #define OPCODE_RDID 0x9f /* Read JEDEC ID */ +#define OPCODE_DOR 0x3B /* Dual Output Read */ +#define OPCODE_QOR 0x6B /* Quad Output Read */ +#define OPCODE_QPP 0x32 /* Quad Page Programming */ +#define OPCODE_QEN 0x02 /* Quad Mode Enable Flag */ /* Used for SST flashes only. */ #define OPCODE_BP 0x02 /* Byte program */ @@ -73,6 +77,14 @@ #define MAX_READY_WAIT_JIFFIES (40 * HZ) /* M25P16 specs 40s max chip erase */ #define MAX_CMD_SIZE 5 +#ifdef CONFIG_M25PXX_USE_FAST_READ +#define OPCODE_READ OPCODE_FAST_READ +#define FAST_READ_DUMMY_BYTE 1 +#else +#define OPCODE_READ OPCODE_NORM_READ +#define FAST_READ_DUMMY_BYTE 0 +#endif + #define JEDEC_MFR(_jedec_id) ((_jedec_id) >> 16) /****************************************************************************/ @@ -85,7 +97,9 @@ struct m25p { u16 addr_width; u8 erase_opcode; u8 *command; - bool fast_read; + u8 read_opcode; + u8 read_dummy; + u8 write_opcode; }; static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd) @@ -94,6 +108,18 @@ static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd) } /****************************************************************************/ +/* + * Write configuration register 2 byte + * Returns negative if error occurred. + */ +static int write_cr(struct m25p *flash, u16 val) +{ + flash->command[0] = OPCODE_WRSR; + flash->command[1] = val >> 8; + flash->command[2] = val; + + return spi_write(flash->spi, flash->command, 3); +} /* * Internal helper functions @@ -336,7 +362,6 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len, struct m25p *flash = mtd_to_m25p(mtd); struct spi_transfer t[2]; struct spi_message m; - uint8_t opcode; pr_debug("%s: %s from 0x%08x, len %zd\n", dev_name(&flash->spi->dev), __func__, (u32)from, len); @@ -349,11 +374,12 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len, * Should add 1 byte DUMMY_BYTE. */ t[0].tx_buf = flash->command; - t[0].len = m25p_cmdsz(flash) + (flash->fast_read ? 1 : 0); + t[0].len = m25p_cmdsz(flash) + flash->read_dummy; spi_message_add_tail(&t[0], &m); 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); @@ -370,15 +396,13 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len, * supports that opcode. */ - /* Set up the write data buffer. */ - opcode = flash->fast_read ? OPCODE_FAST_READ : OPCODE_NORM_READ; - flash->command[0] = opcode; + /* Set up opcode in read buffer */ + flash->command[0] = flash->read_opcode; m25p_addr2cmd(flash, from, flash->command); spi_sync(flash->spi, &m); - *retlen = m.actual_length - m25p_cmdsz(flash) - - (flash->fast_read ? 1 : 0); + *retlen = m.actual_length - m25p_cmdsz(flash) - flash->read_dummy; mutex_unlock(&flash->lock); @@ -409,6 +433,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); @@ -422,7 +447,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len, write_enable(flash); /* Set up the opcode in the write buffer. */ - flash->command[0] = OPCODE_PP; + flash->command[0] = flash->write_opcode; m25p_addr2cmd(flash, to, flash->command); page_offset = to & (flash->page_size - 1); @@ -958,8 +983,7 @@ static int m25p_probe(struct spi_device *spi) flash = kzalloc(sizeof *flash, GFP_KERNEL); if (!flash) return -ENOMEM; - flash->command = kmalloc(MAX_CMD_SIZE + (flash->fast_read ? 1 : 0), - GFP_KERNEL); + flash->command = kmalloc(MAX_CMD_SIZE + FAST_READ_DUMMY_BYTE, GFP_KERNEL); if (!flash->command) { kfree(flash); return -ENOMEM; @@ -1022,16 +1046,6 @@ static int m25p_probe(struct spi_device *spi) flash->page_size = info->page_size; flash->mtd.writebufsize = flash->page_size; - flash->fast_read = false; -#ifdef CONFIG_OF - if (np && of_property_read_bool(np, "m25p,fast-read")) - flash->fast_read = true; -#endif - -#ifdef CONFIG_M25PXX_USE_FAST_READ - flash->fast_read = true; -#endif - if (info->addr_width) flash->addr_width = info->addr_width; else { @@ -1063,6 +1077,43 @@ static int m25p_probe(struct spi_device *spi) flash->mtd.eraseregions[i].erasesize / 1024, flash->mtd.eraseregions[i].numblocks); + if ((flash->spi->rx_bitwidth == SPI_BITWIDTH_QUAD) || + (flash->spi->tx_bitwidth == SPI_BITWIDTH_QUAD)) { + write_enable(flash); + write_cr(flash, OPCODE_QEN); + } + + switch (flash->spi->rx_bitwidth) { + case SPI_BITWIDTH_SINGLE: + flash->read_opcode = OPCODE_READ; + flash->read_dummy = FAST_READ_DUMMY_BYTE; + break; + case SPI_BITWIDTH_DUAL: + flash->read_opcode = OPCODE_DOR; + flash->read_dummy = 1; + break; + case SPI_BITWIDTH_QUAD: + flash->read_opcode = OPCODE_QOR; + flash->read_dummy = 1; + break; + default: + dev_err(&spi->dev, "invalid rx_bitwidth %d\n", + flash->spi->rx_bitwidth); + return -EINVAL; + } + + switch (flash->spi->tx_bitwidth) { + case SPI_BITWIDTH_SINGLE: + flash->write_opcode = OPCODE_PP; + break; + case SPI_BITWIDTH_QUAD: + flash->write_opcode = OPCODE_QPP; + break; + default: + dev_err(&spi->dev, "invalid tx_bitwidth %d\n", + flash->spi->tx_bitwidth); + return -EINVAL; + } /* partitions should match sector boundaries; and it may be good to * use readonly partitions for writeprotected sectors (BP2..BP0). 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 10:24 ` yuhang wang @ 2013-07-05 14:34 ` Johannes Stezenbach 2013-07-05 15:41 ` yuhang wang 0 siblings, 1 reply; 27+ messages in thread From: Johannes Stezenbach @ 2013-07-05 14:34 UTC (permalink / raw) To: linux-arm-kernel On Fri, Jul 05, 2013 at 06:24:40PM +0800, yuhang wang wrote: > > 2 Questions here: > 1.In m25p80.c probe, the kmalloc below I feel very strange. > flash->command = kmalloc(MAX_CMD_SIZE + (flash->fast_read ? 1 : 0), > GFP_KERNEL); > > (flash->fast_read ? 1 : 0) must be 0! So why do it like that. Maybe just increase MAX_CMD_SIZE by 1 unconditonally? Yeah, looks like a bug in m25p80.c, flash->fast_read is initialized too late. > +#define OPCODE_DOR 0x3B /* Dual Output Read */ > +#define OPCODE_QOR 0x6B /* Quad Output Read */ > +#define OPCODE_QPP 0x32 /* Quad Page Programming */ > +#define OPCODE_QEN 0x02 /* Quad Mode Enable Flag */ Which flash chip are these for? The OPCODE_DOR and OPCODE_QOR match MX25L25635E, but OPCODE_QPP not (Macronix uses 0x38). > +#ifdef CONFIG_M25PXX_USE_FAST_READ > +#define OPCODE_READ OPCODE_FAST_READ > +#define FAST_READ_DUMMY_BYTE 1 > +#else > +#define OPCODE_READ OPCODE_NORM_READ > +#define FAST_READ_DUMMY_BYTE 0 > +#endif this doesn't work with the flash->fast_read setting from DT BTW, if we add "bitwidths" to spi_board_info, wouldn't it make sense to also add "use_fast_read" and remove the CONFIG_M25PXX_USE_FAST_READ option? > +/* > + * Write configuration register 2 byte > + * Returns negative if error occurred. > + */ > +static int write_cr(struct m25p *flash, u16 val) > +{ > + flash->command[0] = OPCODE_WRSR; > + flash->command[1] = val >> 8; > + flash->command[2] = val; > + > + return spi_write(flash->spi, flash->command, 3); > +} > + if ((flash->spi->rx_bitwidth == SPI_BITWIDTH_QUAD) || > + (flash->spi->tx_bitwidth == SPI_BITWIDTH_QUAD)) { > + write_enable(flash); > + write_cr(flash, OPCODE_QEN); > + } MX25L25635E doesn't have a cr, instead a bit needs to be set in the sr Maybe we need to add some fields to m25p_ids[] to manage variation between flash devices. Thanks, Johannes ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 14:34 ` Johannes Stezenbach @ 2013-07-05 15:41 ` yuhang wang 0 siblings, 0 replies; 27+ messages in thread From: yuhang wang @ 2013-07-05 15:41 UTC (permalink / raw) To: linux-arm-kernel 2013/7/5 Johannes Stezenbach <js@sig21.net>: > On Fri, Jul 05, 2013 at 06:24:40PM +0800, yuhang wang wrote: >> >> 2 Questions here: >> 1.In m25p80.c probe, the kmalloc below I feel very strange. >> flash->command = kmalloc(MAX_CMD_SIZE + (flash->fast_read ? 1 : 0), >> GFP_KERNEL); >> >> (flash->fast_read ? 1 : 0) must be 0! So why do it like that. > > Maybe just increase MAX_CMD_SIZE by 1 unconditonally? > Yeah, looks like a bug in m25p80.c, flash->fast_read > is initialized too late. > >> +#define OPCODE_DOR 0x3B /* Dual Output Read */ >> +#define OPCODE_QOR 0x6B /* Quad Output Read */ >> +#define OPCODE_QPP 0x32 /* Quad Page Programming */ >> +#define OPCODE_QEN 0x02 /* Quad Mode Enable Flag */ > > Which flash chip are these for? The OPCODE_DOR and OPCODE_QOR > match MX25L25635E, but OPCODE_QPP not (Macronix uses 0x38). > Thanks for the review. And I have said that it is not a general code to support dual and quad, it is just a situation to fit my system. Perhaps it is very hard to provide a general code before a serial-norflash standard comes out. >> +#ifdef CONFIG_M25PXX_USE_FAST_READ >> +#define OPCODE_READ OPCODE_FAST_READ >> +#define FAST_READ_DUMMY_BYTE 1 >> +#else >> +#define OPCODE_READ OPCODE_NORM_READ >> +#define FAST_READ_DUMMY_BYTE 0 >> +#endif > > this doesn't work with the flash->fast_read setting from DT > > BTW, if we add "bitwidths" to spi_board_info, wouldn't > it make sense to also add "use_fast_read" and remove the > CONFIG_M25PXX_USE_FAST_READ option? > Well, I think we should make sure that whether all spi slave has the "fast read" feature before we add "use_fast_read" into the spi_board_info. There should be some other flash or slaves do not support this feature. So in my opinion, Fast read still supported as a M25p config should be more suitable. >> +/* >> + * Write configuration register 2 byte >> + * Returns negative if error occurred. >> + */ >> +static int write_cr(struct m25p *flash, u16 val) >> +{ >> + flash->command[0] = OPCODE_WRSR; >> + flash->command[1] = val >> 8; >> + flash->command[2] = val; >> + >> + return spi_write(flash->spi, flash->command, 3); >> +} > >> + if ((flash->spi->rx_bitwidth == SPI_BITWIDTH_QUAD) || >> + (flash->spi->tx_bitwidth == SPI_BITWIDTH_QUAD)) { >> + write_enable(flash); >> + write_cr(flash, OPCODE_QEN); >> + } > > MX25L25635E doesn't have a cr, instead a bit needs to be set in the sr > > Maybe we need to add some fields to m25p_ids[] to manage variation > between flash devices. > similar to the first answer. Also a standard is the solution to these questions. Thanks a lot. ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-04 11:36 SPI: DUAL/QUAD support yuhang wang 2013-07-04 13:00 ` Johannes Stezenbach @ 2013-07-04 14:36 ` Mark Brown 2013-07-04 18:06 ` Johannes Stezenbach 1 sibling, 1 reply; 27+ messages in thread From: Mark Brown @ 2013-07-04 14:36 UTC (permalink / raw) To: linux-arm-kernel 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. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130704/3999b302/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-04 14:36 ` Mark Brown @ 2013-07-04 18:06 ` Johannes Stezenbach 2013-07-04 19:12 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: Johannes Stezenbach @ 2013-07-04 18:06 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-04 18:06 ` Johannes Stezenbach @ 2013-07-04 19:12 ` Mark Brown 2013-07-05 9:41 ` yuhang wang 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2013-07-04 19:12 UTC (permalink / raw) To: linux-arm-kernel 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. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130704/6b0c5352/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-04 19:12 ` Mark Brown @ 2013-07-05 9:41 ` yuhang wang 2013-07-05 10:12 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: yuhang wang @ 2013-07-05 9:41 UTC (permalink / raw) To: linux-arm-kernel >> 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. ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI: DUAL/QUAD support 2013-07-05 9:41 ` yuhang wang @ 2013-07-05 10:12 ` Mark Brown 0 siblings, 0 replies; 27+ messages in thread From: Mark Brown @ 2013-07-05 10:12 UTC (permalink / raw) To: linux-arm-kernel 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. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130705/71080f89/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* SPI : DUAL/QUAD support @ 2013-07-04 7:07 王宇航 2013-07-04 9:00 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: 王宇航 @ 2013-07-04 7:07 UTC (permalink / raw) To: linux-arm-kernel Hi Now some SPI controllers and Slave Devices have supported DUAL and QUAD transfer mode. But SPI controller driver in kernel has no member to deliver this information from Slave to Master. So in my opinion, adding transfer mode members to deal with the problem. In my SPI system, slave device is a Nor-Flash chip(s25fl129p) which use m25p80 driver. My patch below aims to make spi controller know the way that the slave want. Firstly user provides the SPI slave information into spi_board_info, including tx_bitwidth and rx_bitwidth. When the SPI slave is registered, information in spi_board_info is delivered to spi_device. Then SPI slave driver will add the transfer mode information into the spi_tranfer package and send it out to SPI controller driver. SPI controller driver will analyse the transfer package and set the certain transfer mode to its register. This patch do not support device tree.... Signed-off-by: wangyuhang <wangyuhang2014@gmail.com> --- drivers/mtd/devices/m25p80.c | 2 ++ drivers/spi/spi.c | 2 ++ include/linux/spi/spi.h | 8 ++++++++ 3 files changed, 12 insertions(+) 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 -- 1.7.9.5 Best regards Jay ^ permalink raw reply related [flat|nested] 27+ messages in thread
* SPI : DUAL/QUAD support 2013-07-04 7:07 SPI : " 王宇航 @ 2013-07-04 9:00 ` Mark Brown 0 siblings, 0 replies; 27+ messages in thread From: Mark Brown @ 2013-07-04 9:00 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 04, 2013 at 03:07:31PM +0800, ??? wrote: > Now some SPI controllers and Slave Devices have supported > DUAL and QUAD transfer mode. But SPI controller driver in kernel > has no member to deliver this information from Slave to Master. > So in my opinion, adding transfer mode members to deal with the problem. You'll need to tell us what "DUAL" and "QUAD" modes are and provide some kerneldoc for the new fields. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130704/6b1f6dc6/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2013-07-05 15:41 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-04 11:36 SPI: DUAL/QUAD support yuhang wang 2013-07-04 13:00 ` Johannes Stezenbach 2013-07-04 14:58 ` Thomas.Betker at rohde-schwarz.com 2013-07-04 15:49 ` Mark Brown 2013-07-04 16:04 ` Thomas.Betker at rohde-schwarz.com 2013-07-05 6:25 ` yuhang wang 2013-07-05 6:45 ` Gupta, Pekon 2013-07-05 7:35 ` Johannes Stezenbach 2013-07-05 7:41 ` Sourav Poddar 2013-07-05 8:04 ` Gupta, Pekon 2013-07-05 7:40 ` Sourav Poddar 2013-07-05 8:48 ` yuhang wang 2013-07-05 8:55 ` Sourav Poddar 2013-07-05 9:07 ` yuhang wang 2013-07-05 9:08 ` Sourav Poddar 2013-07-05 9:17 ` yuhang wang 2013-07-05 9:27 ` Sourav Poddar 2013-07-05 10:24 ` yuhang wang 2013-07-05 14:34 ` Johannes Stezenbach 2013-07-05 15:41 ` yuhang wang 2013-07-04 14:36 ` Mark Brown 2013-07-04 18:06 ` Johannes Stezenbach 2013-07-04 19:12 ` Mark Brown 2013-07-05 9:41 ` yuhang wang 2013-07-05 10:12 ` Mark Brown -- strict thread matches above, loose matches on Subject: below -- 2013-07-04 7:07 SPI : " 王宇航 2013-07-04 9:00 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).