[Qemu-devel] [PATCH 0/8] sdcard: cleanup the SD_SPEC version

[Qemu-devel] [PATCH 0/8] sdcard: cleanup the SD_SPEC version

[Philippe Mathieu-Daudé]

Hi,

This series adds a 'spec_version' property to the SD Card device,
to allow to limit some commands to specific spec version range
(some firmwares use this feature to detect which spec version the
card implements).

This restore the SSI/SD support of the Stellaris LM3S6965EVB board,
which allow to stress the SD Card code with a nice integration test
(waiting for another series to get merged to add the Avocado test):

    $ qemu-system-arm -M lm3s6965evb -serial stdio \
      -kernel sd_card.bin -sd sdcard.img

    SD Card Example Program
    Type 'help' for help.

    /> ls
    Open
    listing

    ----A 2012/04/25 17:44        12  README.TXT

       1 File(s),        12 bytes total
       0 Dir(s),      61182K bytes free

    /> cat README.TXT
    Hello World

See:
http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg03790.html
http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg00353.html

Regards,

Phil.

Philippe Mathieu-Daudé (8):
  sdcard: Update the Configuration Register (SCR) to Spec Version 1.10
  sdcard: Allow commands valid in SPI mode
  sdcard: Add a 'spec_version' property
  sdcard: Set Spec v2.00 as default
  hw/sd/ssi-sd: Force cards connected in SPI mode to use Spec v1.10
  sdcard: Disable SEND_IF_COND (CMD8) for Spec v1
  sdcard: Reflect when the Spec v3 is supported in the Config Register (SCR)
  sdcard: Disable CMD19/CMD23 for Spec v2

 include/hw/sd/sd.h |  6 ++++++
 hw/sd/sd.c         | 47 +++++++++++++++++++++++++++++-----------------
 hw/sd/ssi-sd.c     |  2 ++
 3 files changed, 38 insertions(+), 17 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/8] sdcard: Update the Configuration Register (SCR) to Spec Version 1.10

[Philippe Mathieu-Daudé]

This register now matches the description of the header:

 * SD Memory Card emulation as defined in the "SD Memory Card Physical
 * layer specification, Version 1.10."

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7af19fa06c..e1218d1fb6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -310,8 +310,8 @@ static void sd_ocr_powerup(void *opaque)
 
 static void sd_set_scr(SDState *sd)
 {
-    sd->scr[0] = (0 << 4)       /* SCR version 1.0 */
-                 | 0;           /* Spec Versions 1.0 and 1.01 */
+    sd->scr[0] = (0 << 4)       /* SCR structure version 1.0 */
+                 | 1;           /* Spec Version 1.10 */
     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. */
-- 
2.17.1

[Qemu-devel] [PATCH 2/8] sdcard: Allow commands valid in SPI mode

[Philippe Mathieu-Daudé]

>From the "Physical Layer Simplified Specification Version 1.10"
  Chapter 7.3 "SPI Mode Transaction Packets"
    Table 57: "Commands and arguments"

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index e1218d1fb6..80e70dd93e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -960,8 +960,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         return sd_illegal;
 
     case 6:	/* CMD6:   SWITCH_FUNCTION */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->mode) {
         case sd_data_transfer_mode:
             sd_function_switch(sd, req.arg);
@@ -1190,9 +1188,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     /* Block write commands (Class 4) */
     case 24:	/* CMD24:  WRITE_SINGLE_BLOCK */
-        if (sd->spi) {
-            goto unimplemented_spi_cmd;
-        }
         switch (sd->state) {
         case sd_transfer_state:
             /* Writing in SPI mode not implemented.  */
@@ -1217,9 +1212,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 25:	/* CMD25:  WRITE_MULTIPLE_BLOCK */
-        if (sd->spi) {
-            goto unimplemented_spi_cmd;
-        }
         switch (sd->state) {
         case sd_transfer_state:
             /* Writing in SPI mode not implemented.  */
@@ -1259,9 +1251,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 27:	/* CMD27:  PROGRAM_CSD */
-        if (sd->spi) {
-            goto unimplemented_spi_cmd;
-        }
         switch (sd->state) {
         case sd_transfer_state:
             sd->state = sd_receivingdata_state;
@@ -1371,9 +1360,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     /* Lock card commands (Class 7) */
     case 42:	/* CMD42:  LOCK_UNLOCK */
-        if (sd->spi) {
-            goto unimplemented_spi_cmd;
-        }
         switch (sd->state) {
         case sd_transfer_state:
             sd->state = sd_receivingdata_state;
-- 
2.17.1

[Qemu-devel] [PATCH 3/8] sdcard: Add a 'spec_version' property

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/sd/sd.h |  4 ++++
 hw/sd/sd.c         | 11 +++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 9bdb3c9285..49f22b0b89 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -54,6 +54,10 @@
 #define APP_CMD			(1 << 5)
 #define AKE_SEQ_ERROR		(1 << 3)
 
+enum SDPhySpecificationVersion {
+    SD_PHY_SPECv1_10_VERS     = 1,
+};
+
 typedef enum {
     SD_VOLTAGE_0_4V     = 400,  /* currently not supported */
     SD_VOLTAGE_1_8V     = 1800,
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 80e70dd93e..5ddd211114 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -91,6 +91,7 @@ struct SDState {
     uint8_t sd_status[64];
 
     /* Configurable properties */
+    uint8_t spec_version;
     BlockBackend *blk;
     bool spi;
 
@@ -2058,6 +2059,14 @@ static void sd_realize(DeviceState *dev, Error **errp)
 
     sd->proto_name = sd->spi ? "SPI" : "SD";
 
+    switch (sd->spec_version) {
+    case SD_PHY_SPECv1_10_VERS:
+        break;
+    default:
+        error_setg(errp, "Invalid SD card Spec version: %u", sd->spec_version);
+        return;
+    }
+
     if (sd->blk && blk_is_read_only(sd->blk)) {
         error_setg(errp, "Cannot use read-only drive as SD card");
         return;
@@ -2074,6 +2083,8 @@ static void sd_realize(DeviceState *dev, Error **errp)
 }
 
 static Property sd_properties[] = {
+    DEFINE_PROP_UINT8("spec_version", SDState,
+                      spec_version, SD_PHY_SPECv1_10_VERS),
     DEFINE_PROP_DRIVE("drive", SDState, blk),
     /* We do not model the chip select pin, so allow the board to select
      * whether card should be in SSI or MMC/SD mode.  It is also up to the
-- 
2.17.1

[Qemu-devel] [PATCH 4/8] sdcard: Set Spec v2.00 as default

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/sd/sd.h |  1 +
 hw/sd/sd.c         | 13 +++++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 49f22b0b89..7c6ad3c8f1 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -56,6 +56,7 @@
 
 enum SDPhySpecificationVersion {
     SD_PHY_SPECv1_10_VERS     = 1,
+    SD_PHY_SPECv2_00_VERS     = 2,
 };
 
 typedef enum {
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 5ddd211114..81f178b36e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -311,8 +311,12 @@ static void sd_ocr_powerup(void *opaque)
 
 static void sd_set_scr(SDState *sd)
 {
-    sd->scr[0] = (0 << 4)       /* SCR structure version 1.0 */
-                 | 1;           /* Spec Version 1.10 */
+    sd->scr[0] = 0 << 4;        /* SCR structure version 1.0 */
+    if (sd->spec_version == SD_PHY_SPECv1_10_VERS) {
+        sd->scr[0] |= 1;        /* Spec Version 1.10 */
+    } else {
+        sd->scr[0] |= 2;        /* Spec Version 2.00 */
+    }
     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. */
@@ -2060,7 +2064,8 @@ static void sd_realize(DeviceState *dev, Error **errp)
     sd->proto_name = sd->spi ? "SPI" : "SD";
 
     switch (sd->spec_version) {
-    case SD_PHY_SPECv1_10_VERS:
+    case SD_PHY_SPECv1_10_VERS
+     ... SD_PHY_SPECv2_00_VERS:
         break;
     default:
         error_setg(errp, "Invalid SD card Spec version: %u", sd->spec_version);
@@ -2084,7 +2089,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
 
 static Property sd_properties[] = {
     DEFINE_PROP_UINT8("spec_version", SDState,
-                      spec_version, SD_PHY_SPECv1_10_VERS),
+                      spec_version, SD_PHY_SPECv2_00_VERS),
     DEFINE_PROP_DRIVE("drive", SDState, blk),
     /* We do not model the chip select pin, so allow the board to select
      * whether card should be in SSI or MMC/SD mode.  It is also up to the
-- 
2.17.1

[Qemu-devel] [PATCH 5/8] hw/sd/ssi-sd: Force cards connected in SPI mode to use Spec v1.10

[Philippe Mathieu-Daudé]

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

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index ae04b6641b..c62fdc871c 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -253,6 +253,8 @@ static void ssi_sd_realize(SSISlave *d, Error **errp)
     /* FIXME use a qdev drive property instead of drive_get_next() */
     dinfo = drive_get_next(IF_SD);
     carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
+    object_property_set_uint(OBJECT(carddev),
+                             SD_PHY_SPECv1_10_VERS, "spec_version", &err);
     if (dinfo) {
         qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err);
     }
-- 
2.17.1

[Qemu-devel] [PATCH 6/8] sdcard: Disable SEND_IF_COND (CMD8) for Spec v1

[Philippe Mathieu-Daudé]

SEND_IF_COND got introduced by Spec v2.

Firmwares use the CMD8 in SPI mode to poll the Spec version supported:

  7.2.1 Mode Selection and Initialization (SPI mode)

    The SD Card is powered up in the SD mode. It will enter
    SPI mode if the CS signal is asserted (negative) during
    the reception of the reset command (CMD0). If the card
    recognizes that the SD mode is required it will not
    respond to the command and remain in the SD mode. If SPI
    mode is required, the card will switch to SPI and respond
    with the SPI mode R1 response.
    The only way to return to the SD mode is by entering the
    power cycle. In SPI mode, the SD Card protocol state
    machine in SD mode is not observed. All the SD Card
    commands supported in SPI mode are always available.
    [...]
    If the card indicates an illegal command, the card is
    legacy and does not support CMD8. If the card supports
    CMD8 and can operate on the supplied voltage, the response
    echoes back the supply voltage and the check pattern that
    were set in the command argument.

This change restore the SSI/SD functionality of the Stellaris
LM3S6965EVB board.

Message-Id: 201204252110.20873.paul@codesourcery.com
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Paolo, I guess this would be a good use of the
Based-on: 201204252110.20873.paul@codesourcery.com tag in
the git history :P

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 81f178b36e..da65f2b178 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1017,7 +1017,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 8:	/* CMD8:   SEND_IF_COND */
-        /* Physical Layer Specification Version 2.00 command */
+        if (sd->spec_version < SD_PHY_SPECv2_00_VERS) {
+            break;
+        }
         if (sd->state != sd_idle_state) {
             break;
         }
-- 
2.17.1

[Qemu-devel] [PATCH 7/8] sdcard: Reflect when the Spec v3 is supported in the Config Register (SCR)

[Philippe Mathieu-Daudé]

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

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 7c6ad3c8f1..b865aafc33 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -57,6 +57,7 @@
 enum SDPhySpecificationVersion {
     SD_PHY_SPECv1_10_VERS     = 1,
     SD_PHY_SPECv2_00_VERS     = 2,
+    SD_PHY_SPECv3_01_VERS     = 3,
 };
 
 typedef enum {
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index da65f2b178..83426da133 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -315,11 +315,14 @@ static void sd_set_scr(SDState *sd)
     if (sd->spec_version == SD_PHY_SPECv1_10_VERS) {
         sd->scr[0] |= 1;        /* Spec Version 1.10 */
     } else {
-        sd->scr[0] |= 2;        /* Spec Version 2.00 */
+        sd->scr[0] |= 2;        /* Spec Version 2.00 or Version 3.0X */
     }
     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. */
+    if (sd->spec_version >= SD_PHY_SPECv3_01_VERS) {
+        sd->scr[2] |= 1 << 7;   /* Spec Version 3.0X */
+    }
     sd->scr[3] = 0x00;
     /* reserved for manufacturer usage */
     sd->scr[4] = 0x00;
@@ -2067,7 +2070,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
 
     switch (sd->spec_version) {
     case SD_PHY_SPECv1_10_VERS
-     ... SD_PHY_SPECv2_00_VERS:
+     ... SD_PHY_SPECv3_01_VERS:
         break;
     default:
         error_setg(errp, "Invalid SD card Spec version: %u", sd->spec_version);
-- 
2.17.1

[Qemu-devel] [PATCH 8/8] sdcard: Disable CMD19/CMD23 for Spec v2

[Philippe Mathieu-Daudé]

These commands got introduced by Spec v3.

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 83426da133..ac202e05a1 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1178,6 +1178,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 19:    /* CMD19: SEND_TUNING_BLOCK (SD) */
+        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+            break;
+        }
         if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
             sd->data_offset = 0;
@@ -1186,6 +1189,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 23:    /* CMD23: SET_BLOCK_COUNT */
+        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+            break;
+        }
         switch (sd->state) {
         case sd_transfer_state:
             sd->multi_blk_cnt = req.arg;
-- 
2.17.1

Re: [Qemu-devel] [PATCH 6/8] sdcard: Disable SEND_IF_COND (CMD8) for Spec v1

[Paolo Bonzini]

On 03/06/2018 02:08, Philippe Mathieu-Daudé wrote:
> Paolo, I guess this would be a good use of the
> Based-on: 201204252110.20873.paul@codesourcery.com tag in
> the git history :P

Oh, I see you did send it.  Thanks!

"Based-on" means that it applies to a tree with that patch.  It doesn't
mean that's the code you were looking at, when writing the patch.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 0/8] sdcard: cleanup the SD_SPEC version

[Alistair Francis]

On Sat, Jun 2, 2018 at 5:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi,
>
> This series adds a 'spec_version' property to the SD Card device,
> to allow to limit some commands to specific spec version range
> (some firmwares use this feature to detect which spec version the
> card implements).
>
> This restore the SSI/SD support of the Stellaris LM3S6965EVB board,
> which allow to stress the SD Card code with a nice integration test
> (waiting for another series to get merged to add the Avocado test):
>
>     $ qemu-system-arm -M lm3s6965evb -serial stdio \
>       -kernel sd_card.bin -sd sdcard.img
>
>     SD Card Example Program
>     Type 'help' for help.
>
>     /> ls
>     Open
>     listing
>
>     ----A 2012/04/25 17:44        12  README.TXT
>
>        1 File(s),        12 bytes total
>        0 Dir(s),      61182K bytes free
>
>     /> cat README.TXT
>     Hello World
>
> See:
> http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg03790.html
> http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg00353.html

Do you have a branch with all of your SD work available?

Alistair

>
> Regards,
>
> Phil.
>
> Philippe Mathieu-Daudé (8):
>   sdcard: Update the Configuration Register (SCR) to Spec Version 1.10
>   sdcard: Allow commands valid in SPI mode
>   sdcard: Add a 'spec_version' property
>   sdcard: Set Spec v2.00 as default
>   hw/sd/ssi-sd: Force cards connected in SPI mode to use Spec v1.10
>   sdcard: Disable SEND_IF_COND (CMD8) for Spec v1
>   sdcard: Reflect when the Spec v3 is supported in the Config Register (SCR)
>   sdcard: Disable CMD19/CMD23 for Spec v2
>
>  include/hw/sd/sd.h |  6 ++++++
>  hw/sd/sd.c         | 47 +++++++++++++++++++++++++++++-----------------
>  hw/sd/ssi-sd.c     |  2 ++
>  3 files changed, 38 insertions(+), 17 deletions(-)
>
> --
> 2.17.1
>
>

Re: [Qemu-devel] [PATCH 2/8] sdcard: Allow commands valid in SPI mode

[Alistair Francis]

On Sat, Jun 2, 2018 at 5:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> From the "Physical Layer Simplified Specification Version 1.10"
>   Chapter 7.3 "SPI Mode Transaction Packets"
>     Table 57: "Commands and arguments"
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/sd/sd.c | 14 --------------
>  1 file changed, 14 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index e1218d1fb6..80e70dd93e 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -960,8 +960,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>          return sd_illegal;
>
>      case 6:    /* CMD6:   SWITCH_FUNCTION */
> -        if (sd->spi)
> -            goto bad_cmd;
>          switch (sd->mode) {
>          case sd_data_transfer_mode:
>              sd_function_switch(sd, req.arg);
> @@ -1190,9 +1188,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>
>      /* Block write commands (Class 4) */
>      case 24:   /* CMD24:  WRITE_SINGLE_BLOCK */
> -        if (sd->spi) {
> -            goto unimplemented_spi_cmd;
> -        }
>          switch (sd->state) {
>          case sd_transfer_state:
>              /* Writing in SPI mode not implemented.  */
> @@ -1217,9 +1212,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>          break;
>
>      case 25:   /* CMD25:  WRITE_MULTIPLE_BLOCK */
> -        if (sd->spi) {
> -            goto unimplemented_spi_cmd;
> -        }
>          switch (sd->state) {
>          case sd_transfer_state:
>              /* Writing in SPI mode not implemented.  */
> @@ -1259,9 +1251,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>          break;
>
>      case 27:   /* CMD27:  PROGRAM_CSD */
> -        if (sd->spi) {
> -            goto unimplemented_spi_cmd;
> -        }
>          switch (sd->state) {
>          case sd_transfer_state:
>              sd->state = sd_receivingdata_state;
> @@ -1371,9 +1360,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>
>      /* Lock card commands (Class 7) */
>      case 42:   /* CMD42:  LOCK_UNLOCK */
> -        if (sd->spi) {
> -            goto unimplemented_spi_cmd;
> -        }
>          switch (sd->state) {
>          case sd_transfer_state:
>              sd->state = sd_receivingdata_state;
> --
> 2.17.1
>
>

Re: [Qemu-devel] [PATCH 3/8] sdcard: Add a 'spec_version' property

[Alistair Francis]

On Sat, Jun 2, 2018 at 5:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

Alistair

> ---
>  include/hw/sd/sd.h |  4 ++++
>  hw/sd/sd.c         | 11 +++++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index 9bdb3c9285..49f22b0b89 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -54,6 +54,10 @@
>  #define APP_CMD                        (1 << 5)
>  #define AKE_SEQ_ERROR          (1 << 3)
>
> +enum SDPhySpecificationVersion {
> +    SD_PHY_SPECv1_10_VERS     = 1,
> +};
> +
>  typedef enum {
>      SD_VOLTAGE_0_4V     = 400,  /* currently not supported */
>      SD_VOLTAGE_1_8V     = 1800,
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 80e70dd93e..5ddd211114 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -91,6 +91,7 @@ struct SDState {
>      uint8_t sd_status[64];
>
>      /* Configurable properties */
> +    uint8_t spec_version;
>      BlockBackend *blk;
>      bool spi;
>
> @@ -2058,6 +2059,14 @@ static void sd_realize(DeviceState *dev, Error **errp)
>
>      sd->proto_name = sd->spi ? "SPI" : "SD";
>
> +    switch (sd->spec_version) {
> +    case SD_PHY_SPECv1_10_VERS:
> +        break;
> +    default:
> +        error_setg(errp, "Invalid SD card Spec version: %u", sd->spec_version);
> +        return;
> +    }
> +
>      if (sd->blk && blk_is_read_only(sd->blk)) {
>          error_setg(errp, "Cannot use read-only drive as SD card");
>          return;
> @@ -2074,6 +2083,8 @@ static void sd_realize(DeviceState *dev, Error **errp)
>  }
>
>  static Property sd_properties[] = {
> +    DEFINE_PROP_UINT8("spec_version", SDState,
> +                      spec_version, SD_PHY_SPECv1_10_VERS),
>      DEFINE_PROP_DRIVE("drive", SDState, blk),
>      /* We do not model the chip select pin, so allow the board to select
>       * whether card should be in SSI or MMC/SD mode.  It is also up to the
> --
> 2.17.1
>
>

Re: [Qemu-devel] [PATCH 4/8] sdcard: Set Spec v2.00 as default

[Alistair Francis]

On Sat, Jun 2, 2018 at 5:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

Alistair

> ---
>  include/hw/sd/sd.h |  1 +
>  hw/sd/sd.c         | 13 +++++++++----
>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index 49f22b0b89..7c6ad3c8f1 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -56,6 +56,7 @@
>
>  enum SDPhySpecificationVersion {
>      SD_PHY_SPECv1_10_VERS     = 1,
> +    SD_PHY_SPECv2_00_VERS     = 2,
>  };
>
>  typedef enum {
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 5ddd211114..81f178b36e 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -311,8 +311,12 @@ static void sd_ocr_powerup(void *opaque)
>
>  static void sd_set_scr(SDState *sd)
>  {
> -    sd->scr[0] = (0 << 4)       /* SCR structure version 1.0 */
> -                 | 1;           /* Spec Version 1.10 */
> +    sd->scr[0] = 0 << 4;        /* SCR structure version 1.0 */
> +    if (sd->spec_version == SD_PHY_SPECv1_10_VERS) {
> +        sd->scr[0] |= 1;        /* Spec Version 1.10 */
> +    } else {
> +        sd->scr[0] |= 2;        /* Spec Version 2.00 */
> +    }
>      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. */
> @@ -2060,7 +2064,8 @@ static void sd_realize(DeviceState *dev, Error **errp)
>      sd->proto_name = sd->spi ? "SPI" : "SD";
>
>      switch (sd->spec_version) {
> -    case SD_PHY_SPECv1_10_VERS:
> +    case SD_PHY_SPECv1_10_VERS
> +     ... SD_PHY_SPECv2_00_VERS:
>          break;
>      default:
>          error_setg(errp, "Invalid SD card Spec version: %u", sd->spec_version);
> @@ -2084,7 +2089,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
>
>  static Property sd_properties[] = {
>      DEFINE_PROP_UINT8("spec_version", SDState,
> -                      spec_version, SD_PHY_SPECv1_10_VERS),
> +                      spec_version, SD_PHY_SPECv2_00_VERS),
>      DEFINE_PROP_DRIVE("drive", SDState, blk),
>      /* We do not model the chip select pin, so allow the board to select
>       * whether card should be in SSI or MMC/SD mode.  It is also up to the
> --
> 2.17.1
>
>

Re: [Qemu-devel] [PATCH 5/8] hw/sd/ssi-sd: Force cards connected in SPI mode to use Spec v1.10

[Alistair Francis]

On Sat, Jun 2, 2018 at 5:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Can you add a justification for this in the commit message?

Alistair

> ---
>  hw/sd/ssi-sd.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index ae04b6641b..c62fdc871c 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -253,6 +253,8 @@ static void ssi_sd_realize(SSISlave *d, Error **errp)
>      /* FIXME use a qdev drive property instead of drive_get_next() */
>      dinfo = drive_get_next(IF_SD);
>      carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
> +    object_property_set_uint(OBJECT(carddev),
> +                             SD_PHY_SPECv1_10_VERS, "spec_version", &err);
>      if (dinfo) {
>          qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err);
>      }
> --
> 2.17.1
>
>

Re: [Qemu-devel] [PATCH 7/8] sdcard: Reflect when the Spec v3 is supported in the Config Register (SCR)

[Alistair Francis]

On Sat, Jun 2, 2018 at 5:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

Alistair

> ---
>  include/hw/sd/sd.h | 1 +
>  hw/sd/sd.c         | 7 +++++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index 7c6ad3c8f1..b865aafc33 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -57,6 +57,7 @@
>  enum SDPhySpecificationVersion {
>      SD_PHY_SPECv1_10_VERS     = 1,
>      SD_PHY_SPECv2_00_VERS     = 2,
> +    SD_PHY_SPECv3_01_VERS     = 3,
>  };
>
>  typedef enum {
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index da65f2b178..83426da133 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -315,11 +315,14 @@ static void sd_set_scr(SDState *sd)
>      if (sd->spec_version == SD_PHY_SPECv1_10_VERS) {
>          sd->scr[0] |= 1;        /* Spec Version 1.10 */
>      } else {
> -        sd->scr[0] |= 2;        /* Spec Version 2.00 */
> +        sd->scr[0] |= 2;        /* Spec Version 2.00 or Version 3.0X */
>      }
>      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. */
> +    if (sd->spec_version >= SD_PHY_SPECv3_01_VERS) {
> +        sd->scr[2] |= 1 << 7;   /* Spec Version 3.0X */
> +    }
>      sd->scr[3] = 0x00;
>      /* reserved for manufacturer usage */
>      sd->scr[4] = 0x00;
> @@ -2067,7 +2070,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
>
>      switch (sd->spec_version) {
>      case SD_PHY_SPECv1_10_VERS
> -     ... SD_PHY_SPECv2_00_VERS:
> +     ... SD_PHY_SPECv3_01_VERS:
>          break;
>      default:
>          error_setg(errp, "Invalid SD card Spec version: %u", sd->spec_version);
> --
> 2.17.1
>
>

Re: [Qemu-devel] [PATCH 1/8] sdcard: Update the Configuration Register (SCR) to Spec Version 1.10

[Peter Maydell]

On 3 June 2018 at 01:08, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> This register now matches the description of the header:
>
>  * SD Memory Card emulation as defined in the "SD Memory Card Physical
>  * layer specification, Version 1.10."

Can you provide a brief justification in the commit message
of why it makes more sense for us to advertise as 1.10, please?

(eg a summary of the 1.01 to 1.10 differences and a note
that we already implement them)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/8] sdcard: Update the Configuration Register (SCR) to Spec Version 1.10

[Philippe Mathieu-Daudé]

Hi Peter,

On 06/07/2018 08:00 AM, Peter Maydell wrote:
> On 3 June 2018 at 01:08, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> This register now matches the description of the header:
>>
>>  * SD Memory Card emulation as defined in the "SD Memory Card Physical
>>  * layer specification, Version 1.10."
> 
> Can you provide a brief justification in the commit message
> of why it makes more sense for us to advertise as 1.10, please?
> 
> (eg a summary of the 1.01 to 1.10 differences and a note
> that we already implement them)

The only differences I found are:

    4.3.11 High-Speed mode (25MB/sec interface speed)
           (This chapter is newly added in version 1.10)

    Though the Rev 1.01 SD memory card supports up to 12.5MB/sec
    interface speed, the speed of 25MB/sec is necessary to support
    increasing performance needs of the host and because of memory
    size which continues to grow.
    To achieve 25MB/sec interface speed, clock rate is increased to
    50MHz and CLK/CMD/DAT signal timing and circuit conditions are
    reconsidered and changed from Physical Layer Specification
    Version 1.01.

    4.3.12 Command system
           (This chapter is newly added in version 1.10)

    SD commands CMD34-37, CMD50, CMD57 are reserved for SD command
    system expansion via the switch command.
    [These commands] will be considered as illegal commands (as
    defined in revision 1.01 of the SD physical layer specification).

The current codebase is a mix of v1 registers and v2 commands (which
were added slowly without updating all registers).
As of now v1 is broken (See the lm3s6965evb machine can not use the
card) and all users are using v2.
My goal with these series is to also support v3.

It might be more obvious to first enforce the current implementation is
v2, then restore v1 support and go for v3. I think this is doable by an
easy rebase of this series (keep v2 and restore v1).

Do you think this is better?

Regards,

Phil.

Re: [Qemu-devel] [PATCH 1/8] sdcard: Update the Configuration Register (SCR) to Spec Version 1.10

[Peter Maydell]

On 7 June 2018 at 16:14, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> The current codebase is a mix of v1 registers and v2 commands (which
> were added slowly without updating all registers).
> As of now v1 is broken (See the lm3s6965evb machine can not use the
> card) and all users are using v2.
> My goal with these series is to also support v3.
>
> It might be more obvious to first enforce the current implementation is
> v2, then restore v1 support and go for v3. I think this is doable by an
> easy rebase of this series (keep v2 and restore v1).
>
> Do you think this is better?

I don't have a strong opinion (you have a better grasp of the specs
than me); I'd just like justification for why we do what we do
in the commit messages and/or comments as appropriate.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH 5/8] hw/sd/ssi-sd: Force cards connected in SPI mode to use Spec v1.10

[Philippe Mathieu-Daudé]

On 06/04/2018 03:26 PM, Alistair Francis wrote:
> On Sat, Jun 2, 2018 at 5:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Can you add a justification for this in the commit message?

Oops, I misplaced the justification in the following patch
(6/8: Disable SEND_IF_COND (CMD8) for Spec v1)

> 
> Alistair
> 
>> ---
>>  hw/sd/ssi-sd.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
>> index ae04b6641b..c62fdc871c 100644
>> --- a/hw/sd/ssi-sd.c
>> +++ b/hw/sd/ssi-sd.c
>> @@ -253,6 +253,8 @@ static void ssi_sd_realize(SSISlave *d, Error **errp)
>>      /* FIXME use a qdev drive property instead of drive_get_next() */
>>      dinfo = drive_get_next(IF_SD);
>>      carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
>> +    object_property_set_uint(OBJECT(carddev),
>> +                             SD_PHY_SPECv1_10_VERS, "spec_version", &err);
>>      if (dinfo) {
>>          qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err);
>>      }
>> --
>> 2.17.1
>>
>>
> 

Re: [Qemu-devel] [PATCH 1/8] sdcard: Update the Configuration Register (SCR) to Spec Version 1.10

[Philippe Mathieu-Daudé]

On 06/07/2018 12:14 PM, Philippe Mathieu-Daudé wrote:
> On 06/07/2018 08:00 AM, Peter Maydell wrote:
>> On 3 June 2018 at 01:08, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>> This register now matches the description of the header:
>>>
>>>  * SD Memory Card emulation as defined in the "SD Memory Card Physical
>>>  * layer specification, Version 1.10."
>>
>> Can you provide a brief justification in the commit message
>> of why it makes more sense for us to advertise as 1.10, please?
>>
>> (eg a summary of the 1.01 to 1.10 differences and a note
>> that we already implement them)
> 
> The only differences I found are:

And I shamefully missed:

    4.3.10 Switch function command
           (This chapter is newly added in version 1.10)

    Switch function command (CMD6) 1 is used to switch or expand
    memory card functions. [...]

    This is a new feature, introduced in SD physical Layer
    Specification Version 1.10. Therefore, cards that are
    compatible with earlier versions of the spec do not support
    it. The host shall check the "SD_SPEC" field in the SCR
    register to recognize what version of the spec the card
    complies with before using CMD6. It is mandatory for SD
    memory card of Ver1.10 to support CMD6.

> 
>     4.3.11 High-Speed mode (25MB/sec interface speed)
>            (This chapter is newly added in version 1.10)
> 
>     Though the Rev 1.01 SD memory card supports up to 12.5MB/sec
>     interface speed, the speed of 25MB/sec is necessary to support
>     increasing performance needs of the host and because of memory
>     size which continues to grow.
>     To achieve 25MB/sec interface speed, clock rate is increased to
>     50MHz and CLK/CMD/DAT signal timing and circuit conditions are
>     reconsidered and changed from Physical Layer Specification
>     Version 1.01.
> 
>     4.3.12 Command system
>            (This chapter is newly added in version 1.10)
> 
>     SD commands CMD34-37, CMD50, CMD57 are reserved for SD command
>     system expansion via the switch command.
>     [These commands] will be considered as illegal commands (as
>     defined in revision 1.01 of the SD physical layer specification).