From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yang, Wenyou Date: Wed, 30 Aug 2017 09:58:37 +0800 Subject: [U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories In-Reply-To: References: <20170725070102.1344-1-wenyou.yang@microchip.com> Message-ID: <8d0b0156-6e70-f00e-f743-a07f9afcc544@Microchip.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de Hi Jagan, On 2017/8/26 14:34, Jagan Teki wrote: > Hi, > > Thanks for the changes. > > On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang wrote: >> This series of patches are based and have been tested on the 'master' >> branch of the u-boot.git tree. >> >> Tests were passed with a sama5d2 xplained board which embeds both SPI and >> QSPI controllers. >> >> The following tests have been passed: >> >> - QSPI0 + Macronix MX25L25673G: >> + probe: OK >> + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK >> + Page Program 1-1-4 at offset 0x10000: OK >> The Macronix datasheet tells that only Page Program 1-4-4 is >> supported, not Page Program 1-1-4, however it worked, I don't know >> why... >> >> - QSPI0 + Microchip SST26 >> + probe: OK >> + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK >> + Page Program 1-1-1 at offset 0x10000: OK >> SST26 memories support Page Program 1-4-4 but with the op code of >> Page Program 1-1-4, which is not standard so I don't use it. >> >> - QSPI0 + Adesto AT25DF321A >> + probe: OK >> + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK >> + Page Program 1-1-1 at offset 0x10000: OK >> >> - SPI0 + Adesto AT25DF321A >> + probe: OK >> + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK >> + Page Program 1-1-1 at offest 0x6000: OK >> >> - SPI1 + Atmel AT45 >> + probe: OK >> + Read at offset 0 and other than 0: OK >> + Write at offset 0 and other than 0: OK >> >> During my tests, I used: >> - setenv/saveenv, reboot, printenv >> or >> - sf probe, sf read, sf write, sf erase and sf update. >> >> Changes in v3: >> - Add the include to fix build error for corvus_defconfig. >> >> Changes in v2: >> - Rebase on the latest u-boot/master(2710d54f5). >> >> Cyrille Pitchen (8): >> spi: add support of SPI flash commands >> sf: describe all SPI flash commands with 'struct spi_flash_command' >> sf: select the relevant SPI flash protocol for read and write commands >> sf: differentiate Page Program 1-1-4 and 1-4-4 >> sf: add 'addr_len' member to 'struct spi_flash' >> sf: add new option to support SPI flash above 16MiB >> sf: add support to Microchip SST26 QSPI memories >> sf: add driver for Atmel QSPI controller > Comments: > How about writing struct spi_flash_command in spi_flash area > (include/spi_flash.h)? and then write atmel_qspi with > UCLASS_SPI_FLASH? The spi_flash_command struct describes the relevant features of the spi controller, instead of the spi_flash device. The purpose of patch set is used to supersede the spi_xfer() function to access the spi_flash device. So putting it in include/spi.h is suitable, we should not move it in the spi_flash area. Moreover, why do we write atmel_qspi with  UCLASS_SPI_FLASH?  It is not easy to understand. > > Testing: > Basic testing works fine. > > Issues: > - Build issue: with zynq_microzed_defconfig > drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’: > drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set > but not used [-Wunused-but-set-variable] > bool above_16MB; Will send a new version to fix it.  Thanks a lot. > ^~~~~~~~~~ > CC spl/lib/membuff.o > > - issue with spi_flash_cmd_read_ops 4BAIS > Need to calculate bank length only if BAR is in use. Otherwise, > consider the given len as read_len. > > Will send separate patch for this. > > diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c > index 89ceae2..b5d8ef3 100644 > --- a/drivers/mtd/spi/spi_flash.c > +++ b/drivers/mtd/spi/spi_flash.c > @@ -558,13 +558,15 @@ int spi_flash_cmd_read_ops(struct spi_flashmake > *flash, u32 offset, > if (ret < 0) > return ret; > bank_sel = flash->bank_curr; > -#endif > remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) * > (bank_sel + 1)) - offset; > if (len < remain_len) > read_len = len; > else > read_len = remain_len; > +#else > + read_len = len; > +#endif > > cmd.addr = read_addr; > cmd.data_len = read_len; > thanks! Best Regards, Wenyou Yang