Le 16/03/2016 15:14, Jagan Teki a écrit : > On Wednesday 16 March 2016 07:00 PM, Cyrille Pitchen wrote: >> Le 15/03/2016 19:21, Jagan Teki a écrit : >>> On Tuesday 15 March 2016 11:42 PM, Cyrille Pitchen wrote: >>>> Hi all, >>>> >>>> This series of patches fixes and extend the support of QSPI memories >>>> in the SPI flash framework. The updates are split into many parts to >>>> make it easier to understand and review but they should be considered >>>> as a whole. >>>> >>>> This was tested on a Atmel sama5d2 xplained board with a Micron n25q128a >>>> memory. >>>> >>>> Best regards, >>>> >>>> Cyrille >>>> >>>> Cyrille Pitchen (18): >>>> Revert "sf: Fix quad bit set for micron devices" >>>> sf: call spi_claim_bus() and spi_release_bus() only once per read, >>>> write or erase >>>> sf: replace spi_flash_read_common() calls by spi_flash_cmd_read() >>>> sf: remove spi_flash_write_common() >>>> sf: export spi_flash_wait_ready() function >>>> sf: share erase generic algorithm >>>> sf: share write generic algorithm >>>> sf: share read generic algorithm >>>> sf: add hooks to handle register read and write operations >>>> sf: move support of SST flash into generic spi_flash_write_alg() >>>> sf: fix selection of supported READ commands for QSPI memories >>>> sf: fix detection of QSPI memories when they boot in Quad or Dual mode >>>> sf: add helper function to set the number of dummy bytes >>>> sf: add 4byte address opcodes >>>> sf: prepare next fixes to support of QSPI memories by manufacturer >>>> sf: fix support of Micron memories >>>> ARM: at91: clock: add function to get QSPI clocks >>>> sf: add driver for Atmel QSPI controller >>> >>> Appreciate for the work, we're working on spi-nor framework[1] planning to >>> push in couple of weeks. Will let you know once it merged so that you can >>> add your changes on top of that. >>> >>> [1] >>> http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi-nor-next >>> >> >> Hi Jagan, >> >> I've started to have a look on your branch. I hope it's not to late for few >> comments: >> >> Globally I see the new code attend to match the spi-nor framework from Linux. >> OK that's fine but please note the current spi-nor framework in Linux has >> incomplete and sometime not working support of QSPI memories. >> >> First, after a discussion with Brian and Bean on linux-mtd [1], Bean's commit >> to add support to Micron QSPI memories was reverted since it didn't work >> alone. >> In his reply, Brian agreed the code was not tested and should not have been >> merged. >> >> This highlights a more general issue: currently, there is no mean for the >> spi-nor framework to notify the SPI controller driver about a SPI protocol >> change at the QSPI memory side. This applies to Micron memories when they >> enter >> their Quad I/O mode. If so, ALL commands, even JEDEC Read ID, Read Status >> Register, ..., MUST use the SPI 4-4-4 protocol. Commands sent using SPI 1-x-y >> protocols are no longer decoded properly. >> This also applies to Macronix and Winbond memories if they enter their QPI >> mode, which is the equivalent of Micron Quad I/O mode. >> This is why I've suggested to add 4 new fields in the struct spi_nor: >> - .reg_proto: the SPI protocol to be used by .read_reg() and .write_reg() >> hooks. >> - .read_proto: the SPI protocol to be used by the .read() hooks, maybe by the >> .read_mmap() also. >> - .write_proto: the SPI protocol to be used by the .write() hooks >> - .erase_proto: the SPI protocol to be used by the .erase() hooks. >> >> (Q)SPI controller drivers cannot guess the protocol to be used from the >> command >> op code. Indeed, taking the Micron case as un example, the very same 0xeb op >> code may be used with the SPI 1-4-4 protocol (Micron Extended SPI mode) or >> with the SPI 4-4-4 protocol (Micron Quad I/O mode). >> >> >> Also just some words about the naming of SPI x-y-z protocols: >> - x refers to the number of I/O lines used to send the op code byte >> - y is the number of I/O lines used to send the address, mode/dummy cycles >> (if these cycles exist for the command op code) >> - z is the number of I/O lines used to send/receive data (if needed) >> >> So the SNOR_OP_READ_1_1_2_IO macro for the Fast Read Dual I/O command (as >> opposed to the macro SNOR_OP_READ_1_1_2 macro for the Fast Read Dual Output >> command) doesn't make sense: it should be named SNOR_OP_READ_1_2_2. >> >> >> Then about the value used for the dummy cycles, it's not always true that we >> don't care about initializing them. Depending on the configuration of the >> memory, some special dummy cycles, sometime called mode cycles, are used to >> during Fast Read operations to make the memory enter/leaver its Continuous >> Read >> mode. Once is Continuous Read mode, the op code byte is no longer sent, it is >> implicit and the command actually starts from the address cycles. This mode >> is mostly used for XIP applications hence is not relevant for mtd usage. >> However we should take care not to enter this Continuous Mode by mistake. >> Depending on the memory manufacturer, the Continuous Mode can be disabled by >> updating the relevant bit in some configuration register (e.g. setting the >> XIP >> bit in Micron Volatile Configuration Register) or by choosing the right op >> code >> (e.g. for Winbond memories in QPI mode, both the 0x0b and 0xeb op codes can >> be used for Fast Read 4-4-4 operation but only the 0xeb op code cares about >> the dummy cycle value to enter/leave the Continuous Read mode). >> Some Spansion memories use 6 dummy cycles for Fast Read 1-4-4 command as >> factory default, not 8. >> >> Besides when sending a regular JEDEC Read ID (0x9f) command to probe the >> (Q)SPI >> memory, the current spi-nor framework assumes the Quad I/O or QPI mode is not >> already enabled. This not always true, some early bootloarders, such as the >> sama5d2 ROM Code, enables the Micron Quad I/O mode when configured to boot >> from >> the QSPI memory. If so, the QSPI memory no longer replies to the 0x9f command >> in SPI 1-1-1 protocol but instead to the 0xaf command in SPI 4-4-4 protocol. >> >> Finally, about the proper way to describe the SPI controller capabilities, >> the SPI_TX_{DUAL, QUAD} and SPI_RX_{DUAL, QUAD} mode flags are set in the >> SPI framework based on the "spi-rx-bus-width" and "spi-tx-bus-width" DT >> properties already used in Linux. >> This is not enough to make the difference between the SPI 1-4-4 and SPI 4-4-4 >> protocols. Maybe some SPI controllers support the first protocol but not the >> latest. It could be good to add another argument to spi_nor_scan() providing >> an exhaustive list of SPI protocols supported by the SPI controller. >> Then to match this list with the list of SPI protocols supported by the SPI >> memory and select the proper protocol, this new argument should use the same >> range of values as the .flash_read field in the struct spi_nor_info used to >> describe the SPI memories. >> >> For backward compatibility, the m25p80 driver could then convert the SPI >> modes >> into spi-nor read modes. Please have a look at patch 11 of my series; the >> chunk related to spi_flash_probe_slave() in sf_probe.c: >> >> /* Convert SPI mode_rx and mode to SPI flash read commands */ >> + mode_rx = spi->mode_rx; >> + if (mode_rx & SPI_RX_QUAD) { >> + e_rd_cmd = RD_NORM | QUAD_OUTPUT_FAST; >> + if (spi->mode & SPI_TX_QUAD) >> + e_rd_cmd |= QUAD_IO_FAST; >> + } else if (mode_rx & SPI_RX_DUAL) { >> + e_rd_cmd = RD_NORM | DUAL_OUTPUT_FAST; >> + if (spi->mode & SPI_TX_DUAL) >> + e_rd_cmd |= DUAL_IO_FAST; >> + } else if ((mode_rx & (SPI_RX_SLOW | SPI_RX_FAST)) == SPI_RX_SLOW) { >> + e_rd_cmd = ARRAY_SLOW; >> + } else { >> + e_rd_cmd = RD_NORM; >> + } >> + >> [...] >> - ret = spi_flash_scan(flash); >> + ret = spi_flash_scan(flash, e_rd_cmd); >> >> >> I've spent a lot of time working on the QSPI memory topic so I can tell you >> that there are many other traps to avoid! >> If I can help you on this topic during your rework of the SPI NOR support, >> please let me know. > > I understand your points, thanks for that and anyway this spi-nor work is a > starting point for both syncing with Linux as well with new feature or for > better tunning. And more over I started this work in 2014 end and it's been > reviewing over and over and we finally landed up with MTD driver model. > > So, please wait for sometime until this gets merged we definitely work > together for better tunning, thanks! >
OK, no problem! I will just send the additional patches I used to test with the sama5d2 xplained board. They are not related with the spi_flash/spi-nor framework itself but they are part of a WIP update of the sama5d2_xplained board support. If some people need those patches to unblock their own developments using QSPI memories on sama5d2 SoCs, all the material will be available :) Best regards, Cyrille _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
