Messages in this thread | | | Date | Tue, 12 Aug 2014 15:19:12 +0800 | From | Roger <> | Subject | Re: [PATCH] mmc: rtsx: fix incorrect last byte in R2 response |
| |
On 08/11/2014 09:02 PM, Dan Carpenter wrote: > On Mon, Aug 11, 2014 at 04:32:16PM +0800, rogerable@realtek.com wrote: >> From: Roger Tseng <rogerable@realtek.com> >> >> Current code erroneously fill the last byte of R2 response with an undefined >> value. In addition, it is impossible to obtain the real values since the >> controller actually 'offloads' the last byte(CRC7, end bit) while receiving R2 >> response. This could cause mmc stack to obtain inconsistent CID from the same >> card after resume and misidentify it as a different card. >> >> Fix by assigning a dummy value 0x01 to the last byte of R2 response. >> >> Signed-off-by: Roger Tseng <rogerable@realtek.com> >> --- >> drivers/mmc/host/rtsx_pci_sdmmc.c | 1 + >> drivers/mmc/host/rtsx_usb_sdmmc.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c >> index dfde4a2..54849d8 100644 >> --- a/drivers/mmc/host/rtsx_pci_sdmmc.c >> +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c >> @@ -412,6 +412,7 @@ static void sd_send_cmd_get_rsp(struct realtek_pci_sdmmc *host, >> } >> >> if (rsp_type == SD_RSP_TYPE_R2) { >> + ptr[16] = 1; > > Avoid magic numbers like 16 and 0x1. > > This is subtle enough that it deserves a comment. > > ptr[stat_idx] = 0x1 /* 0x1 chosen randomly */ > The 0x1 consists of 7-bit dummy zero CRC and stop bit 1, described in SD card. Anyway, I'll give a comment to this in the next version. >> for (i = 0; i < 4; i++) { >> cmd->resp[i] = get_unaligned_be32(ptr + 1 + i * 4); >> dev_dbg(sdmmc_dev(host), "cmd->resp[%d] = 0x%08x\n", > > There are a lot of magic numbers in this function. We could get rid > of this i < 4 loop but doing: > > memcpy(cmd->resp, ptr + 1, resp_len); > > Currently we don't use resp_len and the resp_len = 5 assignment is off > by one... It should be resp_len = 4. This function is quite ugly.
I can remove the unused rsp_len in this function. But I'm afraid the loop is still required. The destination cmd->resp is cpu-endian, but the raw response from SD card in the buffer (pointed by ptr) is big-endian.
> regards, > dan carpenter > > > ------Please consider the environment before printing this e-mail. > . >
Best regards, Roger Tseng
| |