[PATCH 00/18] misc generic ide stuff

[PATCH 00/18] misc generic ide stuff

[Borislav Petkov]

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 :))

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

[Borislav Petkov]

This is done in the request issue path anyway

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

Use the generic ide_pad_transfer() helper instead

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 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

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

[Bartlomiej Zolnierkiewicz]

On Thursday 12 June 2008, Borislav Petkov wrote:
> Use the generic ide_pad_transfer() helper instead
> 
> Signed-off-by: Borislav Petkov <petkovbb@gmail.com>

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

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

[Borislav Petkov]

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 <petkovbb@gmail.com>
> 
> 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.

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

[Borislav Petkov]

... into cdrom_handle_failed_pc_req().

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 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

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

[Bartlomiej Zolnierkiewicz]

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.

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

[Borislav Petkov]

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 <petkovbb@gmail.com>
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 <petkovbb@gmail.com>
---
 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.

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

[Borislav Petkov]

[.. ]

> 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... 

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

[Borislav Petkov]

From: Borislav Petkov <petkovbb@gmail.com>
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 <petkovbb@gmail.com>
---
 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.

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

[Borislav Petkov]

... 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 <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

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 <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

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 <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

... 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 <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

... 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 <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

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 <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

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 <petkovbb@gmail.com>

---
 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

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

[Bartlomiej Zolnierkiewicz]

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. ]

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

[Borislav Petkov]

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.

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

[Bartlomiej Zolnierkiewicz]

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... :)

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

[Borislav Petkov]

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

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

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 <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

The pc is still embedded in rq->buffer.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

There should be no functionality change introduced by this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 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

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

[Borislav Petkov]

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 <petkovbb@gmail.com>
---
 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

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

[Bartlomiej Zolnierkiewicz]

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.

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

[Bartlomiej Zolnierkiewicz]

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

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

[Borislav Petkov]

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.