qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: qemu-devel@nongnu.org
Cc: "Pragnesh Patel" <pragnesh.patel@sifive.com>,
	"Bin Meng" <bin.meng@windriver.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	qemu-block@nongnu.org
Subject: [PULL 3/9] hw/sd: sd: Drop sd_crc16()
Date: Sun, 24 Jan 2021 21:11:00 +0100	[thread overview]
Message-ID: <20210124201106.2602238-4-f4bug@amsat.org> (raw)
In-Reply-To: <20210124201106.2602238-1-f4bug@amsat.org>

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

commit f6fb1f9b319f ("sdcard: Correct CRC16 offset in sd_function_switch()")
changed the 16-bit CRC to be stored at offset 64. In fact, this CRC
calculation is completely wrong. From the original codes, it wants
to calculate the CRC16 of the first 64 bytes of sd->data[], however
passing 64 as the `width` to sd_crc16() actually counts 256 bytes
starting from the `message` for the CRC16 calculation, which is not
what we want.

Besides that, it seems existing sd_crc16() algorithm does not match
the SD spec, which says CRC16 is the CCITT one but the calculation
does not produce expected result. It turns out the CRC16 was never
transferred outside the sd core, as in sd_read_byte() we see:

    if (sd->data_offset >= 64)
        sd->state = sd_transfer_state;

Given above reasons, let's drop it.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Pragnesh Patel <pragnesh.patel@sifive.com>
Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210123104016.17485-6-bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bfea5547d53..b3952514fed 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -271,23 +271,6 @@ static uint8_t sd_crc7(const void *message, size_t width)
     return shift_reg;
 }
 
-static uint16_t sd_crc16(const void *message, size_t width)
-{
-    int i, bit;
-    uint16_t shift_reg = 0x0000;
-    const uint16_t *msg = (const uint16_t *)message;
-    width <<= 1;
-
-    for (i = 0; i < width; i ++, msg ++)
-        for (bit = 15; bit >= 0; bit --) {
-            shift_reg <<= 1;
-            if ((shift_reg >> 15) ^ ((*msg >> bit) & 1))
-                shift_reg ^= 0x1011;
-        }
-
-    return shift_reg;
-}
-
 #define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
 
 FIELD(OCR, VDD_VOLTAGE_WINDOW,          0, 24)
@@ -843,7 +826,6 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
         sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4);
     }
     memset(&sd->data[17], 0, 47);
-    stw_be_p(sd->data + 64, sd_crc16(sd->data, 64));
 }
 
 static inline bool sd_wp_addr(SDState *sd, uint64_t addr)
-- 
2.26.2



  parent reply	other threads:[~2021-01-24 20:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-24 20:10 [PULL 0/9] SD/MMC patches for 2021-01-24 Philippe Mathieu-Daudé
2021-01-24 20:10 ` [PULL 1/9] hw/sd: ssi-sd: Fix incorrect card response sequence Philippe Mathieu-Daudé
2021-01-24 20:10 ` [PULL 2/9] hw/sd: sd: Support CMD59 for SPI mode Philippe Mathieu-Daudé
2021-01-24 20:11 ` Philippe Mathieu-Daudé [this message]
2021-01-24 20:11 ` [PULL 4/9] util: Add CRC16 (CCITT) calculation routines Philippe Mathieu-Daudé
2021-01-24 20:11 ` [PULL 5/9] hw/sd: ssi-sd: Suffix a data block with CRC16 Philippe Mathieu-Daudé
2021-01-24 20:11 ` [PULL 6/9] hw/sd: ssi-sd: Add a state representing Nac Philippe Mathieu-Daudé
2021-01-24 20:11 ` [PULL 7/9] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION Philippe Mathieu-Daudé
2021-01-24 20:11 ` [PULL 8/9] hw/sd: ssi-sd: Use macros for the dummy value and tokens in the transfer Philippe Mathieu-Daudé
2021-01-24 20:11 ` [PULL 9/9] hw/sd: sd.h: Cosmetic change of using spaces Philippe Mathieu-Daudé
2021-01-25 13:48 ` [PULL 0/9] SD/MMC patches for 2021-01-24 Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210124201106.2602238-4-f4bug@amsat.org \
    --to=f4bug@amsat.org \
    --cc=bin.meng@windriver.com \
    --cc=pragnesh.patel@sifive.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).