Hi Stephen,
On 25 November 2014 at 09:44, Stephen Warren swarren@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@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