| Message ID | 1325967077-10130-1-git-send-email-marek.vasut@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 7 January 2012 20:11, Marek Vasut <marek.vasut@gmail.com> wrote: > The AMBA IDs are supposed to be at the end of 0x2000 block, which the PL011 UART > allocates. Current QEMU implementation puts those IDs at 0x1000 offset, which is > wrong. The QEMU implementation also allocates only 0x1000 instead of 0x2000 of > space. Why do you think this change is correct? The PL011 TRM http://infocenter.arm.com/help/topic/com.arm.doc.ddi0183g/I18381.html says the ID registers are at 0xFE0..0xFFC. and for instance on the PBX-A9 devboard: http://infocenter.arm.com/help/topic/com.arm.doc.dui0440b/Bbajihec.html the UARTs are at 0x10009000, 0x1000A000, 0x1000B000, 0x1000C000, so they clearly can't be 0x2000 in size. -- PMM
> On 7 January 2012 20:11, Marek Vasut <marek.vasut@gmail.com> wrote: > > The AMBA IDs are supposed to be at the end of 0x2000 block, which the > > PL011 UART allocates. Current QEMU implementation puts those IDs at > > 0x1000 offset, which is wrong. The QEMU implementation also allocates > > only 0x1000 instead of 0x2000 of space. > > Why do you think this change is correct? The PL011 TRM > http://infocenter.arm.com/help/topic/com.arm.doc.ddi0183g/I18381.html > says the ID registers are at 0xFE0..0xFFC. > and for instance on the PBX-A9 devboard: > http://infocenter.arm.com/help/topic/com.arm.doc.dui0440b/Bbajihec.html > the UARTs are at 0x10009000, 0x1000A000, 0x1000B000, 0x1000C000, so > they clearly can't be 0x2000 in size. Then we have a problem, because eg. on freescale mx28 they are 0x2000 big. The only conclusion I can draw from this is that the size of the segment that can be assigned to PL011 is variable and the ID-octet is always at the end. M > > -- PMM
On 7 January 2012 20:56, Marek Vasut <marek.vasut@gmail.com> wrote: >> On 7 January 2012 20:11, Marek Vasut <marek.vasut@gmail.com> wrote: >> > The AMBA IDs are supposed to be at the end of 0x2000 block, which the >> > PL011 UART allocates. Current QEMU implementation puts those IDs at >> > 0x1000 offset, which is wrong. The QEMU implementation also allocates >> > only 0x1000 instead of 0x2000 of space. >> >> Why do you think this change is correct? The PL011 TRM >> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0183g/I18381.html >> says the ID registers are at 0xFE0..0xFFC. >> and for instance on the PBX-A9 devboard: >> http://infocenter.arm.com/help/topic/com.arm.doc.dui0440b/Bbajihec.html >> the UARTs are at 0x10009000, 0x1000A000, 0x1000B000, 0x1000C000, so >> they clearly can't be 0x2000 in size. > > Then we have a problem, because eg. on freescale mx28 they are 0x2000 big. The > only conclusion I can draw from this is that the size of the segment that can be > assigned to PL011 is variable and the ID-octet is always at the end. Can you point me to some documentation? I looked at http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf?fsrch=1&sr=7 (i.MX28 Applications Processor Reference Manual) and although it has a number of UARTs none of them looked obviously like PL011s. -- PMM
> On 7 January 2012 20:56, Marek Vasut <marek.vasut@gmail.com> wrote: > >> On 7 January 2012 20:11, Marek Vasut <marek.vasut@gmail.com> wrote: > >> > The AMBA IDs are supposed to be at the end of 0x2000 block, which the > >> > PL011 UART allocates. Current QEMU implementation puts those IDs at > >> > 0x1000 offset, which is wrong. The QEMU implementation also allocates > >> > only 0x1000 instead of 0x2000 of space. > >> > >> Why do you think this change is correct? The PL011 TRM > >> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0183g/I18381.html > >> says the ID registers are at 0xFE0..0xFFC. > >> and for instance on the PBX-A9 devboard: > >> http://infocenter.arm.com/help/topic/com.arm.doc.dui0440b/Bbajihec.html > >> the UARTs are at 0x10009000, 0x1000A000, 0x1000B000, 0x1000C000, so > >> they clearly can't be 0x2000 in size. > > > > Then we have a problem, because eg. on freescale mx28 they are 0x2000 > > big. The only conclusion I can draw from this is that the size of the > > segment that can be assigned to PL011 is variable and the ID-octet is > > always at the end. > > Can you point me to some documentation? I looked at > http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf?fsrch=1&s > r=7 (i.MX28 Applications Processor Reference Manual) and although it has a > number of UARTs none of them looked obviously like PL011s. That's it, look at DUART section and Memory map section. M > > -- PMM
On 8 January 2012 16:02, Marek Vasut <marek.vasut@gmail.com> wrote: >> Can you point me to some documentation? I looked at >> http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf?fsrch=1&s >> r=7 (i.MX28 Applications Processor Reference Manual) and although it has a >> number of UARTs none of them looked obviously like PL011s. > > That's it, look at DUART section and Memory map section. That doesn't document that it implements the ID registers at all :-) Does the hardware give the ID registers the same values as stock PL011 or are the part number/designer/etc fields different? Anyway, the right approach to this is going to be to have either (a) a qdev device with a different name [this is how we handle the pl061 variant in the Stellaris Luminary, for instance] (b) qdev properties so you can get the mx28 right. -- PMM
> On 8 January 2012 16:02, Marek Vasut <marek.vasut@gmail.com> wrote: > >> Can you point me to some documentation? I looked at > >> http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf?fsrch= > >> 1&s r=7 (i.MX28 Applications Processor Reference Manual) and although it > >> has a number of UARTs none of them looked obviously like PL011s. > > > > That's it, look at DUART section and Memory map section. > > That doesn't document that it implements the ID registers at all :-) Well ... can't be helped. > Does the hardware give the ID registers the same values as stock > PL011 or are the part number/designer/etc fields different? Slightly different, but compatible with Linux: 80075fe0: 00000011 00000010 00000024 00000000 ........$....... 80075ff0: 0000000d 000000f0 00000005 000000b1 ................ > > Anyway, the right approach to this is going to be to have either > (a) a qdev device with a different name [this is how we handle > the pl061 variant in the Stellaris Luminary, for instance] > (b) qdev properties > so you can get the mx28 right. This seems more correct/extensible approach. M > > -- PMM
On 8 January 2012 16:57, Marek Vasut <marek.vasut@gmail.com> wrote: >Peter Maydell wrote: >> Does the hardware give the ID registers the same values as stock >> PL011 or are the part number/designer/etc fields different? > > Slightly different, but compatible with Linux: > > 80075fe0: 00000011 00000010 00000024 00000000 ........$....... > 80075ff0: 0000000d 000000f0 00000005 000000b1 ................ OK, that's just the patchlevel -- QEMU currently reports itself as r1p1 (0x14 in UARTPeriphID2), and mx28 is r1p3 or r1p4 (0x24). The manual describes no functional differences between r1p0 up to r1p4 (r1p5 has a different fifo size) so I'd be inclined not to worry about the ID registers here. -- PMM
> On 8 January 2012 16:57, Marek Vasut <marek.vasut@gmail.com> wrote: > >Peter Maydell wrote: > >> Does the hardware give the ID registers the same values as stock > >> PL011 or are the part number/designer/etc fields different? > > > > Slightly different, but compatible with Linux: > > > > 80075fe0: 00000011 00000010 00000024 00000000 ........$....... > > 80075ff0: 0000000d 000000f0 00000005 000000b1 ................ > > OK, that's just the patchlevel -- QEMU currently reports > itself as r1p1 (0x14 in UARTPeriphID2), and mx28 is r1p3 > or r1p4 (0x24). The manual describes no functional differences > between r1p0 up to r1p4 (r1p5 has a different fifo size) so > I'd be inclined not to worry about the ID registers here. It's just their position that's different indeed. And even this can be tuned via props. I'll update the patch once I get FEC operational. M > > -- PMM
On 8 January 2012 19:44, Marek Vasut <marek.vasut@gmail.com> wrote: > It's just their position that's different indeed. And even this can be tuned via > props. I'll update the patch once I get FEC operational. Incidentally, since this is only needed for a platform we don't currently support, you should submit the patch as part of a series adding support for that platform. We don't generally take this kind of thing as a standalone patch, since there's no justification for it without the platform that needs it. -- PMM
> On 8 January 2012 19:44, Marek Vasut <marek.vasut@gmail.com> wrote: > > It's just their position that's different indeed. And even this can be > > tuned via props. I'll update the patch once I get FEC operational. > > Incidentally, since this is only needed for a platform we don't > currently support, you should submit the patch as part of a > series adding support for that platform. Yep, will do! > We don't generally > take this kind of thing as a standalone patch, since there's > no justification for it without the platform that needs it. Definitelly. > > -- PMM
diff --git a/hw/pl011.c b/hw/pl011.c index 1b05d76..49d4de0 100644 --- a/hw/pl011.c +++ b/hw/pl011.c @@ -60,8 +60,8 @@ static uint64_t pl011_read(void *opaque, target_phys_addr_t offset, pl011_state *s = (pl011_state *)opaque; uint32_t c; - if (offset >= 0xfe0 && offset < 0x1000) { - return s->id[(offset - 0xfe0) >> 2]; + if (offset >= 0x1fe0 && offset < 0x2000) { + return s->id[(offset - 0x1fe0) >> 2]; } switch (offset >> 2) { case 0: /* UARTDR */ @@ -260,7 +260,7 @@ static int pl011_init(SysBusDevice *dev, const unsigned char *id) { pl011_state *s = FROM_SYSBUS(pl011_state, dev); - memory_region_init_io(&s->iomem, &pl011_ops, s, "pl011", 0x1000); + memory_region_init_io(&s->iomem, &pl011_ops, s, "pl011", 0x2000); sysbus_init_mmio(dev, &s->iomem); sysbus_init_irq(dev, &s->irq); s->id = id;
The AMBA IDs are supposed to be at the end of 0x2000 block, which the PL011 UART allocates. Current QEMU implementation puts those IDs at 0x1000 offset, which is wrong. The QEMU implementation also allocates only 0x1000 instead of 0x2000 of space. The fix is tested to work with Linux's PL011 driver and U-Boot running in QEMU. Signed-off-by: Marek Vasut <marek.vasut@gmail.com> --- hw/pl011.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)