Patchwork [6/9] pl080: use specific endian ld/st_phys

login
register
mail settings
Submitter Alexander Graf
Date July 5, 2011, 4:28 p.m.
Message ID <1309883290-17595-7-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/103368/
State New
Headers show

Comments

Alexander Graf - July 5, 2011, 4:28 p.m.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/pl080.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Peter Maydell - July 12, 2011, 8:46 p.m.
On 5 July 2011 17:28, Alexander Graf <agraf@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

Patch

diff --git a/hw/pl080.c b/hw/pl080.c
index 901f04a..dd8139b 100644
--- 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;
                 }