| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4937DCAF.3010806@nokia.com>
Date: Thu, 04 Dec 2008 15:35:43 +0200
From: Adrian Hunter <ext-adrian.hunter@...ia.com>
To: Pierre Ossman <drzeus@...eus.cx>
CC: LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] mmc_block: print better data error message after
timeout
Pierre Ossman wrote:
> On Wed, 29 Oct 2008 12:29:48 +0200
> Adrian Hunter <ext-adrian.hunter@...ia.com> wrote:
>
>> Add command response and card status to error
>> messages.
>>
>> Signed-off-by: Adrian Hunter <ext-adrian.hunter@...ia.com>
>> ---
>> drivers/mmc/card/block.c | 56 ++++++++++++++++++++++++++++++++++++++++-----
>> 1 files changed, 49 insertions(+), 7 deletions(-)
>>
>>
>> Pierre
>>
>> This patch amends error messages for all error codes and is independent of SPI.
>
> Looks good. It's ready to queue up except for two very small details
> (see below).
Sorry for not responding sooner. The mail got lost and I only just noticed it.
>> Status values are not OR'd together from different commands as that would
>> muddy the waters for anyone trying to understand the nature of the error.
>
> I suppose. I'm just generally uneasy about error handling in SD/MMC as
> the specs aren't that clear. :)
>
>> Waiting for the card to become ready after an error is also not included,
>> as it is not in the standards and is a separate issue anyway.
>
> What do you mean?
You had suggested
while (card not ready)
status |= send_status();
and I didn't do it, because of the reasons given above.
> This will handle your case of seeing read errors, but for write errors
> we probably need to wait for the write to complete fully. But that's
> something that can be added later...
Multiblock writes are terminated with a stop command when an error occurs,
and there is a loop waiting on the status whether an error occurs or not.
>>
>> +static u32 get_card_status(struct mmc_card *card, struct request *req)
>> +{
>> + struct mmc_command cmd;
>> + int err;
>> +
>> + /* SEND STATUS command is not supported by SDIO */
>> + if (mmc_card_sdio(card))
>> + return 0;
>> +
>
> This is redundant. We never bind to a SDIO card.
OK
>> if (brq.data.error) {
>> - printk(KERN_ERR "%s: error %d transferring data\n",
>> - req->rq_disk->disk_name, brq.data.error);
>> + if (brq.data.error == -ETIMEDOUT) {
>> + /* 'Stop' response contains card status */
>> + if (brq.mrq.stop)
>> + status = brq.mrq.stop->resp[0];
>> + printk(KERN_ERR "%s: error transferring data,"
>> + " sector %u, nr %u, card status %#x\n",
>> + req->rq_disk->disk_name,
>> + (unsigned)req->sector,
>> + (unsigned)req->nr_sectors, status);
>> + } else {
>> + printk(KERN_ERR "%s: error %d transferring data,"
>> + " sector %u, nr %u, card status %#x\n",
>> + req->rq_disk->disk_name, brq.data.error,
>> + (unsigned)req->sector,
>> + (unsigned)req->nr_sectors, status);
>> + }
>> }
>
> Do we need the two different printk:s? It would reduce the code a bit
> if you just let the if clause modify the status value.
OK
From: Adrian Hunter <ext-adrian.hunter@...ia.com>
Date: Thu, 16 Oct 2008 12:55:25 +0300
Subject: [PATCH] mmc_block: print better error messages
Add command response and card status to error
messages.
Signed-off-by: Adrian Hunter <ext-adrian.hunter@...ia.com>
---
drivers/mmc/card/block.c | 44 +++++++++++++++++++++++++++++++++++++-------
1 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 3d067c3..2998112 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -209,6 +209,23 @@ static u32 mmc_sd_num_wr_blocks(struct mmc_card *card)
return blocks;
}
+static u32 get_card_status(struct mmc_card *card, struct request *req)
+{
+ struct mmc_command cmd;
+ int err;
+
+ memset(&cmd, 0, sizeof(struct mmc_command));
+ cmd.opcode = MMC_SEND_STATUS;
+ if (!mmc_host_is_spi(card->host))
+ cmd.arg = card->rca << 16;
+ cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
+ err = mmc_wait_for_cmd(card->host, &cmd, 0);
+ if (err)
+ printk(KERN_ERR "%s: error %d sending status comand",
+ req->rq_disk->disk_name, err);
+ return cmd.resp[0];
+}
+
static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
{
struct mmc_blk_data *md = mq->data;
@@ -220,7 +237,7 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
do {
struct mmc_command cmd;
- u32 readcmd, writecmd;
+ u32 readcmd, writecmd, status = 0;
memset(&brq, 0, sizeof(struct mmc_blk_request));
brq.mrq.cmd = &brq.cmd;
@@ -275,19 +292,32 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
* until later as we need to wait for the card to leave
* programming mode even when things go wrong.
*/
+ if (brq.cmd.error || brq.data.error || brq.stop.error)
+ status = get_card_status(card, req);
+
if (brq.cmd.error) {
- printk(KERN_ERR "%s: error %d sending read/write command\n",
- req->rq_disk->disk_name, brq.cmd.error);
+ printk(KERN_ERR "%s: error %d sending read/write "
+ "command, response %#x, card status %#x\n",
+ req->rq_disk->disk_name, brq.cmd.error,
+ brq.cmd.resp[0], status);
}
if (brq.data.error) {
- printk(KERN_ERR "%s: error %d transferring data\n",
- req->rq_disk->disk_name, brq.data.error);
+ if (brq.data.error == -ETIMEDOUT && brq.mrq.stop)
+ /* 'Stop' response contains card status */
+ status = brq.mrq.stop->resp[0];
+ printk(KERN_ERR "%s: error %d transferring data,"
+ " sector %u, nr %u, card status %#x\n",
+ req->rq_disk->disk_name, brq.data.error,
+ (unsigned)req->sector,
+ (unsigned)req->nr_sectors, status);
}
if (brq.stop.error) {
- printk(KERN_ERR "%s: error %d sending stop command\n",
- req->rq_disk->disk_name, brq.stop.error);
+ printk(KERN_ERR "%s: error %d sending stop command, "
+ "response %#x, card status %#x\n",
+ req->rq_disk->disk_name, brq.stop.error,
+ brq.stop.resp[0], status);
}
if (!mmc_host_is_spi(card->host) && rq_data_dir(req) != READ) {
--
1.5.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists