[U-Boot] socfpga qspi issues on SoCKit devkit

[Dinh Nguyen]

On 02/21/2017 08:02 AM, Rush, Jason A. wrote:
> Marek Vasut wrote:
>> On 02/20/2017 05:53 PM, Rush, Jason A. wrote:
>>> Marek Vasut wrote:
>>>> On 02/20/2017 05:25 AM, Vignesh R wrote:
>>>>> + Marek
>>>>
>>>> Thanks, +CC Dinh and Ley
>>>>
>>>>> On Friday 17 February 2017 05:02 AM, Rush, Jason A. wrote:
>>>>>> The QSPI NOR interface on the Altera Cyclone V SoCKit devkit (Rev
>>>>>> B) appears to be broken in the current release.  I've tracked it
>>>>>> down to working in the
>>>>>> v2016.07 release, but broken in the the v2016.09 release.  With the
>>>>>> help of git bisect, I tracked down the commit that breaks the QSPI to the following:
>>>>>>
>>>>>>   commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1
>>>>>>   Author: Vignesh R <vigneshr at ti.com>
>>>>>>   Date:   Wed Jul 6 10:20:55 2016 +0530
>>>>>>
>>>>>>       spi: cadence_qspi_apb: Support 32 bit AHB address
>>>>>>
>>>>>>       AHB address can be as long as 32 bit, hence remove the
>>>>>>       CQSPI_REG_INDIRECTRDSTARTADDR mask. Since AHB address is passed from DT
>>>>>>       and read as u32 value, it anyway does not make sense to mask upper bits.
>>>>>>
>>>>>>       Signed-off-by: Vignesh R <vigneshr at ti.com>
>>>>>>       Tested-by: Marek Vasut <marex at denx.de>
>>>>>>       Acked-by: Marek Vasut <marex at denx.de>
>>>>>>       Reviewed-by: Jagan Teki <jteki at openedev.com>
>>>>>>
>>>>>> When I try and read anything from the QSPI on the
>>>>>> dac3bf20fb2c9b03476be0d73db620f62ab3cee1 commit I get a "data
>>>>>> abort" and the CPU resets.  Example output below is from a clean
>>>>>> build of U-Boot configured with socfpga_sockit_defconfig:
>>>>>>
>>>>>>  => sf probe
>>>>>>   SF: Detected N25Q1024 with page size 256 Bytes, erase size 64 KiB, total 128 MiB
>>>>>>   => sf read 0x2000000 0x50000 0x5000
>>>>>>   device 0 offset 0x50000, size 0x5000
>>>>>>   data abort
>>>>>>   pc : [<3ff7a9bc>]          lr : [<3ff98359>]
>>>>>>   reloc pc : [<010249fc>]    lr : [<01042399>]
>>>>>>   sp : 3bf4fde8  ip : 3ff7a371     fp : 00000002
>>>>>>   r10: 00000000  r9 : 3bf54ee8     r8 : 3bf55530
>>>>>>   r7 : 0000270d  r6 : 02000000     r5 : 00005000  r4 : 3bf55530
>>>>>>   r3 : 00000004  r2 : 00000000     r1 : 02000000  r0 : ffa00000
>>>>>>   Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>>>>>>   Resetting CPU ...
>>>>>>
>>>>>> When I run the same commands with the previous commit in the git
>>>>>> log
>>>>>> (fdf02a36c52cb96717b64113775c4251ecd49596) I get the following output:
>>>>>>
>>>>>>   => sf probe
>>>>>>   SF: Detected N25Q1024 with page size 256 Bytes, erase size 64 KiB, total 128 MiB
>>>>>>   => sf read 0x2000000 0x50000 0x5000
>>>>>>   device 0 offset 0x50000, size 0x5000
>>>>>>   SF: 20480 bytes @ 0x50000 Read: OK
>>>>>>   =>
>>>>>>
>>>>>> I've done some investigation, and previously the ahbbase was masked
>>>>>> so the value
>>>>>> 0xFFA0_0000 results in 0x0 when writing the CQSPI_REG_INDIRECTTRIGGER register.
>>>>>> I assume the above commit works on some boards, but it appears to
>>>>>> break the socfpga arch.
>>>>>>
>>>>>> Is there someone that could help investigate or confirm this?
>>>>>
>>>>> I am looking at cyclone5_handbook.pdf [1] and it says indaddrtrig
>>>>> field is 32 bit wide. So, I wonder why masking is needed. If you
>>>>> specify ahbbase as  0xFFA0_0000 in DT(reg property) then the
>>>>> expectation is
>>>>> 0xFFA0_0000 gets written to indaddrtrig reg. It looks like the
>>>>> trigger address is 0x0 and not 0xFFA0_0000, therefore you may have
>>>>> to update reg property in DT to reflect the same.
>>>>>
>>>>> [1]https://www.altera.com/en_US/pdfs/literature/hb/cyclone-v/cyclone
>>>>> 5_
>>>>> handbook.pdf
>>>>>
>>>
>>> I agree, according to the handbook the indaddrtrig register is 32-bits
>>> wide, but writing the value 0xFFA0_0000 to indaddrtrig causes the
>>> above data abort.  It looks like a similar issue has been discussed before:
>>> http://lists.denx.de/pipermail/u-boot/2015-August/224649.html
>>>
>>> In that proposed patch, it looks like the address to use for the
>>> indaddrtrig reg was being separated from the ahbbase and the Altera
>>> SoC needed a value of 0x0000_0000.  I'm not very familiar with the
>>> Cadence device on the Altera SoC, but it looks like it needs a
>>> different value than the ahbbase written to the indaddrtrig reg.
>>>
>>> I noticed the Cadence QSPI driver in the Linux tree has a separate
>>> trigger_address that is loaded from the DT, and the socfpga.dtsi sets
>>> the trigger_address to 0x0000_0000.
>>
>> So let's just adopt the Linux bindings and get this fixed. Can you send a patch ? Thanks
> 
> I'll work on a patch.
> 
>> btw where did this 0xffa0_0000 come from, Dinh ?


0Xffa00000 is the starting address for the QSPI module's data portion.

Dinh