2008-01-11 12:03:49

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 00/21] ide-floppy redux v2


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


2008-01-11 12:04:18

by Borislav Petkov

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

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

Signed-off-by: Borislav Petkov <[email protected]>
---
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

2008-01-11 12:04:37

by Borislav Petkov

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

* 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 <[email protected]>
---
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

2008-01-11 12:04:52

by Borislav Petkov

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

Signed-off-by: Borislav Petkov <[email protected]>
---
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

2008-01-11 12:05:11

by Borislav Petkov

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

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 <[email protected]>
---
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 <[email protected]>
* Copyright (C) 2000-2002 Paul Bristow <[email protected]>
* 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

2008-01-11 12:05:38

by Borislav Petkov

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

Signed-off-by: Borislav Petkov <[email protected]>
---
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

2008-01-11 12:05:52

by Borislav Petkov

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

Signed-off-by: Borislav Petkov <[email protected]>
---
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

2008-01-11 12:06:13

by Borislav Petkov

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

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 <[email protected]>
---
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

2008-01-11 12:06:54

by Borislav Petkov

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

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 <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
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

2008-01-11 12:07:59

by Borislav Petkov

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

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

Signed-off-by: Borislav Petkov <[email protected]>
---
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

2008-01-11 12:08:22

by Borislav Petkov

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

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 <[email protected]>
---
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

2008-01-11 12:08:37

by Borislav Petkov

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

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 <[email protected]>
---
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

2008-01-11 12:09:35

by Borislav Petkov

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

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 <[email protected]>
---
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

2008-01-11 13:23:00

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 17/21] ide-floppy: include the proper headers

Signed-off-by: Borislav Petkov <[email protected]>
---
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 c889c16..89b26ea 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -37,9 +37,9 @@
#include <scsi/scsi_ioctl.h>

#include <asm/byteorder.h>
-#include <asm/irq.h>
-#include <asm/uaccess.h>
-#include <asm/io.h>
+#include <linux/irq.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
#include <asm/unaligned.h>

/* The following are used to debug the driver. */
--
1.5.3.7

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

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 <[email protected]>
> ---

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.

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


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 <[email protected]>
> ---
> 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.

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

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 <[email protected]>
> ---
> 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.

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

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

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


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

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

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)

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

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 <[email protected]>
> ---
> 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 ]

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

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.

2008-01-12 20:42:33

by Borislav Petkov

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


[...]

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

2008-01-12 20:54:41

by Borislav Petkov

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

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.

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

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