[PATCH 0/9] ide-atapi: remove ide_atapi_pc from the irq handler

[PATCH 0/9] ide-atapi: remove ide_atapi_pc from the irq handler

[Borislav Petkov]

Hi Bart,

here's a first attempt at removing all references to ide_atapi_pc in the ATAPI
IRQ handler. I've moved some of the members to the drive struct and will deal
with them later :). The next step is to add an ide_atapi_queue_pc() routine
similar to ide_cd_queue_pc() and then rewrite all functions in the drivers to
use struct requests and local buffers instead of pc->buf and then, after that
works reliably, finally get rid of ide_atapi_pc completely.

This has been tested with ide-floppy.

 drivers/ide/ide-atapi.c        |  123 ++++++++++++++++-------------
 drivers/ide/ide-floppy.c       |   56 +++++++-------
 drivers/ide/ide-floppy.h       |    4 +-
 drivers/ide/ide-floppy_ioctl.c |   19 +++--
 drivers/ide/ide-tape.c         |  173 ++++++++++++++++++++++------------------
 include/linux/ide.h            |   20 +++--
 6 files changed, 214 insertions(+), 181 deletions(-)

[PATCH 1/9] ide-atapi: remove PC_FLAG_WRITING atapi flag

[Borislav Petkov]

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c        |   12 +++++++-----
 drivers/ide/ide-floppy.c       |    4 ----
 drivers/ide/ide-floppy_ioctl.c |    1 -
 drivers/ide/ide-tape.c         |    4 ++--
 include/linux/ide.h            |    3 +--
 5 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index d412bd2..2f0d5e7 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -296,11 +296,15 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 	const struct ide_tp_ops *tp_ops = hwif->tp_ops;
 	xfer_func_t *xferfunc;
 	unsigned int timeout, temp;
+	int write = 0;
 	u16 bcount;
 	u8 stat, ireason, dsc = 0;
 
 	debug_log("Enter %s - interrupt handler\n", __func__);
 
+	if (rq)
+		write = (rq_data_dir(rq) == WRITE) ? 1 : 0;
+
 	timeout = (drive->media == ide_floppy) ? WAIT_FLOPPY_CMD
 					       : WAIT_TAPE_CMD;
 
@@ -389,8 +393,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 		return ide_do_reset(drive);
 	}
 
-	if (((ireason & ATAPI_IO) == ATAPI_IO) ==
-		!!(pc->flags & PC_FLAG_WRITING)) {
+	if (((ireason & ATAPI_IO) == ATAPI_IO) == write) {
 		/* Hopefully, we will never get here */
 		printk(KERN_ERR "%s: We wanted to %s, but the device wants us "
 				"to %s!\n", drive->name,
@@ -399,7 +402,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 		return ide_do_reset(drive);
 	}
 
-	if (!(pc->flags & PC_FLAG_WRITING)) {
+	if (!write) {
 		/* Reading - Check that we have enough space */
 		temp = pc->xferred + bcount;
 		if (temp > pc->req_xfer) {
@@ -421,8 +424,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 
 	if ((drive->media == ide_floppy && !pc->buf) ||
 	    (drive->media == ide_tape && pc->bh)) {
-		int done = drive->pc_io_buffers(drive, pc, bcount,
-				  !!(pc->flags & PC_FLAG_WRITING));
+		int done = drive->pc_io_buffers(drive, pc, bcount, write);
 
 		/* FIXME: don't do partial completions */
 		if (drive->media == ide_floppy)
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index fdec729..3ebe75a 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -254,8 +254,6 @@ static void idefloppy_create_rw_cmd(ide_drive_t *drive,
 
 	pc->rq = rq;
 	pc->b_count = 0;
-	if (rq->cmd_flags & REQ_RW)
-		pc->flags |= PC_FLAG_WRITING;
 	pc->buf = NULL;
 	pc->req_xfer = pc->buf_size = blocks * floppy->block_size;
 	pc->flags |= PC_FLAG_DMA_OK;
@@ -268,8 +266,6 @@ static void idefloppy_blockpc_cmd(struct ide_disk_obj *floppy,
 	memcpy(pc->c, rq->cmd, sizeof(pc->c));
 	pc->rq = rq;
 	pc->b_count = 0;
-	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;
diff --git a/drivers/ide/ide-floppy_ioctl.c b/drivers/ide/ide-floppy_ioctl.c
index 8f8be85..513f651 100644
--- a/drivers/ide/ide-floppy_ioctl.c
+++ b/drivers/ide/ide-floppy_ioctl.c
@@ -109,7 +109,6 @@ static void ide_floppy_create_format_unit_cmd(struct ide_atapi_pc *pc, int b,
 	put_unaligned(cpu_to_be32(b), (unsigned int *)(&pc->buf[4]));
 	put_unaligned(cpu_to_be32(l), (unsigned int *)(&pc->buf[8]));
 	pc->buf_size = 12;
-	pc->flags |= PC_FLAG_WRITING;
 }
 
 static int ide_floppy_get_sfrp_bit(ide_drive_t *drive, struct ide_atapi_pc *pc)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index ac9e29a..ba89d6a 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -373,10 +373,11 @@ static void idetape_output_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
 static void idetape_update_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc)
 {
 	struct idetape_bh *bh = pc->bh;
+	struct request *rq = drive->hwif->hwgroup->rq;
 	int count;
 	unsigned int bcount = pc->xferred;
 
-	if (pc->flags & PC_FLAG_WRITING)
+	if (rq_data_dir(rq) == WRITE)
 		return;
 	while (bcount) {
 		if (bh == NULL) {
@@ -774,7 +775,6 @@ static void ide_tape_create_rw_cmd(idetape_tape_t *tape,
 		atomic_set(&bh->b_count, 0);
 	} else if (opcode == WRITE_6) {
 		pc->c[0] = WRITE_6;
-		pc->flags |= PC_FLAG_WRITING;
 		pc->b_data = bh->b_data;
 		pc->b_count = atomic_read(&bh->b_count);
 	}
diff --git a/include/linux/ide.h b/include/linux/ide.h
index ad57a44..2041073 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -386,9 +386,8 @@ enum {
 	PC_FLAG_DMA_OK			= (1 << 3),
 	PC_FLAG_DMA_IN_PROGRESS		= (1 << 4),
 	PC_FLAG_DMA_ERROR		= (1 << 5),
-	PC_FLAG_WRITING			= (1 << 6),
 	/* command timed out */
-	PC_FLAG_TIMEDOUT		= (1 << 7),
+	PC_FLAG_TIMEDOUT		= (1 << 6),
 };
 
 /*
-- 
1.6.0.4

[PATCH 2/9] ide-atapi: replace pc->flags with drive->pc_flags

[Borislav Petkov]

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c        |   43 ++++++++++---------
 drivers/ide/ide-floppy.c       |   31 +++++++------
 drivers/ide/ide-floppy.h       |    4 +-
 drivers/ide/ide-floppy_ioctl.c |   16 ++++---
 drivers/ide/ide-tape.c         |   92 +++++++++++++++++++++-------------------
 include/linux/ide.h            |    5 ++-
 6 files changed, 103 insertions(+), 88 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 2f0d5e7..ec29133 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -117,11 +117,12 @@ int ide_io_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
 }
 EXPORT_SYMBOL_GPL(ide_io_buffers);
 
-void ide_init_pc(struct ide_atapi_pc *pc)
+void ide_init_pc(ide_drive_t *drive, struct ide_atapi_pc *pc)
 {
 	memset(pc, 0, sizeof(*pc));
 	pc->buf = pc->pc_buf;
 	pc->buf_size = IDE_PC_BUFFER_SIZE;
+	drive->pc_flags = 0;
 }
 EXPORT_SYMBOL_GPL(ide_init_pc);
 
@@ -171,7 +172,7 @@ int ide_do_test_unit_ready(ide_drive_t *drive, struct gendisk *disk)
 {
 	struct ide_atapi_pc pc;
 
-	ide_init_pc(&pc);
+	ide_init_pc(drive, &pc);
 	pc.c[0] = TEST_UNIT_READY;
 
 	return ide_queue_pc_tail(drive, disk, &pc);
@@ -182,12 +183,12 @@ int ide_do_start_stop(ide_drive_t *drive, struct gendisk *disk, int start)
 {
 	struct ide_atapi_pc pc;
 
-	ide_init_pc(&pc);
+	ide_init_pc(drive, &pc);
 	pc.c[0] = START_STOP;
 	pc.c[4] = start;
 
 	if (drive->media == ide_tape)
-		pc.flags |= PC_FLAG_WAIT_FOR_DSC;
+		drive->pc_flags |= PC_FLAG_WAIT_FOR_DSC;
 
 	return ide_queue_pc_tail(drive, disk, &pc);
 }
@@ -200,7 +201,7 @@ int ide_set_media_lock(ide_drive_t *drive, struct gendisk *disk, int on)
 	if ((drive->dev_flags & IDE_DFLAG_DOORLOCKING) == 0)
 		return 0;
 
-	ide_init_pc(&pc);
+	ide_init_pc(drive, &pc);
 	pc.c[0] = ALLOW_MEDIUM_REMOVAL;
 	pc.c[4] = on;
 
@@ -210,7 +211,7 @@ EXPORT_SYMBOL_GPL(ide_set_media_lock);
 
 void ide_create_request_sense_cmd(ide_drive_t *drive, struct ide_atapi_pc *pc)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = REQUEST_SENSE;
 	if (drive->media == ide_floppy) {
 		pc->c[4] = 255;
@@ -308,7 +309,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 	timeout = (drive->media == ide_floppy) ? WAIT_FLOPPY_CMD
 					       : WAIT_TAPE_CMD;
 
-	if (pc->flags & PC_FLAG_TIMEDOUT) {
+	if (drive->pc_flags & PC_FLAG_TIMEDOUT) {
 		drive->pc_callback(drive, 0);
 		return ide_stopped;
 	}
@@ -316,14 +317,14 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 	/* Clear the interrupt */
 	stat = tp_ops->read_status(hwif);
 
-	if (pc->flags & PC_FLAG_DMA_IN_PROGRESS) {
+	if (drive->pc_flags & PC_FLAG_DMA_IN_PROGRESS) {
 		if (hwif->dma_ops->dma_end(drive) ||
 		    (drive->media == ide_tape && (stat & ATA_ERR))) {
 			if (drive->media == ide_floppy)
 				printk(KERN_ERR "%s: DMA %s error\n",
 					drive->name, rq_data_dir(pc->rq)
 						     ? "write" : "read");
-			pc->flags |= PC_FLAG_DMA_ERROR;
+			drive->pc_flags |= PC_FLAG_DMA_ERROR;
 		} else {
 			pc->xferred = pc->req_xfer;
 			if (drive->pc_update_buffers)
@@ -337,7 +338,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 		debug_log("Packet command completed, %d bytes transferred\n",
 			  pc->xferred);
 
-		pc->flags &= ~PC_FLAG_DMA_IN_PROGRESS;
+		drive->pc_flags &= ~PC_FLAG_DMA_IN_PROGRESS;
 
 		local_irq_enable_in_hardirq();
 
@@ -345,7 +346,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 		    (stat & ATA_ERR) && rq->cmd[0] == REQUEST_SENSE)
 			stat &= ~ATA_ERR;
 
-		if ((stat & ATA_ERR) || (pc->flags & PC_FLAG_DMA_ERROR)) {
+		if ((stat & ATA_ERR) || (drive->pc_flags & PC_FLAG_DMA_ERROR)) {
 			/* Error detected */
 			debug_log("%s: I/O error\n", drive->name);
 
@@ -368,7 +369,8 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 		}
 		pc->error = 0;
 
-		if ((pc->flags & PC_FLAG_WAIT_FOR_DSC) && (stat & ATA_DSC) == 0)
+		if ((drive->pc_flags & PC_FLAG_WAIT_FOR_DSC) &&
+		    (stat & ATA_DSC) == 0)
 			dsc = 1;
 
 		/* Command finished - Call the callback function */
@@ -377,8 +379,8 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 		return ide_stopped;
 	}
 
-	if (pc->flags & PC_FLAG_DMA_IN_PROGRESS) {
-		pc->flags &= ~PC_FLAG_DMA_IN_PROGRESS;
+	if (drive->pc_flags & PC_FLAG_DMA_IN_PROGRESS) {
+		drive->pc_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);
@@ -489,7 +491,6 @@ static int ide_delayed_transfer_pc(ide_drive_t *drive)
 
 static ide_startstop_t ide_transfer_pc(ide_drive_t *drive)
 {
-	struct ide_atapi_pc *pc = drive->pc;
 	ide_hwif_t *hwif = drive->hwif;
 	struct request *rq = hwif->hwgroup->rq;
 	ide_expiry_t *expiry;
@@ -536,8 +537,8 @@ static ide_startstop_t ide_transfer_pc(ide_drive_t *drive)
 	ide_set_handler(drive, ide_pc_intr, timeout, expiry);
 
 	/* Begin DMA, if necessary */
-	if (pc->flags & PC_FLAG_DMA_OK) {
-		pc->flags |= PC_FLAG_DMA_IN_PROGRESS;
+	if (drive->pc_flags & PC_FLAG_DMA_OK) {
+		drive->pc_flags |= PC_FLAG_DMA_IN_PROGRESS;
 		hwif->dma_ops->dma_start(drive);
 	}
 
@@ -571,18 +572,18 @@ ide_startstop_t ide_issue_pc(ide_drive_t *drive, unsigned int timeout)
 				min(pc->req_xfer, 63 * 1024));
 	}
 
-	if (pc->flags & PC_FLAG_DMA_ERROR) {
-		pc->flags &= ~PC_FLAG_DMA_ERROR;
+	if (drive->pc_flags & PC_FLAG_DMA_ERROR) {
+		drive->pc_flags &= ~PC_FLAG_DMA_ERROR;
 		ide_dma_off(drive);
 	}
 
-	if (((pc->flags & PC_FLAG_DMA_OK) &&
+	if (((drive->pc_flags & PC_FLAG_DMA_OK) &&
 		(drive->dev_flags & IDE_DFLAG_USING_DMA)) ||
 	    drive->dma)
 		drive->dma = !hwif->dma_ops->dma_setup(drive);
 
 	if (!drive->dma)
-		pc->flags &= ~PC_FLAG_DMA_OK;
+		drive->pc_flags &= ~PC_FLAG_DMA_OK;
 
 	ide_pktcmd_tf_load(drive, tf_flags, bcount, drive->dma);
 
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 3ebe75a..44e5b8a 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -183,7 +183,7 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
 	drive->pc = pc;
 
 	if (pc->retries > IDEFLOPPY_MAX_PC_RETRIES) {
-		if (!(pc->flags & PC_FLAG_SUPPRESS_ERROR))
+		if (!(drive->pc_flags & PC_FLAG_SUPPRESS_ERROR))
 			ide_floppy_report_error(floppy, pc);
 		/* Giving up */
 		pc->error = IDEFLOPPY_ERROR_GENERAL;
@@ -200,9 +200,10 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
 	return ide_issue_pc(drive, WAIT_FLOPPY_CMD);
 }
 
-void ide_floppy_create_read_capacity_cmd(struct ide_atapi_pc *pc)
+void ide_floppy_create_read_capacity_cmd(ide_drive_t *drive,
+					 struct ide_atapi_pc *pc)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = GPCMD_READ_FORMAT_CAPACITIES;
 	pc->c[7] = 255;
 	pc->c[8] = 255;
@@ -210,11 +211,12 @@ void ide_floppy_create_read_capacity_cmd(struct ide_atapi_pc *pc)
 }
 
 /* A mode sense command is used to "sense" floppy parameters. */
-void ide_floppy_create_mode_sense_cmd(struct ide_atapi_pc *pc, u8 page_code)
+void ide_floppy_create_mode_sense_cmd(ide_drive_t *drive,
+				      struct ide_atapi_pc *pc, u8 page_code)
 {
 	u16 length = 8; /* sizeof(Mode Parameter Header) = 8 Bytes */
 
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = GPCMD_MODE_SENSE_10;
 	pc->c[1] = 0;
 	pc->c[2] = page_code;
@@ -245,7 +247,7 @@ static void idefloppy_create_rw_cmd(ide_drive_t *drive,
 	ide_debug_log(IDE_DBG_FUNC, "%s: block: %d, blocks: %d\n", __func__,
 		      block, blocks);
 
-	ide_init_pc(pc);
+	ide_init_pc(drive, 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]);
@@ -256,19 +258,19 @@ static void idefloppy_create_rw_cmd(ide_drive_t *drive,
 	pc->b_count = 0;
 	pc->buf = NULL;
 	pc->req_xfer = pc->buf_size = blocks * floppy->block_size;
-	pc->flags |= PC_FLAG_DMA_OK;
+	drive->pc_flags |= PC_FLAG_DMA_OK;
 }
 
-static void idefloppy_blockpc_cmd(struct ide_disk_obj *floppy,
-		struct ide_atapi_pc *pc, struct request *rq)
+static void idefloppy_blockpc_cmd(ide_drive_t *drive, struct ide_atapi_pc *pc,
+				  struct request *rq)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	memcpy(pc->c, rq->cmd, sizeof(pc->c));
 	pc->rq = rq;
 	pc->b_count = 0;
 	pc->buf = rq->data;
 	if (rq->bio)
-		pc->flags |= PC_FLAG_DMA_OK;
+		drive->pc_flags |= PC_FLAG_DMA_OK;
 	/*
 	 * possibly problematic, doesn't look like ide-floppy correctly
 	 * handled scattered requests if dma fails...
@@ -316,7 +318,7 @@ static ide_startstop_t ide_floppy_do_request(ide_drive_t *drive,
 		pc = (struct ide_atapi_pc *) rq->buffer;
 	} else if (blk_pc_request(rq)) {
 		pc = &floppy->queued_pc;
-		idefloppy_blockpc_cmd(floppy, pc, rq);
+		idefloppy_blockpc_cmd(drive, pc, rq);
 	} else {
 		blk_dump_rq_flags(rq, PFX "unsupported command in queue");
 		ide_floppy_end_request(drive, 0, 0);
@@ -348,7 +350,8 @@ static int ide_floppy_get_flexible_disk_page(ide_drive_t *drive,
 	u16 transfer_rate, sector_size, cyls, rpm;
 	u8 heads, sectors;
 
-	ide_floppy_create_mode_sense_cmd(pc, IDEFLOPPY_FLEXIBLE_DISK_PAGE);
+	ide_floppy_create_mode_sense_cmd(drive, pc,
+					 IDEFLOPPY_FLEXIBLE_DISK_PAGE);
 
 	if (ide_queue_pc_tail(drive, disk, pc)) {
 		printk(KERN_ERR PFX "Can't get flexible disk page params\n");
@@ -416,7 +419,7 @@ static int ide_floppy_get_capacity(ide_drive_t *drive)
 	floppy->bs_factor = 1;
 	drive->capacity64 = 0;
 
-	ide_floppy_create_read_capacity_cmd(&pc);
+	ide_floppy_create_read_capacity_cmd(drive, &pc);
 	if (ide_queue_pc_tail(drive, disk, &pc)) {
 		printk(KERN_ERR PFX "Can't get floppy parameters\n");
 		return 1;
diff --git a/drivers/ide/ide-floppy.h b/drivers/ide/ide-floppy.h
index 6dd2beb..cbc7bbe 100644
--- a/drivers/ide/ide-floppy.h
+++ b/drivers/ide/ide-floppy.h
@@ -19,8 +19,8 @@
 
 /* ide-floppy.c */
 extern const struct ide_disk_ops ide_atapi_disk_ops;
-void ide_floppy_create_mode_sense_cmd(struct ide_atapi_pc *, u8);
-void ide_floppy_create_read_capacity_cmd(struct ide_atapi_pc *);
+void ide_floppy_create_mode_sense_cmd(ide_drive_t *, struct ide_atapi_pc *, u8);
+void ide_floppy_create_read_capacity_cmd(ide_drive_t *, struct ide_atapi_pc *);
 
 /* ide-floppy_ioctl.c */
 int ide_floppy_ioctl(ide_drive_t *, struct block_device *, fmode_t,
diff --git a/drivers/ide/ide-floppy_ioctl.c b/drivers/ide/ide-floppy_ioctl.c
index 513f651..88e14e4 100644
--- a/drivers/ide/ide-floppy_ioctl.c
+++ b/drivers/ide/ide-floppy_ioctl.c
@@ -46,7 +46,7 @@ static int ide_floppy_get_format_capacities(ide_drive_t *drive,
 	if (u_array_size <= 0)
 		return -EINVAL;
 
-	ide_floppy_create_read_capacity_cmd(pc);
+	ide_floppy_create_read_capacity_cmd(drive, pc);
 	if (ide_queue_pc_tail(drive, floppy->disk, pc)) {
 		printk(KERN_ERR "ide-floppy: Can't get floppy parameters\n");
 		return -EIO;
@@ -91,10 +91,11 @@ static int ide_floppy_get_format_capacities(ide_drive_t *drive,
 	return 0;
 }
 
-static void ide_floppy_create_format_unit_cmd(struct ide_atapi_pc *pc, int b,
-		int l, int flags)
+static void ide_floppy_create_format_unit_cmd(ide_drive_t *drive,
+					      struct ide_atapi_pc *pc, int b,
+					      int l, int flags)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = GPCMD_FORMAT_UNIT;
 	pc->c[1] = 0x17;
 
@@ -117,8 +118,9 @@ static int ide_floppy_get_sfrp_bit(ide_drive_t *drive, struct ide_atapi_pc *pc)
 
 	drive->atapi_flags &= ~IDE_AFLAG_SRFP;
 
-	ide_floppy_create_mode_sense_cmd(pc, IDEFLOPPY_CAPABILITIES_PAGE);
-	pc->flags |= PC_FLAG_SUPPRESS_ERROR;
+	ide_floppy_create_mode_sense_cmd(drive, pc,
+					 IDEFLOPPY_CAPABILITIES_PAGE);
+	drive->pc_flags |= PC_FLAG_SUPPRESS_ERROR;
 
 	if (ide_queue_pc_tail(drive, floppy->disk, pc))
 		return 1;
@@ -166,7 +168,7 @@ static int ide_floppy_format_unit(ide_drive_t *drive, struct ide_atapi_pc *pc,
 	}
 
 	ide_floppy_get_sfrp_bit(drive, pc);
-	ide_floppy_create_format_unit_cmd(pc, blocks, length, flags);
+	ide_floppy_create_format_unit_cmd(drive, pc, blocks, length, flags);
 
 	if (ide_queue_pc_tail(drive, floppy->disk, pc))
 		err = -EIO;
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index ba89d6a..caccb82 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -411,7 +411,7 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
 		 pc->c[0], tape->sense_key, tape->asc, tape->ascq);
 
 	/* Correct pc->xferred by asking the tape.	 */
-	if (pc->flags & PC_FLAG_DMA_ERROR) {
+	if (drive->pc_flags & PC_FLAG_DMA_ERROR) {
 		pc->xferred = pc->req_xfer -
 			tape->blk_size *
 			get_unaligned_be32(&sense[3]);
@@ -430,26 +430,26 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
 			/* don't report an error, everything's ok */
 			pc->error = 0;
 			/* don't retry read/write */
-			pc->flags |= PC_FLAG_ABORT;
+			drive->pc_flags |= PC_FLAG_ABORT;
 		}
 	}
 	if (pc->c[0] == READ_6 && (sense[2] & 0x80)) {
 		pc->error = IDETAPE_ERROR_FILEMARK;
-		pc->flags |= PC_FLAG_ABORT;
+		drive->pc_flags |= PC_FLAG_ABORT;
 	}
 	if (pc->c[0] == WRITE_6) {
 		if ((sense[2] & 0x40) || (tape->sense_key == 0xd
 		     && tape->asc == 0x0 && tape->ascq == 0x2)) {
 			pc->error = IDETAPE_ERROR_EOD;
-			pc->flags |= PC_FLAG_ABORT;
+			drive->pc_flags |= PC_FLAG_ABORT;
 		}
 	}
 	if (pc->c[0] == READ_6 || pc->c[0] == WRITE_6) {
 		if (tape->sense_key == 8) {
 			pc->error = IDETAPE_ERROR_EOD;
-			pc->flags |= PC_FLAG_ABORT;
+			drive->pc_flags |= PC_FLAG_ABORT;
 		}
-		if (!(pc->flags & PC_FLAG_ABORT) &&
+		if (!(drive->pc_flags & PC_FLAG_ABORT) &&
 		    pc->xferred)
 			pc->retries = IDETAPE_MAX_PC_RETRIES + 1;
 	}
@@ -667,13 +667,13 @@ static ide_startstop_t idetape_issue_pc(ide_drive_t *drive,
 	drive->pc = pc;
 
 	if (pc->retries > IDETAPE_MAX_PC_RETRIES ||
-		(pc->flags & PC_FLAG_ABORT)) {
+		(drive->pc_flags & PC_FLAG_ABORT)) {
 		/*
 		 * We will "abort" retrying a packet command in case legitimate
 		 * error code was received (crossing a filemark, or end of the
 		 * media, for example).
 		 */
-		if (!(pc->flags & PC_FLAG_ABORT)) {
+		if (!(drive->pc_flags & PC_FLAG_ABORT)) {
 			if (!(pc->c[0] == TEST_UNIT_READY &&
 			      tape->sense_key == 2 && tape->asc == 4 &&
 			     (tape->ascq == 1 || tape->ascq == 8))) {
@@ -699,9 +699,11 @@ static ide_startstop_t idetape_issue_pc(ide_drive_t *drive,
 }
 
 /* A mode sense command is used to "sense" tape parameters. */
-static void idetape_create_mode_sense_cmd(struct ide_atapi_pc *pc, u8 page_code)
+static void idetape_create_mode_sense_cmd(ide_drive_t *drive,
+					  struct ide_atapi_pc *pc,
+					  u8 page_code)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = MODE_SENSE;
 	if (page_code != IDETAPE_BLOCK_DESCRIPTOR)
 		/* DBD = 1 - Don't return block descriptors */
@@ -753,14 +755,14 @@ static ide_startstop_t idetape_media_access_finished(ide_drive_t *drive)
 	return ide_stopped;
 }
 
-static void ide_tape_create_rw_cmd(idetape_tape_t *tape,
-				   struct ide_atapi_pc *pc, struct request *rq,
-				   u8 opcode)
+static void ide_tape_create_rw_cmd(ide_drive_t *drive, struct ide_atapi_pc *pc,
+				   struct request *rq, u8 opcode)
 {
+	idetape_tape_t *tape = drive->driver_data;
 	struct idetape_bh *bh = (struct idetape_bh *)rq->special;
 	unsigned int length = rq->current_nr_sectors;
 
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	put_unaligned(cpu_to_be32(length), (unsigned int *) &pc->c[1]);
 	pc->c[1] = 1;
 	pc->bh = bh;
@@ -768,7 +770,7 @@ static void ide_tape_create_rw_cmd(idetape_tape_t *tape,
 	pc->buf_size = length * tape->blk_size;
 	pc->req_xfer = pc->buf_size;
 	if (pc->req_xfer == tape->buffer_size)
-		pc->flags |= PC_FLAG_DMA_OK;
+		drive->pc_flags |= PC_FLAG_DMA_OK;
 
 	if (opcode == READ_6) {
 		pc->c[0] = READ_6;
@@ -859,12 +861,12 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 	}
 	if (rq->cmd[13] & REQ_IDETAPE_READ) {
 		pc = &tape->queued_pc;
-		ide_tape_create_rw_cmd(tape, pc, rq, READ_6);
+		ide_tape_create_rw_cmd(drive, pc, rq, READ_6);
 		goto out;
 	}
 	if (rq->cmd[13] & REQ_IDETAPE_WRITE) {
 		pc = &tape->queued_pc;
-		ide_tape_create_rw_cmd(tape, pc, rq, WRITE_6);
+		ide_tape_create_rw_cmd(drive, pc, rq, WRITE_6);
 		goto out;
 	}
 	if (rq->cmd[13] & REQ_IDETAPE_PC1) {
@@ -1050,10 +1052,10 @@ static void idetape_init_merge_buffer(idetape_tape_t *tape)
 static void idetape_create_write_filemark_cmd(ide_drive_t *drive,
 		struct ide_atapi_pc *pc, int write_filemark)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = WRITE_FILEMARKS;
 	pc->c[4] = write_filemark;
-	pc->flags |= PC_FLAG_WAIT_FOR_DSC;
+	drive->pc_flags |= PC_FLAG_WAIT_FOR_DSC;
 }
 
 static int idetape_wait_ready(ide_drive_t *drive, unsigned long timeout)
@@ -1098,9 +1100,10 @@ static int idetape_flush_tape_buffers(ide_drive_t *drive)
 	return 0;
 }
 
-static void idetape_create_read_position_cmd(struct ide_atapi_pc *pc)
+static void idetape_create_read_position_cmd(ide_drive_t *drive,
+					     struct ide_atapi_pc *pc)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = READ_POSITION;
 	pc->req_xfer = 20;
 }
@@ -1113,7 +1116,7 @@ static int idetape_read_position(ide_drive_t *drive)
 
 	debug_log(DBG_PROCS, "Enter %s\n", __func__);
 
-	idetape_create_read_position_cmd(&pc);
+	idetape_create_read_position_cmd(drive, &pc);
 	if (ide_queue_pc_tail(drive, tape->disk, &pc))
 		return -1;
 	position = tape->first_frame;
@@ -1124,12 +1127,12 @@ static void idetape_create_locate_cmd(ide_drive_t *drive,
 		struct ide_atapi_pc *pc,
 		unsigned int block, u8 partition, int skip)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = POSITION_TO_ELEMENT;
 	pc->c[1] = 2;
 	put_unaligned(cpu_to_be32(block), (unsigned int *) &pc->c[3]);
 	pc->c[8] = partition;
-	pc->flags |= PC_FLAG_WAIT_FOR_DSC;
+	drive->pc_flags |= PC_FLAG_WAIT_FOR_DSC;
 }
 
 static void __ide_tape_discard_merge_buffer(ide_drive_t *drive)
@@ -1171,7 +1174,7 @@ static int idetape_position_tape(ide_drive_t *drive, unsigned int block,
 	if (retval)
 		return (retval);
 
-	idetape_create_read_position_cmd(&pc);
+	idetape_create_read_position_cmd(drive, &pc);
 	return ide_queue_pc_tail(drive, disk, &pc);
 }
 
@@ -1230,37 +1233,40 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
 	return ret;
 }
 
-static void idetape_create_inquiry_cmd(struct ide_atapi_pc *pc)
+static void idetape_create_inquiry_cmd(ide_drive_t *drive,
+				       struct ide_atapi_pc *pc)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = INQUIRY;
 	pc->c[4] = 254;
 	pc->req_xfer = 254;
 }
 
 static void idetape_create_rewind_cmd(ide_drive_t *drive,
-		struct ide_atapi_pc *pc)
+				      struct ide_atapi_pc *pc)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = REZERO_UNIT;
-	pc->flags |= PC_FLAG_WAIT_FOR_DSC;
+	drive->pc_flags |= PC_FLAG_WAIT_FOR_DSC;
 }
 
-static void idetape_create_erase_cmd(struct ide_atapi_pc *pc)
+static void idetape_create_erase_cmd(ide_drive_t *drive,
+				     struct ide_atapi_pc *pc)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = ERASE;
 	pc->c[1] = 1;
-	pc->flags |= PC_FLAG_WAIT_FOR_DSC;
+	drive->pc_flags |= PC_FLAG_WAIT_FOR_DSC;
 }
 
-static void idetape_create_space_cmd(struct ide_atapi_pc *pc, int count, u8 cmd)
+static void idetape_create_space_cmd(ide_drive_t *drive,
+				     struct ide_atapi_pc *pc, int count, u8 cmd)
 {
-	ide_init_pc(pc);
+	ide_init_pc(drive, pc);
 	pc->c[0] = SPACE;
 	put_unaligned(cpu_to_be32(count), (unsigned int *) &pc->c[1]);
 	pc->c[1] = cmd;
-	pc->flags |= PC_FLAG_WAIT_FOR_DSC;
+	drive->pc_flags |= PC_FLAG_WAIT_FOR_DSC;
 }
 
 /* Queue up a character device originated write request. */
@@ -1431,7 +1437,7 @@ static int idetape_rewind_tape(ide_drive_t *drive)
 	if (retval)
 		return retval;
 
-	idetape_create_read_position_cmd(&pc);
+	idetape_create_read_position_cmd(drive, &pc);
 	retval = ide_queue_pc_tail(drive, disk, &pc);
 	if (retval)
 		return retval;
@@ -1498,7 +1504,7 @@ static int idetape_space_over_filemarks(ide_drive_t *drive, short mt_op,
 	switch (mt_op) {
 	case MTFSF:
 	case MTBSF:
-		idetape_create_space_cmd(&pc, mt_count - count,
+		idetape_create_space_cmd(drive, &pc, mt_count - count,
 					 IDETAPE_SPACE_OVER_FILEMARK);
 		return ide_queue_pc_tail(drive, disk, &pc);
 	case MTFSFM:
@@ -1778,11 +1784,11 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
 		return ide_do_start_stop(drive, disk,
 			IDETAPE_LU_RETENSION_MASK | IDETAPE_LU_LOAD_MASK);
 	case MTEOM:
-		idetape_create_space_cmd(&pc, 0, IDETAPE_SPACE_TO_EOD);
+		idetape_create_space_cmd(drive, &pc, 0, IDETAPE_SPACE_TO_EOD);
 		return ide_queue_pc_tail(drive, disk, &pc);
 	case MTERASE:
 		(void)idetape_rewind_tape(drive);
-		idetape_create_erase_cmd(&pc);
+		idetape_create_erase_cmd(drive, &pc);
 		return ide_queue_pc_tail(drive, disk, &pc);
 	case MTSETBLK:
 		if (mt_count) {
@@ -1891,7 +1897,7 @@ static void ide_tape_get_bsize_from_bdesc(ide_drive_t *drive)
 	idetape_tape_t *tape = drive->driver_data;
 	struct ide_atapi_pc pc;
 
-	idetape_create_mode_sense_cmd(&pc, IDETAPE_BLOCK_DESCRIPTOR);
+	idetape_create_mode_sense_cmd(drive, &pc, IDETAPE_BLOCK_DESCRIPTOR);
 	if (ide_queue_pc_tail(drive, tape->disk, &pc)) {
 		printk(KERN_ERR "ide-tape: Can't get block descriptor\n");
 		if (tape->blk_size == 0) {
@@ -2043,7 +2049,7 @@ static void idetape_get_inquiry_results(ide_drive_t *drive)
 	struct ide_atapi_pc pc;
 	char fw_rev[4], vendor_id[8], product_id[16];
 
-	idetape_create_inquiry_cmd(&pc);
+	idetape_create_inquiry_cmd(drive, &pc);
 	if (ide_queue_pc_tail(drive, tape->disk, &pc)) {
 		printk(KERN_ERR "ide-tape: %s: can't get INQUIRY results\n",
 				tape->name);
@@ -2072,7 +2078,7 @@ static void idetape_get_mode_sense_results(ide_drive_t *drive)
 	u8 *caps;
 	u8 speed, max_speed;
 
-	idetape_create_mode_sense_cmd(&pc, IDETAPE_CAPABILITIES_PAGE);
+	idetape_create_mode_sense_cmd(drive, &pc, IDETAPE_CAPABILITIES_PAGE);
 	if (ide_queue_pc_tail(drive, tape->disk, &pc)) {
 		printk(KERN_ERR "ide-tape: Can't get tape parameters - assuming"
 				" some default values\n");
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 2041073..57afe1f 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -654,6 +654,9 @@ struct ide_drive_s {
 	/* current packet command */
 	struct ide_atapi_pc *pc;
 
+	/* atapi pc flags: temporary */
+	unsigned long pc_flags;
+
 	/* callback for packet commands */
 	void (*pc_callback)(struct ide_drive_s *, int);
 
@@ -1215,7 +1218,7 @@ void ide_pktcmd_tf_load(ide_drive_t *, u32, u16, u8);
 
 int ide_check_atapi_device(ide_drive_t *, const char *);
 
-void ide_init_pc(struct ide_atapi_pc *);
+void ide_init_pc(ide_drive_t *, struct ide_atapi_pc *);
 
 /* Disk head parking */
 extern wait_queue_head_t ide_park_wq;
-- 
1.6.0.4

[PATCH 3/9] ide-atapi: replace pc->buf with rq->data in the irq handler

[Borislav Petkov]

... and also
1. pc->xferred with rq->data_len

2. add a dummy end request callback similar to ide-cd so that rqs don't get
killed while DRQ is still set

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c  |   41 +++++++++++++++++++++++++----------------
 drivers/ide/ide-floppy.c |    3 ++-
 drivers/ide/ide-tape.c   |   12 ++++++------
 3 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index ec29133..964a7e2 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -142,6 +142,8 @@ static void ide_queue_pc_head(ide_drive_t *drive, struct gendisk *disk,
 	memcpy(rq->cmd, pc->c, 12);
 	if (drive->media == ide_tape)
 		rq->cmd[13] = REQ_IDETAPE_PC1;
+	rq->data = pc->buf;
+	rq->data_len = 0;
 	ide_do_drive_cmd(drive, rq);
 }
 
@@ -161,6 +163,8 @@ int ide_queue_pc_tail(ide_drive_t *drive, struct gendisk *disk,
 	memcpy(rq->cmd, pc->c, 12);
 	if (drive->media == ide_tape)
 		rq->cmd[13] = REQ_IDETAPE_PC1;
+	rq->data = pc->buf;
+	rq->data_len = 0;
 	error = blk_execute_rq(drive->queue, disk, rq, 0);
 	blk_put_request(rq);
 
@@ -284,6 +288,11 @@ int ide_cd_get_xferlen(struct request *rq)
 }
 EXPORT_SYMBOL_GPL(ide_cd_get_xferlen);
 
+static int ide_floppy_irq_dummy_cb(struct request *rq)
+{
+	return 1;
+}
+
 /*
  * This is the usual interrupt handler which will be called during a packet
  * command.  We will transfer some of the data (as requested by the drive)
@@ -326,7 +335,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 						     ? "write" : "read");
 			drive->pc_flags |= PC_FLAG_DMA_ERROR;
 		} else {
-			pc->xferred = pc->req_xfer;
+			rq->data_len = pc->req_xfer;
 			if (drive->pc_update_buffers)
 				drive->pc_update_buffers(drive, pc);
 		}
@@ -336,7 +345,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 	/* No more interrupts */
 	if ((stat & ATA_DRQ) == 0) {
 		debug_log("Packet command completed, %d bytes transferred\n",
-			  pc->xferred);
+			  rq->data_len);
 
 		drive->pc_flags &= ~PC_FLAG_DMA_IN_PROGRESS;
 
@@ -406,9 +415,10 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 
 	if (!write) {
 		/* Reading - Check that we have enough space */
-		temp = pc->xferred + bcount;
+		temp = rq->data_len + bcount;
 		if (temp > pc->req_xfer) {
 			if (temp > pc->buf_size) {
+
 				printk(KERN_ERR "%s: The device wants to send "
 						"us more data than expected - "
 						"discarding data\n",
@@ -424,19 +434,22 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 	} else
 		xferfunc = tp_ops->output_data;
 
-	if ((drive->media == ide_floppy && !pc->buf) ||
+	if ((drive->media == ide_floppy && !rq->data) ||
 	    (drive->media == ide_tape && pc->bh)) {
-		int done = drive->pc_io_buffers(drive, pc, bcount, write);
 
-		/* FIXME: don't do partial completions */
+		drive->pc_io_buffers(drive, pc, bcount, write);
+
 		if (drive->media == ide_floppy)
-			ide_end_request(drive, 1, done >> 9);
-	} else
-		xferfunc(drive, NULL, pc->cur_pos, bcount);
+			blk_end_request_callback(rq, 0, bcount,
+						 ide_floppy_irq_dummy_cb);
 
-	/* Update the current position */
-	pc->xferred += bcount;
-	pc->cur_pos += bcount;
+	} else {
+		xferfunc(drive, NULL, rq->data, bcount);
+
+		/* Update the current position */
+		rq->data_len += bcount;
+		rq->data += bcount;
+	}
 
 	debug_log("[cmd %x] transferred %d bytes on that intr.\n",
 		  rq->cmd[0], bcount);
@@ -557,10 +570,6 @@ ide_startstop_t ide_issue_pc(ide_drive_t *drive, unsigned int timeout)
 	u32 tf_flags;
 	u16 bcount;
 
-	/* We haven't transferred any data yet */
-	pc->xferred = 0;
-	pc->cur_pos = pc->buf;
-
 	if (dev_is_idecd(drive)) {
 		tf_flags = IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL;
 		bcount = ide_cd_get_xferlen(hwif->hwgroup->rq);
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 44e5b8a..f88ccd1 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -256,7 +256,8 @@ static void idefloppy_create_rw_cmd(ide_drive_t *drive,
 
 	pc->rq = rq;
 	pc->b_count = 0;
-	pc->buf = NULL;
+	rq->data = NULL;
+	rq->data_len = 0;
 	pc->req_xfer = pc->buf_size = blocks * floppy->block_size;
 	drive->pc_flags |= PC_FLAG_DMA_OK;
 }
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index caccb82..1890f9f 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -375,7 +375,7 @@ static void idetape_update_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc)
 	struct idetape_bh *bh = pc->bh;
 	struct request *rq = drive->hwif->hwgroup->rq;
 	int count;
-	unsigned int bcount = pc->xferred;
+	unsigned int bcount = rq->data_len;
 
 	if (rq_data_dir(rq) == WRITE)
 		return;
@@ -401,6 +401,7 @@ static void idetape_update_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc)
 static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
 {
 	idetape_tape_t *tape = drive->driver_data;
+	struct request *rq = drive->hwif->hwgroup->rq;
 	struct ide_atapi_pc *pc = tape->failed_pc;
 
 	tape->sense_key = sense[2] & 0xF;
@@ -410,9 +411,9 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
 	debug_log(DBG_ERR, "pc = %x, sense key = %x, asc = %x, ascq = %x\n",
 		 pc->c[0], tape->sense_key, tape->asc, tape->ascq);
 
-	/* Correct pc->xferred by asking the tape.	 */
+	/* Correct rq->data_len by asking the tape.	 */
 	if (drive->pc_flags & PC_FLAG_DMA_ERROR) {
-		pc->xferred = pc->req_xfer -
+		rq->data_len = pc->req_xfer -
 			tape->blk_size *
 			get_unaligned_be32(&sense[3]);
 		idetape_update_buffers(drive, pc);
@@ -449,8 +450,7 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
 			pc->error = IDETAPE_ERROR_EOD;
 			drive->pc_flags |= PC_FLAG_ABORT;
 		}
-		if (!(drive->pc_flags & PC_FLAG_ABORT) &&
-		    pc->xferred)
+		if (!(drive->pc_flags & PC_FLAG_ABORT) && rq->data_len)
 			pc->retries = IDETAPE_MAX_PC_RETRIES + 1;
 	}
 }
@@ -533,7 +533,7 @@ static void ide_tape_callback(ide_drive_t *drive, int dsc)
 					"itself - Aborting request!\n");
 	} else if (pc->c[0] == READ_6 || pc->c[0] == WRITE_6) {
 		struct request *rq = drive->hwif->hwgroup->rq;
-		int blocks = pc->xferred / tape->blk_size;
+		int blocks = rq->data_len / tape->blk_size;
 
 		tape->avg_size += blocks * tape->blk_size;
 
-- 
1.6.0.4

[PATCH 4/9] ide-atapi: replace pc->req_xfer with drive->pc_req_xfer

[Borislav Petkov]

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c  |   14 +++++++-------
 drivers/ide/ide-floppy.c |    8 ++++----
 drivers/ide/ide-tape.c   |   16 ++++++++--------
 include/linux/ide.h      |    5 +++--
 4 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 964a7e2..1b4d157 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -123,6 +123,7 @@ void ide_init_pc(ide_drive_t *drive, struct ide_atapi_pc *pc)
 	pc->buf = pc->pc_buf;
 	pc->buf_size = IDE_PC_BUFFER_SIZE;
 	drive->pc_flags = 0;
+	drive->pc_req_xfer = 0;
 }
 EXPORT_SYMBOL_GPL(ide_init_pc);
 
@@ -219,10 +220,10 @@ void ide_create_request_sense_cmd(ide_drive_t *drive, struct ide_atapi_pc *pc)
 	pc->c[0] = REQUEST_SENSE;
 	if (drive->media == ide_floppy) {
 		pc->c[4] = 255;
-		pc->req_xfer = 18;
+		drive->pc_req_xfer = 18;
 	} else {
 		pc->c[4] = 20;
-		pc->req_xfer = 20;
+		drive->pc_req_xfer = 20;
 	}
 }
 EXPORT_SYMBOL_GPL(ide_create_request_sense_cmd);
@@ -335,7 +336,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 						     ? "write" : "read");
 			drive->pc_flags |= PC_FLAG_DMA_ERROR;
 		} else {
-			rq->data_len = pc->req_xfer;
+			rq->data_len = drive->pc_req_xfer;
 			if (drive->pc_update_buffers)
 				drive->pc_update_buffers(drive, pc);
 		}
@@ -416,7 +417,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 	if (!write) {
 		/* Reading - Check that we have enough space */
 		temp = rq->data_len + bcount;
-		if (temp > pc->req_xfer) {
+		if (temp > drive->pc_req_xfer) {
 			if (temp > pc->buf_size) {
 
 				printk(KERN_ERR "%s: The device wants to send "
@@ -564,7 +565,6 @@ static ide_startstop_t ide_transfer_pc(ide_drive_t *drive)
 
 ide_startstop_t ide_issue_pc(ide_drive_t *drive, unsigned int timeout)
 {
-	struct ide_atapi_pc *pc = drive->pc;
 	ide_hwif_t *hwif = drive->hwif;
 	ide_expiry_t *expiry = NULL;
 	u32 tf_flags;
@@ -577,8 +577,8 @@ ide_startstop_t ide_issue_pc(ide_drive_t *drive, unsigned int timeout)
 	} else {
 		tf_flags = IDE_TFLAG_OUT_DEVICE;
 		bcount = ((drive->media == ide_tape) ?
-				pc->req_xfer :
-				min(pc->req_xfer, 63 * 1024));
+				drive->pc_req_xfer :
+				min(drive->pc_req_xfer, 63 * 1024));
 	}
 
 	if (drive->pc_flags & PC_FLAG_DMA_ERROR) {
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index f88ccd1..99f0c49 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -207,7 +207,7 @@ void ide_floppy_create_read_capacity_cmd(ide_drive_t *drive,
 	pc->c[0] = GPCMD_READ_FORMAT_CAPACITIES;
 	pc->c[7] = 255;
 	pc->c[8] = 255;
-	pc->req_xfer = 255;
+	drive->pc_req_xfer = 255;
 }
 
 /* A mode sense command is used to "sense" floppy parameters. */
@@ -232,7 +232,7 @@ void ide_floppy_create_mode_sense_cmd(ide_drive_t *drive,
 		printk(KERN_ERR PFX "unsupported page code in %s\n", __func__);
 	}
 	put_unaligned(cpu_to_be16(length), (u16 *) &pc->c[7]);
-	pc->req_xfer = length;
+	drive->pc_req_xfer = length;
 }
 
 static void idefloppy_create_rw_cmd(ide_drive_t *drive,
@@ -258,7 +258,7 @@ static void idefloppy_create_rw_cmd(ide_drive_t *drive,
 	pc->b_count = 0;
 	rq->data = NULL;
 	rq->data_len = 0;
-	pc->req_xfer = pc->buf_size = blocks * floppy->block_size;
+	drive->pc_req_xfer = pc->buf_size = blocks * floppy->block_size;
 	drive->pc_flags |= PC_FLAG_DMA_OK;
 }
 
@@ -276,7 +276,7 @@ static void idefloppy_blockpc_cmd(ide_drive_t *drive, struct ide_atapi_pc *pc,
 	 * possibly problematic, doesn't look like ide-floppy correctly
 	 * handled scattered requests if dma fails...
 	 */
-	pc->req_xfer = pc->buf_size = rq->data_len;
+	drive->pc_req_xfer = pc->buf_size = rq->data_len;
 }
 
 static ide_startstop_t ide_floppy_do_request(ide_drive_t *drive,
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 1890f9f..35a41e2 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -413,7 +413,7 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
 
 	/* Correct rq->data_len by asking the tape.	 */
 	if (drive->pc_flags & PC_FLAG_DMA_ERROR) {
-		rq->data_len = pc->req_xfer -
+		rq->data_len = drive->pc_req_xfer -
 			tape->blk_size *
 			get_unaligned_be32(&sense[3]);
 		idetape_update_buffers(drive, pc);
@@ -720,11 +720,11 @@ static void idetape_create_mode_sense_cmd(ide_drive_t *drive,
 	/* We will just discard data in that case */
 	pc->c[4] = 255;
 	if (page_code == IDETAPE_BLOCK_DESCRIPTOR)
-		pc->req_xfer = 12;
+		drive->pc_req_xfer = 12;
 	else if (page_code == IDETAPE_CAPABILITIES_PAGE)
-		pc->req_xfer = 24;
+		drive->pc_req_xfer = 24;
 	else
-		pc->req_xfer = 50;
+		drive->pc_req_xfer = 50;
 }
 
 static ide_startstop_t idetape_media_access_finished(ide_drive_t *drive)
@@ -768,8 +768,8 @@ static void ide_tape_create_rw_cmd(ide_drive_t *drive, struct ide_atapi_pc *pc,
 	pc->bh = bh;
 	pc->buf = NULL;
 	pc->buf_size = length * tape->blk_size;
-	pc->req_xfer = pc->buf_size;
-	if (pc->req_xfer == tape->buffer_size)
+	drive->pc_req_xfer = pc->buf_size;
+	if (drive->pc_req_xfer == tape->buffer_size)
 		drive->pc_flags |= PC_FLAG_DMA_OK;
 
 	if (opcode == READ_6) {
@@ -1105,7 +1105,7 @@ static void idetape_create_read_position_cmd(ide_drive_t *drive,
 {
 	ide_init_pc(drive, pc);
 	pc->c[0] = READ_POSITION;
-	pc->req_xfer = 20;
+	drive->pc_req_xfer = 20;
 }
 
 static int idetape_read_position(ide_drive_t *drive)
@@ -1239,7 +1239,7 @@ static void idetape_create_inquiry_cmd(ide_drive_t *drive,
 	ide_init_pc(drive, pc);
 	pc->c[0] = INQUIRY;
 	pc->c[4] = 254;
-	pc->req_xfer = 254;
+	drive->pc_req_xfer = 254;
 }
 
 static void idetape_create_rewind_cmd(ide_drive_t *drive,
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 57afe1f..7874ac5 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -587,7 +587,7 @@ struct ide_drive_s {
 	struct request_queue	*queue;	/* request queue */
 
 	struct request		*rq;	/* current request */
-	struct ide_drive_s 	*next;	/* circular list of hwgroup drives */
+	struct ide_drive_s	*next;	/* circular list of hwgroup drives */
 	void		*driver_data;	/* extra driver data */
 	u16			*id;	/* identification info */
 #ifdef CONFIG_IDE_PROC_FS
@@ -654,8 +654,9 @@ struct ide_drive_s {
 	/* current packet command */
 	struct ide_atapi_pc *pc;
 
-	/* atapi pc flags: temporary */
+	/* atapi pc members: temporarily harbored here */
 	unsigned long pc_flags;
+	int pc_req_xfer;
 
 	/* callback for packet commands */
 	void (*pc_callback)(struct ide_drive_s *, int);
-- 
1.6.0.4

[PATCH 5/9] ide-atapi: replace pc->buf_size with drive->pc_buf_size

[Borislav Petkov]

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c        |    4 ++--
 drivers/ide/ide-floppy.c       |    4 ++--
 drivers/ide/ide-floppy_ioctl.c |    2 +-
 drivers/ide/ide-tape.c         |    4 ++--
 include/linux/ide.h            |    1 +
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 1b4d157..f299d8e 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -121,7 +121,7 @@ void ide_init_pc(ide_drive_t *drive, struct ide_atapi_pc *pc)
 {
 	memset(pc, 0, sizeof(*pc));
 	pc->buf = pc->pc_buf;
-	pc->buf_size = IDE_PC_BUFFER_SIZE;
+	drive->pc_buf_size = IDE_PC_BUFFER_SIZE;
 	drive->pc_flags = 0;
 	drive->pc_req_xfer = 0;
 }
@@ -418,7 +418,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 		/* Reading - Check that we have enough space */
 		temp = rq->data_len + bcount;
 		if (temp > drive->pc_req_xfer) {
-			if (temp > pc->buf_size) {
+			if (temp > drive->pc_buf_size) {
 
 				printk(KERN_ERR "%s: The device wants to send "
 						"us more data than expected - "
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 99f0c49..0977229 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -258,7 +258,7 @@ static void idefloppy_create_rw_cmd(ide_drive_t *drive,
 	pc->b_count = 0;
 	rq->data = NULL;
 	rq->data_len = 0;
-	drive->pc_req_xfer = pc->buf_size = blocks * floppy->block_size;
+	drive->pc_req_xfer = drive->pc_buf_size = blocks * floppy->block_size;
 	drive->pc_flags |= PC_FLAG_DMA_OK;
 }
 
@@ -276,7 +276,7 @@ static void idefloppy_blockpc_cmd(ide_drive_t *drive, struct ide_atapi_pc *pc,
 	 * possibly problematic, doesn't look like ide-floppy correctly
 	 * handled scattered requests if dma fails...
 	 */
-	drive->pc_req_xfer = pc->buf_size = rq->data_len;
+	drive->pc_req_xfer = drive->pc_buf_size = rq->data_len;
 }
 
 static ide_startstop_t ide_floppy_do_request(ide_drive_t *drive,
diff --git a/drivers/ide/ide-floppy_ioctl.c b/drivers/ide/ide-floppy_ioctl.c
index 88e14e4..1316271 100644
--- a/drivers/ide/ide-floppy_ioctl.c
+++ b/drivers/ide/ide-floppy_ioctl.c
@@ -109,7 +109,7 @@ static void ide_floppy_create_format_unit_cmd(ide_drive_t *drive,
 
 	put_unaligned(cpu_to_be32(b), (unsigned int *)(&pc->buf[4]));
 	put_unaligned(cpu_to_be32(l), (unsigned int *)(&pc->buf[8]));
-	pc->buf_size = 12;
+	drive->pc_buf_size = 12;
 }
 
 static int ide_floppy_get_sfrp_bit(ide_drive_t *drive, struct ide_atapi_pc *pc)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 35a41e2..fef9a78 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -767,8 +767,8 @@ static void ide_tape_create_rw_cmd(ide_drive_t *drive, struct ide_atapi_pc *pc,
 	pc->c[1] = 1;
 	pc->bh = bh;
 	pc->buf = NULL;
-	pc->buf_size = length * tape->blk_size;
-	drive->pc_req_xfer = pc->buf_size;
+	drive->pc_buf_size = length * tape->blk_size;
+	drive->pc_req_xfer = drive->pc_buf_size;
 	if (drive->pc_req_xfer == tape->buffer_size)
 		drive->pc_flags |= PC_FLAG_DMA_OK;
 
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 7874ac5..dac382b 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -657,6 +657,7 @@ struct ide_drive_s {
 	/* atapi pc members: temporarily harbored here */
 	unsigned long pc_flags;
 	int pc_req_xfer;
+	int pc_buf_size;
 
 	/* callback for packet commands */
 	void (*pc_callback)(struct ide_drive_s *, int);
-- 
1.6.0.4

[PATCH 6/9] ide-atapi: remove pc arg to drive->pc_io_buffers

[Borislav Petkov]

... and get the ptr from drive->pc.

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c |    6 +++---
 drivers/ide/ide-tape.c  |   16 ++++++++--------
 include/linux/ide.h     |    5 ++---
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index f299d8e..89ce8f0 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -68,9 +68,9 @@ int ide_check_atapi_device(ide_drive_t *drive, const char *s)
 EXPORT_SYMBOL_GPL(ide_check_atapi_device);
 
 /* PIO data transfer routine using the scatter gather table. */
-int ide_io_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
-		    unsigned int bcount, int write)
+int ide_io_buffers(ide_drive_t *drive, unsigned int bcount, int write)
 {
+	struct ide_atapi_pc *pc = drive->pc;
 	ide_hwif_t *hwif = drive->hwif;
 	const struct ide_tp_ops *tp_ops = hwif->tp_ops;
 	xfer_func_t *xf = write ? tp_ops->output_data : tp_ops->input_data;
@@ -438,7 +438,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 	if ((drive->media == ide_floppy && !rq->data) ||
 	    (drive->media == ide_tape && pc->bh)) {
 
-		drive->pc_io_buffers(drive, pc, bcount, write);
+		drive->pc_io_buffers(drive, bcount, write);
 
 		if (drive->media == ide_floppy)
 			blk_end_request_callback(rq, 0, bcount,
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index fef9a78..2bfa612 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -313,9 +313,9 @@ static struct ide_tape_obj *ide_tape_chrdev_get(unsigned int i)
 	return tape;
 }
 
-static void idetape_input_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
-				  unsigned int bcount)
+static void idetape_input_buffers(ide_drive_t *drive, unsigned int bcount)
 {
+	struct ide_atapi_pc *pc = drive->pc;
 	struct idetape_bh *bh = pc->bh;
 	int count;
 
@@ -342,9 +342,9 @@ static void idetape_input_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
 	pc->bh = bh;
 }
 
-static void idetape_output_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
-				   unsigned int bcount)
+static void idetape_output_buffers(ide_drive_t *drive, unsigned int bcount)
 {
+	struct ide_atapi_pc *pc = drive->pc;
 	struct idetape_bh *bh = pc->bh;
 	int count;
 
@@ -601,13 +601,13 @@ static void ide_tape_handle_dsc(ide_drive_t *drive)
 	idetape_postpone_request(drive);
 }
 
-static int ide_tape_io_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
-				unsigned int bcount, int write)
+static int ide_tape_io_buffers(ide_drive_t *drive, unsigned int bcount,
+			       int write)
 {
 	if (write)
-		idetape_output_buffers(drive, pc, bcount);
+		idetape_output_buffers(drive, bcount);
 	else
-		idetape_input_buffers(drive, pc, bcount);
+		idetape_input_buffers(drive, bcount);
 
 	return bcount;
 }
diff --git a/include/linux/ide.h b/include/linux/ide.h
index dac382b..0940db9 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -663,8 +663,7 @@ struct ide_drive_s {
 	void (*pc_callback)(struct ide_drive_s *, int);
 
 	void (*pc_update_buffers)(struct ide_drive_s *, struct ide_atapi_pc *);
-	int  (*pc_io_buffers)(struct ide_drive_s *, struct ide_atapi_pc *,
-			      unsigned int, int);
+	int  (*pc_io_buffers)(struct ide_drive_s *, unsigned int, int);
 
 	unsigned long atapi_flags;
 
@@ -1206,7 +1205,7 @@ void ide_tf_read(ide_drive_t *, ide_task_t *);
 void ide_input_data(ide_drive_t *, struct request *, void *, unsigned int);
 void ide_output_data(ide_drive_t *, struct request *, void *, unsigned int);
 
-int ide_io_buffers(ide_drive_t *, struct ide_atapi_pc *, unsigned int, int);
+int ide_io_buffers(ide_drive_t *, unsigned int, int);
 
 extern void SELECT_DRIVE(ide_drive_t *);
 void SELECT_MASK(ide_drive_t *, int);
-- 
1.6.0.4

[PATCH 7/9] ide-atapi: use rq pointer directly instead of pc->rq deref

[Borislav Petkov]

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 89ce8f0..e514b47 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -332,7 +332,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 		    (drive->media == ide_tape && (stat & ATA_ERR))) {
 			if (drive->media == ide_floppy)
 				printk(KERN_ERR "%s: DMA %s error\n",
-					drive->name, rq_data_dir(pc->rq)
+					drive->name, rq_data_dir(rq)
 						     ? "write" : "read");
 			drive->pc_flags |= PC_FLAG_DMA_ERROR;
 		} else {
@@ -361,7 +361,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 			debug_log("%s: I/O error\n", drive->name);
 
 			if (drive->media != ide_tape)
-				pc->rq->errors++;
+				rq->errors++;
 
 			if (rq->cmd[0] == REQUEST_SENSE) {
 				printk(KERN_ERR "%s: I/O error in request sense"
-- 
1.6.0.4

[PATCH 8/9] ide-atapi: replace pc->error with drive->pc_error

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c  |    3 ++-
 drivers/ide/ide-floppy.c |    6 +++---
 drivers/ide/ide-tape.c   |   20 ++++++++++----------
 include/linux/ide.h      |    1 +
 4 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index e514b47..160d085 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -124,6 +124,7 @@ void ide_init_pc(ide_drive_t *drive, struct ide_atapi_pc *pc)
 	drive->pc_buf_size = IDE_PC_BUFFER_SIZE;
 	drive->pc_flags = 0;
 	drive->pc_req_xfer = 0;
+	drive->pc_error = 0;
 }
 EXPORT_SYMBOL_GPL(ide_init_pc);
 
@@ -377,7 +378,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 			/* queued, but not started */
 			return ide_stopped;
 		}
-		pc->error = 0;
+		drive->pc_error = 0;
 
 		if ((drive->pc_flags & PC_FLAG_WAIT_FOR_DSC) &&
 		    (stat & ATA_DSC) == 0)
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 0977229..3d6345e 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -119,7 +119,7 @@ static void ide_floppy_callback(ide_drive_t *drive, int dsc)
 {
 	struct ide_disk_obj *floppy = drive->driver_data;
 	struct ide_atapi_pc *pc = drive->pc;
-	int uptodate = pc->error ? 0 : 1;
+	int uptodate = drive->pc_error ? 0 : 1;
 
 	ide_debug_log(IDE_DBG_FUNC, "Call %s\n", __func__);
 
@@ -132,7 +132,7 @@ static void ide_floppy_callback(ide_drive_t *drive, int dsc)
 	else if (pc->c[0] == GPCMD_REQUEST_SENSE) {
 		u8 *buf = pc->buf;
 
-		if (!pc->error) {
+		if (!drive->pc_error) {
 			floppy->sense_key = buf[2] & 0x0F;
 			floppy->asc = buf[12];
 			floppy->ascq = buf[13];
@@ -186,7 +186,7 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
 		if (!(drive->pc_flags & PC_FLAG_SUPPRESS_ERROR))
 			ide_floppy_report_error(floppy, pc);
 		/* Giving up */
-		pc->error = IDEFLOPPY_ERROR_GENERAL;
+		drive->pc_error = IDEFLOPPY_ERROR_GENERAL;
 
 		floppy->failed_pc = NULL;
 		drive->pc_callback(drive, 0);
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 2bfa612..015b417 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -429,25 +429,25 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
 	    && pc->c[4] == 0 && pc->c[3] == 0 && pc->c[2] == 0) {
 		if (tape->sense_key == 5) {
 			/* don't report an error, everything's ok */
-			pc->error = 0;
+			drive->pc_error = 0;
 			/* don't retry read/write */
 			drive->pc_flags |= PC_FLAG_ABORT;
 		}
 	}
 	if (pc->c[0] == READ_6 && (sense[2] & 0x80)) {
-		pc->error = IDETAPE_ERROR_FILEMARK;
+		drive->pc_error = IDETAPE_ERROR_FILEMARK;
 		drive->pc_flags |= PC_FLAG_ABORT;
 	}
 	if (pc->c[0] == WRITE_6) {
 		if ((sense[2] & 0x40) || (tape->sense_key == 0xd
 		     && tape->asc == 0x0 && tape->ascq == 0x2)) {
-			pc->error = IDETAPE_ERROR_EOD;
+			drive->pc_error = IDETAPE_ERROR_EOD;
 			drive->pc_flags |= PC_FLAG_ABORT;
 		}
 	}
 	if (pc->c[0] == READ_6 || pc->c[0] == WRITE_6) {
 		if (tape->sense_key == 8) {
-			pc->error = IDETAPE_ERROR_EOD;
+			drive->pc_error = IDETAPE_ERROR_EOD;
 			drive->pc_flags |= PC_FLAG_ABORT;
 		}
 		if (!(drive->pc_flags & PC_FLAG_ABORT) && rq->data_len)
@@ -515,7 +515,7 @@ static void ide_tape_callback(ide_drive_t *drive, int dsc)
 {
 	idetape_tape_t *tape = drive->driver_data;
 	struct ide_atapi_pc *pc = drive->pc;
-	int uptodate = pc->error ? 0 : 1;
+	int uptodate = drive->pc_error ? 0 : 1;
 
 	debug_log(DBG_PROCS, "Enter %s\n", __func__);
 
@@ -547,8 +547,8 @@ static void ide_tape_callback(ide_drive_t *drive, int dsc)
 		tape->first_frame += blocks;
 		rq->current_nr_sectors -= blocks;
 
-		if (pc->error)
-			uptodate = pc->error;
+		if (drive->pc_error)
+			uptodate = drive->pc_error;
 	} else if (pc->c[0] == READ_POSITION && uptodate) {
 		u8 *readpos = pc->buf;
 
@@ -685,7 +685,7 @@ static ide_startstop_t idetape_issue_pc(ide_drive_t *drive,
 						tape->ascq);
 			}
 			/* Giving up */
-			pc->error = IDETAPE_ERROR_GENERAL;
+			drive->pc_error = IDETAPE_ERROR_GENERAL;
 		}
 		tape->failed_pc = NULL;
 		drive->pc_callback(drive, 0);
@@ -746,9 +746,9 @@ static ide_startstop_t idetape_media_access_finished(ide_drive_t *drive)
 			ide_retry_pc(drive, tape->disk);
 			return ide_stopped;
 		}
-		pc->error = 0;
+		drive->pc_error = 0;
 	} else {
-		pc->error = IDETAPE_ERROR_GENERAL;
+		drive->pc_error = IDETAPE_ERROR_GENERAL;
 		tape->failed_pc = NULL;
 	}
 	drive->pc_callback(drive, 0);
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 0940db9..291600c 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -658,6 +658,7 @@ struct ide_drive_s {
 	unsigned long pc_flags;
 	int pc_req_xfer;
 	int pc_buf_size;
+	int pc_error;
 
 	/* callback for packet commands */
 	void (*pc_callback)(struct ide_drive_s *, int);
-- 
1.6.0.4

[PATCH 9/9] ide-atapi: remove pc arg to drive->pc_update_buffers

[Borislav Petkov]

... however, since this is called with different pc's in ide-tape also, use a
temporary int arg to differentiate between the call sites.

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c  |    2 +-
 drivers/ide/ide-floppy.c |    4 ++--
 drivers/ide/ide-tape.c   |   15 ++++++++++++---
 include/linux/ide.h      |    2 +-
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 160d085..e83f5e3 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -339,7 +339,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 		} else {
 			rq->data_len = drive->pc_req_xfer;
 			if (drive->pc_update_buffers)
-				drive->pc_update_buffers(drive, pc);
+				drive->pc_update_buffers(drive, 0);
 		}
 		debug_log("%s: DMA finished\n", drive->name);
 	}
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 3d6345e..6dd3e8b 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -105,9 +105,9 @@ static int ide_floppy_end_request(ide_drive_t *drive, int uptodate, int nsecs)
 	return 0;
 }
 
-static void idefloppy_update_buffers(ide_drive_t *drive,
-				struct ide_atapi_pc *pc)
+static void idefloppy_update_buffers(ide_drive_t *drive, unsigned int which_pc)
 {
+	struct ide_atapi_pc *pc = drive->pc;
 	struct request *rq = pc->rq;
 	struct bio *bio = rq->bio;
 
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 015b417..94fc3e6 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -370,13 +370,22 @@ static void idetape_output_buffers(ide_drive_t *drive, unsigned int bcount)
 	}
 }
 
-static void idetape_update_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc)
+static void idetape_update_buffers(ide_drive_t *drive, unsigned int which_pc)
 {
-	struct idetape_bh *bh = pc->bh;
+	struct idetape_bh *bh;
+	struct ide_atapi_pc *pc;
+	idetape_tape_t *tape = drive->driver_data;
 	struct request *rq = drive->hwif->hwgroup->rq;
 	int count;
 	unsigned int bcount = rq->data_len;
 
+	if (which_pc)
+		pc = tape->failed_pc;
+	else
+		pc = drive->pc;
+
+	bh = pc->bh;
+
 	if (rq_data_dir(rq) == WRITE)
 		return;
 	while (bcount) {
@@ -416,7 +425,7 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
 		rq->data_len = drive->pc_req_xfer -
 			tape->blk_size *
 			get_unaligned_be32(&sense[3]);
-		idetape_update_buffers(drive, pc);
+		idetape_update_buffers(drive, 1);
 	}
 
 	/*
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 291600c..ac4d30f 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -663,7 +663,7 @@ struct ide_drive_s {
 	/* callback for packet commands */
 	void (*pc_callback)(struct ide_drive_s *, int);
 
-	void (*pc_update_buffers)(struct ide_drive_s *, struct ide_atapi_pc *);
+	void (*pc_update_buffers)(struct ide_drive_s *, unsigned int);
 	int  (*pc_io_buffers)(struct ide_drive_s *, unsigned int, int);
 
 	unsigned long atapi_flags;
-- 
1.6.0.4

Re: [PATCH 0/9] ide-atapi: remove ide_atapi_pc from the irq handler

[Bartlomiej Zolnierkiewicz]

Hi,

On Tuesday 16 December 2008, Borislav Petkov wrote:
> Hi Bart,
> 
> here's a first attempt at removing all references to ide_atapi_pc in the ATAPI
> IRQ handler. I've moved some of the members to the drive struct and will deal
> with them later :). The next step is to add an ide_atapi_queue_pc() routine

Hmm.  I recall discussing this before and having some concerns about struct
ide_atapi_pc removal idea probably being premature...

Looking at the patchset itself -- I'm very sorry but I just cannot apply it
with all these command specific fields going into ide_drive_t:

        unsigned long pc_flags;
        int pc_req_xfer;
        int pc_buf_size;
        int pc_error;

While they may look similar to the existing pc_* fields:

        /* callback for packet commands */
        void (*pc_callback)(struct ide_drive_s *, int);

        void (*pc_update_buffers)(struct ide_drive_s *, struct ide_atapi_pc *);
        int  (*pc_io_buffers)(struct ide_drive_s *, struct ide_atapi_pc *,
                              unsigned int, int);

they are clearly not.  Existing pc_* callbacks are per-device type (they
should have been probably named drv_*) and new ones are per-command ones.

I also don't see a way for code to differentiate between pc and failed_pc.

Moreover if we would like to ever add support for multiple queued commands
(actually I think that in a very limited way ide-tape does it already with
DSC) we would have to completely revert these changes.

Re-assuming: this is one-step forward / one-step back approach and not
exactly the kind of design that we would like to advocate.

> similar to ide_cd_queue_pc() and then rewrite all functions in the drivers to
> use struct requests and local buffers instead of pc->buf and then, after that
> works reliably, finally get rid of ide_atapi_pc completely.

Why not take care of low hanging fruits first?  There is still plenty...

- ide-cd uses private struct request for REQUEST SENSE (=> ide_drive_t's)

- it should be possible to port ide-cd to use ide_{issue,transfer}_pc()
  _without_ dealing with struct ide_atapi_pc issue at all

- why not investigate the possibility of porting ide-cd to using sg-s for
  PIO transfers first -- this should get rid of ide_cd_restore_request()
  and most of other rq fields manipulations

...and this is from taking not so long look at the current driver.

Why attack the "hard" problem first (struct ide_atapi_pc one) while there
are easier ones to deal with?  It may happen that once these easier ones
are fixed we will be able to view the "hard" one from a completely different
perspective (because code's complexity will decrease a lot in the meantime
it no longer will be a "hard" problem) and apply a better solution.

Besides I'm not fully convinced that struct ide_atapi_pc has to go first in
order to port ide-cd over ide-atapi -- we can just instead port ide-cd to
use struct ide_atapi_pc (however probably only after converting the driver
to use sg-s for PIO transfers -- otherwise it becomes another "hard" problem)
and then deal with struct ide_atapi_pc for all ATAPI code (be using struct
request fields directly or by merging struct ide_atapi_pc into ide_task_t
and always using the latter structure for ATA[PI] commands -- which is the
option that I personally came to prefer lately)...

Thanks,
Bart

Re: [PATCH 0/9] ide-atapi: remove ide_atapi_pc from the irq handler

[Borislav Petkov]

Hi,

> Hmm.  I recall discussing this before and having some concerns about struct
> ide_atapi_pc removal idea probably being premature...
> 
> Looking at the patchset itself -- I'm very sorry but I just cannot apply it
> with all these command specific fields going into ide_drive_t:
> 
>         unsigned long pc_flags;
>         int pc_req_xfer;
>         int pc_buf_size;
>         int pc_error;
> 
> While they may look similar to the existing pc_* fields:
> 
>         /* callback for packet commands */
>         void (*pc_callback)(struct ide_drive_s *, int);
> 
>         void (*pc_update_buffers)(struct ide_drive_s *, struct ide_atapi_pc *);
>         int  (*pc_io_buffers)(struct ide_drive_s *, struct ide_atapi_pc *,
>                               unsigned int, int);
> 
> they are clearly not.  Existing pc_* callbacks are per-device type (they
> should have been probably named drv_*) and new ones are per-command ones.

maybe I didn't express myself clear in the commit messages - those
fields are only temporary - the emphasis being on temporary - and was
going to remove them after the dust settles.

> I also don't see a way for code to differentiate between pc and failed_pc.

This happens only in once place in ide-tape and this is taken care of rather
clumsily with an additional flag.

> Why attack the "hard" problem first (struct ide_atapi_pc one) while there
> are easier ones to deal with?  It may happen that once these easier ones
> are fixed we will be able to view the "hard" one from a completely different
> perspective (because code's complexity will decrease a lot in the meantime
> it no longer will be a "hard" problem) and apply a better solution.
> 
> Besides I'm not fully convinced that struct ide_atapi_pc has to go first in
> order to port ide-cd over ide-atapi -- we can just instead port ide-cd to
> use struct ide_atapi_pc (however probably only after converting the driver
> to use sg-s for PIO transfers -- otherwise it becomes another "hard" problem)
> and then deal with struct ide_atapi_pc for all ATAPI code (be using struct
> request fields directly or by merging struct ide_atapi_pc into ide_task_t
> and always using the latter structure for ATA[PI] commands -- which is the
> option that I personally came to prefer lately)...

Here are the problems i see with ide_atapi_pc, judging by my somewhat limited
experience:

1. ide_issue_pc/ide_transfer_pc and all those other ATAPI functions have to
check the pc pointer depending on being entered from ide-cd or from the other
drivers, which means that either ide-cd is ported to use ide_atapi_pc's or we
have lots of spaghetti if/else checks.

2. But, it really seems too unnatural to have ide_atapi_pc structs representing
an ATAPI cmd, IMHO, and doing all those memcpy(rq->cmd, pc->c, BLK_MAX_CDB) and
generally mapping the atapi_pc's to an rq - I'd rather much prefer to use the
rqs which are pretty much sufficient for most tasks and have maybe a smaller
struct which is in rq->buffer or similar handling all the ATAPI specific things
like pc->req_xfer and such.

3. I also think that it is kinda not so smart to have a buffer in the atapi_pc
and move it all around. True, Linus removed some of the allocations on stack but
ide-floppy used to be one of the biggest stack users, ide-tape might still be,
haven't checked.

So, to sum up, I think that ide-cd and especially all the logic around
ide_cd_queue_pc and its users is much more cleaner, IMHO, than what
ide-floppy/tape do by saying

ide_init_pc()
ide_create_bla_cmd()
ide_queue_pc_head/tail() /* here you unnecessarily map the pc to an rq */
ide_issue_pc()

and, in most cases, ide_create_bla_cmd() is used only in one place so
it gets inlined by gcc. So, I think using private buffers in all those
get_capacity/check_status/read_tocentry and sending them down through an
rq is the cleaner solution because you do

cdrom_check_status()
ide_cd_queue_pc()

and this might become

ide_atapi_check_status()
ide_atapi_queue_pc()

and that's pretty awesome, don't you think?

p.s. I'm sure there's something else I'm forgetting.

-- 
Regards/Gruss,
    Boris.

Re: [PATCH 0/9] ide-atapi: remove ide_atapi_pc from the irq handler

[Bartlomiej Zolnierkiewicz]

Hi,

On Wednesday 17 December 2008, Borislav Petkov wrote:
> Hi,
> 
> > Hmm.  I recall discussing this before and having some concerns about struct
> > ide_atapi_pc removal idea probably being premature...
> > 
> > Looking at the patchset itself -- I'm very sorry but I just cannot apply it
> > with all these command specific fields going into ide_drive_t:
> > 
> >         unsigned long pc_flags;
> >         int pc_req_xfer;
> >         int pc_buf_size;
> >         int pc_error;
> > 
> > While they may look similar to the existing pc_* fields:
> > 
> >         /* callback for packet commands */
> >         void (*pc_callback)(struct ide_drive_s *, int);
> > 
> >         void (*pc_update_buffers)(struct ide_drive_s *, struct ide_atapi_pc *);
> >         int  (*pc_io_buffers)(struct ide_drive_s *, struct ide_atapi_pc *,
> >                               unsigned int, int);
> > 
> > they are clearly not.  Existing pc_* callbacks are per-device type (they
> > should have been probably named drv_*) and new ones are per-command ones.
> 
> maybe I didn't express myself clear in the commit messages - those
> fields are only temporary - the emphasis being on temporary - and was
> going to remove them after the dust settles.

We accept temporary / hacky solutions as an exception and only when there is
a justified need, i.e. it fixes real user problem and we are close to release
date, or it is a standalone issue in a newly merged driver and we don't want
to delay it more from being used by users just because of it.  This is not
the case here.  The alone fact that you need such invasive temporary changes
is a hint that there is a problem with either the implemention or the proposed
solution.

Looking once again at the patchset, it doesn't seem to remove a single struct
ide_atapi_pc field.  The problem is just moved from one place into the other
introducing new design issues, clearly not an improvement.  This again seems
to come back to the approach taken for removing struct ide_atapi_pc (which is
a "hard" problem currently) -- if we stop being so focused on the idea that
struct ide_atapi_pc has to be removed _now_ at all cost and instead look into
possibilities of solving smaller problems, the big one may solve itself in
the meantime...

[ That being said it still good that you've posted this patchset early so we
  can still discuss it and potentially choose the other way before investing
  too much time into it. ]

> > I also don't see a way for code to differentiate between pc and failed_pc.
> 
> This happens only in once place in ide-tape and this is taken care of rather
> clumsily with an additional flag.

Unfortunately no.  Sorry but upon closer look the new code is just broken.

Before the patchset we had separate fields (i.e. pc_flags) for drive->pc
and floppy->failed_pc.  After the changes we have a _single_ pc_flags for
both REQUEST SENSE packet command and the original failed packet command.

[ ->failed_pc is used to keep pointer to the failed packet command while
  the driver is issuing REQUEST SENSE pc to the device (=> drive->pc now
  points to REQUEST SENSE pc) ]

Since REQUEST SENSE pc is using exactly the same pc issue code path as any
other pc [ ide_issue_pc() ] the code will be incorrectly checking pc_flags
of the original pc (i.e. original pc may have PC_FLAG_DMA_OK set which will
be incorrect for REQUEST SENSE pc).

> > Why attack the "hard" problem first (struct ide_atapi_pc one) while there
> > are easier ones to deal with?  It may happen that once these easier ones
> > are fixed we will be able to view the "hard" one from a completely different
> > perspective (because code's complexity will decrease a lot in the meantime
> > it no longer will be a "hard" problem) and apply a better solution.
> > 
> > Besides I'm not fully convinced that struct ide_atapi_pc has to go first in
> > order to port ide-cd over ide-atapi -- we can just instead port ide-cd to
> > use struct ide_atapi_pc (however probably only after converting the driver
> > to use sg-s for PIO transfers -- otherwise it becomes another "hard" problem)
> > and then deal with struct ide_atapi_pc for all ATAPI code (be using struct
> > request fields directly or by merging struct ide_atapi_pc into ide_task_t
> > and always using the latter structure for ATA[PI] commands -- which is the
> > option that I personally came to prefer lately)...
> 
> Here are the problems i see with ide_atapi_pc, judging by my somewhat limited
> experience:
> 
> 1. ide_issue_pc/ide_transfer_pc and all those other ATAPI functions have to
> check the pc pointer depending on being entered from ide-cd or from the other
> drivers, which means that either ide-cd is ported to use ide_atapi_pc's or we
> have lots of spaghetti if/else checks.

Or use mixed approach.

> 2. But, it really seems too unnatural to have ide_atapi_pc structs representing
> an ATAPI cmd, IMHO, and doing all those memcpy(rq->cmd, pc->c, BLK_MAX_CDB) and
> generally mapping the atapi_pc's to an rq - I'd rather much prefer to use the
> rqs which are pretty much sufficient for most tasks and have maybe a smaller
> struct which is in rq->buffer or similar handling all the ATAPI specific things
> like pc->req_xfer and such.

Well, we don't have to port ide-cd over struct ide_atapi_pc completely.

Moreover we can be fixing other drivers as we go (BTW I thought that all ATAPI
drivers have been already fixed WRT s/pc->c/rq->cmd/).

> 3. I also think that it is kinda not so smart to have a buffer in the atapi_pc
> and move it all around. True, Linus removed some of the allocations on stack but
> ide-floppy used to be one of the biggest stack users, ide-tape might still be,
> haven't checked.

Yes, we have to be very careful with using private buffers by not increasing
stack usage too much.

> So, to sum up, I think that ide-cd and especially all the logic around
> ide_cd_queue_pc and its users is much more cleaner, IMHO, than what
> ide-floppy/tape do by saying
> 
> ide_init_pc()
> ide_create_bla_cmd()
> ide_queue_pc_head/tail() /* here you unnecessarily map the pc to an rq */
> ide_issue_pc()
> 
> and, in most cases, ide_create_bla_cmd() is used only in one place so
> it gets inlined by gcc. So, I think using private buffers in all those
> get_capacity/check_status/read_tocentry and sending them down through an
> rq is the cleaner solution because you do
> 
> cdrom_check_status()
> ide_cd_queue_pc()
> 
> and this might become
> 
> ide_atapi_check_status()
> ide_atapi_queue_pc()
> 
> and that's pretty awesome, don't you think?

Sure and I didn't question the goal.

What I was asking in my mail was for you to take your ide-cd maintainer hat
off for a minute and instead put IDE maintainer one on.  From the perspective
of the whole subsystem porting ide-cd over to ide-atapi is of much higher
importance than getting rid of struct ide_atapi_pc.  The former can be pretty
much done now and shouldn't really be difficult problem once you start working
towards solving it.  Porting ide-cd over ide-atapi will decrease the amount
of all ATAPI code and its complexity immediately enabling the possibility of
other improvements inside IDE subsystem itself (right now they would require
ide-cd specific changes), which in turn should give a feedback loop into
ide-cd which may as well help solving the other issue (struct ide_atapi_pc
removal).  OTOH having struct ide_atapi_pc removed just for the sake of it
alone wouldn't give us much benetifs, especially looking at the potential
time it will take to done properly.

[ Yes, I'm a bit disappointed that 2.6.29 is going to be the next release
  that we will miss on ide-cd -> ide-atapi conversion (I was initially
  hoping for .28) and the fact that in few weeks this will become a limiting
  factor to the further progress on core IDE changes (despite few issues
  to still take care of things are starting to look really well there)... ]

Thanks,
Bart

Re: [PATCH 0/9] ide-atapi: remove ide_atapi_pc from the irq handler

[Borislav Petkov]

Hi,

> [ Yes, I'm a bit disappointed that 2.6.29 is going to be the next release
>   that we will miss on ide-cd -> ide-atapi conversion (I was initially
>   hoping for .28) and the fact that in few weeks this will become a limiting
>   factor to the further progress on core IDE changes (despite few issues
>   to still take care of things are starting to look really well there)... ]

ok, I get the idea, and I'm sorry for mis-prioritizing the things we
have to work on but it's not like I've been idling - as you're well
aware, we had a couple of nasty bugs in the past months which had to be
taken care of.

Anyways, I don't think it is too late yet for .29 since the merge window
is not open yet and I might whip up some patches in the next couple of
days...

-- 
Regards/Gruss,
    Boris.