[U-Boot] [PATCH v6 00/76] mtd: Add SPI-NOR core support

Jagan Teki jteki at openedev.com
Thu Feb 18 08:13:42 CET 2016


Hi  Bin,

On 18 February 2016 at 10:54, Bin Meng <bmeng.cn at gmail.com> wrote:
> On Thu, Feb 18, 2016 at 1:13 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Jagan,
>>
>> On Thu, Feb 18, 2016 at 12:05 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Hi Jagan,
>>>
>>> On Wed, Feb 17, 2016 at 5:10 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>> Hi Jagan,
>>>>
>>>> On Wed, Feb 17, 2016 at 3:38 PM, Jagan Teki <jteki at openedev.com> wrote:
>>>>> Hi Bin,
>>>>>
>>>>> On 17 February 2016 at 13:07, Jagan Teki <jteki at openedev.com> wrote:
>>>>>> Hi Bin,
>>>>>>
>>>>>> On 17 February 2016 at 10:52, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>>>> Hi Jagan,
>>>>>>>
>>>>>>> On Wed, Feb 17, 2016 at 3:47 AM, Jagan Teki <jteki at openedev.com> wrote:
>>>>>>>> On 15 February 2016 at 02:16, Jagan Teki <jteki at openedev.com> wrote:
>>>>>>>>> Compared to previous patch series this series adds spi-nor
>>>>>>>>> core with spi-nor controller drivers are of "mtd uclass"
>>>>>>>>>
>>>>>>>>> This is whole series for all spi-nor related changes, and while
>>>>>>>>> series tested on spansion spi-nor chip.
>>>>>>>>>
>>>>>>>>> Know issue:
>>>>>>>>> - arch/x86/lib/mrccache.c uses dm_spi_flash_ops, this need to fix.
>>>>>>>>>
>>>>>>>>> Why this framework:
>>>>>>>>>
>>>>>>>>> Some of the SPI device drivers at drivers/spi not a real
>>>>>>>>> spi controllers, Unlike normal/generic SPI controllers they
>>>>>>>>> operates only with SPI-NOR flash devices. these were technically
>>>>>>>>> termed as SPI-NOR controllers, Ex: drivers/spi/fsl_qspi.c
>>>>>>>>>
>>>>>>>>> The problem with these were resides at drivers/spi is entire
>>>>>>>>> SPI layer becomes SPI-NOR flash oriented which is absolutely
>>>>>>>>> a wrong indication where SPI layer getting effected more with
>>>>>>>>> flash operations - So this SPI-NOR core will resolve this issue
>>>>>>>>> by separating all SPI-NOR flash operations from spi layer and
>>>>>>>>> creats a generic layer called SPI-NOR core which can be used to
>>>>>>>>> interact SPI-NOR to SPI driver interface layer and the SPI-NOR
>>>>>>>>> controller driver. The idea is taken from Linux spi-nor framework.
>>>>>>>>>
>>>>>>>>> Before SPI-NOR:
>>>>>>>>>
>>>>>>>>> -----------------------
>>>>>>>>>         cmd/sf.c
>>>>>>>>> -----------------------
>>>>>>>>>         spi_flash.c
>>>>>>>>> -----------------------
>>>>>>>>>         sf_probe.c
>>>>>>>>> -----------------------
>>>>>>>>>         spi-uclass
>>>>>>>>> -----------------------
>>>>>>>>>         spi drivers
>>>>>>>>> -----------------------
>>>>>>>>>         SPI NOR chip
>>>>>>>>> -----------------------
>>>>>>>>>
>>>>>>>>> After SPI-NOR:
>>>>>>>>>
>>>>>>>>> ------------------------------
>>>>>>>>>         cmd/sf.c
>>>>>>>>> ------------------------------
>>>>>>>>>         spi-nor.c
>>>>>>>>> -------------------------------
>>>>>>>>> m25p80.c        spi nor drivers
>>>>>>>>> -------------------------------
>>>>>>>>> spi-uclass      SPI NOR chip
>>>>>>>>> -------------------------------
>>>>>>>>> spi drivers
>>>>>>>>> -------------------------------
>>>>>>>>> SPI NOR chip
>>>>>>>>> -------------------------------
>>>>>>>>>
>>>>>>>>> SPI-NOR with MTD:
>>>>>>>>>
>>>>>>>>> ------------------------------
>>>>>>>>>         cmd/sf.c
>>>>>>>>> ------------------------------
>>>>>>>>>         MTD core
>>>>>>>>> ------------------------------
>>>>>>>>>         spi-nor.c
>>>>>>>>> -------------------------------
>>>>>>>>> m25p80.c        spi nor drivers
>>>>>>>>> -------------------------------
>>>>>>>>> spi-uclass      SPI NOR chip
>>>>>>>>> -------------------------------
>>>>>>>>> spi drivers
>>>>>>>>> -------------------------------
>>>>>>>>> SPI NOR chip
>>>>>>>>> -------------------------------
>>>>>>>>>
>>>>>>>>> drivers/mtd/spi-nor/spi-nor.c: spi-nor core
>>>>>>>>> drivers/mtd/spi-nor/m25p80.c: mtd uclass driver
>>>>>>>>> which is an interface layer b/w spi-nor core drivers/spi
>>>>>>>>> drivers/mtd/spi-nor/fsl_qspi.c: spi-nor controller driver(mtd uclass)
>>>>>>>>
>>>>>>>> Tested both DM and non-DM models
>>>>>>>>
>>>>>>>> Tested-by: Jagan Teki <jteki at openedev.com>
>>>>>>>
>>>>>>> My test shows on Intel Crown Bay, ''saveenv" does write U-Boot env to
>>>>>>> the SPI flash correctly, however after "reset" U-Boot still shows: ***
>>>>>>> Warning - bad CRC, using default environment
>>>>>>>
>>>>>>> spi-nor: detected sst25vf016b with page size 256 Bytes, erase size 4
>>>>>>> KiB, total 2 MiB
>>>>>>> *** Warning - bad CRC, using default environment
>>>>>>>
>>>>>>> Anything wrong?
>>>>>>
>>>>>>
>>>>>
>>>>>  I'm not getting the warning, after saveenv.
>>>>>
>>>>> DRAM:  ECC disabled 1 GiB
>>>>> MMC:
>>>>> spi-nor: detected s25fl128s with page size 256 Bytes, erase size 64
>>>>> KiB, total 16 MiB
>>>>> In:    serial at e0001000
>>>>> Out:   serial at e0001000
>>>>> Err:   serial at e0001000
>>>>> Model: Zynq MicroZED Board
>>>>>
>>>>
>>>> Tested on another x86 board, still see the 'bad CRC' warning, log below:
>>>>
>>>> U-Boot 2016.03-rc1-00254-gaba43ff (Feb 17 2016 - 17:03:54 +0800)
>>>>
>>>> CPU: x86, vendor Intel, device 590h
>>>> DRAM:  256 MiB
>>>> MMC:   Quark SDHCI: 0
>>>> spi-nor: detected w25q64 with page size 256 Bytes, erase size 4 KiB, total 8 MiB
>>>> *** Warning - bad CRC, using default environment
>>>>
>>>> Model: Intel Galileo
>>>> Net:
>>>> Warning: eth_designware#0 (eth0) using random MAC address - 5a:58:ba:e9:22:fb
>>>> eth0: eth_designware#0
>>>> Warning: eth_designware#1 (eth1) using random MAC address - c6:a3:69:ff:b7:2e
>>>> , eth1: eth_designware#1
>>>> Hit any key to stop autoboot:  0
>>>> => saveenv
>>>> Saving Environment to SPI Flash...
>>>> Erasing SPI flash...Writing to SPI flash...done
>>>> => reset
>>>> resetting ...
>>>>
>>>>
>>>> U-Boot 2016.03-rc1-00254-gaba43ff (Feb 17 2016 - 17:03:54 +0800)
>>>>
>>>> CPU: x86, vendor Intel, device 590h
>>>> DRAM:  256 MiB
>>>> MMC:   Quark SDHCI: 0
>>>> spi-nor: detected w25q64 with page size 256 Bytes, erase size 4 KiB, total 8 MiB
>>>> *** Warning - bad CRC, using default environment
>>>>
>>>> Model: Intel Galileo
>>>> Net:
>>>> Warning: eth_designware#0 (eth0) using random MAC address - 5a:58:ba:e9:22:fb
>>>> eth0: eth_designware#0
>>>> Warning: eth_designware#1 (eth1) using random MAC address - c6:a3:69:ff:b7:2e
>>>> , eth1: eth_designware#1
>>>> Hit any key to stop autoboot:  0
>>>> =>
>>>>
>>>
>>> Looks like you messed up the ICH SPI driver. Something went wrong with
>>> your patch series. ICH SPI controller is a special one (only supports
>>> slow read) especially when it comes to deal with an SST flash which is
>>> even odd (only supports byte program). I feel this ICH SPI controller
>>> is quite vulnerable to SPI changes as it breaks many times. Maybe
>>> Jagan you can purchase one Crown Bay board for the SPI flash testing
>>> (a joke!)
>>>
>>> logs below:
>>>
>>> => sf read 100000 100000 100
>>> device 0 offset 0x100000, size 0x100
>>> SF: 256 bytes @ 0x100000 Read: OK
>>> => crc fff00000 100
>>> crc32 for fff00000 ... fff000ff ==> ac8e7061
>>> => crc 100000 100
>>> crc32 for 00100000 ... 001000ff ==> b44fdefc
>>>
>>> See? the contents of 'sf read' to address 0x100000 does not match the
>>> original one in SPI flash (0xfff00000)
>>>
>>> Anyway, I will continue debugging this.
>>>
>>
>> Root cause identified. Patch available @
>> http://patchwork.ozlabs.org/patch/584500/.
>
> This should be http://patchwork.ozlabs.org/patch/584517/
>
> I doubt storing all SPI modes into a single spi->mode is a good idea.
> Previously they are separate variables.
>
>> Please squash mine into an
>> appropriate place in your series.
>>
>> Given the logic detection is completely wrong, I doubt how you tested
>> this whole series. What boards/flashes have you tested this on?

Yes the patch[1] where I missed the mode detection, over looked with
respect to previous one. thanks for identifying this. And the boards I
tested uses default read and write opcode that's the reason I
succeeded. Will squash your change on this.

[1] https://patchwork.ozlabs.org/patch/582615/

thanks!
-- 
Jagan.


More information about the U-Boot mailing list