Subject: [PATCH 1/5] ide-cd: respect REQ_QUIET for fs requests in cdrom_decode_status()

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ide-cd: respect REQ_QUIET for fs requests in cdrom_decode_status()

Cc: Borislav Petkov <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ide/ide-cd.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -382,7 +382,8 @@ static int cdrom_decode_status(ide_drive
cdrom_saw_media_change(drive);

/* fail the request */
- printk(KERN_ERR PFX "%s: tray open\n",
+ if ((rq->cmd_flags & REQ_QUIET) == 0)
+ printk(KERN_ERR PFX "%s: tray open\n",
drive->name);
} else {
if (ide_cd_breathe(drive, rq))
@@ -405,19 +406,23 @@ static int cdrom_decode_status(ide_drive
* No point in retrying after an illegal request or data
* protect error.
*/
- ide_dump_status(drive, "command error", stat);
+ if ((rq->cmd_flags & REQ_QUIET) == 0)
+ ide_dump_status(drive, "command error", stat);
do_end_request = 1;
} else if (sense_key == MEDIUM_ERROR) {
/*
* No point in re-trying a zillion times on a bad
* sector. If we got here the error is not correctable.
*/
- ide_dump_status(drive, "media error (bad sector)",
- stat);
+ if ((rq->cmd_flags & REQ_QUIET) == 0)
+ ide_dump_status(drive, "media error "
+ "(bad sector)", stat);
do_end_request = 1;
} else if (sense_key == BLANK_CHECK) {
/* disk appears blank ?? */
- ide_dump_status(drive, "media error (blank)", stat);
+ if ((rq->cmd_flags & REQ_QUIET) == 0)
+ ide_dump_status(drive, "media error (blank)",
+ stat);
do_end_request = 1;
} else if ((err & ~ATA_ABORTED) != 0) {
/* go to the default handler for other errors */


Subject: [PATCH 2/5] ide-cd: update debugging support

From: Borislav Petkov <[email protected]>
Subject: [PATCH] ide-cd: update debugging support

Signed-off-by: Borislav Petkov <[email protected]>
[bart: extracted from "ide-cd: cleanup cdrom_decode_status" patch]
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ide/ide-cd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -317,8 +317,9 @@ static int cdrom_decode_status(ide_drive
err = ide_read_error(drive);
sense_key = err >> 4;

- ide_debug_log(IDE_DBG_RQ, "cmd[0]: 0x%x, rq->cmd_type: 0x%x, err: 0x%x",
- rq->cmd[0], rq->cmd_type, err);
+ ide_debug_log(IDE_DBG_RQ, "cmd: 0x%x, rq->cmd_type: 0x%x, err: 0x%x, "
+ "stat 0x%x",
+ rq->cmd[0], rq->cmd_type, err, stat);

if (blk_sense_request(rq)) {
/*
@@ -636,8 +637,7 @@ static ide_startstop_t cdrom_newpc_intr(
u16 len;
u8 ireason, stat;

- ide_debug_log(IDE_DBG_PC, "cmd[0]: 0x%x, write: 0x%x",
- rq->cmd[0], write);
+ ide_debug_log(IDE_DBG_PC, "cmd: 0x%x, write: 0x%x", rq->cmd[0], write);

/* check for errors */
dma = drive->dma;

Subject: [PATCH 3/5] ide-cd: convert cdrom_decode_status() to use switch statements

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ide-cd: convert cdrom_decode_status() to use switch statements

Based on earlier work by Borislav Petkov.

Convert cdrom_decode_status() to use switch statements in
preparation to unify handling of fs and pc requests.

While at it:
- remove superfluous comments and do minor CodingStyle fixups

There should be no functional changes caused by this patch.

Cc: Borislav Petkov <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ide/ide-cd.c | 57 ++++++++++++++++++++++++++++-----------------------
1 file changed, 32 insertions(+), 25 deletions(-)

Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -339,15 +339,14 @@ static int cdrom_decode_status(ide_drive
if (blk_pc_request(rq) && !rq->errors)
rq->errors = SAM_STAT_CHECK_CONDITION;

- /* check for tray open */
- if (sense_key == NOT_READY) {
+ switch (sense_key) {
+ case NOT_READY:
cdrom_saw_media_change(drive);
- } else if (sense_key == UNIT_ATTENTION) {
- /* check for media change */
+ break;
+ case UNIT_ATTENTION:
cdrom_saw_media_change(drive);
return 0;
- } else if (sense_key == ILLEGAL_REQUEST &&
- rq->cmd[0] == GPCMD_START_STOP_UNIT) {
+ case ILLEGAL_REQUEST:
/*
* Don't print error message for this condition--
* SFF8090i indicates that 5/24/00 is the correct
@@ -355,9 +354,13 @@ static int cdrom_decode_status(ide_drive
* drive doesn't have that capability.
* cdrom_log_sense() knows this!
*/
- } else if (!(rq->cmd_flags & REQ_QUIET)) {
- /* otherwise, print an error */
- ide_dump_status(drive, "packet command error", stat);
+ if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
+ break;
+ /* fall-through */
+ default:
+ if ((rq->cmd_flags & REQ_QUIET) == 0)
+ ide_dump_status(drive, "packet command error",
+ stat);
}

rq->cmd_flags |= REQ_FAILED;
@@ -377,12 +380,11 @@ static int cdrom_decode_status(ide_drive
if (blk_noretry_request(rq))
do_end_request = 1;

- if (sense_key == NOT_READY) {
- /* tray open */
+ switch (sense_key) {
+ case NOT_READY:
if (rq_data_dir(rq) == READ) {
cdrom_saw_media_change(drive);

- /* fail the request */
if ((rq->cmd_flags & REQ_QUIET) == 0)
printk(KERN_ERR PFX "%s: tray open\n",
drive->name);
@@ -391,8 +393,8 @@ static int cdrom_decode_status(ide_drive
return 1;
}
do_end_request = 1;
- } else if (sense_key == UNIT_ATTENTION) {
- /* media change */
+ break;
+ case UNIT_ATTENTION:
cdrom_saw_media_change(drive);

/*
@@ -401,8 +403,9 @@ static int cdrom_decode_status(ide_drive
*/
if (++rq->errors > ERROR_MAX)
do_end_request = 1;
- } else if (sense_key == ILLEGAL_REQUEST ||
- sense_key == DATA_PROTECT) {
+ break;
+ case ILLEGAL_REQUEST:
+ case DATA_PROTECT:
/*
* No point in retrying after an illegal request or data
* protect error.
@@ -410,7 +413,8 @@ static int cdrom_decode_status(ide_drive
if ((rq->cmd_flags & REQ_QUIET) == 0)
ide_dump_status(drive, "command error", stat);
do_end_request = 1;
- } else if (sense_key == MEDIUM_ERROR) {
+ break;
+ case MEDIUM_ERROR:
/*
* No point in re-trying a zillion times on a bad
* sector. If we got here the error is not correctable.
@@ -419,19 +423,22 @@ static int cdrom_decode_status(ide_drive
ide_dump_status(drive, "media error "
"(bad sector)", stat);
do_end_request = 1;
- } else if (sense_key == BLANK_CHECK) {
+ break;
+ case BLANK_CHECK:
/* disk appears blank ?? */
if ((rq->cmd_flags & REQ_QUIET) == 0)
ide_dump_status(drive, "media error (blank)",
stat);
do_end_request = 1;
- } else if ((err & ~ATA_ABORTED) != 0) {
- /* go to the default handler for other errors */
- ide_error(drive, "cdrom_decode_status", stat);
- return 1;
- } else if ((++rq->errors > ERROR_MAX)) {
- /* we've racked up too many retries, abort */
- do_end_request = 1;
+ break;
+ default:
+ if (err & ~ATA_ABORTED) {
+ /* go to the default handler for other errors */
+ ide_error(drive, "cdrom_decode_status", stat);
+ return 1;
+ } else if (++rq->errors > ERROR_MAX)
+ /* we've racked up too many retries, abort */
+ do_end_request = 1;
}

/*

Subject: [PATCH 4/5] ide-cd: unify handling of fs and pc requests in cdrom_decode_status()

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ide-cd: unify handling of fs and pc requests in cdrom_decode_status()

Based on earlier work by Borislav Petkov.

Unify handling of fs and pc requests in cdrom_decode_status().

While at it:
- remove unreachable code

The only change in functionality is that for pc requests more
detailed error message will be printed for following sense keys:
* ILLEGAL_REQUEST
* DATA_PROTECT
* MEDIUM_ERROR
* BLANK_CHECK

Cc: Borislav Petkov <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ide/ide-cd.c | 70 ++++++++++++++++++---------------------------------
1 file changed, 25 insertions(+), 45 deletions(-)

Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -329,8 +329,8 @@ static int cdrom_decode_status(ide_drive
*/
rq->cmd_flags |= REQ_FAILED;
return 2;
- } else if (blk_pc_request(rq) || rq->cmd_type == REQ_TYPE_ATA_PC) {
- /* All other functions, except for READ. */
+ } else {
+ int do_end_request = 0;

/*
* if we have an error, pass back CHECK_CONDITION as the
@@ -339,53 +339,17 @@ static int cdrom_decode_status(ide_drive
if (blk_pc_request(rq) && !rq->errors)
rq->errors = SAM_STAT_CHECK_CONDITION;

- switch (sense_key) {
- case NOT_READY:
- cdrom_saw_media_change(drive);
- break;
- case UNIT_ATTENTION:
- cdrom_saw_media_change(drive);
- return 0;
- case ILLEGAL_REQUEST:
- /*
- * Don't print error message for this condition--
- * SFF8090i indicates that 5/24/00 is the correct
- * response to a request to close the tray if the
- * drive doesn't have that capability.
- * cdrom_log_sense() knows this!
- */
- if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
- break;
- /* fall-through */
- default:
- if ((rq->cmd_flags & REQ_QUIET) == 0)
- ide_dump_status(drive, "packet command error",
- stat);
- }
-
- rq->cmd_flags |= REQ_FAILED;
-
- /*
- * instead of playing games with moving completions around,
- * remove failed request completely and end it when the
- * request sense has completed
- */
- goto end_request;
-
- } else if (blk_fs_request(rq)) {
- int do_end_request = 0;
-
- /* handle errors from READ and WRITE requests */
-
if (blk_noretry_request(rq))
do_end_request = 1;

switch (sense_key) {
case NOT_READY:
- if (rq_data_dir(rq) == READ) {
+ if (blk_fs_request(rq) == 0 ||
+ rq_data_dir(rq) == READ) {
cdrom_saw_media_change(drive);

- if ((rq->cmd_flags & REQ_QUIET) == 0)
+ if (blk_fs_request(rq) &&
+ (rq->cmd_flags & REQ_QUIET) == 0)
printk(KERN_ERR PFX "%s: tray open\n",
drive->name);
} else {
@@ -397,6 +361,8 @@ static int cdrom_decode_status(ide_drive
case UNIT_ATTENTION:
cdrom_saw_media_change(drive);

+ if (blk_fs_request(rq) == 0)
+ return 0;
/*
* Arrange to retry the request but be sure to give up
* if we've retried too many times.
@@ -405,6 +371,16 @@ static int cdrom_decode_status(ide_drive
do_end_request = 1;
break;
case ILLEGAL_REQUEST:
+ /*
+ * Don't print error message for this condition--
+ * SFF8090i indicates that 5/24/00 is the correct
+ * response to a request to close the tray if the
+ * drive doesn't have that capability.
+ * cdrom_log_sense() knows this!
+ */
+ if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
+ break;
+ /* fall-through */
case DATA_PROTECT:
/*
* No point in retrying after an illegal request or data
@@ -432,6 +408,8 @@ static int cdrom_decode_status(ide_drive
do_end_request = 1;
break;
default:
+ if (blk_fs_request(rq) == 0)
+ break;
if (err & ~ATA_ABORTED) {
/* go to the default handler for other errors */
ide_error(drive, "cdrom_decode_status", stat);
@@ -441,6 +419,11 @@ static int cdrom_decode_status(ide_drive
do_end_request = 1;
}

+ if (blk_fs_request(rq) == 0) {
+ rq->cmd_flags |= REQ_FAILED;
+ do_end_request = 1;
+ }
+
/*
* End a request through request sense analysis when we have
* sense data. We need this in order to perform end of media
@@ -456,9 +439,6 @@ static int cdrom_decode_status(ide_drive
if (stat & ATA_ERR)
cdrom_queue_request_sense(drive, NULL, NULL);
return 1;
- } else {
- blk_dump_rq_flags(rq, PFX "bad rq");
- return 2;
}

end_request:

Subject: [PATCH 5/5] ide-cd: fix intendation in cdrom_decode_status()

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ide-cd: fix intendation in cdrom_decode_status()

Based on earlier work by Borislav Petkov.

Fix intendation in cdrom_decode_status(), no real code changes.

While at it:
- beautify comments

There should be no functional changes caused by this patch.

Cc: Borislav Petkov <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ide/ide-cd.c | 190 ++++++++++++++++++++++++---------------------------
1 file changed, 90 insertions(+), 100 deletions(-)

Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -311,7 +311,7 @@ static int cdrom_decode_status(ide_drive
{
ide_hwif_t *hwif = drive->hwif;
struct request *rq = hwif->rq;
- int err, sense_key;
+ int err, sense_key, do_end_request = 0;

/* get the IDE error register */
err = ide_read_error(drive);
@@ -329,118 +329,108 @@ static int cdrom_decode_status(ide_drive
*/
rq->cmd_flags |= REQ_FAILED;
return 2;
- } else {
- int do_end_request = 0;
-
- /*
- * if we have an error, pass back CHECK_CONDITION as the
- * scsi status byte
- */
- if (blk_pc_request(rq) && !rq->errors)
- rq->errors = SAM_STAT_CHECK_CONDITION;
-
- if (blk_noretry_request(rq))
- do_end_request = 1;
-
- switch (sense_key) {
- case NOT_READY:
- if (blk_fs_request(rq) == 0 ||
- rq_data_dir(rq) == READ) {
- cdrom_saw_media_change(drive);
+ }

- if (blk_fs_request(rq) &&
- (rq->cmd_flags & REQ_QUIET) == 0)
- printk(KERN_ERR PFX "%s: tray open\n",
- drive->name);
- } else {
- if (ide_cd_breathe(drive, rq))
- return 1;
- }
- do_end_request = 1;
- break;
- case UNIT_ATTENTION:
+ /* if we got an error, pass CHECK_CONDITION as the SCSI status byte */
+ if (blk_pc_request(rq) && !rq->errors)
+ rq->errors = SAM_STAT_CHECK_CONDITION;
+
+ if (blk_noretry_request(rq))
+ do_end_request = 1;
+
+ switch (sense_key) {
+ case NOT_READY:
+ if (blk_fs_request(rq) == 0 || rq_data_dir(rq) == READ) {
cdrom_saw_media_change(drive);

- if (blk_fs_request(rq) == 0)
- return 0;
- /*
- * Arrange to retry the request but be sure to give up
- * if we've retried too many times.
- */
- if (++rq->errors > ERROR_MAX)
- do_end_request = 1;
- break;
- case ILLEGAL_REQUEST:
- /*
- * Don't print error message for this condition--
- * SFF8090i indicates that 5/24/00 is the correct
- * response to a request to close the tray if the
- * drive doesn't have that capability.
- * cdrom_log_sense() knows this!
- */
- if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
- break;
- /* fall-through */
- case DATA_PROTECT:
- /*
- * No point in retrying after an illegal request or data
- * protect error.
- */
- if ((rq->cmd_flags & REQ_QUIET) == 0)
- ide_dump_status(drive, "command error", stat);
- do_end_request = 1;
- break;
- case MEDIUM_ERROR:
- /*
- * No point in re-trying a zillion times on a bad
- * sector. If we got here the error is not correctable.
- */
- if ((rq->cmd_flags & REQ_QUIET) == 0)
- ide_dump_status(drive, "media error "
- "(bad sector)", stat);
- do_end_request = 1;
- break;
- case BLANK_CHECK:
- /* disk appears blank ?? */
- if ((rq->cmd_flags & REQ_QUIET) == 0)
- ide_dump_status(drive, "media error (blank)",
- stat);
- do_end_request = 1;
- break;
- default:
- if (blk_fs_request(rq) == 0)
- break;
- if (err & ~ATA_ABORTED) {
- /* go to the default handler for other errors */
- ide_error(drive, "cdrom_decode_status", stat);
+ if (blk_fs_request(rq) &&
+ (rq->cmd_flags & REQ_QUIET) == 0)
+ printk(KERN_ERR PFX "%s: tray open\n",
+ drive->name);
+ } else {
+ if (ide_cd_breathe(drive, rq))
return 1;
- } else if (++rq->errors > ERROR_MAX)
- /* we've racked up too many retries, abort */
- do_end_request = 1;
}
+ do_end_request = 1;
+ break;
+ case UNIT_ATTENTION:
+ cdrom_saw_media_change(drive);

- if (blk_fs_request(rq) == 0) {
- rq->cmd_flags |= REQ_FAILED;
+ if (blk_fs_request(rq) == 0)
+ return 0;
+ /*
+ * Arrange to retry the request but be sure to give up if we've
+ * retried too many times.
+ */
+ if (++rq->errors > ERROR_MAX)
do_end_request = 1;
- }
-
+ break;
+ case ILLEGAL_REQUEST:
/*
- * End a request through request sense analysis when we have
- * sense data. We need this in order to perform end of media
- * processing.
+ * Don't print error message for this condition -- SFF8090i
+ * indicates that 5/24/00 is the correct response to a request
+ * to close the tray if the drive doesn't have that capability.
+ *
+ * cdrom_log_sense() knows this!
*/
- if (do_end_request)
- goto end_request;
-
+ if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
+ break;
+ /* fall-through */
+ case DATA_PROTECT:
/*
- * If we got a CHECK_CONDITION status, queue
- * a request sense command.
+ * No point in retrying after an illegal request or data
+ * protect error.
*/
- if (stat & ATA_ERR)
- cdrom_queue_request_sense(drive, NULL, NULL);
- return 1;
+ if ((rq->cmd_flags & REQ_QUIET) == 0)
+ ide_dump_status(drive, "command error", stat);
+ do_end_request = 1;
+ break;
+ case MEDIUM_ERROR:
+ /*
+ * No point in re-trying a zillion times on a bad sector.
+ * If we got here the error is not correctable.
+ */
+ if ((rq->cmd_flags & REQ_QUIET) == 0)
+ ide_dump_status(drive, "media error "
+ "(bad sector)", stat);
+ do_end_request = 1;
+ break;
+ case BLANK_CHECK:
+ /* disk appears blank? */
+ if ((rq->cmd_flags & REQ_QUIET) == 0)
+ ide_dump_status(drive, "media error (blank)",
+ stat);
+ do_end_request = 1;
+ break;
+ default:
+ if (blk_fs_request(rq) == 0)
+ break;
+ if (err & ~ATA_ABORTED) {
+ /* go to the default handler for other errors */
+ ide_error(drive, "cdrom_decode_status", stat);
+ return 1;
+ } else if (++rq->errors > ERROR_MAX)
+ /* we've racked up too many retries, abort */
+ do_end_request = 1;
}

+ if (blk_fs_request(rq) == 0) {
+ rq->cmd_flags |= REQ_FAILED;
+ do_end_request = 1;
+ }
+
+ /*
+ * End a request through request sense analysis when we have sense data.
+ * We need this in order to perform end of media processing.
+ */
+ if (do_end_request)
+ goto end_request;
+
+ /* if we got a CHECK_CONDITION status, queue a request sense command */
+ if (stat & ATA_ERR)
+ cdrom_queue_request_sense(drive, NULL, NULL);
+ return 1;
+
end_request:
if (stat & ATA_ERR) {
struct request_queue *q = drive->queue;

2009-04-05 05:14:01

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/5] ide-cd: respect REQ_QUIET for fs requests in cdrom_decode_status()

On Fri, Apr 03, 2009 at 09:57:57PM +0200, Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz <[email protected]>
> Subject: [PATCH] ide-cd: respect REQ_QUIET for fs requests in cdrom_decode_status()
>
> Cc: Borislav Petkov <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> ---
> drivers/ide/ide-cd.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> Index: b/drivers/ide/ide-cd.c
> ===================================================================
> --- a/drivers/ide/ide-cd.c
> +++ b/drivers/ide/ide-cd.c
> @@ -382,7 +382,8 @@ static int cdrom_decode_status(ide_drive
> cdrom_saw_media_change(drive);
>
> /* fail the request */
> - printk(KERN_ERR PFX "%s: tray open\n",
> + if ((rq->cmd_flags & REQ_QUIET) == 0)
> + printk(KERN_ERR PFX "%s: tray open\n",
> drive->name);
> } else {
> if (ide_cd_breathe(drive, rq))
> @@ -405,19 +406,23 @@ static int cdrom_decode_status(ide_drive
> * No point in retrying after an illegal request or data
> * protect error.
> */
> - ide_dump_status(drive, "command error", stat);
> + if ((rq->cmd_flags & REQ_QUIET) == 0)
> + ide_dump_status(drive, "command error", stat);
> do_end_request = 1;
> } else if (sense_key == MEDIUM_ERROR) {
> /*
> * No point in re-trying a zillion times on a bad
> * sector. If we got here the error is not correctable.
> */
> - ide_dump_status(drive, "media error (bad sector)",
> - stat);
> + if ((rq->cmd_flags & REQ_QUIET) == 0)
> + ide_dump_status(drive, "media error "
> + "(bad sector)", stat);
> do_end_request = 1;
> } else if (sense_key == BLANK_CHECK) {
> /* disk appears blank ?? */
> - ide_dump_status(drive, "media error (blank)", stat);
> + if ((rq->cmd_flags & REQ_QUIET) == 0)
> + ide_dump_status(drive, "media error (blank)",
> + stat);
> do_end_request = 1;
> } else if ((err & ~ATA_ABORTED) != 0) {
> /* go to the default handler for other errors */


Let's cache the REQ_QUIET value instead, for slightly better readability:

---
From: Borislav Petkov <[email protected]>
Date: Sun, 5 Apr 2009 06:40:50 +0200
Subject: [PATCH] ide-cd: respect REQ_QUIET for fs requests in cdrom_decode_status()

There should be no functional change resulting from this patch.

Suggested-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-cd.c | 18 ++++++++++++------
1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index a4afd90..6cf2916 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -276,6 +276,7 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
ide_hwif_t *hwif = drive->hwif;
struct request *rq = hwif->rq;
int err, sense_key;
+ u8 quiet = rq->cmd_flags & REQ_QUIET;

/* get the IDE error register */
err = ide_read_error(drive);
@@ -318,7 +319,7 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
* drive doesn't have that capability.
* cdrom_log_sense() knows this!
*/
- } else if (!(rq->cmd_flags & REQ_QUIET)) {
+ } else if (!quiet) {
/* otherwise, print an error */
ide_dump_status(drive, "packet command error", stat);
}
@@ -346,7 +347,8 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
cdrom_saw_media_change(drive);

/* fail the request */
- printk(KERN_ERR PFX "%s: tray open\n",
+ if (!quiet)
+ printk(KERN_ERR PFX "%s: tray open\n",
drive->name);
do_end_request = 1;
} else {
@@ -394,19 +396,23 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
* No point in retrying after an illegal request or data
* protect error.
*/
- ide_dump_status(drive, "command error", stat);
+ if (!quiet)
+ ide_dump_status(drive, "command error", stat);
do_end_request = 1;
} else if (sense_key == MEDIUM_ERROR) {
/*
* No point in re-trying a zillion times on a bad
* sector. If we got here the error is not correctable.
*/
- ide_dump_status(drive, "media error (bad sector)",
- stat);
+ if (!quiet)
+ ide_dump_status(drive, "media error "
+ "(bad sector)", stat);
do_end_request = 1;
} else if (sense_key == BLANK_CHECK) {
/* disk appears blank ?? */
- ide_dump_status(drive, "media error (blank)", stat);
+ if (!quiet)
+ ide_dump_status(drive, "media error (blank)",
+ stat);
do_end_request = 1;
} else if ((err & ~ATA_ABORTED) != 0) {
/* go to the default handler for other errors */
--
1.6.2.1


--
Regards/Gruss,
Boris.

2009-04-05 06:09:31

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 3/5] ide-cd: convert cdrom_decode_status() to use switch statements

On Fri, Apr 03, 2009 at 09:58:10PM +0200, Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz <[email protected]>
> Subject: [PATCH] ide-cd: convert cdrom_decode_status() to use switch statements

Here's a rediffed version:
---
From: Borislav Petkov <[email protected]>
Date: Sun, 5 Apr 2009 08:07:17 +0200
Subject: [PATCH] ide-cd: convert cdrom_decode_status() to use switch statements

Convert cdrom_decode_status() to use switch statements in
preparation to unify handling of fs and pc requests.

While at it:
- remove superfluous comments and do minor CodingStyle fixups

There should be no functional changes caused by this patch.

Signed-off-by: Borislav Petkov <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ide/ide-cd.c | 57 ++++++++++++++++++++++++++++----------------------
1 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index d95a2f0..df01b91 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -304,15 +304,14 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
if (blk_pc_request(rq) && !rq->errors)
rq->errors = SAM_STAT_CHECK_CONDITION;

- /* check for tray open */
- if (sense_key == NOT_READY) {
+ switch (sense_key) {
+ case NOT_READY:
cdrom_saw_media_change(drive);
- } else if (sense_key == UNIT_ATTENTION) {
- /* check for media change */
+ break;
+ case UNIT_ATTENTION:
cdrom_saw_media_change(drive);
return 0;
- } else if (sense_key == ILLEGAL_REQUEST &&
- rq->cmd[0] == GPCMD_START_STOP_UNIT) {
+ case ILLEGAL_REQUEST:
/*
* Don't print error message for this condition--
* SFF8090i indicates that 5/24/00 is the correct
@@ -320,9 +319,13 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
* drive doesn't have that capability.
* cdrom_log_sense() knows this!
*/
- } else if (!quiet) {
- /* otherwise, print an error */
- ide_dump_status(drive, "packet command error", stat);
+ if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
+ break;
+ /* fall-through */
+ default:
+ if (!quiet)
+ ide_dump_status(drive, "packet command error",
+ stat);
}

rq->cmd_flags |= REQ_FAILED;
@@ -342,12 +345,11 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
if (blk_noretry_request(rq))
do_end_request = 1;

- if (sense_key == NOT_READY) {
- /* tray open */
+ switch (sense_key) {
+ case NOT_READY:
if (rq_data_dir(rq) == READ) {
cdrom_saw_media_change(drive);

- /* fail the request */
if (!quiet)
printk(KERN_ERR PFX "%s: tray open\n",
drive->name);
@@ -381,8 +383,8 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
return 1;
}
}
- } else if (sense_key == UNIT_ATTENTION) {
- /* media change */
+ break;
+ case UNIT_ATTENTION:
cdrom_saw_media_change(drive);

/*
@@ -391,8 +393,9 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
*/
if (++rq->errors > ERROR_MAX)
do_end_request = 1;
- } else if (sense_key == ILLEGAL_REQUEST ||
- sense_key == DATA_PROTECT) {
+ break;
+ case ILLEGAL_REQUEST:
+ case DATA_PROTECT:
/*
* No point in retrying after an illegal request or data
* protect error.
@@ -400,7 +403,8 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
if (!quiet)
ide_dump_status(drive, "command error", stat);
do_end_request = 1;
- } else if (sense_key == MEDIUM_ERROR) {
+ break;
+ case MEDIUM_ERROR:
/*
* No point in re-trying a zillion times on a bad
* sector. If we got here the error is not correctable.
@@ -409,19 +413,22 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
ide_dump_status(drive, "media error "
"(bad sector)", stat);
do_end_request = 1;
- } else if (sense_key == BLANK_CHECK) {
+ break;
+ case BLANK_CHECK:
/* disk appears blank ?? */
if (!quiet)
ide_dump_status(drive, "media error (blank)",
stat);
do_end_request = 1;
- } else if ((err & ~ATA_ABORTED) != 0) {
- /* go to the default handler for other errors */
- ide_error(drive, "cdrom_decode_status", stat);
- return 1;
- } else if ((++rq->errors > ERROR_MAX)) {
- /* we've racked up too many retries, abort */
- do_end_request = 1;
+ break;
+ default:
+ if (err & ~ATA_ABORTED) {
+ /* go to the default handler for other errors */
+ ide_error(drive, "cdrom_decode_status", stat);
+ return 1;
+ } else if (++rq->errors > ERROR_MAX)
+ /* we've racked up too many retries, abort */
+ do_end_request = 1;
}

/*
--
1.6.2.1

--
Regards/Gruss,
Boris.

2009-04-05 06:25:46

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 3/5] ide-cd: convert cdrom_decode_status() to use switch statements

On Sun, Apr 05, 2009 at 08:09:11AM +0200, Borislav Petkov wrote:
> On Fri, Apr 03, 2009 at 09:58:10PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > From: Bartlomiej Zolnierkiewicz <[email protected]>
> > Subject: [PATCH] ide-cd: convert cdrom_decode_status() to use switch statements
>
> Here's a rediffed version:

Sorry, I'd forgotten the ide_cd_breathe patch, so here's the correct one:

---
From: Borislav Petkov <[email protected]>
Date: Sun, 5 Apr 2009 08:20:47 +0200
Subject: [PATCH] ide-cd: convert cdrom_decode_status() to use switch statements

Convert cdrom_decode_status() to use switch statements in
preparation to unify handling of fs and pc requests.

While at it:
- remove superfluous comments and do minor CodingStyle fixups

There should be no functional changes caused by this patch.

Signed-off-by: Borislav Petkov <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ide/ide-cd.c | 57 ++++++++++++++++++++++++++++----------------------
1 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index e946f0e..43db330 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -340,15 +340,14 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
if (blk_pc_request(rq) && !rq->errors)
rq->errors = SAM_STAT_CHECK_CONDITION;

- /* check for tray open */
- if (sense_key == NOT_READY) {
+ switch (sense_key) {
+ case NOT_READY:
cdrom_saw_media_change(drive);
- } else if (sense_key == UNIT_ATTENTION) {
- /* check for media change */
+ break;
+ case UNIT_ATTENTION:
cdrom_saw_media_change(drive);
return 0;
- } else if (sense_key == ILLEGAL_REQUEST &&
- rq->cmd[0] == GPCMD_START_STOP_UNIT) {
+ case ILLEGAL_REQUEST:
/*
* Don't print error message for this condition--
* SFF8090i indicates that 5/24/00 is the correct
@@ -356,9 +355,13 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
* drive doesn't have that capability.
* cdrom_log_sense() knows this!
*/
- } else if (!quiet) {
- /* otherwise, print an error */
- ide_dump_status(drive, "packet command error", stat);
+ if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
+ break;
+ /* fall-through */
+ default:
+ if (!quiet)
+ ide_dump_status(drive, "packet command error",
+ stat);
}

rq->cmd_flags |= REQ_FAILED;
@@ -378,12 +381,11 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
if (blk_noretry_request(rq))
do_end_request = 1;

- if (sense_key == NOT_READY) {
- /* tray open */
+ switch (sense_key) {
+ case NOT_READY:
if (rq_data_dir(rq) == READ) {
cdrom_saw_media_change(drive);

- /* fail the request */
if (!quiet)
printk(KERN_ERR PFX "%s: tray open\n",
drive->name);
@@ -392,8 +394,8 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
return 1;
}
do_end_request = 1;
- } else if (sense_key == UNIT_ATTENTION) {
- /* media change */
+ break;
+ case UNIT_ATTENTION:
cdrom_saw_media_change(drive);

/*
@@ -402,8 +404,9 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
*/
if (++rq->errors > ERROR_MAX)
do_end_request = 1;
- } else if (sense_key == ILLEGAL_REQUEST ||
- sense_key == DATA_PROTECT) {
+ break;
+ case ILLEGAL_REQUEST:
+ case DATA_PROTECT:
/*
* No point in retrying after an illegal request or data
* protect error.
@@ -411,7 +414,8 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
if (!quiet)
ide_dump_status(drive, "command error", stat);
do_end_request = 1;
- } else if (sense_key == MEDIUM_ERROR) {
+ break;
+ case MEDIUM_ERROR:
/*
* No point in re-trying a zillion times on a bad
* sector. If we got here the error is not correctable.
@@ -420,19 +424,22 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
ide_dump_status(drive, "media error "
"(bad sector)", stat);
do_end_request = 1;
- } else if (sense_key == BLANK_CHECK) {
+ break;
+ case BLANK_CHECK:
/* disk appears blank ?? */
if (!quiet)
ide_dump_status(drive, "media error (blank)",
stat);
do_end_request = 1;
- } else if ((err & ~ATA_ABORTED) != 0) {
- /* go to the default handler for other errors */
- ide_error(drive, "cdrom_decode_status", stat);
- return 1;
- } else if ((++rq->errors > ERROR_MAX)) {
- /* we've racked up too many retries, abort */
- do_end_request = 1;
+ break;
+ default:
+ if (err & ~ATA_ABORTED) {
+ /* go to the default handler for other errors */
+ ide_error(drive, "cdrom_decode_status", stat);
+ return 1;
+ } else if (++rq->errors > ERROR_MAX)
+ /* we've racked up too many retries, abort */
+ do_end_request = 1;
}

/*
--
1.6.2.1


--
Regards/Gruss,
Boris.

2009-04-05 06:46:50

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 4/5] ide-cd: unify handling of fs and pc requests in cdrom_decode_status()

On Fri, Apr 03, 2009 at 09:58:17PM +0200, Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz <[email protected]>
> Subject: [PATCH] ide-cd: unify handling of fs and pc requests in cdrom_decode_status()

Rediffed:
---
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Fri, 3 Apr 2009 21:58:17 +0200
Subject: [PATCH] ide-cd: unify handling of fs and pc requests in cdrom_decode_status()

Unify handling of fs and pc requests in cdrom_decode_status().

While at it:
- remove unreachable code

The only change in functionality is that for pc requests more
detailed error message will be printed for following sense keys:
* ILLEGAL_REQUEST
* DATA_PROTECT
* MEDIUM_ERROR
* BLANK_CHECK

Signed-off-by: Borislav Petkov <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ide/ide-cd.c | 69 +++++++++++++++++--------------------------------
1 files changed, 24 insertions(+), 45 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 43db330..ffbaa6d 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -330,8 +330,8 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
*/
rq->cmd_flags |= REQ_FAILED;
return 2;
- } else if (blk_pc_request(rq) || rq->cmd_type == REQ_TYPE_ATA_PC) {
- /* All other functions, except for READ. */
+ } else {
+ int do_end_request = 0;

/*
* if we have an error, pass back CHECK_CONDITION as the
@@ -340,53 +340,16 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
if (blk_pc_request(rq) && !rq->errors)
rq->errors = SAM_STAT_CHECK_CONDITION;

- switch (sense_key) {
- case NOT_READY:
- cdrom_saw_media_change(drive);
- break;
- case UNIT_ATTENTION:
- cdrom_saw_media_change(drive);
- return 0;
- case ILLEGAL_REQUEST:
- /*
- * Don't print error message for this condition--
- * SFF8090i indicates that 5/24/00 is the correct
- * response to a request to close the tray if the
- * drive doesn't have that capability.
- * cdrom_log_sense() knows this!
- */
- if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
- break;
- /* fall-through */
- default:
- if (!quiet)
- ide_dump_status(drive, "packet command error",
- stat);
- }
-
- rq->cmd_flags |= REQ_FAILED;
-
- /*
- * instead of playing games with moving completions around,
- * remove failed request completely and end it when the
- * request sense has completed
- */
- goto end_request;
-
- } else if (blk_fs_request(rq)) {
- int do_end_request = 0;
-
- /* handle errors from READ and WRITE requests */
-
if (blk_noretry_request(rq))
do_end_request = 1;

switch (sense_key) {
case NOT_READY:
- if (rq_data_dir(rq) == READ) {
+ if (blk_fs_request(rq) == 0 ||
+ rq_data_dir(rq) == READ) {
cdrom_saw_media_change(drive);

- if (!quiet)
+ if (blk_fs_request(rq) && !quiet)
printk(KERN_ERR PFX "%s: tray open\n",
drive->name);
} else {
@@ -398,6 +361,8 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
case UNIT_ATTENTION:
cdrom_saw_media_change(drive);

+ if (blk_fs_request(rq) == 0)
+ return 0;
/*
* Arrange to retry the request but be sure to give up
* if we've retried too many times.
@@ -406,6 +371,16 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
do_end_request = 1;
break;
case ILLEGAL_REQUEST:
+ /*
+ * Don't print error message for this condition--
+ * SFF8090i indicates that 5/24/00 is the correct
+ * response to a request to close the tray if the
+ * drive doesn't have that capability.
+ * cdrom_log_sense() knows this!
+ */
+ if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
+ break;
+ /* fall-through */
case DATA_PROTECT:
/*
* No point in retrying after an illegal request or data
@@ -433,6 +408,8 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
do_end_request = 1;
break;
default:
+ if (blk_fs_request(rq) == 0)
+ break;
if (err & ~ATA_ABORTED) {
/* go to the default handler for other errors */
ide_error(drive, "cdrom_decode_status", stat);
@@ -442,6 +419,11 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
do_end_request = 1;
}

+ if (blk_fs_request(rq) == 0) {
+ rq->cmd_flags |= REQ_FAILED;
+ do_end_request = 1;
+ }
+
/*
* End a request through request sense analysis when we have
* sense data. We need this in order to perform end of media
@@ -457,9 +439,6 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
if (stat & ATA_ERR)
cdrom_queue_request_sense(drive, NULL, NULL);
return 1;
- } else {
- blk_dump_rq_flags(rq, PFX "bad rq");
- return 2;
}

end_request:
--
1.6.2.1

--
Regards/Gruss,
Boris.

2009-04-05 08:02:35

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 5/5] ide-cd: fix intendation in cdrom_decode_status()

On Fri, Apr 03, 2009 at 09:58:24PM +0200, Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz <[email protected]>
> Subject: [PATCH] ide-cd: fix intendation in cdrom_decode_status()a

Rediffed & lightly tested the whole series:

---
From: Borislav Petkov <[email protected]>
Date: Sun, 5 Apr 2009 09:59:42 +0200
Subject: [PATCH] ide-cd: fix intendation in cdrom_decode_status()

Fix intendation in cdrom_decode_status(), no real code changes.

While at it:
- beautify comments

There should be no functional changes caused by this patch.

Signed-off-by: Borislav Petkov <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ide/ide-cd.c | 186 ++++++++++++++++++++++++--------------------------
1 files changed, 89 insertions(+), 97 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index ffbaa6d..2989aa8 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -311,7 +311,7 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
{
ide_hwif_t *hwif = drive->hwif;
struct request *rq = hwif->rq;
- int err, sense_key;
+ int err, sense_key, do_end_request = 0;
u8 quiet = rq->cmd_flags & REQ_QUIET;

/* get the IDE error register */
@@ -330,116 +330,108 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
*/
rq->cmd_flags |= REQ_FAILED;
return 2;
- } else {
- int do_end_request = 0;
-
- /*
- * if we have an error, pass back CHECK_CONDITION as the
- * scsi status byte
- */
- if (blk_pc_request(rq) && !rq->errors)
- rq->errors = SAM_STAT_CHECK_CONDITION;
+ }

- if (blk_noretry_request(rq))
- do_end_request = 1;
+ /* if we have an error, pass CHECK_CONDITION as the SCSI status byte */
+ if (blk_pc_request(rq) && !rq->errors)
+ rq->errors = SAM_STAT_CHECK_CONDITION;

- switch (sense_key) {
- case NOT_READY:
- if (blk_fs_request(rq) == 0 ||
- rq_data_dir(rq) == READ) {
- cdrom_saw_media_change(drive);
+ if (blk_noretry_request(rq))
+ do_end_request = 1;

- if (blk_fs_request(rq) && !quiet)
- printk(KERN_ERR PFX "%s: tray open\n",
- drive->name);
- } else {
- if (ide_cd_breathe(drive, rq))
- return 1;
- }
- do_end_request = 1;
- break;
- case UNIT_ATTENTION:
+ switch (sense_key) {
+ case NOT_READY:
+ if (blk_fs_request(rq) == 0 || rq_data_dir(rq) == READ) {
cdrom_saw_media_change(drive);

- if (blk_fs_request(rq) == 0)
- return 0;
- /*
- * Arrange to retry the request but be sure to give up
- * if we've retried too many times.
- */
- if (++rq->errors > ERROR_MAX)
- do_end_request = 1;
- break;
- case ILLEGAL_REQUEST:
- /*
- * Don't print error message for this condition--
- * SFF8090i indicates that 5/24/00 is the correct
- * response to a request to close the tray if the
- * drive doesn't have that capability.
- * cdrom_log_sense() knows this!
- */
- if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
- break;
- /* fall-through */
- case DATA_PROTECT:
- /*
- * No point in retrying after an illegal request or data
- * protect error.
- */
- if (!quiet)
- ide_dump_status(drive, "command error", stat);
- do_end_request = 1;
- break;
- case MEDIUM_ERROR:
- /*
- * No point in re-trying a zillion times on a bad
- * sector. If we got here the error is not correctable.
- */
- if (!quiet)
- ide_dump_status(drive, "media error "
- "(bad sector)", stat);
- do_end_request = 1;
- break;
- case BLANK_CHECK:
- /* disk appears blank ?? */
- if (!quiet)
- ide_dump_status(drive, "media error (blank)",
- stat);
- do_end_request = 1;
- break;
- default:
- if (blk_fs_request(rq) == 0)
- break;
- if (err & ~ATA_ABORTED) {
- /* go to the default handler for other errors */
- ide_error(drive, "cdrom_decode_status", stat);
+ if (blk_fs_request(rq) && !quiet)
+ printk(KERN_ERR PFX "%s: tray open\n",
+ drive->name);
+ } else {
+ if (ide_cd_breathe(drive, rq))
return 1;
- } else if (++rq->errors > ERROR_MAX)
- /* we've racked up too many retries, abort */
- do_end_request = 1;
}
+ do_end_request = 1;
+ break;
+ case UNIT_ATTENTION:
+ cdrom_saw_media_change(drive);

- if (blk_fs_request(rq) == 0) {
- rq->cmd_flags |= REQ_FAILED;
- do_end_request = 1;
- }
+ if (blk_fs_request(rq) == 0)
+ return 0;

/*
- * End a request through request sense analysis when we have
- * sense data. We need this in order to perform end of media
- * processing.
+ * Arrange to retry the request but be sure to give up if we've
+ * retried too many times.
*/
- if (do_end_request)
- goto end_request;
-
+ if (++rq->errors > ERROR_MAX)
+ do_end_request = 1;
+ break;
+ case ILLEGAL_REQUEST:
+ /*
+ * Don't print error message for this condition -- SFF8090i
+ * indicates that 5/24/00 is the correct response to a request
+ * to close the tray if the drive doesn't have that capability.
+ *
+ * cdrom_log_sense() knows this!
+ */
+ if (rq->cmd[0] == GPCMD_START_STOP_UNIT)
+ break;
+ /* fall-through */
+ case DATA_PROTECT:
/*
- * If we got a CHECK_CONDITION status, queue
- * a request sense command.
+ * No point in retrying after an illegal request or data
+ * protect error.
*/
- if (stat & ATA_ERR)
- cdrom_queue_request_sense(drive, NULL, NULL);
- return 1;
+ if (!quiet)
+ ide_dump_status(drive, "command error", stat);
+ do_end_request = 1;
+ break;
+ case MEDIUM_ERROR:
+ /*
+ * No point in re-trying a zillion times on a bad sector.
+ * If we got here the error is not correctable.
+ */
+ if (!quiet)
+ ide_dump_status(drive, "media error "
+ "(bad sector)", stat);
+ do_end_request = 1;
+ break;
+ case BLANK_CHECK:
+ /* disk appears blank? */
+ if (!quiet)
+ ide_dump_status(drive, "media error (blank)",
+ stat);
+ do_end_request = 1;
+ break;
+ default:
+ if (blk_fs_request(rq) == 0)
+ break;
+ if (err & ~ATA_ABORTED) {
+ /* go to the default handler for other errors */
+ ide_error(drive, "cdrom_decode_status", stat);
+ return 1;
+ } else if (++rq->errors > ERROR_MAX)
+ /* we've racked up too many retries, abort */
+ do_end_request = 1;
+
+
+ }
+
+ if (blk_fs_request(rq) == 0) {
+ rq->cmd_flags |= REQ_FAILED;
+ do_end_request = 1;
}
+ /*
+ * End a request through request sense analysis when we have sense data.
+ * We need this in order to perform end of media processing.
+ */
+ if (do_end_request)
+ goto end_request;
+
+ /* if we got a CHECK_CONDITION status, queue a request sense command */
+ if (stat & ATA_ERR)
+ cdrom_queue_request_sense(drive, NULL, NULL);
+ return 1;

end_request:
if (stat & ATA_ERR) {
--
1.6.2.1


--
Regards/Gruss,
Boris.

Subject: Re: [PATCH 1/5] ide-cd: respect REQ_QUIET for fs requests in cdrom_decode_status()

On Sunday 05 April 2009, Borislav Petkov wrote:
> On Fri, Apr 03, 2009 at 09:57:57PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > From: Bartlomiej Zolnierkiewicz <[email protected]>
> > Subject: [PATCH] ide-cd: respect REQ_QUIET for fs requests in cdrom_decode_status()
> >
> > Cc: Borislav Petkov <[email protected]>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> > ---
> > drivers/ide/ide-cd.c | 15 ++++++++++-----
> > 1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > Index: b/drivers/ide/ide-cd.c
> > ===================================================================
> > --- a/drivers/ide/ide-cd.c
> > +++ b/drivers/ide/ide-cd.c
> > @@ -382,7 +382,8 @@ static int cdrom_decode_status(ide_drive
> > cdrom_saw_media_change(drive);
> >
> > /* fail the request */
> > - printk(KERN_ERR PFX "%s: tray open\n",
> > + if ((rq->cmd_flags & REQ_QUIET) == 0)
> > + printk(KERN_ERR PFX "%s: tray open\n",
> > drive->name);
> > } else {
> > if (ide_cd_breathe(drive, rq))
> > @@ -405,19 +406,23 @@ static int cdrom_decode_status(ide_drive
> > * No point in retrying after an illegal request or data
> > * protect error.
> > */
> > - ide_dump_status(drive, "command error", stat);
> > + if ((rq->cmd_flags & REQ_QUIET) == 0)
> > + ide_dump_status(drive, "command error", stat);
> > do_end_request = 1;
> > } else if (sense_key == MEDIUM_ERROR) {
> > /*
> > * No point in re-trying a zillion times on a bad
> > * sector. If we got here the error is not correctable.
> > */
> > - ide_dump_status(drive, "media error (bad sector)",
> > - stat);
> > + if ((rq->cmd_flags & REQ_QUIET) == 0)
> > + ide_dump_status(drive, "media error "
> > + "(bad sector)", stat);
> > do_end_request = 1;
> > } else if (sense_key == BLANK_CHECK) {
> > /* disk appears blank ?? */
> > - ide_dump_status(drive, "media error (blank)", stat);
> > + if ((rq->cmd_flags & REQ_QUIET) == 0)
> > + ide_dump_status(drive, "media error (blank)",
> > + stat);
> > do_end_request = 1;
> > } else if ((err & ~ATA_ABORTED) != 0) {
> > /* go to the default handler for other errors */
>
>
> Let's cache the REQ_QUIET value instead, for slightly better readability:
>
> ---
> From: Borislav Petkov <[email protected]>
> Date: Sun, 5 Apr 2009 06:40:50 +0200
> Subject: [PATCH] ide-cd: respect REQ_QUIET for fs requests in cdrom_decode_status()
>
> There should be no functional change resulting from this patch.
>
> Suggested-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>

Yes, this version is better, applied.

I also applied patches 2-5 (correcting "From:" line in patches 3 & 5,
re-adding your attribution to 3-5 and fixing "S-o-b:" order in 3-5).

[ S-o-B order should be:

- me
- you
- me (this one added now)

to show the correct patch history -- the fact that I'm also
the final committer doesn't matter. ]

Patch #5 had a small whitespace mismatch introduced while re-diffing
(also fixed now):

@@ -412,13 +413,14 @@
} else if (++rq->errors > ERROR_MAX)
/* we've racked up too many retries, abort */
do_end_request = 1;
+
+
}

if (blk_fs_request(rq) == 0) {
rq->cmd_flags |= REQ_FAILED;
do_end_request = 1;
}
-
/*
* End a request through request sense analysis when we have sense data.
* We need this in order to perform end of media processing.

Thanks,
Bart