[Qemu-devel] [PATCH v3 00/18] SDCard: bugfixes, support UHS-I (part 5)

[Qemu-devel] [PATCH v3 00/18] SDCard: bugfixes, support UHS-I (part 5)

[Philippe Mathieu-Daudé]

Some refactors, few bugfixes, better SD/SPI support.

With this series apply, machines can use cards in UHS-I mode.
(mostly imported from Alistair Francis work)

MMC mode split out for another series.

Since v2:
- split again in 2... other part is cleanup/tracing

Since v1:
- rewrote mostly all patches to keep it simpler.

$ git backport-diff
001/18:[0003] [FC] 'sdcard: Don't always set the high capacity bit'
002/18:[down] 'sdcard: update the CSD CRC register regardless the CSD structure version'
003/18:[down] 'sdcard: fix the 'maximum data transfer rate' to 25MHz'
004/18:[down] 'sdcard: clean the SCR register and add few comments'
005/18:[down] 'sdcard: remove commands from unsupported old MMC specification'
006/18:[down] 'sdcard: simplify using the ldst API'
007/18:[down] 'sdcard: use the correct masked OCR in the R3 reply'
008/18:[down] 'sdcard: use the registerfields API for the CARD_STATUS register masks'
009/18:[down] 'sdcard: handles more commands in SPI mode'
010/18:[down] 'sdcard: handle CMD54 (SDIO)'
011/18:[down] 'sdcard: check the card is in correct state for APP CMD (CMD55)'
012/18:[down] 'sdcard: warn if host uses an incorrect address for APP CMD (CMD55)'
013/18:[down] 'sdcard: simplify SEND_IF_COND (CMD8)'
014/18:[down] 'sdcard: simplify SD_SEND_OP_COND (ACMD41)'
015/18:[down] 'sdcard: add SD SEND_TUNING_BLOCK (CMD19)'
016/18:[down] 'sdcard: add a 'uhs' property, update the OCR register ACCEPT_SWITCH_1V8 bit'
017/18:[down] 'sdcard: implement the UHS-I SWITCH_FUNCTION entries (Spec v3)'
018/18:[0006] [FC] 'sdcard: add an enum for the SD PHY Spec version'

Based-on: 20180123032135.28863-13-f4bug@amsat.org

Philippe Mathieu-Daudé (18):
  sdcard: Don't always set the high capacity bit
  sdcard: update the CSD CRC register regardless the CSD structure version
  sdcard: fix the 'maximum data transfer rate' to 25MHz
  sdcard: clean the SCR register and add few comments
  sdcard: remove commands from unsupported old MMC specification
  sdcard: simplify using the ldst API
  sdcard: use the correct masked OCR in the R3 reply
  sdcard: use the registerfields API for the CARD_STATUS register masks
  sdcard: handles more commands in SPI mode
  sdcard: handle CMD54 (SDIO)
  sdcard: check the card is in correct state for APP CMD (CMD55)
  sdcard: warn if host uses an incorrect address for APP CMD (CMD55)
  sdcard: simplify SEND_IF_COND (CMD8)
  sdcard: simplify SD_SEND_OP_COND (ACMD41)
  sdcard: add SD SEND_TUNING_BLOCK (CMD19)
  sdcard: add a 'uhs' property, update the OCR register ACCEPT_SWITCH_1V8 bit
  sdcard: implement the UHS-I SWITCH_FUNCTION entries (Spec v3)
  sdcard: add an enum for the SD PHY Spec version

 hw/sd/sd.c         | 501 +++++++++++++++++++++++++++++++++++++----------------
 hw/sd/trace-events |   1 +
 2 files changed, 354 insertions(+), 148 deletions(-)

-- 
2.15.1

[Qemu-devel] [PATCH v3 01/18] sdcard: Don't always set the high capacity bit

[Philippe Mathieu-Daudé]

Don't set the high capacity bit by default as it will be set if required
in the sd_set_csd() function.

[based on a patch from Alistair Francis <alistair.francis@xilinx.com>
 and Peter Ogden <ogden@xilinx.com> from qemu/xilinx tag xilinx-v2015.4]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 437ce25f79..20764451f2 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -289,6 +289,10 @@ static void sd_ocr_powerup(void *opaque)
 
     /* card power-up OK */
     sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
+
+    if (sd->size > 1 * G_BYTE) {
+        sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
+    }
 }
 
 static void sd_reset_scr(SDState *sd)
@@ -393,7 +397,6 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
         sd->csd[13] = 0x40;
         sd->csd[14] = 0x00;
         sd->csd[15] = 0x00;
-        sd->ocr |= 1 << 30;     /* High Capacity SD Memory Card */
     }
 }
 
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 01/18] sdcard: Don't always set the high capacity bit

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Don't set the high capacity bit by default as it will be set if required
> in the sd_set_csd() function.
>
> [based on a patch from Alistair Francis <alistair.francis@xilinx.com>
>  and Peter Ogden <ogden@xilinx.com> from qemu/xilinx tag xilinx-v2015.4]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 437ce25f79..20764451f2 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -289,6 +289,10 @@ static void sd_ocr_powerup(void *opaque)
>
>      /* card power-up OK */
>      sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
> +
> +    if (sd->size > 1 * G_BYTE) {
> +        sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
> +    }
>  }
>
>  static void sd_reset_scr(SDState *sd)
> @@ -393,7 +397,6 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
>          sd->csd[13] = 0x40;
>          sd->csd[14] = 0x00;
>          sd->csd[15] = 0x00;
> -        sd->ocr |= 1 << 30;     /* High Capacity SD Memory Card */
>      }
>  }
>
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 02/18] sdcard: update the CSD CRC register regardless the CSD structure version

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 20764451f2..f6318c6fdb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -377,7 +377,6 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
         sd->csd[13] = 0x20 |	/* Max. write data block length */
             ((HWBLOCK_SHIFT << 6) & 0xc0);
         sd->csd[14] = 0x00;	/* File format group */
-        sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
     } else {			/* SDHC */
         size /= 512 * 1024;
         size -= 1;
@@ -396,8 +395,8 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
         sd->csd[12] = 0x0a;
         sd->csd[13] = 0x40;
         sd->csd[14] = 0x00;
-        sd->csd[15] = 0x00;
     }
+    sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
 }
 
 static void sd_reset_rca(SDState *sd)
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 02/18] sdcard: update the CSD CRC register regardless the CSD structure version

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 20764451f2..f6318c6fdb 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -377,7 +377,6 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
>          sd->csd[13] = 0x20 |   /* Max. write data block length */
>              ((HWBLOCK_SHIFT << 6) & 0xc0);
>          sd->csd[14] = 0x00;    /* File format group */
> -        sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
>      } else {                   /* SDHC */
>          size /= 512 * 1024;
>          size -= 1;
> @@ -396,8 +395,8 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
>          sd->csd[12] = 0x0a;
>          sd->csd[13] = 0x40;
>          sd->csd[14] = 0x00;
> -        sd->csd[15] = 0x00;
>      }
> +    sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
>  }
>
>  static void sd_reset_rca(SDState *sd)
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 03/18] sdcard: fix the 'maximum data transfer rate' to 25MHz

[Philippe Mathieu-Daudé]

To comply with Spec v1.10 (and 2.00, 3.01):

. TRAN_SPEED

for current SD Memory Cards that field must be always 0_0110_010b (032h) which is
equal to 25MHz - the mandatory maximum operating frequency of SD Memory Card.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f6318c6fdb..56df5b660a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -356,7 +356,7 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
         sd->csd[0] = 0x00;	/* CSD structure */
         sd->csd[1] = 0x26;	/* Data read access-time-1 */
         sd->csd[2] = 0x00;	/* Data read access-time-2 */
-        sd->csd[3] = 0x5a;	/* Max. data transfer rate */
+        sd->csd[3] = 0x32;  /* Max. data transfer rate: 25 MHz */
         sd->csd[4] = 0x5f;	/* Card Command Classes */
         sd->csd[5] = 0x50 |	/* Max. read data block length */
             HWBLOCK_SHIFT;
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 03/18] sdcard: fix the 'maximum data transfer rate' to 25MHz

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> To comply with Spec v1.10 (and 2.00, 3.01):
>
> . TRAN_SPEED
>
> for current SD Memory Cards that field must be always 0_0110_010b (032h) which is
> equal to 25MHz - the mandatory maximum operating frequency of SD Memory Card.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index f6318c6fdb..56df5b660a 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -356,7 +356,7 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
>          sd->csd[0] = 0x00;     /* CSD structure */
>          sd->csd[1] = 0x26;     /* Data read access-time-1 */
>          sd->csd[2] = 0x00;     /* Data read access-time-2 */
> -        sd->csd[3] = 0x5a;     /* Max. data transfer rate */
> +        sd->csd[3] = 0x32;  /* Max. data transfer rate: 25 MHz */
>          sd->csd[4] = 0x5f;     /* Card Command Classes */
>          sd->csd[5] = 0x50 |    /* Max. read data block length */
>              HWBLOCK_SHIFT;
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 04/18] sdcard: clean the SCR register and add few comments

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 56df5b660a..5d50d48097 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -297,10 +297,13 @@ static void sd_ocr_powerup(void *opaque)
 
 static void sd_reset_scr(SDState *sd)
 {
-    sd->scr[0] = 0x00;		/* SCR Structure */
-    sd->scr[1] = 0x2f;		/* SD Security Support */
-    sd->scr[2] = 0x00;
+    sd->scr[0] = (0 << 4)       /* SCR version 1.0 */
+                 | 0;           /* Spec Versions 1.0 and 1.01 */
+    sd->scr[1] = (2 << 4)       /* SDSC Card (Security Version 1.01) */
+                 | 0b0101;      /* 1-bit or 4-bit width bus modes */
+    sd->scr[2] = 0x00;          /* Extended Security is not supported. */
     sd->scr[3] = 0x00;
+    /* reserved for manufacturer usage */
     sd->scr[4] = 0x00;
     sd->scr[5] = 0x00;
     sd->scr[6] = 0x00;
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 04/18] sdcard: clean the SCR register and add few comments

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 56df5b660a..5d50d48097 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -297,10 +297,13 @@ static void sd_ocr_powerup(void *opaque)
>
>  static void sd_reset_scr(SDState *sd)
>  {
> -    sd->scr[0] = 0x00;         /* SCR Structure */
> -    sd->scr[1] = 0x2f;         /* SD Security Support */
> -    sd->scr[2] = 0x00;
> +    sd->scr[0] = (0 << 4)       /* SCR version 1.0 */
> +                 | 0;           /* Spec Versions 1.0 and 1.01 */
> +    sd->scr[1] = (2 << 4)       /* SDSC Card (Security Version 1.01) */
> +                 | 0b0101;      /* 1-bit or 4-bit width bus modes */
> +    sd->scr[2] = 0x00;          /* Extended Security is not supported. */
>      sd->scr[3] = 0x00;
> +    /* reserved for manufacturer usage */
>      sd->scr[4] = 0x00;
>      sd->scr[5] = 0x00;
>      sd->scr[6] = 0x00;
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 05/18] sdcard: remove commands from unsupported old MMC specification

[Philippe Mathieu-Daudé]

This device does not model MMCA Specification previous to v4.2

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 33 ---------------------------------
 1 file changed, 33 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 5d50d48097..3c66521862 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1044,24 +1044,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 11:	/* CMD11:  READ_DAT_UNTIL_STOP */
-        if (sd->spi)
-            goto bad_cmd;
-        switch (sd->state) {
-        case sd_transfer_state:
-            sd->state = sd_sendingdata_state;
-            sd->data_start = req.arg;
-            sd->data_offset = 0;
-
-            if (sd->data_start + sd->blk_len > sd->size)
-                sd->card_status |= ADDRESS_ERROR;
-            return sd_r0;
-
-        default:
-            break;
-        }
-        break;
-
     case 12:	/* CMD12:  STOP_TRANSMISSION */
         switch (sd->state) {
         case sd_sendingdata_state:
@@ -1874,21 +1856,6 @@ uint8_t sd_read_data(SDState *sd)
             sd->state = sd_transfer_state;
         break;
 
-    case 11:	/* CMD11:  READ_DAT_UNTIL_STOP */
-        if (sd->data_offset == 0)
-            BLK_READ_BLOCK(sd->data_start, io_len);
-        ret = sd->data[sd->data_offset ++];
-
-        if (sd->data_offset >= io_len) {
-            sd->data_start += io_len;
-            sd->data_offset = 0;
-            if (sd->data_start + io_len > sd->size) {
-                sd->card_status |= ADDRESS_ERROR;
-                break;
-            }
-        }
-        break;
-
     case 13:	/* ACMD13: SD_STATUS */
         ret = sd->sd_status[sd->data_offset ++];
 
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 05/18] sdcard: remove commands from unsupported old MMC specification

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> This device does not model MMCA Specification previous to v4.2
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 33 ---------------------------------
>  1 file changed, 33 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 5d50d48097..3c66521862 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1044,24 +1044,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>          }
>          break;
>
> -    case 11:   /* CMD11:  READ_DAT_UNTIL_STOP */
> -        if (sd->spi)
> -            goto bad_cmd;
> -        switch (sd->state) {
> -        case sd_transfer_state:
> -            sd->state = sd_sendingdata_state;
> -            sd->data_start = req.arg;
> -            sd->data_offset = 0;
> -
> -            if (sd->data_start + sd->blk_len > sd->size)
> -                sd->card_status |= ADDRESS_ERROR;
> -            return sd_r0;
> -
> -        default:
> -            break;
> -        }
> -        break;
> -
>      case 12:   /* CMD12:  STOP_TRANSMISSION */
>          switch (sd->state) {
>          case sd_sendingdata_state:
> @@ -1874,21 +1856,6 @@ uint8_t sd_read_data(SDState *sd)
>              sd->state = sd_transfer_state;
>          break;
>
> -    case 11:   /* CMD11:  READ_DAT_UNTIL_STOP */
> -        if (sd->data_offset == 0)
> -            BLK_READ_BLOCK(sd->data_start, io_len);
> -        ret = sd->data[sd->data_offset ++];
> -
> -        if (sd->data_offset >= io_len) {
> -            sd->data_start += io_len;
> -            sd->data_offset = 0;
> -            if (sd->data_start + io_len > sd->size) {
> -                sd->card_status |= ADDRESS_ERROR;
> -                break;
> -            }
> -        }
> -        break;
> -
>      case 13:   /* ACMD13: SD_STATUS */
>          ret = sd->sd_status[sd->data_offset ++];
>
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 06/18] sdcard: simplify using the ldst API

[Philippe Mathieu-Daudé]

the code is easier to review/refactor.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 38 +++++++++-----------------------------
 1 file changed, 9 insertions(+), 29 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3c66521862..23f5d47782 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -435,57 +435,39 @@ static int sd_req_crc_validate(SDRequest *req)
 {
     uint8_t buffer[5];
     buffer[0] = 0x40 | req->cmd;
-    buffer[1] = (req->arg >> 24) & 0xff;
-    buffer[2] = (req->arg >> 16) & 0xff;
-    buffer[3] = (req->arg >> 8) & 0xff;
-    buffer[4] = (req->arg >> 0) & 0xff;
+    stl_be_p(&buffer[1], req->arg);
     return 0;
     return sd_crc7(buffer, 5) != req->crc;	/* TODO */
 }
 
 static void sd_response_r1_make(SDState *sd, uint8_t *response)
 {
-    uint32_t status = sd->card_status;
+    stl_be_p(response, sd->card_status);
+
     /* Clear the "clear on read" status bits */
     sd->card_status &= ~CARD_STATUS_C;
-
-    response[0] = (status >> 24) & 0xff;
-    response[1] = (status >> 16) & 0xff;
-    response[2] = (status >> 8) & 0xff;
-    response[3] = (status >> 0) & 0xff;
 }
 
 static void sd_response_r3_make(SDState *sd, uint8_t *response)
 {
-    response[0] = (sd->ocr >> 24) & 0xff;
-    response[1] = (sd->ocr >> 16) & 0xff;
-    response[2] = (sd->ocr >> 8) & 0xff;
-    response[3] = (sd->ocr >> 0) & 0xff;
+    stl_be_p(response, sd->ocr);
 }
 
 static void sd_response_r6_make(SDState *sd, uint8_t *response)
 {
-    uint16_t arg;
     uint16_t status;
 
-    arg = sd->rca;
     status = ((sd->card_status >> 8) & 0xc000) |
              ((sd->card_status >> 6) & 0x2000) |
               (sd->card_status & 0x1fff);
     sd->card_status &= ~(CARD_STATUS_C & 0xc81fff);
-
-    response[0] = (arg >> 8) & 0xff;
-    response[1] = arg & 0xff;
-    response[2] = (status >> 8) & 0xff;
-    response[3] = status & 0xff;
+    stw_be_p(response + 0, sd->rca);
+    stw_be_p(response + 2, status);
 }
 
 static void sd_response_r7_make(SDState *sd, uint8_t *response)
 {
-    response[0] = (sd->vhs >> 24) & 0xff;
-    response[1] = (sd->vhs >> 16) & 0xff;
-    response[2] = (sd->vhs >>  8) & 0xff;
-    response[3] = (sd->vhs >>  0) & 0xff;
+    stl_be_p(response, sd->vhs);
 }
 
 static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
@@ -731,7 +713,7 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
 
 static void sd_function_switch(SDState *sd, uint32_t arg)
 {
-    int i, mode, new_func, crc;
+    int i, mode, new_func;
     mode = !!(arg & 0x80000000);
 
     sd->data[0] = 0x00;		/* Maximum current consumption */
@@ -755,9 +737,7 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
         sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4);
     }
     memset(&sd->data[17], 0, 47);
-    crc = sd_crc16(sd->data, 64);
-    sd->data[65] = crc >> 8;
-    sd->data[66] = crc & 0xff;
+    stw_be_p(sd->data + 65, sd_crc16(sd->data, 64));
 }
 
 static inline bool sd_wp_addr(SDState *sd, uint64_t addr)
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 06/18] sdcard: simplify using the ldst API

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> the code is easier to review/refactor.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 38 +++++++++-----------------------------
>  1 file changed, 9 insertions(+), 29 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 3c66521862..23f5d47782 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -435,57 +435,39 @@ static int sd_req_crc_validate(SDRequest *req)
>  {
>      uint8_t buffer[5];
>      buffer[0] = 0x40 | req->cmd;
> -    buffer[1] = (req->arg >> 24) & 0xff;
> -    buffer[2] = (req->arg >> 16) & 0xff;
> -    buffer[3] = (req->arg >> 8) & 0xff;
> -    buffer[4] = (req->arg >> 0) & 0xff;
> +    stl_be_p(&buffer[1], req->arg);
>      return 0;
>      return sd_crc7(buffer, 5) != req->crc;     /* TODO */
>  }
>
>  static void sd_response_r1_make(SDState *sd, uint8_t *response)
>  {
> -    uint32_t status = sd->card_status;
> +    stl_be_p(response, sd->card_status);
> +
>      /* Clear the "clear on read" status bits */
>      sd->card_status &= ~CARD_STATUS_C;
> -
> -    response[0] = (status >> 24) & 0xff;
> -    response[1] = (status >> 16) & 0xff;
> -    response[2] = (status >> 8) & 0xff;
> -    response[3] = (status >> 0) & 0xff;
>  }
>
>  static void sd_response_r3_make(SDState *sd, uint8_t *response)
>  {
> -    response[0] = (sd->ocr >> 24) & 0xff;
> -    response[1] = (sd->ocr >> 16) & 0xff;
> -    response[2] = (sd->ocr >> 8) & 0xff;
> -    response[3] = (sd->ocr >> 0) & 0xff;
> +    stl_be_p(response, sd->ocr);
>  }
>
>  static void sd_response_r6_make(SDState *sd, uint8_t *response)
>  {
> -    uint16_t arg;
>      uint16_t status;
>
> -    arg = sd->rca;
>      status = ((sd->card_status >> 8) & 0xc000) |
>               ((sd->card_status >> 6) & 0x2000) |
>                (sd->card_status & 0x1fff);
>      sd->card_status &= ~(CARD_STATUS_C & 0xc81fff);
> -
> -    response[0] = (arg >> 8) & 0xff;
> -    response[1] = arg & 0xff;
> -    response[2] = (status >> 8) & 0xff;
> -    response[3] = status & 0xff;
> +    stw_be_p(response + 0, sd->rca);
> +    stw_be_p(response + 2, status);
>  }
>
>  static void sd_response_r7_make(SDState *sd, uint8_t *response)
>  {
> -    response[0] = (sd->vhs >> 24) & 0xff;
> -    response[1] = (sd->vhs >> 16) & 0xff;
> -    response[2] = (sd->vhs >>  8) & 0xff;
> -    response[3] = (sd->vhs >>  0) & 0xff;
> +    stl_be_p(response, sd->vhs);
>  }
>
>  static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
> @@ -731,7 +713,7 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
>
>  static void sd_function_switch(SDState *sd, uint32_t arg)
>  {
> -    int i, mode, new_func, crc;
> +    int i, mode, new_func;
>      mode = !!(arg & 0x80000000);
>
>      sd->data[0] = 0x00;                /* Maximum current consumption */
> @@ -755,9 +737,7 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
>          sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4);
>      }
>      memset(&sd->data[17], 0, 47);
> -    crc = sd_crc16(sd->data, 64);
> -    sd->data[65] = crc >> 8;
> -    sd->data[66] = crc & 0xff;
> +    stw_be_p(sd->data + 65, sd_crc16(sd->data, 64));
>  }
>
>  static inline bool sd_wp_addr(SDState *sd, uint64_t addr)
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 07/18] sdcard: use the correct masked OCR in the R3 reply

[Philippe Mathieu-Daudé]

use the registerfields API to access the OCR register

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 23f5d47782..ee381540aa 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -47,8 +47,6 @@
 
 //#define DEBUG_SD 1
 
-#define ACMD41_ENQUIRY_MASK     0x00ffffff
-
 typedef enum {
     sd_r0 = 0,    /* no response */
     sd_r1,        /* normal response command */
@@ -271,13 +269,26 @@ static uint16_t sd_crc16(void *message, size_t width)
 
 #define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
 
+FIELD(OCR, VDD_VOLTAGE_WINDOW,          0, 24)
+FIELD(OCR, VDD_VOLTAGE_WIN_LO,          0,  8)
+FIELD(OCR, DUAL_VOLTAGE_CARD,           7,  1)
+FIELD(OCR, VDD_VOLTAGE_WIN_HI,          8, 16)
+FIELD(OCR, ACCEPT_SWITCH_1V8,          24,  1) /* Only UHS-I */
+FIELD(OCR, UHS_II_CARD,                29,  1) /* Only UHS-II */
 FIELD(OCR, CARD_CAPACITY,              30,  1) /* 0:SDSC, 1:SDHC/SDXC */
 FIELD(OCR, CARD_POWER_UP,              31,  1)
 
+#define ACMD41_ENQUIRY_MASK     0x00ffffff
+#define ACMD41_R3_MASK          (R_OCR_VDD_VOLTAGE_WIN_HI_MASK \
+                               | R_OCR_ACCEPT_SWITCH_1V8_MASK \
+                               | R_OCR_UHS_II_CARD_MASK \
+                               | R_OCR_CARD_CAPACITY_MASK \
+                               | R_OCR_CARD_POWER_UP_MASK)
+
 static void sd_reset_ocr(SDState *sd)
 {
-    /* All voltages OK, Standard Capacity SD Memory Card, not yet powered up */
-    sd->ocr = 0x00ffff00;
+    /* All voltages OK */
+    sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
 }
 
 static void sd_ocr_powerup(void *opaque)
@@ -450,7 +461,7 @@ static void sd_response_r1_make(SDState *sd, uint8_t *response)
 
 static void sd_response_r3_make(SDState *sd, uint8_t *response)
 {
-    stl_be_p(response, sd->ocr);
+    stl_be_p(response, sd->ocr & ACMD41_R3_MASK);
 }
 
 static void sd_response_r6_make(SDState *sd, uint8_t *response)
@@ -1634,7 +1645,13 @@ send_response:
     }
 
 #ifdef DEBUG_SD
-    qemu_hexdump((const char *)response, stderr, "Response", rsplen);
+    if (rsplen) {
+        int i;
+        DPRINTF("Response:");
+        for (i = 0; i < rsplen; i++)
+            fprintf(stderr, " %02x", response[i]);
+        fputc('\n', stderr);
+    }
 #endif
 
     return rsplen;
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 07/18] sdcard: use the correct masked OCR in the R3 reply

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> use the registerfields API to access the OCR register
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 23f5d47782..ee381540aa 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -47,8 +47,6 @@
>
>  //#define DEBUG_SD 1
>
> -#define ACMD41_ENQUIRY_MASK     0x00ffffff
> -
>  typedef enum {
>      sd_r0 = 0,    /* no response */
>      sd_r1,        /* normal response command */
> @@ -271,13 +269,26 @@ static uint16_t sd_crc16(void *message, size_t width)
>
>  #define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
>
> +FIELD(OCR, VDD_VOLTAGE_WINDOW,          0, 24)
> +FIELD(OCR, VDD_VOLTAGE_WIN_LO,          0,  8)
> +FIELD(OCR, DUAL_VOLTAGE_CARD,           7,  1)
> +FIELD(OCR, VDD_VOLTAGE_WIN_HI,          8, 16)
> +FIELD(OCR, ACCEPT_SWITCH_1V8,          24,  1) /* Only UHS-I */
> +FIELD(OCR, UHS_II_CARD,                29,  1) /* Only UHS-II */
>  FIELD(OCR, CARD_CAPACITY,              30,  1) /* 0:SDSC, 1:SDHC/SDXC */
>  FIELD(OCR, CARD_POWER_UP,              31,  1)
>
> +#define ACMD41_ENQUIRY_MASK     0x00ffffff
> +#define ACMD41_R3_MASK          (R_OCR_VDD_VOLTAGE_WIN_HI_MASK \
> +                               | R_OCR_ACCEPT_SWITCH_1V8_MASK \
> +                               | R_OCR_UHS_II_CARD_MASK \
> +                               | R_OCR_CARD_CAPACITY_MASK \
> +                               | R_OCR_CARD_POWER_UP_MASK)
> +
>  static void sd_reset_ocr(SDState *sd)
>  {
> -    /* All voltages OK, Standard Capacity SD Memory Card, not yet powered up */
> -    sd->ocr = 0x00ffff00;
> +    /* All voltages OK */
> +    sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
>  }
>
>  static void sd_ocr_powerup(void *opaque)
> @@ -450,7 +461,7 @@ static void sd_response_r1_make(SDState *sd, uint8_t *response)
>
>  static void sd_response_r3_make(SDState *sd, uint8_t *response)
>  {
> -    stl_be_p(response, sd->ocr);
> +    stl_be_p(response, sd->ocr & ACMD41_R3_MASK);
>  }
>
>  static void sd_response_r6_make(SDState *sd, uint8_t *response)
> @@ -1634,7 +1645,13 @@ send_response:
>      }
>
>  #ifdef DEBUG_SD
> -    qemu_hexdump((const char *)response, stderr, "Response", rsplen);
> +    if (rsplen) {
> +        int i;
> +        DPRINTF("Response:");
> +        for (i = 0; i < rsplen; i++)
> +            fprintf(stderr, " %02x", response[i]);
> +        fputc('\n', stderr);
> +    }
>  #endif
>
>      return rsplen;
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 08/18] sdcard: use the registerfields API for the CARD_STATUS register masks

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ee381540aa..2eca999bc3 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -423,14 +423,56 @@ static void sd_set_rca(SDState *sd)
     sd->rca += 0x4567;
 }
 
+FIELD(CSR, AKE_SEQ_ERROR,               3,  1)
+FIELD(CSR, APP_CMD,                     5,  1)
+FIELD(CSR, FX_EVENT,                    6,  1)
+FIELD(CSR, READY_FOR_DATA,              8,  1)
+FIELD(CSR, CURRENT_STATE,               9,  4)
+FIELD(CSR, ERASE_RESET,                13,  1)
+FIELD(CSR, CARD_ECC_DISABLED,          14,  1)
+FIELD(CSR, WP_ERASE_SKIP,              15,  1)
+FIELD(CSR, CSD_OVERWRITE,              16,  1)
+FIELD(CSR, DEFERRED_RESPONSE,          17,  1)
+FIELD(CSR, ERROR,                      19,  1)
+FIELD(CSR, CC_ERROR,                   20,  1)
+FIELD(CSR, CARD_ECC_FAILED,            21,  1)
+FIELD(CSR, ILLEGAL_COMMAND,            22,  1)
+FIELD(CSR, COM_CRC_ERROR,              23,  1)
+FIELD(CSR, LOCK_UNLOCK_FAILED,         24,  1)
+FIELD(CSR, CARD_IS_LOCKED,             25,  1)
+FIELD(CSR, WP_VIOLATION,               26,  1)
+FIELD(CSR, ERASE_PARAM,                27,  1)
+FIELD(CSR, ERASE_SEQ_ERROR,            28,  1)
+FIELD(CSR, BLOCK_LEN_ERROR,            29,  1)
+FIELD(CSR, ADDRESS_ERROR,              30,  1)
+FIELD(CSR, OUT_OF_RANGE,               31,  1)
+
 /* Card status bits, split by clear condition:
  * A : According to the card current state
  * B : Always related to the previous command
  * C : Cleared by read
  */
-#define CARD_STATUS_A	0x02004100
-#define CARD_STATUS_B	0x00c01e00
-#define CARD_STATUS_C	0xfd39a028
+#define CARD_STATUS_A           (R_CSR_READY_FOR_DATA_MASK \
+                               | R_CSR_CARD_ECC_DISABLED_MASK \
+                               | R_CSR_CARD_IS_LOCKED_MASK)
+#define CARD_STATUS_B           (R_CSR_CURRENT_STATE_MASK \
+                               | R_CSR_ILLEGAL_COMMAND_MASK \
+                               | R_CSR_COM_CRC_ERROR_MASK)
+#define CARD_STATUS_C           (R_CSR_AKE_SEQ_ERROR_MASK \
+                               | R_CSR_APP_CMD_MASK \
+                               | R_CSR_ERASE_RESET_MASK \
+                               | R_CSR_WP_ERASE_SKIP_MASK \
+                               | R_CSR_CSD_OVERWRITE_MASK \
+                               | R_CSR_ERROR_MASK \
+                               | R_CSR_CC_ERROR_MASK \
+                               | R_CSR_CARD_ECC_FAILED_MASK \
+                               | R_CSR_LOCK_UNLOCK_FAILED_MASK \
+                               | R_CSR_WP_VIOLATION_MASK \
+                               | R_CSR_ERASE_PARAM_MASK \
+                               | R_CSR_ERASE_SEQ_ERROR_MASK \
+                               | R_CSR_BLOCK_LEN_ERROR_MASK \
+                               | R_CSR_ADDRESS_ERROR_MASK \
+                               | R_CSR_OUT_OF_RANGE_MASK)
 
 static void sd_reset_cardstatus(SDState *sd)
 {
-- 
2.15.1

[Qemu-devel] [PATCH v3 09/18] sdcard: handles more commands in SPI mode

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2eca999bc3..07424aa56e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1390,9 +1390,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     /* Application specific commands (Class 8) */
     case 55:	/* CMD55:  APP_CMD */
-        if (sd->rca != rca)
-            return sd_r0;
-
+        if (!sd->spi) {
+            if (sd->rca != rca) {
+                return sd_r0;
+            }
+        }
         sd->expecting_acmd = true;
         sd->card_status |= APP_CMD;
         return sd_r1;
@@ -1412,6 +1414,18 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
+    case 58:    /* CMD58:   READ_OCR (SPI) */
+        if (!sd->spi) {
+            goto bad_cmd;
+        }
+        return sd_r3;
+
+    case 59:    /* CMD59:   CRC_ON_OFF (SPI) */
+        if (!sd->spi) {
+            goto bad_cmd;
+        }
+        goto unimplemented_cmd;
+
     default:
     bad_cmd:
         qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
@@ -1436,6 +1450,9 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
+        if (sd->spi) {
+            goto unimplemented_cmd;
+        }
         switch (sd->state) {
         case sd_transfer_state:
             sd->sd_status[0] &= 0x3f;
@@ -1460,6 +1477,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
+    case 18:
+        if (sd->spi) {
+            goto unimplemented_cmd;
+        }
+        break;
+
     case 22:	/* ACMD22: SEND_NUM_WR_BLOCKS */
         switch (sd->state) {
         case sd_transfer_state:
@@ -1485,6 +1508,19 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
+    case 25:
+    case 26:
+        if (sd->spi) {
+            goto unimplemented_cmd;
+        }
+        break;
+
+    case 38:
+        if (sd->spi) {
+            goto unimplemented_cmd;
+        }
+        break;
+
     case 41:	/* ACMD41: SD_APP_OP_COND */
         if (sd->spi) {
             /* SEND_OP_CMD */
@@ -1542,6 +1578,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
+    case 43 ... 49:
+        if (sd->spi) {
+            goto unimplemented_cmd;
+        }
+        break;
+
     case 51:	/* ACMD51: SEND_SCR */
         switch (sd->state) {
         case sd_transfer_state:
@@ -1555,9 +1597,18 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
+    case 55:    /* Not exist */
+        break;
+
     default:
         /* Fall back to standard commands.  */
         return sd_normal_command(sd, req);
+
+    unimplemented_cmd:
+        /* Commands that are recognised but not yet implemented in SPI mode.  */
+        qemu_log_mask(LOG_UNIMP, "SD: CMD%i not implemented in SPI mode\n",
+                      req.cmd);
+        return sd_illegal;
     }
 
     qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd);
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 09/18] sdcard: handles more commands in SPI mode

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/sd.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 54 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 2eca999bc3..07424aa56e 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1390,9 +1390,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>
>      /* Application specific commands (Class 8) */
>      case 55:   /* CMD55:  APP_CMD */
> -        if (sd->rca != rca)
> -            return sd_r0;
> -
> +        if (!sd->spi) {
> +            if (sd->rca != rca) {
> +                return sd_r0;
> +            }
> +        }
>          sd->expecting_acmd = true;
>          sd->card_status |= APP_CMD;
>          return sd_r1;
> @@ -1412,6 +1414,18 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>          }
>          break;
>
> +    case 58:    /* CMD58:   READ_OCR (SPI) */
> +        if (!sd->spi) {
> +            goto bad_cmd;
> +        }
> +        return sd_r3;
> +
> +    case 59:    /* CMD59:   CRC_ON_OFF (SPI) */
> +        if (!sd->spi) {
> +            goto bad_cmd;
> +        }
> +        goto unimplemented_cmd;
> +
>      default:
>      bad_cmd:
>          qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
> @@ -1436,6 +1450,9 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>      sd->card_status |= APP_CMD;
>      switch (req.cmd) {
>      case 6:    /* ACMD6:  SET_BUS_WIDTH */
> +        if (sd->spi) {
> +            goto unimplemented_cmd;
> +        }
>          switch (sd->state) {
>          case sd_transfer_state:
>              sd->sd_status[0] &= 0x3f;
> @@ -1460,6 +1477,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>          }
>          break;
>
> +    case 18:

These should all have a comment describing what they are and for the
others as well.

> +        if (sd->spi) {
> +            goto unimplemented_cmd;
> +        }
> +        break;
> +
>      case 22:   /* ACMD22: SEND_NUM_WR_BLOCKS */
>          switch (sd->state) {
>          case sd_transfer_state:
> @@ -1485,6 +1508,19 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>          }
>          break;
>
> +    case 25:
> +    case 26:
> +        if (sd->spi) {
> +            goto unimplemented_cmd;
> +        }
> +        break;
> +
> +    case 38:
> +        if (sd->spi) {
> +            goto unimplemented_cmd;
> +        }
> +        break;
> +
>      case 41:   /* ACMD41: SD_APP_OP_COND */
>          if (sd->spi) {
>              /* SEND_OP_CMD */
> @@ -1542,6 +1578,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>          }
>          break;
>
> +    case 43 ... 49:
> +        if (sd->spi) {
> +            goto unimplemented_cmd;
> +        }
> +        break;
> +
>      case 51:   /* ACMD51: SEND_SCR */
>          switch (sd->state) {
>          case sd_transfer_state:
> @@ -1555,9 +1597,18 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>          }
>          break;
>
> +    case 55:    /* Not exist */
> +        break;
> +
>      default:
>          /* Fall back to standard commands.  */
>          return sd_normal_command(sd, req);
> +
> +    unimplemented_cmd:
> +        /* Commands that are recognised but not yet implemented in SPI mode.  */

This should be unimplemented_spi_cmd then, to be more clear.

Alistair

> +        qemu_log_mask(LOG_UNIMP, "SD: CMD%i not implemented in SPI mode\n",
> +                      req.cmd);
> +        return sd_illegal;
>      }
>
>      qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd);
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH v3 09/18] sdcard: handles more commands in SPI mode

[Philippe Mathieu-Daudé]

On 01/31/2018 09:11 PM, Alistair Francis wrote:
> On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/sd/sd.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 54 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 2eca999bc3..07424aa56e 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -1390,9 +1390,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>>
>>      /* Application specific commands (Class 8) */
>>      case 55:   /* CMD55:  APP_CMD */
>> -        if (sd->rca != rca)
>> -            return sd_r0;
>> -
>> +        if (!sd->spi) {
>> +            if (sd->rca != rca) {
>> +                return sd_r0;
>> +            }
>> +        }
>>          sd->expecting_acmd = true;
>>          sd->card_status |= APP_CMD;
>>          return sd_r1;
>> @@ -1412,6 +1414,18 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>>          }
>>          break;
>>
>> +    case 58:    /* CMD58:   READ_OCR (SPI) */
>> +        if (!sd->spi) {
>> +            goto bad_cmd;
>> +        }
>> +        return sd_r3;
>> +
>> +    case 59:    /* CMD59:   CRC_ON_OFF (SPI) */
>> +        if (!sd->spi) {
>> +            goto bad_cmd;
>> +        }
>> +        goto unimplemented_cmd;
>> +
>>      default:
>>      bad_cmd:
>>          qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
>> @@ -1436,6 +1450,9 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>>      sd->card_status |= APP_CMD;
>>      switch (req.cmd) {
>>      case 6:    /* ACMD6:  SET_BUS_WIDTH */
>> +        if (sd->spi) {
>> +            goto unimplemented_cmd;
>> +        }
>>          switch (sd->state) {
>>          case sd_transfer_state:
>>              sd->sd_status[0] &= 0x3f;
>> @@ -1460,6 +1477,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>>          }
>>          break;
>>
>> +    case 18:
> 
> These should all have a comment describing what they are and for the
> others as well.

This is backported from a patch of you or Peter Crosthwaite =)

I'll check the specs and add documentation.

> 
>> +        if (sd->spi) {
>> +            goto unimplemented_cmd;
>> +        }
>> +        break;
>> +
>>      case 22:   /* ACMD22: SEND_NUM_WR_BLOCKS */
>>          switch (sd->state) {
>>          case sd_transfer_state:
>> @@ -1485,6 +1508,19 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>>          }
>>          break;
>>
>> +    case 25:
>> +    case 26:
>> +        if (sd->spi) {
>> +            goto unimplemented_cmd;
>> +        }
>> +        break;
>> +
>> +    case 38:
>> +        if (sd->spi) {
>> +            goto unimplemented_cmd;
>> +        }
>> +        break;
>> +
>>      case 41:   /* ACMD41: SD_APP_OP_COND */
>>          if (sd->spi) {
>>              /* SEND_OP_CMD */
>> @@ -1542,6 +1578,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>>          }
>>          break;
>>
>> +    case 43 ... 49:
>> +        if (sd->spi) {
>> +            goto unimplemented_cmd;
>> +        }
>> +        break;
>> +
>>      case 51:   /* ACMD51: SEND_SCR */
>>          switch (sd->state) {
>>          case sd_transfer_state:
>> @@ -1555,9 +1597,18 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>>          }
>>          break;
>>
>> +    case 55:    /* Not exist */
>> +        break;
>> +
>>      default:
>>          /* Fall back to standard commands.  */
>>          return sd_normal_command(sd, req);
>> +
>> +    unimplemented_cmd:
>> +        /* Commands that are recognised but not yet implemented in SPI mode.  */
> 
> This should be unimplemented_spi_cmd then, to be more clear.

Good idea.

> 
> Alistair
> 
>> +        qemu_log_mask(LOG_UNIMP, "SD: CMD%i not implemented in SPI mode\n",
>> +                      req.cmd);
>> +        return sd_illegal;
>>      }
>>
>>      qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd);
>> --
>> 2.15.1
>>
>>

[Qemu-devel] [PATCH v3 10/18] sdcard: handle CMD54 (SDIO)

[Philippe Mathieu-Daudé]

Linux uses it to poll the bus before polling for a card.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 07424aa56e..bbf9496e8a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1378,9 +1378,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 52:
-    case 53:
-        /* CMD52, CMD53: reserved for SDIO cards
+    case 52 ... 54:
+        /* CMD52, CMD53, CMD54: reserved for SDIO cards
          * (see the SDIO Simplified Specification V2.0)
          * Handle as illegal command but do not complain
          * on stderr, as some OSes may use these in their
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 10/18] sdcard: handle CMD54 (SDIO)

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Linux uses it to poll the bus before polling for a card.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 07424aa56e..bbf9496e8a 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1378,9 +1378,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>          }
>          break;
>
> -    case 52:
> -    case 53:
> -        /* CMD52, CMD53: reserved for SDIO cards
> +    case 52 ... 54:
> +        /* CMD52, CMD53, CMD54: reserved for SDIO cards
>           * (see the SDIO Simplified Specification V2.0)
>           * Handle as illegal command but do not complain
>           * on stderr, as some OSes may use these in their
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 11/18] sdcard: check the card is in correct state for APP CMD (CMD55)

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bbf9496e8a..434d1fbc47 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1389,6 +1389,14 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     /* Application specific commands (Class 8) */
     case 55:	/* CMD55:  APP_CMD */
+        switch (sd->state) {
+        case sd_ready_state:
+        case sd_identification_state:
+        case sd_inactive_state:
+            return sd_illegal;
+        default:
+            break;
+        }
         if (!sd->spi) {
             if (sd->rca != rca) {
                 return sd_r0;
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 11/18] sdcard: check the card is in correct state for APP CMD (CMD55)

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index bbf9496e8a..434d1fbc47 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1389,6 +1389,14 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>
>      /* Application specific commands (Class 8) */
>      case 55:   /* CMD55:  APP_CMD */
> +        switch (sd->state) {
> +        case sd_ready_state:
> +        case sd_identification_state:
> +        case sd_inactive_state:
> +            return sd_illegal;
> +        default:
> +            break;
> +        }
>          if (!sd->spi) {
>              if (sd->rca != rca) {
>                  return sd_r0;
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 12/18] sdcard: warn if host uses an incorrect address for APP CMD (CMD55)

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 434d1fbc47..b5c947df62 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1394,6 +1394,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         case sd_identification_state:
         case sd_inactive_state:
             return sd_illegal;
+        case sd_idle_state:
+            if (rca) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "SD: illegal RCA 0x%04x for APP_CMD\n", req.cmd);
+            }
         default:
             break;
         }
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 12/18] sdcard: warn if host uses an incorrect address for APP CMD (CMD55)

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 434d1fbc47..b5c947df62 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1394,6 +1394,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>          case sd_identification_state:
>          case sd_inactive_state:
>              return sd_illegal;
> +        case sd_idle_state:
> +            if (rca) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "SD: illegal RCA 0x%04x for APP_CMD\n", req.cmd);
> +            }
>          default:
>              break;
>          }
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 13/18] sdcard: simplify SEND_IF_COND (CMD8)

[Philippe Mathieu-Daudé]

replace switch(single case) -> if()

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b5c947df62..707c294169 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1015,23 +1015,19 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     case 8:	/* CMD8:   SEND_IF_COND */
         /* Physical Layer Specification Version 2.00 command */
-        switch (sd->state) {
-        case sd_idle_state:
-            sd->vhs = 0;
-
-            /* No response if not exactly one VHS bit is set.  */
-            if (!(req.arg >> 8) || (req.arg >> (ctz32(req.arg & ~0xff) + 1))) {
-                return sd->spi ? sd_r7 : sd_r0;
-            }
-
-            /* Accept.  */
-            sd->vhs = req.arg;
-            return sd_r7;
-
-        default:
+        if (sd->state != sd_idle_state) {
             break;
         }
-        break;
+        sd->vhs = 0;
+
+        /* No response if not exactly one VHS bit is set.  */
+        if (!(req.arg >> 8) || (req.arg >> (ctz32(req.arg & ~0xff) + 1))) {
+            return sd->spi ? sd_r7 : sd_r0;
+        }
+
+        /* Accept.  */
+        sd->vhs = req.arg;
+        return sd_r7;
 
     case 9:	/* CMD9:   SEND_CSD */
         switch (sd->state) {
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 13/18] sdcard: simplify SEND_IF_COND (CMD8)

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> replace switch(single case) -> if()
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index b5c947df62..707c294169 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1015,23 +1015,19 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>
>      case 8:    /* CMD8:   SEND_IF_COND */
>          /* Physical Layer Specification Version 2.00 command */
> -        switch (sd->state) {
> -        case sd_idle_state:
> -            sd->vhs = 0;
> -
> -            /* No response if not exactly one VHS bit is set.  */
> -            if (!(req.arg >> 8) || (req.arg >> (ctz32(req.arg & ~0xff) + 1))) {
> -                return sd->spi ? sd_r7 : sd_r0;
> -            }
> -
> -            /* Accept.  */
> -            sd->vhs = req.arg;
> -            return sd_r7;
> -
> -        default:
> +        if (sd->state != sd_idle_state) {
>              break;
>          }
> -        break;
> +        sd->vhs = 0;
> +
> +        /* No response if not exactly one VHS bit is set.  */
> +        if (!(req.arg >> 8) || (req.arg >> (ctz32(req.arg & ~0xff) + 1))) {
> +            return sd->spi ? sd_r7 : sd_r0;
> +        }
> +
> +        /* Accept.  */
> +        sd->vhs = req.arg;
> +        return sd_r7;
>
>      case 9:    /* CMD9:   SEND_CSD */
>          switch (sd->state) {
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 14/18] sdcard: simplify SD_SEND_OP_COND (ACMD41)

[Philippe Mathieu-Daudé]

replace switch(single case) -> if()

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 56 ++++++++++++++++++++++++++------------------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 707c294169..6efcacb942 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1535,45 +1535,41 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
             sd->state = sd_transfer_state;
             return sd_r1;
         }
-        switch (sd->state) {
-        case sd_idle_state:
-            /* If it's the first ACMD41 since reset, we need to decide
-             * whether to power up. If this is not an enquiry ACMD41,
-             * we immediately report power on and proceed below to the
-             * ready state, but if it is, we set a timer to model a
-             * delay for power up. This works around a bug in EDK2
-             * UEFI, which sends an initial enquiry ACMD41, but
-             * assumes that the card is in ready state as soon as it
-             * sees the power up bit set. */
-            if (!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP)) {
-                if ((req.arg & ACMD41_ENQUIRY_MASK) != 0) {
-                    timer_del(sd->ocr_power_timer);
-                    sd_ocr_powerup(sd);
-                } else {
-                    trace_sdcard_inquiry_cmd41();
-                    if (!timer_pending(sd->ocr_power_timer)) {
-                        timer_mod_ns(sd->ocr_power_timer,
-                                     (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
-                                      + OCR_POWER_DELAY_NS));
-                    }
+        if (sd->state != sd_idle_state) {
+            break;
+        }
+        /* If it's the first ACMD41 since reset, we need to decide
+         * whether to power up. If this is not an enquiry ACMD41,
+         * we immediately report power on and proceed below to the
+         * ready state, but if it is, we set a timer to model a
+         * delay for power up. This works around a bug in EDK2
+         * UEFI, which sends an initial enquiry ACMD41, but
+         * assumes that the card is in ready state as soon as it
+         * sees the power up bit set. */
+        if (!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP)) {
+            if ((req.arg & ACMD41_ENQUIRY_MASK) != 0) {
+                timer_del(sd->ocr_power_timer);
+                sd_ocr_powerup(sd);
+            } else {
+                trace_sdcard_inquiry_cmd41();
+                if (!timer_pending(sd->ocr_power_timer)) {
+                    timer_mod_ns(sd->ocr_power_timer,
+                                 (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
+                                  + OCR_POWER_DELAY_NS));
                 }
             }
+        }
 
+        if (FIELD_EX32(sd->ocr & req.arg, OCR, VDD_VOLTAGE_WINDOW)) {
             /* We accept any voltage.  10000 V is nothing.
              *
              * Once we're powered up, we advance straight to ready state
              * unless it's an enquiry ACMD41 (bits 23:0 == 0).
              */
-            if (req.arg & ACMD41_ENQUIRY_MASK) {
-                sd->state = sd_ready_state;
-            }
-
-            return sd_r3;
-
-        default:
-            break;
+            sd->state = sd_ready_state;
         }
-        break;
+
+        return sd_r3;
 
     case 42:	/* ACMD42: SET_CLR_CARD_DETECT */
         switch (sd->state) {
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 14/18] sdcard: simplify SD_SEND_OP_COND (ACMD41)

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> replace switch(single case) -> if()
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 56 ++++++++++++++++++++++++++------------------------------
>  1 file changed, 26 insertions(+), 30 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 707c294169..6efcacb942 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1535,45 +1535,41 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>              sd->state = sd_transfer_state;
>              return sd_r1;
>          }
> -        switch (sd->state) {
> -        case sd_idle_state:
> -            /* If it's the first ACMD41 since reset, we need to decide
> -             * whether to power up. If this is not an enquiry ACMD41,
> -             * we immediately report power on and proceed below to the
> -             * ready state, but if it is, we set a timer to model a
> -             * delay for power up. This works around a bug in EDK2
> -             * UEFI, which sends an initial enquiry ACMD41, but
> -             * assumes that the card is in ready state as soon as it
> -             * sees the power up bit set. */
> -            if (!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP)) {
> -                if ((req.arg & ACMD41_ENQUIRY_MASK) != 0) {
> -                    timer_del(sd->ocr_power_timer);
> -                    sd_ocr_powerup(sd);
> -                } else {
> -                    trace_sdcard_inquiry_cmd41();
> -                    if (!timer_pending(sd->ocr_power_timer)) {
> -                        timer_mod_ns(sd->ocr_power_timer,
> -                                     (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
> -                                      + OCR_POWER_DELAY_NS));
> -                    }
> +        if (sd->state != sd_idle_state) {
> +            break;
> +        }
> +        /* If it's the first ACMD41 since reset, we need to decide
> +         * whether to power up. If this is not an enquiry ACMD41,
> +         * we immediately report power on and proceed below to the
> +         * ready state, but if it is, we set a timer to model a
> +         * delay for power up. This works around a bug in EDK2
> +         * UEFI, which sends an initial enquiry ACMD41, but
> +         * assumes that the card is in ready state as soon as it
> +         * sees the power up bit set. */
> +        if (!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP)) {
> +            if ((req.arg & ACMD41_ENQUIRY_MASK) != 0) {
> +                timer_del(sd->ocr_power_timer);
> +                sd_ocr_powerup(sd);
> +            } else {
> +                trace_sdcard_inquiry_cmd41();
> +                if (!timer_pending(sd->ocr_power_timer)) {
> +                    timer_mod_ns(sd->ocr_power_timer,
> +                                 (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
> +                                  + OCR_POWER_DELAY_NS));
>                  }
>              }
> +        }
>
> +        if (FIELD_EX32(sd->ocr & req.arg, OCR, VDD_VOLTAGE_WINDOW)) {
>              /* We accept any voltage.  10000 V is nothing.
>               *
>               * Once we're powered up, we advance straight to ready state
>               * unless it's an enquiry ACMD41 (bits 23:0 == 0).
>               */
> -            if (req.arg & ACMD41_ENQUIRY_MASK) {
> -                sd->state = sd_ready_state;
> -            }
> -
> -            return sd_r3;
> -
> -        default:
> -            break;
> +            sd->state = sd_ready_state;
>          }
> -        break;
> +
> +        return sd_r3;
>
>      case 42:   /* ACMD42: SET_CLR_CARD_DETECT */
>          switch (sd->state) {
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v3 15/18] sdcard: add SD SEND_TUNING_BLOCK (CMD19)

[Philippe Mathieu-Daudé]

[based on a patch from Alistair Francis <alistair.francis@xilinx.com>
 from qemu/xilinx tag xilinx-v2015.2]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6efcacb942..27176ba33e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1169,6 +1169,14 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
+    case 19:    /* CMD19: SEND_TUNING_BLOCK (SD) */
+        if (sd->state == sd_transfer_state) {
+            sd->state = sd_sendingdata_state;
+            sd->data_offset = 0;
+            return sd_r1;
+        }
+        break;
+
     case 23:    /* CMD23: SET_BLOCK_COUNT */
         switch (sd->state) {
         case sd_transfer_state:
@@ -1911,6 +1919,15 @@ void sd_write_data(SDState *sd, uint8_t value)
     }
 }
 
+#define SD_TUNING_BLOCK_SIZE    64
+
+static const uint32_t sd_tunning_data[SD_TUNING_BLOCK_SIZE / 4] = {
+    0xFF0FFF00, 0x0FFCC3CC, 0xC33CCCFF, 0xFEFFFEEF,
+    0xFFDFFFDD, 0xFFFBFFFB, 0XBFFF7FFF, 0X77F7BDEF,
+    0XFFF0FFF0, 0X0FFCCC3C, 0XCC33CCCF, 0XFFEFFFEE,
+    0XFFFDFFFD, 0XDFFFBFFF, 0XBBFFF7FF, 0XF77F7BDE,
+};
+
 uint8_t sd_read_data(SDState *sd)
 {
     /* TODO: Append CRCs */
@@ -1990,6 +2007,13 @@ uint8_t sd_read_data(SDState *sd)
         }
         break;
 
+    case 19:    /* CMD19:  SEND_TUNING_BLOCK (SD) */
+        if (sd->data_offset >= SD_TUNING_BLOCK_SIZE - 1) {
+            sd->state = sd_transfer_state;
+        }
+        ret = ((uint8_t *)(&sd_tunning_data))[sd->data_offset++];
+        break;
+
     case 22:	/* ACMD22: SEND_NUM_WR_BLOCKS */
         ret = sd->data[sd->data_offset ++];
 
-- 
2.15.1

[Qemu-devel] [PATCH v3 16/18] sdcard: add a 'uhs' property, update the OCR register ACCEPT_SWITCH_1V8 bit

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 27176ba33e..b3b6859bc4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -93,6 +93,7 @@ struct SDState {
     /* Configurable properties */
     BlockBackend *blk;
     bool spi;
+    uint8_t uhs_supported;
 
     uint32_t mode;    /* current card mode, one of SDCardModes */
     int32_t state;    /* current card state, one of SDCardStates */
@@ -289,6 +290,8 @@ static void sd_reset_ocr(SDState *sd)
 {
     /* All voltages OK */
     sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
+
+    sd->ocr = FIELD_DP32(sd->ocr, OCR, ACCEPT_SWITCH_1V8, !!sd->uhs_supported);
 }
 
 static void sd_ocr_powerup(void *opaque)
@@ -2107,6 +2110,7 @@ static Property sd_properties[] = {
      * board to ensure that ssi transfers only occur when the chip select
      * is asserted.  */
     DEFINE_PROP_BOOL("spi", SDState, spi, false),
+    DEFINE_PROP_UINT8("uhs", SDState, uhs_supported, UHS_NOT_SUPPORTED),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 16/18] sdcard: add a 'uhs' property, update the OCR register ACCEPT_SWITCH_1V8 bit

[Philippe Mathieu-Daudé]

On 01/23/2018 12:30 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/sd.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 27176ba33e..b3b6859bc4 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -93,6 +93,7 @@ struct SDState {
>      /* Configurable properties */
>      BlockBackend *blk;
>      bool spi;
> +    uint8_t uhs_supported;
>  
>      uint32_t mode;    /* current card mode, one of SDCardModes */
>      int32_t state;    /* current card state, one of SDCardStates */
> @@ -289,6 +290,8 @@ static void sd_reset_ocr(SDState *sd)
>  {
>      /* All voltages OK */
>      sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
> +
> +    sd->ocr = FIELD_DP32(sd->ocr, OCR, ACCEPT_SWITCH_1V8, !!sd->uhs_supported);

This patch has to go after this series #17 "sdcard: implement the UHS-I
SWITCH_FUNCTION entries (Spec v3)"

>  }
>  
>  static void sd_ocr_powerup(void *opaque)
> @@ -2107,6 +2110,7 @@ static Property sd_properties[] = {
>       * board to ensure that ssi transfers only occur when the chip select
>       * is asserted.  */
>      DEFINE_PROP_BOOL("spi", SDState, spi, false),
> +    DEFINE_PROP_UINT8("uhs", SDState, uhs_supported, UHS_NOT_SUPPORTED),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> 

Re: [Qemu-devel] [PATCH v3 16/18] sdcard: add a 'uhs' property, update the OCR register ACCEPT_SWITCH_1V8 bit

[Philippe Mathieu-Daudé]

On 01/25/2018 07:30 AM, Philippe Mathieu-Daudé wrote:
> On 01/23/2018 12:30 AM, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/sd/sd.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 27176ba33e..b3b6859bc4 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -93,6 +93,7 @@ struct SDState {
>>      /* Configurable properties */
>>      BlockBackend *blk;
>>      bool spi;
>> +    uint8_t uhs_supported;
>>  
>>      uint32_t mode;    /* current card mode, one of SDCardModes */
>>      int32_t state;    /* current card state, one of SDCardStates */
>> @@ -289,6 +290,8 @@ static void sd_reset_ocr(SDState *sd)
>>  {
>>      /* All voltages OK */
>>      sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
>> +
>> +    sd->ocr = FIELD_DP32(sd->ocr, OCR, ACCEPT_SWITCH_1V8, !!sd->uhs_supported);
> 
> This patch has to go after this series #17 "sdcard: implement the UHS-I
> SWITCH_FUNCTION entries (Spec v3)"

Also although the SD interface is working/tested, the MMC interface is
missing from this series, and UHS enabled MMC cards are not usable:

kernel: mmc0: SDHCI controller on PCI [0000:00:05.0] using ADMA
kernel: sd 0:0:0:0: Attached scsi generic sg0 type 0
kernel: mmc0: Skipping voltage switch
[mmc kthread looping]

> 
>>  }
>>  
>>  static void sd_ocr_powerup(void *opaque)
>> @@ -2107,6 +2110,7 @@ static Property sd_properties[] = {
>>       * board to ensure that ssi transfers only occur when the chip select
>>       * is asserted.  */
>>      DEFINE_PROP_BOOL("spi", SDState, spi, false),
>> +    DEFINE_PROP_UINT8("uhs", SDState, uhs_supported, UHS_NOT_SUPPORTED),
>>      DEFINE_PROP_END_OF_LIST()
>>  };
>>  
>>

[Qemu-devel] [PATCH v3 17/18] sdcard: implement the UHS-I SWITCH_FUNCTION entries (Spec v3)

[Philippe Mathieu-Daudé]

[based on a patch from Alistair Francis <alistair.francis@xilinx.com>
 from qemu/xilinx tag xilinx-v2015.2]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c         | 148 +++++++++++++++++++++++++++++++++++++++++++++--------
 hw/sd/trace-events |   1 +
 2 files changed, 127 insertions(+), 22 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b3b6859bc4..1f6c4ce2a4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -125,6 +125,7 @@ struct SDState {
     bool enable;
     uint8_t dat_lines;
     bool cmd_line;
+    bool uhs_enabled;
 };
 
 static const char *sd_state_name(enum SDCardStates state)
@@ -569,6 +570,7 @@ static void sd_reset(DeviceState *dev)
     sd->expecting_acmd = false;
     sd->dat_lines = 0xf;
     sd->cmd_line = true;
+    sd->uhs_enabled = false;
     sd->multi_blk_cnt = 0;
 }
 
@@ -767,30 +769,132 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
     return ret;
 }
 
+/* Function Group */
+enum {
+    SD_FG_MIN               = 1,
+    SD_FG_ACCESS_MODE       = 1,
+    SD_FG_COMMAND_SYSTEM    = 2,
+    SD_FG_DRIVER_STRENGTH   = 3,
+    SD_FG_CURRENT_LIMIT     = 4,
+    SD_FG_RSVD_5            = 5,
+    SD_FG_RSVD_6            = 6,
+    SD_FG_COUNT
+};
+
+/* Function name */
+#define SD_FN_COUNT 16
+
+static const char *sd_fn_grp_name[SD_FG_COUNT] = {
+    [SD_FG_ACCESS_MODE]     = "ACCESS_MODE",
+    [SD_FG_COMMAND_SYSTEM]  = "COMMAND_SYSTEM",
+    [SD_FG_DRIVER_STRENGTH] = "DRIVER_STRENGTH",
+    [SD_FG_CURRENT_LIMIT]   = "CURRENT_LIMIT",
+    [SD_FG_RSVD_5]          = "RSVD5",
+    [SD_FG_RSVD_6]          = "RSVD6",
+};
+
+typedef struct sd_fn_support {
+    const char *name;
+    bool uhs_only;
+    bool unimp;
+} sd_fn_support;
+
+static const sd_fn_support *sd_fn_support_defs[SD_FG_COUNT] = {
+    [SD_FG_ACCESS_MODE] = (sd_fn_support [SD_FN_COUNT]) {
+        [0] = { .name = "default/SDR12" },
+        [1] = { .name = "high-speed/SDR25" },
+        [2] = { .name = "SDR50",    .uhs_only = true },
+        [3] = { .name = "SDR104",   .uhs_only = true },
+        [4] = { .name = "DDR50",    .uhs_only = true },
+    },
+    [SD_FG_COMMAND_SYSTEM] = (sd_fn_support [SD_FN_COUNT]) {
+        [0] = { .name = "default" },
+        [1] = { .name = "For eC" },
+        [3] = { .name = "OTP",      .unimp = true },
+        [4] = { .name = "ASSD",     .unimp = true },
+    },
+    [SD_FG_DRIVER_STRENGTH] = (sd_fn_support [SD_FN_COUNT]) {
+        [0] = { .name = "default/Type B" },
+        [1] = { .name = "Type A",   .uhs_only = true },
+        [2] = { .name = "Type C",   .uhs_only = true },
+        [3] = { .name = "Type D",   .uhs_only = true },
+    },
+    [SD_FG_CURRENT_LIMIT] = (sd_fn_support [SD_FN_COUNT]) {
+        [0] = { .name = "default/200mA" },
+        [1] = { .name = "400mA",    .uhs_only = true },
+        [2] = { .name = "600mA",    .uhs_only = true },
+        [3] = { .name = "800mA",    .uhs_only = true },
+    },
+    [SD_FG_RSVD_5] = (sd_fn_support [SD_FN_COUNT]) {
+        [0] = { .name = "default" },
+    },
+    [SD_FG_RSVD_6] = (sd_fn_support [SD_FN_COUNT]) {
+        [0] = { .name = "default" },
+    },
+};
+
+#define SD_FN_NO_INFLUENCE          (1 << 15)
+
 static void sd_function_switch(SDState *sd, uint32_t arg)
 {
-    int i, mode, new_func;
-    mode = !!(arg & 0x80000000);
-
-    sd->data[0] = 0x00;		/* Maximum current consumption */
-    sd->data[1] = 0x01;
-    sd->data[2] = 0x80;		/* Supported group 6 functions */
-    sd->data[3] = 0x01;
-    sd->data[4] = 0x80;		/* Supported group 5 functions */
-    sd->data[5] = 0x01;
-    sd->data[6] = 0x80;		/* Supported group 4 functions */
-    sd->data[7] = 0x01;
-    sd->data[8] = 0x80;		/* Supported group 3 functions */
-    sd->data[9] = 0x01;
-    sd->data[10] = 0x80;	/* Supported group 2 functions */
-    sd->data[11] = 0x43;
-    sd->data[12] = 0x80;	/* Supported group 1 functions */
-    sd->data[13] = 0x03;
-    for (i = 0; i < 6; i ++) {
-        new_func = (arg >> (i * 4)) & 0x0f;
-        if (mode && new_func != 0x0f)
-            sd->function_group[i] = new_func;
-        sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4);
+    int fn_grp, new_func, i;
+    uint8_t *data_p;
+    bool mode = extract32(arg, 31, 1);  /* 0: check only, 1: do switch */
+
+    stw_be_p(sd->data + 0, 0x0001);     /* Maximum current consumption */
+
+    data_p = &sd->data[2];
+    for (fn_grp = SD_FG_COUNT - 1; fn_grp >= SD_FG_MIN; fn_grp--) {
+        uint16_t supported_fns = SD_FN_NO_INFLUENCE;
+        for (i = 0; i < SD_FN_COUNT; ++i) {
+            const sd_fn_support *def = &sd_fn_support_defs[fn_grp][i];
+
+            if (def->name && !def->unimp &&
+                    !(def->uhs_only && !sd->uhs_enabled)) {
+                supported_fns |= 1 << i;
+            }
+        }
+        stw_be_p(data_p, supported_fns);
+        data_p += 2;
+    }
+
+    assert(data_p == &sd->data[14]);
+    for (fn_grp = SD_FG_COUNT - 1; fn_grp >= SD_FG_MIN; fn_grp--) {
+        new_func = (arg >> ((fn_grp - 1) * 4)) & 0x0f;
+        if (new_func == 0xf) {
+            new_func = sd->function_group[fn_grp - 1];
+        } else {
+            const sd_fn_support *def = &sd_fn_support_defs[fn_grp][new_func];
+            if (mode) {
+                if (!def->name) {
+                    qemu_log_mask(LOG_GUEST_ERROR,
+                                  "Function %d not a valid for "
+                                  "function group %d\n",
+                                  new_func, fn_grp);
+                    new_func = 0xf;
+                } else if (def->unimp) {
+                    qemu_log_mask(LOG_UNIMP,
+                                  "Function %s (fn grp %d) not implemented\n",
+                                  def->name, fn_grp);
+                    new_func = 0xf;
+                } else if (def->uhs_only && !sd->uhs_enabled) {
+                    qemu_log_mask(LOG_GUEST_ERROR,
+                                  "Function %s (fn grp %d) only "
+                                  "valid in UHS mode\n",
+                                  def->name, fn_grp);
+                    new_func = 0xf;
+                } else {
+                    sd->function_group[fn_grp - 1] = new_func;
+                }
+            }
+            trace_sdcard_function_select(def->name, sd_fn_grp_name[fn_grp],
+                                         mode);
+        }
+        if (!(fn_grp & 0x1)) { /* evens go in high nibble */
+            *data_p = new_func << 4;
+        } else { /* odds go in low nibble */
+            *(data_p++) |= new_func;
+        }
     }
     memset(&sd->data[17], 0, 47);
     stw_be_p(sd->data + 65, sd_crc16(sd->data, 64));
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 2059ace61f..c106541a47 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -42,6 +42,7 @@ sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd, uint8_t value) "%s %20s/ CMD%02d value 0x%02x"
 sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd, int length) "%s %20s/ CMD%02d len %d"
 sdcard_set_voltage(uint16_t millivolts) "%u mV"
+sdcard_function_select(const char *fn_name, const char *grp_name, bool do_switch) "Function %s (group: %s, sw: %u)"
 
 # hw/sd/milkymist-memcard.c
 milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
-- 
2.15.1

[Qemu-devel] [PATCH v3 18/18] sdcard: add an enum for the SD PHY Spec version

[Philippe Mathieu-Daudé]

So far this device intends to model the Spec v1.10

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1f6c4ce2a4..9880a5d090 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -47,6 +47,11 @@
 
 //#define DEBUG_SD 1
 
+typedef enum {
+    SD_PHY_SPEC_VER_1_10 = 110,
+    SD_PHY_SPEC_VER_2_00 = 200, /* not yet supported */
+} sd_phy_spec_ver_t;
+
 typedef enum {
     sd_r0 = 0,    /* no response */
     sd_r1,        /* normal response command */
@@ -122,6 +127,7 @@ struct SDState {
     qemu_irq inserted_cb;
     QEMUTimer *ocr_power_timer;
     const char *proto_name;
+    int spec_version;
     bool enable;
     uint8_t dat_lines;
     bool cmd_line;
@@ -2191,6 +2197,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
     int ret;
 
     sd->proto_name = sd->spi ? "SPI" : "SD";
+    sd->spec_version = SD_PHY_SPEC_VER_1_10;
 
     if (sd->blk && blk_is_read_only(sd->blk)) {
         error_setg(errp, "Cannot use read-only drive as SD card");
-- 
2.15.1

Re: [Qemu-devel] [PATCH v3 18/18] sdcard: add an enum for the SD PHY Spec version

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:30 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> So far this device intends to model the Spec v1.10
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 1f6c4ce2a4..9880a5d090 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -47,6 +47,11 @@
>
>  //#define DEBUG_SD 1
>
> +typedef enum {
> +    SD_PHY_SPEC_VER_1_10 = 110,
> +    SD_PHY_SPEC_VER_2_00 = 200, /* not yet supported */
> +} sd_phy_spec_ver_t;
> +
>  typedef enum {
>      sd_r0 = 0,    /* no response */
>      sd_r1,        /* normal response command */
> @@ -122,6 +127,7 @@ struct SDState {
>      qemu_irq inserted_cb;
>      QEMUTimer *ocr_power_timer;
>      const char *proto_name;
> +    int spec_version;
>      bool enable;
>      uint8_t dat_lines;
>      bool cmd_line;
> @@ -2191,6 +2197,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
>      int ret;
>
>      sd->proto_name = sd->spi ? "SPI" : "SD";
> +    sd->spec_version = SD_PHY_SPEC_VER_1_10;
>
>      if (sd->blk && blk_is_read_only(sd->blk)) {
>          error_setg(errp, "Cannot use read-only drive as SD card");
> --
> 2.15.1
>
>