[PULL 0/5] SD/MMC patches for 2020-10-26

[PULL 0/5] SD/MMC patches for 2020-10-26

[Philippe Mathieu-Daudé]

The following changes since commit 4c5b97bfd0dd54dc27717ae8d1cd10e14eef1430:

  Merge remote-tracking branch 'remotes/kraxel/tags/modules-20201022-pull-req=
uest' into staging (2020-10-22 12:33:21 +0100)

are available in the Git repository at:

  https://gitlab.com/philmd/qemu.git tags/sd-next-20201026

for you to fetch changes up to 89c6700fe7eed9195f10055751edbc6d5e7ab940:

  hw/sd/sdcard: Zero out function selection fields before being populated (20=
20-10-26 09:23:47 +0100)

----------------------------------------------------------------
SD/MMC patches

- Fix a bug in CMD6/SWITCH_FUNCTION (Bin Meng)
- Minor housekeeping patches

CI jobs result:
. https://cirrus-ci.com/build/5461987880599552
. https://gitlab.com/philmd/qemu/-/pipelines/207532287
. https://travis-ci.org/github/philmd/qemu/builds/738901111
. https://app.shippable.com/github/philmd/qemu/runs/888/summary/console
----------------------------------------------------------------

Bin Meng (1):
  hw/sd/sdcard: Zero out function selection fields before being
    populated

Philippe Mathieu-Daud=C3=A9 (4):
  hw/sd/sdcard: Update the SDState documentation
  hw/sd/sdcard: Simplify cmd_valid_while_locked()
  hw/sd/sdcard: Constify sd_crc*()'s message argument
  hw/sd/sdcard: Make iolen unsigned

 hw/sd/sd.c         | 27 +++++++++++++++------------
 hw/sd/trace-events |  2 +-
 2 files changed, 16 insertions(+), 13 deletions(-)

--=20
2.26.2

[PULL 1/5] hw/sd/sdcard: Update the SDState documentation

[Philippe Mathieu-Daudé]

Add more descriptive comments to keep a clear separation
between static property vs runtime changeable.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630133912.9428-12-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 c3febed2434..9b53053527e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -102,11 +102,14 @@ struct SDState {
     uint32_t card_status;
     uint8_t sd_status[64];
 
-    /* Configurable properties */
+    /* Static properties */
+
     uint8_t spec_version;
     BlockBackend *blk;
     bool spi;
 
+    /* Runtime changeables */
+
     uint32_t mode;    /* current card mode, one of SDCardModes */
     int32_t state;    /* current card state, one of SDCardStates */
     uint32_t vhs;
-- 
2.26.2

[PULL 2/5] hw/sd/sdcard: Simplify cmd_valid_while_locked()

[Philippe Mathieu-Daudé]

cmd_valid_while_locked() only needs to read SDRequest->cmd,
pass it directly and make it const.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630133912.9428-13-f4bug@amsat.org>
---
 hw/sd/sd.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 9b53053527e..5510a64401b 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1679,7 +1679,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     return sd_illegal;
 }
 
-static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
+static int cmd_valid_while_locked(SDState *sd, const uint8_t cmd)
 {
     /* Valid commands in locked state:
      * basic class (0)
@@ -1690,13 +1690,12 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
      * Anything else provokes an "illegal command" response.
      */
     if (sd->expecting_acmd) {
-        return req->cmd == 41 || req->cmd == 42;
+        return cmd == 41 || cmd == 42;
     }
-    if (req->cmd == 16 || req->cmd == 55) {
+    if (cmd == 16 || cmd == 55) {
         return 1;
     }
-    return sd_cmd_class[req->cmd] == 0
-            || sd_cmd_class[req->cmd] == 7;
+    return sd_cmd_class[cmd] == 0 || sd_cmd_class[cmd] == 7;
 }
 
 int sd_do_command(SDState *sd, SDRequest *req,
@@ -1722,7 +1721,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
     }
 
     if (sd->card_status & CARD_IS_LOCKED) {
-        if (!cmd_valid_while_locked(sd, req)) {
+        if (!cmd_valid_while_locked(sd, req->cmd)) {
             sd->card_status |= ILLEGAL_COMMAND;
             sd->expecting_acmd = false;
             qemu_log_mask(LOG_GUEST_ERROR, "SD: Card is locked\n");
-- 
2.26.2

[PULL 3/5] hw/sd/sdcard: Constify sd_crc*()'s message argument

[Philippe Mathieu-Daudé]

CRC functions don't modify the buffer argument,
make it const.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20200630133912.9428-14-f4bug@amsat.org>
---
 hw/sd/sd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 5510a64401b..aa6059d2ad6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -254,11 +254,11 @@ static const int sd_cmd_class[SDMMC_CMD_MAX] = {
     7,  7, 10,  7,  9,  9,  9,  8,  8, 10,  8,  8,  8,  8,  8,  8,
 };
 
-static uint8_t sd_crc7(void *message, size_t width)
+static uint8_t sd_crc7(const void *message, size_t width)
 {
     int i, bit;
     uint8_t shift_reg = 0x00;
-    uint8_t *msg = (uint8_t *) message;
+    const uint8_t *msg = (const uint8_t *)message;
 
     for (i = 0; i < width; i ++, msg ++)
         for (bit = 7; bit >= 0; bit --) {
@@ -270,11 +270,11 @@ static uint8_t sd_crc7(void *message, size_t width)
     return shift_reg;
 }
 
-static uint16_t sd_crc16(void *message, size_t width)
+static uint16_t sd_crc16(const void *message, size_t width)
 {
     int i, bit;
     uint16_t shift_reg = 0x0000;
-    uint16_t *msg = (uint16_t *) message;
+    const uint16_t *msg = (const uint16_t *)message;
     width <<= 1;
 
     for (i = 0; i < width; i ++, msg ++)
-- 
2.26.2

[PULL 4/5] hw/sd/sdcard: Make iolen unsigned

[Philippe Mathieu-Daudé]

I/O request length can not be negative.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630133912.9428-15-f4bug@amsat.org>
---
 hw/sd/sd.c         | 2 +-
 hw/sd/trace-events | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index aa6059d2ad6..fcbc1fd9d6e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1982,7 +1982,7 @@ uint8_t sd_read_byte(SDState *sd)
 {
     /* TODO: Append CRCs */
     uint8_t ret;
-    int io_len;
+    uint32_t io_len;
 
     if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable)
         return 0x00;
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 96c7ea5e52f..4140e485403 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -52,7 +52,7 @@ sdcard_unlock(void) ""
 sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 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_read_data(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t length) "%s %20s/ CMD%02d len %" PRIu32
 sdcard_set_voltage(uint16_t millivolts) "%u mV"
 
 # milkymist-memcard.c
-- 
2.26.2

[PULL 5/5] hw/sd/sdcard: Zero out function selection fields before being populated

[Philippe Mathieu-Daudé]

From: Bin Meng <bin.meng@windriver.com>

The function selection fields (399:376) should be zeroed out to
prevent leftover from being or'ed into the switch function status
data structure.

This fixes the boot failure as seen in the acceptance testing on
the orangepi target.

Fixes: b638627c723a ("hw/sd: Fix incorrect populated function switch status data structure")
Reported-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201024014954.21330-1-bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index fcbc1fd9d6e..30913826145 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -827,6 +827,7 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
     sd->data[12] = 0x80;	/* Supported group 1 functions */
     sd->data[13] = 0x03;
 
+    memset(&sd->data[14], 0, 3);
     for (i = 0; i < 6; i ++) {
         new_func = (arg >> (i * 4)) & 0x0f;
         if (mode && new_func != 0x0f)
-- 
2.26.2

Re: [PULL 0/5] SD/MMC patches for 2020-10-26

[Peter Maydell]

On Mon, 26 Oct 2020 at 11:14, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The following changes since commit 4c5b97bfd0dd54dc27717ae8d1cd10e14eef1430:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/modules-20201022-pull-req=
> uest' into staging (2020-10-22 12:33:21 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/philmd/qemu.git tags/sd-next-20201026
>
> for you to fetch changes up to 89c6700fe7eed9195f10055751edbc6d5e7ab940:
>
>   hw/sd/sdcard: Zero out function selection fields before being populated (20=
> 20-10-26 09:23:47 +0100)
>
> ----------------------------------------------------------------
> SD/MMC patches
>
> - Fix a bug in CMD6/SWITCH_FUNCTION (Bin Meng)
> - Minor housekeeping patches
>
> CI jobs result:
> . https://cirrus-ci.com/build/5461987880599552
> . https://gitlab.com/philmd/qemu/-/pipelines/207532287
> . https://travis-ci.org/github/philmd/qemu/builds/738901111
> . https://app.shippable.com/github/philmd/qemu/runs/888/summary/console
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM