From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jagan Teki Date: Wed, 30 Aug 2017 18:55:23 +0530 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> <285a60d7-6f71-5196-d52b-a5a2aad383b3@Microchip.com> <63106f2a-0fd9-4dcd-4077-b10cc40a1275@Microchip.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de Hi Bin, On Wed, Aug 30, 2017 at 1:17 PM, Bin Meng wrote: > Hi Jagan, > > On Wed, Aug 30, 2017 at 2:30 PM, Jagan Teki wrote: >> Hi Bin, >> >> On Wed, Aug 30, 2017 at 11:11 AM, Bin Meng wrote: >>> On Wed, Aug 30, 2017 at 1:27 PM, Yang, Wenyou wrote: >>>> >>>> >>>> On 2017/8/30 11:43, Bin Meng wrote: >>>>> >>>>> On Wed, Aug 30, 2017 at 11:25 AM, Yang, Wenyou >>>>> wrote: >>>>>> >>>>>> >>>>>> 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? >>>>>>> >>>>>>> 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; >>>>>>> ^~~~~~~~~~ >>>>>>> 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. >>>>>> >>>>>> Will You send a separate patch? or I include it in this patch set? >>>>> >>>>> This should not be a separate patch. Since every patch needs to pass >>>>> buildman testing. >>>> >>>> But it is not introduced by this patch set. So should be a separate patch to >>>> fix. >>> >>> Do you mean the build warnings exist in current u-boot/master? >>> >>> If so, Jagan can you please explain why you mention this? This is >>> nothing related to this patch review. >> >> Please read my previous comments, I was clearly explain the issue and >> diff. Issue came up this series with 4BAIS on spi_flash_cmd_read_ops >> > > So your words and Wenyou are contradictory. Wenyou claimed the > warnings do not come from his patchset while you said yes. Anyway will > leave you guys to resolve. No, I didn't say that I wrote "Will send separate patch for this." for 4BIAS fix not for the warning issue. thanks! -- Jagan Teki Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India.