Re: [Qemu-devel] PL181 write problem

[Hans-Erik Floryd <hans-erik.floryd@rt-labs.com>]

>> The following patch fixes this by writing all four bytes before checking
>> fifo_len.
> 
> I think your analysis is probably correct, but I'm not so keen on your 
> solution. You've got redundant loops/conditions.
> 

It seemed like a good idea to not modify the read path since it works, 
but it does make the code a bit complicated. In the following patch I've 
broken read and write into separate loops which hopefully makes the code 
easier to follow. s->datacnt needs to be checked for every byte written 
or read since it might not be a multiple of 4.

Index: hw/pl181.c
===================================================================
--- hw/pl181.c  (revision 5648)
+++ hw/pl181.c  (working copy)
@@ -184,38 +184,28 @@
      uint32_t bits;
      uint32_t value;
      int n;
-    int limit;
      int is_read;

      is_read = (s->datactrl & PL181_DATA_DIRECTION) != 0;
      if (s->datacnt != 0 && (!is_read || sd_data_ready(s->card))
              && !s->linux_hack) {
-        limit = is_read ? PL181_FIFO_LEN : 0;
-        n = 0;
-        value = 0;
-        while (s->datacnt && s->fifo_len != limit) {
-            if (is_read) {
+       if (is_read) {
+          while (s->datacnt > 0 && s->fifo_len < PL181_FIFO_LEN) {
+             value = 0;
+             for (n = 0; n < 4 && s->datacnt > 0; n++, s->datacnt--) {
                  value |= (uint32_t)sd_read_data(s->card) << (n * 8);
-                n++;
-                if (n == 4) {
-                    pl181_fifo_push(s, value);
-                    value = 0;
-                    n = 0;
-                }
-            } else {
-                if (n == 0) {
-                    value = pl181_fifo_pop(s);
-                    n = 4;
-                }
+             }
+             pl181_fifo_push(s, value);
+          }
+       } else {
+          while (s->datacnt > 0 && s->fifo_len > 0) {
+             value = pl181_fifo_pop(s);
+             for (n = 0; n < 4 && s->datacnt > 0; n++, s->datacnt--) {
                  sd_write_data(s->card, value & 0xff);
                  value >>= 8;
-                n--;
-            }
-            s->datacnt--;
-        }
-        if (n && is_read) {
-            pl181_fifo_push(s, value);
-        }
+             }
+          }
+       }
      }
      s->status &= ~(PL181_STATUS_RX_FIFO | PL181_STATUS_TX_FIFO);
      if (s->datacnt == 0) {