2008-01-11 12:07:22

by Borislav Petkov

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

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

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

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 49d83a1..b718615 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -707,16 +707,18 @@ static ide_startstop_t idefloppy_transfer_pc1(ide_drive_t *drive)
return ide_started;
}

-/*
- * Suppresses error messages resulting from Medium not present.
- */
-static inline int idefloppy_should_report_error(idefloppy_floppy_t *floppy)
+static void idefloppy_report_error(idefloppy_floppy_t *floppy,
+ idefloppy_pc_t *pc)
{
if (floppy->sense_key == 0x02 &&
floppy->asc == 0x3a &&
floppy->ascq == 0x00)
- return 0;
- return 1;
+ return;
+
+ printk(KERN_ERR "ide-floppy: %s: I/O error, pc = %2x, key = %2x, "
+ "asc = %2x, ascq = %2x\n",
+ floppy->drive->name, pc->c[0], floppy->sense_key,
+ floppy->asc, floppy->ascq);
}

static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
@@ -741,15 +743,8 @@ static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
* a legitimate error code was received.
*/
if (!test_bit(PC_ABORT, &pc->flags)) {
- if (!test_bit(PC_SUPPRESS_ERROR, &pc->flags)) {
- if (idefloppy_should_report_error(floppy))
- printk(KERN_ERR "ide-floppy: %s: I/O error, "
- "pc = %2x, key = %2x, "
- "asc = %2x, ascq = %2x\n",
- drive->name, pc->c[0],
- floppy->sense_key,
- floppy->asc, floppy->ascq);
- }
+ if (!test_bit(PC_SUPPRESS_ERROR, &pc->flags))
+ idefloppy_report_error(floppy, pc);
/* Giving up */
pc->error = IDEFLOPPY_ERROR_GENERAL;
}
@@ -958,16 +953,12 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
rq->nr_sectors, rq->current_nr_sectors);

if (rq->errors >= ERROR_MAX) {
- if (floppy->failed_pc != NULL) {
- if (idefloppy_should_report_error(floppy))
- printk(KERN_ERR "ide-floppy: %s: I/O error, pc = %2x,"
- " key = %2x, asc = %2x, ascq = %2x\n",
- drive->name, floppy->failed_pc->c[0],
- floppy->sense_key, floppy->asc, floppy->ascq);
- }
+ if (floppy->failed_pc != NULL)
+ idefloppy_report_error(floppy, floppy->failed_pc);
else
printk(KERN_ERR "ide-floppy: %s: I/O error\n",
- drive->name);
+ drive->name);
+
idefloppy_do_end_request(drive, 0, 0);
return ide_stopped;
}
--
1.5.3.7


2008-01-11 12:07:43

by Borislav Petkov

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

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

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

Signed-off-by: Borislav Petkov <[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 b718615..6e8926a 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -234,12 +234,12 @@ static struct ide_floppy_obj *ide_floppy_get(struct gendisk *disk)
return floppy;
}

-static void ide_floppy_release(struct kref *);
+static void idefloppy_cleanup_obj(struct kref *);

static void ide_floppy_put(struct ide_floppy_obj *floppy)
{
mutex_lock(&idefloppy_ref_mutex);
- kref_put(&floppy->kref, ide_floppy_release);
+ kref_put(&floppy->kref, idefloppy_cleanup_obj);
mutex_unlock(&idefloppy_ref_mutex);
}

@@ -1431,7 +1431,7 @@ static void ide_floppy_remove(ide_drive_t *drive)
ide_floppy_put(floppy);
}

-static void ide_floppy_release(struct kref *kref)
+static void idefloppy_cleanup_obj(struct kref *kref)
{
struct ide_floppy_obj *floppy = to_ide_floppy(kref);
ide_drive_t *drive = floppy->drive;
--
1.5.3.7

2008-01-11 13:17:59

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 16/21] ide-floppy: use an xfer_func_t and io_buf_t typedefs in order to unify rw

Also, move xfer_func_t typedef to the ide.h since it is used by two drivers
now (more coming).

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/ide/ide-cd.c | 2 --
drivers/ide/ide-floppy.c | 37 ++++++++++++++++++-------------------
include/linux/ide.h | 3 +++
3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 3b3e6c1..7e53a03 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -604,8 +604,6 @@ static ide_startstop_t cdrom_transfer_packet_command (ide_drive_t *drive,
* Block read functions.
*/

-typedef void (xfer_func_t)(ide_drive_t *, void *, u32);
-
static void ide_cd_pad_transfer(ide_drive_t *drive, xfer_func_t *xf, int len)
{
while (len > 0) {
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 6e8926a..c889c16 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -491,19 +491,22 @@ static void idefloppy_retry_pc(ide_drive_t *drive)
idefloppy_queue_pc_head(drive, pc, rq);
}

-/*
- * The usual interrupt handler called during a packet command.
- */
+typedef void (io_buf_t)(ide_drive_t *, idefloppy_pc_t *, unsigned int);
+
+/* The usual interrupt handler called during a packet command. */
static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
{
idefloppy_floppy_t *floppy = drive->driver_data;
ide_hwif_t *hwif = drive->hwif;
idefloppy_pc_t *pc = floppy->pc;
struct request *rq = pc->rq;
+ xfer_func_t *xferfunc;
+ io_buf_t *iobuf_func;
unsigned int temp;
u16 bcount;
u8 stat, ireason;
int dma_error = 0;
+ int write = (rq_data_dir(rq) == WRITE) ? 1 : 0;

debug_log("Reached %s interrupt handler\n", __FUNCTION__);

@@ -511,9 +514,7 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
dma_error = HWIF(drive)->ide_dma_end(drive);
if (dma_error) {
printk(KERN_ERR "%s: DMA %s error\n", drive->name,
- rq_data_dir(rq) == WRITE ?
- "write" :
- "read");
+ write ? "write" : "read");
set_bit(PC_DMA_ERROR, &pc->flags);
} else {
pc->actually_transferred = pc->request_transfer;
@@ -568,7 +569,7 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
ireason = hwif->INB(IDE_IREASON_REG);

if (ireason & CD) {
- printk(KERN_ERR "ide-floppy: CoD != 0 in idefloppy_pc_intr\n");
+ printk(KERN_ERR "ide-floppy: CoD != 0 in %s\n", __FUNCTION__);
return ide_do_reset(drive);
}
if (((ireason & IO) == IO) == test_bit(PC_WRITING, &pc->flags)) {
@@ -600,20 +601,18 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
}
}
if (test_bit(PC_WRITING, &pc->flags)) {
- if (pc->buffer != NULL)
- /* Write the current buffer */
- hwif->atapi_output_bytes(drive, pc->current_position,
- bcount);
- else
- idefloppy_output_buffers(drive, pc, bcount);
+ xferfunc = hwif->atapi_output_bytes;
+ iobuf_func = &idefloppy_output_buffers;
} else {
- if (pc->buffer != NULL)
- /* Read the current buffer */
- hwif->atapi_input_bytes(drive, pc->current_position,
- bcount);
- else
- idefloppy_input_buffers(drive, pc, bcount);
+ xferfunc = hwif->atapi_input_bytes;
+ iobuf_func = &idefloppy_input_buffers;
}
+
+ if (pc->buffer != NULL)
+ xferfunc(drive, pc->current_position, bcount);
+ else
+ iobuf_func(drive, pc, bcount);
+
/* Update the current position */
pc->actually_transferred += bcount;
pc->current_position += bcount;
diff --git a/include/linux/ide.h b/include/linux/ide.h
index bfbc975..a6cf280 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -626,6 +626,9 @@ typedef struct hwif_s {
typedef ide_startstop_t (ide_handler_t)(ide_drive_t *);
typedef int (ide_expiry_t)(ide_drive_t *);

+/* used by ide-cd, ide-floppy, etc. */
+typedef void (xfer_func_t)(ide_drive_t *, void *, u32);
+
typedef struct hwgroup_s {
/* irq handler, if active */
ide_startstop_t (*handler)(ide_drive_t *);
--
1.5.3.7

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

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

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

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

Otherwise the patch is fine.