[Qemu-devel] [PATCH arm-devs v2 0/5] SD and SDHCI Fixes

[Qemu-devel] [PATCH arm-devs v2 0/5] SD and SDHCI Fixes

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Fixes found in SD and SDHCI found doing some corner case testing.

Changed from v1:
Corrected ACMD41 patch.
Moved ACMD41 patch to back of series.


Peter Crosthwaite (5):
  sd/sdhci.c: Only reset data_count on new commands
  sd/sdhci: Fix Buffer Write Ready interrupt
  sd/sdhci.c: Fix bdata_read DPRINT message
  sd/sdhci:ADMA: fix interrupt
  sd/sd.c: Fix "inquiry" ACMD41

 hw/sd/sd.c    |  3 ++-
 hw/sd/sdhci.c | 28 ++++++++++++++--------------
 2 files changed, 16 insertions(+), 15 deletions(-)

-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH arm-devs v2 1/5] sd/sdhci.c: Only reset data_count on new commands

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

The data_count variable was being reset on every transfer, including
DMA transfer resumptions. This is incorrect, it should only be set
on a new command.

Manifests as a bug when using ADMA and there is a timer delay between
ADMA frames where the fifo is left in a non empty state.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
---

 hw/sd/sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 91dc9b0..0a84540 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -260,6 +260,7 @@ static void sdhci_send_command(SDHCIState *s)
     sdhci_update_irq(s);
 
     if (s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
+        s->data_count = 0;
         sdhci_do_data_transfer(s);
     }
 }
@@ -773,7 +774,6 @@ static void sdhci_do_adma(SDHCIState *s)
 static void sdhci_data_transfer(SDHCIState *s)
 {
     SDHCIClass *k = SDHCI_GET_CLASS(s);
-    s->data_count = 0;
 
     if (s->trnmod & SDHC_TRNS_DMA) {
         switch (SDHC_DMA_TYPE(s->hostctl)) {
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH arm-devs v2 2/5] sd/sdhci: Fix Buffer Write Ready interrupt

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

This interrupt is not risen after the last block is written to sd. It
is mutually exclusive with the end of transfer conditions. Fix.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
---

 hw/sd/sdhci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 0a84540..ea510b5 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -405,15 +405,14 @@ static void sdhci_write_block_to_card(SDHCIState *s)
 
     /* Next data can be written through BUFFER DATORT register */
     s->prnsts |= SDHC_SPACE_AVAILABLE;
-    if (s->norintstsen & SDHC_NISEN_WBUFRDY) {
-        s->norintsts |= SDHC_NIS_WBUFRDY;
-    }
 
     /* Finish transfer if that was the last block of data */
     if ((s->trnmod & SDHC_TRNS_MULTI) == 0 ||
             ((s->trnmod & SDHC_TRNS_MULTI) &&
             (s->trnmod & SDHC_TRNS_BLK_CNT_EN) && (s->blkcnt == 0))) {
         SDHCI_GET_CLASS(s)->end_data_transfer(s);
+    } else if (s->norintstsen & SDHC_NISEN_WBUFRDY) {
+        s->norintsts |= SDHC_NIS_WBUFRDY;
     }
 
     /* Generate Block Gap Event if requested and if not the last block */
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH arm-devs v2 3/5] sd/sdhci.c: Fix bdata_read DPRINT message

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

This message was printing out the data in decimal only, which is not
very friendly to the debugging developer. Add hex variant in
parenthesis to make it consistent with other similar messages in this
module.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---

 hw/sd/sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index ea510b5..15345dc 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -880,7 +880,8 @@ static uint32_t sdhci_read(SDHCIState *s, unsigned int offset, unsigned size)
     case  SDHC_BDATA:
         if (sdhci_buff_access_is_sequential(s, offset - SDHC_BDATA)) {
             ret = SDHCI_GET_CLASS(s)->bdata_read(s, size);
-            DPRINT_L2("read %ub: addr[0x%04x] -> %u\n", size, offset, ret);
+            DPRINT_L2("read %ub: addr[0x%04x] -> %u(0x%x)\n", size, offset,
+                      ret, ret);
             return ret;
         }
         break;
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH arm-devs v2 4/5] sd/sdhci:ADMA: fix interrupt

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

The end of transfer check was occurring and potentially returning before
the interrupt flag was checked. This means the interrupt will be missed
if it occurs on the last packet. Fix by checking for the interrupt
before checking for the end of transfer.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
---

 hw/sd/sdhci.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 15345dc..e64899c 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -730,6 +730,15 @@ static void sdhci_do_adma(SDHCIState *s)
             break;
         }
 
+        if (dscr.attr & SDHC_ADMA_ATTR_INT) {
+            DPRINT_L1("ADMA interrupt: admasysaddr=0x%lx\n", s->admasysaddr);
+            if (s->norintstsen & SDHC_NISEN_DMA) {
+                s->norintsts |= SDHC_NIS_DMA;
+            }
+
+            sdhci_update_irq(s);
+        }
+
         /* ADMA transfer terminates if blkcnt == 0 or by END attribute */
         if (((s->trnmod & SDHC_TRNS_BLK_CNT_EN) &&
                     (s->blkcnt == 0)) || (dscr.attr & SDHC_ADMA_ATTR_END)) {
@@ -752,15 +761,6 @@ static void sdhci_do_adma(SDHCIState *s)
             return;
         }
 
-        if (dscr.attr & SDHC_ADMA_ATTR_INT) {
-            DPRINT_L1("ADMA interrupt: admasysaddr=0x%lx\n", s->admasysaddr);
-            if (s->norintstsen & SDHC_NISEN_DMA) {
-                s->norintsts |= SDHC_NIS_DMA;
-            }
-
-            sdhci_update_irq(s);
-            return;
-        }
     }
 
     /* we have unfinished business - reschedule to continue ADMA */
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH arm-devs v2 5/5] sd/sd.c: Fix "inquiry" ACMD41

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

QEMU models two (of the three) ACMD41 has two modes, "inquiry" and
"first". The selection logic for which of the two is incorrect - it
compares != 0 for the entire argument value rather than only bits 23:0
as per the spec. Fix.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Changed since v1:
Total rewrite

 hw/sd/sd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2e0ef3e..4d26578 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1278,8 +1278,9 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         switch (sd->state) {
         case sd_idle_state:
             /* We accept any voltage.  10000 V is nothing.  */
-            if (req.arg)
+            if (req.arg & 0x00ffffff) {
                 sd->state = sd_ready_state;
+            }
 
             return sd_r3;
 
-- 
1.8.3.rc1.44.gb387c77.dirty

Re: [Qemu-devel] [PATCH arm-devs v2 5/5] sd/sd.c: Fix "inquiry" ACMD41

[Peter Maydell]

On 24 May 2013 06:18,  <peter.crosthwaite@xilinx.com> wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> QEMU models two (of the three) ACMD41 has two modes, "inquiry" and
> "first". The selection logic for which of the two is incorrect - it
> compares != 0 for the entire argument value rather than only bits 23:0
> as per the spec. Fix.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> Changed since v1:
> Total rewrite
>
>  hw/sd/sd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 2e0ef3e..4d26578 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1278,8 +1278,9 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>          switch (sd->state) {
>          case sd_idle_state:
>              /* We accept any voltage.  10000 V is nothing.  */
> -            if (req.arg)
> +            if (req.arg & 0x00ffffff) {
>                  sd->state = sd_ready_state;
> +            }
>
>              return sd_r3;

Could we have a comment here please?

thanks
-- PMM

Re: [Qemu-devel] [PATCH arm-devs v2 0/5] SD and SDHCI Fixes

[Peter Maydell]

On 24 May 2013 06:14,  <peter.crosthwaite@xilinx.com> wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Fixes found in SD and SDHCI found doing some corner case testing.
>
> Changed from v1:
> Corrected ACMD41 patch.
> Moved ACMD41 patch to back of series.

Patches 1-4 (but not the ACMD41 one) queued to arm-devs.next.

thanks
-- PMM