On 5 July 2011 17:28, Alexander Graf <ag...@suse.de> wrote: > --- a/hw/pl080.c > +++ b/hw/pl080.c > @@ -199,10 +199,10 @@ again: > if (size == 0) { > /* Transfer complete. */ > if (ch->lli) { > - ch->src = ldl_phys(ch->lli); > - ch->dest = ldl_phys(ch->lli + 4); > - ch->ctrl = ldl_phys(ch->lli + 12); > - ch->lli = ldl_phys(ch->lli + 8); > + ch->src = ldl_le_phys(ch->lli); > + ch->dest = ldl_le_phys(ch->lli + 4); > + ch->ctrl = ldl_le_phys(ch->lli + 12); > + ch->lli = ldl_le_phys(ch->lli + 8); > } else { > ch->conf &= ~PL080_CCONF_E; > }
(Mostly this email is to get this info into the archive before I forget; we talked about it on IRC. It's not intended to be a nak, which is just as well given the patch is already committed :-)) This is no worse than the existing code, but it's not actually the right behaviour. Bit 0 of ch->lli indicates which AHB master we make the request on; you use that to identify which of bits 1 and 2 in the DMACConfiguration Register to test to determine whether to be little endian or big endian for each master: if (ch->conf & (PL080_CONF_M1 << (ch->lli & 1))) { bigendian; } else { littleendian; } However since that function pl080_run() has this early on: hw_error("DMA active\n"); we'll never reach this code, so it's all a bit moot :-) (There are no doubt other bits of that code which would need fixing to properly support bigendian DMA too, which is a bit much effort for functionality that isn't used.) -- PMM