[PATCH 00/21] ide-floppy redux v2

[PATCH 00/21] ide-floppy redux v2

[Borislav Petkov]

Hi Bart,

   here's the second version of the ide-floppy refactoring trail. All the
patches are based on the version of your quilt tree from the 05.01. Also, you've
already applied patch 5 in this series but i'm submitting it still for the sake
of completeness.


 drivers/ide/ide-cd.c     |    2 -
 drivers/ide/ide-floppy.c | 1403 ++++++++++++++++++----------------------------
 include/linux/cdrom.h    |    1 +
 include/linux/ide.h      |    3 +
 4 files changed, 558 insertions(+), 851 deletions(-)

p.s. Next stop: ide-tape.c :)

[PATCH 01/21] ide-floppy: convert to generic packet commands

[Borislav Petkov]

Replace the ide-floppy packet commands opcode defines with the generic ones.
Add a missing GPCMD_WRITE_12 (opcode 0xaa) to the generic ones in cdrom.h. The
last one can be found in the current version of INF-8090, p.905.

CC: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   44 ++++++++++++--------------------------------
 include/linux/cdrom.h    |    1 +
 2 files changed, 13 insertions(+), 32 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 3512637..e4ebb21 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -273,26 +273,6 @@ typedef struct ide_floppy_obj {
 #define IDEFLOPPY_ZIP_DRIVE		5	/* Requires BH algorithm for packets */
 
 /*
- *	ATAPI floppy drive packet commands
- */
-#define IDEFLOPPY_FORMAT_UNIT_CMD	0x04
-#define IDEFLOPPY_INQUIRY_CMD		0x12
-#define IDEFLOPPY_MODE_SELECT_CMD	0x55
-#define IDEFLOPPY_MODE_SENSE_CMD	0x5a
-#define IDEFLOPPY_READ10_CMD		0x28
-#define IDEFLOPPY_READ12_CMD		0xa8
-#define IDEFLOPPY_READ_CAPACITY_CMD	0x23
-#define IDEFLOPPY_REQUEST_SENSE_CMD	0x03
-#define IDEFLOPPY_PREVENT_REMOVAL_CMD	0x1e
-#define IDEFLOPPY_SEEK_CMD		0x2b
-#define IDEFLOPPY_START_STOP_CMD	0x1b
-#define IDEFLOPPY_TEST_UNIT_READY_CMD	0x00
-#define IDEFLOPPY_VERIFY_CMD		0x2f
-#define IDEFLOPPY_WRITE10_CMD		0x2a
-#define IDEFLOPPY_WRITE12_CMD		0xaa
-#define IDEFLOPPY_WRITE_VERIFY_CMD	0x2e
-
-/*
  *	Defines for the mode sense command
  */
 #define MODE_SENSE_CURRENT		0x00
@@ -696,8 +676,8 @@ static void idefloppy_init_pc (idefloppy_pc_t *pc)
 
 static void idefloppy_create_request_sense_cmd (idefloppy_pc_t *pc)
 {
-	idefloppy_init_pc(pc);	
-	pc->c[0] = IDEFLOPPY_REQUEST_SENSE_CMD;
+	idefloppy_init_pc(pc);
+	pc->c[0] = GPCMD_REQUEST_SENSE;
 	pc->c[4] = 255;
 	pc->request_transfer = 18;
 	pc->callback = &idefloppy_request_sense_callback;
@@ -762,7 +742,7 @@ static ide_startstop_t idefloppy_pc_intr (ide_drive_t *drive)
 			debug_log(KERN_INFO "ide-floppy: %s: I/O error\n",
 				drive->name);
 			rq->errors++;
-			if (pc->c[0] == IDEFLOPPY_REQUEST_SENSE_CMD) {
+			if (pc->c[0] == GPCMD_REQUEST_SENSE) {
 				printk(KERN_ERR "ide-floppy: I/O error in "
 					"request sense command\n");
 				return ide_do_reset(drive);
@@ -962,7 +942,7 @@ static ide_startstop_t idefloppy_issue_pc (ide_drive_t *drive, idefloppy_pc_t *p
 	u8 dma;
 
 	if (floppy->failed_pc == NULL &&
-	    pc->c[0] != IDEFLOPPY_REQUEST_SENSE_CMD)
+	    pc->c[0] != GPCMD_REQUEST_SENSE)
 		floppy->failed_pc = pc;
 	/* Set the current packet command */
 	floppy->pc = pc;
@@ -1052,14 +1032,14 @@ static void idefloppy_create_prevent_cmd (idefloppy_pc_t *pc, int prevent)
 		"prevent = %d\n", prevent);
 
 	idefloppy_init_pc(pc);
-	pc->c[0] = IDEFLOPPY_PREVENT_REMOVAL_CMD;
+	pc->c[0] = GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL;
 	pc->c[4] = prevent;
 }
 
 static void idefloppy_create_read_capacity_cmd (idefloppy_pc_t *pc)
 {
 	idefloppy_init_pc(pc);
-	pc->c[0] = IDEFLOPPY_READ_CAPACITY_CMD;
+	pc->c[0] = GPCMD_READ_FORMAT_CAPACITIES;
 	pc->c[7] = 255;
 	pc->c[8] = 255;
 	pc->request_transfer = 255;
@@ -1069,7 +1049,7 @@ static void idefloppy_create_format_unit_cmd (idefloppy_pc_t *pc, int b, int l,
 					      int flags)
 {
 	idefloppy_init_pc(pc);
-	pc->c[0] = IDEFLOPPY_FORMAT_UNIT_CMD;
+	pc->c[0] = GPCMD_FORMAT_UNIT;
 	pc->c[1] = 0x17;
 
 	memset(pc->buffer, 0, 12);
@@ -1094,7 +1074,7 @@ static void idefloppy_create_mode_sense_cmd (idefloppy_pc_t *pc, u8 page_code, u
 	u16 length = sizeof(idefloppy_mode_parameter_header_t);
 	
 	idefloppy_init_pc(pc);
-	pc->c[0] = IDEFLOPPY_MODE_SENSE_CMD;
+	pc->c[0] = GPCMD_MODE_SENSE_10;
 	pc->c[1] = 0;
 	pc->c[2] = page_code + (type << 6);
 
@@ -1116,14 +1096,14 @@ static void idefloppy_create_mode_sense_cmd (idefloppy_pc_t *pc, u8 page_code, u
 static void idefloppy_create_start_stop_cmd (idefloppy_pc_t *pc, int start)
 {
 	idefloppy_init_pc(pc);
-	pc->c[0] = IDEFLOPPY_START_STOP_CMD;
+	pc->c[0] = GPCMD_START_STOP_UNIT;
 	pc->c[4] = start;
 }
 
 static void idefloppy_create_test_unit_ready_cmd(idefloppy_pc_t *pc)
 {
 	idefloppy_init_pc(pc);
-	pc->c[0] = IDEFLOPPY_TEST_UNIT_READY_CMD;
+	pc->c[0] = GPCMD_TEST_UNIT_READY;
 }
 
 static void idefloppy_create_rw_cmd (idefloppy_floppy_t *floppy, idefloppy_pc_t *pc, struct request *rq, unsigned long sector)
@@ -1138,10 +1118,10 @@ static void idefloppy_create_rw_cmd (idefloppy_floppy_t *floppy, idefloppy_pc_t
 
 	idefloppy_init_pc(pc);
 	if (test_bit(IDEFLOPPY_USE_READ12, &floppy->flags)) {
-		pc->c[0] = cmd == READ ? IDEFLOPPY_READ12_CMD : IDEFLOPPY_WRITE12_CMD;
+		pc->c[0] = cmd == READ ? GPCMD_READ_12 : GPCMD_WRITE_12;
 		put_unaligned(htonl(blocks), (unsigned int *) &pc->c[6]);
 	} else {
-		pc->c[0] = cmd == READ ? IDEFLOPPY_READ10_CMD : IDEFLOPPY_WRITE10_CMD;
+		pc->c[0] = cmd == READ ? GPCMD_READ_10 : GPCMD_WRITE_10;
 		put_unaligned(htons(blocks), (unsigned short *) &pc->c[7]);
 	}
 	put_unaligned(htonl(block), (unsigned int *) &pc->c[2]);
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index ba32479..dd8e72b 100644
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -480,6 +480,7 @@ struct cdrom_generic_command
 #define GPCMD_TEST_UNIT_READY		    0x00
 #define GPCMD_VERIFY_10			    0x2f
 #define GPCMD_WRITE_10			    0x2a
+#define GPCMD_WRITE_12			    0xaa
 #define GPCMD_WRITE_AND_VERIFY_10	    0x2e
 /* This is listed as optional in ATAPI 2.6, but is (curiously) 
  * missing from Mt. Fuji, Table 57.  It _is_ mentioned in Mt. Fuji
-- 
1.5.3.7

[PATCH 02/21] ide-floppy: replace ntoh{s,l} and hton{s,l} calls with the generic byteorder

[Borislav Petkov]

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index e4ebb21..e63758a 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -1060,8 +1060,8 @@ static void idefloppy_create_format_unit_cmd (idefloppy_pc_t *pc, int b, int l,
 		pc->buffer[1] ^= 0x20;		/* ... turn off DCRT bit */
 	pc->buffer[3] = 8;
 
-	put_unaligned(htonl(b), (unsigned int *)(&pc->buffer[4]));
-	put_unaligned(htonl(l), (unsigned int *)(&pc->buffer[8]));
+	put_unaligned(cpu_to_be32(b), (unsigned int *)(&pc->buffer[4]));
+	put_unaligned(cpu_to_be32(l), (unsigned int *)(&pc->buffer[8]));
 	pc->buffer_size=12;
 	set_bit(PC_WRITING, &pc->flags);
 }
@@ -1089,7 +1089,7 @@ static void idefloppy_create_mode_sense_cmd (idefloppy_pc_t *pc, u8 page_code, u
 			printk(KERN_ERR "ide-floppy: unsupported page code "
 				"in create_mode_sense_cmd\n");
 	}
-	put_unaligned(htons(length), (u16 *) &pc->c[7]);
+	put_unaligned(cpu_to_be16(length), (u16 *) &pc->c[7]);
 	pc->request_transfer = length;
 }
 
@@ -1119,12 +1119,12 @@ static void idefloppy_create_rw_cmd (idefloppy_floppy_t *floppy, idefloppy_pc_t
 	idefloppy_init_pc(pc);
 	if (test_bit(IDEFLOPPY_USE_READ12, &floppy->flags)) {
 		pc->c[0] = cmd == READ ? GPCMD_READ_12 : GPCMD_WRITE_12;
-		put_unaligned(htonl(blocks), (unsigned int *) &pc->c[6]);
+		put_unaligned(cpu_to_be32(blocks), (unsigned int *) &pc->c[6]);
 	} else {
 		pc->c[0] = cmd == READ ? GPCMD_READ_10 : GPCMD_WRITE_10;
-		put_unaligned(htons(blocks), (unsigned short *) &pc->c[7]);
+		put_unaligned(cpu_to_be16(blocks), (unsigned short *)&pc->c[7]);
 	}
-	put_unaligned(htonl(block), (unsigned int *) &pc->c[2]);
+	put_unaligned(cpu_to_be32(block), (unsigned int *) &pc->c[2]);
 	pc->callback = &idefloppy_rw_callback;
 	pc->rq = rq;
 	pc->b_count = cmd == READ ? 0 : rq->bio->bi_size;
@@ -1252,10 +1252,10 @@ static int idefloppy_get_flexible_disk_page (ide_drive_t *drive)
 	set_disk_ro(floppy->disk, floppy->wp);
 	page = (idefloppy_flexible_disk_page_t *) (header + 1);
 
-	page->transfer_rate = ntohs(page->transfer_rate);
-	page->sector_size = ntohs(page->sector_size);
-	page->cyls = ntohs(page->cyls);
-	page->rpm = ntohs(page->rpm);
+	page->transfer_rate = be16_to_cpu(page->transfer_rate);
+	page->sector_size = be16_to_cpu(page->sector_size);
+	page->cyls = be16_to_cpu(page->cyls);
+	page->rpm = be16_to_cpu(page->rpm);
 	capacity = page->cyls * page->heads * page->sectors * page->sector_size;
 	if (memcmp (page, &floppy->flexible_disk_page, sizeof (idefloppy_flexible_disk_page_t)))
 		printk(KERN_INFO "%s: %dkB, %d/%d/%d CHS, %d kBps, "
@@ -1328,8 +1328,8 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
 	descriptor = (idefloppy_capacity_descriptor_t *) (header + 1);
 
 	for (i = 0; i < descriptors; i++, descriptor++) {
-		blocks = descriptor->blocks = ntohl(descriptor->blocks);
-		length = descriptor->length = ntohs(descriptor->length);
+		blocks = descriptor->blocks = be32_to_cpu(descriptor->blocks);
+		length = descriptor->length = be16_to_cpu(descriptor->length);
 
 		if (!i) 
 		{
@@ -1456,8 +1456,8 @@ static int idefloppy_get_format_capacities(ide_drive_t *drive, int __user *arg)
 		if (i == 0)
 			continue;	/* Skip the first descriptor */
 
-		blocks = ntohl(descriptor->blocks);
-		length = ntohs(descriptor->length);
+		blocks = be32_to_cpu(descriptor->blocks);
+		length = be16_to_cpu(descriptor->length);
 
 		if (put_user(blocks, argp))
 			return(-EFAULT);
-- 
1.5.3.7

[PATCH 03/21] ide-floppy: remove unnecessary ->handler != NULL check

[Borislav Petkov]

This BUG_ON is unneeded since the ->handler != NULL check is performed in
ide_set_handler().

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index e63758a..66dfd18 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -794,7 +794,7 @@ static ide_startstop_t idefloppy_pc_intr (ide_drive_t *drive)
 					"to send us more data than expected "
 					"- discarding data\n");
 				idefloppy_discard_data(drive, bcount);
-				BUG_ON(HWGROUP(drive)->handler != NULL);
+
 				ide_set_handler(drive,
 						&idefloppy_pc_intr,
 						IDEFLOPPY_WAIT_CMD,
@@ -825,7 +825,6 @@ static ide_startstop_t idefloppy_pc_intr (ide_drive_t *drive)
 	pc->actually_transferred += bcount;
 	pc->current_position += bcount;
 
-	BUG_ON(HWGROUP(drive)->handler != NULL);
 	ide_set_handler(drive, &idefloppy_pc_intr, IDEFLOPPY_WAIT_CMD, NULL);		/* And set the interrupt handler again */
 	return ide_started;
 }
@@ -852,7 +851,7 @@ static ide_startstop_t idefloppy_transfer_pc (ide_drive_t *drive)
 				"issuing a packet command\n");
 		return ide_do_reset(drive);
 	}
-	BUG_ON(HWGROUP(drive)->handler != NULL);
+
 	/* Set the interrupt routine */
 	ide_set_handler(drive, &idefloppy_pc_intr, IDEFLOPPY_WAIT_CMD, NULL);
 	/* Send the actual packet */
@@ -908,7 +907,7 @@ static ide_startstop_t idefloppy_transfer_pc1 (ide_drive_t *drive)
 	 * 40 and 50msec work well. idefloppy_pc_intr will not be actually
 	 * used until after the packet is moved in about 50 msec.
 	 */
-	BUG_ON(HWGROUP(drive)->handler != NULL);
+
 	ide_set_handler(drive, 
 	  &idefloppy_pc_intr, 		/* service routine for packet command */
 	  floppy->ticks,		/* wait this long before "failing" */
-- 
1.5.3.7

[PATCH 04/21] ide-floppy: cleanup and unify debugging macro calls

[Borislav Petkov]

* some debug_log() calls were not using "ide-floppy: " prefix

* a few used printk levels different than KERN_INFO (KERN_NOTICE
  and KERN_ERR, which is the default one if no level is given)

There should be no functional change resulting from this patch.

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   66 +++++++++++++++++++++-------------------------
 1 files changed, 30 insertions(+), 36 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 66dfd18..e8fe8ef 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -58,7 +58,8 @@
 #define IDEFLOPPY_DEBUG( fmt, args... )
 
 #if IDEFLOPPY_DEBUG_LOG
-#define debug_log printk
+#define debug_log(fmt, args...) \
+	printk(KERN_INFO "ide-floppy: " fmt, ## args)
 #else
 #define debug_log(fmt, args... ) do {} while(0)
 #endif
@@ -478,7 +479,7 @@ static int idefloppy_do_end_request(ide_drive_t *drive, int uptodate, int nsecs)
 	struct request *rq = HWGROUP(drive)->rq;
 	int error;
 
-	debug_log(KERN_INFO "Reached idefloppy_end_request\n");
+	debug_log("Reached %s\n", __FUNCTION__);
 
 	switch (uptodate) {
 		case 0: error = IDEFLOPPY_ERROR_GENERAL; break;
@@ -624,21 +625,20 @@ static void idefloppy_analyze_error (ide_drive_t *drive,idefloppy_request_sense_
 	floppy->progress_indication = result->sksv[0] & 0x80 ?
 		(u16)get_unaligned((u16 *)(result->sksv+1)):0x10000;
 	if (floppy->failed_pc)
-		debug_log(KERN_INFO "ide-floppy: pc = %x, sense key = %x, "
-			"asc = %x, ascq = %x\n", floppy->failed_pc->c[0],
-			result->sense_key, result->asc, result->ascq);
+		debug_log("pc = %x, sense key = %x, asc = %x, ascq = %x\n",
+				floppy->failed_pc->c[0], result->sense_key,
+				result->asc, result->ascq);
 	else
-		debug_log(KERN_INFO "ide-floppy: sense key = %x, asc = %x, "
-			"ascq = %x\n", result->sense_key,
-			result->asc, result->ascq);
+		debug_log("sense key = %x, asc = %x, ascq = %x\n",
+				result->sense_key, result->asc, result->ascq);
 }
 
 static void idefloppy_request_sense_callback (ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 
-	debug_log(KERN_INFO "ide-floppy: Reached %s\n", __FUNCTION__);
-	
+	debug_log("Reached %s\n", __FUNCTION__);
+
 	if (!floppy->pc->error) {
 		idefloppy_analyze_error(drive,(idefloppy_request_sense_result_t *) floppy->pc->buffer);
 		idefloppy_do_end_request(drive, 1, 0);
@@ -654,8 +654,8 @@ static void idefloppy_request_sense_callback (ide_drive_t *drive)
 static void idefloppy_pc_callback (ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
-	
-	debug_log(KERN_INFO "ide-floppy: Reached %s\n", __FUNCTION__);
+
+	debug_log("Reached %s\n", __FUNCTION__);
 
 	idefloppy_do_end_request(drive, floppy->pc->error ? 0 : 1, 0);
 }
@@ -714,8 +714,7 @@ static ide_startstop_t idefloppy_pc_intr (ide_drive_t *drive)
 	u16 bcount;
 	u8 stat, ireason;
 
-	debug_log(KERN_INFO "ide-floppy: Reached %s interrupt handler\n",
-		__FUNCTION__);
+	debug_log("Reached %s interrupt handler\n", __FUNCTION__);
 
 	if (test_bit(PC_DMA_IN_PROGRESS, &pc->flags)) {
 		if (HWIF(drive)->ide_dma_end(drive)) {
@@ -724,23 +723,22 @@ static ide_startstop_t idefloppy_pc_intr (ide_drive_t *drive)
 			pc->actually_transferred = pc->request_transfer;
 			idefloppy_update_buffers(drive, pc);
 		}
-		debug_log(KERN_INFO "ide-floppy: DMA finished\n");
+		debug_log("DMA finished\n");
 	}
 
 	/* Clear the interrupt */
 	stat = drive->hwif->INB(IDE_STATUS_REG);
 
 	if ((stat & DRQ_STAT) == 0) {		/* No more interrupts */
-		debug_log(KERN_INFO "Packet command completed, %d bytes "
-			"transferred\n", pc->actually_transferred);
+		debug_log("Packet command completed, %d bytes transferred\n",
+				pc->actually_transferred);
 		clear_bit(PC_DMA_IN_PROGRESS, &pc->flags);
 
 		local_irq_enable_in_hardirq();
 
 		if ((stat & ERR_STAT) || test_bit(PC_DMA_ERROR, &pc->flags)) {
 			/* Error detected */
-			debug_log(KERN_INFO "ide-floppy: %s: I/O error\n",
-				drive->name);
+			debug_log("I/O error\n", drive->name);
 			rq->errors++;
 			if (pc->c[0] == GPCMD_REQUEST_SENSE) {
 				printk(KERN_ERR "ide-floppy: I/O error in "
@@ -801,9 +799,8 @@ static ide_startstop_t idefloppy_pc_intr (ide_drive_t *drive)
 						NULL);
 				return ide_started;
 			}
-			debug_log(KERN_NOTICE "ide-floppy: The floppy wants to "
-				"send us more data than expected - "
-				"allowing transfer\n");
+			debug_log("The floppy wants to send us more data than"
+					" expected - allowing transfer\n");
 		}
 	}
 	if (test_bit(PC_WRITING, &pc->flags)) {
@@ -970,7 +967,7 @@ static ide_startstop_t idefloppy_issue_pc (ide_drive_t *drive, idefloppy_pc_t *p
 		return ide_stopped;
 	}
 
-	debug_log(KERN_INFO "Retry number - %d\n",pc->retries);
+	debug_log("Retry number - %d\n", pc->retries);
 
 	pc->retries++;
 	/* We haven't transferred any data yet */
@@ -1019,7 +1016,7 @@ static ide_startstop_t idefloppy_issue_pc (ide_drive_t *drive, idefloppy_pc_t *p
 
 static void idefloppy_rw_callback (ide_drive_t *drive)
 {
-	debug_log(KERN_INFO "ide-floppy: Reached idefloppy_rw_callback\n");
+	debug_log("Reached %s\n", __FUNCTION__);
 
 	idefloppy_do_end_request(drive, 1, 0);
 	return;
@@ -1027,8 +1024,7 @@ static void idefloppy_rw_callback (ide_drive_t *drive)
 
 static void idefloppy_create_prevent_cmd (idefloppy_pc_t *pc, int prevent)
 {
-	debug_log(KERN_INFO "ide-floppy: creating prevent removal command, "
-		"prevent = %d\n", prevent);
+	debug_log("creating prevent removal command, prevent = %d\n", prevent);
 
 	idefloppy_init_pc(pc);
 	pc->c[0] = GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL;
@@ -1164,10 +1160,10 @@ static ide_startstop_t idefloppy_do_request (ide_drive_t *drive, struct request
 	idefloppy_pc_t *pc;
 	unsigned long block = (unsigned long)block_s;
 
-	debug_log(KERN_INFO "dev: %s, flags: %lx, errors: %d\n",
+	debug_log("dev: %s, flags: %lx, errors: %d\n",
 			rq->rq_disk ? rq->rq_disk->disk_name : "?",
 			rq->flags, rq->errors);
-	debug_log(KERN_INFO "sector: %ld, nr_sectors: %ld, "
+	debug_log("sector: %ld, nr_sectors: %ld, "
 			"current_nr_sectors: %d\n", (long)rq->sector,
 			rq->nr_sectors, rq->current_nr_sectors);
 
@@ -1376,12 +1372,10 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
 		}
 		}
 		if (!i) {
-			debug_log( "Descriptor 0 Code: %d\n",
-				descriptor->dc);
+			debug_log("Descriptor 0 Code: %d\n", descriptor->dc);
 		}
-		debug_log( "Descriptor %d: %dkB, %d blocks, %d "
-			"sector size\n", i, blocks * length / 1024, blocks,
-			length);
+		debug_log("Descriptor %d: %dkB, %d blocks, %d sector size\n",
+				i, blocks * length / 1024, blocks, length);
 	}
 
 	/* Clik! disk does not support get_flexible_disk_page */
@@ -1773,7 +1767,7 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
 	idefloppy_pc_t pc;
 	int ret = 0;
 
-	debug_log(KERN_INFO "Reached idefloppy_open\n");
+	debug_log("Reached %s\n", __FUNCTION__);
 
 	if (!(floppy = ide_floppy_get(disk)))
 		return -ENXIO;
@@ -1833,8 +1827,8 @@ 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;
 	idefloppy_pc_t pc;
-	
-	debug_log(KERN_INFO "Reached idefloppy_release\n");
+
+	debug_log("Reached %s\n", __FUNCTION__);
 
 	if (floppy->openers == 1) {
 		/* IOMEGA Clik! drives do not support lock/unlock commands */
-- 
1.5.3.7

[PATCH 05/21] ide-floppy: remove struct idefloppy_capabilities_page

[Borislav Petkov]

BIG FAT WARNING: This patch has already been applied to Bart's quilt tree!

This change is rather temporary and is in preparation of using generic commands
as is the case with ide-cd and the uniform cdrom layer (i.e. init_cdrom_command())
However, before this happens, we'll have to remove all typedefs and teach
idefloppy_create_mode_sense_cmd() to work directly on u8 buffers.

Also, since idefloppy_get_capability_page() was used to read only the sfrp bit,
rename the latter so that the name reflects what it does.

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   55 +++++-----------------------------------------
 1 files changed, 6 insertions(+), 49 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index e8fe8ef..2b9885f 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -120,44 +120,6 @@ typedef struct idefloppy_packet_command_s {
 #define	PC_SUPPRESS_ERROR		6	/* Suppress error reporting */
 
 /*
- *	Removable Block Access Capabilities Page
- */
-typedef struct {
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-	unsigned	page_code	:6;	/* Page code - Should be 0x1b */
-	unsigned	reserved1_6	:1;	/* Reserved */
-	unsigned	ps		:1;	/* Should be 0 */
-#elif defined(__BIG_ENDIAN_BITFIELD)
-	unsigned	ps		:1;	/* Should be 0 */
-	unsigned	reserved1_6	:1;	/* Reserved */
-	unsigned	page_code	:6;	/* Page code - Should be 0x1b */
-#else
-#error "Bitfield endianness not defined! Check your byteorder.h"
-#endif
-	u8		page_length;		/* Page Length - Should be 0xa */
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-	unsigned	reserved2	:6;
-	unsigned	srfp		:1;	/* Supports reporting progress of format */
-	unsigned	sflp		:1;	/* System floppy type device */
-	unsigned	tlun		:3;	/* Total logical units supported by the device */
-	unsigned	reserved3	:3;
-	unsigned	sml		:1;	/* Single / Multiple lun supported */
-	unsigned	ncd		:1;	/* Non cd optical device */
-#elif defined(__BIG_ENDIAN_BITFIELD)
-	unsigned	sflp		:1;	/* System floppy type device */
-	unsigned	srfp		:1;	/* Supports reporting progress of format */
-	unsigned	reserved2	:6;
-	unsigned	ncd		:1;	/* Non cd optical device */
-	unsigned	sml		:1;	/* Single / Multiple lun supported */
-	unsigned	reserved3	:3;
-	unsigned	tlun		:3;	/* Total logical units supported by the device */
-#else
-#error "Bitfield endianness not defined! Check your byteorder.h"
-#endif
-	u8		reserved[8];
-} idefloppy_capabilities_page_t;
-
-/*
  *	Flexible disk page.
  */
 typedef struct {
@@ -397,7 +359,8 @@ typedef struct {
 } idefloppy_request_sense_result_t;
 
 /*
- *	Pages of the SELECT SENSE / MODE SENSE packet commands.
+ * Pages of the SELECT SENSE / MODE SENSE packet commands.
+ * See SFF-8070i spec.
  */
 #define	IDEFLOPPY_CAPABILITIES_PAGE	0x1b
 #define IDEFLOPPY_FLEXIBLE_DISK_PAGE	0x05
@@ -1273,25 +1236,20 @@ static int idefloppy_get_flexible_disk_page (ide_drive_t *drive)
 	return 0;
 }
 
-static int idefloppy_get_capability_page(ide_drive_t *drive)
+static int idefloppy_get_sfrp_bit(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 	idefloppy_pc_t pc;
-	idefloppy_mode_parameter_header_t *header;
-	idefloppy_capabilities_page_t *page;
 
 	floppy->srfp = 0;
 	idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_CAPABILITIES_PAGE,
 						 MODE_SENSE_CURRENT);
 
 	set_bit(PC_SUPPRESS_ERROR, &pc.flags);
-	if (idefloppy_queue_pc_tail(drive,&pc)) {
+	if (idefloppy_queue_pc_tail(drive, &pc))
 		return 1;
-	}
 
-	header = (idefloppy_mode_parameter_header_t *) pc.buffer;
-	page= (idefloppy_capabilities_page_t *)(header+1);
-	floppy->srfp = page->srfp;
+	floppy->srfp = pc.buffer[8 + 2] & 0x40;
 	return (0);
 }
 
@@ -1497,8 +1455,7 @@ static int idefloppy_begin_format(ide_drive_t *drive, int __user *arg)
 		return (-EFAULT);
 	}
 
-	/* Get the SFRP bit */
-	(void) idefloppy_get_capability_page(drive);
+	(void) idefloppy_get_sfrp_bit(drive);
 	idefloppy_create_format_unit_cmd(&pc, blocks, length, flags);
 	if (idefloppy_queue_pc_tail(drive, &pc)) {
                 return (-EIO);
-- 
1.5.3.7

[PATCH 06/21] ide-floppy: remove struct idefloppy_flexible_disk_page

[Borislav Petkov]

The driver used to test whether the flexible disk page has changed by memcmp-ing
it with a cached copy of a previous version of the page from a different remo-
vable medium. Since, according to the SFF-8070i spec, the flexible disk page
"specifies parameters relating to the currently installed medium type," this
comparison is now done by simply checking whether the medium has changed.

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   89 ++++++++++++++++-----------------------------
 1 files changed, 32 insertions(+), 57 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 2b9885f..679d48e 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -120,33 +120,6 @@ typedef struct idefloppy_packet_command_s {
 #define	PC_SUPPRESS_ERROR		6	/* Suppress error reporting */
 
 /*
- *	Flexible disk page.
- */
-typedef struct {
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-	unsigned	page_code	:6;	/* Page code - Should be 0x5 */
-	unsigned	reserved1_6	:1;	/* Reserved */
-	unsigned	ps		:1;	/* The device is capable of saving the page */
-#elif defined(__BIG_ENDIAN_BITFIELD)
-	unsigned	ps		:1;	/* The device is capable of saving the page */
-	unsigned	reserved1_6	:1;	/* Reserved */
-	unsigned	page_code	:6;	/* Page code - Should be 0x5 */
-#else
-#error "Bitfield endianness not defined! Check your byteorder.h"
-#endif
-	u8		page_length;		/* Page Length - Should be 0x1e */
-	u16		transfer_rate;		/* In kilobits per second */
-	u8		heads, sectors;		/* Number of heads, Number of sectors per track */
-	u16		sector_size;		/* Byes per sector */
-	u16		cyls;			/* Number of cylinders */
-	u8		reserved10[10];
-	u8		motor_delay;		/* Motor off delay */
-	u8		reserved21[7];
-	u16		rpm;			/* Rotations per minute */
-	u8		reserved30[2];
-} idefloppy_flexible_disk_page_t;
- 
-/*
  *	Format capacity
  */
 typedef struct {
@@ -213,8 +186,6 @@ typedef struct ide_floppy_obj {
 	int blocks, block_size, bs_factor;
 	/* Last format capacity */
 	idefloppy_capacity_descriptor_t capacity;
-	/* Copy of the flexible disk page */
-	idefloppy_flexible_disk_page_t flexible_disk_page;
 	/* Write protect */
 	int wp;
 	/* Supports format progress report */
@@ -1188,50 +1159,54 @@ static int idefloppy_queue_pc_tail (ide_drive_t *drive,idefloppy_pc_t *pc)
 }
 
 /*
- *	Look at the flexible disk page parameters. We will ignore the CHS
- *	capacity parameters and use the LBA parameters instead.
+ * Look at the flexible disk page parameters. We will ignore the CHS capacity
+ * parameters and use the LBA parameters instead.
  */
-static int idefloppy_get_flexible_disk_page (ide_drive_t *drive)
+static int idefloppy_get_flexible_disk_page(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 	idefloppy_pc_t pc;
-	idefloppy_mode_parameter_header_t *header;
-	idefloppy_flexible_disk_page_t *page;
 	int capacity, lba_capacity;
+	u8 heads, sectors;
+	u16 transfer_rate, sector_size, cyls, rpm;
 
-	idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_FLEXIBLE_DISK_PAGE, MODE_SENSE_CURRENT);
-	if (idefloppy_queue_pc_tail(drive,&pc)) {
-		printk(KERN_ERR "ide-floppy: Can't get flexible disk "
-			"page parameters\n");
+	idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_FLEXIBLE_DISK_PAGE,
+			MODE_SENSE_CURRENT);
+
+	if (idefloppy_queue_pc_tail(drive, &pc)) {
+		printk(KERN_ERR "ide-floppy: Can't get flexible disk page"
+				" parameters\n");
 		return 1;
 	}
-	header = (idefloppy_mode_parameter_header_t *) pc.buffer;
-	floppy->wp = header->wp;
+	floppy->wp = pc.buffer[3] & 0x80;
 	set_disk_ro(floppy->disk, floppy->wp);
-	page = (idefloppy_flexible_disk_page_t *) (header + 1);
-
-	page->transfer_rate = be16_to_cpu(page->transfer_rate);
-	page->sector_size = be16_to_cpu(page->sector_size);
-	page->cyls = be16_to_cpu(page->cyls);
-	page->rpm = be16_to_cpu(page->rpm);
-	capacity = page->cyls * page->heads * page->sectors * page->sector_size;
-	if (memcmp (page, &floppy->flexible_disk_page, sizeof (idefloppy_flexible_disk_page_t)))
+
+	transfer_rate = be16_to_cpu(*(u16 *)&pc.buffer[8 + 2]);
+	sector_size   = be16_to_cpu(*(u16 *)&pc.buffer[8 + 6]);
+	cyls          = be16_to_cpu(*(u16 *)&pc.buffer[8 + 8]);
+	rpm           = be16_to_cpu(*(u16 *)&pc.buffer[8 + 28]);
+	heads         = pc.buffer[8 + 4];
+	sectors       = pc.buffer[8 + 5];
+
+	capacity = cyls * heads * sectors * sector_size;
+
+	if ((1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags)
 		printk(KERN_INFO "%s: %dkB, %d/%d/%d CHS, %d kBps, "
 				"%d sector size, %d rpm\n",
-			drive->name, capacity / 1024, page->cyls,
-			page->heads, page->sectors,
-			page->transfer_rate / 8, page->sector_size, page->rpm);
-
-	floppy->flexible_disk_page = *page;
-	drive->bios_cyl = page->cyls;
-	drive->bios_head = page->heads;
-	drive->bios_sect = page->sectors;
+			drive->name, capacity / 1024, cyls, heads, sectors,
+			transfer_rate / 8, sector_size, rpm);
+
+	drive->bios_cyl = cyls;
+	drive->bios_head = heads;
+	drive->bios_sect = sectors;
 	lba_capacity = floppy->blocks * floppy->block_size;
+
 	if (capacity < lba_capacity) {
 		printk(KERN_NOTICE "%s: The disk reports a capacity of %d "
 			"bytes, but the drive only handles %d\n",
 			drive->name, lba_capacity, capacity);
-		floppy->blocks = floppy->block_size ? capacity / floppy->block_size : 0;
+		floppy->blocks = floppy->block_size ?
+			capacity / floppy->block_size : 0;
 	}
 	return 0;
 }
-- 
1.5.3.7

[PATCH 07/21] ide-floppy: remove struct idefloppy_capacity_descriptor

[Borislav Petkov]

We test here for updated capacity descriptors by checking whether the media
has changed instead of memcmp-ing with a cached copy of the capacity
descriptors.

Also:

- remove one of 2 consecutive if (!i)-tests.
- start loop at 1 in idefloppy_get_format_capacities() since userspace doesn't
need the current/maximum capacity descriptor. i.e the first one.
- fix comments formatting

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |  132 +++++++++++++++++----------------------------
 1 files changed, 50 insertions(+), 82 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 679d48e..5c85833 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -119,29 +119,7 @@ typedef struct idefloppy_packet_command_s {
 
 #define	PC_SUPPRESS_ERROR		6	/* Suppress error reporting */
 
-/*
- *	Format capacity
- */
-typedef struct {
-	u8		reserved[3];
-	u8		length;			/* Length of the following descriptors in bytes */
-} idefloppy_capacity_header_t;
-
-typedef struct {
-	u32		blocks;			/* Number of blocks */
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-	unsigned	dc		:2;	/* Descriptor Code */
-	unsigned	reserved	:6;
-#elif defined(__BIG_ENDIAN_BITFIELD)
-	unsigned	reserved	:6;
-	unsigned	dc		:2;	/* Descriptor Code */
-#else
-#error "Bitfield endianness not defined! Check your byteorder.h"
-#endif
-	u8		length_msb;		/* Block Length (MSB)*/
-	u16		length;			/* Block Length */
-} idefloppy_capacity_descriptor_t;
-
+/* format capacities descriptor codes */
 #define CAPACITY_INVALID	0x00
 #define CAPACITY_UNFORMATTED	0x01
 #define CAPACITY_CURRENT	0x02
@@ -184,8 +162,6 @@ typedef struct ide_floppy_obj {
 	 */
 	/* Current format */
 	int blocks, block_size, bs_factor;
-	/* Last format capacity */
-	idefloppy_capacity_descriptor_t capacity;
 	/* Write protect */
 	int wp;
 	/* Supports format progress report */
@@ -1229,17 +1205,16 @@ static int idefloppy_get_sfrp_bit(ide_drive_t *drive)
 }
 
 /*
- *	Determine if a media is present in the floppy drive, and if so,
- *	its LBA capacity.
+ * Determine if a media is present in the floppy drive, and if so, its LBA
+ * capacity.
  */
-static int idefloppy_get_capacity (ide_drive_t *drive)
+static int idefloppy_get_capacity(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 	idefloppy_pc_t pc;
-	idefloppy_capacity_header_t *header;
-	idefloppy_capacity_descriptor_t *descriptor;
-	int i, descriptors, rc = 1, blocks, length;
-	
+	int i, desc_cnt, rc = 1, blocks, length;
+	u8 header_len;
+
 	drive->bios_cyl = 0;
 	drive->bios_head = drive->bios_sect = 0;
 	floppy->blocks = 0;
@@ -1251,17 +1226,17 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
 		printk(KERN_ERR "ide-floppy: Can't get floppy parameters\n");
 		return 1;
 	}
-	header = (idefloppy_capacity_header_t *) pc.buffer;
-	descriptors = header->length / sizeof(idefloppy_capacity_descriptor_t);
-	descriptor = (idefloppy_capacity_descriptor_t *) (header + 1);
+	header_len = pc.buffer[3];
+	desc_cnt = header_len / 8; /* capacity descriptor of 8 bytes */
 
-	for (i = 0; i < descriptors; i++, descriptor++) {
-		blocks = descriptor->blocks = be32_to_cpu(descriptor->blocks);
-		length = descriptor->length = be16_to_cpu(descriptor->length);
+	for (i = 0; i < desc_cnt; i++) {
+		unsigned int desc_start = 4 + i*8;
+		blocks = be32_to_cpu(*(u32 *)&pc.buffer[desc_start]);
+		length = be16_to_cpu(*(u16 *)&pc.buffer[desc_start + 6]);
 
-		if (!i) 
+		if (!i)
 		{
-		switch (descriptor->dc) {
+		switch (pc.buffer[desc_start + 4] & 0x03) {
 		/* Clik! drive returns this instead of CAPACITY_CURRENT */
 		case CAPACITY_UNFORMATTED:
 			if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags))
@@ -1272,11 +1247,11 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
 				break;
 		case CAPACITY_CURRENT:
 			/* Normal Zip/LS-120 disks */
-			if (memcmp(descriptor, &floppy->capacity, sizeof (idefloppy_capacity_descriptor_t)))
+			if ((1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags)
 				printk(KERN_INFO "%s: %dkB, %d blocks, %d "
 					"sector size\n", drive->name,
 					blocks * length / 1024, blocks, length);
-			floppy->capacity = *descriptor;
+
 			if (!length || length % 512) {
 				printk(KERN_NOTICE "%s: %d bytes block size "
 					"not supported\n", drive->name, length);
@@ -1303,9 +1278,8 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
 				"in drive\n", drive->name);
 			break;
 		}
-		}
-		if (!i) {
-			debug_log("Descriptor 0 Code: %d\n", descriptor->dc);
+		debug_log("Descriptor 0 Code: %d\n",
+				pc.buffer[desc_start + 4] & 0x03);
 		}
 		debug_log("Descriptor %d: %dkB, %d blocks, %d sector size\n",
 				i, blocks * length / 1024, blocks, length);
@@ -1321,35 +1295,32 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
 }
 
 /*
-** Obtain the list of formattable capacities.
-** Very similar to idefloppy_get_capacity, except that we push the capacity
-** descriptors to userland, instead of our own structures.
-**
-** Userland gives us the following structure:
-**
-** struct idefloppy_format_capacities {
-**        int nformats;
-**        struct {
-**                int nblocks;
-**                int blocksize;
-**                } formats[];
-**        } ;
-**
-** userland initializes nformats to the number of allocated formats[]
-** records.  On exit we set nformats to the number of records we've
-** actually initialized.
-**
-*/
+ * Obtain the list of formattable capacities.
+ * Very similar to idefloppy_get_capacity, except that we push the capacity
+ * descriptors to userland, instead of our own structures.
+ *
+ * Userland gives us the following structure:
+ *
+ * struct idefloppy_format_capacities {
+ *        int nformats;
+ *        struct {
+ *                int nblocks;
+ *                int blocksize;
+ *                } formats[];
+ *        } ;
+ *
+ * userland initializes nformats to the number of allocated formats[]
+ * records.  On exit we set nformats to the number of records we've
+ * actually initialized.
+ *
+ */
 
 static int idefloppy_get_format_capacities(ide_drive_t *drive, int __user *arg)
 {
         idefloppy_pc_t pc;
-	idefloppy_capacity_header_t *header;
-        idefloppy_capacity_descriptor_t *descriptor;
-	int i, descriptors, blocks, length;
-	int u_array_size;
-	int u_index;
+	int i, desc_cnt, blocks, length, u_array_size, u_index;
 	int __user *argp;
+	u8 header_len;
 
 	if (get_user(u_array_size, arg))
 		return (-EFAULT);
@@ -1362,28 +1333,25 @@ static int idefloppy_get_format_capacities(ide_drive_t *drive, int __user *arg)
 		printk(KERN_ERR "ide-floppy: Can't get floppy parameters\n");
                 return (-EIO);
         }
-        header = (idefloppy_capacity_header_t *) pc.buffer;
-        descriptors = header->length /
-		sizeof(idefloppy_capacity_descriptor_t);
-	descriptor = (idefloppy_capacity_descriptor_t *) (header + 1);
+	header_len = pc.buffer[3];
+	desc_cnt = header_len / 8; /* capacity descriptor of 8 bytes */
 
 	u_index = 0;
 	argp = arg + 1;
 
 	/*
-	** We always skip the first capacity descriptor.  That's the
-	** current capacity.  We are interested in the remaining descriptors,
-	** the formattable capacities.
-	*/
+	 * We always skip the first capacity descriptor.  That's the current
+	 * capacity.  We are interested in the remaining descriptors, the
+	 * formattable capacities.
+	 */
+	for (i = 1; i < desc_cnt; i++) {
+		unsigned int desc_start = 4 + i*8;
 
-	for (i=0; i<descriptors; i++, descriptor++) {
 		if (u_index >= u_array_size)
 			break;	/* User-supplied buffer too small */
-		if (i == 0)
-			continue;	/* Skip the first descriptor */
 
-		blocks = be32_to_cpu(descriptor->blocks);
-		length = be16_to_cpu(descriptor->length);
+		blocks = be32_to_cpu(*(u32 *)&pc.buffer[desc_start]);
+		length = be16_to_cpu(*(u16 *)&pc.buffer[desc_start + 6]);
 
 		if (put_user(blocks, argp))
 			return(-EFAULT);
-- 
1.5.3.7

[PATCH 08/21] ide-floppy: remove struct idefloppy_inquiry_result

[Borislav Petkov]

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   41 -----------------------------------------
 1 files changed, 0 insertions(+), 41 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 5c85833..d98264e 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -232,47 +232,6 @@ struct idefloppy_id_gcw {
 };
 
 /*
- *	INQUIRY packet command - Data Format
- */
-typedef struct {
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-	unsigned	device_type	:5;	/* Peripheral Device Type */
-	unsigned	reserved0_765	:3;	/* Peripheral Qualifier - Reserved */
-	unsigned	reserved1_6t0	:7;	/* Reserved */
-	unsigned	rmb		:1;	/* Removable Medium Bit */
-	unsigned	ansi_version	:3;	/* ANSI Version */
-	unsigned	ecma_version	:3;	/* ECMA Version */
-	unsigned	iso_version	:2;	/* ISO Version */
-	unsigned	response_format :4;	/* Response Data Format */
-	unsigned	reserved3_45	:2;	/* Reserved */
-	unsigned	reserved3_6	:1;	/* TrmIOP - Reserved */
-	unsigned	reserved3_7	:1;	/* AENC - Reserved */
-#elif defined(__BIG_ENDIAN_BITFIELD)
-	unsigned	reserved0_765	:3;	/* Peripheral Qualifier - Reserved */
-	unsigned	device_type	:5;	/* Peripheral Device Type */
-	unsigned	rmb		:1;	/* Removable Medium Bit */
-	unsigned	reserved1_6t0	:7;	/* Reserved */
-	unsigned	iso_version	:2;	/* ISO Version */
-	unsigned	ecma_version	:3;	/* ECMA Version */
-	unsigned	ansi_version	:3;	/* ANSI Version */
-	unsigned	reserved3_7	:1;	/* AENC - Reserved */
-	unsigned	reserved3_6	:1;	/* TrmIOP - Reserved */
-	unsigned	reserved3_45	:2;	/* Reserved */
-	unsigned	response_format :4;	/* Response Data Format */
-#else
-#error "Bitfield endianness not defined! Check your byteorder.h"
-#endif
-	u8		additional_length;	/* Additional Length (total_length-4) */
-	u8		rsv5, rsv6, rsv7;	/* Reserved */
-	u8		vendor_id[8];		/* Vendor Identification */
-	u8		product_id[16];		/* Product Identification */
-	u8		revision_level[4];	/* Revision Level */
-	u8		vendor_specific[20];	/* Vendor Specific - Optional */
-	u8		reserved56t95[40];	/* Reserved - Optional */
-						/* Additional information may be returned */
-} idefloppy_inquiry_result_t;
-
-/*
  *	REQUEST SENSE packet command result - Data Format.
  */
 typedef struct {
-- 
1.5.3.7

[PATCH 09/21] ide-floppy: remove struct idefloppy_request_sense_result

[Borislav Petkov]

While at it, collapse idefloppy_analyze_error() into
idefloppy_request_sense_callback() since the latter was its only user.

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   82 +++++++++++++--------------------------------
 1 files changed, 24 insertions(+), 58 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index d98264e..7d4ac0b 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -232,39 +232,6 @@ struct idefloppy_id_gcw {
 };
 
 /*
- *	REQUEST SENSE packet command result - Data Format.
- */
-typedef struct {
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-	unsigned	error_code	:7;	/* Current error (0x70) */
-	unsigned	valid		:1;	/* The information field conforms to SFF-8070i */
-	u8		reserved1	:8;	/* Reserved */
-	unsigned	sense_key	:4;	/* Sense Key */
-	unsigned	reserved2_4	:1;	/* Reserved */
-	unsigned	ili		:1;	/* Incorrect Length Indicator */
-	unsigned	reserved2_67	:2;
-#elif defined(__BIG_ENDIAN_BITFIELD)
-	unsigned	valid		:1;	/* The information field conforms to SFF-8070i */
-	unsigned	error_code	:7;	/* Current error (0x70) */
-	u8		reserved1	:8;	/* Reserved */
-	unsigned	reserved2_67	:2;
-	unsigned	ili		:1;	/* Incorrect Length Indicator */
-	unsigned	reserved2_4	:1;	/* Reserved */
-	unsigned	sense_key	:4;	/* Sense Key */
-#else
-#error "Bitfield endianness not defined! Check your byteorder.h"
-#endif
-	u32		information __attribute__ ((packed));
-	u8		asl;			/* Additional sense length (n-7) */
-	u32		command_specific;	/* Additional command specific information */
-	u8		asc;			/* Additional Sense Code */
-	u8		ascq;			/* Additional Sense Code Qualifier */
-	u8		replaceable_unit_code;	/* Field Replaceable Unit Code */
-	u8		sksv[3];
-	u8		pad[2];			/* Padding to 20 bytes */
-} idefloppy_request_sense_result_t;
-
-/*
  * Pages of the SELECT SENSE / MODE SENSE packet commands.
  * See SFF-8070i spec.
  */
@@ -480,39 +447,38 @@ static struct request *idefloppy_next_rq_storage (ide_drive_t *drive)
 	return (&floppy->rq_stack[floppy->rq_stack_index++]);
 }
 
-/*
- *	idefloppy_analyze_error is called on each failed packet command retry
- *	to analyze the request sense.
- */
-static void idefloppy_analyze_error (ide_drive_t *drive,idefloppy_request_sense_result_t *result)
-{
-	idefloppy_floppy_t *floppy = drive->driver_data;
-
-	floppy->sense_key = result->sense_key;
-	floppy->asc = result->asc;
-	floppy->ascq = result->ascq;
-	floppy->progress_indication = result->sksv[0] & 0x80 ?
-		(u16)get_unaligned((u16 *)(result->sksv+1)):0x10000;
-	if (floppy->failed_pc)
-		debug_log("pc = %x, sense key = %x, asc = %x, ascq = %x\n",
-				floppy->failed_pc->c[0], result->sense_key,
-				result->asc, result->ascq);
-	else
-		debug_log("sense key = %x, asc = %x, ascq = %x\n",
-				result->sense_key, result->asc, result->ascq);
-}
-
-static void idefloppy_request_sense_callback (ide_drive_t *drive)
+static void idefloppy_request_sense_callback(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
+	u8 *buf = floppy->pc->buffer;
 
 	debug_log("Reached %s\n", __FUNCTION__);
 
 	if (!floppy->pc->error) {
-		idefloppy_analyze_error(drive,(idefloppy_request_sense_result_t *) floppy->pc->buffer);
+		floppy->sense_key = buf[2] & 0x0F;
+		floppy->asc = buf[12];
+		floppy->ascq = buf[13];
+		floppy->progress_indication = buf[15] & 0x80 ?
+			(u16)get_unaligned((u16 *)&buf[16]) : 0x10000;
+
+		if (floppy->failed_pc)
+			debug_log("pc = %x, sense key = %x, asc = %x,"
+					" ascq = %x\n",
+					floppy->failed_pc->c[0],
+					floppy->sense_key,
+					floppy->asc,
+					floppy->ascq);
+		else
+			debug_log("sense key = %x, asc = %x, ascq = %x\n",
+					floppy->sense_key,
+					floppy->asc,
+					floppy->ascq);
+
+
 		idefloppy_do_end_request(drive, 1, 0);
 	} else {
-		printk(KERN_ERR "Error in REQUEST SENSE itself - Aborting request!\n");
+		printk(KERN_ERR "Error in REQUEST SENSE itself - Aborting"
+				" request!\n");
 		idefloppy_do_end_request(drive, 0, 0);
 	}
 }
-- 
1.5.3.7

[PATCH 10/21] ide-floppy: remove struct idefloppy_mode_parameter_header

[Borislav Petkov]

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   25 ++++---------------------
 1 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 7d4ac0b..11c2c9b 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -238,24 +238,6 @@ struct idefloppy_id_gcw {
 #define	IDEFLOPPY_CAPABILITIES_PAGE	0x1b
 #define IDEFLOPPY_FLEXIBLE_DISK_PAGE	0x05
 
-/*
- *	Mode Parameter Header for the MODE SENSE packet command
- */
-typedef struct {
-	u16		mode_data_length;	/* Length of the following data transfer */
-	u8		medium_type;		/* Medium Type */
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-	unsigned	reserved3	:7;
-	unsigned	wp		:1;	/* Write protect */
-#elif defined(__BIG_ENDIAN_BITFIELD)
-	unsigned	wp		:1;	/* Write protect */
-	unsigned	reserved3	:7;
-#else
-#error "Bitfield endianness not defined! Check your byteorder.h"
-#endif
-	u8		reserved[4];
-} idefloppy_mode_parameter_header_t;
-
 static DEFINE_MUTEX(idefloppy_ref_mutex);
 
 #define to_ide_floppy(obj) container_of(obj, struct ide_floppy_obj, kref)
@@ -899,10 +881,11 @@ static void idefloppy_create_format_unit_cmd (idefloppy_pc_t *pc, int b, int l,
 /*
  *	A mode sense command is used to "sense" floppy parameters.
  */
-static void idefloppy_create_mode_sense_cmd (idefloppy_pc_t *pc, u8 page_code, u8 type)
+static void idefloppy_create_mode_sense_cmd(idefloppy_pc_t *pc, u8 page_code,
+		u8 type)
 {
-	u16 length = sizeof(idefloppy_mode_parameter_header_t);
-	
+	u16 length = 8; /* sizeof(Mode Parameter Header) = 8 Bytes */
+
 	idefloppy_init_pc(pc);
 	pc->c[0] = GPCMD_MODE_SENSE_10;
 	pc->c[1] = 0;
-- 
1.5.3.7

[PATCH 11/21] ide-floppy: fix comments formatting

[Borislav Petkov]

That is,
- remove unnecessary comments
- shorten comments
- shorten lines longer 80 columns
- cleanup whitespace
- add a missing loglevel KERN_ to a printk-call
- fix misc checkpatch warnings

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |  402 +++++++++++++++++++++-------------------------
 1 files changed, 181 insertions(+), 221 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 11c2c9b..5d612e7 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -4,11 +4,6 @@
  * Copyright (C) 1996-1999  Gadi Oxman <gadio@netvision.net.il>
  * Copyright (C) 2000-2002  Paul Bristow <paul@paulbristow.net>
  * Copyright (C) 2005       Bartlomiej Zolnierkiewicz
- */
-
-/*
- * The driver currently doesn't have any fancy features, just the bare
- * minimum read/write support.
  *
  * This driver supports the following IDE floppy drives:
  *
@@ -47,14 +42,11 @@
 #include <asm/io.h>
 #include <asm/unaligned.h>
 
-/*
- *	The following are used to debug the driver.
- */
+/* The following are used to debug the driver. */
 #define IDEFLOPPY_DEBUG_LOG		0
 #define IDEFLOPPY_DEBUG_INFO		0
 #define IDEFLOPPY_DEBUG_BUGS		1
 
-/* #define IDEFLOPPY_DEBUG(fmt, args...) printk(KERN_INFO fmt, ## args) */
 #define IDEFLOPPY_DEBUG( fmt, args... )
 
 #if IDEFLOPPY_DEBUG_LOG
@@ -65,59 +57,56 @@
 #endif
 
 
-/*
- *	Some drives require a longer irq timeout.
- */
+/* Some drives require a longer irq timeout. */
 #define IDEFLOPPY_WAIT_CMD		(5 * WAIT_CMD)
 
 /*
- *	After each failed packet command we issue a request sense command
- *	and retry the packet command IDEFLOPPY_MAX_PC_RETRIES times.
+ * After each failed packet command we issue a request sense command and retry
+ * the packet command IDEFLOPPY_MAX_PC_RETRIES times.
  */
 #define IDEFLOPPY_MAX_PC_RETRIES	3
 
 /*
- *	With each packet command, we allocate a buffer of
- *	IDEFLOPPY_PC_BUFFER_SIZE bytes.
+ * With each packet command, we allocate a buffer of
+ * IDEFLOPPY_PC_BUFFER_SIZE bytes.
  */
 #define IDEFLOPPY_PC_BUFFER_SIZE	256
 
 /*
- *	In various places in the driver, we need to allocate storage
- *	for packet commands and requests, which will remain valid while
- *	we leave the driver to wait for an interrupt or a timeout event.
+ * In various places in the driver, we need to allocate storage for packet
+ * commands and requests, which will remain valid while we leave the driver to
+ * wait for an interrupt or a timeout event.
  */
 #define IDEFLOPPY_PC_STACK		(10 + IDEFLOPPY_MAX_PC_RETRIES)
 
-/*
- *	Our view of a packet command.
- */
 typedef struct idefloppy_packet_command_s {
 	u8 c[12];				/* Actual packet bytes */
-	int retries;				/* On each retry, we increment retries */
+	int retries;				/* On each retry, we increment
+						   retries */
 	int error;				/* Error code */
 	int request_transfer;			/* Bytes to transfer */
 	int actually_transferred;		/* Bytes actually transferred */
 	int buffer_size;			/* Size of our data buffer */
-	int b_count;				/* Missing/Available data on the current buffer */
+	int b_count;				/* Missing/Available data on
+						   the current buffer */
 	struct request *rq;			/* The corresponding request */
 	u8 *buffer;				/* Data buffer */
-	u8 *current_position;			/* Pointer into the above buffer */
-	void (*callback) (ide_drive_t *);	/* Called when this packet command is completed */
+	u8 *current_position;			/* Pointer into above buffer */
+	void (*callback) (ide_drive_t *);	/* Called when this packet
+						   command is completed */
 	u8 pc_buffer[IDEFLOPPY_PC_BUFFER_SIZE];	/* Temporary buffer */
-	unsigned long flags;			/* Status/Action bit flags: long for set_bit */
+	unsigned long flags;			/* Status/Action bit flags:
+						   long for set_bit */
 } idefloppy_pc_t;
 
-/*
- *	Packet command flag bits.
- */
-#define	PC_ABORT			0	/* Set when an error is considered normal - We won't retry */
-#define PC_DMA_RECOMMENDED		2	/* 1 when we prefer to use DMA if possible */
-#define	PC_DMA_IN_PROGRESS		3	/* 1 while DMA in progress */
-#define	PC_DMA_ERROR			4	/* 1 when encountered problem during DMA */
-#define	PC_WRITING			5	/* Data direction */
-
-#define	PC_SUPPRESS_ERROR		6	/* Suppress error reporting */
+/* Packet command flag bits. */
+#define	PC_ABORT		0	/* Set when an error is considered\
+					   normal - We won't retry */
+#define PC_DMA_RECOMMENDED	2	/* DMA use preferred, if possible */
+#define	PC_DMA_IN_PROGRESS	3	/* 1 while DMA in progress */
+#define	PC_DMA_ERROR		4	/* problem encountered during DMA */
+#define	PC_WRITING		5	/* Data direction */
+#define	PC_SUPPRESS_ERROR	6	/* Suppress error reporting */
 
 /* format capacities descriptor codes */
 #define CAPACITY_INVALID	0x00
@@ -126,9 +115,9 @@ typedef struct idefloppy_packet_command_s {
 #define CAPACITY_NO_CARTRIDGE	0x03
 
 /*
- *	Most of our global data which we need to save even as we leave the
- *	driver due to an interrupt or a timer event is stored in a variable
- *	of type idefloppy_floppy_t, defined below.
+ * Most of our global data which we need to save even as we leave the driver
+ * due to an interrupt or a timer event is stored in a variable of type
+ * idefloppy_floppy_t, defined below.
  */
 typedef struct ide_floppy_obj {
 	ide_drive_t	*drive;
@@ -149,17 +138,13 @@ typedef struct ide_floppy_obj {
 	/* We implement a circular array */
 	int rq_stack_index;
 
-	/*
-	 *	Last error information
-	 */
+	/* Last error information */
 	u8 sense_key, asc, ascq;
 	/* delay this long before sending packet command */
 	u8 ticks;
 	int progress_indication;
 
-	/*
-	 *	Device information
-	 */
+	/* Device information */
 	/* Current format */
 	int blocks, block_size, bs_factor;
 	/* Write protect */
@@ -172,44 +157,36 @@ typedef struct ide_floppy_obj {
 
 #define IDEFLOPPY_TICKS_DELAY	HZ/20	/* default delay for ZIP 100 (50ms) */
 
-/*
- *	Floppy flag bits values.
- */
+/* Floppy flag bits values. */
 #define IDEFLOPPY_DRQ_INTERRUPT		0	/* DRQ interrupt device */
 #define IDEFLOPPY_MEDIA_CHANGED		1	/* Media may have changed */
-#define IDEFLOPPY_USE_READ12		2	/* Use READ12/WRITE12 or READ10/WRITE10 */
+#define IDEFLOPPY_USE_READ12		2	/* READ(10|12)/WRITE(10|12) */
 #define	IDEFLOPPY_FORMAT_IN_PROGRESS	3	/* Format in progress */
-#define IDEFLOPPY_CLIK_DRIVE	        4       /* Avoid commands not supported in Clik drive */
-#define IDEFLOPPY_ZIP_DRIVE		5	/* Requires BH algorithm for packets */
+#define IDEFLOPPY_CLIK_DRIVE	        4       /* Avoid commands not supported\
+						   in Clik drive */
+#define IDEFLOPPY_ZIP_DRIVE		5	/* Requires BH algorithm for\
+						   packets */
 
-/*
- *	Defines for the mode sense command
- */
+/* Defines for the MODE SENSE command */
 #define MODE_SENSE_CURRENT		0x00
 #define MODE_SENSE_CHANGEABLE		0x01
-#define MODE_SENSE_DEFAULT		0x02 
+#define MODE_SENSE_DEFAULT		0x02
 #define MODE_SENSE_SAVED		0x03
 
-/*
- *	IOCTLs used in low-level formatting.
- */
-
+/* IOCTLs used in low-level formatting. */
 #define	IDEFLOPPY_IOCTL_FORMAT_SUPPORTED	0x4600
 #define	IDEFLOPPY_IOCTL_FORMAT_GET_CAPACITY	0x4601
 #define	IDEFLOPPY_IOCTL_FORMAT_START		0x4602
 #define IDEFLOPPY_IOCTL_FORMAT_GET_PROGRESS	0x4603
 
-/*
- *	Error codes which are returned in rq->errors to the higher part
- *	of the driver.
- */
+/* Error code returned in rq->errors to the higher part of the driver. */
 #define	IDEFLOPPY_ERROR_GENERAL		101
 
 /*
- *	The following is used to format the general configuration word of
- *	the ATAPI IDENTIFY DEVICE command.
+ * The following is used to format the general configuration word of the ATAPI
+ * IDENTIFY DEVICE command.
  */
-struct idefloppy_id_gcw {	
+struct idefloppy_id_gcw {
 #if defined(__LITTLE_ENDIAN_BITFIELD)
 	unsigned packet_size		:2;	/* Packet Size */
 	unsigned reserved234		:3;	/* Reserved */
@@ -267,17 +244,17 @@ static void ide_floppy_put(struct ide_floppy_obj *floppy)
 }
 
 /*
- *	Too bad. The drive wants to send us data which we are not ready to accept.
- *	Just throw it away.
+ * Too bad. The drive wants to send us data which we are not ready to accept.
+ * Just throw it away.
  */
-static void idefloppy_discard_data (ide_drive_t *drive, unsigned int bcount)
+static void idefloppy_discard_data(ide_drive_t *drive, unsigned int bcount)
 {
 	while (bcount--)
 		(void) HWIF(drive)->INB(IDE_DATA_REG);
 }
 
 #if IDEFLOPPY_DEBUG_BUGS
-static void idefloppy_write_zeros (ide_drive_t *drive, unsigned int bcount)
+static void idefloppy_write_zeros(ide_drive_t *drive, unsigned int bcount)
 {
 	while (bcount--)
 		HWIF(drive)->OUTB(0, IDE_DATA_REG);
@@ -286,10 +263,8 @@ static void idefloppy_write_zeros (ide_drive_t *drive, unsigned int bcount)
 
 
 /*
- *	idefloppy_do_end_request is used to finish servicing a request.
- *
- *	For read/write requests, we will call ide_end_request to pass to the
- *	next buffer.
+ * Used to finish servicing a request. For read/write requests, we will call
+ * ide_end_request to pass to the next buffer.
  */
 static int idefloppy_do_end_request(ide_drive_t *drive, int uptodate, int nsecs)
 {
@@ -320,7 +295,8 @@ static int idefloppy_do_end_request(ide_drive_t *drive, int uptodate, int nsecs)
 	return 0;
 }
 
-static void idefloppy_input_buffers (ide_drive_t *drive, idefloppy_pc_t *pc, unsigned int bcount)
+static void idefloppy_input_buffers(ide_drive_t *drive, idefloppy_pc_t *pc,
+		unsigned int bcount)
 {
 	struct request *rq = pc->rq;
 	struct bio_vec *bvec;
@@ -347,12 +323,14 @@ static void idefloppy_input_buffers (ide_drive_t *drive, idefloppy_pc_t *pc, uns
 	idefloppy_do_end_request(drive, 1, done >> 9);
 
 	if (bcount) {
-		printk(KERN_ERR "%s: leftover data in idefloppy_input_buffers, bcount == %d\n", drive->name, bcount);
+		printk(KERN_ERR "%s: leftover data in %s, bcount == %d\n",
+				drive->name, __FUNCTION__, bcount);
 		idefloppy_discard_data(drive, bcount);
 	}
 }
 
-static void idefloppy_output_buffers (ide_drive_t *drive, idefloppy_pc_t *pc, unsigned int bcount)
+static void idefloppy_output_buffers(ide_drive_t *drive, idefloppy_pc_t *pc,
+		unsigned int bcount)
 {
 	struct request *rq = pc->rq;
 	struct req_iterator iter;
@@ -380,13 +358,14 @@ static void idefloppy_output_buffers (ide_drive_t *drive, idefloppy_pc_t *pc, un
 
 #if IDEFLOPPY_DEBUG_BUGS
 	if (bcount) {
-		printk(KERN_ERR "%s: leftover data in idefloppy_output_buffers, bcount == %d\n", drive->name, bcount);
+		printk(KERN_ERR "%s: leftover data in %s, bcount == %d\n",
+				drive->name, __FUNCTION__, bcount);
 		idefloppy_write_zeros(drive, bcount);
 	}
 #endif
 }
 
-static void idefloppy_update_buffers (ide_drive_t *drive, idefloppy_pc_t *pc)
+static void idefloppy_update_buffers(ide_drive_t *drive, idefloppy_pc_t *pc)
 {
 	struct request *rq = pc->rq;
 	struct bio *bio = rq->bio;
@@ -396,11 +375,12 @@ static void idefloppy_update_buffers (ide_drive_t *drive, idefloppy_pc_t *pc)
 }
 
 /*
- *	idefloppy_queue_pc_head generates a new packet command request in front
- *	of the request queue, before the current request, so that it will be
- *	processed immediately, on the next pass through the driver.
+ * Generate a new packet command request in front of the request queue, before
+ * the current request so that it will be processed immediately, on the next
+ * pass through the driver.
  */
-static void idefloppy_queue_pc_head (ide_drive_t *drive,idefloppy_pc_t *pc,struct request *rq)
+static void idefloppy_queue_pc_head(ide_drive_t *drive, idefloppy_pc_t *pc,
+		struct request *rq)
 {
 	struct ide_floppy_obj *floppy = drive->driver_data;
 
@@ -411,7 +391,7 @@ static void idefloppy_queue_pc_head (ide_drive_t *drive,idefloppy_pc_t *pc,struc
 	(void) ide_do_drive_cmd(drive, rq, ide_preempt);
 }
 
-static idefloppy_pc_t *idefloppy_next_pc_storage (ide_drive_t *drive)
+static idefloppy_pc_t *idefloppy_next_pc_storage(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 
@@ -420,7 +400,7 @@ static idefloppy_pc_t *idefloppy_next_pc_storage (ide_drive_t *drive)
 	return (&floppy->pc_stack[floppy->pc_stack_index++]);
 }
 
-static struct request *idefloppy_next_rq_storage (ide_drive_t *drive)
+static struct request *idefloppy_next_rq_storage(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 
@@ -465,10 +445,8 @@ static void idefloppy_request_sense_callback(ide_drive_t *drive)
 	}
 }
 
-/*
- *	General packet command callback function.
- */
-static void idefloppy_pc_callback (ide_drive_t *drive)
+/* General packet command callback function. */
+static void idefloppy_pc_callback(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 
@@ -477,10 +455,7 @@ static void idefloppy_pc_callback (ide_drive_t *drive)
 	idefloppy_do_end_request(drive, floppy->pc->error ? 0 : 1, 0);
 }
 
-/*
- *	idefloppy_init_pc initializes a packet command.
- */
-static void idefloppy_init_pc (idefloppy_pc_t *pc)
+static void idefloppy_init_pc(idefloppy_pc_t *pc)
 {
 	memset(pc->c, 0, 12);
 	pc->retries = 0;
@@ -491,7 +466,7 @@ static void idefloppy_init_pc (idefloppy_pc_t *pc)
 	pc->callback = &idefloppy_pc_callback;
 }
 
-static void idefloppy_create_request_sense_cmd (idefloppy_pc_t *pc)
+static void idefloppy_create_request_sense_cmd(idefloppy_pc_t *pc)
 {
 	idefloppy_init_pc(pc);
 	pc->c[0] = GPCMD_REQUEST_SENSE;
@@ -501,11 +476,10 @@ static void idefloppy_create_request_sense_cmd (idefloppy_pc_t *pc)
 }
 
 /*
- *	idefloppy_retry_pc is called when an error was detected during the
- *	last packet command. We queue a request sense packet command in
- *	the head of the request list.
+ * Called when an error was detected during the last packet command. We queue a
+ * request sense packet command in the head of the request list.
  */
-static void idefloppy_retry_pc (ide_drive_t *drive)
+static void idefloppy_retry_pc(ide_drive_t *drive)
 {
 	idefloppy_pc_t *pc;
 	struct request *rq;
@@ -518,10 +492,9 @@ static void idefloppy_retry_pc (ide_drive_t *drive)
 }
 
 /*
- *	idefloppy_pc_intr is the usual interrupt handler which will be called
- *	during a packet command.
+ * The usual interrupt handler called during a packet command.
  */
-static ide_startstop_t idefloppy_pc_intr (ide_drive_t *drive)
+static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 	ide_hwif_t *hwif = drive->hwif;
@@ -639,16 +612,17 @@ static ide_startstop_t idefloppy_pc_intr (ide_drive_t *drive)
 	pc->actually_transferred += bcount;
 	pc->current_position += bcount;
 
-	ide_set_handler(drive, &idefloppy_pc_intr, IDEFLOPPY_WAIT_CMD, NULL);		/* And set the interrupt handler again */
+	/* And set the interrupt handler again */
+	ide_set_handler(drive, &idefloppy_pc_intr, IDEFLOPPY_WAIT_CMD, NULL);
 	return ide_started;
 }
 
 /*
  * This is the original routine that did the packet transfer.
  * It fails at high speeds on the Iomega ZIP drive, so there's a slower version
- * for that drive below. The algorithm is chosen based on drive type
+ * for that drive below. The algorithm is chosen based on drive type.
  */
-static ide_startstop_t idefloppy_transfer_pc (ide_drive_t *drive)
+static ide_startstop_t idefloppy_transfer_pc(ide_drive_t *drive)
 {
 	ide_startstop_t startstop;
 	idefloppy_floppy_t *floppy = drive->driver_data;
@@ -675,18 +649,16 @@ static ide_startstop_t idefloppy_transfer_pc (ide_drive_t *drive)
 
 
 /*
- * What we have here is a classic case of a top half / bottom half
- * interrupt service routine. In interrupt mode, the device sends
- * an interrupt to signal it's ready to receive a packet. However,
- * we need to delay about 2-3 ticks before issuing the packet or we
- * gets in trouble.
+ * What we have here is a classic case of a top half / bottom half interrupt
+ * service routine. In interrupt mode, the device sends an interrupt to signal
+ * that it is ready to receive a packet. However, we need to delay about 2-3
+ * ticks before issuing the packet or we gets in trouble.
  *
- * So, follow carefully. transfer_pc1 is called as an interrupt (or
- * directly). In either case, when the device says it's ready for a 
- * packet, we schedule the packet transfer to occur about 2-3 ticks
- * later in transfer_pc2.
+ * So, follow carefully. transfer_pc1 is called as an interrupt (or directly).
+ * In either case, when the device says it's ready for a packet, we schedule
+ * the packet transfer to occur about 2-3 ticks later in transfer_pc2.
  */
-static int idefloppy_transfer_pc2 (ide_drive_t *drive)
+static int idefloppy_transfer_pc2(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 
@@ -696,7 +668,7 @@ static int idefloppy_transfer_pc2 (ide_drive_t *drive)
 	return IDEFLOPPY_WAIT_CMD;
 }
 
-static ide_startstop_t idefloppy_transfer_pc1 (ide_drive_t *drive)
+static ide_startstop_t idefloppy_transfer_pc1(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 	ide_startstop_t startstop;
@@ -713,7 +685,7 @@ static ide_startstop_t idefloppy_transfer_pc1 (ide_drive_t *drive)
 				"while issuing a packet command\n");
 		return ide_do_reset(drive);
 	}
-	/* 
+	/*
 	 * The following delay solves a problem with ATAPI Zip 100 drives
 	 * where the Busy flag was apparently being deasserted before the
 	 * unit was ready to receive data. This was happening on a
@@ -722,17 +694,15 @@ static ide_startstop_t idefloppy_transfer_pc1 (ide_drive_t *drive)
 	 * used until after the packet is moved in about 50 msec.
 	 */
 
-	ide_set_handler(drive, 
-	  &idefloppy_pc_intr, 		/* service routine for packet command */
+	ide_set_handler(drive,
+	  &idefloppy_pc_intr,		/* service routine for packet command */
 	  floppy->ticks,		/* wait this long before "failing" */
 	  &idefloppy_transfer_pc2);	/* fail == transfer_pc2 */
 	return ide_started;
 }
 
-/**
- * idefloppy_should_report_error()
- *
- * Supresses error messages resulting from Medium not present
+/*
+ * Suppresses error messages resulting from Medium not present.
  */
 static inline int idefloppy_should_report_error(idefloppy_floppy_t *floppy)
 {
@@ -743,10 +713,8 @@ static inline int idefloppy_should_report_error(idefloppy_floppy_t *floppy)
 	return 1;
 }
 
-/*
- *	Issue a packet command
- */
-static ide_startstop_t idefloppy_issue_pc (ide_drive_t *drive, idefloppy_pc_t *pc)
+static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
+		idefloppy_pc_t *pc)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 	ide_hwif_t *hwif = drive->hwif;
@@ -816,7 +784,7 @@ static ide_startstop_t idefloppy_issue_pc (ide_drive_t *drive, idefloppy_pc_t *p
 		/* immediate */
 		pkt_xfer_routine = &idefloppy_transfer_pc;
 	}
-	
+
 	if (test_bit (IDEFLOPPY_DRQ_INTERRUPT, &floppy->flags)) {
 		/* Issue the packet command */
 		ide_execute_command(drive, WIN_PACKETCMD,
@@ -831,7 +799,7 @@ static ide_startstop_t idefloppy_issue_pc (ide_drive_t *drive, idefloppy_pc_t *p
 	}
 }
 
-static void idefloppy_rw_callback (ide_drive_t *drive)
+static void idefloppy_rw_callback(ide_drive_t *drive)
 {
 	debug_log("Reached %s\n", __FUNCTION__);
 
@@ -839,7 +807,7 @@ static void idefloppy_rw_callback (ide_drive_t *drive)
 	return;
 }
 
-static void idefloppy_create_prevent_cmd (idefloppy_pc_t *pc, int prevent)
+static void idefloppy_create_prevent_cmd(idefloppy_pc_t *pc, int prevent)
 {
 	debug_log("creating prevent removal command, prevent = %d\n", prevent);
 
@@ -848,7 +816,7 @@ static void idefloppy_create_prevent_cmd (idefloppy_pc_t *pc, int prevent)
 	pc->c[4] = prevent;
 }
 
-static void idefloppy_create_read_capacity_cmd (idefloppy_pc_t *pc)
+static void idefloppy_create_read_capacity_cmd(idefloppy_pc_t *pc)
 {
 	idefloppy_init_pc(pc);
 	pc->c[0] = GPCMD_READ_FORMAT_CAPACITIES;
@@ -857,7 +825,7 @@ static void idefloppy_create_read_capacity_cmd (idefloppy_pc_t *pc)
 	pc->request_transfer = 255;
 }
 
-static void idefloppy_create_format_unit_cmd (idefloppy_pc_t *pc, int b, int l,
+static void idefloppy_create_format_unit_cmd(idefloppy_pc_t *pc, int b, int l,
 					      int flags)
 {
 	idefloppy_init_pc(pc);
@@ -878,9 +846,7 @@ static void idefloppy_create_format_unit_cmd (idefloppy_pc_t *pc, int b, int l,
 	set_bit(PC_WRITING, &pc->flags);
 }
 
-/*
- *	A mode sense command is used to "sense" floppy parameters.
- */
+/* A mode sense command is used to "sense" floppy parameters. */
 static void idefloppy_create_mode_sense_cmd(idefloppy_pc_t *pc, u8 page_code,
 		u8 type)
 {
@@ -906,7 +872,7 @@ static void idefloppy_create_mode_sense_cmd(idefloppy_pc_t *pc, u8 page_code,
 	pc->request_transfer = length;
 }
 
-static void idefloppy_create_start_stop_cmd (idefloppy_pc_t *pc, int start)
+static void idefloppy_create_start_stop_cmd(idefloppy_pc_t *pc, int start)
 {
 	idefloppy_init_pc(pc);
 	pc->c[0] = GPCMD_START_STOP_UNIT;
@@ -919,7 +885,8 @@ static void idefloppy_create_test_unit_ready_cmd(idefloppy_pc_t *pc)
 	pc->c[0] = GPCMD_TEST_UNIT_READY;
 }
 
-static void idefloppy_create_rw_cmd (idefloppy_floppy_t *floppy, idefloppy_pc_t *pc, struct request *rq, unsigned long sector)
+static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
+		idefloppy_pc_t *pc, struct request *rq, unsigned long sector)
 {
 	int block = sector / floppy->bs_factor;
 	int blocks = rq->nr_sectors / floppy->bs_factor;
@@ -948,8 +915,8 @@ static void idefloppy_create_rw_cmd (idefloppy_floppy_t *floppy, idefloppy_pc_t
 	set_bit(PC_DMA_RECOMMENDED, &pc->flags);
 }
 
-static void
-idefloppy_blockpc_cmd(idefloppy_floppy_t *floppy, idefloppy_pc_t *pc, struct request *rq)
+static void idefloppy_blockpc_cmd(idefloppy_floppy_t *floppy,
+		idefloppy_pc_t *pc, struct request *rq)
 {
 	idefloppy_init_pc(pc);
 	pc->callback = &idefloppy_rw_callback;
@@ -961,18 +928,17 @@ idefloppy_blockpc_cmd(idefloppy_floppy_t *floppy, idefloppy_pc_t *pc, struct req
 	pc->buffer = rq->data;
 	if (rq->bio)
 		set_bit(PC_DMA_RECOMMENDED, &pc->flags);
-		
+
 	/*
-	 * possibly problematic, doesn't look like ide-floppy correctly
-	 * handled scattered requests if dma fails...
+	 * possibly problematic, doesn't look like ide-floppy correctly handled
+	 * scattered requests if dma fails...
 	 */
 	pc->request_transfer = pc->buffer_size = rq->data_len;
 }
 
-/*
- *	idefloppy_do_request is our request handling function.	
- */
-static ide_startstop_t idefloppy_do_request (ide_drive_t *drive, struct request *rq, sector_t block_s)
+/* Our request handler */
+static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
+		struct request *rq, sector_t block_s)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 	idefloppy_pc_t *pc;
@@ -1026,10 +992,10 @@ static ide_startstop_t idefloppy_do_request (ide_drive_t *drive, struct request
 }
 
 /*
- *	idefloppy_queue_pc_tail adds a special packet command request to the
- *	tail of the request queue, and waits for it to be serviced.
+ * 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,idefloppy_pc_t *pc)
+static int idefloppy_queue_pc_tail(ide_drive_t *drive, idefloppy_pc_t *pc)
 {
 	struct ide_floppy_obj *floppy = drive->driver_data;
 	struct request rq;
@@ -1278,20 +1244,20 @@ static int idefloppy_get_format_capacities(ide_drive_t *drive, int __user *arg)
 }
 
 /*
-** Send ATAPI_FORMAT_UNIT to the drive.
-**
-** Userland gives us the following structure:
-**
-** struct idefloppy_format_command {
-**        int nblocks;
-**        int blocksize;
-**        int flags;
-**        } ;
-**
-** flags is a bitmask, currently, the only defined flag is:
-**
-**        0x01 - verify media after format.
-*/
+ * Send ATAPI_FORMAT_UNIT to the drive.
+ *
+ * Userland gives us the following structure:
+ *
+ * struct idefloppy_format_command {
+ *        int nblocks;
+ *        int blocksize;
+ *        int flags;
+ *        } ;
+ *
+ * flags is a bitmask, currently, the only defined flag is:
+ *
+ *        0x01 - verify media after format.
+ */
 
 static int idefloppy_begin_format(ide_drive_t *drive, int __user *arg)
 {
@@ -1316,14 +1282,14 @@ static int idefloppy_begin_format(ide_drive_t *drive, int __user *arg)
 }
 
 /*
-** Get ATAPI_FORMAT_UNIT progress indication.
-**
-** Userland gives a pointer to an int.  The int is set to a progress
-** indicator 0-65536, with 65536=100%.
-**
-** If the drive does not support format progress indication, we just check
-** the dsc bit, and return either 0 or 65536.
-*/
+ * Get ATAPI_FORMAT_UNIT progress indication.
+ *
+ * Userland gives a pointer to an int.  The int is set to a progress
+ * indicator 0-65536, with 65536=100%.
+ *
+ * If the drive does not support format progress indication, we just check
+ * the dsc bit, and return either 0 or 65536.
+ */
 
 static int idefloppy_get_format_progress(ide_drive_t *drive, int __user *arg)
 {
@@ -1342,8 +1308,7 @@ static int idefloppy_get_format_progress(ide_drive_t *drive, int __user *arg)
 		    floppy->ascq == 4) {
 			progress_indication = floppy->progress_indication;
 		}
-		/* Else assume format_unit has finished, and we're
-		** at 0x10000 */
+		/* Else assume format_unit has finished, and we're at 0x10000 */
 	} else {
 		unsigned long flags;
 		u8 stat;
@@ -1360,10 +1325,7 @@ static int idefloppy_get_format_progress(ide_drive_t *drive, int __user *arg)
 	return (0);
 }
 
-/*
- *	Return the current floppy capacity.
- */
-static sector_t idefloppy_capacity (ide_drive_t *drive)
+static sector_t idefloppy_capacity(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 	unsigned long capacity = floppy->blocks * floppy->bs_factor;
@@ -1372,8 +1334,7 @@ static sector_t idefloppy_capacity (ide_drive_t *drive)
 }
 
 /*
- *	idefloppy_identify_device checks if we can support a drive,
- *	based on the ATAPI IDENTIFY command results.
+ * Check if we can support a drive, based on the ATAPI IDENTIFY command results.
  */
 static int idefloppy_identify_device (ide_drive_t *drive,struct hd_driveid *id)
 {
@@ -1389,7 +1350,7 @@ static int idefloppy_identify_device (ide_drive_t *drive,struct hd_driveid *id)
 	if ((gcw.device_type == 5) &&
 	    !strstr(id->model, "CD-ROM") &&
 	    strstr(id->model, "ZIP"))
-		gcw.device_type = 0;			
+		gcw.device_type = 0;
 #endif
 
 #if IDEFLOPPY_DEBUG_INFO
@@ -1411,7 +1372,7 @@ static int idefloppy_identify_device (ide_drive_t *drive,struct hd_driveid *id)
 		default: sprintf(buffer, "Reserved");
 	}
 	printk(KERN_INFO "Device Type: %x - %s\n", gcw.device_type, buffer);
-	printk(KERN_INFO "Removable: %s\n",gcw.removable ? "Yes":"No");	
+	printk(KERN_INFO "Removable: %s\n", gcw.removable ? "Yes":"No");
 	switch (gcw.drq_type) {
 		case 0: sprintf(buffer, "Microprocessor DRQ");break;
 		case 1: sprintf(buffer, "Interrupt DRQ");break;
@@ -1447,22 +1408,20 @@ static void idefloppy_add_settings(ide_drive_t *drive)
 {
 	idefloppy_floppy_t *floppy = drive->driver_data;
 
-/*
- *			drive	setting name	read/write	data type	min	max	mul_factor	div_factor	data pointer		set function
- */
-	ide_add_setting(drive,	"bios_cyl",	SETTING_RW,	TYPE_INT,	0,	1023,		1,		1,	&drive->bios_cyl,	NULL);
-	ide_add_setting(drive,	"bios_head",	SETTING_RW,	TYPE_BYTE,	0,	255,		1,		1,	&drive->bios_head,	NULL);
-	ide_add_setting(drive,	"bios_sect",	SETTING_RW,	TYPE_BYTE,	0,	63,		1,		1,	&drive->bios_sect,	NULL);
-	ide_add_setting(drive,	"ticks",	SETTING_RW,	TYPE_BYTE,	0,	255,		1,		1,	&floppy->ticks,		NULL);
+	ide_add_setting(drive, "bios_cyl", SETTING_RW, TYPE_INT, 0, 1023, 1, 1,
+			&drive->bios_cyl, NULL);
+	ide_add_setting(drive, "bios_head", SETTING_RW, TYPE_BYTE, 0, 255, 1, 1,
+			&drive->bios_head, NULL);
+	ide_add_setting(drive, "bios_sect", SETTING_RW,	TYPE_BYTE, 0,  63, 1, 1,
+			&drive->bios_sect, NULL);
+	ide_add_setting(drive, "ticks",	   SETTING_RW, TYPE_BYTE, 0, 255, 1, 1,
+			&floppy->ticks,	 NULL);
 }
 #else
 static inline void idefloppy_add_settings(ide_drive_t *drive) { ; }
 #endif
 
-/*
- *	Driver initialization.
- */
-static void idefloppy_setup (ide_drive_t *drive, idefloppy_floppy_t *floppy)
+static void idefloppy_setup(ide_drive_t *drive, idefloppy_floppy_t *floppy)
 {
 	struct idefloppy_id_gcw gcw;
 
@@ -1470,15 +1429,15 @@ static void idefloppy_setup (ide_drive_t *drive, idefloppy_floppy_t *floppy)
 	floppy->pc = floppy->pc_stack;
 	if (gcw.drq_type == 1)
 		set_bit(IDEFLOPPY_DRQ_INTERRUPT, &floppy->flags);
+
 	/*
-	 *	We used to check revisions here. At this point however
-	 *	I'm giving up. Just assume they are all broken, its easier.
+	 * We used to check revisions here. At this point however I'm giving up.
+	 * Just assume they are all broken, its easier.
 	 *
-	 *	The actual reason for the workarounds was likely
-	 *	a driver bug after all rather than a firmware bug,
-	 *	and the workaround below used to hide it. It should
-	 *	be fixed as of version 1.9, but to be on the safe side
-	 *	we'll leave the limitation below for the 2.2.x tree.
+	 * The actual reason for the workarounds was likely a driver bug after
+	 * all rather than a firmware bug, and the workaround below used to hide
+	 * it. It should be fixed as of version 1.9, but to be on the safe side
+	 * we'll leave the limitation below for the 2.2.x tree.
 	 */
 
 	if (!strncmp(drive->id->model, "IOMEGA ZIP 100 ATAPI", 20)) {
@@ -1489,16 +1448,14 @@ static void idefloppy_setup (ide_drive_t *drive, idefloppy_floppy_t *floppy)
 	}
 
 	/*
-	*      Guess what?  The IOMEGA Clik! drive also needs the
-	*      above fix.  It makes nasty clicking noises without
-	*      it, so please don't remove this.
-	*/
+	 * Guess what?  The IOMEGA Clik! drive also needs the above fix. It
+	 * makes nasty clicking noises without it, so please don't remove this.
+	 */
 	if (strncmp(drive->id->model, "IOMEGA Clik!", 11) == 0) {
 		blk_queue_max_sectors(drive->queue, 64);
 		set_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags);
 	}
 
-
 	(void) idefloppy_get_capacity(drive);
 	idefloppy_add_settings(drive);
 }
@@ -1528,8 +1485,8 @@ static void ide_floppy_release(struct kref *kref)
 }
 
 #ifdef CONFIG_IDE_PROC_FS
-static int proc_idefloppy_read_capacity
-	(char *page, char **start, off_t off, int count, int *eof, void *data)
+static int proc_idefloppy_read_capacity(char *page, char **start, off_t off,
+		int count, int *eof, void *data)
 {
 	ide_drive_t*drive = (ide_drive_t *)data;
 	int len;
@@ -1539,8 +1496,8 @@ static int proc_idefloppy_read_capacity
 }
 
 static ide_proc_entry_t idefloppy_proc[] = {
-	{ "capacity",	S_IFREG|S_IRUGO,	proc_idefloppy_read_capacity, NULL },
-	{ "geometry",	S_IFREG|S_IRUGO,	proc_ide_read_geometry,	NULL },
+	{ "capacity",	S_IFREG|S_IRUGO, proc_idefloppy_read_capacity,	NULL },
+	{ "geometry",	S_IFREG|S_IRUGO, proc_ide_read_geometry,	NULL },
 	{ NULL, 0, NULL, NULL }
 };
 #endif	/* CONFIG_IDE_PROC_FS */
@@ -1597,10 +1554,10 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
 		if (idefloppy_get_capacity (drive)
 		   && (filp->f_flags & O_NDELAY) == 0
 		    /*
-		    ** Allow O_NDELAY to open a drive without a disk, or with
-		    ** an unreadable disk, so that we can get the format
-		    ** capacity of the drive or begin the format - Sam
-		    */
+		     * Allow O_NDELAY to open a drive without a disk, or with an
+		     * unreadable disk, so that we can get the format capacity
+		     * of the drive or begin the format - Sam
+		     */
 		    ) {
 			ret = -EIO;
 			goto out_put_floppy;
@@ -1685,7 +1642,8 @@ static int idefloppy_ioctl(struct inode *inode, struct file *file,
 		if (floppy->openers > 1)
 			return -EBUSY;
 
-		/* The IOMEGA Clik! Drive doesn't support this command - no room for an eject mechanism */
+		/* The IOMEGA Clik! Drive doesn't support this command - no room
+		 * for an eject mechanism */
                 if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
 			idefloppy_create_prevent_cmd(&pc, prevent);
 			(void) idefloppy_queue_pc_tail(drive, &pc);
@@ -1719,11 +1677,10 @@ static int idefloppy_ioctl(struct inode *inode, struct file *file,
 			clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
 		return err;
 		/*
-		** Note, the bit will be cleared when the device is
-		** closed.  This is the cleanest way to handle the
-		** situation where the drive does not support
-		** format progress reporting.
-		*/
+		 * Note, the bit will be cleared when the device is closed. This
+		 * is the cleanest way to handle the situation where the drive
+		 * does not support format progress reporting.
+		 */
 	case IDEFLOPPY_IOCTL_FORMAT_GET_PROGRESS:
 		return idefloppy_get_format_progress(drive, argp);
 	}
@@ -1786,15 +1743,18 @@ static int ide_floppy_probe(ide_drive_t *drive)
 	if (drive->media != ide_floppy)
 		goto failed;
 	if (!idefloppy_identify_device (drive, drive->id)) {
-		printk (KERN_ERR "ide-floppy: %s: not supported by this version of ide-floppy\n", drive->name);
+		printk(KERN_ERR "ide-floppy: %s: not supported by this version"
+				" of ide-floppy\n", drive->name);
 		goto failed;
 	}
 	if (drive->scsi) {
-		printk("ide-floppy: passing drive %s to ide-scsi emulation.\n", drive->name);
+		printk(KERN_INFO "ide-floppy: passing drive %s to ide-scsi"
+				" emulation.\n", drive->name);
 		goto failed;
 	}
 	if ((floppy = kzalloc(sizeof (idefloppy_floppy_t), GFP_KERNEL)) == NULL) {
-		printk (KERN_ERR "ide-floppy: %s: Can't allocate a floppy structure\n", drive->name);
+		printk(KERN_ERR "ide-floppy: %s: Can't allocate a floppy"
+				" structure\n", drive->name);
 		goto failed;
 	}
 
-- 
1.5.3.7

[PATCH 12/21] ide-floppy: factor out ioctl handlers from idefloppy_ioctl()

[Borislav Petkov]

By passing idefloppy_floppy_t *floppy to the factored out functions, we get
rid of (almost) all local vars so stack usage should be at minimum here. Also,
we merge idefloppy_begin_format() into idefloppy_format_start() since it is its
only user.

Also,
- remove unneeded scsi ioctl chunk
- rename idefloppy_format_start to idefloppy_format_unit

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |  169 +++++++++++++++++++++------------------------
 1 files changed, 79 insertions(+), 90 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 5d612e7..a33d651 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -1244,44 +1244,6 @@ static int idefloppy_get_format_capacities(ide_drive_t *drive, int __user *arg)
 }
 
 /*
- * Send ATAPI_FORMAT_UNIT to the drive.
- *
- * Userland gives us the following structure:
- *
- * struct idefloppy_format_command {
- *        int nblocks;
- *        int blocksize;
- *        int flags;
- *        } ;
- *
- * flags is a bitmask, currently, the only defined flag is:
- *
- *        0x01 - verify media after format.
- */
-
-static int idefloppy_begin_format(ide_drive_t *drive, int __user *arg)
-{
-	int blocks;
-	int length;
-	int flags;
-	idefloppy_pc_t pc;
-
-	if (get_user(blocks, arg) ||
-	    get_user(length, arg+1) ||
-	    get_user(flags, arg+2)) {
-		return (-EFAULT);
-	}
-
-	(void) idefloppy_get_sfrp_bit(drive);
-	idefloppy_create_format_unit_cmd(&pc, blocks, length, flags);
-	if (idefloppy_queue_pc_tail(drive, &pc)) {
-                return (-EIO);
-	}
-
-	return (0);
-}
-
-/*
  * Get ATAPI_FORMAT_UNIT progress indication.
  *
  * Userland gives a pointer to an int.  The int is set to a progress
@@ -1623,6 +1585,82 @@ static int idefloppy_getgeo(struct block_device *bdev, struct hd_geometry *geo)
 	return 0;
 }
 
+static int idefloppy_lockdoor(idefloppy_floppy_t *floppy, idefloppy_pc_t *pc,
+		unsigned long arg, unsigned int cmd)
+{
+	if (floppy->openers > 1)
+		return -EBUSY;
+
+	/* The IOMEGA Clik! Drive doesn't support this command -
+	 * no room for an eject mechanism */
+	if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
+		int prevent = (arg) ? 1 : 0;
+
+		if (cmd == CDROMEJECT)
+			prevent = 0;
+
+		idefloppy_create_prevent_cmd(pc, prevent);
+		(void) idefloppy_queue_pc_tail(floppy->drive, pc);
+	}
+
+	if (cmd == CDROMEJECT) {
+		idefloppy_create_start_stop_cmd(pc, 2);
+		(void) idefloppy_queue_pc_tail(floppy->drive, pc);
+	}
+
+	return 0;
+}
+
+static int idefloppy_format_unit(idefloppy_floppy_t *floppy, unsigned long arg)
+{
+	int blocks, length, flags, err = 0;
+	int __user *argp = (int __user *)arg;
+	idefloppy_pc_t pc;
+
+	if (floppy->openers > 1) {
+		/* Don't format if someone is using the disk */
+		clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
+		return -EBUSY;
+	}
+
+	set_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
+
+	/*
+	 * Send ATAPI_FORMAT_UNIT to the drive.
+	 *
+	 * Userland gives us the following structure:
+	 *
+	 * struct idefloppy_format_command {
+	 *        int nblocks;
+	 *        int blocksize;
+	 *        int flags;
+	 *        } ;
+	 *
+	 * flags is a bitmask, currently, the only defined flag is:
+	 *
+	 *        0x01 - verify media after format.
+	 */
+	if (get_user(blocks, argp) ||
+			get_user(length, argp+1) ||
+			get_user(flags, argp+2)) {
+		err = -EFAULT;
+		goto out;
+	}
+
+	(void) idefloppy_get_sfrp_bit(floppy->drive);
+	idefloppy_create_format_unit_cmd(&pc, blocks, length, flags);
+
+	if (idefloppy_queue_pc_tail(floppy->drive, &pc)) {
+		err = -EIO;
+		goto out;
+	}
+
+out:
+	if (err)
+		clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
+	return err;
+}
+
 static int idefloppy_ioctl(struct inode *inode, struct file *file,
 			unsigned int cmd, unsigned long arg)
 {
@@ -1630,75 +1668,26 @@ static int idefloppy_ioctl(struct inode *inode, struct file *file,
 	struct ide_floppy_obj *floppy = ide_floppy_g(bdev->bd_disk);
 	ide_drive_t *drive = floppy->drive;
 	void __user *argp = (void __user *)arg;
-	int err;
-	int prevent = (arg) ? 1 : 0;
 	idefloppy_pc_t pc;
 
 	switch (cmd) {
 	case CDROMEJECT:
-		prevent = 0;
 		/* fall through */
 	case CDROM_LOCKDOOR:
-		if (floppy->openers > 1)
-			return -EBUSY;
-
-		/* The IOMEGA Clik! Drive doesn't support this command - no room
-		 * for an eject mechanism */
-                if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
-			idefloppy_create_prevent_cmd(&pc, prevent);
-			(void) idefloppy_queue_pc_tail(drive, &pc);
-		}
-		if (cmd == CDROMEJECT) {
-			idefloppy_create_start_stop_cmd(&pc, 2);
-			(void) idefloppy_queue_pc_tail(drive, &pc);
-		}
-		return 0;
+		return idefloppy_lockdoor(floppy, &pc, arg, cmd);
 	case IDEFLOPPY_IOCTL_FORMAT_SUPPORTED:
 		return 0;
 	case IDEFLOPPY_IOCTL_FORMAT_GET_CAPACITY:
 		return idefloppy_get_format_capacities(drive, argp);
 	case IDEFLOPPY_IOCTL_FORMAT_START:
-
 		if (!(file->f_mode & 2))
 			return -EPERM;
-
-		if (floppy->openers > 1) {
-			/* Don't format if someone is using the disk */
-
-			clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS,
-				  &floppy->flags);
-			return -EBUSY;
-		}
-
-		set_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
-
-		err = idefloppy_begin_format(drive, argp);
-		if (err)
-			clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
-		return err;
-		/*
-		 * Note, the bit will be cleared when the device is closed. This
-		 * is the cleanest way to handle the situation where the drive
-		 * does not support format progress reporting.
-		 */
+		return idefloppy_format_unit(floppy, arg);
 	case IDEFLOPPY_IOCTL_FORMAT_GET_PROGRESS:
 		return idefloppy_get_format_progress(drive, argp);
 	}
 
-	/*
-	 * skip SCSI_IOCTL_SEND_COMMAND (deprecated)
-	 * and CDROM_SEND_PACKET (legacy) ioctls
-	 */
-	if (cmd != CDROM_SEND_PACKET && cmd != SCSI_IOCTL_SEND_COMMAND)
-		err = scsi_cmd_ioctl(file, bdev->bd_disk->queue,
-					bdev->bd_disk, cmd, argp);
-	else
-		err = -ENOTTY;
-
-	if (err == -ENOTTY)
-		err = generic_ide_ioctl(drive, file, bdev, cmd, arg);
-
-	return err;
+	return generic_ide_ioctl(drive, file, bdev, cmd, arg);
 }
 
 static int idefloppy_media_changed(struct gendisk *disk)
-- 
1.5.3.7

[PATCH 14/21] ide-floppy: mv idefloppy_{should_,}report_error

[Borislav Petkov]

In addition to shortening the function name, move the printk-call into the
function thereby saving some code lines. Also, make the function out_of_line
since it is not on a performance critical path.

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   37 ++++++++++++++-----------------------
 1 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 49d83a1..b718615 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -707,16 +707,18 @@ static ide_startstop_t idefloppy_transfer_pc1(ide_drive_t *drive)
 	return ide_started;
 }
 
-/*
- * Suppresses error messages resulting from Medium not present.
- */
-static inline int idefloppy_should_report_error(idefloppy_floppy_t *floppy)
+static void idefloppy_report_error(idefloppy_floppy_t *floppy,
+		idefloppy_pc_t *pc)
 {
 	if (floppy->sense_key == 0x02 &&
 	    floppy->asc       == 0x3a &&
 	    floppy->ascq      == 0x00)
-		return 0;
-	return 1;
+		return;
+
+	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->asc, floppy->ascq);
 }
 
 static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
@@ -741,15 +743,8 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
 		 *	a legitimate error code was received.
 		 */
 		if (!test_bit(PC_ABORT, &pc->flags)) {
-			if (!test_bit(PC_SUPPRESS_ERROR, &pc->flags)) {
-				if (idefloppy_should_report_error(floppy))
-					printk(KERN_ERR "ide-floppy: %s: I/O error, "
-					       "pc = %2x, key = %2x, "
-					       "asc = %2x, ascq = %2x\n",
-					       drive->name, pc->c[0],
-					       floppy->sense_key,
-					       floppy->asc, floppy->ascq);
-			}
+			if (!test_bit(PC_SUPPRESS_ERROR, &pc->flags))
+				idefloppy_report_error(floppy, pc);
 			/* Giving up */
 			pc->error = IDEFLOPPY_ERROR_GENERAL;
 		}
@@ -958,16 +953,12 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
 			rq->nr_sectors, rq->current_nr_sectors);
 
 	if (rq->errors >= ERROR_MAX) {
-		if (floppy->failed_pc != NULL) {
-			if (idefloppy_should_report_error(floppy))
-				printk(KERN_ERR "ide-floppy: %s: I/O error, pc = %2x,"
-				       " key = %2x, asc = %2x, ascq = %2x\n",
-				       drive->name, floppy->failed_pc->c[0],
-				       floppy->sense_key, floppy->asc, floppy->ascq);
-		}
+		if (floppy->failed_pc != NULL)
+			idefloppy_report_error(floppy, floppy->failed_pc);
 		else
 			printk(KERN_ERR "ide-floppy: %s: I/O error\n",
-				drive->name);
+					drive->name);
+
 		idefloppy_do_end_request(drive, 0, 0);
 		return ide_stopped;
 	}
-- 
1.5.3.7

[PATCH 15/21] ide-floppy: disambiguate function names

[Borislav Petkov]

There were two almost identical function names in ide-floppy.c, which makes their
distinction almost impossible. While ide_floppy_release() cleans up the object
after the last reference to it has been dropped, idefloppy_release() is the blkdev
.release method from struct block_device_operations which releases that last reference.

Rename ide_floppy_release() to idefloppy_cleanup_obj() in order to make its purpose
more clear. There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index b718615..6e8926a 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -234,12 +234,12 @@ static struct ide_floppy_obj *ide_floppy_get(struct gendisk *disk)
 	return floppy;
 }
 
-static void ide_floppy_release(struct kref *);
+static void idefloppy_cleanup_obj(struct kref *);
 
 static void ide_floppy_put(struct ide_floppy_obj *floppy)
 {
 	mutex_lock(&idefloppy_ref_mutex);
-	kref_put(&floppy->kref, ide_floppy_release);
+	kref_put(&floppy->kref, idefloppy_cleanup_obj);
 	mutex_unlock(&idefloppy_ref_mutex);
 }
 
@@ -1431,7 +1431,7 @@ static void ide_floppy_remove(ide_drive_t *drive)
 	ide_floppy_put(floppy);
 }
 
-static void ide_floppy_release(struct kref *kref)
+static void idefloppy_cleanup_obj(struct kref *kref)
 {
 	struct ide_floppy_obj *floppy = to_ide_floppy(kref);
 	ide_drive_t *drive = floppy->drive;
-- 
1.5.3.7

[PATCH 18/21] ide-floppy: fix error handling in idefloppy_probe()

[Borislav Petkov]

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 89b26ea..0729df5 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -1737,7 +1737,8 @@ static int ide_floppy_probe(ide_drive_t *drive)
 				" emulation.\n", drive->name);
 		goto failed;
 	}
-	if ((floppy = kzalloc(sizeof (idefloppy_floppy_t), GFP_KERNEL)) == NULL) {
+	floppy = kzalloc(sizeof(idefloppy_floppy_t), GFP_KERNEL);
+	if (!floppy) {
 		printk(KERN_ERR "ide-floppy: %s: Can't allocate a floppy"
 				" structure\n", drive->name);
 		goto failed;
-- 
1.5.3.7

[PATCH 19/21] ide-floppy: fix most of the remaining checkpatch.pl issues

[Borislav Petkov]

i.e.,
ERROR: switch and case should be at the same indent
ERROR: need spaces around that '=' (ctx:VxV)
ERROR: trailing statements should be on next line
WARNING: no space between function name and open parenthesis '('
WARNING: printk() should include KERN_ facility level
ERROR: That open brace { should be on the previous line
ERROR: use tabs not spaces
ERROR: do not use assignment in if condition
WARNING: braces {} are not necessary for single statement blocks
ERROR: need space after that ',' (ctx:VxV)
WARNING: line over 80 characters
ERROR: do not use assignment in if condition
...

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |  147 +++++++++++++++++++++++----------------------
 1 files changed, 75 insertions(+), 72 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 0729df5..3d9b1e5 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -47,13 +47,13 @@
 #define IDEFLOPPY_DEBUG_INFO		0
 #define IDEFLOPPY_DEBUG_BUGS		1
 
-#define IDEFLOPPY_DEBUG( fmt, args... )
+#define IDEFLOPPY_DEBUG(fmt, args...)
 
 #if IDEFLOPPY_DEBUG_LOG
 #define debug_log(fmt, args...) \
 	printk(KERN_INFO "ide-floppy: " fmt, ## args)
 #else
-#define debug_log(fmt, args... ) do {} while(0)
+#define debug_log(fmt, args...) do {} while (0)
 #endif
 
 
@@ -275,9 +275,9 @@ static int idefloppy_do_end_request(ide_drive_t *drive, int uptodate, int nsecs)
 	debug_log("Reached %s\n", __FUNCTION__);
 
 	switch (uptodate) {
-		case 0: error = IDEFLOPPY_ERROR_GENERAL; break;
-		case 1: error = 0; break;
-		default: error = uptodate;
+	case 0: error = IDEFLOPPY_ERROR_GENERAL; break;
+	case 1: error = 0; break;
+	default: error = uptodate;
 	}
 	if (error)
 		floppy->failed_pc = NULL;
@@ -396,7 +396,7 @@ static idefloppy_pc_t *idefloppy_next_pc_storage(ide_drive_t *drive)
 	idefloppy_floppy_t *floppy = drive->driver_data;
 
 	if (floppy->pc_stack_index == IDEFLOPPY_PC_STACK)
-		floppy->pc_stack_index=0;
+		floppy->pc_stack_index = 0;
 	return (&floppy->pc_stack[floppy->pc_stack_index++]);
 }
 
@@ -526,7 +526,8 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
 	/* Clear the interrupt */
 	stat = drive->hwif->INB(IDE_STATUS_REG);
 
-	if ((stat & DRQ_STAT) == 0) {		/* No more interrupts */
+	if ((stat & DRQ_STAT) == 0) {
+		/* No more interrupts */
 		debug_log("Packet command completed, %d bytes transferred\n",
 				pc->actually_transferred);
 		clear_bit(PC_DMA_IN_PROGRESS, &pc->flags);
@@ -771,7 +772,8 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
 	ide_pktcmd_tf_load(drive, IDE_TFLAG_NO_SELECT_MASK |
 			   IDE_TFLAG_OUT_DEVICE, bcount, dma);
 
-	if (dma) {	/* Begin DMA, if necessary */
+	if (dma) {
+		/* Begin DMA, if necessary */
 		set_bit(PC_DMA_IN_PROGRESS, &pc->flags);
 		hwif->dma_start(drive);
 	}
@@ -785,7 +787,7 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
 		pkt_xfer_routine = &idefloppy_transfer_pc;
 	}
 
-	if (test_bit (IDEFLOPPY_DRQ_INTERRUPT, &floppy->flags)) {
+	if (test_bit(IDEFLOPPY_DRQ_INTERRUPT, &floppy->flags)) {
 		/* Issue the packet command */
 		ide_execute_command(drive, WIN_PACKETCMD,
 				pkt_xfer_routine,
@@ -842,7 +844,7 @@ static void idefloppy_create_format_unit_cmd(idefloppy_pc_t *pc, int b, int l,
 
 	put_unaligned(cpu_to_be32(b), (unsigned int *)(&pc->buffer[4]));
 	put_unaligned(cpu_to_be32(l), (unsigned int *)(&pc->buffer[8]));
-	pc->buffer_size=12;
+	pc->buffer_size = 12;
 	set_bit(PC_WRITING, &pc->flags);
 }
 
@@ -858,15 +860,15 @@ static void idefloppy_create_mode_sense_cmd(idefloppy_pc_t *pc, u8 page_code,
 	pc->c[2] = page_code + (type << 6);
 
 	switch (page_code) {
-		case IDEFLOPPY_CAPABILITIES_PAGE:
-			length += 12;
-			break;
-		case IDEFLOPPY_FLEXIBLE_DISK_PAGE:
-			length += 32;
-			break;
-		default:
-			printk(KERN_ERR "ide-floppy: unsupported page code "
-				"in create_mode_sense_cmd\n");
+	case IDEFLOPPY_CAPABILITIES_PAGE:
+		length += 12;
+		break;
+	case IDEFLOPPY_FLEXIBLE_DISK_PAGE:
+		length += 32;
+		break;
+	default:
+		printk(KERN_ERR "ide-floppy: unsupported page code in %s\n",
+				__FUNCTION__);
 	}
 	put_unaligned(cpu_to_be16(length), (u16 *) &pc->c[7]);
 	pc->request_transfer = length;
@@ -893,7 +895,7 @@ static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
 	int cmd = rq_data_dir(rq);
 
 	debug_log("create_rw1%d_cmd: block == %d, blocks == %d\n",
-		2 * test_bit (IDEFLOPPY_USE_READ12, &floppy->flags),
+		2 * test_bit(IDEFLOPPY_USE_READ12, &floppy->flags),
 		block, blocks);
 
 	idefloppy_init_pc(pc);
@@ -964,7 +966,7 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
 	if (blk_fs_request(rq)) {
 		if (((long)rq->sector % floppy->bs_factor) ||
 		    (rq->nr_sectors % floppy->bs_factor)) {
-			printk("%s: unsupported r/w request size\n",
+			printk(KERN_ERR "%s: unsupported r/w request size\n",
 				drive->name);
 			idefloppy_do_end_request(drive, 0, 0);
 			return ide_stopped;
@@ -996,7 +998,7 @@ static int idefloppy_queue_pc_tail(ide_drive_t *drive, idefloppy_pc_t *pc)
 	struct ide_floppy_obj *floppy = drive->driver_data;
 	struct request rq;
 
-	ide_init_drive_cmd (&rq);
+	ide_init_drive_cmd(&rq);
 	rq.buffer = (char *) pc;
 	rq.cmd_type = REQ_TYPE_SPECIAL;
 	rq.rq_disk = floppy->disk;
@@ -1110,7 +1112,7 @@ static int idefloppy_get_capacity(ide_drive_t *drive)
 		/* Clik! drive returns this instead of CAPACITY_CURRENT */
 		case CAPACITY_UNFORMATTED:
 			if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags))
-                                /*
+				/*
 				 * If it is not a clik drive, break out
 				 * (maintains previous driver behaviour)
 				 */
@@ -1126,14 +1128,15 @@ static int idefloppy_get_capacity(ide_drive_t *drive)
 				printk(KERN_NOTICE "%s: %d bytes block size "
 					"not supported\n", drive->name, length);
 			} else {
-                                floppy->blocks = blocks;
-                                floppy->block_size = length;
-                                if ((floppy->bs_factor = length / 512) != 1)
-                                        printk(KERN_NOTICE "%s: warning: non "
+				floppy->blocks = blocks;
+				floppy->block_size = length;
+				floppy->bs_factor = length / 512;
+				if ((floppy->bs_factor) != 1)
+					printk(KERN_NOTICE "%s: warning: non "
 						"512 bytes block size not "
 						"fully supported\n",
 						drive->name);
-                                rc = 0;
+				rc = 0;
 			}
 			break;
 		case CAPACITY_NO_CARTRIDGE:
@@ -1156,9 +1159,8 @@ static int idefloppy_get_capacity(ide_drive_t *drive)
 	}
 
 	/* Clik! disk does not support get_flexible_disk_page */
-        if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
+	if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags))
 		(void) idefloppy_get_flexible_disk_page(drive);
-	}
 
 	set_capacity(floppy->disk, floppy->blocks * floppy->bs_factor);
 	return rc;
@@ -1187,7 +1189,7 @@ static int idefloppy_get_capacity(ide_drive_t *drive)
 
 static int idefloppy_get_format_capacities(ide_drive_t *drive, int __user *arg)
 {
-        idefloppy_pc_t pc;
+	idefloppy_pc_t pc;
 	int i, desc_cnt, blocks, length, u_array_size, u_index;
 	int __user *argp;
 	u8 header_len;
@@ -1201,8 +1203,8 @@ static int idefloppy_get_format_capacities(ide_drive_t *drive, int __user *arg)
 	idefloppy_create_read_capacity_cmd(&pc);
 	if (idefloppy_queue_pc_tail(drive, &pc)) {
 		printk(KERN_ERR "ide-floppy: Can't get floppy parameters\n");
-                return (-EIO);
-        }
+		return (-EIO);
+	}
 	header_len = pc.buffer[3];
 	desc_cnt = header_len / 8; /* capacity descriptor of 8 bytes */
 
@@ -1257,15 +1259,13 @@ static int idefloppy_get_format_progress(ide_drive_t *drive, int __user *arg)
 
 	if (floppy->srfp) {
 		idefloppy_create_request_sense_cmd(&pc);
-		if (idefloppy_queue_pc_tail(drive, &pc)) {
+		if (idefloppy_queue_pc_tail(drive, &pc))
 			return (-EIO);
-		}
 
-		if (floppy->sense_key == 2 &&
-		    floppy->asc == 4 &&
-		    floppy->ascq == 4) {
+		if (floppy->sense_key == 2 && floppy->asc == 4 &&
+				floppy->ascq == 4)
 			progress_indication = floppy->progress_indication;
-		}
+
 		/* Else assume format_unit has finished, and we're at 0x10000 */
 	} else {
 		unsigned long flags;
@@ -1294,7 +1294,7 @@ static sector_t idefloppy_capacity(ide_drive_t *drive)
 /*
  * Check if we can support a drive, based on the ATAPI IDENTIFY command results.
  */
-static int idefloppy_identify_device (ide_drive_t *drive,struct hd_driveid *id)
+static int idefloppy_identify_device(ide_drive_t *drive, struct hd_driveid *id)
 {
 	struct idefloppy_id_gcw gcw;
 #if IDEFLOPPY_DEBUG_INFO
@@ -1314,34 +1314,34 @@ static int idefloppy_identify_device (ide_drive_t *drive,struct hd_driveid *id)
 #if IDEFLOPPY_DEBUG_INFO
 	printk(KERN_INFO "Dumping ATAPI Identify Device floppy parameters\n");
 	switch (gcw.protocol) {
-		case 0: case 1: sprintf(buffer, "ATA");break;
-		case 2:	sprintf(buffer, "ATAPI");break;
-		case 3: sprintf(buffer, "Reserved (Unknown to ide-floppy)");break;
+	case 0: case 1: sprintf(buffer, "ATA"); break;
+	case 2:	sprintf(buffer, "ATAPI"); break;
+	case 3: sprintf(buffer, "Reserved (Unknown to ide-floppy)"); break;
 	}
 	printk(KERN_INFO "Protocol Type: %s\n", buffer);
 	switch (gcw.device_type) {
-		case 0: sprintf(buffer, "Direct-access Device");break;
-		case 1: sprintf(buffer, "Streaming Tape Device");break;
-		case 2: case 3: case 4: sprintf (buffer, "Reserved");break;
-		case 5: sprintf(buffer, "CD-ROM Device");break;
-		case 6: sprintf(buffer, "Reserved");
-		case 7: sprintf(buffer, "Optical memory Device");break;
-		case 0x1f: sprintf(buffer, "Unknown or no Device type");break;
-		default: sprintf(buffer, "Reserved");
+	case 0: sprintf(buffer, "Direct-access Device"); break;
+	case 1: sprintf(buffer, "Streaming Tape Device"); break;
+	case 2: case 3: case 4: sprintf(buffer, "Reserved"); break;
+	case 5: sprintf(buffer, "CD-ROM Device"); break;
+	case 6: sprintf(buffer, "Reserved");
+	case 7: sprintf(buffer, "Optical memory Device"); break;
+	case 0x1f: sprintf(buffer, "Unknown or no Device type"); break;
+	default: sprintf(buffer, "Reserved");
 	}
 	printk(KERN_INFO "Device Type: %x - %s\n", gcw.device_type, buffer);
 	printk(KERN_INFO "Removable: %s\n", gcw.removable ? "Yes":"No");
 	switch (gcw.drq_type) {
-		case 0: sprintf(buffer, "Microprocessor DRQ");break;
-		case 1: sprintf(buffer, "Interrupt DRQ");break;
-		case 2: sprintf(buffer, "Accelerated DRQ");break;
-		case 3: sprintf(buffer, "Reserved");break;
+	case 0: sprintf(buffer, "Microprocessor DRQ"); break;
+	case 1: sprintf(buffer, "Interrupt DRQ"); break;
+	case 2: sprintf(buffer, "Accelerated DRQ"); break;
+	case 3: sprintf(buffer, "Reserved"); break;
 	}
 	printk(KERN_INFO "Command Packet DRQ Type: %s\n", buffer);
 	switch (gcw.packet_size) {
-		case 0: sprintf(buffer, "12 bytes");break;
-		case 1: sprintf(buffer, "16 bytes");break;
-		default: sprintf(buffer, "Reserved");break;
+	case 0: sprintf(buffer, "12 bytes"); break;
+	case 1: sprintf(buffer, "16 bytes"); break;
+	default: sprintf(buffer, "Reserved"); break;
 	}
 	printk(KERN_INFO "Command Packet Size: %s\n", buffer);
 #endif /* IDEFLOPPY_DEBUG_INFO */
@@ -1349,13 +1349,16 @@ static int idefloppy_identify_device (ide_drive_t *drive,struct hd_driveid *id)
 	if (gcw.protocol != 2)
 		printk(KERN_ERR "ide-floppy: Protocol is not ATAPI\n");
 	else if (gcw.device_type != 0)
-		printk(KERN_ERR "ide-floppy: Device type is not set to floppy\n");
+		printk(KERN_ERR "ide-floppy: Device type is not set to"
+				" floppy\n");
 	else if (!gcw.removable)
 		printk(KERN_ERR "ide-floppy: The removable flag is not set\n");
 	else if (gcw.drq_type == 3) {
-		printk(KERN_ERR "ide-floppy: Sorry, DRQ type %d not supported\n", gcw.drq_type);
+		printk(KERN_ERR "ide-floppy: Sorry, DRQ type %d not"
+				" supported\n", gcw.drq_type);
 	} else if (gcw.packet_size != 0) {
-		printk(KERN_ERR "ide-floppy: Packet size is not 12 bytes long\n");
+		printk(KERN_ERR "ide-floppy: Packet size is not 12 bytes"
+				" long\n");
 	} else
 		return 1;
 	return 0;
@@ -1449,8 +1452,8 @@ static int proc_idefloppy_read_capacity(char *page, char **start, off_t off,
 	ide_drive_t*drive = (ide_drive_t *)data;
 	int len;
 
-	len = sprintf(page,"%llu\n", (long long)idefloppy_capacity(drive));
-	PROC_IDE_READ_RETURN(page,start,off,count,eof,len);
+	len = sprintf(page, "%llu\n", (long long)idefloppy_capacity(drive));
+	PROC_IDE_READ_RETURN(page, start, off, count, eof, len);
 }
 
 static ide_proc_entry_t idefloppy_proc[] = {
@@ -1492,7 +1495,8 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
 
 	debug_log("Reached %s\n", __FUNCTION__);
 
-	if (!(floppy = ide_floppy_get(disk)))
+	floppy = ide_floppy_get(disk);
+	if (!floppy)
 		return -ENXIO;
 
 	drive = floppy->drive;
@@ -1509,7 +1513,7 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
 			(void) idefloppy_queue_pc_tail(drive, &pc);
 		}
 
-		if (idefloppy_get_capacity (drive)
+		if (idefloppy_get_capacity(drive)
 		   && (filp->f_flags & O_NDELAY) == 0
 		    /*
 		     * Allow O_NDELAY to open a drive without a disk, or with an
@@ -1527,7 +1531,7 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
 		}
 		set_bit(IDEFLOPPY_MEDIA_CHANGED, &floppy->flags);
 		/* IOMEGA Clik! drives do not support lock/unlock commands */
-                if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
+		if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
 			idefloppy_create_prevent_cmd(&pc, 1);
 			(void) idefloppy_queue_pc_tail(drive, &pc);
 		}
@@ -1555,7 +1559,7 @@ static int idefloppy_release(struct inode *inode, struct file *filp)
 
 	if (floppy->openers == 1) {
 		/* IOMEGA Clik! drives do not support lock/unlock commands */
-                if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
+		if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
 			idefloppy_create_prevent_cmd(&pc, 0);
 			(void) idefloppy_queue_pc_tail(drive, &pc);
 		}
@@ -1727,7 +1731,7 @@ static int ide_floppy_probe(ide_drive_t *drive)
 		goto failed;
 	if (drive->media != ide_floppy)
 		goto failed;
-	if (!idefloppy_identify_device (drive, drive->id)) {
+	if (!idefloppy_identify_device(drive, drive->id)) {
 		printk(KERN_ERR "ide-floppy: %s: not supported by this version"
 				" of ide-floppy\n", drive->name);
 		goto failed;
@@ -1762,7 +1766,7 @@ static int ide_floppy_probe(ide_drive_t *drive)
 
 	drive->driver_data = floppy;
 
-	idefloppy_setup (drive, floppy);
+	idefloppy_setup(drive, floppy);
 
 	g->minors = 1 << PARTN_BITS;
 	g->driverfs_dev = &drive->gendev;
@@ -1778,9 +1782,7 @@ failed:
 	return -ENODEV;
 }
 
-MODULE_DESCRIPTION("ATAPI FLOPPY Driver");
-
-static void __exit idefloppy_exit (void)
+static void __exit idefloppy_exit(void)
 {
 	driver_unregister(&idefloppy_driver.gen_driver);
 }
@@ -1795,3 +1797,4 @@ MODULE_ALIAS("ide:*m-floppy*");
 module_init(idefloppy_init);
 module_exit(idefloppy_exit);
 MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ATAPI FLOPPY Driver");
-- 
1.5.3.7

[PATCH 20/21] ide-floppy: merge idefloppy_{input,output}_buffers

[Borislav Petkov]

We merge idefloppy_{input,output}_buffers() into idefloppy_io_buffers() by
introducing a 4th arg. called direction. According to its value
we atapi_input_bytes() or atapi_output_bytes(). Also, simplify the interrupt
handler by removing multiple calls testing the data direction and using a local
variable instead.

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   67 +++++++++++----------------------------------
 1 files changed, 17 insertions(+), 50 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 3d9b1e5..4106eb4 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -295,42 +295,8 @@ static int idefloppy_do_end_request(ide_drive_t *drive, int uptodate, int nsecs)
 	return 0;
 }
 
-static void idefloppy_input_buffers(ide_drive_t *drive, idefloppy_pc_t *pc,
-		unsigned int bcount)
-{
-	struct request *rq = pc->rq;
-	struct bio_vec *bvec;
-	struct req_iterator iter;
-	unsigned long flags;
-	char *data;
-	int count, done = 0;
-
-	rq_for_each_segment(bvec, rq, iter) {
-		if (!bcount)
-			break;
-
-		count = min(bvec->bv_len, bcount);
-
-		data = bvec_kmap_irq(bvec, &flags);
-		drive->hwif->atapi_input_bytes(drive, data, count);
-		bvec_kunmap_irq(data, &flags);
-
-		bcount -= count;
-		pc->b_count += count;
-		done += count;
-	}
-
-	idefloppy_do_end_request(drive, 1, done >> 9);
-
-	if (bcount) {
-		printk(KERN_ERR "%s: leftover data in %s, bcount == %d\n",
-				drive->name, __FUNCTION__, bcount);
-		idefloppy_discard_data(drive, bcount);
-	}
-}
-
-static void idefloppy_output_buffers(ide_drive_t *drive, idefloppy_pc_t *pc,
-		unsigned int bcount)
+static void idefloppy_io_buffers(ide_drive_t *drive, idefloppy_pc_t *pc,
+		unsigned int bcount, int direction)
 {
 	struct request *rq = pc->rq;
 	struct req_iterator iter;
@@ -346,7 +312,10 @@ static void idefloppy_output_buffers(ide_drive_t *drive, idefloppy_pc_t *pc,
 		count = min(bvec->bv_len, bcount);
 
 		data = bvec_kmap_irq(bvec, &flags);
-		drive->hwif->atapi_output_bytes(drive, data, count);
+		if (direction)
+			drive->hwif->atapi_output_bytes(drive, data, count);
+		else
+			drive->hwif->atapi_input_bytes(drive, data, count);
 		bvec_kunmap_irq(data, &flags);
 
 		bcount -= count;
@@ -360,7 +329,10 @@ static void idefloppy_output_buffers(ide_drive_t *drive, idefloppy_pc_t *pc,
 	if (bcount) {
 		printk(KERN_ERR "%s: leftover data in %s, bcount == %d\n",
 				drive->name, __FUNCTION__, bcount);
-		idefloppy_write_zeros(drive, bcount);
+		if (direction)
+			idefloppy_write_zeros(drive, bcount);
+		else
+			idefloppy_discard_data(drive, bcount);
 	}
 #endif
 }
@@ -491,8 +463,6 @@ static void idefloppy_retry_pc(ide_drive_t *drive)
 	idefloppy_queue_pc_head(drive, pc, rq);
 }
 
-typedef void (io_buf_t)(ide_drive_t *, idefloppy_pc_t *, unsigned int);
-
 /*  The usual interrupt handler called during a packet command. */
 static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
 {
@@ -501,7 +471,6 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
 	idefloppy_pc_t *pc = floppy->pc;
 	struct request *rq = pc->rq;
 	xfer_func_t *xferfunc;
-	io_buf_t *iobuf_func;
 	unsigned int temp;
 	u16 bcount;
 	u8 stat, ireason;
@@ -573,7 +542,7 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
 		printk(KERN_ERR "ide-floppy: CoD != 0 in %s\n", __FUNCTION__);
 		return ide_do_reset(drive);
 	}
-	if (((ireason & IO) == IO) == test_bit(PC_WRITING, &pc->flags)) {
+	if (((ireason & IO) == IO) == write) {
 		/* Hopefully, we will never get here */
 		printk(KERN_ERR "ide-floppy: We wanted to %s, ",
 				(ireason & IO) ? "Write" : "Read");
@@ -581,7 +550,7 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
 				(ireason & IO) ? "Read" : "Write");
 		return ide_do_reset(drive);
 	}
-	if (!test_bit(PC_WRITING, &pc->flags)) {
+	if (!write) {
 		/* Reading - Check that we have enough space */
 		temp = pc->actually_transferred + bcount;
 		if (temp > pc->request_transfer) {
@@ -601,18 +570,16 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
 					" expected - allowing transfer\n");
 		}
 	}
-	if (test_bit(PC_WRITING, &pc->flags)) {
+
+	if (write)
 		xferfunc = hwif->atapi_output_bytes;
-		iobuf_func = &idefloppy_output_buffers;
-	} else {
+	 else
 		xferfunc = hwif->atapi_input_bytes;
-		iobuf_func = &idefloppy_input_buffers;
-	}
 
-	if (pc->buffer != NULL)
+	if (pc->buffer)
 		xferfunc(drive, pc->current_position, bcount);
 	else
-		iobuf_func(drive, pc, bcount);
+		idefloppy_io_buffers(drive, pc, bcount, write);
 
 	/* Update the current position */
 	pc->actually_transferred += bcount;
-- 
1.5.3.7

[PATCH 21/21] ide-floppy: remove atomic test_*bit macros

[Borislav Petkov]

This change is temporary and after unification of the IDE subsystem proper
bit setting and testing macros will be introduced.

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
---
 drivers/ide/ide-floppy.c |   82 +++++++++++++++++++++++++---------------------
 1 files changed, 45 insertions(+), 37 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 4106eb4..29c1983 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -479,12 +479,12 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
 
 	debug_log("Reached %s interrupt handler\n", __FUNCTION__);
 
-	if (test_bit(PC_DMA_IN_PROGRESS, &pc->flags)) {
+	if ((1UL << PC_DMA_IN_PROGRESS) & pc->flags) {
 		dma_error = HWIF(drive)->ide_dma_end(drive);
 		if (dma_error) {
 			printk(KERN_ERR "%s: DMA %s error\n", drive->name,
 					write ?	"write" : "read");
-			set_bit(PC_DMA_ERROR, &pc->flags);
+			pc->flags |= (1UL << PC_DMA_ERROR);
 		} else {
 			pc->actually_transferred = pc->request_transfer;
 			idefloppy_update_buffers(drive, pc);
@@ -499,11 +499,11 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
 		/* No more interrupts */
 		debug_log("Packet command completed, %d bytes transferred\n",
 				pc->actually_transferred);
-		clear_bit(PC_DMA_IN_PROGRESS, &pc->flags);
+		pc->flags &= ((1UL << PC_DMA_IN_PROGRESS) ^ ~0UL);
 
 		local_irq_enable_in_hardirq();
 
-		if ((stat & ERR_STAT) || test_bit(PC_DMA_ERROR, &pc->flags)) {
+		if ((stat & ERR_STAT) || ((1UL << PC_DMA_ERROR) & pc->flags)) {
 			/* Error detected */
 			debug_log("I/O error\n", drive->name);
 			rq->errors++;
@@ -525,7 +525,8 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
 		return ide_stopped;
 	}
 
-	if (test_and_clear_bit(PC_DMA_IN_PROGRESS, &pc->flags)) {
+	if ((1UL << PC_DMA_IN_PROGRESS) & pc->flags) {
+		pc->flags &= ((1UL << PC_DMA_IN_PROGRESS) ^ ~0UL);
 		printk(KERN_ERR "ide-floppy: The floppy wants to issue "
 			"more interrupts in DMA mode\n");
 		ide_dma_off(drive);
@@ -704,13 +705,13 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
 	floppy->pc = pc;
 
 	if (pc->retries > IDEFLOPPY_MAX_PC_RETRIES ||
-	    test_bit(PC_ABORT, &pc->flags)) {
+			((1UL << PC_ABORT) & pc->flags)) {
 		/*
 		 *	We will "abort" retrying a packet command in case
 		 *	a legitimate error code was received.
 		 */
-		if (!test_bit(PC_ABORT, &pc->flags)) {
-			if (!test_bit(PC_SUPPRESS_ERROR, &pc->flags))
+		if (!((1UL << PC_ABORT) & pc->flags)) {
+			if (!((1UL << PC_SUPPRESS_ERROR) & pc->flags))
 				idefloppy_report_error(floppy, pc);
 			/* Giving up */
 			pc->error = IDEFLOPPY_ERROR_GENERAL;
@@ -728,12 +729,14 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
 	pc->current_position = pc->buffer;
 	bcount = min(pc->request_transfer, 63 * 1024);
 
-	if (test_and_clear_bit(PC_DMA_ERROR, &pc->flags))
+	if ((1UL << PC_DMA_ERROR) & pc->flags) {
+		pc->flags &= ((1UL << PC_DMA_ERROR) ^ ~0UL);
 		ide_dma_off(drive);
+	}
 
 	dma = 0;
 
-	if (test_bit(PC_DMA_RECOMMENDED, &pc->flags) && drive->using_dma)
+	if (((1UL << PC_DMA_RECOMMENDED) & pc->flags) && drive->using_dma)
 		dma = !hwif->dma_setup(drive);
 
 	ide_pktcmd_tf_load(drive, IDE_TFLAG_NO_SELECT_MASK |
@@ -741,12 +744,12 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
 
 	if (dma) {
 		/* Begin DMA, if necessary */
-		set_bit(PC_DMA_IN_PROGRESS, &pc->flags);
+		pc->flags |= 1UL << PC_DMA_IN_PROGRESS;
 		hwif->dma_start(drive);
 	}
 
 	/* Can we transfer the packet when we get the interrupt or wait? */
-	if (test_bit(IDEFLOPPY_ZIP_DRIVE, &floppy->flags)) {
+	if ((1UL << IDEFLOPPY_ZIP_DRIVE) & floppy->flags) {
 		/* wait */
 		pkt_xfer_routine = &idefloppy_transfer_pc1;
 	} else {
@@ -754,7 +757,7 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
 		pkt_xfer_routine = &idefloppy_transfer_pc;
 	}
 
-	if (test_bit(IDEFLOPPY_DRQ_INTERRUPT, &floppy->flags)) {
+	if ((1UL << IDEFLOPPY_DRQ_INTERRUPT) & floppy->flags) {
 		/* Issue the packet command */
 		ide_execute_command(drive, WIN_PACKETCMD,
 				pkt_xfer_routine,
@@ -812,7 +815,7 @@ static void idefloppy_create_format_unit_cmd(idefloppy_pc_t *pc, int b, int l,
 	put_unaligned(cpu_to_be32(b), (unsigned int *)(&pc->buffer[4]));
 	put_unaligned(cpu_to_be32(l), (unsigned int *)(&pc->buffer[8]));
 	pc->buffer_size = 12;
-	set_bit(PC_WRITING, &pc->flags);
+	pc->flags |= 1 << PC_WRITING;
 }
 
 /* A mode sense command is used to "sense" floppy parameters. */
@@ -862,11 +865,11 @@ static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
 	int cmd = rq_data_dir(rq);
 
 	debug_log("create_rw1%d_cmd: block == %d, blocks == %d\n",
-		2 * test_bit(IDEFLOPPY_USE_READ12, &floppy->flags),
+		2 * ((1UL << IDEFLOPPY_USE_READ12) & floppy->flags),
 		block, blocks);
 
 	idefloppy_init_pc(pc);
-	if (test_bit(IDEFLOPPY_USE_READ12, &floppy->flags)) {
+	if ((1UL << IDEFLOPPY_USE_READ12) & floppy->flags) {
 		pc->c[0] = cmd == READ ? GPCMD_READ_12 : GPCMD_WRITE_12;
 		put_unaligned(cpu_to_be32(blocks), (unsigned int *) &pc->c[6]);
 	} else {
@@ -878,10 +881,10 @@ static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
 	pc->rq = rq;
 	pc->b_count = cmd == READ ? 0 : rq->bio->bi_size;
 	if (rq->cmd_flags & REQ_RW)
-		set_bit(PC_WRITING, &pc->flags);
+		pc->flags |= 1UL << PC_WRITING;
 	pc->buffer = NULL;
 	pc->request_transfer = pc->buffer_size = blocks * floppy->block_size;
-	set_bit(PC_DMA_RECOMMENDED, &pc->flags);
+	pc->flags |= 1UL << PC_DMA_RECOMMENDED;
 }
 
 static void idefloppy_blockpc_cmd(idefloppy_floppy_t *floppy,
@@ -893,10 +896,10 @@ static void idefloppy_blockpc_cmd(idefloppy_floppy_t *floppy,
 	pc->rq = rq;
 	pc->b_count = rq->data_len;
 	if (rq->data_len && rq_data_dir(rq) == WRITE)
-		set_bit(PC_WRITING, &pc->flags);
+		pc->flags |= 1UL << PC_WRITING;
 	pc->buffer = rq->data;
 	if (rq->bio)
-		set_bit(PC_DMA_RECOMMENDED, &pc->flags);
+		pc->flags |= 1UL << PC_DMA_RECOMMENDED;
 
 	/*
 	 * possibly problematic, doesn't look like ide-floppy correctly handled
@@ -1035,7 +1038,7 @@ static int idefloppy_get_sfrp_bit(ide_drive_t *drive)
 	idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_CAPABILITIES_PAGE,
 						 MODE_SENSE_CURRENT);
 
-	set_bit(PC_SUPPRESS_ERROR, &pc.flags);
+	pc.flags |= 1UL << PC_SUPPRESS_ERROR;
 	if (idefloppy_queue_pc_tail(drive, &pc))
 		return 1;
 
@@ -1078,7 +1081,7 @@ static int idefloppy_get_capacity(ide_drive_t *drive)
 		switch (pc.buffer[desc_start + 4] & 0x03) {
 		/* Clik! drive returns this instead of CAPACITY_CURRENT */
 		case CAPACITY_UNFORMATTED:
-			if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags))
+			if (!((1UL << IDEFLOPPY_CLIK_DRIVE) & floppy->flags))
 				/*
 				 * If it is not a clik drive, break out
 				 * (maintains previous driver behaviour)
@@ -1126,7 +1129,7 @@ static int idefloppy_get_capacity(ide_drive_t *drive)
 	}
 
 	/* Clik! disk does not support get_flexible_disk_page */
-	if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags))
+	if (!((1UL << IDEFLOPPY_CLIK_DRIVE) & floppy->flags))
 		(void) idefloppy_get_flexible_disk_page(drive);
 
 	set_capacity(floppy->disk, floppy->blocks * floppy->bs_factor);
@@ -1356,7 +1359,7 @@ static void idefloppy_setup(ide_drive_t *drive, idefloppy_floppy_t *floppy)
 	*((u16 *) &gcw) = drive->id->config;
 	floppy->pc = floppy->pc_stack;
 	if (gcw.drq_type == 1)
-		set_bit(IDEFLOPPY_DRQ_INTERRUPT, &floppy->flags);
+		floppy->flags |= 1UL << IDEFLOPPY_DRQ_INTERRUPT;
 
 	/*
 	 * We used to check revisions here. At this point however I'm giving up.
@@ -1369,7 +1372,7 @@ static void idefloppy_setup(ide_drive_t *drive, idefloppy_floppy_t *floppy)
 	 */
 
 	if (!strncmp(drive->id->model, "IOMEGA ZIP 100 ATAPI", 20)) {
-		set_bit(IDEFLOPPY_ZIP_DRIVE, &floppy->flags);
+		floppy->flags |= 1UL << IDEFLOPPY_ZIP_DRIVE;
 		/* This value will be visible in the /proc/ide/hdx/settings */
 		floppy->ticks = IDEFLOPPY_TICKS_DELAY;
 		blk_queue_max_sectors(drive->queue, 64);
@@ -1381,7 +1384,7 @@ static void idefloppy_setup(ide_drive_t *drive, idefloppy_floppy_t *floppy)
 	 */
 	if (strncmp(drive->id->model, "IOMEGA Clik!", 11) == 0) {
 		blk_queue_max_sectors(drive->queue, 64);
-		set_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags);
+		floppy->flags |= 1UL << IDEFLOPPY_CLIK_DRIVE;
 	}
 
 	(void) idefloppy_get_capacity(drive);
@@ -1471,7 +1474,7 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
 	floppy->openers++;
 
 	if (floppy->openers == 1) {
-		clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
+		floppy->flags &= (1UL << IDEFLOPPY_FORMAT_IN_PROGRESS) ^ ~0UL;
 		/* Just in case */
 
 		idefloppy_create_test_unit_ready_cmd(&pc);
@@ -1496,14 +1499,14 @@ static int idefloppy_open(struct inode *inode, struct file *filp)
 			ret = -EROFS;
 			goto out_put_floppy;
 		}
-		set_bit(IDEFLOPPY_MEDIA_CHANGED, &floppy->flags);
+		floppy->flags |= 1UL << IDEFLOPPY_MEDIA_CHANGED;
 		/* IOMEGA Clik! drives do not support lock/unlock commands */
-		if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
+		if (!((1UL << IDEFLOPPY_CLIK_DRIVE) & floppy->flags)) {
 			idefloppy_create_prevent_cmd(&pc, 1);
 			(void) idefloppy_queue_pc_tail(drive, &pc);
 		}
 		check_disk_change(inode->i_bdev);
-	} else if (test_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags)) {
+	} else if ((1UL << IDEFLOPPY_FORMAT_IN_PROGRESS) & floppy->flags) {
 		ret = -EBUSY;
 		goto out_put_floppy;
 	}
@@ -1526,12 +1529,12 @@ static int idefloppy_release(struct inode *inode, struct file *filp)
 
 	if (floppy->openers == 1) {
 		/* IOMEGA Clik! drives do not support lock/unlock commands */
-		if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
+		if (!((1UL << IDEFLOPPY_CLIK_DRIVE) & floppy->flags)) {
 			idefloppy_create_prevent_cmd(&pc, 0);
 			(void) idefloppy_queue_pc_tail(drive, &pc);
 		}
 
-		clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
+		floppy->flags &= (1UL << IDEFLOPPY_FORMAT_IN_PROGRESS) ^ ~0UL;
 	}
 
 	floppy->openers--;
@@ -1560,7 +1563,7 @@ static int idefloppy_lockdoor(idefloppy_floppy_t *floppy, idefloppy_pc_t *pc,
 
 	/* The IOMEGA Clik! Drive doesn't support this command -
 	 * no room for an eject mechanism */
-	if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags)) {
+	if (!((1UL << IDEFLOPPY_CLIK_DRIVE) & floppy->flags)) {
 		int prevent = (arg) ? 1 : 0;
 
 		if (cmd == CDROMEJECT)
@@ -1586,11 +1589,11 @@ static int idefloppy_format_unit(idefloppy_floppy_t *floppy, unsigned long arg)
 
 	if (floppy->openers > 1) {
 		/* Don't format if someone is using the disk */
-		clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
+		floppy->flags &= (1UL << IDEFLOPPY_FORMAT_IN_PROGRESS) ^ ~0UL;
 		return -EBUSY;
 	}
 
-	set_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
+	floppy->flags |= 1UL << IDEFLOPPY_FORMAT_IN_PROGRESS;
 
 	/*
 	 * Send ATAPI_FORMAT_UNIT to the drive.
@@ -1624,7 +1627,7 @@ static int idefloppy_format_unit(idefloppy_floppy_t *floppy, unsigned long arg)
 
 out:
 	if (err)
-		clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
+		floppy->flags &= (1UL << IDEFLOPPY_FORMAT_IN_PROGRESS) ^ ~0UL;
 	return err;
 }
 
@@ -1661,13 +1664,18 @@ static int idefloppy_media_changed(struct gendisk *disk)
 {
 	struct ide_floppy_obj *floppy = ide_floppy_g(disk);
 	ide_drive_t *drive = floppy->drive;
+	int ret;
 
 	/* do not scan partitions twice if this is a removable device */
 	if (drive->attach) {
 		drive->attach = 0;
 		return 0;
 	}
-	return test_and_clear_bit(IDEFLOPPY_MEDIA_CHANGED, &floppy->flags);
+	/* test_and_clear_bit */
+	ret = (1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags;
+	floppy->flags &= (1UL << IDEFLOPPY_MEDIA_CHANGED) ^ ~0UL;
+
+	return ret;
 }
 
 static int idefloppy_revalidate_disk(struct gendisk *disk)
-- 
1.5.3.7

Re: [PATCH 04/21] ide-floppy: cleanup and unify debugging macro calls

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> * some debug_log() calls were not using "ide-floppy: " prefix
> 
> * a few used printk levels different than KERN_INFO (KERN_NOTICE
>   and KERN_ERR, which is the default one if no level is given)
> 
> There should be no functional change resulting from this patch.

Hmm, but there are functional changes as noted above, I removed this line
from the patch description.

> Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
> ---

with IDEFLOPPY_DEBUG_LOG set to 1:

drivers/ide/ide-floppy.c: In function ‘idefloppy_pc_intr’:
drivers/ide/ide-floppy.c:704: warning: too many arguments for format
drivers/ide/ide-floppy.c: In function ‘idefloppy_do_request’:
drivers/ide/ide-floppy.c:1126: error: ‘struct request’ has no member named ‘flags’
make[1]: *** [drivers/ide/ide-floppy.o] Error 1
make: *** [drivers/ide/ide-floppy.o] Error 2

which translate to:

[...]

>  		if ((stat & ERR_STAT) || test_bit(PC_DMA_ERROR, &pc->flags)) {
>  			/* Error detected */
> -			debug_log(KERN_INFO "ide-floppy: %s: I/O error\n",
> -				drive->name);
> +			debug_log("I/O error\n", drive->name);

"%s: I/O error\n"

[...]

> -	debug_log(KERN_INFO "dev: %s, flags: %lx, errors: %d\n",
> +	debug_log("dev: %s, flags: %lx, errors: %d\n",
>  			rq->rq_disk ? rq->rq_disk->disk_name : "?",
>  			rq->flags, rq->errors);

This was broken before this patch by rq->flags -> rq->cmd_type change
(so your patch is not to blame for breaking IDEFLOPPY_DEBUG_LOG ;).

I fixed the above issues while merging the patch.

Re: [PATCH 06/21] ide-floppy: remove struct idefloppy_flexible_disk_page

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> The driver used to test whether the flexible disk page has changed by memcmp-ing
> it with a cached copy of a previous version of the page from a different remo-
> vable medium. Since, according to the SFF-8070i spec, the flexible disk page
> "specifies parameters relating to the currently installed medium type," this
> comparison is now done by simply checking whether the medium has changed.
> 
> Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
> ---
>  drivers/ide/ide-floppy.c |   89 ++++++++++++++++-----------------------------
>  1 files changed, 32 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 2b9885f..679d48e 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c

[...]

> @@ -1188,50 +1159,54 @@ static int idefloppy_queue_pc_tail (ide_drive_t *drive,idefloppy_pc_t *pc)
>  }
>  
>  /*
> - *	Look at the flexible disk page parameters. We will ignore the CHS
> - *	capacity parameters and use the LBA parameters instead.
> + * Look at the flexible disk page parameters. We will ignore the CHS capacity
> + * parameters and use the LBA parameters instead.
>   */
> -static int idefloppy_get_flexible_disk_page (ide_drive_t *drive)
> +static int idefloppy_get_flexible_disk_page(ide_drive_t *drive)

Care to rename it to ide_floppy_get_flexible_disk_page() while at it
(it has only one user)?

>  {
>  	idefloppy_floppy_t *floppy = drive->driver_data;
>  	idefloppy_pc_t pc;
> -	idefloppy_mode_parameter_header_t *header;
> -	idefloppy_flexible_disk_page_t *page;
>  	int capacity, lba_capacity;
> +	u8 heads, sectors;
> +	u16 transfer_rate, sector_size, cyls, rpm;

some CodingStyle nitpicks (not really that important, rather personal taste):

	u16 transfer_rate, sector_size, cyls, rpm;
	u8 heads, sectors;

> -	idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_FLEXIBLE_DISK_PAGE, MODE_SENSE_CURRENT);
> -	if (idefloppy_queue_pc_tail(drive,&pc)) {
> -		printk(KERN_ERR "ide-floppy: Can't get flexible disk "
> -			"page parameters\n");
> +	idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_FLEXIBLE_DISK_PAGE,
> +			MODE_SENSE_CURRENT);

	idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_FLEXIBLE_DISK_PAGE,
					MODE_SENSE_CURRENT);

> +	if (idefloppy_queue_pc_tail(drive, &pc)) {
> +		printk(KERN_ERR "ide-floppy: Can't get flexible disk page"
> +				" parameters\n");
>  		return 1;
>  	}
> -	header = (idefloppy_mode_parameter_header_t *) pc.buffer;
> -	floppy->wp = header->wp;
> +	floppy->wp = pc.buffer[3] & 0x80;

This is not an equivalent transformation:

header->wp is 0 or 1
pc.buffer[3] & 0x80 is 0 or 0x80

It seems to work fine for ->wp (because it is needlessly defined as 'int')
but may seriously confuse set_disk_ro() and thus bdev_read_only() users.

Should be fixed to '(pc.buffer[3] & 0x80) ? 1 : 0' (or something similar).

>  	set_disk_ro(floppy->disk, floppy->wp);
> -	page = (idefloppy_flexible_disk_page_t *) (header + 1);
> -
> -	page->transfer_rate = be16_to_cpu(page->transfer_rate);
> -	page->sector_size = be16_to_cpu(page->sector_size);
> -	page->cyls = be16_to_cpu(page->cyls);
> -	page->rpm = be16_to_cpu(page->rpm);
> -	capacity = page->cyls * page->heads * page->sectors * page->sector_size;
> -	if (memcmp (page, &floppy->flexible_disk_page, sizeof (idefloppy_flexible_disk_page_t)))
> +
> +	transfer_rate = be16_to_cpu(*(u16 *)&pc.buffer[8 + 2]);
> +	sector_size   = be16_to_cpu(*(u16 *)&pc.buffer[8 + 6]);
> +	cyls          = be16_to_cpu(*(u16 *)&pc.buffer[8 + 8]);
> +	rpm           = be16_to_cpu(*(u16 *)&pc.buffer[8 + 28]);
> +	heads         = pc.buffer[8 + 4];
> +	sectors       = pc.buffer[8 + 5];
> +
> +	capacity = cyls * heads * sectors * sector_size;
> +
> +	if ((1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags)

IDEFLOPPY_MEDIA_CHANGED is set when block device is opened for the first
time (please check idefloppy_open() for details) so I don't think it is
the right change.  'Flexible Disk Page' is only 32 bytes so we are better
off with leaving 'u8 flexible_disk_page[32]' in idefloppy_floppy_t and
doing things the old way.

Besides please do not intermix real changes like the above one with purely
cleanup ones like idefloppy_flexible_disk_page_t removal.  This is bad from
maintainability point of view.  If some patch causes problems it is easier
to narrow it down by heaving purely cleanup changes separated out + if we
would need to revert the real change we would have to make a separate patch
doing it instead of just reverting the guilty commit (given that we don't
want cleanup changes to be reverted as well).

>  		printk(KERN_INFO "%s: %dkB, %d/%d/%d CHS, %d kBps, "
>  				"%d sector size, %d rpm\n",
> -			drive->name, capacity / 1024, page->cyls,
> -			page->heads, page->sectors,
> -			page->transfer_rate / 8, page->sector_size, page->rpm);
> -
> -	floppy->flexible_disk_page = *page;
> -	drive->bios_cyl = page->cyls;
> -	drive->bios_head = page->heads;
> -	drive->bios_sect = page->sectors;
> +			drive->name, capacity / 1024, cyls, heads, sectors,
> +			transfer_rate / 8, sector_size, rpm);

more CodingStyle nitpicks:

		printk(KERN_INFO "%s: %dkB, %d/%d/%d CHS, %d kBps, "
				 "%d sector size, %d rpm\n",
				 drive->name, capacity / 1024, cyls, heads, sectors,
				 transfer_rate / 8, sector_size, rpm);

would be more readable IMO

> +	drive->bios_cyl = cyls;
> +	drive->bios_head = heads;
> +	drive->bios_sect = sectors;

extra newline would distinguish the above block from the code below

>  	lba_capacity = floppy->blocks * floppy->block_size;
> +
>  	if (capacity < lba_capacity) {
>  		printk(KERN_NOTICE "%s: The disk reports a capacity of %d "
>  			"bytes, but the drive only handles %d\n",
>  			drive->name, lba_capacity, capacity);
> -		floppy->blocks = floppy->block_size ? capacity / floppy->block_size : 0;
> +		floppy->blocks = floppy->block_size ?
> +			capacity / floppy->block_size : 0;
>  	}
>  	return 0;
>  }

Otherwise looks fine, please recast and resubmit.

Re: [PATCH 07/21] ide-floppy: remove struct idefloppy_capacity_descriptor

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> We test here for updated capacity descriptors by checking whether the media
> has changed instead of memcmp-ing with a cached copy of the capacity
> descriptors.
> 
> Also:
> 
> - remove one of 2 consecutive if (!i)-tests.
> - start loop at 1 in idefloppy_get_format_capacities() since userspace doesn't
> need the current/maximum capacity descriptor. i.e the first one.
> - fix comments formatting
> 
> Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
> ---
>  drivers/ide/ide-floppy.c |  132 +++++++++++++++++----------------------------
>  1 files changed, 50 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 679d48e..5c85833 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -119,29 +119,7 @@ typedef struct idefloppy_packet_command_s {
>  
>  #define	PC_SUPPRESS_ERROR		6	/* Suppress error reporting */
>  
> -/*
> - *	Format capacity
> - */
> -typedef struct {
> -	u8		reserved[3];
> -	u8		length;			/* Length of the following descriptors in bytes */
> -} idefloppy_capacity_header_t;

minor nitpick:
idefloppy_capacity_header_t removal wasn't mentioned in the patch description

[...]

> @@ -1229,17 +1205,16 @@ static int idefloppy_get_sfrp_bit(ide_drive_t *drive)
>  }
>  
>  /*
> - *	Determine if a media is present in the floppy drive, and if so,
> - *	its LBA capacity.
> + * Determine if a media is present in the floppy drive, and if so, its LBA
> + * capacity.
>   */
> -static int idefloppy_get_capacity (ide_drive_t *drive)
> +static int idefloppy_get_capacity(ide_drive_t *drive)

Care to rename it to ide_floppy_get_capacity() while at it
(it has only two users)?

>  {
>  	idefloppy_floppy_t *floppy = drive->driver_data;
>  	idefloppy_pc_t pc;
> -	idefloppy_capacity_header_t *header;
> -	idefloppy_capacity_descriptor_t *descriptor;
> -	int i, descriptors, rc = 1, blocks, length;
> -	
> +	int i, desc_cnt, rc = 1, blocks, length;
> +	u8 header_len;

desc_cnt (== header_len / 8) can be u8 as well

>  	drive->bios_cyl = 0;
>  	drive->bios_head = drive->bios_sect = 0;
>  	floppy->blocks = 0;
> @@ -1251,17 +1226,17 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
>  		printk(KERN_ERR "ide-floppy: Can't get floppy parameters\n");
>  		return 1;
>  	}
> -	header = (idefloppy_capacity_header_t *) pc.buffer;
> -	descriptors = header->length / sizeof(idefloppy_capacity_descriptor_t);
> -	descriptor = (idefloppy_capacity_descriptor_t *) (header + 1);
> +	header_len = pc.buffer[3];
> +	desc_cnt = header_len / 8; /* capacity descriptor of 8 bytes */
>  
> -	for (i = 0; i < descriptors; i++, descriptor++) {
> -		blocks = descriptor->blocks = be32_to_cpu(descriptor->blocks);
> -		length = descriptor->length = be16_to_cpu(descriptor->length);
> +	for (i = 0; i < desc_cnt; i++) {
> +		unsigned int desc_start = 4 + i*8;

missing newline

> +		blocks = be32_to_cpu(*(u32 *)&pc.buffer[desc_start]);
> +		length = be16_to_cpu(*(u16 *)&pc.buffer[desc_start + 6]);

if the last debug_log() call in the loop will be moved here
  
> -		if (!i) 
> +		if (!i)
>  		{

the 'if (!i)' can be replaced with:

		if (i)
			continue;

> -		switch (descriptor->dc) {
> +		switch (pc.buffer[desc_start + 4] & 0x03) {
>  		/* Clik! drive returns this instead of CAPACITY_CURRENT */
>  		case CAPACITY_UNFORMATTED:
>  			if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags))
> @@ -1272,11 +1247,11 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
>  				break;
>  		case CAPACITY_CURRENT:
>  			/* Normal Zip/LS-120 disks */
> -			if (memcmp(descriptor, &floppy->capacity, sizeof (idefloppy_capacity_descriptor_t)))
> +			if ((1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags)

Same issue as with similar change in patch #6:

[ Please note that idefloppy_get_capacity() is called from idefloppy_setup()
  and IDEFLOPPY_MEDIA_CHANGED flag is first set in idefloppy_open(). ]

IMO it is the best to leave floppy->capacity alone for now.

>  				printk(KERN_INFO "%s: %dkB, %d blocks, %d "
>  					"sector size\n", drive->name,
>  					blocks * length / 1024, blocks, length);
> -			floppy->capacity = *descriptor;
> +
>  			if (!length || length % 512) {
>  				printk(KERN_NOTICE "%s: %d bytes block size "
>  					"not supported\n", drive->name, length);
> @@ -1303,9 +1278,8 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
>  				"in drive\n", drive->name);
>  			break;
>  		}
> -		}
> -		if (!i) {
> -			debug_log("Descriptor 0 Code: %d\n", descriptor->dc);
> +		debug_log("Descriptor 0 Code: %d\n",
> +				pc.buffer[desc_start + 4] & 0x03);

minor CodingStyle nitpick:

		debug_log("Descriptor 0 Code: %d\n",
			  pc.buffer[desc_start + 4] & 0x03);

>  		}
>  		debug_log("Descriptor %d: %dkB, %d blocks, %d sector size\n",
>  				i, blocks * length / 1024, blocks, length);
> @@ -1321,35 +1295,32 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
>  }
>  
>  /*
> -** Obtain the list of formattable capacities.
> -** Very similar to idefloppy_get_capacity, except that we push the capacity
> -** descriptors to userland, instead of our own structures.
> -**
> -** Userland gives us the following structure:
> -**
> -** struct idefloppy_format_capacities {
> -**        int nformats;
> -**        struct {
> -**                int nblocks;
> -**                int blocksize;
> -**                } formats[];
> -**        } ;
> -**
> -** userland initializes nformats to the number of allocated formats[]
> -** records.  On exit we set nformats to the number of records we've
> -** actually initialized.
> -**
> -*/
> + * Obtain the list of formattable capacities.
> + * Very similar to idefloppy_get_capacity, except that we push the capacity
> + * descriptors to userland, instead of our own structures.
> + *
> + * Userland gives us the following structure:
> + *
> + * struct idefloppy_format_capacities {
> + *        int nformats;
> + *        struct {
> + *                int nblocks;
> + *                int blocksize;
> + *                } formats[];
> + *        } ;

nitpicking taken to an extreme:
I would replace spaces with tabs while at it...

> + *
> + * userland initializes nformats to the number of allocated formats[]
> + * records.  On exit we set nformats to the number of records we've
> + * actually initialized.
> + *
> + */
>  
>  static int idefloppy_get_format_capacities(ide_drive_t *drive, int __user *arg)

ide_floppy_get_format_capacities() (only one user)? :)

>  {
>          idefloppy_pc_t pc;

would be nice to fix this whitespace damage while at it

> -	idefloppy_capacity_header_t *header;
> -        idefloppy_capacity_descriptor_t *descriptor;
> -	int i, descriptors, blocks, length;
> -	int u_array_size;
> -	int u_index;
> +	int i, desc_cnt, blocks, length, u_array_size, u_index;
>  	int __user *argp;
> +	u8 header_len;

desc_cnt can be u8
 
>  	if (get_user(u_array_size, arg))
>  		return (-EFAULT);
> @@ -1362,28 +1333,25 @@ static int idefloppy_get_format_capacities(ide_drive_t *drive, int __user *arg)
>  		printk(KERN_ERR "ide-floppy: Can't get floppy parameters\n");
>                  return (-EIO);
>          }

Care to fix the above whitespace damage while at it?

[...]

Otherwise it looks good.  Please recast/resubmit.

Re: [PATCH 00/21] ide-floppy redux v2

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> 
> Hi Bart,
> 
>    here's the second version of the ide-floppy refactoring trail. All the
> patches are based on the version of your quilt tree from the 05.01. Also, you've
> already applied patch 5 in this series but i'm submitting it still for the sake
> of completeness.
> 
> 
>  drivers/ide/ide-cd.c     |    2 -
>  drivers/ide/ide-floppy.c | 1403 ++++++++++++++++++----------------------------
>  include/linux/cdrom.h    |    1 +
>  include/linux/ide.h      |    3 +
>  4 files changed, 558 insertions(+), 851 deletions(-)

applied patches #1-4, #8
(#4 with some changes)

#5 was already applied so I just re-ordered it into the right spot

I have some comments for #6-7

Re: [PATCH 00/21] ide-floppy redux v2

[Bartlomiej Zolnierkiewicz]

Hi,

On Saturday 12 January 2008, Bartlomiej Zolnierkiewicz wrote:
> On Friday 11 January 2008, Borislav Petkov wrote:
> > 
> > Hi Bart,
> > 
> >    here's the second version of the ide-floppy refactoring trail. All the
> > patches are based on the version of your quilt tree from the 05.01. Also, you've
> > already applied patch 5 in this series but i'm submitting it still for the sake
> > of completeness.
> > 
> > 
> >  drivers/ide/ide-cd.c     |    2 -
> >  drivers/ide/ide-floppy.c | 1403 ++++++++++++++++++----------------------------
> >  include/linux/cdrom.h    |    1 +
> >  include/linux/ide.h      |    3 +
> >  4 files changed, 558 insertions(+), 851 deletions(-)
> 
> applied patches #1-4, #8
> (#4 with some changes)
> 
> #5 was already applied so I just re-ordered it into the right spot
> 
> I have some comments for #6-7

applied #9, #15 and #17

some comments for #11-12, #14 and #18-21 in separate mails

#10 and #16 are fine but because they depend on unmerged patches
I'm unable to apply them currently

Overall: good job!  300 LOC removed from the driver, code size savings
and a lot of preparations for the future ATAPI handling unification. :)

Thanks,
Bart

PS1 Please rebase the patches still needing polishing on top of updated
IDE quilt tree, recast them and respin the patch series (no need to post
already merged patches).

PS2 what happend to "fix DMA error reporting" patch? (#13 is missing, hmm?)

Re: [PATCH 06/21] ide-floppy: remove struct idefloppy_flexible_disk_page

[Bartlomiej Zolnierkiewicz]

On Saturday 12 January 2008, Bartlomiej Zolnierkiewicz wrote:

[...]

> > -	header = (idefloppy_mode_parameter_header_t *) pc.buffer;
> > -	floppy->wp = header->wp;
> > +	floppy->wp = pc.buffer[3] & 0x80;
> 
> This is not an equivalent transformation:
> 
> header->wp is 0 or 1
> pc.buffer[3] & 0x80 is 0 or 0x80
> 
> It seems to work fine for ->wp (because it is needlessly defined as 'int')
> but may seriously confuse set_disk_ro() and thus bdev_read_only() users.
> 
> Should be fixed to '(pc.buffer[3] & 0x80) ? 1 : 0' (or something similar).

Update: this change belongs to patch #10 (+ the need for such change in
patch #6 is a hint that #10 should be before #6)

Re: [PATCH 11/21] ide-floppy: fix comments formatting

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> That is,
> - remove unnecessary comments
> - shorten comments
> - shorten lines longer 80 columns
> - cleanup whitespace
> - add a missing loglevel KERN_ to a printk-call
> - fix misc checkpatch warnings

Majority of this patch consists of checkpatch.pl fixes so it should be merged
with patch #20.  Also a lot of code beautified here is _heavily_ modified in
later patches so some of fixups below could be moved there (which would also
decrease the size of this patch significantly).

> Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
> ---
>  drivers/ide/ide-floppy.c |  402 +++++++++++++++++++++-------------------------
>  1 files changed, 181 insertions(+), 221 deletions(-)

[...]

> +#define	PC_ABORT		0	/* Set when an error is considered\
> +					   normal - We won't retry */

'\' shouldn't be there and

/* ... */
#define		PC_ABORT		0

would be probably more readable

> +#define PC_DMA_RECOMMENDED	2	/* DMA use preferred, if possible */

please make it match the other flags while at it (space -> tab)

[...]

> -#define IDEFLOPPY_USE_READ12		2	/* Use READ12/WRITE12 or READ10/WRITE10 */
> +#define IDEFLOPPY_USE_READ12		2	/* READ(10|12)/WRITE(10|12) */

The original comment was way more informative.

Moreover this particular flag is never set so it could be just removed
(together with some dead code for handling it) in a separate (pre-)patch.

>  #define	IDEFLOPPY_FORMAT_IN_PROGRESS	3	/* Format in progress */

please make it match the other flags while at it (tab -> space)

> -#define IDEFLOPPY_CLIK_DRIVE	        4       /* Avoid commands not supported in Clik drive */
> -#define IDEFLOPPY_ZIP_DRIVE		5	/* Requires BH algorithm for packets */
> +#define IDEFLOPPY_CLIK_DRIVE	        4       /* Avoid commands not supported\
> +						   in Clik drive */
> +#define IDEFLOPPY_ZIP_DRIVE		5	/* Requires BH algorithm for\
> +						   packets */

no need for '\' characters

[...]

> -static void idefloppy_queue_pc_head (ide_drive_t *drive,idefloppy_pc_t *pc,struct request *rq)
> +static void idefloppy_queue_pc_head(ide_drive_t *drive, idefloppy_pc_t *pc,
> +		struct request *rq)

minor CodingStyle nitpick:

static void idefloppy_queue_pc_head(ide_drive_t *drive, idefloppy_pc_t *pc,
				    struct request *rq)

would be more readable IMO but it is a matter of personal taste

[ same applies for other similar modifications done by this patch series ]

Re: [PATCH 12/21] ide-floppy: factor out ioctl handlers from idefloppy_ioctl()

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> By passing idefloppy_floppy_t *floppy to the factored out functions, we get
> rid of (almost) all local vars so stack usage should be at minimum here. Also,
> we merge idefloppy_begin_format() into idefloppy_format_start() since it is its
> only user.
> 
> Also,
> - remove unneeded scsi ioctl chunk

They are _needed_, despite the name these ioctls are _not_ limited
to SCSI subsystem.

[...]

> +		int prevent = (arg) ? 1 : 0;

parentheses are unnecessary

[...]

> +static int idefloppy_format_unit(idefloppy_floppy_t *floppy, unsigned long arg)

__user tag was dropped from 'arg'
(I bet that this would make sparse checking unhappy)

> +{
> +	int blocks, length, flags, err = 0;
> +	int __user *argp = (int __user *)arg;

wouldn't be needed if the 'arg' was of 'int __user' type and the casting was
done in the caller function

[...]

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

'goto out' is unnecessary

> +	}
> +
> +out:
> +	if (err)
> +		clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
> +	return err;
> +}

[...]

> -	/*
> -	 * skip SCSI_IOCTL_SEND_COMMAND (deprecated)
> -	 * and CDROM_SEND_PACKET (legacy) ioctls
> -	 */
> -	if (cmd != CDROM_SEND_PACKET && cmd != SCSI_IOCTL_SEND_COMMAND)
> -		err = scsi_cmd_ioctl(file, bdev->bd_disk->queue,
> -					bdev->bd_disk, cmd, argp);
> -	else
> -		err = -ENOTTY;
> -
> -	if (err == -ENOTTY)
> -		err = generic_ide_ioctl(drive, file, bdev, cmd, arg);
> -
> -	return err;
> +	return generic_ide_ioctl(drive, file, bdev, cmd, arg);

this whole chunk needs to be reverted

Please recast/resubmit.

Re: [PATCH 14/21] ide-floppy: mv idefloppy_{should_,}report_error

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> In addition to shortening the function name, move the printk-call into the
> function thereby saving some code lines. Also, make the function out_of_line
> since it is not on a performance critical path.
> 
> Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
> ---
>  drivers/ide/ide-floppy.c |   37 ++++++++++++++-----------------------
>  1 files changed, 14 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 49d83a1..b718615 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -707,16 +707,18 @@ static ide_startstop_t idefloppy_transfer_pc1(ide_drive_t *drive)
>  	return ide_started;
>  }
>  
> -/*
> - * Suppresses error messages resulting from Medium not present.
> - */
> -static inline int idefloppy_should_report_error(idefloppy_floppy_t *floppy)
> +static void idefloppy_report_error(idefloppy_floppy_t *floppy,
> +		idefloppy_pc_t *pc)
>  {

-> Would make a sense to move the comment here instead of removing it
(it is useful unless you remeber all ->{sense_key,asc,ascq} value).

>  	if (floppy->sense_key == 0x02 &&
>  	    floppy->asc       == 0x3a &&
>  	    floppy->ascq      == 0x00)
> -		return 0;
> -	return 1;
> +		return;

Otherwise the patch is fine.

Re: [PATCH 18/21] ide-floppy: fix error handling in idefloppy_probe()

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
> ---
>  drivers/ide/ide-floppy.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 89b26ea..0729df5 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -1737,7 +1737,8 @@ static int ide_floppy_probe(ide_drive_t *drive)
>  				" emulation.\n", drive->name);
>  		goto failed;
>  	}
> -	if ((floppy = kzalloc(sizeof (idefloppy_floppy_t), GFP_KERNEL)) == NULL) {
> +	floppy = kzalloc(sizeof(idefloppy_floppy_t), GFP_KERNEL);
> +	if (!floppy) {
>  		printk(KERN_ERR "ide-floppy: %s: Can't allocate a floppy"
>  				" structure\n", drive->name);
>  		goto failed;

I'm unable to see any problem with error handling here?

This change should be combined with the rest of checkpatch.pl fixes.

Re: [PATCH 19/21] ide-floppy: fix most of the remaining checkpatch.pl issues

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> i.e.,
> ERROR: switch and case should be at the same indent
> ERROR: need spaces around that '=' (ctx:VxV)
> ERROR: trailing statements should be on next line
> WARNING: no space between function name and open parenthesis '('
> WARNING: printk() should include KERN_ facility level
> ERROR: That open brace { should be on the previous line
> ERROR: use tabs not spaces
> ERROR: do not use assignment in if condition
> WARNING: braces {} are not necessary for single statement blocks
> ERROR: need space after that ',' (ctx:VxV)
> WARNING: line over 80 characters
> ERROR: do not use assignment in if condition
> ...

This should be the very last patch in the series
(and combined with patch #11).

> Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
> ---
>  drivers/ide/ide-floppy.c |  147 +++++++++++++++++++++++----------------------
>  1 files changed, 75 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 0729df5..3d9b1e5 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -47,13 +47,13 @@
>  #define IDEFLOPPY_DEBUG_INFO		0
>  #define IDEFLOPPY_DEBUG_BUGS		1
>  
> -#define IDEFLOPPY_DEBUG( fmt, args... )
> +#define IDEFLOPPY_DEBUG(fmt, args...)
>  
>  #if IDEFLOPPY_DEBUG_LOG
>  #define debug_log(fmt, args...) \
>  	printk(KERN_INFO "ide-floppy: " fmt, ## args)
>  #else
> -#define debug_log(fmt, args... ) do {} while(0)
> +#define debug_log(fmt, args...) do {} while (0)
>  #endif

Hmmm, these could have been dealt with in patch #4...

[...]

> @@ -1314,34 +1314,34 @@ static int idefloppy_identify_device (ide_drive_t *drive,struct hd_driveid *id)
>  #if IDEFLOPPY_DEBUG_INFO
>  	printk(KERN_INFO "Dumping ATAPI Identify Device floppy parameters\n");
>  	switch (gcw.protocol) {
> -		case 0: case 1: sprintf(buffer, "ATA");break;
> -		case 2:	sprintf(buffer, "ATAPI");break;
> -		case 3: sprintf(buffer, "Reserved (Unknown to ide-floppy)");break;
> +	case 0: case 1: sprintf(buffer, "ATA"); break;
> +	case 2:	sprintf(buffer, "ATAPI"); break;
> +	case 3: sprintf(buffer, "Reserved (Unknown to ide-floppy)"); break;
>  	}
>  	printk(KERN_INFO "Protocol Type: %s\n", buffer);
>  	switch (gcw.device_type) {
> -		case 0: sprintf(buffer, "Direct-access Device");break;
> -		case 1: sprintf(buffer, "Streaming Tape Device");break;
> -		case 2: case 3: case 4: sprintf (buffer, "Reserved");break;
> -		case 5: sprintf(buffer, "CD-ROM Device");break;
> -		case 6: sprintf(buffer, "Reserved");
> -		case 7: sprintf(buffer, "Optical memory Device");break;
> -		case 0x1f: sprintf(buffer, "Unknown or no Device type");break;
> -		default: sprintf(buffer, "Reserved");
> +	case 0: sprintf(buffer, "Direct-access Device"); break;
> +	case 1: sprintf(buffer, "Streaming Tape Device"); break;
> +	case 2: case 3: case 4: sprintf(buffer, "Reserved"); break;
> +	case 5: sprintf(buffer, "CD-ROM Device"); break;
> +	case 6: sprintf(buffer, "Reserved");
> +	case 7: sprintf(buffer, "Optical memory Device"); break;
> +	case 0x1f: sprintf(buffer, "Unknown or no Device type"); break;
> +	default: sprintf(buffer, "Reserved");
>  	}
>  	printk(KERN_INFO "Device Type: %x - %s\n", gcw.device_type, buffer);
>  	printk(KERN_INFO "Removable: %s\n", gcw.removable ? "Yes":"No");
>  	switch (gcw.drq_type) {
> -		case 0: sprintf(buffer, "Microprocessor DRQ");break;
> -		case 1: sprintf(buffer, "Interrupt DRQ");break;
> -		case 2: sprintf(buffer, "Accelerated DRQ");break;
> -		case 3: sprintf(buffer, "Reserved");break;
> +	case 0: sprintf(buffer, "Microprocessor DRQ"); break;
> +	case 1: sprintf(buffer, "Interrupt DRQ"); break;
> +	case 2: sprintf(buffer, "Accelerated DRQ"); break;
> +	case 3: sprintf(buffer, "Reserved"); break;
>  	}
>  	printk(KERN_INFO "Command Packet DRQ Type: %s\n", buffer);
>  	switch (gcw.packet_size) {
> -		case 0: sprintf(buffer, "12 bytes");break;
> -		case 1: sprintf(buffer, "16 bytes");break;
> -		default: sprintf(buffer, "Reserved");break;
> +	case 0: sprintf(buffer, "12 bytes"); break;
> +	case 1: sprintf(buffer, "16 bytes"); break;
> +	default: sprintf(buffer, "Reserved"); break;
>  	}
>  	printk(KERN_INFO "Command Packet Size: %s\n", buffer);
>  #endif /* IDEFLOPPY_DEBUG_INFO */

> @@ -1349,13 +1349,16 @@ static int idefloppy_identify_device (ide_drive_t *drive,struct hd_driveid *id)
>  	if (gcw.protocol != 2)
>  		printk(KERN_ERR "ide-floppy: Protocol is not ATAPI\n");
>  	else if (gcw.device_type != 0)
> -		printk(KERN_ERR "ide-floppy: Device type is not set to floppy\n");
> +		printk(KERN_ERR "ide-floppy: Device type is not set to"
> +				" floppy\n");
>  	else if (!gcw.removable)
>  		printk(KERN_ERR "ide-floppy: The removable flag is not set\n");
>  	else if (gcw.drq_type == 3) {
> -		printk(KERN_ERR "ide-floppy: Sorry, DRQ type %d not supported\n", gcw.drq_type);
> +		printk(KERN_ERR "ide-floppy: Sorry, DRQ type %d not"
> +				" supported\n", gcw.drq_type);
>  	} else if (gcw.packet_size != 0) {
> -		printk(KERN_ERR "ide-floppy: Packet size is not 12 bytes long\n");
> +		printk(KERN_ERR "ide-floppy: Packet size is not 12 bytes"
> +				" long\n");
>  	} else
>  		return 1;
>  	return 0;

If we convert the above code to dump gcw fields on error, i.e.

	if (gcw.protocol != 2)
		printk(KERN_ERR "ide-floppy: Protocol 0x%02 is not ATAPI\n",
				gcw.protocol);

we could just remove IDEFLOPPY_DEBUG_INFO (which is disabled by default).

Care to make a separate (pre-)patch?

Re: [PATCH 20/21] ide-floppy: merge idefloppy_{input,output}_buffers

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> We merge idefloppy_{input,output}_buffers() into idefloppy_io_buffers() by
> introducing a 4th arg. called direction. According to its value
> we atapi_input_bytes() or atapi_output_bytes(). Also, simplify the interrupt

This change is fine but ...

> handler by removing multiple calls testing the data direction and using a local
> variable instead.

... the patch replaces 'test_bit(PC_WRITING, &pc->flags)' check with
'rq_data_dir(rq) == WRITE' one.  While this may look as "trivial" change it
is not such.  It should be done only after auditing the driver and making
sure that we are not introducing subtle regressions (=> I see that some
commands are setting PC_WRITING but are not setting REQ_RW bit), especially
given that these changes were not tested with the real hardware.

Please separate this change to another (post-)patch.

PS It would also be nice to remove IDEFLOPPY_DEBUG_BUGS define in a pre-patch.

Re: [PATCH 21/21] ide-floppy: remove atomic test_*bit macros

[Bartlomiej Zolnierkiewicz]

On Friday 11 January 2008, Borislav Petkov wrote:
> This change is temporary and after unification of the IDE subsystem proper
> bit setting and testing macros will be introduced.
> 
> Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
> ---
>  drivers/ide/ide-floppy.c |   82 +++++++++++++++++++++++++---------------------
>  1 files changed, 45 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 4106eb4..29c1983 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -479,12 +479,12 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
>  
>  	debug_log("Reached %s interrupt handler\n", __FUNCTION__);
>  
> -	if (test_bit(PC_DMA_IN_PROGRESS, &pc->flags)) {
> +	if ((1UL << PC_DMA_IN_PROGRESS) & pc->flags) {

How's about introducing new defines i.e.

enum {
	IDE_FLOPPY_FLAG_PC_ABORT		= (1 << 0),
	IDE_FLOPPY_FLAG_PC_DMA_RECOMMENDED	= (1 << 1),
	IDE_FLOPPY_FLAG_PC_DMA_IN_PROGRESS	= (1 << 2),
	...
}

instead of open-coding the bit-shifts?

>  		dma_error = HWIF(drive)->ide_dma_end(drive);
>  		if (dma_error) {
>  			printk(KERN_ERR "%s: DMA %s error\n", drive->name,
>  					write ?	"write" : "read");
> -			set_bit(PC_DMA_ERROR, &pc->flags);
> +			pc->flags |= (1UL << PC_DMA_ERROR);
>  		} else {
>  			pc->actually_transferred = pc->request_transfer;
>  			idefloppy_update_buffers(drive, pc);
> @@ -499,11 +499,11 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
>  		/* No more interrupts */
>  		debug_log("Packet command completed, %d bytes transferred\n",
>  				pc->actually_transferred);
> -		clear_bit(PC_DMA_IN_PROGRESS, &pc->flags);
> +		pc->flags &= ((1UL << PC_DMA_IN_PROGRESS) ^ ~0UL);

Same can be achieved with:

		pc->flags &= ~(1 << PC_DMA_IN_PROGRESS);

Re: [PATCH 06/21] ide-floppy: remove struct idefloppy_flexible_disk_page

[Borislav Petkov]

[...]

> This is not an equivalent transformation:
> 
> header->wp is 0 or 1
> pc.buffer[3] & 0x80 is 0 or 0x80
> 
> It seems to work fine for ->wp (because it is needlessly defined as 'int')
> but may seriously confuse set_disk_ro() and thus bdev_read_only() users.
> 
> Should be fixed to '(pc.buffer[3] & 0x80) ? 1 : 0' (or something similar).

upps, sorry, that was silly. I changed it to:

        floppy->wp = !!(pc.buffer[3] & 0x80);

> >  	set_disk_ro(floppy->disk, floppy->wp);
> > -	page = (idefloppy_flexible_disk_page_t *) (header + 1);
> > -
> > -	page->transfer_rate = be16_to_cpu(page->transfer_rate);
> > -	page->sector_size = be16_to_cpu(page->sector_size);
> > -	page->cyls = be16_to_cpu(page->cyls);
> > -	page->rpm = be16_to_cpu(page->rpm);
> > -	capacity = page->cyls * page->heads * page->sectors * page->sector_size;
> > -	if (memcmp (page, &floppy->flexible_disk_page, sizeof (idefloppy_flexible_disk_page_t)))
> > +
> > +	transfer_rate = be16_to_cpu(*(u16 *)&pc.buffer[8 + 2]);
> > +	sector_size   = be16_to_cpu(*(u16 *)&pc.buffer[8 + 6]);
> > +	cyls          = be16_to_cpu(*(u16 *)&pc.buffer[8 + 8]);
> > +	rpm           = be16_to_cpu(*(u16 *)&pc.buffer[8 + 28]);
> > +	heads         = pc.buffer[8 + 4];
> > +	sectors       = pc.buffer[8 + 5];
> > +
> > +	capacity = cyls * heads * sectors * sector_size;
> > +
> > +	if ((1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags)
> 
> IDEFLOPPY_MEDIA_CHANGED is set when block device is opened for the first
> time (please check idefloppy_open() for details) so I don't think it is
> the right change.  'Flexible Disk Page' is only 32 bytes so we are better
> off with leaving 'u8 flexible_disk_page[32]' in idefloppy_floppy_t and
> doing things the old way.
> 
> Besides please do not intermix real changes like the above one with purely
> cleanup ones like idefloppy_flexible_disk_page_t removal.  This is bad from
> maintainability point of view.  If some patch causes problems it is easier
> to narrow it down by heaving purely cleanup changes separated out + if we
> would need to revert the real change we would have to make a separate patch
> doing it instead of just reverting the guilty commit (given that we don't
> want cleanup changes to be reverted as well).

How about we get rid of that chunk altogether? floppy->flexible_disk_page is
used only here for the purpose of printk-ing to the syslog and has no "real"
purpose otherwise. Do we need that info spewed into the syslog at all?

-- 
Regards/Gruß,
    Boris.

Re: [PATCH 00/21] ide-floppy redux v2

[Borislav Petkov]

On Sat, Jan 12, 2008 at 09:14:39PM +0100, Bartlomiej Zolnierkiewicz wrote:
[...]

> PS1 Please rebase the patches still needing polishing on top of updated
> IDE quilt tree, recast them and respin the patch series (no need to post
> already merged patches).

sure, will do.

> PS2 what happend to "fix DMA error reporting" patch? (#13 is missing, hmm?)

Yeah, this is strange. It seems git-send-email missed some of the patches. I had
to send #16,#17 manually but didn't notice #13 was also missing. Will send with
the next batch.

-- 
Regards/Gruß,
    Boris.

Re: [PATCH 18/21] ide-floppy: fix error handling in idefloppy_probe()

[Borislav Petkov]

On Sat, Jan 12, 2008 at 09:18:01PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Friday 11 January 2008, Borislav Petkov wrote:
> > Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
> > ---
> >  drivers/ide/ide-floppy.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> > index 89b26ea..0729df5 100644
> > --- a/drivers/ide/ide-floppy.c
> > +++ b/drivers/ide/ide-floppy.c
> > @@ -1737,7 +1737,8 @@ static int ide_floppy_probe(ide_drive_t *drive)
> >  				" emulation.\n", drive->name);
> >  		goto failed;
> >  	}
> > -	if ((floppy = kzalloc(sizeof (idefloppy_floppy_t), GFP_KERNEL)) == NULL) {
> > +	floppy = kzalloc(sizeof(idefloppy_floppy_t), GFP_KERNEL);
> > +	if (!floppy) {
> >  		printk(KERN_ERR "ide-floppy: %s: Can't allocate a floppy"
> >  				" structure\n", drive->name);
> >  		goto failed;
> 
> I'm unable to see any problem with error handling here?

I changed it simply because checkpatch.pl complains so:

ERROR: do not use assignment in if condition (+ if ((floppy = kzalloc(sizeof(idefloppy_floppy_t), GFP_KERNEL)) == NULL))
#1740: FILE: home/boris/tmp/ide-floppy.c:1740:
+       if ((floppy = kzalloc(sizeof (idefloppy_floppy_t), GFP_KERNEL)) == NULL)
{

> This change should be combined with the rest of checkpatch.pl fixes.
ok.

-- 
Regards/Gruß,
    Boris.

Re: [PATCH 06/21] ide-floppy: remove struct idefloppy_flexible_disk_page

[Bartlomiej Zolnierkiewicz]

On Saturday 12 January 2008, Borislav Petkov wrote:

[...]

> > >  	set_disk_ro(floppy->disk, floppy->wp);
> > > -	page = (idefloppy_flexible_disk_page_t *) (header + 1);
> > > -
> > > -	page->transfer_rate = be16_to_cpu(page->transfer_rate);
> > > -	page->sector_size = be16_to_cpu(page->sector_size);
> > > -	page->cyls = be16_to_cpu(page->cyls);
> > > -	page->rpm = be16_to_cpu(page->rpm);
> > > -	capacity = page->cyls * page->heads * page->sectors * page->sector_size;
> > > -	if (memcmp (page, &floppy->flexible_disk_page, sizeof (idefloppy_flexible_disk_page_t)))
> > > +
> > > +	transfer_rate = be16_to_cpu(*(u16 *)&pc.buffer[8 + 2]);
> > > +	sector_size   = be16_to_cpu(*(u16 *)&pc.buffer[8 + 6]);
> > > +	cyls          = be16_to_cpu(*(u16 *)&pc.buffer[8 + 8]);
> > > +	rpm           = be16_to_cpu(*(u16 *)&pc.buffer[8 + 28]);
> > > +	heads         = pc.buffer[8 + 4];
> > > +	sectors       = pc.buffer[8 + 5];
> > > +
> > > +	capacity = cyls * heads * sectors * sector_size;
> > > +
> > > +	if ((1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags)
> > 
> > IDEFLOPPY_MEDIA_CHANGED is set when block device is opened for the first
> > time (please check idefloppy_open() for details) so I don't think it is
> > the right change.  'Flexible Disk Page' is only 32 bytes so we are better
> > off with leaving 'u8 flexible_disk_page[32]' in idefloppy_floppy_t and
> > doing things the old way.
> > 
> > Besides please do not intermix real changes like the above one with purely
> > cleanup ones like idefloppy_flexible_disk_page_t removal.  This is bad from
> > maintainability point of view.  If some patch causes problems it is easier
> > to narrow it down by heaving purely cleanup changes separated out + if we
> > would need to revert the real change we would have to make a separate patch
> > doing it instead of just reverting the guilty commit (given that we don't
> > want cleanup changes to be reverted as well).
> 
> How about we get rid of that chunk altogether? floppy->flexible_disk_page is
> used only here for the purpose of printk-ing to the syslog and has no "real"
> purpose otherwise. Do we need that info spewed into the syslog at all?

Well, it has some debugging value since drive's capabilities are given in
'Flexible Disk Page' but fine with me given that this change is separated
out from idefloppy_flexible_disk_page_t removal and pushed at the end of
patch series.

Thanks,
Bart