[U-Boot] Raspberry Pi with driver model

[Simon Glass]

Hi Stephen,

On 25 November 2014 at 09:44, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 11/24/2014 09:32 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 24 November 2014 at 21:21, Stephen Warren <swarren at nvidia.com> wrote:
>>>
>>> On 11/24/2014 08:58 AM, Simon Glass wrote:
>>>>
>>>> Hi Stephen,
>>>>
>>>> There was another thread where you reported a hang when booting
>>>> Raspberry Pi with driver model. It can be repeated by applying this
>>>> patch and trying to boot with syslinux.
>>>>
>>>> http://patchwork.ozlabs.org/patch/392180/
>>>>
>>>> When it hangs, the HDMI display displays lots of spaces, then lots of
>>>> dots, scrolling forever. The serial console displays nothing further.
>>>>
>>>> I narrowed the problem down to writing to address 0. So it is easy to
>>>> repeat with something like 'mw 0 0'.
>>>>
>>>> However I'm really not sure what is going on. Something with driver
>>>> model seems to make address 0 sensitive to writes. I can't see what
>>>> that might be, but I don't understand the platform very well. Do you
>>>> have any ideas?
>>>
>>>
>>> The problem seems to be that inside pl01x_serial_probe() (well, I assume
>>> all serial functions), dev_get_priv() returns 0, which ends up using
>>> address 0 as the UART address since that's the first field in struct
>>> pl01x_priv.
>>>
>>> I found this by amending pl01x_serial_probe() as follows, and dumping
>>> the data after boot:
>>>
>>> uint32_t *log = (uint32_t *)(128*1024*1024);
>>> uint32_t log_i = 1;
>>>
>>> static int pl01x_serial_probe(struct udevice *dev)
>>> {
>>>          struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
>>>          struct pl01x_priv *priv = dev_get_priv(dev);
>>>
>>>          log[log_i++] = (uint32_t)dev;
>>>          log[log_i++] = (uint32_t)plat;
>>>          log[log_i++] = (uint32_t)priv;
>>>          log[0] = log_i;
>>>          priv->regs = (struct pl01x_regs *)plat->base;
>>>          priv->type = plat->type;
>>>          return pl01x_generic_serial_init(priv->regs, priv->type);
>>> }
>>>
>>> U-Boot> md.l 0x08000000
>>> 08000000: 00000007 07fffaf8 0003972c 00000000    ........,.......
>>> 08000010: 1db4e108 1df8372c 00000000 00000000    ....,7..........
>>>
>>> It seems a bit odd that pl01x_serial_probe() is called twice, but
>>> perhaps that's expected?
>>
>>
>> Thanks for finding that! It just needs this in the U_BOOT_DRIVER()
>> declaration in the serial driver:
>>
>> .priv_auto_alloc_size = sizeof(struct pl01x_priv),
>>
>> Then it all works. I'll update the series and resend. Thanks again.
>
>
> Does it make sense to add an error check for one or more of the following to
> the driver core:
>
> a) Drivers/devices with .priv_auto_alloc_size == 0.

That is perfectly valid if drivers want to alloc their own space.

>
> b) Devices with NULL priv fields, when the core is about to call the
> driver-specific probe() function.

Also valid :-(

I did write a longer screed but my plan is for dev_get_priv() to have
checks as to what it is actually returning. I've done this a few times
in life and it's not very hard. That's why I'm trying to access things
through functions.

Regards,
Simon