[U-Boot] [RFC PATCH v2 00/11] SF: Migrate to Linux SPI NOR framework

Simon Goldschmidt
2018-12-06T16:36:23Z

Am 06.12.2018 um 14:54 schrieb Simon Goldschmidt:
> On Thu, Dec 6, 2018 at 2:45 PM Vignesh R <vigneshr@ti.com> wrote:
>>
>> Hi Simon,
>>
>> On 06/12/18 2:15 AM, Simon Goldschmidt wrote:
>>> Am 05.12.2018 um 07:55 schrieb Simon Goldschmidt:
>>>> On Wed, Dec 5, 2018 at 7:51 AM Vignesh R <vigneshr@ti.com> wrote:
>> [...]
>>>>>> I did some compilation tests first and I'm happy to say that with the
>>>>>> SPL_SPI_FLASH_TINY option enabled, my SPL is about 1900 byte smaller
>>>>>> than before :-)
>>>>>>
>>>>>> However, my socfpga socrates board does not boot. I'll have to
>>>>>> investigate why. I just applied this series and compiled for
>>>>>> socfpga_socrates_defconfig. Is there anything else I should have changed
>>>>>> to make it work?
>>>>>>
>>>>>
>>>>> Oops, that's unfortunate.
>>>>> Just to be sure, does SPL fail to come up or SPL fails to load U-Boot
>>>>> from flash? Is this with SPL_SPI_FLASH_TINY enabled?
>>>>
>>>> I tried both TINY and standard. Both are failing to load U-Boot (I get
>>>> the standard error message that loading from SPI flash failed).
>>>>
>>>>> Could you enable debug prints at the end of spi_mem_exec_op() in
>>>>> drivers/spi/spi-mem.c to see what commands are sent and their responses?
>>>>
>>>> OK, I'll do that.
>>>>
>>>>> I have TI EVM with Cadence QSPI(like SoCFPGA) but with a Spansion flash
>>>>> and that seems to work fine with both full and tiny stack.
>>>
>>> OK, so I enabled some debugging and the first issue is that U-Boot has
>>> SNOR_MFR_MICRON and SNOR_MFR_ST while Linux only has SNOR_MFR_MICRON but
>>> defines it to CFI_MFR_ST (0x20, U-Boot ends up with 0x2c). While this
>>> might be correct, it leads to my n25q256a not being handled as
>>> "micron-like" and the code falls through to using Spansion-like opcode
>>> (0x17) that my chip does not understand.
>>>
>>
>> Oops, sorry, the micron chip I had has CFI MFR id of 0x2c, so I
>> overlooked this. I have updated this now.
>>
>>> However, after changing this (and using opcode 0xb7 to bring the chip
>>> into 4 byte mode, including 0x06 for write-enable), it still does not
>>> work as it uses the 'READ' opcode 0x03 instead of the 'FAST READ' opcode
>>> 0x0b that U-Boot used until now. The problem here might be that 'READ'
>>> cannot do Quad-SPI (4-4-4) and I'm not sure the Cadence driver can
>>> handle Extended-SPI (1-1-1) that the 'READ' command is limited to...
>>>
>>
>> I had accidentally dropped Fast Read from SPI NOR layer and have sync'd
>> it up now. I see that in socfpga dts, flash does not contain:
>> spi-rx-bus-width = <4>;
>
> We can add that if it helps. I'll try to update the upstream Linux dts
> anyway to add "jeded,spi-nor" compatible once Neil's series is
> applied.
>
>> Therefore SPI NOR layer falls back to READ (even though its a QSPI
>> flash). But nevertheless with updated branch[1] you should see falling
>> back to FAST READ with Cadence QSPI on your board (and 1-1-4, if rx bus
>> width is 4).
>>
>>> Using SFDP would probably help, but that doesn't compile (as I wrote in
>>> the other thread).
>>>
>>
>> Sorry for that. I have fixed that up and tested SFDP parsing logic with
>> a Spansion flash. Note that Cadence QSPI wont support SFDP parsing as
>> driver enables Quad mode for all reads and SFDP is only 1-1-1.
>
> Oh, I haven't really looked into that yet. That's sad.
>
>>
>>> On the other hand, enabling SFDP will increase the text/rodata size for
>>> SPL. We might need to remove the non-SFDP and write code as a counter
>>> measure to prevent increasing code size.
>>>
>>
>> That's not an option as Flash devices (even though have same JEDEC ID)
>> manufactured before JESD216 std dont have SFDP populated. So, we will
>> need to have both non-SFDP and SFDP support with option to disable SFDP
>> (for size and boot time optimizations).
>
> Well, my idea was if you'd know that all the flash chips on your board
> supported it, you'd be safe. It would only be a Kconfig option to
> reduce binary size (get rid of the flash IDs table).
>
> But if SFDP doesn't work for QSPI anyway, that's not the best idea, I guess...
>
>> Thanks for all the debugging! It would be great if you could try below
>> branch and let me know if it fixes all the issues.
>> If yes, I will remove RFC and post new series.
>
> Sure, I'll try to find the time soon.

Had a quick test with both standard and tiny in SPL and both configs now
work on my board. I haven't tested SFDP as you said it does not works on
Cadence QSPI. (I tested compiling it though, and that works now.)

Oh, and I think it's important to say that with your spi-nor-tiny code,
SPL code size is reduced by 1980 bytes for my config
(socfpga_socrates_defconfig)!

Regards,
Simon

>
> Regards,
> Simon
>
>>
>> [1] https://github.com/r-vignesh/u-boot.git spi-nor-mig-patch-v1
>>
>> --
>> Regards
>> Vignesh