[U-Boot] ZynqMP breakage

[Michal Simek]

Hi Simon,

On 6.9.2016 16:23, Simon Glass wrote:
> Hi Michal,
> 
> On 6 September 2016 at 07:40, Michal Simek <michal.simek at xilinx.com> wrote:
>> Hi,
>>
>> On 6.9.2016 14:57, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On 6 September 2016 at 06:55, Alexander Graf <agraf at suse.de> wrote:
>>>> On 09/06/2016 02:52 PM, Simon Glass wrote:
>>>>>
>>>>> Hi Alex,
>>>>>
>>>>> On 6 September 2016 at 03:09, Alexander Graf <agraf at suse.de> wrote:
>>>>>>
>>>>>> On 09/06/2016 03:05 AM, Simon Glass wrote:
>>>>>>>
>>>>>>> Hi Alex,
>>>>>>>
>>>>>>> On 5 September 2016 at 04:51, Alexander Graf <agraf at suse.de> wrote:
>>>>>>>>
>>>>>>>> On 08/19/2016 08:45 AM, Michal Simek wrote:
>>>>>>>>>
>>>>>>>>> On 16.8.2016 20:39, Alexander Graf wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Michal,
>>>>>>>>>>
>>>>>>>>>> I just tried to run the latest u-boot master + a few patches to
>>>>>>>>>> implement
>>>>>>>>>> generic PSCI RTS support on zynqmp and got this:
>>>>>>>>>>
>>>>>>>>>> e
>>>>>>>>>> U-Boot 2016.09-rc1-00453-ga0592f1 (Aug 16 2016 - 20:27:40 +0200)
>>>>>>>>>> Xilinx
>>>>>>>>>> ZynqMP ZCU102
>>>>>>>>>>
>>>>>>>>>> I2C:   ready
>>>>>>>>>> DRAM:  4 GiB
>>>>>>>>>> EL Level:       EL2
>>>>>>>>>> MMC:   sdhci at ff170000: 0
>>>>>>>>>> Using default environment
>>>>>>>>>>
>>>>>>>>>> In:    serial at ff000000
>>>>>>>>>> Out:   serial at ff000000
>>>>>>>>>> Err:   serial at ff000000
>>>>>>>>>> Bootmode: SD_MODE1
>>>>>>>>>> SCSI:  SATA link 0 timeout.
>>>>>>>>>> Target spinup took 0 ms.
>>>>>>>>>> AHCI 0001.0301 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
>>>>>>>>>> flags: 64bit ncq pm clo only pmp fbss pio slum part ccc apst
>>>>>>>>>> scanning bus for devices...
>>>>>>>>>> "Synchronous Abort" handler, esr 0x96000004
>>>>>>>>>> ELR:     7ff42d20
>>>>>>>>>> LR:      7ff3ff10
>>>>>>>>>> x0 : 0000000000000000 x1 : 0000000000000000
>>>>>>>>>> x2 : 0000000000000001 x3 : 000000007df1aa80
>>>>>>>>>> x4 : aaaaaaaaaaaaaaaa x5 : 0000000000000001
>>>>>>>>>> x6 : 0000000000000200 x7 : 0000000000000004
>>>>>>>>>> x8 : 000000007ff9f800 x9 : 0000000000000008
>>>>>>>>>> x10: 000000007ff9f8f0 x11: 0000000000000000
>>>>>>>>>> x12: 00000000ffffffff x13: 00000000ffffffff
>>>>>>>>>> x14: 000000007ff8242f x15: 000000007ff82435
>>>>>>>>>> x16: 000000007ff84388 x17: 000000007ff84388
>>>>>>>>>> x18: 000000007df1ade8 x19: 000000007df1aa80
>>>>>>>>>> x20: 000000007ff92cb8 x21: 000000007ff9f800
>>>>>>>>>> x22: 000000007ff9f000 x23: 0000000000000078
>>>>>>>>>> x24: 000000007ff9f8f0 x25: 0000000000000000
>>>>>>>>>> x26: 000000007ff9f800 x27: 000000007ff9f000
>>>>>>>>>> x28: 000000007ff9fb00 x29: 000000007df1aca0
>>>>>>>>>>
>>>>>>>>>> Resetting CPU ...
>>>>>>>>>>
>>>>>>>>>> resetting …
>>>>>>>>>>
>>>>>>>>>> Is this a known problem?
>>>>>>>>>
>>>>>>>>> Is this issue with usb?
>>>>>>>>> I have usb off on zcu102 that's why if this usb issue
>>>>>>>>> I am not aware about it.
>>>>>>>>
>>>>>>>>
>>>>>>>> After a bit of digging, turns out it's CONFIG_BLK. If I disable that
>>>>>>>> things
>>>>>>>> work again (albeit without mmc access, since that one requires
>>>>>>>> CONFIG_BLK
>>>>>>>> now).
>>>>>>>
>>>>>>> I don't have a zynqmp device, so cannot test with that unfortunately.
>>>>>>
>>>>>>
>>>>>> Well, QEMU supports zcu102 emulation in the latest version, so you could
>>>>>> use
>>>>>> that to emulate the board:
>>>>>>
>>>>>>    $ qemu-system-aarch64 -M xlnx-zcu102 -kernel u-boot.elf -nographic -m
>>>>>> 2G
>>>>>> -drive file=u-boot,id=d,if=none -device ide-drive,drive=d,bus=ide.0
>>>>>>
>>>>>> However, I don't see the data abort with the emulated device, only with
>>>>>> actual hardware. Probably because real hardware is more upset about
>>>>>> reading
>>>>>> from address 0 ;). But I can provoke the oops even in QEMU if I unmap the
>>>>>> first page from the memory map using the patch below.
>>>>>>
>>>>>> The oops happens in blk_dread because block_dev->bdev is NULL.
>>>>>
>>>>> Yes, this field really should be removed. As the comment says it's a
>>>>> hangover from pre-CONFIG_BLK where functions use a struct blk_dev * to
>>>>> specify the block device instead of a struct udevice *. It came up
>>>>> recently on a patch you sent also which is why I am so against using
>>>>> it.
>>>>>
>>>>> That said, I'm not sure why it is unset. The path should be:
>>>>>
>>>>> sdhci_bind()
>>>>> mmc_bind()
>>>>> blk_create_devicef()
>>>>> blk_create_device()
>>>>> which sets:
>>>>>
>>>>> desc->bdev = dev;
>>>>>
>>>>> [snip]
>>>>
>>>>
>>>> The special case about ZynqMP is that it has 2 storage backends, one that
>>>> uses DM (the sdhc controller) and one that isn't converted to DM (the AHCI
>>>> controller). I guess something goes wrong with the latter.
>>>
>>> OK, well it would be good to convert it. It certainly won't work
>>> without it and I'm a little surprised that it compiles :-)
>>
>> probably good time to convert it. Driver itself has only sata init
>> sequence and then it is just compatible with ahci. That's why conversion
>> should be pretty easy.
>>
>> Simon: Which driver should I use as inspiration for conversion?
> 
> I looked at sata a while back, so here are a few ideas...
> 
> Existing sata.h functions should be #ifdef'd out
> 
> - init_sata() should be handled by the driver probe() method
> - hopefully sata_stop() can be handled by driver remove() method
> - the block device can handle read and write
> - we probably need sata methods for scan and reset
> 
> There is also ahci.h.
> 
> There is a sata_sandbox.c driver which you could convert to DM,
> perhaps with a new DM_SATA option (like DM_MMC).

I have sent RFC for moving ceva driver to DM with some core changes.
There will be probably a need to test it on platform which is capable to
connect more HDDs to make sure that everything is correct.

Thanks,
Michal