Re: [PATCH 1/3] ide-cd: debug log enhancements

Previous thread: Building two (external) modules from the same sources by Andrey Borzenkov on Sunday, September 28, 2008 - 10:49 pm. (5 messages)

Next thread: Re: [kernel-abi] symbol versioning vs. out-of-tree modules. by Alan Young on Sunday, September 28, 2008 - 11:51 pm. (2 messages)
From: Borislav Petkov
Date: Sunday, September 28, 2008 - 11:29 pm

Hi Bart,

here are some trivial enhancements to ide-cd which resulted from the debugging
sessions lately.

 drivers/ide/ide-cd.c |   44 ++++++++++++++++++++++++++------------------
 1 files changed, 26 insertions(+), 18 deletions(-)
--

From: Borislav Petkov
Date: Sunday, September 28, 2008 - 11:29 pm

... since HWGROUP(drive)->rq points to the same rq as in the irq handler. While
at it, remove stale comment.

There should be no functionality change resulting from this patch.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index e41b94d..bf92cd6 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -843,12 +843,11 @@ static void ide_cd_restore_request(ide_drive_t *drive, struct request *rq)
 	rq->q->prep_rq_fn(rq->q, rq);
 }
 
-/*
- * All other packet commands.
- */
-static void ide_cd_request_sense_fixup(ide_drive_t *drive, struct request *rq)
+static void ide_cd_request_sense_fixup(ide_drive_t *drive)
 {
 
+	struct request *rq = HWGROUP(drive)->rq;
+
 	ide_debug_log(IDE_DBG_FUNC, "Call %s, rq->cmd[0]: 0x%x\n",
 		      __func__, rq->cmd[0]);
 
@@ -1021,7 +1020,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
 			cdrom_end_request(drive, uptodate);
 			return ide_stopped;
 		} else if (!blk_pc_request(rq)) {
-			ide_cd_request_sense_fixup(drive, rq);
+			ide_cd_request_sense_fixup(drive);
 			/* complain if we still have data left to transfer */
 			uptodate = rq->data_len ? 0 : 1;
 		}
-- 
1.5.5.4

--

From: Bartlomiej Zolnierkiewicz
Date: Friday, October 3, 2008 - 10:29 am

Since we already have obtained rq pointer why not pass it to the helper
function instead of having the helper function waste cycles on it?


--

From: Borislav Petkov
Date: Sunday, September 28, 2008 - 11:29 pm

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 9ffce7f..e41b94d 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1784,7 +1784,7 @@ static int ide_cdrom_probe_capabilities(ide_drive_t *drive)
 	if ((cdi->mask & CDC_DVD_R) == 0 || (cdi->mask & CDC_DVD_RAM) == 0)
 		printk(KERN_CONT " DVD%s%s",
 				 (cdi->mask & CDC_DVD_R) ? "" : "-R",
-				 (cdi->mask & CDC_DVD_RAM) ? "" : "-RAM");
+				 (cdi->mask & CDC_DVD_RAM) ? "" : "/RAM");
 
 	if ((cdi->mask & CDC_CD_R) == 0 || (cdi->mask & CDC_CD_RW) == 0)
 		printk(KERN_CONT " CD%s%s",
-- 
1.5.5.4

--

From: Bartlomiej Zolnierkiewicz
Date: Friday, October 3, 2008 - 10:18 am

From: Borislav Petkov
Date: Saturday, October 4, 2008 - 1:28 am

It simply makes the format consistent with the lines below: CD-R/RW - which is
the more common way of enumerating supported media you find in different specs

-- 
Regards/Gruss,
    Boris.
--

From: Bartlomiej Zolnierkiewicz
Date: Sunday, October 5, 2008 - 8:54 am

Indeed, thanks for explaining it.
--

From: Borislav Petkov
Date: Sunday, September 28, 2008 - 11:29 pm

Add some more verbosity to key function calls in ide-cd debug code. While at it,
delete a superfluous comment.

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

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 474bb1d..9ffce7f 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -340,8 +340,8 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret)
 	}
 
 	ide_debug_log(IDE_DBG_RQ, "%s: stat: 0x%x, good_stat: 0x%x, "
-		      "rq->cmd_type: 0x%x, err: 0x%x\n", __func__, stat,
-		      good_stat, rq->cmd_type, err);
+		      "rq->cmd[0]: 0x%x, rq->cmd_type: 0x%x, err: 0x%x\n",
+		      __func__, stat, good_stat, rq->cmd[0], rq->cmd_type, err);
 
 	if (blk_sense_request(rq)) {
 		/*
@@ -849,7 +849,8 @@ static void ide_cd_restore_request(ide_drive_t *drive, struct request *rq)
 static void ide_cd_request_sense_fixup(ide_drive_t *drive, struct request *rq)
 {
 
-	ide_debug_log(IDE_DBG_FUNC, "Call %s\n", __func__);
+	ide_debug_log(IDE_DBG_FUNC, "Call %s, rq->cmd[0]: 0x%x\n",
+		      __func__, rq->cmd[0]);
 
 	/*
 	 * Some of the trailing request sense fields are optional,
@@ -876,7 +877,7 @@ int ide_cd_queue_pc(ide_drive_t *drive, const unsigned char *cmd,
 	if (!sense)
 		sense = &local_sense;
 
-	ide_debug_log(IDE_DBG_PC, "Call %s, rq->cmd[0]: 0x%x, write: 0x%x, "
+	ide_debug_log(IDE_DBG_PC, "Call %s, cmd[0]: 0x%x, write: 0x%x, "
 		      "timeout: %d, cmd_flags: 0x%x\n", __func__, cmd[0], write,
 		      timeout, cmd_flags);
 
@@ -1177,8 +1178,9 @@ static ide_startstop_t cdrom_start_rw(ide_drive_t *drive, struct request *rq)
 	unsigned short sectors_per_frame =
 		queue_hardsect_size(drive->queue) >> SECTOR_BITS;
 
-	ide_debug_log(IDE_DBG_RQ, "Call %s, write: 0x%x, secs_per_frame: %u\n",
-		      __func__, write, sectors_per_frame);
+	ide_debug_log(IDE_DBG_RQ, "Call %s, rq->cmd[0]: 0x%x, ...
From: Bartlomiej Zolnierkiewicz
Date: Friday, October 3, 2008 - 10:15 am

applied
--

Previous thread: Building two (external) modules from the same sources by Andrey Borzenkov on Sunday, September 28, 2008 - 10:49 pm. (5 messages)

Next thread: Re: [kernel-abi] symbol versioning vs. out-of-tree modules. by Alan Young on Sunday, September 28, 2008 - 11:51 pm. (2 messages)