[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Yang, Wenyou]

On 2017/8/30 11:43, Bin Meng wrote:
> On Wed, Aug 30, 2017 at 11:25 AM, Yang, Wenyou
> <Wenyou.Yang at 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 at 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.
>
>>> 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_flash
>>> *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;
>>>
> Regards,
> Bin

Best Regards,
Wenyou Yang