2008-06-12 06:41:31

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 00/18] misc generic ide stuff

Hi Bart,

here are some generic ide conversion patches. The first 12 are what i thought
you suggested :) concerning prepping the ide-cd code for the generic layer. The
remaining 6 are what i've done so far wrt removing ide_atapi_pc from the ide
drivers. It is obvious that this is not trivial and I basically tiptoe around the
landmines in the IRQ handler and request issue paths :). This raises also
several questions:

1. ide-cd uses the cdrom_info struct for e.g. dma status notification, the other
ide drivers use it per packet command in pc->flags. Well the last are kinda too
much to carry for _each_ command and i'm thinking maybe put all that in the
ide_drive_t and make it generic enough to fit all drivers. This way
pc->callback() can also be harboured there. One concern might be when a flag is
strictly per packet command which could be put somewhere else (maybe in the
struct request since it already has members when it is used as a packet
command).

2. Can all that pc->xferred, pc->b_count, pc->errors and pc->cur_pos accounting
be safely mapped to a rq? I see some discrepancies like is pc->buf_size ==
rq->data_len, what about pc->req_xfer? I'll have a more detailed look at those
when i have more spare time later.

3. (I'm sure there are more :))


2008-06-12 06:41:53

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 01/18] ide-cd: remove wait-for-idle-controller bit in cdrom_start_packet_command

This is done in the request issue path anyway

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index d998471..92a8a36 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -517,14 +517,9 @@ static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive,
int xferlen,
ide_handler_t *handler)
{
- ide_startstop_t startstop;
struct cdrom_info *info = drive->driver_data;
ide_hwif_t *hwif = drive->hwif;

- /* wait for the controller to be idle */
- if (ide_wait_stat(&startstop, drive, 0, BUSY_STAT, WAIT_READY))
- return startstop;
-
/* FIXME: for Virtual DMA we must check harder */
if (info->dma)
info->dma = !hwif->dma_ops->dma_setup(drive);
--
1.5.5.1

2008-06-12 06:42:15

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 04/18] ide-cd: cdrom_decode_status: factor out fs request error handling code

... into cdrom_handle_failed_fs_req(). There's a slight change in internal
functionality in the case when we have sense NOT_READY and the request is WRITE: the
original function cdrom_decode_status returned 1 in this case, which means "request
was ended." We accomplish the same by returning 0 instead, which falls through in the
if..else block and the surrounding cdrom_decode_status() returns 1 at the end instead
of jumping to the end_request label as is in all the other cases, so no change
in obvious functionality.

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-cd.c | 206 +++++++++++++++++++++++++++-----------------------
1 files changed, 111 insertions(+), 95 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 16c4ce9..e63eea2 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -314,12 +314,115 @@ static int cdrom_handle_failed_pc_req(ide_drive_t *drive, struct request *rq,
return 1;
}

-
/*
- * Returns:
- * 0: if the request should be continued.
- * 1: if the request was ended.
+ * Handle errors from READ and WRITE requests. Do a request sense analysis when
+ * we have sense data. We need this in order to perform end of media processing.
*/
+static int cdrom_handle_failed_fs_req(ide_drive_t *drive, struct request *rq,
+ int sense_key, int stat)
+{
+ int err = 0;
+
+ if (blk_noretry_request(rq))
+ err = 1;
+
+ switch (sense_key) {
+ case NOT_READY:
+ /* tray open */
+ if (rq_data_dir(rq) == READ) {
+ cdrom_saw_media_change(drive);
+
+ /* fail the request */
+ printk(KERN_ERR "%s: tray open\n", drive->name);
+ err = 1;
+ } else {
+ struct cdrom_info *info = drive->driver_data;
+
+ /*
+ * Allow the drive 5 seconds to recover, some devices
+ * will return this error while flushing data from
+ * cache.
+ */
+ if (!rq->errors)
+ info->write_timeout = jiffies +
+ ATAPI_WAIT_WRITE_BUSY;
+ rq->errors = 1;
+
+ if (time_after(jiffies, info->write_timeout))
+ err = 1;
+ else {
+ unsigned long flags;
+
+ /*
+ * take a breather relying on the unplug timer
+ * to kick us again
+ */
+ spin_lock_irqsave(&ide_lock, flags);
+ blk_plug_device(drive->queue);
+ spin_unlock_irqrestore(&ide_lock, flags);
+ err = 0;
+ }
+ }
+ break;
+
+ case UNIT_ATTENTION:
+ /* media change */
+ cdrom_saw_media_change(drive);
+
+ /*
+ * Arrange to retry the request but be sure to give up if we've
+ * retried too many times.
+ */
+ if (++rq->errors > ERROR_MAX)
+ err = 1;
+ break;
+
+ case ILLEGAL_REQUEST:
+ case DATA_PROTECT:
+
+ /*
+ * No point in retrying after an illegal request or data
+ * protect error.
+ */
+ ide_dump_status_no_sense(drive, "command error", stat);
+ err = 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.
+ */
+ ide_dump_status_no_sense(drive, "media error (bad sector)",
+ stat);
+ err = 1;
+ break;
+
+ case BLANK_CHECK:
+
+ /* disk appears blank ?? */
+ ide_dump_status_no_sense(drive, "media error (blank)", stat);
+ err = 1;
+ break;
+
+ default:
+ break;
+ }
+
+ if ((err & ~ABRT_ERR) != 0) {
+ /* go to the default handler for other errors */
+ ide_error(drive, "cdrom_handle_failed_fs_req", stat);
+ err = 1;
+ } else if ((++rq->errors > ERROR_MAX))
+
+ /* we've racked up too many retries, abort */
+ err = 1;
+
+ return err;
+}
+
+/* Returns 0 if the request should be continued, 1 otherwise. */
static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret)
{
struct request *rq = HWGROUP(drive)->rq;
@@ -363,104 +466,17 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret)
return 0;

} 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;
-
- if (sense_key == NOT_READY) {
- /* tray open */
- if (rq_data_dir(rq) == READ) {
- cdrom_saw_media_change(drive);

- /* fail the request */
- printk(KERN_ERR "%s: tray open\n", drive->name);
- do_end_request = 1;
- } else {
- struct cdrom_info *info = drive->driver_data;
-
- /*
- * Allow the drive 5 seconds to recover, some
- * devices will return this error while flushing
- * data from cache.
- */
- if (!rq->errors)
- info->write_timeout = jiffies +
- ATAPI_WAIT_WRITE_BUSY;
- rq->errors = 1;
- if (time_after(jiffies, info->write_timeout))
- do_end_request = 1;
- else {
- unsigned long flags;
-
- /*
- * take a breather relying on the unplug
- * timer to kick us again
- */
- spin_lock_irqsave(&ide_lock, flags);
- blk_plug_device(drive->queue);
- spin_unlock_irqrestore(&ide_lock,
- flags);
- return 1;
- }
- }
- } else if (sense_key == UNIT_ATTENTION) {
- /* media change */
- cdrom_saw_media_change(drive);
-
- /*
- * 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;
- } else if (sense_key == ILLEGAL_REQUEST ||
- sense_key == DATA_PROTECT) {
- /*
- * No point in retrying after an illegal request or data
- * protect error.
- */
- ide_dump_status_no_sense(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_no_sense(drive,
- "media error (bad sector)",
- stat);
- do_end_request = 1;
- } else if (sense_key == BLANK_CHECK) {
- /* disk appears blank ?? */
- ide_dump_status_no_sense(drive, "media error (blank)",
- stat);
- do_end_request = 1;
- } else if ((err & ~ABRT_ERR) != 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;
- }
-
- /*
- * 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)
+ if (cdrom_handle_failed_fs_req(drive, rq, sense_key, stat))
goto end_request;

/*
- * If we got a CHECK_CONDITION status, queue
- * a request sense command.
+ * If we got a CHECK_CONDITION status, queue a request sense
+ * command.
*/
if (stat & ERR_STAT)
cdrom_queue_request_sense(drive, NULL, NULL);
+
} else {
blk_dump_rq_flags(rq, "ide-cd: bad rq");
cdrom_end_request(drive, 0);
--
1.5.5.1

2008-06-12 06:42:31

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 06/18] ide-cd: cdrom_start_seek: remove unused argument block

There should be no functionality change resulting from this patch.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 21c4510..40dab2b 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -778,7 +778,7 @@ static ide_startstop_t cdrom_start_seek_continuation(ide_drive_t *drive)
return cdrom_transfer_packet_command(drive, rq, &cdrom_seek_intr);
}

-static ide_startstop_t cdrom_start_seek(ide_drive_t *drive, unsigned int block)
+static ide_startstop_t cdrom_start_seek(ide_drive_t *drive)
{
struct cdrom_info *info = drive->driver_data;

@@ -1237,7 +1237,7 @@ static ide_startstop_t ide_do_rw_cdrom(ide_drive_t *drive, struct request *rq,
IDE_LARGE_SEEK(info->last_block, block,
IDECD_SEEK_THRESHOLD) &&
drive->dsc_overlap)
- action = cdrom_start_seek(drive, block);
+ action = cdrom_start_seek(drive);
else
action = cdrom_start_rw(drive, rq);
info->last_block = block;
--
1.5.5.1

2008-06-12 06:43:13

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 07/18] ide-cd: mv ide_do_rw_cdrom ide_cd_do_request

There should be no functionality change resulting from this patch.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 40dab2b..e9d9363 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1211,7 +1211,7 @@ static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq)
/*
* cdrom driver request routine.
*/
-static ide_startstop_t ide_do_rw_cdrom(ide_drive_t *drive, struct request *rq,
+static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
sector_t block)
{
ide_startstop_t action;
@@ -1949,7 +1949,7 @@ static ide_driver_t ide_cdrom_driver = {
.version = IDECD_VERSION,
.media = ide_cdrom,
.supports_dsc_overlap = 1,
- .do_request = ide_do_rw_cdrom,
+ .do_request = ide_cd_do_request,
.end_request = ide_end_request,
.error = __ide_error,
.abort = __ide_abort,
--
1.5.5.1

2008-06-12 06:42:50

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 03/18] ide-cd: cdrom_decode_status: factor out block pc error handling code

... into cdrom_handle_failed_pc_req().

There should be no functionality change resulting from this patch.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 2f0c9d4..16c4ce9 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -273,6 +273,48 @@ static void ide_dump_status_no_sense(ide_drive_t *drive, const char *msg, u8 st)
ide_dump_status(drive, msg, st);
}

+/* All other functions, except for READ. */
+static int cdrom_handle_failed_pc_req(ide_drive_t *drive, struct request *rq,
+ int sense_key, int stat)
+{
+ /*
+ * 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;
+
+ /* check for tray open */
+ if (sense_key == NOT_READY) {
+ cdrom_saw_media_change(drive);
+ } else if (sense_key == UNIT_ATTENTION) {
+ /* check for media change */
+ cdrom_saw_media_change(drive);
+ return 0;
+ } else if (sense_key == ILLEGAL_REQUEST &&
+ rq->cmd[0] == GPCMD_START_STOP_UNIT) {
+
+ /*
+ * 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!
+ */
+ } else if (!(rq->cmd_flags & REQ_QUIET)) {
+ /* otherwise, print an error */
+ 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.
+ */
+ return 1;
+}
+
+
/*
* Returns:
* 0: if the request should be continued.
@@ -314,44 +356,11 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret)
return 1;

} else if (blk_pc_request(rq) || rq->cmd_type == REQ_TYPE_ATA_PC) {
- /* All other functions, except for READ. */

- /*
- * 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;
-
- /* check for tray open */
- if (sense_key == NOT_READY) {
- cdrom_saw_media_change(drive);
- } else if (sense_key == UNIT_ATTENTION) {
- /* check for media change */
- cdrom_saw_media_change(drive);
+ if (cdrom_handle_failed_pc_req(drive, rq, sense_key, stat))
+ goto end_request;
+ else
return 0;
- } else if (sense_key == ILLEGAL_REQUEST &&
- rq->cmd[0] == GPCMD_START_STOP_UNIT) {
- /*
- * 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!
- */
- } else if (!(rq->cmd_flags & REQ_QUIET)) {
- /* otherwise, print an error */
- 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;
--
1.5.5.1

2008-06-12 06:43:33

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 05/18] ide-cd: ide_do_rw_cdrom: add the catch-all bad request case to the if-else block

There should be no functionality change resulting from this patch.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index e63eea2..21c4510 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1249,11 +1249,11 @@ static ide_startstop_t ide_do_rw_cdrom(ide_drive_t *drive, struct request *rq,
/* right now this can only be a reset... */
cdrom_end_request(drive, 1);
return ide_stopped;
+ } else {
+ blk_dump_rq_flags(rq, "ide-cd bad flags");
+ cdrom_end_request(drive, 0);
+ return ide_stopped;
}
-
- blk_dump_rq_flags(rq, "ide-cd bad flags");
- cdrom_end_request(drive, 0);
- return ide_stopped;
}


--
1.5.5.1

2008-06-12 06:44:14

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 15/18] ide-floppy: zero out the whole struct ide_atapi_pc on init

This is a precaution just to make sure a new pc is clean when allocd. There
should be functional change introduced by this patch.

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-floppy.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 615d3a3..8ce7513 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -355,9 +355,8 @@ static void idefloppy_init_pc(struct ide_atapi_pc *pc, unsigned char *cmd)
if (cmd)
memset(cmd, 0, 12);

- pc->retries = 0;
- pc->flags = 0;
- pc->req_xfer = 0;
+ memset(pc, 0, sizeof(*pc));
+
pc->buf = pc->pc_buf;
pc->buf_size = IDEFLOPPY_PC_BUFFER_SIZE;
pc->callback = ide_floppy_callback;
--
1.5.5.1

2008-06-12 06:43:49

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 16/18] ide-floppy: pass rq instead of pc to idefloppy_issue_pc

The pc is still embedded in rq->buffer.

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-floppy.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 8ce7513..1ec4951 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -464,12 +464,12 @@ static void ide_floppy_report_error(idefloppy_floppy_t *floppy,
}

static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
- struct ide_atapi_pc *pc)
+ struct request *rq)
{
idefloppy_floppy_t *floppy = drive->driver_data;
+ struct ide_atapi_pc *pc = (struct ide_atapi_pc *) rq->buffer;

- if (floppy->failed_pc == NULL &&
- pc->rq->cmd[0] != GPCMD_REQUEST_SENSE)
+ if (floppy->failed_pc == NULL && rq->cmd[0] != GPCMD_REQUEST_SENSE)
floppy->failed_pc = pc;
/* Set the current packet command */
floppy->pc = pc;
@@ -626,6 +626,7 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
}
pc = idefloppy_next_pc_storage(drive);
idefloppy_create_rw_cmd(floppy, pc, rq, block);
+ rq->buffer = (char *) pc;
} else if (blk_pc_request(rq))
pc = (struct ide_atapi_pc *) rq->buffer;
else {
@@ -643,7 +644,7 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,

pc->rq = rq;

- return idefloppy_issue_pc(drive, pc);
+ return idefloppy_issue_pc(drive, rq);
}

/*
--
1.5.5.1

2008-06-12 06:44:35

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 18/18] ide: use flags in IRQ handler

Pass pc->flags to the IRQ handler from the drivers (ide-tape/floppy/scsi) thus
making it more pc-unaware.

There should be no functionality change resulting from this.

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-atapi.c | 25 +++++++++++++------------
drivers/ide/ide-floppy.c | 3 ++-
drivers/ide/ide-tape.c | 3 ++-
drivers/scsi/ide-scsi.c | 2 +-
include/linux/ide.h | 2 +-
5 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 3a6ae79..dfe1c55 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -19,7 +19,8 @@ ide_startstop_t ide_pc_intr(ide_drive_t *drive, struct ide_atapi_pc *pc,
ide_handler_t *handler, unsigned int timeout, ide_expiry_t *expiry,
void (*update_buffers)(ide_drive_t *, struct ide_atapi_pc *),
void (*retry_pc)(ide_drive_t *), void (*dsc_handle)(ide_drive_t *),
- void (*io_buffers)(ide_drive_t *, struct ide_atapi_pc *, unsigned, int))
+ void (*io_buffers)(ide_drive_t *, struct ide_atapi_pc *, unsigned, int),
+ unsigned long *flags)
{
ide_hwif_t *hwif = drive->hwif;
xfer_func_t *xferfunc;
@@ -35,7 +36,7 @@ ide_startstop_t ide_pc_intr(ide_drive_t *drive, struct ide_atapi_pc *pc,

debug_log("Enter %s - interrupt handler\n", __func__);

- if (pc->flags & PC_FLAG_TIMEDOUT) {
+ if (*flags & PC_FLAG_TIMEDOUT) {
pc->callback(drive);
return ide_stopped;
}
@@ -43,14 +44,14 @@ ide_startstop_t ide_pc_intr(ide_drive_t *drive, struct ide_atapi_pc *pc,
/* Clear the interrupt */
stat = ide_read_status(drive);

- if (pc->flags & PC_FLAG_DMA_IN_PROGRESS) {
+ if (*flags & PC_FLAG_DMA_IN_PROGRESS) {
if (hwif->dma_ops->dma_end(drive) ||
(drive->media == ide_tape && !scsi && (stat & ERR_STAT))) {
if (drive->media == ide_floppy && !scsi)
printk(KERN_ERR "%s: DMA %s error\n",
drive->name, rq_data_dir(pc->rq)
? "write" : "read");
- pc->flags |= PC_FLAG_DMA_ERROR;
+ *flags |= PC_FLAG_DMA_ERROR;
} else {
pc->xferred = pc->req_xfer;
if (update_buffers)
@@ -64,14 +65,14 @@ ide_startstop_t ide_pc_intr(ide_drive_t *drive, struct ide_atapi_pc *pc,
debug_log("Packet command completed, %d bytes transferred\n",
pc->xferred);

- pc->flags &= ~PC_FLAG_DMA_IN_PROGRESS;
+ *flags &= ~PC_FLAG_DMA_IN_PROGRESS;

local_irq_enable_in_hardirq();

if (drive->media == ide_tape && !scsi &&
(stat & ERR_STAT) && cmd[0] == REQUEST_SENSE)
stat &= ~ERR_STAT;
- if ((stat & ERR_STAT) || (pc->flags & PC_FLAG_DMA_ERROR)) {
+ if ((stat & ERR_STAT) || (*flags & PC_FLAG_DMA_ERROR)) {
/* Error detected */
debug_log("%s: I/O error\n", drive->name);

@@ -96,7 +97,7 @@ ide_startstop_t ide_pc_intr(ide_drive_t *drive, struct ide_atapi_pc *pc,
}
cmd_finished:
pc->error = 0;
- if ((pc->flags & PC_FLAG_WAIT_FOR_DSC) &&
+ if ((*flags & PC_FLAG_WAIT_FOR_DSC) &&
(stat & SEEK_STAT) == 0) {
dsc_handle(drive);
return ide_stopped;
@@ -106,8 +107,8 @@ cmd_finished:
return ide_stopped;
}

- if (pc->flags & PC_FLAG_DMA_IN_PROGRESS) {
- pc->flags &= ~PC_FLAG_DMA_IN_PROGRESS;
+ if (*flags & PC_FLAG_DMA_IN_PROGRESS) {
+ *flags &= ~PC_FLAG_DMA_IN_PROGRESS;
printk(KERN_ERR "%s: The device wants to issue more interrupts "
"in DMA mode\n", drive->name);
ide_dma_off(drive);
@@ -123,7 +124,7 @@ cmd_finished:
printk(KERN_ERR "%s: CoD != 0 in %s\n", drive->name, __func__);
return ide_do_reset(drive);
}
- if (((ireason & IO) == IO) == !!(pc->flags & PC_FLAG_WRITING)) {
+ if (((ireason & IO) == IO) == !!(*flags & PC_FLAG_WRITING)) {
/* Hopefully, we will never get here */
printk(KERN_ERR "%s: We wanted to %s, but the device wants us "
"to %s!\n", drive->name,
@@ -131,7 +132,7 @@ cmd_finished:
(ireason & IO) ? "Read" : "Write");
return ide_do_reset(drive);
}
- if (!(pc->flags & PC_FLAG_WRITING)) {
+ if (!(*flags & PC_FLAG_WRITING)) {
/* Reading - Check that we have enough space */
temp = pc->xferred + bcount;
if (temp > pc->req_xfer) {
@@ -172,7 +173,7 @@ cmd_finished:
if ((drive->media == ide_floppy && !scsi && !pc->buf) ||
(drive->media == ide_tape && !scsi && pc->bh) ||
(scsi && pc->sg))
- io_buffers(drive, pc, bcount, !!(pc->flags & PC_FLAG_WRITING));
+ io_buffers(drive, pc, bcount, !!(*flags & PC_FLAG_WRITING));
else
xferfunc(drive, NULL, pc->cur_pos, bcount);

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 8bdef60..d3fcb51 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -395,7 +395,8 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)

return ide_pc_intr(drive, floppy->pc, idefloppy_pc_intr,
IDEFLOPPY_WAIT_CMD, NULL, idefloppy_update_buffers,
- idefloppy_retry_pc, NULL, ide_floppy_io_buffers);
+ idefloppy_retry_pc, NULL, ide_floppy_io_buffers,
+ &floppy->pc->flags);
}

/*
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 6e1233b..0c24426 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -802,7 +802,8 @@ static ide_startstop_t idetape_pc_intr(ide_drive_t *drive)

return ide_pc_intr(drive, tape->pc, idetape_pc_intr, IDETAPE_WAIT_CMD,
NULL, idetape_update_buffers, idetape_retry_pc,
- ide_tape_handle_dsc, ide_tape_io_buffers);
+ ide_tape_handle_dsc, ide_tape_io_buffers,
+ &tape->pc->flags);
}

/*
diff --git a/drivers/scsi/ide-scsi.c b/drivers/scsi/ide-scsi.c
index 683bce3..ef9e693 100644
--- a/drivers/scsi/ide-scsi.c
+++ b/drivers/scsi/ide-scsi.c
@@ -360,7 +360,7 @@ static ide_startstop_t idescsi_pc_intr (ide_drive_t *drive)

return ide_pc_intr(drive, pc, idescsi_pc_intr, get_timeout(pc),
idescsi_expiry, NULL, NULL, NULL,
- ide_scsi_io_buffers);
+ ide_scsi_io_buffers, &pc->flags);
}

static ide_startstop_t idescsi_transfer_pc(ide_drive_t *drive)
diff --git a/include/linux/ide.h b/include/linux/ide.h
index df47a5e..09d95c7 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -946,7 +946,7 @@ ide_startstop_t ide_pc_intr(ide_drive_t *drive, struct ide_atapi_pc *pc,
void (*update_buffers)(ide_drive_t *, struct ide_atapi_pc *),
void (*retry_pc)(ide_drive_t *), void (*dsc_handle)(ide_drive_t *),
void (*io_buffers)(ide_drive_t *, struct ide_atapi_pc *, unsigned int,
- int));
+ int), unsigned long *);
ide_startstop_t ide_transfer_pc(ide_drive_t *, struct ide_atapi_pc *,
ide_handler_t *, unsigned int, ide_expiry_t *);
ide_startstop_t ide_issue_pc(ide_drive_t *, struct ide_atapi_pc *,
--
1.5.5.1

2008-06-12 06:44:52

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 17/18] ide-floppy: cleanup idefloppy_create_rw_cmd

There should be no functionality change introduced by this patch.

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-floppy.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 1ec4951..8bdef60 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -576,8 +576,7 @@ static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
int blocks = rq->nr_sectors / floppy->bs_factor;
int rw_dir = rq_data_dir(rq);

- debug_log("create_rw10_cmd: block == %d, blocks == %d\n",
- block, blocks);
+ debug_log("%s: block == %d, blocks == %d\n", __func__, block, blocks);

idefloppy_init_pc(pc, NULL);
rq->cmd[0] = rw_dir == READ ? GPCMD_READ_10 : GPCMD_WRITE_10;
@@ -586,8 +585,10 @@ static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,

pc->rq = rq;
pc->b_count = rw_dir == READ ? 0 : rq->bio->bi_size;
- if (rq->cmd_flags & REQ_RW)
+
+ if (rw_dir)
pc->flags |= PC_FLAG_WRITING;
+
pc->buf = NULL;
pc->req_xfer = pc->buf_size = blocks * floppy->block_size;
pc->flags |= PC_FLAG_DMA_OK;
--
1.5.5.1

2008-06-12 06:45:14

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 14/18] ide-floppy: fold idefloppy_create_test_unit_ready_cmd into idefloppy_open

There's no need for this function since it is used only once.

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-floppy.c | 11 +++--------
1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index ffebf83..615d3a3 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -569,13 +569,6 @@ static void idefloppy_create_start_stop_cmd(struct ide_atapi_pc *pc, int start,
cmd[4] = start;
}

-static void idefloppy_create_test_unit_ready_cmd(struct ide_atapi_pc *pc,
- unsigned char *cmd)
-{
- idefloppy_init_pc(pc, cmd);
- cmd[0] = GPCMD_TEST_UNIT_READY;
-}
-
static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
struct ide_atapi_pc *pc, struct request *rq,
unsigned long sector)
@@ -1161,7 +1154,9 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
floppy->flags &= ~IDEFLOPPY_FLAG_FORMAT_IN_PROGRESS;
/* Just in case */

- idefloppy_create_test_unit_ready_cmd(&pc, cmd);
+ idefloppy_init_pc(&pc, cmd);
+ cmd[0] = GPCMD_TEST_UNIT_READY;
+
if (idefloppy_queue_pc_tail(drive, &pc, cmd)) {
idefloppy_create_start_stop_cmd(&pc, 1, cmd);
(void) idefloppy_queue_pc_tail(drive, &pc, cmd);
--
1.5.5.1

2008-06-12 06:45:40

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 10/18] ide-cd: move request prep from cdrom_start_seek_continuation to rq issue path

... by factoring out the rq preparation code into a separate
function called in the request routine. Bart: As a nice
side effect, this minimizes the IRQ handler execution time.

There should be no functionality change resulting from this patch.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index b8c98e1..da0f28c 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -763,9 +763,8 @@ static ide_startstop_t cdrom_seek_intr(ide_drive_t *drive)
return ide_stopped;
}

-static ide_startstop_t cdrom_start_seek_continuation(ide_drive_t *drive)
+static void ide_cd_prepare_seek_request(ide_drive_t *drive, struct request *rq)
{
- struct request *rq = HWGROUP(drive)->rq;
sector_t frame = rq->sector;

sector_div(frame, queue_hardsect_size(drive->queue) >> SECTOR_BITS);
@@ -775,6 +774,11 @@ static ide_startstop_t cdrom_start_seek_continuation(ide_drive_t *drive)
put_unaligned(cpu_to_be32(frame), (unsigned int *) &rq->cmd[2]);

rq->timeout = ATAPI_WAIT_PC;
+}
+
+static ide_startstop_t cdrom_start_seek_continuation(ide_drive_t *drive)
+{
+ struct request *rq = HWGROUP(drive)->rq;
return cdrom_transfer_packet_command(drive, rq, &cdrom_seek_intr);
}

@@ -1223,10 +1227,14 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
IDE_LARGE_SEEK(info->last_block, block,
IDECD_SEEK_THRESHOLD) &&
drive->dsc_overlap) {
+
xferlen = 0;
fn = cdrom_start_seek_continuation;
info->dma = 0;
info->start_seek = jiffies;
+
+ ide_cd_prepare_seek_request(drive, rq);
+
} else {
xferlen = 32768;
fn = cdrom_start_rw_cont;
--
1.5.5.1

2008-06-12 06:45:57

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 08/18] ide-cd: simplify request issuing path

call cdrom_start_packet_command() only from the do_request()
routine. As a nice side effect, this improves code readability a bit.

There should be no functionality change resulting from this patch.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index e9d9363..abf9af2 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -778,14 +778,12 @@ static ide_startstop_t cdrom_start_seek_continuation(ide_drive_t *drive)
return cdrom_transfer_packet_command(drive, rq, &cdrom_seek_intr);
}

-static ide_startstop_t cdrom_start_seek(ide_drive_t *drive)
+static void cdrom_start_seek(ide_drive_t *drive)
{
struct cdrom_info *info = drive->driver_data;

info->dma = 0;
info->start_seek = jiffies;
- return cdrom_start_packet_command(drive, 0,
- cdrom_start_seek_continuation);
}

/*
@@ -1160,8 +1158,7 @@ static ide_startstop_t cdrom_start_rw(ide_drive_t *drive, struct request *rq)
if (write)
cd->devinfo.media_written = 1;

- /* start sending the read/write request to the drive */
- return cdrom_start_packet_command(drive, 32768, cdrom_start_rw_cont);
+ return ide_started;
}

static ide_startstop_t cdrom_do_newpc_cont(ide_drive_t *drive)
@@ -1174,7 +1171,7 @@ static ide_startstop_t cdrom_do_newpc_cont(ide_drive_t *drive)
return cdrom_transfer_packet_command(drive, rq, cdrom_newpc_intr);
}

-static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq)
+static void cdrom_do_block_pc(ide_drive_t *drive, struct request *rq)
{
struct cdrom_info *info = drive->driver_data;

@@ -1202,10 +1199,6 @@ static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq)
if ((rq->data_len & 15) || (addr & mask))
info->dma = 0;
}
-
- /* start sending the command to the drive */
- return cdrom_start_packet_command(drive, rq->data_len,
- cdrom_do_newpc_cont);
}

/*
@@ -1214,8 +1207,9 @@ static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq)
static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
sector_t block)
{
- ide_startstop_t action;
struct cdrom_info *info = drive->driver_data;
+ ide_handler_t *fn;
+ int xferlen;

if (blk_fs_request(rq)) {
if (info->cd_flags & IDE_CD_FLAG_SEEKING) {
@@ -1235,16 +1229,23 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
}
if (rq_data_dir(rq) == READ &&
IDE_LARGE_SEEK(info->last_block, block,
- IDECD_SEEK_THRESHOLD) &&
- drive->dsc_overlap)
- action = cdrom_start_seek(drive);
- else
- action = cdrom_start_rw(drive, rq);
+ IDECD_SEEK_THRESHOLD) &&
+ drive->dsc_overlap) {
+ xferlen = 0;
+ fn = cdrom_start_seek_continuation;
+ cdrom_start_seek(drive);
+ } else {
+ xferlen = 32768;
+ fn = cdrom_start_rw_cont;
+ if (cdrom_start_rw(drive, rq) == ide_stopped)
+ return ide_stopped;
+ }
info->last_block = block;
- return action;
} else if (blk_sense_request(rq) || blk_pc_request(rq) ||
rq->cmd_type == REQ_TYPE_ATA_PC) {
- return cdrom_do_block_pc(drive, rq);
+ xferlen = rq->data_len;
+ fn = cdrom_do_newpc_cont;
+ cdrom_do_block_pc(drive, rq);
} else if (blk_special_request(rq)) {
/* right now this can only be a reset... */
cdrom_end_request(drive, 1);
@@ -1254,6 +1255,8 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
cdrom_end_request(drive, 0);
return ide_stopped;
}
+
+ return cdrom_start_packet_command(drive, xferlen, fn);
}


--
1.5.5.1

2008-06-12 06:46:21

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 02/18] ide-cd: remove ide_cd_drain_data and ide_cd_pad_transfer

Use the generic ide_pad_transfer() helper instead

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-cd.c | 33 ++++-----------------------------
1 files changed, 4 insertions(+), 29 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 92a8a36..2f0c9d4 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -599,28 +599,6 @@ static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive,
}

/*
- * Block read functions.
- */
-static void ide_cd_pad_transfer(ide_drive_t *drive, xfer_func_t *xf, int len)
-{
- while (len > 0) {
- int dum = 0;
- xf(drive, NULL, &dum, sizeof(dum));
- len -= sizeof(dum);
- }
-}
-
-static void ide_cd_drain_data(ide_drive_t *drive, int nsects)
-{
- while (nsects > 0) {
- static char dum[SECTOR_SIZE];
-
- drive->hwif->input_data(drive, NULL, dum, sizeof(dum));
- nsects--;
- }
-}
-
-/*
* Check the contents of the interrupt reason register from the cdrom
* and attempt to recover if there are problems. Returns 0 if everything's
* ok; nonzero if the request has been terminated.
@@ -635,15 +613,12 @@ static int ide_cd_check_ireason(ide_drive_t *drive, struct request *rq,
if (ireason == (!rw << 1))
return 0;
else if (ireason == (rw << 1)) {
- ide_hwif_t *hwif = drive->hwif;
- xfer_func_t *xf;

/* whoops... */
printk(KERN_ERR "%s: %s: wrong transfer direction!\n",
drive->name, __func__);

- xf = rw ? hwif->output_data : hwif->input_data;
- ide_cd_pad_transfer(drive, xf, len);
+ ide_pad_transfer(drive, rw, len);
} else if (rw == 0 && ireason == 1) {
/*
* Some drives (ASUS) seem to tell us that status info is
@@ -1006,7 +981,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
- bio_cur_sectors(rq->bio),
thislen >> 9);
if (nskip > 0) {
- ide_cd_drain_data(drive, nskip);
+ ide_pad_transfer(drive, write, nskip);
rq->current_nr_sectors -= nskip;
thislen -= (nskip << 9);
}
@@ -1043,7 +1018,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
* If the buffers are full, pipe the rest into
* oblivion.
*/
- ide_cd_drain_data(drive, thislen >> 9);
+ ide_pad_transfer(drive, 0, thislen >> 9);
else {
printk(KERN_ERR "%s: confused, missing data\n",
drive->name);
@@ -1091,7 +1066,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)

/* pad, if necessary */
if (!blk_fs_request(rq) && len > 0)
- ide_cd_pad_transfer(drive, xferfunc, len);
+ ide_pad_transfer(drive, write, len);

if (blk_pc_request(rq)) {
timeout = rq->timeout;
--
1.5.5.1

2008-06-12 06:47:24

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 11/18] ide-cd: move request prep from cdrom_start_rw_cont to rq issue path

... by factoring out the rq preparation code into a separate
function called in the request routine. Bart: As a nice
side effect, this minimizes the IRQ handler execution time.

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-cd.c | 29 ++++++++++++++++++++---------
1 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index da0f28c..a5715b1 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -691,16 +691,9 @@ static int ide_cd_check_transfer_size(ide_drive_t *drive, int len)

static ide_startstop_t cdrom_newpc_intr(ide_drive_t *);

-/*
- * Routine to send a read/write packet command to the drive. This is usually
- * called directly from cdrom_start_{read,write}(). However, for drq_interrupt
- * devices, it is called from an interrupt when the drive is ready to accept
- * the command.
- */
-static ide_startstop_t cdrom_start_rw_cont(ide_drive_t *drive)
+static ide_startstop_t ide_cd_prepare_rw_request(ide_drive_t *drive,
+ struct request *rq)
{
- struct request *rq = HWGROUP(drive)->rq;
-
if (rq_data_dir(rq) == READ) {
unsigned short sectors_per_frame =
queue_hardsect_size(drive->queue) >> SECTOR_BITS;
@@ -737,6 +730,19 @@ static ide_startstop_t cdrom_start_rw_cont(ide_drive_t *drive)
/* set up the command */
rq->timeout = ATAPI_WAIT_PC;

+ return ide_started;
+}
+
+/*
+ * Routine to send a read/write packet command to the drive. This is usually
+ * called directly from cdrom_start_{read,write}(). However, for drq_interrupt
+ * devices, it is called from an interrupt when the drive is ready to accept
+ * the command.
+ */
+static ide_startstop_t cdrom_start_rw_cont(ide_drive_t *drive)
+{
+ struct request *rq = HWGROUP(drive)->rq;
+
/* send the command to the drive and return */
return cdrom_transfer_packet_command(drive, rq, cdrom_newpc_intr);
}
@@ -1238,10 +1244,15 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
} else {
xferlen = 32768;
fn = cdrom_start_rw_cont;
+
if (cdrom_start_rw(drive, rq) == ide_stopped)
return ide_stopped;
+
+ if (ide_cd_prepare_rw_request(drive, rq) == ide_stopped)
+ return ide_stopped;
}
info->last_block = block;
+
} else if (blk_sense_request(rq) || blk_pc_request(rq) ||
rq->cmd_type == REQ_TYPE_ATA_PC) {
xferlen = rq->data_len;
--
1.5.5.1

2008-06-12 06:47:40

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 13/18] ide-floppy: replace pc->c with rq->cmd

This is the first one in the series attempting to phase out ide_atapi_pc and use
block request structs. Also, the pc request type is now REQ_TYPE_BLOCK_PC. As a result,
idefloppy_blockpc_cmd becomes unused and can go.

Signed-off-by: Borislav Petkov <[email protected]>

---
drivers/ide/ide-atapi.c | 22 ++++-
drivers/ide/ide-floppy.c | 207 +++++++++++++++++++++++-----------------------
2 files changed, 120 insertions(+), 109 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 2802031..3a6ae79 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -26,6 +26,12 @@ ide_startstop_t ide_pc_intr(ide_drive_t *drive, struct ide_atapi_pc *pc,
unsigned int temp;
u16 bcount;
u8 stat, ireason, scsi = drive->scsi;
+ unsigned char *cmd;
+
+ if (drive->media == ide_floppy)
+ cmd = pc->rq->cmd;
+ else
+ cmd = pc->c;

debug_log("Enter %s - interrupt handler\n", __func__);

@@ -63,7 +69,7 @@ ide_startstop_t ide_pc_intr(ide_drive_t *drive, struct ide_atapi_pc *pc,
local_irq_enable_in_hardirq();

if (drive->media == ide_tape && !scsi &&
- (stat & ERR_STAT) && pc->c[0] == REQUEST_SENSE)
+ (stat & ERR_STAT) && cmd[0] == REQUEST_SENSE)
stat &= ~ERR_STAT;
if ((stat & ERR_STAT) || (pc->flags & PC_FLAG_DMA_ERROR)) {
/* Error detected */
@@ -75,13 +81,13 @@ ide_startstop_t ide_pc_intr(ide_drive_t *drive, struct ide_atapi_pc *pc,
goto cmd_finished;
}

- if (pc->c[0] == REQUEST_SENSE) {
+ if (cmd[0] == REQUEST_SENSE) {
printk(KERN_ERR "%s: I/O error in request sense"
" command\n", drive->name);
return ide_do_reset(drive);
}

- debug_log("[cmd %x]: check condition\n", pc->c[0]);
+ debug_log("[cmd %x]: check condition\n", cmd[0]);

/* Retry operation */
retry_pc(drive);
@@ -175,7 +181,7 @@ cmd_finished:
pc->cur_pos += bcount;

debug_log("[cmd %x] transferred %d bytes on that intr.\n",
- pc->c[0], bcount);
+ cmd[0], bcount);

/* And set the interrupt handler again */
ide_set_handler(drive, handler, timeout, expiry);
@@ -212,6 +218,12 @@ ide_startstop_t ide_transfer_pc(ide_drive_t *drive, struct ide_atapi_pc *pc,
ide_hwif_t *hwif = drive->hwif;
ide_startstop_t startstop;
u8 ireason;
+ unsigned char *cmd;
+
+ if (drive->media == ide_floppy)
+ cmd = pc->rq->cmd;
+ else
+ cmd = pc->c;

if (ide_wait_stat(&startstop, drive, DRQ_STAT, BUSY_STAT, WAIT_READY)) {
printk(KERN_ERR "%s: Strange, packet command initiated yet "
@@ -240,7 +252,7 @@ ide_startstop_t ide_transfer_pc(ide_drive_t *drive, struct ide_atapi_pc *pc,

/* Send the actual packet */
if ((pc->flags & PC_FLAG_ZIP_DRIVE) == 0)
- hwif->output_data(drive, NULL, pc->c, 12);
+ hwif->output_data(drive, NULL, cmd, 12);

return ide_started;
}
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index b368943..ffebf83 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -217,7 +217,7 @@ static int idefloppy_end_request(ide_drive_t *drive, int uptodate, int nsecs)
/* Why does this happen? */
if (!rq)
return 0;
- if (!blk_special_request(rq)) {
+ if (!blk_pc_request(rq)) {
/* our real local end request function */
ide_end_request(drive, uptodate, nsecs);
return 0;
@@ -282,13 +282,14 @@ static void idefloppy_update_buffers(ide_drive_t *drive,
* pass through the driver.
*/
static void idefloppy_queue_pc_head(ide_drive_t *drive, struct ide_atapi_pc *pc,
- struct request *rq)
+ struct request *rq, unsigned char *cmd)
{
struct ide_floppy_obj *floppy = drive->driver_data;

blk_rq_init(NULL, rq);
rq->buffer = (char *) pc;
- rq->cmd_type = REQ_TYPE_SPECIAL;
+ rq->cmd_type = REQ_TYPE_BLOCK_PC;
+ memcpy(rq->cmd, cmd, 12);
rq->cmd_flags |= REQ_PREEMPT;
rq->rq_disk = floppy->disk;
ide_do_drive_cmd(drive, rq);
@@ -323,10 +324,10 @@ static void ide_floppy_callback(ide_drive_t *drive)
if (floppy->failed_pc == pc)
floppy->failed_pc = NULL;

- if (pc->c[0] == GPCMD_READ_10 || pc->c[0] == GPCMD_WRITE_10 ||
- (pc->rq && blk_pc_request(pc->rq)))
+ if (pc->rq->cmd[0] == GPCMD_READ_10 || pc->rq->cmd[0] == GPCMD_WRITE_10
+ || (pc->rq && blk_pc_request(pc->rq)))
uptodate = 1; /* FIXME */
- else if (pc->c[0] == GPCMD_REQUEST_SENSE) {
+ else if (pc->rq->cmd[0] == GPCMD_REQUEST_SENSE) {
u8 *buf = floppy->pc->buf;

if (!pc->error) {
@@ -349,9 +350,11 @@ static void ide_floppy_callback(ide_drive_t *drive)
idefloppy_end_request(drive, uptodate, 0);
}

-static void idefloppy_init_pc(struct ide_atapi_pc *pc)
+static void idefloppy_init_pc(struct ide_atapi_pc *pc, unsigned char *cmd)
{
- memset(pc->c, 0, 12);
+ if (cmd)
+ memset(cmd, 0, 12);
+
pc->retries = 0;
pc->flags = 0;
pc->req_xfer = 0;
@@ -360,11 +363,12 @@ static void idefloppy_init_pc(struct ide_atapi_pc *pc)
pc->callback = ide_floppy_callback;
}

-static void idefloppy_create_request_sense_cmd(struct ide_atapi_pc *pc)
+static void idefloppy_create_request_sense_cmd(struct ide_atapi_pc *pc,
+ unsigned char *cmd)
{
- idefloppy_init_pc(pc);
- pc->c[0] = GPCMD_REQUEST_SENSE;
- pc->c[4] = 255;
+ idefloppy_init_pc(pc, cmd);
+ cmd[0] = GPCMD_REQUEST_SENSE;
+ cmd[4] = 18;
pc->req_xfer = 18;
}

@@ -376,12 +380,13 @@ static void idefloppy_retry_pc(ide_drive_t *drive)
{
struct ide_atapi_pc *pc;
struct request *rq;
+ unsigned char cmd[12];

(void)ide_read_error(drive);
pc = idefloppy_next_pc_storage(drive);
rq = idefloppy_next_rq_storage(drive);
- idefloppy_create_request_sense_cmd(pc);
- idefloppy_queue_pc_head(drive, pc, rq);
+ idefloppy_create_request_sense_cmd(pc, cmd);
+ idefloppy_queue_pc_head(drive, pc, rq, cmd);
}

/* The usual interrupt handler called during a packet command. */
@@ -405,7 +410,7 @@ static int idefloppy_transfer_pc(ide_drive_t *drive)
idefloppy_floppy_t *floppy = drive->driver_data;

/* Send the actual packet */
- drive->hwif->output_data(drive, NULL, floppy->pc->c, 12);
+ drive->hwif->output_data(drive, NULL, floppy->pc->rq->cmd, 12);

/* Timeout for the packet command */
return IDEFLOPPY_WAIT_CMD;
@@ -454,7 +459,7 @@ static void ide_floppy_report_error(idefloppy_floppy_t *floppy,

printk(KERN_ERR "ide-floppy: %s: I/O error, pc = %2x, key = %2x, "
"asc = %2x, ascq = %2x\n",
- floppy->drive->name, pc->c[0], floppy->sense_key,
+ floppy->drive->name, pc->rq->cmd[0], floppy->sense_key,
floppy->asc, floppy->ascq);

}
@@ -465,7 +470,7 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
idefloppy_floppy_t *floppy = drive->driver_data;

if (floppy->failed_pc == NULL &&
- pc->c[0] != GPCMD_REQUEST_SENSE)
+ pc->rq->cmd[0] != GPCMD_REQUEST_SENSE)
floppy->failed_pc = pc;
/* Set the current packet command */
floppy->pc = pc;
@@ -489,30 +494,31 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
IDEFLOPPY_WAIT_CMD, NULL);
}

-static void idefloppy_create_prevent_cmd(struct ide_atapi_pc *pc, int prevent)
+static void idefloppy_create_prevent_cmd(struct ide_atapi_pc *pc, int prevent,
+ unsigned char *cmd)
{
- debug_log("creating prevent removal command, prevent = %d\n", prevent);
-
- idefloppy_init_pc(pc);
- pc->c[0] = GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL;
- pc->c[4] = prevent;
+ idefloppy_init_pc(pc, cmd);
+ cmd[0] = GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL;
+ cmd[4] = prevent;
}

-static void idefloppy_create_read_capacity_cmd(struct ide_atapi_pc *pc)
+static void idefloppy_create_read_capacity_cmd(struct ide_atapi_pc *pc,
+ unsigned char *cmd)
{
- idefloppy_init_pc(pc);
- pc->c[0] = GPCMD_READ_FORMAT_CAPACITIES;
- pc->c[7] = 255;
- pc->c[8] = 255;
+ idefloppy_init_pc(pc, cmd);
+ cmd[0] = GPCMD_READ_FORMAT_CAPACITIES;
+ cmd[7] = 255;
+ cmd[8] = 255;
pc->req_xfer = 255;
}

static void idefloppy_create_format_unit_cmd(struct ide_atapi_pc *pc, int b,
- int l, int flags)
+ int l, int flags,
+ unsigned char *cmd)
{
- idefloppy_init_pc(pc);
- pc->c[0] = GPCMD_FORMAT_UNIT;
- pc->c[1] = 0x17;
+ idefloppy_init_pc(pc, cmd);
+ cmd[0] = GPCMD_FORMAT_UNIT;
+ cmd[1] = 0x17;

memset(pc->buf, 0, 12);
pc->buf[1] = 0xA2;
@@ -530,14 +536,15 @@ static void idefloppy_create_format_unit_cmd(struct ide_atapi_pc *pc, int b,

/* A mode sense command is used to "sense" floppy parameters. */
static void idefloppy_create_mode_sense_cmd(struct ide_atapi_pc *pc,
- u8 page_code, u8 type)
+ u8 page_code, u8 type,
+ unsigned char *cmd)
{
u16 length = 8; /* sizeof(Mode Parameter Header) = 8 Bytes */

- idefloppy_init_pc(pc);
- pc->c[0] = GPCMD_MODE_SENSE_10;
- pc->c[1] = 0;
- pc->c[2] = page_code + (type << 6);
+ idefloppy_init_pc(pc, cmd);
+ cmd[0] = GPCMD_MODE_SENSE_10;
+ cmd[1] = 0;
+ cmd[2] = page_code + (type << 6);

switch (page_code) {
case IDEFLOPPY_CAPABILITIES_PAGE:
@@ -550,21 +557,23 @@ static void idefloppy_create_mode_sense_cmd(struct ide_atapi_pc *pc,
printk(KERN_ERR "ide-floppy: unsupported page code "
"in create_mode_sense_cmd\n");
}
- put_unaligned(cpu_to_be16(length), (u16 *) &pc->c[7]);
+ put_unaligned(cpu_to_be16(length), (u16 *) &cmd[7]);
pc->req_xfer = length;
}

-static void idefloppy_create_start_stop_cmd(struct ide_atapi_pc *pc, int start)
+static void idefloppy_create_start_stop_cmd(struct ide_atapi_pc *pc, int start,
+ unsigned char *cmd)
{
- idefloppy_init_pc(pc);
- pc->c[0] = GPCMD_START_STOP_UNIT;
- pc->c[4] = start;
+ idefloppy_init_pc(pc, cmd);
+ cmd[0] = GPCMD_START_STOP_UNIT;
+ cmd[4] = start;
}

-static void idefloppy_create_test_unit_ready_cmd(struct ide_atapi_pc *pc)
+static void idefloppy_create_test_unit_ready_cmd(struct ide_atapi_pc *pc,
+ unsigned char *cmd)
{
- idefloppy_init_pc(pc);
- pc->c[0] = GPCMD_TEST_UNIT_READY;
+ idefloppy_init_pc(pc, cmd);
+ cmd[0] = GPCMD_TEST_UNIT_READY;
}

static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
@@ -573,18 +582,18 @@ static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
{
int block = sector / floppy->bs_factor;
int blocks = rq->nr_sectors / floppy->bs_factor;
- int cmd = rq_data_dir(rq);
+ int rw_dir = rq_data_dir(rq);

debug_log("create_rw10_cmd: block == %d, blocks == %d\n",
- block, blocks);
+ block, blocks);

- idefloppy_init_pc(pc);
- pc->c[0] = cmd == READ ? GPCMD_READ_10 : GPCMD_WRITE_10;
- put_unaligned(cpu_to_be16(blocks), (unsigned short *)&pc->c[7]);
- put_unaligned(cpu_to_be32(block), (unsigned int *) &pc->c[2]);
+ idefloppy_init_pc(pc, NULL);
+ rq->cmd[0] = rw_dir == READ ? GPCMD_READ_10 : GPCMD_WRITE_10;
+ put_unaligned(cpu_to_be16(blocks), (unsigned short *)&rq->cmd[7]);
+ put_unaligned(cpu_to_be32(block), (unsigned int *) &rq->cmd[2]);

pc->rq = rq;
- pc->b_count = cmd == READ ? 0 : rq->bio->bi_size;
+ pc->b_count = rw_dir == READ ? 0 : rq->bio->bi_size;
if (rq->cmd_flags & REQ_RW)
pc->flags |= PC_FLAG_WRITING;
pc->buf = NULL;
@@ -592,25 +601,6 @@ static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
pc->flags |= PC_FLAG_DMA_OK;
}

-static void idefloppy_blockpc_cmd(idefloppy_floppy_t *floppy,
- struct ide_atapi_pc *pc, struct request *rq)
-{
- idefloppy_init_pc(pc);
- memcpy(pc->c, rq->cmd, sizeof(pc->c));
- pc->rq = rq;
- pc->b_count = rq->data_len;
- if (rq->data_len && rq_data_dir(rq) == WRITE)
- pc->flags |= PC_FLAG_WRITING;
- pc->buf = rq->data;
- if (rq->bio)
- pc->flags |= PC_FLAG_DMA_OK;
- /*
- * possibly problematic, doesn't look like ide-floppy correctly
- * handled scattered requests if dma fails...
- */
- pc->req_xfer = pc->buf_size = rq->data_len;
-}
-
static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
struct request *rq, sector_t block_s)
{
@@ -618,9 +608,9 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
struct ide_atapi_pc *pc;
unsigned long block = (unsigned long)block_s;

- debug_log("dev: %s, cmd_type: %x, errors: %d\n",
+ debug_log("dev: %s, cmd: %x, cmd_type: %x, errors: %d\n",
rq->rq_disk ? rq->rq_disk->disk_name : "?",
- rq->cmd_type, rq->errors);
+ rq->cmd[0], rq->cmd_type, rq->errors);
debug_log("sector: %ld, nr_sectors: %ld, "
"current_nr_sectors: %d\n", (long)rq->sector,
rq->nr_sectors, rq->current_nr_sectors);
@@ -644,12 +634,9 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
}
pc = idefloppy_next_pc_storage(drive);
idefloppy_create_rw_cmd(floppy, pc, rq, block);
- } else if (blk_special_request(rq)) {
+ } else if (blk_pc_request(rq))
pc = (struct ide_atapi_pc *) rq->buffer;
- } else if (blk_pc_request(rq)) {
- pc = idefloppy_next_pc_storage(drive);
- idefloppy_blockpc_cmd(floppy, pc, rq);
- } else {
+ else {
blk_dump_rq_flags(rq,
"ide-floppy: unsupported command in queue");
idefloppy_end_request(drive, 0, 0);
@@ -671,7 +658,8 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
* Add a special packet command request to the tail of the request queue,
* and wait for it to be serviced.
*/
-static int idefloppy_queue_pc_tail(ide_drive_t *drive, struct ide_atapi_pc *pc)
+static int idefloppy_queue_pc_tail(ide_drive_t *drive, struct ide_atapi_pc *pc,
+ unsigned char *cmd)
{
struct ide_floppy_obj *floppy = drive->driver_data;
struct request *rq;
@@ -679,7 +667,8 @@ static int idefloppy_queue_pc_tail(ide_drive_t *drive, struct ide_atapi_pc *pc)

rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
rq->buffer = (char *) pc;
- rq->cmd_type = REQ_TYPE_SPECIAL;
+ memcpy(rq->cmd, cmd, 12);
+ rq->cmd_type = REQ_TYPE_BLOCK_PC;
error = blk_execute_rq(drive->queue, floppy->disk, rq, 0);
blk_put_request(rq);

@@ -694,15 +683,16 @@ static int ide_floppy_get_flexible_disk_page(ide_drive_t *drive)
{
idefloppy_floppy_t *floppy = drive->driver_data;
struct ide_atapi_pc pc;
+ unsigned char cmd[12];
u8 *page;
int capacity, lba_capacity;
u16 transfer_rate, sector_size, cyls, rpm;
u8 heads, sectors;

idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_FLEXIBLE_DISK_PAGE,
- MODE_SENSE_CURRENT);
+ MODE_SENSE_CURRENT, cmd);

- if (idefloppy_queue_pc_tail(drive, &pc)) {
+ if (idefloppy_queue_pc_tail(drive, &pc, cmd)) {
printk(KERN_ERR "ide-floppy: Can't get flexible disk page"
" parameters\n");
return 1;
@@ -746,13 +736,14 @@ static int idefloppy_get_sfrp_bit(ide_drive_t *drive)
{
idefloppy_floppy_t *floppy = drive->driver_data;
struct ide_atapi_pc pc;
+ unsigned char cmd[12];

floppy->srfp = 0;
idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_CAPABILITIES_PAGE,
- MODE_SENSE_CURRENT);
+ MODE_SENSE_CURRENT, cmd);

pc.flags |= PC_FLAG_SUPPRESS_ERROR;
- if (idefloppy_queue_pc_tail(drive, &pc))
+ if (idefloppy_queue_pc_tail(drive, &pc, cmd))
return 1;

floppy->srfp = pc.buf[8 + 2] & 0x40;
@@ -767,6 +758,7 @@ static int ide_floppy_get_capacity(ide_drive_t *drive)
{
idefloppy_floppy_t *floppy = drive->driver_data;
struct ide_atapi_pc pc;
+ unsigned char cmd[12];
u8 *cap_desc;
u8 header_len, desc_cnt;
int i, rc = 1, blocks, length;
@@ -777,8 +769,8 @@ static int ide_floppy_get_capacity(ide_drive_t *drive)
floppy->bs_factor = 1;
set_capacity(floppy->disk, 0);

- idefloppy_create_read_capacity_cmd(&pc);
- if (idefloppy_queue_pc_tail(drive, &pc)) {
+ idefloppy_create_read_capacity_cmd(&pc, cmd);
+ if (idefloppy_queue_pc_tail(drive, &pc, cmd)) {
printk(KERN_ERR "ide-floppy: Can't get floppy parameters\n");
return 1;
}
@@ -882,6 +874,7 @@ static int ide_floppy_get_format_capacities(ide_drive_t *drive, int __user *arg)
u8 header_len, desc_cnt;
int i, blocks, length, u_array_size, u_index;
int __user *argp;
+ unsigned char cmd[12];

if (get_user(u_array_size, arg))
return (-EFAULT);
@@ -889,8 +882,8 @@ static int ide_floppy_get_format_capacities(ide_drive_t *drive, int __user *arg)
if (u_array_size <= 0)
return (-EINVAL);

- idefloppy_create_read_capacity_cmd(&pc);
- if (idefloppy_queue_pc_tail(drive, &pc)) {
+ idefloppy_create_read_capacity_cmd(&pc, cmd);
+ if (idefloppy_queue_pc_tail(drive, &pc, cmd)) {
printk(KERN_ERR "ide-floppy: Can't get floppy parameters\n");
return (-EIO);
}
@@ -944,11 +937,12 @@ static int idefloppy_get_format_progress(ide_drive_t *drive, int __user *arg)
{
idefloppy_floppy_t *floppy = drive->driver_data;
struct ide_atapi_pc pc;
+ unsigned char cmd[12];
int progress_indication = 0x10000;

if (floppy->srfp) {
- idefloppy_create_request_sense_cmd(&pc);
- if (idefloppy_queue_pc_tail(drive, &pc))
+ idefloppy_create_request_sense_cmd(&pc, cmd);
+ if (idefloppy_queue_pc_tail(drive, &pc, cmd))
return (-EIO);

if (floppy->sense_key == 2 &&
@@ -1150,6 +1144,7 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
struct ide_floppy_obj *floppy;
ide_drive_t *drive;
struct ide_atapi_pc pc;
+ unsigned char cmd[12];
int ret = 0;

debug_log("Reached %s\n", __func__);
@@ -1166,10 +1161,10 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
floppy->flags &= ~IDEFLOPPY_FLAG_FORMAT_IN_PROGRESS;
/* Just in case */

- idefloppy_create_test_unit_ready_cmd(&pc);
- if (idefloppy_queue_pc_tail(drive, &pc)) {
- idefloppy_create_start_stop_cmd(&pc, 1);
- (void) idefloppy_queue_pc_tail(drive, &pc);
+ idefloppy_create_test_unit_ready_cmd(&pc, cmd);
+ if (idefloppy_queue_pc_tail(drive, &pc, cmd)) {
+ idefloppy_create_start_stop_cmd(&pc, 1, cmd);
+ (void) idefloppy_queue_pc_tail(drive, &pc, cmd);
}

if (ide_floppy_get_capacity(drive)
@@ -1191,8 +1186,8 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
floppy->flags |= IDEFLOPPY_FLAG_MEDIA_CHANGED;
/* IOMEGA Clik! drives do not support lock/unlock commands */
if (!(floppy->flags & IDEFLOPPY_FLAG_CLIK_DRIVE)) {
- idefloppy_create_prevent_cmd(&pc, 1);
- (void) idefloppy_queue_pc_tail(drive, &pc);
+ idefloppy_create_prevent_cmd(&pc, 1, cmd);
+ (void) idefloppy_queue_pc_tail(drive, &pc, cmd);
}
check_disk_change(inode->i_bdev);
} else if (floppy->flags & IDEFLOPPY_FLAG_FORMAT_IN_PROGRESS) {
@@ -1213,14 +1208,15 @@ static int idefloppy_release(struct inode *inode, struct file *filp)
struct ide_floppy_obj *floppy = ide_floppy_g(disk);
ide_drive_t *drive = floppy->drive;
struct ide_atapi_pc pc;
+ unsigned char cmd[12];

debug_log("Reached %s\n", __func__);

if (floppy->openers == 1) {
/* IOMEGA Clik! drives do not support lock/unlock commands */
if (!(floppy->flags & IDEFLOPPY_FLAG_CLIK_DRIVE)) {
- idefloppy_create_prevent_cmd(&pc, 0);
- (void) idefloppy_queue_pc_tail(drive, &pc);
+ idefloppy_create_prevent_cmd(&pc, 0, cmd);
+ (void) idefloppy_queue_pc_tail(drive, &pc, cmd);
}

floppy->flags &= ~IDEFLOPPY_FLAG_FORMAT_IN_PROGRESS;
@@ -1247,6 +1243,8 @@ static int idefloppy_getgeo(struct block_device *bdev, struct hd_geometry *geo)
static int ide_floppy_lockdoor(idefloppy_floppy_t *floppy,
struct ide_atapi_pc *pc, unsigned long arg, unsigned int cmd)
{
+ unsigned char pc_cmd[12];
+
if (floppy->openers > 1)
return -EBUSY;

@@ -1258,13 +1256,13 @@ static int ide_floppy_lockdoor(idefloppy_floppy_t *floppy,
if (cmd == CDROMEJECT)
prevent = 0;

- idefloppy_create_prevent_cmd(pc, prevent);
- (void) idefloppy_queue_pc_tail(floppy->drive, pc);
+ idefloppy_create_prevent_cmd(pc, prevent, pc_cmd);
+ (void) idefloppy_queue_pc_tail(floppy->drive, pc, pc_cmd);
}

if (cmd == CDROMEJECT) {
- idefloppy_create_start_stop_cmd(pc, 2);
- (void) idefloppy_queue_pc_tail(floppy->drive, pc);
+ idefloppy_create_start_stop_cmd(pc, 2, pc_cmd);
+ (void) idefloppy_queue_pc_tail(floppy->drive, pc, pc_cmd);
}

return 0;
@@ -1275,6 +1273,7 @@ static int ide_floppy_format_unit(idefloppy_floppy_t *floppy,
{
int blocks, length, flags, err = 0;
struct ide_atapi_pc pc;
+ unsigned char cmd[12];

if (floppy->openers > 1) {
/* Don't format if someone is using the disk */
@@ -1307,9 +1306,9 @@ static int ide_floppy_format_unit(idefloppy_floppy_t *floppy,
}

(void) idefloppy_get_sfrp_bit(floppy->drive);
- idefloppy_create_format_unit_cmd(&pc, blocks, length, flags);
+ idefloppy_create_format_unit_cmd(&pc, blocks, length, flags, cmd);

- if (idefloppy_queue_pc_tail(floppy->drive, &pc))
+ if (idefloppy_queue_pc_tail(floppy->drive, &pc, cmd))
err = -EIO;

out:
--
1.5.5.1

2008-06-12 06:47:57

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 12/18] ide-cd: move request prep chunk from cdrom_do_newpc_cont to rq issue path

Bart: As a nice side effect, this minimizes the IRQ handler execution time.

There should be no functionality change resulting from this patch.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index a5715b1..0cd23d4 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1167,9 +1167,6 @@ static ide_startstop_t cdrom_do_newpc_cont(ide_drive_t *drive)
{
struct request *rq = HWGROUP(drive)->rq;

- if (!rq->timeout)
- rq->timeout = ATAPI_WAIT_PC;
-
return cdrom_transfer_packet_command(drive, rq, cdrom_newpc_intr);
}

@@ -1253,11 +1250,18 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
}
info->last_block = block;

- } else if (blk_sense_request(rq) || blk_pc_request(rq) ||
+ } else if (blk_sense_request(rq) ||
+ blk_pc_request(rq) ||
rq->cmd_type == REQ_TYPE_ATA_PC) {
+
xferlen = rq->data_len;
fn = cdrom_do_newpc_cont;
+
+ if (!rq->timeout)
+ rq->timeout = ATAPI_WAIT_PC;
+
cdrom_do_block_pc(drive, rq);
+
} else if (blk_special_request(rq)) {
/* right now this can only be a reset... */
cdrom_end_request(drive, 1);
@@ -1271,8 +1275,6 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
return cdrom_start_packet_command(drive, xferlen, fn);
}

-
-
/*
* Ioctl handling.
*
--
1.5.5.1

2008-06-12 06:48:24

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 09/18] ide-cd: fold cdrom_start_seek into ide_cd_do_request

Do what the compiler does anyway: inline a function that is used only once. This
saves us the overhead of a function call and the function is small enough to be
embedded in the callsite anyways.

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-cd.c | 11 ++---------
1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index abf9af2..b8c98e1 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -778,14 +778,6 @@ static ide_startstop_t cdrom_start_seek_continuation(ide_drive_t *drive)
return cdrom_transfer_packet_command(drive, rq, &cdrom_seek_intr);
}

-static void cdrom_start_seek(ide_drive_t *drive)
-{
- struct cdrom_info *info = drive->driver_data;
-
- info->dma = 0;
- info->start_seek = jiffies;
-}
-
/*
* Fix up a possibly partially-processed request so that we can start it over
* entirely, or even put it back on the request queue.
@@ -1233,7 +1225,8 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
drive->dsc_overlap) {
xferlen = 0;
fn = cdrom_start_seek_continuation;
- cdrom_start_seek(drive);
+ info->dma = 0;
+ info->start_seek = jiffies;
} else {
xferlen = 32768;
fn = cdrom_start_rw_cont;
--
1.5.5.1

Subject: Re: [PATCH 00/18] misc generic ide stuff


Hi,

On Thursday 12 June 2008, Borislav Petkov wrote:
> Hi Bart,
>
> here are some generic ide conversion patches. The first 12 are what i thought
> you suggested :) concerning prepping the ide-cd code for the generic layer. The
> remaining 6 are what i've done so far wrt removing ide_atapi_pc from the ide
> drivers. It is obvious that this is not trivial and I basically tiptoe around the

Thanks.

I applied patches #1-2, #5-12 and #14-15.

I skipped patches #3-4, #13 and #16-18 for now
(more details in replies to corresponding mails).

> landmines in the IRQ handler and request issue paths :). This raises also
> several questions:
>
> 1. ide-cd uses the cdrom_info struct for e.g. dma status notification, the other
> ide drivers use it per packet command in pc->flags. Well the last are kinda too
> much to carry for _each_ command and i'm thinking maybe put all that in the
> ide_drive_t and make it generic enough to fit all drivers. This way
> pc->callback() can also be harboured there. One concern might be when a flag is
> strictly per packet command which could be put somewhere else (maybe in the
> struct request since it already has members when it is used as a packet
> command).

Some pc->flags describe device's properties and thus should be moved to
ide_drive_t (->dev_flags) while some other correspond to the queued command
and moving them to ide_drive_t would be a bad idea IMO.

For ATA commands I was planning to put taskfile flags into rq->special field
(well, I actually implemented it in draft patch and it looks OK) so maybe we
can do something similar for packet commands.

> 2. Can all that pc->xferred, pc->b_count, pc->errors and pc->cur_pos accounting
> be safely mapped to a rq? I see some discrepancies like is pc->buf_size ==
> rq->data_len, what about pc->req_xfer? I'll have a more detailed look at those
> when i have more spare time later.

If you ask if they can be mapped 'directly' then the answer is: "probably no"
but if the question is whether it is possible to do it after some changes then
the answer is: "probably yes". :)

[ However it may be necessary to convert ATAPI drivers to use scatterlists
instead of open-coded ->bio walking for PIO transfers first. ]

Thanks,
Bart

Subject: Re: [PATCH 02/18] ide-cd: remove ide_cd_drain_data and ide_cd_pad_transfer

On Thursday 12 June 2008, Borislav Petkov wrote:
> Use the generic ide_pad_transfer() helper instead
>
> Signed-off-by: Borislav Petkov <[email protected]>

applied w/ ide_cd_drain_data() -> ide_pad_transfer() conversion fixup

[...]

> @@ -1006,7 +981,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
> - bio_cur_sectors(rq->bio),
> thislen >> 9);
> if (nskip > 0) {
> - ide_cd_drain_data(drive, nskip);
> + ide_pad_transfer(drive, write, nskip);
> rq->current_nr_sectors -= nskip;
> thislen -= (nskip << 9);
> }

ide_cd_drain_data() took number for _sectors_ as an argument
while ide_pad_transfer() wants to be given number of _bytes_

> @@ -1043,7 +1018,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
> * If the buffers are full, pipe the rest into
> * oblivion.
> */
> - ide_cd_drain_data(drive, thislen >> 9);
> + ide_pad_transfer(drive, 0, thislen >> 9);

ditto

Subject: Re: [PATCH 03/18] ide-cd: cdrom_decode_status: factor out block pc error handling code

On Thursday 12 June 2008, Borislav Petkov wrote:
> ... into cdrom_handle_failed_pc_req().

I actually think that we should try to unite pc/fs error handling as much
as possible as it shouldn't really matter if i.e. READ command came through
fs or pc request - the error handling w.r.t. hardware should be the same
(at the moment it is not always the case - the most blatant example of this
disrepancy is handling of NOT_READY sense key for WRITE commands).

When I was suggesting factoring out error handling I rather meant moving
out _everything_ after OK_STAT() (sorry for the confusion). On the second
thought we may do it in even simpler way by moving:

...
/* check for errors */
stat = ide_read_status(drive);

if (stat_ret)
*stat_ret = stat;

if (OK_STAT(stat, good_stat, BAD_R_STAT))
return 0;
...

to cdrom_decode_status() users and passing as an argument 'stat' instead
of 'good_stat' and 'stat_ret'.

Therefore I skipped this patch (and also patch #4) for now.

Subject: Re: [PATCH 13/18] ide-floppy: replace pc->c with rq->cmd

On Thursday 12 June 2008, Borislav Petkov wrote:
> This is the first one in the series attempting to phase out ide_atapi_pc and use
> block request structs. Also, the pc request type is now REQ_TYPE_BLOCK_PC. As a result,
> idefloppy_blockpc_cmd becomes unused and can go.

Do not attack too many dragons at once or they will slay you... ;)

IOW this patch mixes too many changes (some _really_ non-trivial)
in one go which results in rather bad outcome...

[...]

> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index b368943..ffebf83 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -217,7 +217,7 @@ static int idefloppy_end_request(ide_drive_t *drive, int uptodate, int nsecs)
> /* Why does this happen? */
> if (!rq)
> return 0;
> - if (!blk_special_request(rq)) {
> + if (!blk_pc_request(rq)) {
> /* our real local end request function */
> ide_end_request(drive, uptodate, nsecs);
> return 0;
> @@ -282,13 +282,14 @@ static void idefloppy_update_buffers(ide_drive_t *drive,
> * pass through the driver.
> */
> static void idefloppy_queue_pc_head(ide_drive_t *drive, struct ide_atapi_pc *pc,
> - struct request *rq)
> + struct request *rq, unsigned char *cmd)
> {
> struct ide_floppy_obj *floppy = drive->driver_data;
>
> blk_rq_init(NULL, rq);
> rq->buffer = (char *) pc;
> - rq->cmd_type = REQ_TYPE_SPECIAL;
> + rq->cmd_type = REQ_TYPE_BLOCK_PC;
> + memcpy(rq->cmd, cmd, 12);
> rq->cmd_flags |= REQ_PREEMPT;
> rq->rq_disk = floppy->disk;
> ide_do_drive_cmd(drive, rq);

REQUEST SENSE command is switched from REQ_TYPE_SPECIAL to REQ_TYPE_BLOCK_PC
which should belong to a separate patch and is premature IMHO (the other
ATAPI drivers seem to still use REQ_TYPE_SPECIAL so probably it makes sense
to unify/move REQUEST_SENSE handling to ide-atapi.c first).

> @@ -323,10 +324,10 @@ static void ide_floppy_callback(ide_drive_t *drive)
> if (floppy->failed_pc == pc)
> floppy->failed_pc = NULL;
>
> - if (pc->c[0] == GPCMD_READ_10 || pc->c[0] == GPCMD_WRITE_10 ||
> - (pc->rq && blk_pc_request(pc->rq)))
> + if (pc->rq->cmd[0] == GPCMD_READ_10 || pc->rq->cmd[0] == GPCMD_WRITE_10
> + || (pc->rq && blk_pc_request(pc->rq)))
> uptodate = 1; /* FIXME */

This actually seems to break REQUEST SENSE handling
(please note the blk_pc_request() check above)... :(

> - else if (pc->c[0] == GPCMD_REQUEST_SENSE) {
> + else if (pc->rq->cmd[0] == GPCMD_REQUEST_SENSE) {

[...]

> @@ -592,25 +601,6 @@ static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
> pc->flags |= PC_FLAG_DMA_OK;
> }
>
> -static void idefloppy_blockpc_cmd(idefloppy_floppy_t *floppy,
> - struct ide_atapi_pc *pc, struct request *rq)
> -{
> - idefloppy_init_pc(pc);
> - memcpy(pc->c, rq->cmd, sizeof(pc->c));
> - pc->rq = rq;
> - pc->b_count = rq->data_len;
> - if (rq->data_len && rq_data_dir(rq) == WRITE)
> - pc->flags |= PC_FLAG_WRITING;
> - pc->buf = rq->data;
> - if (rq->bio)
> - pc->flags |= PC_FLAG_DMA_OK;
> - /*
> - * possibly problematic, doesn't look like ide-floppy correctly
> - * handled scattered requests if dma fails...
> - */
> - pc->req_xfer = pc->buf_size = rq->data_len;
> -}

[...]

> @@ -644,12 +634,9 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
> }
> pc = idefloppy_next_pc_storage(drive);
> idefloppy_create_rw_cmd(floppy, pc, rq, block);
> - } else if (blk_special_request(rq)) {
> + } else if (blk_pc_request(rq))
> pc = (struct ide_atapi_pc *) rq->buffer;
> - } else if (blk_pc_request(rq)) {
> - pc = idefloppy_next_pc_storage(drive);
> - idefloppy_blockpc_cmd(floppy, pc, rq);
> - } else {
> + else {
> blk_dump_rq_flags(rq,
> "ide-floppy: unsupported command in queue");
> idefloppy_end_request(drive, 0, 0);

Also the above changes seem to break handling of blk_pc_request() requests
(i.e. SG_IO ioctl) because they will be no longer initialized properly.

OTOH the main change (pc->c to rq->cmd) looks OK (only minor complaint is
that 'u8' is both shorter and more readable than 'unsigned short')...

[ I had to also skip patches #16-17 because of skipping this patch. ]

Subject: Re: [PATCH 18/18] ide: use flags in IRQ handler

On Thursday 12 June 2008, Borislav Petkov wrote:
> Pass pc->flags to the IRQ handler from the drivers (ide-tape/floppy/scsi) thus
> making it more pc-unaware.

This moves the problem around without improving the overall situation
w.r.t. pc->flags (now ATAPI drivers are more pc-aware instead).

It also adds one more argument to the already overloaded ide_pc_intr()
(my fault, I went a bit overboard but I added a nice TODO explanation! ;).

Since there are better ideas on how to deal with pc->flags (you've even
described them yourself in the first mail :) I skipped this patch for now.

2008-06-15 09:28:26

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 02/18] ide-cd: remove ide_cd_drain_data and ide_cd_pad_transfer

On Sat, Jun 14, 2008 at 07:29:05PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 12 June 2008, Borislav Petkov wrote:
> > Use the generic ide_pad_transfer() helper instead
> >
> > Signed-off-by: Borislav Petkov <[email protected]>
>
> applied w/ ide_cd_drain_data() -> ide_pad_transfer() conversion fixup
>
> [...]
>
> > @@ -1006,7 +981,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
> > - bio_cur_sectors(rq->bio),
> > thislen >> 9);
> > if (nskip > 0) {
> > - ide_cd_drain_data(drive, nskip);
> > + ide_pad_transfer(drive, write, nskip);
> > rq->current_nr_sectors -= nskip;
> > thislen -= (nskip << 9);
> > }
>
> ide_cd_drain_data() took number for _sectors_ as an argument
> while ide_pad_transfer() wants to be given number of _bytes_

/me crawls ashamed back into the black place behind the wall cardboard ... :)
>
> > @@ -1043,7 +1018,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
> > * If the buffers are full, pipe the rest into
> > * oblivion.
> > */
> > - ide_cd_drain_data(drive, thislen >> 9);
> > + ide_pad_transfer(drive, 0, thislen >> 9);
>
> ditto

--
Regards/Gru?,
Boris.

2008-06-15 10:21:44

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 13/18] ide-floppy: replace pc->c with rq->cmd

On Sat, Jun 14, 2008 at 07:40:22PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 12 June 2008, Borislav Petkov wrote:
> > This is the first one in the series attempting to phase out ide_atapi_pc and use
> > block request structs. Also, the pc request type is now REQ_TYPE_BLOCK_PC. As a result,
> > idefloppy_blockpc_cmd becomes unused and can go.
>
> Do not attack too many dragons at once or they will slay you... ;)
>
> IOW this patch mixes too many changes (some _really_ non-trivial)
> in one go which results in rather bad outcome...

yeah, this was more of a RFC patch for me not intended in any way for submission
but instead to get your comments on it. Atapi-land is full of surprises and you
have to be _really_ careful when changing stuff around. I got bitten by that
couple times when preparing those patches. Anyways, don't take those seriously,
they were simply a warmup :).

Thanks,
Boris.

2008-06-15 10:27:29

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 00/18] misc generic ide stuff

On Sat, Jun 14, 2008 at 07:29:00PM +0200, Bartlomiej Zolnierkiewicz wrote:

Hi,

>
> Hi,
>
> On Thursday 12 June 2008, Borislav Petkov wrote:
> > Hi Bart,
> >
> > here are some generic ide conversion patches. The first 12 are what i thought
> > you suggested :) concerning prepping the ide-cd code for the generic layer. The
> > remaining 6 are what i've done so far wrt removing ide_atapi_pc from the ide
> > drivers. It is obvious that this is not trivial and I basically tiptoe around the
>
> Thanks.
>
> I applied patches #1-2, #5-12 and #14-15.
>
> I skipped patches #3-4, #13 and #16-18 for now
> (more details in replies to corresponding mails).
>
> > landmines in the IRQ handler and request issue paths :). This raises also
> > several questions:
> >
> > 1. ide-cd uses the cdrom_info struct for e.g. dma status notification, the other
> > ide drivers use it per packet command in pc->flags. Well the last are kinda too
> > much to carry for _each_ command and i'm thinking maybe put all that in the
> > ide_drive_t and make it generic enough to fit all drivers. This way
> > pc->callback() can also be harboured there. One concern might be when a flag is
> > strictly per packet command which could be put somewhere else (maybe in the
> > struct request since it already has members when it is used as a packet
> > command).
>
> Some pc->flags describe device's properties and thus should be moved to
> ide_drive_t (->dev_flags) while some other correspond to the queued command
> and moving them to ide_drive_t would be a bad idea IMO.

With "the last" i meant pc->flags and not "per packet" so i completely agree:
per-packet commands go somewhere in rq and the others land in ide_drive_t.

> For ATA commands I was planning to put taskfile flags into rq->special field
> (well, I actually implemented it in draft patch and it looks OK) so maybe we
> can do something similar for packet commands.

Yep, i was looking for a field to put all those per-command flags into so
rq->special sounds good. Will prepare some patches for that later.

> > 2. Can all that pc->xferred, pc->b_count, pc->errors and pc->cur_pos accounting
> > be safely mapped to a rq? I see some discrepancies like is pc->buf_size ==
> > rq->data_len, what about pc->req_xfer? I'll have a more detailed look at those
> > when i have more spare time later.
>
> If you ask if they can be mapped 'directly' then the answer is: "probably no"
> but if the question is whether it is possible to do it after some changes then
> the answer is: "probably yes". :)
>
> [ However it may be necessary to convert ATAPI drivers to use scatterlists
> instead of open-coded ->bio walking for PIO transfers first. ]

Looking into it right now...

Thanks.

--
Regards/Gru?,
Boris.

Subject: Re: [PATCH 13/18] ide-floppy: replace pc->c with rq->cmd

On Sunday 15 June 2008, Borislav Petkov wrote:
> On Sat, Jun 14, 2008 at 07:40:22PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Thursday 12 June 2008, Borislav Petkov wrote:
> > > This is the first one in the series attempting to phase out ide_atapi_pc and use
> > > block request structs. Also, the pc request type is now REQ_TYPE_BLOCK_PC. As a result,
> > > idefloppy_blockpc_cmd becomes unused and can go.
> >
> > Do not attack too many dragons at once or they will slay you... ;)
> >
> > IOW this patch mixes too many changes (some _really_ non-trivial)
> > in one go which results in rather bad outcome...
>
> yeah, this was more of a RFC patch for me not intended in any way for submission
> but instead to get your comments on it. Atapi-land is full of surprises and you
> have to be _really_ careful when changing stuff around. I got bitten by that
> couple times when preparing those patches. Anyways, don't take those seriously,
> they were simply a warmup :).

I'm not worried at all - I'm sure that if you keep fighting these dragons
one by one they won't stand a chance... :)

2008-08-15 07:34:23

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 03/18] ide-cd: cdrom_decode_status: factor out block pc error handling code

On Sat, Jun 14, 2008 at 07:29:21PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 12 June 2008, Borislav Petkov wrote:
> > ... into cdrom_handle_failed_pc_req().
>
> I actually think that we should try to unite pc/fs error handling as much
> as possible as it shouldn't really matter if i.e. READ command came through
> fs or pc request - the error handling w.r.t. hardware should be the same
> (at the moment it is not always the case - the most blatant example of this
> disrepancy is handling of NOT_READY sense key for WRITE commands).
>
> When I was suggesting factoring out error handling I rather meant moving
> out _everything_ after OK_STAT() (sorry for the confusion). On the second
> thought we may do it in even simpler way by moving:
>
> ...
> /* check for errors */
> stat = ide_read_status(drive);
>
> if (stat_ret)
> *stat_ret = stat;
>
> if (OK_STAT(stat, good_stat, BAD_R_STAT))
> return 0;
> ...
>
> to cdrom_decode_status() users and passing as an argument 'stat' instead
> of 'good_stat' and 'stat_ret'.
>
> Therefore I skipped this patch (and also patch #4) for now.

Hi Bart,

here's a first attempt at that. These are only RFC of nature, please take a look
when there's time. They've been lightly tested but I'll do more later since this
path is not hit that often and a special test case will be required.

---
From: Borislav Petkov <[email protected]>
Date: Sun, 10 Aug 2008 18:54:03 +0200
Subject: [PATCH 1/2] ide-cd: move error handling outside of cdrom_decode_status()

There should be no functionality change resulting from this patch.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index f675cee..f2c12be 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -284,20 +284,11 @@ static void ide_dump_status_no_sense(ide_drive_t *drive, const char *msg, u8 st)
* 0: if the request should be continued.
* 1: if the request was ended.
*/
-static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret)
+static int cdrom_decode_status(ide_drive_t *drive, int stat)
{
ide_hwif_t *hwif = drive->hwif;
struct request *rq = hwif->hwgroup->rq;
- int stat, err, sense_key;
-
- /* check for errors */
- stat = hwif->tp_ops->read_status(hwif);
-
- if (stat_ret)
- *stat_ret = stat;
-
- if (OK_STAT(stat, good_stat, BAD_R_STAT))
- return 0;
+ int err, sense_key;

/* get the IDE error register */
err = ide_read_error(drive);
@@ -563,7 +554,7 @@ static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive,
ide_handler_t *handler)
{
ide_hwif_t *hwif = drive->hwif;
- int cmd_len;
+ int cmd_len, stat;
struct cdrom_info *info = drive->driver_data;
ide_startstop_t startstop;

@@ -574,8 +565,11 @@ static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive,
*/

/* check for errors */
- if (cdrom_decode_status(drive, ATA_DRQ, NULL))
- return ide_stopped;
+ stat = hwif->tp_ops->read_status(hwif);
+
+ if (!OK_STAT(stat, ATA_DRQ, BAD_R_STAT))
+ if (cdrom_decode_status(drive, stat))
+ return ide_stopped;

/* ok, next interrupt will be DMA interrupt */
if (info->dma)
@@ -739,8 +733,11 @@ static ide_startstop_t cdrom_seek_intr(ide_drive_t *drive)
int stat;
static int retry = 10;

- if (cdrom_decode_status(drive, 0, &stat))
- return ide_stopped;
+ stat = drive->hwif->tp_ops->read_status(drive->hwif);
+
+ if (!OK_STAT(stat, 0, BAD_R_STAT))
+ if (cdrom_decode_status(drive, stat))
+ return ide_stopped;

drive->atapi_flags |= IDE_AFLAG_SEEKING;

@@ -917,8 +914,11 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
}
}

- if (cdrom_decode_status(drive, 0, &stat))
- return ide_stopped;
+ stat = hwif->tp_ops->read_status(hwif);
+
+ if (!OK_STAT(stat, 0, BAD_R_STAT))
+ if (cdrom_decode_status(drive, stat))
+ return ide_stopped;

/* using dma, transfer is complete now */
if (dma) {
--
1.5.5.4


--
Regards/Gruss,
Boris.

2008-08-15 07:35:09

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 03/18] ide-cd: cdrom_decode_status: factor out block pc error handling code

From: Borislav Petkov <[email protected]>
Date: Mon, 11 Aug 2008 07:55:27 +0200
Subject: [PATCH 2/2] ide-cd: unify error handling in cdrom_decode_status()

There should be no difference in handling rw commands
based on the type of the request (pc|rq) they came on. As
a nice side effect, this makes the code more readable.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index f2c12be..ae53a74 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -311,8 +311,13 @@ static int cdrom_decode_status(ide_drive_t *drive, int stat)
ide_error(drive, "request sense failure", stat);
return 1;

- } else if (blk_pc_request(rq) || rq->cmd_type == REQ_TYPE_ATA_PC) {
- /* All other functions, except for READ. */
+ } else if (blk_pc_request(rq) || blk_fs_request(rq) ||
+ rq->cmd_type == REQ_TYPE_ATA_PC) {
+
+ int do_end_request = 0;
+
+ if (blk_noretry_request(rq))
+ do_end_request = 1;

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

- /* check for tray open */
- if (sense_key == NOT_READY) {
- cdrom_saw_media_change(drive);
- } else if (sense_key == UNIT_ATTENTION) {
- /* check for media change */
- cdrom_saw_media_change(drive);
- return 0;
- } else if (sense_key == ILLEGAL_REQUEST &&
- rq->cmd[0] == GPCMD_START_STOP_UNIT) {
- /*
- * 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!
- */
- } else if (!(rq->cmd_flags & REQ_QUIET)) {
- /* otherwise, print an error */
- 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;
-
- if (sense_key == NOT_READY) {
+ switch (sense_key) {
+ case NOT_READY:
/* tray open */
if (rq_data_dir(rq) == READ) {
cdrom_saw_media_change(drive);
@@ -395,8 +363,9 @@ static int cdrom_decode_status(ide_drive_t *drive, int stat)
return 1;
}
}
- } else if (sense_key == UNIT_ATTENTION) {
- /* media change */
+ break;
+
+ case UNIT_ATTENTION:
cdrom_saw_media_change(drive);

/*
@@ -405,15 +374,33 @@ static int cdrom_decode_status(ide_drive_t *drive, int stat)
*/
if (++rq->errors > ERROR_MAX)
do_end_request = 1;
- } else if (sense_key == ILLEGAL_REQUEST ||
- sense_key == DATA_PROTECT) {
- /*
- * No point in retrying after an illegal request or data
- * protect error.
- */
- ide_dump_status_no_sense(drive, "command error", stat);
- do_end_request = 1;
- } else if (sense_key == MEDIUM_ERROR) {
+ else
+ return 0;
+ break;
+
+ case ILLEGAL_REQUEST:
+ case DATA_PROTECT:
+
+ if (rq->cmd[0] == GPCMD_START_STOP_UNIT) {
+ /*
+ * 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!
+ */
+ } else {
+ /*
+ * No point in retrying after an illegal req.
+ * or data protect error.
+ */
+ ide_dump_status_no_sense(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.
@@ -422,20 +409,41 @@ static int cdrom_decode_status(ide_drive_t *drive, int stat)
"media error (bad sector)",
stat);
do_end_request = 1;
- } else if (sense_key == BLANK_CHECK) {
+ break;
+
+ case BLANK_CHECK:
/* disk appears blank ?? */
ide_dump_status_no_sense(drive, "media error (blank)",
stat);
do_end_request = 1;
- } else if ((err & ~ATA_ABORTED) != 0) {
+ break;
+
+ default:
+ ide_error(drive, "bad sense", stat);
+ break;
+ }
+
+ if (!(rq->cmd_flags & REQ_QUIET)) {
+ /* print an error */
+ ide_dump_status(drive, (blk_fs_request(rq)) ?
+ "fs request error" :
+ "packet command error", stat);
+ }
+
+ 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)) {
+
+ }
+
+ if ((++rq->errors > ERROR_MAX)) {
/* we've racked up too many retries, abort */
do_end_request = 1;
}

+ rq->cmd_flags |= REQ_FAILED;
+
/*
* End a request through request sense analysis when we have
* sense data. We need this in order to perform end of media
@@ -445,11 +453,12 @@ static int cdrom_decode_status(ide_drive_t *drive, int stat)
goto end_request;

/*
- * If we got a CHECK_CONDITION status, queue
- * a request sense command.
+ * If we got a CHECK_CONDITION status, queue a request sense
+ * command.
*/
if (stat & ATA_ERR)
cdrom_queue_request_sense(drive, NULL, NULL);
+
} else {
blk_dump_rq_flags(rq, "ide-cd: bad rq");
cdrom_end_request(drive, 0);
--
1.5.5.4

--
Regards/Gruss,
Boris.

2008-08-16 20:26:24

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 03/18] ide-cd: cdrom_decode_status: factor out block pc error handling code

[.. ]

> here's a first attempt at that. These are only RFC of nature, please take a look
> when there's time. They've been lightly tested but I'll do more later since this
> path is not hit that often and a special test case will be required.

Yep, this one has problems as I just noticed. Reworking...