On Fri, 2007-12-28 at 01:18 +0100, Simon Holm Thøgersen wrote:
> > -   while ((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80) && (time_before(jiffies, 
> > timeout)))
> > +   while (gdrom_is_busy() && (time_before(jiffies, timeout)))
> You don't need those parentheses.
> > +           while (((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x88) != 8) &&
> > +                  (time_before(jiffies, timeout)))
> What about a nice telling function like gdrom_is_busy for this?

Perhaps (uncompiled/untested):

    Remove unnecessary parenthesis
    Remove GDROM: prefix from sense_texts
    Add function gdrom_data_request
    Check sense_key against sense_text array size

Signed-off-by: Joe Perches <[EMAIL PROTECTED]>
---
 drivers/cdrom/gdrom.c |   53 ++++++++++++++++++++++++++----------------------
 1 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c
index 59d26e0..ab95438 100644
--- a/drivers/cdrom/gdrom.c
+++ b/drivers/cdrom/gdrom.c
@@ -80,16 +80,15 @@ static const struct {
        int sense_key;
        const char * const text;
 } sense_texts[] = {
-       {NO_SENSE, "GDROM: OK"},
-       {RECOVERED_ERROR, "GDROM: Recovered from error"},
-       {NOT_READY, "GDROM: Device not ready"},
-       {MEDIUM_ERROR, "GDROM: Disk not ready"},
-       {HARDWARE_ERROR, "GDROM: Hardware error"},
-       {ILLEGAL_REQUEST, "GDROM: Command has failed"},
-       {UNIT_ATTENTION, "GDROM: Device needs attention - "
-                        "disk may have been changed"},
-       {DATA_PROTECT, "GDROM: Data protection error"},
-       {ABORTED_COMMAND, "GDROM: Command aborted"},
+       {NO_SENSE, "OK"},
+       {RECOVERED_ERROR, "Recovered from error"},
+       {NOT_READY, "Device not ready"},
+       {MEDIUM_ERROR, "Disk not ready"},
+       {HARDWARE_ERROR, "Hardware error"},
+       {ILLEGAL_REQUEST, "Command has failed"},
+       {UNIT_ATTENTION, "Device needs attention - disk may have been changed"},
+       {DATA_PROTECT, "Data protection error"},
+       {ABORTED_COMMAND, "Command aborted"},
 };
 
 static struct platform_device *pd;
@@ -140,11 +139,16 @@ static bool gdrom_is_busy(void)
        return (ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80) != 0;
 }
 
+static bool gdrom_data_request(void)
+{
+       return (ctrl_inb(GDROM_ALTSTATUS_REG) & 0x88) == 8;
+}
+
 static void gdrom_wait_clrbusy(void)
 {
        /* long timeouts - typical for a CD Rom */
        unsigned long timeout = jiffies + HZ * 60;
-       while (gdrom_is_busy() && (time_before(jiffies, timeout)))
+       while (gdrom_is_busy() && time_before(jiffies, timeout))
                cpu_relax();
 }
 
@@ -153,7 +157,7 @@ static void gdrom_wait_busy_sleeps(void)
        unsigned long timeout;
        /* Wait to get busy first */
        timeout = jiffies + HZ * 60;
-       while (!gdrom_is_busy() && (time_before(jiffies, timeout)))
+       while (!gdrom_is_busy() && time_before(jiffies, timeout))
                cpu_relax();
        /* Now wait for busy to clear */
        gdrom_wait_clrbusy();
@@ -186,8 +190,8 @@ static void gdrom_spicommand(void *spi_string, int buflen)
        /* Wait until we can go */
        gdrom_wait_clrbusy();
        ctrl_outb(GDROM_COM_PACKET, GDROM_STATUSCOMMAND_REG);
-       while ((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x88) != 8)
-               cpu_relax(); /* wait for DRQ to be set to 1 */
+       while (!gdrom_data_request())
+               cpu_relax();
        outsw(PHYSADDR(GDROM_DATA_REG), cmd, 6);
 }
 
@@ -354,7 +358,7 @@ static int gdrom_drivestatus(struct cdrom_device_info 
*cd_info, int ignore)
 static int gdrom_mediachanged(struct cdrom_device_info *cd_info, int ignore)
 {
        /* check the sense key */
-       return ((ctrl_inb(GDROM_ERROR_REG) & 0xF0) == 0x60);
+       return (ctrl_inb(GDROM_ERROR_REG) & 0xF0) == 0x60;
 }
 
 /* reset the G1 bus */
@@ -412,11 +416,13 @@ static int gdrom_getsense(short *bufstring)
                return -EIO;
        }
        sense_key = sense[1] & 0x0F;
-       printk(KERN_INFO "%s\n", sense_texts[sense_key].text);
+       if (sense_key < ARRAY_SIZE(sense_texts))
+               printk(KERN_INFO "GDROM: %s\n", sense_texts[sense_key].text);
+       else
+               printk(KERN_ERR "GDROM: Unknown sense key: %d\n", sense_key);
 
-       if (bufstring)
+       if (bufstring)                  /* return additional sense data */
                memcpy(bufstring, &sense[4], 2);
-                                       /* return additional sense data */
 
        if (sense_key < 2)
                return 0;
@@ -550,19 +556,18 @@ static void gdrom_readdisk_dma(struct work_struct *work)
                ctrl_outb(0, GDROM_INTSEC_REG);
                /* In multiple DMA transfers need to wait */
                timeout = jiffies + HZ / 2;
-               while (gdrom_is_busy() && (time_before(jiffies, timeout)))
+               while (gdrom_is_busy() && time_before(jiffies, timeout))
                        cpu_relax();
                ctrl_outb(GDROM_COM_PACKET, GDROM_STATUSCOMMAND_REG);
                timeout = jiffies + HZ / 2;
-               while (((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x88) != 8) &&
-                      (time_before(jiffies, timeout)))
-                       cpu_relax(); /* wait for DRQ to be set to 1 */
+               while (!gdrom_data_request() && time_before(jiffies, timeout))
+                       cpu_relax();
                gd.pending = 1;
                gd.transfer = 1;
                outsw(PHYSADDR(GDROM_DATA_REG), &read_command->cmd, 6);
                timeout = jiffies + HZ / 2;
-               while ((ctrl_inb(GDROM_DMA_STATUS_REG)) &&
-                      (time_before(jiffies, timeout)))
+               while (ctrl_inb(GDROM_DMA_STATUS_REG) &&
+                      time_before(jiffies, timeout))
                        cpu_relax();
                ctrl_outb(1, GDROM_DMA_STATUS_REG);
                /* 5 second error margin here seems more reasonable */


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to