Hi Jagan,
On Wed, Aug 30, 2017 at 2:30 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Bin,
On Wed, Aug 30, 2017 at 11:11 AM, Bin Meng bmeng.cn@gmail.com wrote:
On Wed, Aug 30, 2017 at 1:27 PM, Yang, Wenyou Wenyou.Yang@microchip.com wrote:
On 2017/8/30 11:43, Bin Meng wrote:
On Wed, Aug 30, 2017 at 11:25 AM, Yang, Wenyou Wenyou.Yang@microchip.com 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 wenyou.yang@microchip.com 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 <spi.h> 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.
My point is: if the warnings exist in current mainline, this should be fixed as a unrelated patch. If the warnings come from this series, this should be fixed in the particular patch that introduces the warnings!
Regards, Bin