Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Sun, Dec 30, 2007 at 01:38:24PM +, Adrian McMenamin wrote: > On Thu, 2007-12-27 at 14:58 -0800, Joe Perches wrote: > > On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote: > > > This patch adds support for the CD-Rom drive on the SEGA Dreamcast. > > > > Because it was already so close, might as well make it checkpatch clean. > > > > I also added a function gdrom_is_busy() to make a couple of tests > > fit on a single line and perhaps easier to read. > > > > Signed-off-by: Joe Perches <[EMAIL PROTECTED]> > > > Not really for me to decide if this gets applied - I'll leave that to > Paul as the maintainer. But I have no objection to them :) Both patches are fine with me, but it is ultimately Jens (as the drivers/cdrom authority) that will have to decide whether to sign off on the GDROM patches or not, and through which tree the patches will end up being merged. I can certainly merge them through my tree with his Acked-by if that's deemed the most convenient. -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, 2007-12-27 at 14:58 -0800, Joe Perches wrote: > On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote: > > This patch adds support for the CD-Rom drive on the SEGA Dreamcast. > > Because it was already so close, might as well make it checkpatch clean. > > I also added a function gdrom_is_busy() to make a couple of tests > fit on a single line and perhaps easier to read. > > Signed-off-by: Joe Perches <[EMAIL PROTECTED]> > Not really for me to decide if this gets applied - I'll leave that to Paul as the maintainer. But I have no objection to them :) -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, 2007-12-27 at 14:58 -0800, Joe Perches wrote: On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote: This patch adds support for the CD-Rom drive on the SEGA Dreamcast. Because it was already so close, might as well make it checkpatch clean. I also added a function gdrom_is_busy() to make a couple of tests fit on a single line and perhaps easier to read. Signed-off-by: Joe Perches [EMAIL PROTECTED] Not really for me to decide if this gets applied - I'll leave that to Paul as the maintainer. But I have no objection to them :) -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Sun, Dec 30, 2007 at 01:38:24PM +, Adrian McMenamin wrote: On Thu, 2007-12-27 at 14:58 -0800, Joe Perches wrote: On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote: This patch adds support for the CD-Rom drive on the SEGA Dreamcast. Because it was already so close, might as well make it checkpatch clean. I also added a function gdrom_is_busy() to make a couple of tests fit on a single line and perhaps easier to read. Signed-off-by: Joe Perches [EMAIL PROTECTED] Not really for me to decide if this gets applied - I'll leave that to Paul as the maintainer. But I have no objection to them :) Both patches are fine with me, but it is ultimately Jens (as the drivers/cdrom authority) that will have to decide whether to sign off on the GDROM patches or not, and through which tree the patches will end up being merged. I can certainly merge them through my tree with his Acked-by if that's deemed the most convenient. -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Sat, 2007-12-29 at 12:03 +, Adrian McMenamin wrote:
> This won't work see include/scsi/scsi.h
> /*
> * SENSE KEYS
> */
>
> #define NO_SENSE0x00
> #define RECOVERED_ERROR 0x01
> #define NOT_READY 0x02
> #define MEDIUM_ERROR0x03
> #define HARDWARE_ERROR 0x04
> #define ILLEGAL_REQUEST 0x05
> #define UNIT_ATTENTION 0x06
> #define DATA_PROTECT0x07
> #define BLANK_CHECK 0x08
> #define COPY_ABORTED0x0a
> #define ABORTED_COMMAND 0x0b
> #define VOLUME_OVERFLOW 0x0d
> #define MISCOMPARE 0x0e
>
> (The GD device specs says it supports 0, 1, 2, 3,4, 5, 6, 7 and 0xB)
>
> ie we could get a sense key of 0x0B which would be greater than the
> array size. I think you'd have to hard code the limit.
Then shouldn't this test be:
for (i = 0; i < ARRAY_SIZE(sense_texts); i++) {
if (sense_key == sense_texts[i].sense_key)
printk(KERN_INFO "GDROM: %s\n", sense_texts[i].text);
}
if (i >= ARRAY_SIZE(sense_texts))
printk(KERN_ERR "GDROM: Unknown sense key: %d\n", sense_key);
cheers, Joe
--
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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Sat, 2007-12-29 at 12:03 +, Adrian McMenamin wrote:
> On Fri, 2007-12-28 at 17:57 -0800, Joe Perches wrote:
>
> >
> > 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"},
> > };
>
> >
> > /* 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, [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), _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 */
> >
> >
> > -
>
>
> This won't work see include/scsi/scsi.h
>
>
> /*
> * SENSE KEYS
> */
>
> #define NO_SENSE0x00
> #define RECOVERED_ERROR 0x01
> #define NOT_READY 0x02
> #define MEDIUM_ERROR0x03
> #define HARDWARE_ERROR 0x04
> #define ILLEGAL_REQUEST 0x05
> #define UNIT_ATTENTION 0x06
> #define DATA_PROTECT0x07
> #define BLANK_CHECK 0x08
> #define COPY_ABORTED0x0a
> #define ABORTED_COMMAND 0x0b
> #define VOLUME_OVERFLOW 0x0d
> #define MISCOMPARE 0x0e
>
> (The GD device specs says it supports 0, 1, 2, 3,4, 5, 6, 7 and 0xB)
>
> ie we could get a sense key of 0x0B which would be greater than the
> array size. I think you'd have to hard code the limit.
>
>
Ignore that. Talking rubbish. As usual.
--
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
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Fri, 2007-12-28 at 17:57 -0800, Joe Perches wrote:
>
> 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"},
> };
>
> /* 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, [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), _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 */
>
>
> -
This won't work see include/scsi/scsi.h
/*
* SENSE KEYS
*/
#define NO_SENSE0x00
#define RECOVERED_ERROR 0x01
#define NOT_READY 0x02
#define MEDIUM_ERROR0x03
#define HARDWARE_ERROR 0x04
#define ILLEGAL_REQUEST 0x05
#define UNIT_ATTENTION 0x06
#define DATA_PROTECT0x07
#define BLANK_CHECK 0x08
#define COPY_ABORTED0x0a
#define ABORTED_COMMAND 0x0b
#define VOLUME_OVERFLOW 0x0d
#define MISCOMPARE 0x0e
(The GD device specs says it supports 0, 1, 2, 3,4, 5, 6, 7 and 0xB)
ie we could get a sense key of 0x0B which would be greater than the
array size. I think you'd have to hard code the limit.
--
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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Fri, 2007-12-28 at 17:57 -0800, Joe Perches wrote:
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},
};
/* 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 */
-
This won't work see include/scsi/scsi.h
/*
* SENSE KEYS
*/
#define NO_SENSE0x00
#define RECOVERED_ERROR 0x01
#define NOT_READY 0x02
#define MEDIUM_ERROR0x03
#define HARDWARE_ERROR 0x04
#define ILLEGAL_REQUEST 0x05
#define UNIT_ATTENTION 0x06
#define DATA_PROTECT0x07
#define BLANK_CHECK 0x08
#define COPY_ABORTED0x0a
#define ABORTED_COMMAND 0x0b
#define VOLUME_OVERFLOW 0x0d
#define MISCOMPARE 0x0e
(The GD device specs says it supports 0, 1, 2, 3,4, 5, 6, 7 and 0xB)
ie we could get a sense key of 0x0B which would be greater than the
array size. I think you'd have to hard code the limit.
--
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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Sat, 2007-12-29 at 12:03 +, Adrian McMenamin wrote:
On Fri, 2007-12-28 at 17:57 -0800, Joe Perches wrote:
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},
};
/* 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 */
-
This won't work see include/scsi/scsi.h
/*
* SENSE KEYS
*/
#define NO_SENSE0x00
#define RECOVERED_ERROR 0x01
#define NOT_READY 0x02
#define MEDIUM_ERROR0x03
#define HARDWARE_ERROR 0x04
#define ILLEGAL_REQUEST 0x05
#define UNIT_ATTENTION 0x06
#define DATA_PROTECT0x07
#define BLANK_CHECK 0x08
#define COPY_ABORTED0x0a
#define ABORTED_COMMAND 0x0b
#define VOLUME_OVERFLOW 0x0d
#define MISCOMPARE 0x0e
(The GD device specs says it supports 0, 1, 2, 3,4, 5, 6, 7 and 0xB)
ie we could get a sense key of 0x0B which would be greater than the
array size. I think you'd have to hard code the limit.
Ignore that. Talking rubbish. As usual.
--
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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Sat, 2007-12-29 at 12:03 +, Adrian McMenamin wrote:
This won't work see include/scsi/scsi.h
/*
* SENSE KEYS
*/
#define NO_SENSE0x00
#define RECOVERED_ERROR 0x01
#define NOT_READY 0x02
#define MEDIUM_ERROR0x03
#define HARDWARE_ERROR 0x04
#define ILLEGAL_REQUEST 0x05
#define UNIT_ATTENTION 0x06
#define DATA_PROTECT0x07
#define BLANK_CHECK 0x08
#define COPY_ABORTED0x0a
#define ABORTED_COMMAND 0x0b
#define VOLUME_OVERFLOW 0x0d
#define MISCOMPARE 0x0e
(The GD device specs says it supports 0, 1, 2, 3,4, 5, 6, 7 and 0xB)
ie we could get a sense key of 0x0B which would be greater than the
array size. I think you'd have to hard code the limit.
Then shouldn't this test be:
for (i = 0; i ARRAY_SIZE(sense_texts); i++) {
if (sense_key == sense_texts[i].sense_key)
printk(KERN_INFO GDROM: %s\n, sense_texts[i].text);
}
if (i = ARRAY_SIZE(sense_texts))
printk(KERN_ERR GDROM: Unknown sense key: %d\n, sense_key);
cheers, Joe
--
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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
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, [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,
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Fri, 2007-12-28 at 20:17 +0100, Gino Badouri wrote: > Applied this over the last patch and I can confirm it works like a > charm :) It's Adrian's patch that works well. Thanks Adrian. -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
Op donderdag 27-12-2007 om 14:58 uur [tijdzone -0800], schreef Joe
Perches:
> On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote:
> > This patch adds support for the CD-Rom drive on the SEGA Dreamcast.
>
> Because it was already so close, might as well make it checkpatch clean.
>
> I also added a function gdrom_is_busy() to make a couple of tests
> fit on a single line and perhaps easier to read.
>
> Signed-off-by: Joe Perches <[EMAIL PROTECTED]>
>
> --- a/drivers/cdrom/gdrom.c 2007-12-27 14:49:27.0 -0800
> +++ b/drivers/cdrom/gdrom.c 2007-12-27 14:46:20.0 -0800
> @@ -86,7 +86,8 @@
> {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"},
> + {UNIT_ATTENTION, "GDROM: Device needs attention - "
> + "disk may have been changed"},
> {DATA_PROTECT, "GDROM: Data protection error"},
> {ABORTED_COMMAND, "GDROM: Command aborted"},
> };
> @@ -130,14 +131,20 @@
> };
>
> static int gdrom_getsense(short *bufstring);
> -static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
> packet_command *command);
> +static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
> +struct packet_command *command);
> static int gdrom_hardreset(struct cdrom_device_info *cd_info);
>
> +static bool gdrom_is_busy(void)
> +{
> + return (ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80) != 0;
> +}
> +
> static void gdrom_wait_clrbusy(void)
> {
> /* long timeouts - typical for a CD Rom */
> unsigned long timeout = jiffies + HZ * 60;
> - while ((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80) && (time_before(jiffies,
> timeout)))
> + while (gdrom_is_busy() && (time_before(jiffies, timeout)))
> cpu_relax();
> }
>
> @@ -146,7 +153,7 @@
> unsigned long timeout;
> /* Wait to get busy first */
> timeout = jiffies + HZ * 60;
> - while (((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80) == 0) &&
> (time_before(jiffies, timeout)))
> + while (!gdrom_is_busy() && (time_before(jiffies, timeout)))
> cpu_relax();
> /* Now wait for busy to clear */
> gdrom_wait_clrbusy();
> @@ -216,7 +223,8 @@
> gd.pending = 1;
> gdrom_packetcommand(gd.cd_info, spin_command);
> /* 60 second timeout */
> - wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
> 60);
> + wait_event_interruptible_timeout(command_queue,
> + gd.pending == 0, HZ * 60);
> gd.pending = 0;
> kfree(spin_command);
> if (gd.status & 0x01) {
> @@ -249,7 +257,8 @@
> toc_command->buflen = tocsize;
> gd.pending = 1;
> gdrom_packetcommand(gd.cd_info, toc_command);
> - wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
> 60);
> + wait_event_interruptible_timeout(command_queue,
> + gd.pending == 0, HZ * 60);
> gd.pending = 0;
> insw(PHYSADDR(GDROM_DATA_REG), toc, tocsize/2);
> kfree(toc_command);
> @@ -274,7 +283,8 @@
> return (track & 0xff00) >> 8;
> }
>
> -static int gdrom_get_last_session(struct cdrom_device_info *cd_info, struct
> cdrom_multisession *ms_info)
> +static int gdrom_get_last_session(struct cdrom_device_info *cd_info,
> + struct cdrom_multisession *ms_info)
> {
> int fentry, lentry, track, data, tocuse, err;
> if (!gd.toc)
> @@ -287,7 +297,8 @@
> tocuse = 0;
> err = gdrom_readtoc_cmd(gd.toc, 0);
> if (err) {
> - printk(KERN_INFO "GDROM: Could not get CD table of
> contents\n");
> + printk(KERN_INFO "GDROM: Could not get CD "
> +"table of contents\n");
> return -ENXIO;
> }
> }
> @@ -305,7 +316,8 @@
>
> if ((track > 100) || (track < get_entry_track(gd.toc->first))) {
> gdrom_getsense(NULL);
> - printk(KERN_INFO "GDROM: No data on the last session of the
> CD\n");
> + printk(KERN_INFO "GDROM: No data on the last session "
> +"of the CD\n");
> return -ENXIO;
> }
>
> @@ -355,8 +367,10 @@
> return 0;
> }
>
> -/* keep the function looking like the universal CD Rom specification -
> returning int*/
> -static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
> packet_command *command)
> +/* keep the function looking like the universal CD Rom specification -
> + * returning int */
> +static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
> +struct packet_command *command)
> {
> gdrom_spicommand(>cmd, command->buflen);
> return 0;
> @@ -388,7 +402,8 @@
>
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
Op donderdag 27-12-2007 om 14:58 uur [tijdzone -0800], schreef Joe
Perches:
On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote:
This patch adds support for the CD-Rom drive on the SEGA Dreamcast.
Because it was already so close, might as well make it checkpatch clean.
I also added a function gdrom_is_busy() to make a couple of tests
fit on a single line and perhaps easier to read.
Signed-off-by: Joe Perches [EMAIL PROTECTED]
--- a/drivers/cdrom/gdrom.c 2007-12-27 14:49:27.0 -0800
+++ b/drivers/cdrom/gdrom.c 2007-12-27 14:46:20.0 -0800
@@ -86,7 +86,8 @@
{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},
+ {UNIT_ATTENTION, GDROM: Device needs attention -
+ disk may have been changed},
{DATA_PROTECT, GDROM: Data protection error},
{ABORTED_COMMAND, GDROM: Command aborted},
};
@@ -130,14 +131,20 @@
};
static int gdrom_getsense(short *bufstring);
-static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
packet_command *command);
+static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
+struct packet_command *command);
static int gdrom_hardreset(struct cdrom_device_info *cd_info);
+static bool gdrom_is_busy(void)
+{
+ return (ctrl_inb(GDROM_ALTSTATUS_REG) 0x80) != 0;
+}
+
static void gdrom_wait_clrbusy(void)
{
/* long timeouts - typical for a CD Rom */
unsigned long timeout = jiffies + HZ * 60;
- while ((ctrl_inb(GDROM_ALTSTATUS_REG) 0x80) (time_before(jiffies,
timeout)))
+ while (gdrom_is_busy() (time_before(jiffies, timeout)))
cpu_relax();
}
@@ -146,7 +153,7 @@
unsigned long timeout;
/* Wait to get busy first */
timeout = jiffies + HZ * 60;
- while (((ctrl_inb(GDROM_ALTSTATUS_REG) 0x80) == 0)
(time_before(jiffies, timeout)))
+ while (!gdrom_is_busy() (time_before(jiffies, timeout)))
cpu_relax();
/* Now wait for busy to clear */
gdrom_wait_clrbusy();
@@ -216,7 +223,8 @@
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, spin_command);
/* 60 second timeout */
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
60);
+ wait_event_interruptible_timeout(command_queue,
+ gd.pending == 0, HZ * 60);
gd.pending = 0;
kfree(spin_command);
if (gd.status 0x01) {
@@ -249,7 +257,8 @@
toc_command-buflen = tocsize;
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, toc_command);
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
60);
+ wait_event_interruptible_timeout(command_queue,
+ gd.pending == 0, HZ * 60);
gd.pending = 0;
insw(PHYSADDR(GDROM_DATA_REG), toc, tocsize/2);
kfree(toc_command);
@@ -274,7 +283,8 @@
return (track 0xff00) 8;
}
-static int gdrom_get_last_session(struct cdrom_device_info *cd_info, struct
cdrom_multisession *ms_info)
+static int gdrom_get_last_session(struct cdrom_device_info *cd_info,
+ struct cdrom_multisession *ms_info)
{
int fentry, lentry, track, data, tocuse, err;
if (!gd.toc)
@@ -287,7 +297,8 @@
tocuse = 0;
err = gdrom_readtoc_cmd(gd.toc, 0);
if (err) {
- printk(KERN_INFO GDROM: Could not get CD table of
contents\n);
+ printk(KERN_INFO GDROM: Could not get CD
+table of contents\n);
return -ENXIO;
}
}
@@ -305,7 +316,8 @@
if ((track 100) || (track get_entry_track(gd.toc-first))) {
gdrom_getsense(NULL);
- printk(KERN_INFO GDROM: No data on the last session of the
CD\n);
+ printk(KERN_INFO GDROM: No data on the last session
+of the CD\n);
return -ENXIO;
}
@@ -355,8 +367,10 @@
return 0;
}
-/* keep the function looking like the universal CD Rom specification -
returning int*/
-static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
packet_command *command)
+/* keep the function looking like the universal CD Rom specification -
+ * returning int */
+static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
+struct packet_command *command)
{
gdrom_spicommand(command-cmd, command-buflen);
return 0;
@@ -388,7 +402,8 @@
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, sense_command);
/* 60 second timeout */
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Fri, 2007-12-28 at 20:17 +0100, Gino Badouri wrote: Applied this over the last patch and I can confirm it works like a charm :) It's Adrian's patch that works well. Thanks Adrian. -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
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;
-
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, Dec 27, 2007 at 07:49:01PM -0500, Mike Frysinger wrote: > On Thursday 27 December 2007, Joe Perches wrote: > > I also added a function gdrom_is_busy() to make a couple of tests > > fit on a single line and perhaps easier to read. > > i'd tend to agree it does make it easier to read, however you didnt make it > inline which means gcc may not inline it which means there's pointless > overhead of doing a function call. > These days GCC is usually smart enough to automatically inline things that are that small, it doesn't need to be explicitly labelled. If your toolchain doesn't inline that gdrom_is_busy() implementation, get a better toolchain. > otherwise, arbitrary breaking on 80 cols actually makes the code harder to > read for no gain. Hey now, some of us have no intention of giving up our 80x25 consoles, not everyone buys in to this whole newfangled X thing. ;-) More than 80 columns is generally fine so long as it's done so tastefully rather than arbitrarily based on a larger console width. However, if it can be split up cleanly for 80 columns, then it should be. ie, use common sense, rather than blindly adhering to the whims of an anal-retentive script. -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thursday 27 December 2007, Joe Perches wrote: > On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote: > > This patch adds support for the CD-Rom drive on the SEGA Dreamcast. > > Because it was already so close, might as well make it checkpatch clean. for the 80 col limit, that's up to the author. if Adrian is happy with it as is, that's fine. > I also added a function gdrom_is_busy() to make a couple of tests > fit on a single line and perhaps easier to read. i'd tend to agree it does make it easier to read, however you didnt make it inline which means gcc may not inline it which means there's pointless overhead of doing a function call. otherwise, arbitrary breaking on 80 cols actually makes the code harder to read for no gain. -mike signature.asc Description: This is a digitally signed message part.
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
tor, 27 12 2007 kl. 14:58 -0800, skrev Joe Perches:
> On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote:
> > This patch adds support for the CD-Rom drive on the SEGA Dreamcast.
>
> Because it was already so close, might as well make it checkpatch clean.
>
> I also added a function gdrom_is_busy() to make a couple of tests
> fit on a single line and perhaps easier to read.
>
> Signed-off-by: Joe Perches <[EMAIL PROTECTED]>
>
> --- a/drivers/cdrom/gdrom.c 2007-12-27 14:49:27.0 -0800
> +++ b/drivers/cdrom/gdrom.c 2007-12-27 14:46:20.0 -0800
> @@ -86,7 +86,8 @@
> {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"},
> + {UNIT_ATTENTION, "GDROM: Device needs attention - "
> + "disk may have been changed"},
> {DATA_PROTECT, "GDROM: Data protection error"},
> {ABORTED_COMMAND, "GDROM: Command aborted"},
> };
> @@ -130,14 +131,20 @@
> };
>
> static int gdrom_getsense(short *bufstring);
> -static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
> packet_command *command);
> +static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
> +struct packet_command *command);
> static int gdrom_hardreset(struct cdrom_device_info *cd_info);
>
> +static bool gdrom_is_busy(void)
> +{
> + return (ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80) != 0;
> +}
> +
> static void gdrom_wait_clrbusy(void)
> {
> /* long timeouts - typical for a CD Rom */
> unsigned long timeout = jiffies + HZ * 60;
> - 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.
> cpu_relax();
> }
>
> @@ -146,7 +153,7 @@
> unsigned long timeout;
> /* Wait to get busy first */
> timeout = jiffies + HZ * 60;
> - while (((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80) == 0) &&
> (time_before(jiffies, timeout)))
> + while (!gdrom_is_busy() && (time_before(jiffies, timeout)))
Same here.
> cpu_relax();
> /* Now wait for busy to clear */
> gdrom_wait_clrbusy();
> @@ -216,7 +223,8 @@
> gd.pending = 1;
> gdrom_packetcommand(gd.cd_info, spin_command);
> /* 60 second timeout */
> - wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
> 60);
> + wait_event_interruptible_timeout(command_queue,
> + gd.pending == 0, HZ * 60);
> gd.pending = 0;
All three users of gdrom_packetcommand do the same timeout, so consider
moving this block into gdrom_packcommand.
> kfree(spin_command);
> if (gd.status & 0x01) {
> @@ -249,7 +257,8 @@
> toc_command->buflen = tocsize;
> gd.pending = 1;
> gdrom_packetcommand(gd.cd_info, toc_command);
> - wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
> 60);
> + wait_event_interruptible_timeout(command_queue,
> + gd.pending == 0, HZ * 60);
> gd.pending = 0;
> insw(PHYSADDR(GDROM_DATA_REG), toc, tocsize/2);
> kfree(toc_command);
> @@ -274,7 +283,8 @@
> return (track & 0xff00) >> 8;
> }
>
> -static int gdrom_get_last_session(struct cdrom_device_info *cd_info, struct
> cdrom_multisession *ms_info)
> +static int gdrom_get_last_session(struct cdrom_device_info *cd_info,
> + struct cdrom_multisession *ms_info)
> {
> int fentry, lentry, track, data, tocuse, err;
> if (!gd.toc)
> @@ -287,7 +297,8 @@
> tocuse = 0;
> err = gdrom_readtoc_cmd(gd.toc, 0);
> if (err) {
> - printk(KERN_INFO "GDROM: Could not get CD table of
> contents\n");
> + printk(KERN_INFO "GDROM: Could not get CD "
> +"table of contents\n");
> return -ENXIO;
> }
> }
> @@ -305,7 +316,8 @@
>
> if ((track > 100) || (track < get_entry_track(gd.toc->first))) {
> gdrom_getsense(NULL);
> - printk(KERN_INFO "GDROM: No data on the last session of the
> CD\n");
> + printk(KERN_INFO "GDROM: No data on the last session "
> +"of the CD\n");
> return -ENXIO;
> }
>
> @@ -355,8 +367,10 @@
> return 0;
> }
>
> -/* keep the function looking like the universal CD Rom specification -
> returning int*/
> -static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
> packet_command *command)
> +/* keep the function looking like the universal CD Rom specification -
> + * returning int */
> +static int
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote:
> This patch adds support for the CD-Rom drive on the SEGA Dreamcast.
Because it was already so close, might as well make it checkpatch clean.
I also added a function gdrom_is_busy() to make a couple of tests
fit on a single line and perhaps easier to read.
Signed-off-by: Joe Perches <[EMAIL PROTECTED]>
--- a/drivers/cdrom/gdrom.c 2007-12-27 14:49:27.0 -0800
+++ b/drivers/cdrom/gdrom.c 2007-12-27 14:46:20.0 -0800
@@ -86,7 +86,8 @@
{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"},
+ {UNIT_ATTENTION, "GDROM: Device needs attention - "
+"disk may have been changed"},
{DATA_PROTECT, "GDROM: Data protection error"},
{ABORTED_COMMAND, "GDROM: Command aborted"},
};
@@ -130,14 +131,20 @@
};
static int gdrom_getsense(short *bufstring);
-static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
packet_command *command);
+static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
+ struct packet_command *command);
static int gdrom_hardreset(struct cdrom_device_info *cd_info);
+static bool gdrom_is_busy(void)
+{
+ return (ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80) != 0;
+}
+
static void gdrom_wait_clrbusy(void)
{
/* long timeouts - typical for a CD Rom */
unsigned long timeout = jiffies + HZ * 60;
- while ((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80) && (time_before(jiffies,
timeout)))
+ while (gdrom_is_busy() && (time_before(jiffies, timeout)))
cpu_relax();
}
@@ -146,7 +153,7 @@
unsigned long timeout;
/* Wait to get busy first */
timeout = jiffies + HZ * 60;
- while (((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80) == 0) &&
(time_before(jiffies, timeout)))
+ while (!gdrom_is_busy() && (time_before(jiffies, timeout)))
cpu_relax();
/* Now wait for busy to clear */
gdrom_wait_clrbusy();
@@ -216,7 +223,8 @@
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, spin_command);
/* 60 second timeout */
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
60);
+ wait_event_interruptible_timeout(command_queue,
+gd.pending == 0, HZ * 60);
gd.pending = 0;
kfree(spin_command);
if (gd.status & 0x01) {
@@ -249,7 +257,8 @@
toc_command->buflen = tocsize;
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, toc_command);
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
60);
+ wait_event_interruptible_timeout(command_queue,
+gd.pending == 0, HZ * 60);
gd.pending = 0;
insw(PHYSADDR(GDROM_DATA_REG), toc, tocsize/2);
kfree(toc_command);
@@ -274,7 +283,8 @@
return (track & 0xff00) >> 8;
}
-static int gdrom_get_last_session(struct cdrom_device_info *cd_info, struct
cdrom_multisession *ms_info)
+static int gdrom_get_last_session(struct cdrom_device_info *cd_info,
+ struct cdrom_multisession *ms_info)
{
int fentry, lentry, track, data, tocuse, err;
if (!gd.toc)
@@ -287,7 +297,8 @@
tocuse = 0;
err = gdrom_readtoc_cmd(gd.toc, 0);
if (err) {
- printk(KERN_INFO "GDROM: Could not get CD table of
contents\n");
+ printk(KERN_INFO "GDROM: Could not get CD "
+ "table of contents\n");
return -ENXIO;
}
}
@@ -305,7 +316,8 @@
if ((track > 100) || (track < get_entry_track(gd.toc->first))) {
gdrom_getsense(NULL);
- printk(KERN_INFO "GDROM: No data on the last session of the
CD\n");
+ printk(KERN_INFO "GDROM: No data on the last session "
+ "of the CD\n");
return -ENXIO;
}
@@ -355,8 +367,10 @@
return 0;
}
-/* keep the function looking like the universal CD Rom specification -
returning int*/
-static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
packet_command *command)
+/* keep the function looking like the universal CD Rom specification -
+ * returning int */
+static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
+ struct packet_command *command)
{
gdrom_spicommand(>cmd, command->buflen);
return 0;
@@ -388,7 +402,8 @@
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, sense_command);
/* 60 second timeout */
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
60);
+
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, Dec 27, 2007 at 04:52:19PM +, Adrian McMenamin wrote: > This patch adds support for the CD-Rom drive on the SEGA Dreamcast. > > The SEGA Dreamcast has a built in CD-Rom drive, electrically similar > to an ATA-3 drive, but implementing a proprietary packet interface - > the so-called Sega Packet Interface (SPI)- and also supporting a > proprietary format of disk - the Giga Disk Rom, with a 1GB capacity. > The drive is know as the "GD-Rom drive". > > This patch partially implements the SPI and also supports reading GD > Rom disks. Unlike previous GD Rom drivers (which were never in the > mainline), this driver implements DMA and not PIO for reads. It is a > new driver, not a modified version of previous drivers. > > This is the fifth iteration of this patch - which should work with > both 2.6-24-rc5 and Paul Mundt's 2.6.25 queue. > > Signed-off by: Adrian McMenamin <[EMAIL PROTECTED]> > Looks fine to me. Thanks for cleaning it up, Adrian. -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote: > This patch adds support for the CD-Rom drive on the SEGA Dreamcast. > > The SEGA Dreamcast has a built in CD-Rom drive, electrically similar > to an ATA-3 drive, but implementing a proprietary packet interface - > the so-called Sega Packet Interface (SPI)- and also supporting a > proprietary format of disk - the Giga Disk Rom, with a 1GB capacity. > The drive is know as the "GD-Rom drive". > > This patch partially implements the SPI and also supports reading GD > Rom disks. Unlike previous GD Rom drivers (which were never in the > mainline), this driver implements DMA and not PIO for reads. It is a > new driver, not a modified version of previous drivers. > > This is the fifth iteration of this patch - which should work with > both 2.6-24-rc5 and Paul Mundt's 2.6.25 queue. > > Signed-off by: Adrian McMenamin <[EMAIL PROTECTED]> > > > Following up on more comments, here is a slightly reworked version of the patch with better request handling. Signed-off by: Adrian McMenamin <[EMAIL PROTECTED]> diff -rupN linux-2.6-orig/drivers/block/Kconfig linux-2.6/drivers/block/Kconfig --- linux-2.6-orig/drivers/block/Kconfig2007-12-26 17:27:14.0 + +++ linux-2.6/drivers/block/Kconfig 2007-12-27 16:39:05.0 + @@ -105,6 +105,17 @@ config PARIDE "MicroSolutions backpack protocol", "DataStor Commuter protocol" etc.). +config GDROM + tristate "SEGA Dreamcast GD-ROM drive" + depends on SH_DREAMCAST + help + A standard SEGA Dreamcast comes with a modified CD ROM drive called a + "GD-ROM" by SEGA to signify it is capable of reading special disks + with up to 1 GB of data. This drive will also read standard CD ROM + disks. Select this option to access any disks in your GD ROM drive. + Most users will want to say "Y" here. + You can also build this as a module which will be called gdrom.ko + source "drivers/block/paride/Kconfig" config BLK_CPQ_DA diff -rupN linux-2.6-orig/drivers/cdrom/gdrom.c linux-2.6/drivers/cdrom/gdrom.c --- linux-2.6-orig/drivers/cdrom/gdrom.c1970-01-01 01:00:00.0 +0100 +++ linux-2.6/drivers/cdrom/gdrom.c 2007-12-27 20:45:18.0 + @@ -0,0 +1,772 @@ +/* GD ROM driver for the SEGA Dreamcast + * copyright Adrian McMenamin, 2007 + * With thanks to Marcus Comstedt and Nathan Keynes + * for work in reversing PIO and DMA + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define GDROM_DEV_NAME "gdrom" +#define GD_SESSION_OFFSET 150 + +/* GD Rom commands */ +#define GDROM_COM_SOFTRESET 0x08 +#define GDROM_COM_EXECDIAG 0x90 +#define GDROM_COM_PACKET 0xA0 +#define GDROM_COM_IDDEV 0xA1 + +/* GD Rom registers */ +#define GDROM_BASE_REG 0xA05F7000 +#define GDROM_ALTSTATUS_REG(GDROM_BASE_REG + 0x18) +#define GDROM_DATA_REG (GDROM_BASE_REG + 0x80) +#define GDROM_ERROR_REG(GDROM_BASE_REG + 0x84) +#define GDROM_INTSEC_REG (GDROM_BASE_REG + 0x88) +#define GDROM_SECNUM_REG (GDROM_BASE_REG + 0x8C) +#define GDROM_BCL_REG (GDROM_BASE_REG + 0x90) +#define GDROM_BCH_REG (GDROM_BASE_REG + 0x94) +#define GDROM_DSEL_REG (GDROM_BASE_REG + 0x98) +#define GDROM_STATUSCOMMAND_REG(GDROM_BASE_REG + 0x9C) +#define GDROM_RESET_REG(GDROM_BASE_REG + 0x4E4) + +#define GDROM_DMA_STARTADDR_REG(GDROM_BASE_REG + 0x404) +#define GDROM_DMA_LENGTH_REG (GDROM_BASE_REG + 0x408) +#define GDROM_DMA_DIRECTION_REG(GDROM_BASE_REG + 0x40C) +#define GDROM_DMA_ENABLE_REG (GDROM_BASE_REG + 0x414) +#define GDROM_DMA_STATUS_REG (GDROM_BASE_REG + 0x418) +#define GDROM_DMA_WAIT_REG (GDROM_BASE_REG + 0x4A0) +#define GDROM_DMA_ACCESS_CTRL_REG (GDROM_BASE_REG + 0x4B8) + +#define GDROM_HARD_SECTOR 2048 +#define BLOCK_LAYER_SECTOR 512 +#define GD_TO_BLK
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, Dec 27 2007, Adrian McMenamin wrote: > > On Thu, 2007-12-27 at 17:18 +0900, Paul Mundt wrote: > > On Thu, Dec 27, 2007 at 01:26:47AM +, Adrian McMenamin wrote: > > > > > > + /* now seek to take the request spinlock > > > + * before handling ending the request */ > > > + spin_lock(_lock); > > > + list_del_init(>queuelist); > > > + blk_requeue_request(gd.gdrom_rq, req); > > > + if (err) > > > + end_request(req, 0); > > > + else > > > + end_request(req, 1); > > > + } > > > + spin_unlock(_lock); > > > + kfree(read_command); > > > +} > > > + > > This locking is all over the place. What is this lock supposed to be > > accomplishing? > > - > > I have to hold the lock to access the request queue. As the linked list > of deferred requests is under the control of code also protected by the > lock, it is also held to ensure manipulation of that list is serialised. > > The first step of the loop manipulates that linked list - so it is held > as we re-iterate over the loop. > > This is pretty much the way Jens recommended I do it. I didn't recommend the last requeue bit, it looks like a work-around due to the way that end_request() works. The kerneldoc comment for that function also tells you NOT to use this in new code. Use end_dequeued_request() and get rid of the requeue, and streamline 'err' so you can just pass it directly in. The locking otherwise looks fine to me. -- Jens Axboe -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thursday 27 December 2007, Paul Mundt wrote:
> On Thu, Dec 27, 2007 at 01:26:47AM +, Adrian McMenamin wrote:
> > +static int gdrom_get_last_session(struct cdrom_device_info *cd_info,
> > struct cdrom_multisession *ms_info) +{
> > + int fentry, lentry, track, data, tocuse, err;
> > + kfree(gd.toc);
> > + gd.toc = kzalloc(sizeof(struct gdromtoc), GFP_KERNEL);
>
> Er, what? The size of this never changes, allocate it once, and just
> overload it every time you step in to this function. There's no reason to
> free it and reallocate every time. Shove it in your probe routine with
> the rest of your kzallocs.
since gd is a global, just dont declare toc as a pointer ...
-mike
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, 2007-12-27 at 17:18 +0900, Paul Mundt wrote: > On Thu, Dec 27, 2007 at 01:26:47AM +, Adrian McMenamin wrote: > > > + /* now seek to take the request spinlock > > +* before handling ending the request */ > > + spin_lock(_lock); > > + list_del_init(>queuelist); > > + blk_requeue_request(gd.gdrom_rq, req); > > + if (err) > > + end_request(req, 0); > > + else > > + end_request(req, 1); > > + } > > + spin_unlock(_lock); > > + kfree(read_command); > > +} > > + > This locking is all over the place. What is this lock supposed to be > accomplishing? > - I have to hold the lock to access the request queue. As the linked list of deferred requests is under the control of code also protected by the lock, it is also held to ensure manipulation of that list is serialised. The first step of the loop manipulates that linked list - so it is held as we re-iterate over the loop. This is pretty much the way Jens recommended I do it. -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, Dec 27, 2007 at 01:26:47AM +, Adrian McMenamin wrote:
> This patch adds support for the CD-Rom drive on the SEGA Dreamcast.
>
Please fix your mailer to inline the patch, preferably without word
wrapping. This is not a difficult thing, send yourself some test mails
until you get it sorted out.
> diff -rupN linux-2.6-orig/drivers/block/Kconfig
> linux-2.6/drivers/block/Kconfig
> --- linux-2.6-orig/drivers/block/Kconfig 2007-12-26 17:27:14.0
> +
> +++ linux-2.6/drivers/block/Kconfig 2007-12-27 00:08:39.0 +
> @@ -105,6 +105,18 @@ config PARIDE
> "MicroSolutions backpack protocol", "DataStor Commuter protocol"
> etc.).
>
> +config GDROM
Why is this using a tab?
> + tristate "SEGA Dreamcast GD-ROM drive"
> + depends on SH_DREAMCAST
> + help
> + A standard SEGA Dreamcast comes with a modified CD ROM drive called a
> + "GD-ROM" by SEGA to signify it is capable of reading special disks
> + with up to 1 GB of data. This drive will also read standard CD ROM
> + disks. Select this option to access any disks in your GD ROM drive.
> + Most users will want to say "Y" here.
> + You can also build this as a module which will be called gdrom.ko
> +
> +
Superfluous whitespace.
> +static int gdrom_get_last_session(struct cdrom_device_info *cd_info, struct
> cdrom_multisession *ms_info)
> +{
> + int fentry, lentry, track, data, tocuse, err;
> + kfree(gd.toc);
> + gd.toc = kzalloc(sizeof(struct gdromtoc), GFP_KERNEL);
Er, what? The size of this never changes, allocate it once, and just
overload it every time you step in to this function. There's no reason to
free it and reallocate every time. Shove it in your probe routine with
the rest of your kzallocs.
> +/* keep the function loioking like the universal CD Rom specification -
> returning int*/
looking.
> +static int gdrom_set_command_interrupt_handler(void)
> +{
> + /* need a queue to wait in */
> + init_waitqueue_head(_queue);
> + return request_irq(HW_EVENT_GDROM_CMD, gdrom_command_interrupt,
> IRQF_DISABLED, "gdrom_command", );
> +}
> +
> +static int gdrom_set_dma_interrupt_handler(void)
> +{
> + init_waitqueue_head(_queue);
> + return request_irq(HW_EVENT_GDROM_DMA, gdrom_dma_interrupt,
> IRQF_DISABLED, "gdrom_dma", );
> +}
> +
Having separate functions for these is pretty pointless.
> + spin_lock(_lock);
> + list_for_each_safe(elem, next, _deferred) {
> + req = list_entry(elem, struct request, queuelist);
> + spin_unlock(_lock);
[snip]
> + /* now seek to take the request spinlock
> + * before handling ending the request */
> + spin_lock(_lock);
> + list_del_init(>queuelist);
> + blk_requeue_request(gd.gdrom_rq, req);
> + if (err)
> + end_request(req, 0);
> + else
> + end_request(req, 1);
> + }
> + spin_unlock(_lock);
> + kfree(read_command);
> +}
> +
This locking is all over the place. What is this lock supposed to be
accomplishing?
--
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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, Dec 27, 2007 at 01:26:47AM +, Adrian McMenamin wrote:
This patch adds support for the CD-Rom drive on the SEGA Dreamcast.
Please fix your mailer to inline the patch, preferably without word
wrapping. This is not a difficult thing, send yourself some test mails
until you get it sorted out.
diff -rupN linux-2.6-orig/drivers/block/Kconfig
linux-2.6/drivers/block/Kconfig
--- linux-2.6-orig/drivers/block/Kconfig 2007-12-26 17:27:14.0
+
+++ linux-2.6/drivers/block/Kconfig 2007-12-27 00:08:39.0 +
@@ -105,6 +105,18 @@ config PARIDE
MicroSolutions backpack protocol, DataStor Commuter protocol
etc.).
+config GDROM
Why is this using a tab?
+ tristate SEGA Dreamcast GD-ROM drive
+ depends on SH_DREAMCAST
+ help
+ A standard SEGA Dreamcast comes with a modified CD ROM drive called a
+ GD-ROM by SEGA to signify it is capable of reading special disks
+ with up to 1 GB of data. This drive will also read standard CD ROM
+ disks. Select this option to access any disks in your GD ROM drive.
+ Most users will want to say Y here.
+ You can also build this as a module which will be called gdrom.ko
+
+
Superfluous whitespace.
+static int gdrom_get_last_session(struct cdrom_device_info *cd_info, struct
cdrom_multisession *ms_info)
+{
+ int fentry, lentry, track, data, tocuse, err;
+ kfree(gd.toc);
+ gd.toc = kzalloc(sizeof(struct gdromtoc), GFP_KERNEL);
Er, what? The size of this never changes, allocate it once, and just
overload it every time you step in to this function. There's no reason to
free it and reallocate every time. Shove it in your probe routine with
the rest of your kzallocs.
+/* keep the function loioking like the universal CD Rom specification -
returning int*/
looking.
+static int gdrom_set_command_interrupt_handler(void)
+{
+ /* need a queue to wait in */
+ init_waitqueue_head(command_queue);
+ return request_irq(HW_EVENT_GDROM_CMD, gdrom_command_interrupt,
IRQF_DISABLED, gdrom_command, gd);
+}
+
+static int gdrom_set_dma_interrupt_handler(void)
+{
+ init_waitqueue_head(request_queue);
+ return request_irq(HW_EVENT_GDROM_DMA, gdrom_dma_interrupt,
IRQF_DISABLED, gdrom_dma, gd);
+}
+
Having separate functions for these is pretty pointless.
+ spin_lock(gdrom_lock);
+ list_for_each_safe(elem, next, gdrom_deferred) {
+ req = list_entry(elem, struct request, queuelist);
+ spin_unlock(gdrom_lock);
[snip]
+ /* now seek to take the request spinlock
+ * before handling ending the request */
+ spin_lock(gdrom_lock);
+ list_del_init(req-queuelist);
+ blk_requeue_request(gd.gdrom_rq, req);
+ if (err)
+ end_request(req, 0);
+ else
+ end_request(req, 1);
+ }
+ spin_unlock(gdrom_lock);
+ kfree(read_command);
+}
+
This locking is all over the place. What is this lock supposed to be
accomplishing?
--
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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, 2007-12-27 at 17:18 +0900, Paul Mundt wrote: On Thu, Dec 27, 2007 at 01:26:47AM +, Adrian McMenamin wrote: + /* now seek to take the request spinlock +* before handling ending the request */ + spin_lock(gdrom_lock); + list_del_init(req-queuelist); + blk_requeue_request(gd.gdrom_rq, req); + if (err) + end_request(req, 0); + else + end_request(req, 1); + } + spin_unlock(gdrom_lock); + kfree(read_command); +} + This locking is all over the place. What is this lock supposed to be accomplishing? - I have to hold the lock to access the request queue. As the linked list of deferred requests is under the control of code also protected by the lock, it is also held to ensure manipulation of that list is serialised. The first step of the loop manipulates that linked list - so it is held as we re-iterate over the loop. This is pretty much the way Jens recommended I do it. -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thursday 27 December 2007, Paul Mundt wrote:
On Thu, Dec 27, 2007 at 01:26:47AM +, Adrian McMenamin wrote:
+static int gdrom_get_last_session(struct cdrom_device_info *cd_info,
struct cdrom_multisession *ms_info) +{
+ int fentry, lentry, track, data, tocuse, err;
+ kfree(gd.toc);
+ gd.toc = kzalloc(sizeof(struct gdromtoc), GFP_KERNEL);
Er, what? The size of this never changes, allocate it once, and just
overload it every time you step in to this function. There's no reason to
free it and reallocate every time. Shove it in your probe routine with
the rest of your kzallocs.
since gd is a global, just dont declare toc as a pointer ...
-mike
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, Dec 27 2007, Adrian McMenamin wrote: On Thu, 2007-12-27 at 17:18 +0900, Paul Mundt wrote: On Thu, Dec 27, 2007 at 01:26:47AM +, Adrian McMenamin wrote: + /* now seek to take the request spinlock + * before handling ending the request */ + spin_lock(gdrom_lock); + list_del_init(req-queuelist); + blk_requeue_request(gd.gdrom_rq, req); + if (err) + end_request(req, 0); + else + end_request(req, 1); + } + spin_unlock(gdrom_lock); + kfree(read_command); +} + This locking is all over the place. What is this lock supposed to be accomplishing? - I have to hold the lock to access the request queue. As the linked list of deferred requests is under the control of code also protected by the lock, it is also held to ensure manipulation of that list is serialised. The first step of the loop manipulates that linked list - so it is held as we re-iterate over the loop. This is pretty much the way Jens recommended I do it. I didn't recommend the last requeue bit, it looks like a work-around due to the way that end_request() works. The kerneldoc comment for that function also tells you NOT to use this in new code. Use end_dequeued_request() and get rid of the requeue, and streamline 'err' so you can just pass it directly in. The locking otherwise looks fine to me. -- Jens Axboe -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, Dec 27, 2007 at 04:52:19PM +, Adrian McMenamin wrote: This patch adds support for the CD-Rom drive on the SEGA Dreamcast. The SEGA Dreamcast has a built in CD-Rom drive, electrically similar to an ATA-3 drive, but implementing a proprietary packet interface - the so-called Sega Packet Interface (SPI)- and also supporting a proprietary format of disk - the Giga Disk Rom, with a 1GB capacity. The drive is know as the GD-Rom drive. This patch partially implements the SPI and also supports reading GD Rom disks. Unlike previous GD Rom drivers (which were never in the mainline), this driver implements DMA and not PIO for reads. It is a new driver, not a modified version of previous drivers. This is the fifth iteration of this patch - which should work with both 2.6-24-rc5 and Paul Mundt's 2.6.25 queue. Signed-off by: Adrian McMenamin [EMAIL PROTECTED] Looks fine to me. Thanks for cleaning it up, Adrian. -- 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/
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote: This patch adds support for the CD-Rom drive on the SEGA Dreamcast. The SEGA Dreamcast has a built in CD-Rom drive, electrically similar to an ATA-3 drive, but implementing a proprietary packet interface - the so-called Sega Packet Interface (SPI)- and also supporting a proprietary format of disk - the Giga Disk Rom, with a 1GB capacity. The drive is know as the GD-Rom drive. This patch partially implements the SPI and also supports reading GD Rom disks. Unlike previous GD Rom drivers (which were never in the mainline), this driver implements DMA and not PIO for reads. It is a new driver, not a modified version of previous drivers. This is the fifth iteration of this patch - which should work with both 2.6-24-rc5 and Paul Mundt's 2.6.25 queue. Signed-off by: Adrian McMenamin [EMAIL PROTECTED] Following up on more comments, here is a slightly reworked version of the patch with better request handling. Signed-off by: Adrian McMenamin [EMAIL PROTECTED] diff -rupN linux-2.6-orig/drivers/block/Kconfig linux-2.6/drivers/block/Kconfig --- linux-2.6-orig/drivers/block/Kconfig2007-12-26 17:27:14.0 + +++ linux-2.6/drivers/block/Kconfig 2007-12-27 16:39:05.0 + @@ -105,6 +105,17 @@ config PARIDE MicroSolutions backpack protocol, DataStor Commuter protocol etc.). +config GDROM + tristate SEGA Dreamcast GD-ROM drive + depends on SH_DREAMCAST + help + A standard SEGA Dreamcast comes with a modified CD ROM drive called a + GD-ROM by SEGA to signify it is capable of reading special disks + with up to 1 GB of data. This drive will also read standard CD ROM + disks. Select this option to access any disks in your GD ROM drive. + Most users will want to say Y here. + You can also build this as a module which will be called gdrom.ko + source drivers/block/paride/Kconfig config BLK_CPQ_DA diff -rupN linux-2.6-orig/drivers/cdrom/gdrom.c linux-2.6/drivers/cdrom/gdrom.c --- linux-2.6-orig/drivers/cdrom/gdrom.c1970-01-01 01:00:00.0 +0100 +++ linux-2.6/drivers/cdrom/gdrom.c 2007-12-27 20:45:18.0 + @@ -0,0 +1,772 @@ +/* GD ROM driver for the SEGA Dreamcast + * copyright Adrian McMenamin, 2007 + * With thanks to Marcus Comstedt and Nathan Keynes + * for work in reversing PIO and DMA + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ + +#include linux/init.h +#include linux/module.h +#include linux/fs.h +#include linux/kernel.h +#include linux/list.h +#include linux/slab.h +#include linux/dma-mapping.h +#include linux/cdrom.h +#include linux/genhd.h +#include linux/bio.h +#include linux/blkdev.h +#include linux/interrupt.h +#include linux/device.h +#include linux/wait.h +#include linux/workqueue.h +#include linux/platform_device.h +#include scsi/scsi.h +#include asm/io.h +#include asm/dma.h +#include asm/delay.h +#include asm/mach/dma.h +#include asm/mach/sysasic.h + +#define GDROM_DEV_NAME gdrom +#define GD_SESSION_OFFSET 150 + +/* GD Rom commands */ +#define GDROM_COM_SOFTRESET 0x08 +#define GDROM_COM_EXECDIAG 0x90 +#define GDROM_COM_PACKET 0xA0 +#define GDROM_COM_IDDEV 0xA1 + +/* GD Rom registers */ +#define GDROM_BASE_REG 0xA05F7000 +#define GDROM_ALTSTATUS_REG(GDROM_BASE_REG + 0x18) +#define GDROM_DATA_REG (GDROM_BASE_REG + 0x80) +#define GDROM_ERROR_REG(GDROM_BASE_REG + 0x84) +#define GDROM_INTSEC_REG (GDROM_BASE_REG + 0x88) +#define GDROM_SECNUM_REG (GDROM_BASE_REG + 0x8C) +#define GDROM_BCL_REG (GDROM_BASE_REG + 0x90) +#define GDROM_BCH_REG (GDROM_BASE_REG + 0x94) +#define GDROM_DSEL_REG (GDROM_BASE_REG + 0x98) +#define GDROM_STATUSCOMMAND_REG(GDROM_BASE_REG + 0x9C) +#define GDROM_RESET_REG(GDROM_BASE_REG + 0x4E4) + +#define GDROM_DMA_STARTADDR_REG(GDROM_BASE_REG + 0x404) +#define GDROM_DMA_LENGTH_REG (GDROM_BASE_REG + 0x408) +#define GDROM_DMA_DIRECTION_REG(GDROM_BASE_REG + 0x40C) +#define GDROM_DMA_ENABLE_REG (GDROM_BASE_REG + 0x414) +#define GDROM_DMA_STATUS_REG
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote:
This patch adds support for the CD-Rom drive on the SEGA Dreamcast.
Because it was already so close, might as well make it checkpatch clean.
I also added a function gdrom_is_busy() to make a couple of tests
fit on a single line and perhaps easier to read.
Signed-off-by: Joe Perches [EMAIL PROTECTED]
--- a/drivers/cdrom/gdrom.c 2007-12-27 14:49:27.0 -0800
+++ b/drivers/cdrom/gdrom.c 2007-12-27 14:46:20.0 -0800
@@ -86,7 +86,8 @@
{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},
+ {UNIT_ATTENTION, GDROM: Device needs attention -
+disk may have been changed},
{DATA_PROTECT, GDROM: Data protection error},
{ABORTED_COMMAND, GDROM: Command aborted},
};
@@ -130,14 +131,20 @@
};
static int gdrom_getsense(short *bufstring);
-static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
packet_command *command);
+static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
+ struct packet_command *command);
static int gdrom_hardreset(struct cdrom_device_info *cd_info);
+static bool gdrom_is_busy(void)
+{
+ return (ctrl_inb(GDROM_ALTSTATUS_REG) 0x80) != 0;
+}
+
static void gdrom_wait_clrbusy(void)
{
/* long timeouts - typical for a CD Rom */
unsigned long timeout = jiffies + HZ * 60;
- while ((ctrl_inb(GDROM_ALTSTATUS_REG) 0x80) (time_before(jiffies,
timeout)))
+ while (gdrom_is_busy() (time_before(jiffies, timeout)))
cpu_relax();
}
@@ -146,7 +153,7 @@
unsigned long timeout;
/* Wait to get busy first */
timeout = jiffies + HZ * 60;
- while (((ctrl_inb(GDROM_ALTSTATUS_REG) 0x80) == 0)
(time_before(jiffies, timeout)))
+ while (!gdrom_is_busy() (time_before(jiffies, timeout)))
cpu_relax();
/* Now wait for busy to clear */
gdrom_wait_clrbusy();
@@ -216,7 +223,8 @@
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, spin_command);
/* 60 second timeout */
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
60);
+ wait_event_interruptible_timeout(command_queue,
+gd.pending == 0, HZ * 60);
gd.pending = 0;
kfree(spin_command);
if (gd.status 0x01) {
@@ -249,7 +257,8 @@
toc_command-buflen = tocsize;
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, toc_command);
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
60);
+ wait_event_interruptible_timeout(command_queue,
+gd.pending == 0, HZ * 60);
gd.pending = 0;
insw(PHYSADDR(GDROM_DATA_REG), toc, tocsize/2);
kfree(toc_command);
@@ -274,7 +283,8 @@
return (track 0xff00) 8;
}
-static int gdrom_get_last_session(struct cdrom_device_info *cd_info, struct
cdrom_multisession *ms_info)
+static int gdrom_get_last_session(struct cdrom_device_info *cd_info,
+ struct cdrom_multisession *ms_info)
{
int fentry, lentry, track, data, tocuse, err;
if (!gd.toc)
@@ -287,7 +297,8 @@
tocuse = 0;
err = gdrom_readtoc_cmd(gd.toc, 0);
if (err) {
- printk(KERN_INFO GDROM: Could not get CD table of
contents\n);
+ printk(KERN_INFO GDROM: Could not get CD
+ table of contents\n);
return -ENXIO;
}
}
@@ -305,7 +316,8 @@
if ((track 100) || (track get_entry_track(gd.toc-first))) {
gdrom_getsense(NULL);
- printk(KERN_INFO GDROM: No data on the last session of the
CD\n);
+ printk(KERN_INFO GDROM: No data on the last session
+ of the CD\n);
return -ENXIO;
}
@@ -355,8 +367,10 @@
return 0;
}
-/* keep the function looking like the universal CD Rom specification -
returning int*/
-static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
packet_command *command)
+/* keep the function looking like the universal CD Rom specification -
+ * returning int */
+static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
+ struct packet_command *command)
{
gdrom_spicommand(command-cmd, command-buflen);
return 0;
@@ -388,7 +402,8 @@
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, sense_command);
/* 60 second timeout */
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
60);
+ wait_event_interruptible_timeout(command_queue,
+
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
tor, 27 12 2007 kl. 14:58 -0800, skrev Joe Perches:
On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote:
This patch adds support for the CD-Rom drive on the SEGA Dreamcast.
Because it was already so close, might as well make it checkpatch clean.
I also added a function gdrom_is_busy() to make a couple of tests
fit on a single line and perhaps easier to read.
Signed-off-by: Joe Perches [EMAIL PROTECTED]
--- a/drivers/cdrom/gdrom.c 2007-12-27 14:49:27.0 -0800
+++ b/drivers/cdrom/gdrom.c 2007-12-27 14:46:20.0 -0800
@@ -86,7 +86,8 @@
{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},
+ {UNIT_ATTENTION, GDROM: Device needs attention -
+ disk may have been changed},
{DATA_PROTECT, GDROM: Data protection error},
{ABORTED_COMMAND, GDROM: Command aborted},
};
@@ -130,14 +131,20 @@
};
static int gdrom_getsense(short *bufstring);
-static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
packet_command *command);
+static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
+struct packet_command *command);
static int gdrom_hardreset(struct cdrom_device_info *cd_info);
+static bool gdrom_is_busy(void)
+{
+ return (ctrl_inb(GDROM_ALTSTATUS_REG) 0x80) != 0;
+}
+
static void gdrom_wait_clrbusy(void)
{
/* long timeouts - typical for a CD Rom */
unsigned long timeout = jiffies + HZ * 60;
- 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.
cpu_relax();
}
@@ -146,7 +153,7 @@
unsigned long timeout;
/* Wait to get busy first */
timeout = jiffies + HZ * 60;
- while (((ctrl_inb(GDROM_ALTSTATUS_REG) 0x80) == 0)
(time_before(jiffies, timeout)))
+ while (!gdrom_is_busy() (time_before(jiffies, timeout)))
Same here.
cpu_relax();
/* Now wait for busy to clear */
gdrom_wait_clrbusy();
@@ -216,7 +223,8 @@
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, spin_command);
/* 60 second timeout */
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
60);
+ wait_event_interruptible_timeout(command_queue,
+ gd.pending == 0, HZ * 60);
gd.pending = 0;
All three users of gdrom_packetcommand do the same timeout, so consider
moving this block into gdrom_packcommand.
kfree(spin_command);
if (gd.status 0x01) {
@@ -249,7 +257,8 @@
toc_command-buflen = tocsize;
gd.pending = 1;
gdrom_packetcommand(gd.cd_info, toc_command);
- wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ *
60);
+ wait_event_interruptible_timeout(command_queue,
+ gd.pending == 0, HZ * 60);
gd.pending = 0;
insw(PHYSADDR(GDROM_DATA_REG), toc, tocsize/2);
kfree(toc_command);
@@ -274,7 +283,8 @@
return (track 0xff00) 8;
}
-static int gdrom_get_last_session(struct cdrom_device_info *cd_info, struct
cdrom_multisession *ms_info)
+static int gdrom_get_last_session(struct cdrom_device_info *cd_info,
+ struct cdrom_multisession *ms_info)
{
int fentry, lentry, track, data, tocuse, err;
if (!gd.toc)
@@ -287,7 +297,8 @@
tocuse = 0;
err = gdrom_readtoc_cmd(gd.toc, 0);
if (err) {
- printk(KERN_INFO GDROM: Could not get CD table of
contents\n);
+ printk(KERN_INFO GDROM: Could not get CD
+table of contents\n);
return -ENXIO;
}
}
@@ -305,7 +316,8 @@
if ((track 100) || (track get_entry_track(gd.toc-first))) {
gdrom_getsense(NULL);
- printk(KERN_INFO GDROM: No data on the last session of the
CD\n);
+ printk(KERN_INFO GDROM: No data on the last session
+of the CD\n);
return -ENXIO;
}
@@ -355,8 +367,10 @@
return 0;
}
-/* keep the function looking like the universal CD Rom specification -
returning int*/
-static int gdrom_packetcommand(struct cdrom_device_info *cd_info, struct
packet_command *command)
+/* keep the function looking like the universal CD Rom specification -
+ * returning int */
+static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
+struct packet_command *command)
{
gdrom_spicommand(command-cmd, command-buflen);
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thursday 27 December 2007, Joe Perches wrote: On Thu, 2007-12-27 at 16:52 +, Adrian McMenamin wrote: This patch adds support for the CD-Rom drive on the SEGA Dreamcast. Because it was already so close, might as well make it checkpatch clean. for the 80 col limit, that's up to the author. if Adrian is happy with it as is, that's fine. I also added a function gdrom_is_busy() to make a couple of tests fit on a single line and perhaps easier to read. i'd tend to agree it does make it easier to read, however you didnt make it inline which means gcc may not inline it which means there's pointless overhead of doing a function call. otherwise, arbitrary breaking on 80 cols actually makes the code harder to read for no gain. -mike signature.asc Description: This is a digitally signed message part.
Re: [PATCH] SH/Dreamcast - add support for GD-Rom device
On Thu, Dec 27, 2007 at 07:49:01PM -0500, Mike Frysinger wrote: On Thursday 27 December 2007, Joe Perches wrote: I also added a function gdrom_is_busy() to make a couple of tests fit on a single line and perhaps easier to read. i'd tend to agree it does make it easier to read, however you didnt make it inline which means gcc may not inline it which means there's pointless overhead of doing a function call. These days GCC is usually smart enough to automatically inline things that are that small, it doesn't need to be explicitly labelled. If your toolchain doesn't inline that gdrom_is_busy() implementation, get a better toolchain. otherwise, arbitrary breaking on 80 cols actually makes the code harder to read for no gain. Hey now, some of us have no intention of giving up our 80x25 consoles, not everyone buys in to this whole newfangled X thing. ;-) More than 80 columns is generally fine so long as it's done so tastefully rather than arbitrarily based on a larger console width. However, if it can be split up cleanly for 80 columns, then it should be. ie, use common sense, rather than blindly adhering to the whims of an anal-retentive script. -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Thursday 20 December 2007, Adrian McMenamin wrote:
> On 20/12/2007, Adrian McMenamin <[EMAIL PROTECTED]> wrote:
> > This patch adds support for the CD Rom device (called a "GD Rom") on
> > the SEGA Dreamcast.This device has a command block similar to a
> > standard ATA-3 device, though implements Sega's proprietary packet
> > interface - the so-called "Sega Packet Interface".
thanks for keeping the dc port up to date :)
> diff -ruN linux-2.6-orig/drivers/block/Kconfig
> +config GDROM
most people use a space here *shrug*
> + tristate "SEGA Dreamcast GD-ROM drive"
> + depends on SH_DREAMCAST
> + help
> + A standard SEGA Dreamcast comes with a modified CD ROM drive called a
> + "GD-ROM" by SEGA to signify it is capable of reading special disks
> + with up to 1 GB of data. This drive will also read standard CD ROM
> + disks. Select this option to access any disks in your GD ROM drive.
> + Most users will want to say "Y" here.
this line has broken whitespace at the start
> + You can also build this as a module - which will be called gdrom.ko
no need for the - there ...
> +static int gdrom_preparedisk_cmd(void)
> + if ((gd.status & 0x01) != 0) {
no need for the compare i dont think ?
if (gd.status & 0x01)
> +static int gdrom_readtoc_cmd(struct gdromtoc *toc, int session)
> + if ((gd.status & 0x01) != 0)
same here
> +static int gdrom_drivestatus(struct cdrom_device_info *cd_info, int
> + sense &=0xF0;
missing a space after the = ...
> +static void gdrom_request(struct request_queue *rq)
> + if (! blk_fs_request(req)) {
extraneous space with the ! there
> +static int __init probe_gdrom(struct platform_device *devptr)
> + sprintf(gd.cd_info->name, GDROM_DEV_NAME);
> + sprintf(gd.disk->disk_name, GDROM_DEV_NAME);
strcpy() prob runs with lower overhead
> +static int __init init_gdrom(void)
> +{
> + rc = platform_driver_register(_driver);
> + if (rc) {
> + printk(KERN_INFO "Could not register GDROM driver - error
> 0x%X\n", rc);
> + return -EPERM;
shoudnt you return rc ? then there's probably no need to display the rc value
in the printk() as it'd get passed back to higher levels ...
> + pd = platform_device_register_simple(GDROM_DEV_NAME, -1, NULL, 0);
> + if (IS_ERR(pd)) {
> + platform_driver_unregister(_driver);
> + return -ENODEV;
similar thing ... return the error stored in pd
-mike
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Thursday 20 December 2007, Adrian McMenamin wrote:
On 20/12/2007, Adrian McMenamin [EMAIL PROTECTED] wrote:
This patch adds support for the CD Rom device (called a GD Rom) on
the SEGA Dreamcast.This device has a command block similar to a
standard ATA-3 device, though implements Sega's proprietary packet
interface - the so-called Sega Packet Interface.
thanks for keeping the dc port up to date :)
diff -ruN linux-2.6-orig/drivers/block/Kconfig
+config GDROM
most people use a space here *shrug*
+ tristate SEGA Dreamcast GD-ROM drive
+ depends on SH_DREAMCAST
+ help
+ A standard SEGA Dreamcast comes with a modified CD ROM drive called a
+ GD-ROM by SEGA to signify it is capable of reading special disks
+ with up to 1 GB of data. This drive will also read standard CD ROM
+ disks. Select this option to access any disks in your GD ROM drive.
+ Most users will want to say Y here.
this line has broken whitespace at the start
+ You can also build this as a module - which will be called gdrom.ko
no need for the - there ...
+static int gdrom_preparedisk_cmd(void)
+ if ((gd.status 0x01) != 0) {
no need for the compare i dont think ?
if (gd.status 0x01)
+static int gdrom_readtoc_cmd(struct gdromtoc *toc, int session)
+ if ((gd.status 0x01) != 0)
same here
+static int gdrom_drivestatus(struct cdrom_device_info *cd_info, int
+ sense =0xF0;
missing a space after the = ...
+static void gdrom_request(struct request_queue *rq)
+ if (! blk_fs_request(req)) {
extraneous space with the ! there
+static int __init probe_gdrom(struct platform_device *devptr)
+ sprintf(gd.cd_info-name, GDROM_DEV_NAME);
+ sprintf(gd.disk-disk_name, GDROM_DEV_NAME);
strcpy() prob runs with lower overhead
+static int __init init_gdrom(void)
+{
+ rc = platform_driver_register(gdrom_driver);
+ if (rc) {
+ printk(KERN_INFO Could not register GDROM driver - error
0x%X\n, rc);
+ return -EPERM;
shoudnt you return rc ? then there's probably no need to display the rc value
in the printk() as it'd get passed back to higher levels ...
+ pd = platform_device_register_simple(GDROM_DEV_NAME, -1, NULL, 0);
+ if (IS_ERR(pd)) {
+ platform_driver_unregister(gdrom_driver);
+ return -ENODEV;
similar thing ... return the error stored in pd
-mike
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, 2007-12-21 at 20:35 +0100, Jens Axboe wrote: > backing_dev_info Sorry, I know what you mean now ... left over from an earlier experiments with read ahead. Will delete it now: didn't realise it was still lurking there) -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, Dec 21 2007, Adrian McMenamin wrote: > > On Fri, 2007-12-21 at 13:14 +0100, Jens Axboe wrote: > > > > > What is pages doing in gdrom_request()? > > > > As the device doesn't do scatter-gather I've set > blk_queue_max_segment_size to 1 and am not bothering with pages - is > that wrong? I mean this: unsigned long pages; pages = rq->backing_dev_info.ra_pages; and you never use 'pages'. -- Jens Axboe -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, 2007-12-21 at 13:14 +0100, Jens Axboe wrote: > > What is pages doing in gdrom_request()? > As the device doesn't do scatter-gather I've set blk_queue_max_segment_size to 1 and am not bothering with pages - is that wrong? -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, Dec 21 2007, Adrian McMenamin wrote: > On Fri, December 21, 2007 2:22 pm, Jens Axboe wrote: > > On Fri, Dec 21 2007, Adrian McMenamin wrote: > >> On 21/12/2007, Jens Axboe <[EMAIL PROTECTED]> wrote: > >> > > >> > Your design is also heavily geared towards there just being a single > >> > CDROM, I'm assuming this wont be a problem given your hw (it sets a > >> bad > >> > example for others to follow though, lots of violations against normal > >> > programming practice for multiple devices and smp). > >> > >> > >> Yes, because is there only one device and there will only ever be one > >> device (unless you know of somebody doing Dreamcast hardware > >> development). > >> > >> I understand the point you are making but adding in additional code > >> would only diminish resource availability or slow performance on a > >> small machine without actually delivering any better outcomes for > >> kernel users.. > >> > >> Is it reaslly necessary? > > > > No it's not necessary, it was just a general reflection on how it could > > have been done more in style with "regular" drivers. > > > > > I have another look at the code, though i am anxious not to add to the > driver's footprint if it is not bring any real benefit. Let me see if > there is a way round that. Don't break your neck over it, it was nothing more than an observation :-) BTW, I saw something else - you should always use sector_div() to do divisions on a sector_t type. -- Jens Axboe -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, December 21, 2007 2:22 pm, Jens Axboe wrote: > On Fri, Dec 21 2007, Adrian McMenamin wrote: >> On 21/12/2007, Jens Axboe <[EMAIL PROTECTED]> wrote: >> > >> > Your design is also heavily geared towards there just being a single >> > CDROM, I'm assuming this wont be a problem given your hw (it sets a >> bad >> > example for others to follow though, lots of violations against normal >> > programming practice for multiple devices and smp). >> >> >> Yes, because is there only one device and there will only ever be one >> device (unless you know of somebody doing Dreamcast hardware >> development). >> >> I understand the point you are making but adding in additional code >> would only diminish resource availability or slow performance on a >> small machine without actually delivering any better outcomes for >> kernel users.. >> >> Is it reaslly necessary? > > No it's not necessary, it was just a general reflection on how it could > have been done more in style with "regular" drivers. > I have another look at the code, though i am anxious not to add to the driver's footprint if it is not bring any real benefit. Let me see if there is a way round that. -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, Dec 21 2007, Adrian McMenamin wrote: > On 21/12/2007, Jens Axboe <[EMAIL PROTECTED]> wrote: > > > > Your design is also heavily geared towards there just being a single > > CDROM, I'm assuming this wont be a problem given your hw (it sets a bad > > example for others to follow though, lots of violations against normal > > programming practice for multiple devices and smp). > > > Yes, because is there only one device and there will only ever be one > device (unless you know of somebody doing Dreamcast hardware > development). > > I understand the point you are making but adding in additional code > would only diminish resource availability or slow performance on a > small machine without actually delivering any better outcomes for > kernel users.. > > Is it reaslly necessary? No it's not necessary, it was just a general reflection on how it could have been done more in style with "regular" drivers. -- Jens Axboe -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On 21/12/2007, Jens Axboe <[EMAIL PROTECTED]> wrote: > > Your design is also heavily geared towards there just being a single > CDROM, I'm assuming this wont be a problem given your hw (it sets a bad > example for others to follow though, lots of violations against normal > programming practice for multiple devices and smp). Yes, because is there only one device and there will only ever be one device (unless you know of somebody doing Dreamcast hardware development). I understand the point you are making but adding in additional code would only diminish resource availability or slow performance on a small machine without actually delivering any better outcomes for kernel users.. Is it reaslly necessary? -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, Dec 21 2007, Jens Axboe wrote:
> static void gdrom_readdisk_dma(struct work_struct *work)
> {
> ...
>
> read_command = kzalloc(sizeof(struct packet_command), GFP_KERNEL);
> if (!read_command)
> probably just defer the work to some time later
>
> spin_lock(_lock);
> while (!list_empty(_deferred)) {
> req = list_entry(gdrom_deferred.next, struct request,
> queuelist);
> list_del_init(>queuelist);
> spin_unlock(_lock);
>
> ...
>
> spin_lock(_lock);
> };
>
+ spin_unlock(_lock);
> kfree(read_command);
> }
is missing, of course.
--
Jens Axboe
--
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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, Dec 21 2007, Adrian McMenamin wrote:
> On 20/12/2007, Adrian McMenamin <[EMAIL PROTECTED]> wrote:
> > This patch adds support for the CD Rom device (called a "GD Rom") on
> > the SEGA Dreamcast.This device has a command block similar to a
> > standard ATA-3 device, though implements Sega's proprietary packet
> > interface - the so-called "Sega Packet Interface".
> >
>
> Fairly typically, I noticed I had chopped the final line from the
> patch as soon as I had sent it.
>
> I've also fixed a small difference in the Kconfig
You should properly protect gdrom_deferred, the locking is not clear
there. In gdrom_readdisk_dma() I would do:
static void gdrom_readdisk_dma(struct work_struct *work)
{
...
read_command = kzalloc(sizeof(struct packet_command), GFP_KERNEL);
if (!read_command)
probably just defer the work to some time later
spin_lock(_lock);
while (!list_empty(_deferred)) {
req = list_entry(gdrom_deferred.next, struct request,
queuelist);
list_del_init(>queuelist);
spin_unlock(_lock);
...
spin_lock(_lock);
};
kfree(read_command);
}
That's a lot more obvious imho and doesn't suffer from potential races
or list reordering. Your read_command allocation and free for every
command also seems pretty pointless, so move that outside the loop.
What is pages doing in gdrom_request()?
Your design is also heavily geared towards there just being a single
CDROM, I'm assuming this wont be a problem given your hw (it sets a bad
example for others to follow though, lots of violations against normal
programming practice for multiple devices and smp).
--
Jens Axboe
--
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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, Dec 21 2007, Jens Axboe wrote:
static void gdrom_readdisk_dma(struct work_struct *work)
{
...
read_command = kzalloc(sizeof(struct packet_command), GFP_KERNEL);
if (!read_command)
probably just defer the work to some time later
spin_lock(gdrom_lock);
while (!list_empty(gdrom_deferred)) {
req = list_entry(gdrom_deferred.next, struct request,
queuelist);
list_del_init(req-queuelist);
spin_unlock(gdrom_lock);
...
spin_lock(gdrom_lock);
};
+ spin_unlock(gdrom_lock);
kfree(read_command);
}
is missing, of course.
--
Jens Axboe
--
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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, Dec 21 2007, Adrian McMenamin wrote:
On 20/12/2007, Adrian McMenamin [EMAIL PROTECTED] wrote:
This patch adds support for the CD Rom device (called a GD Rom) on
the SEGA Dreamcast.This device has a command block similar to a
standard ATA-3 device, though implements Sega's proprietary packet
interface - the so-called Sega Packet Interface.
Fairly typically, I noticed I had chopped the final line from the
patch as soon as I had sent it.
I've also fixed a small difference in the Kconfig
You should properly protect gdrom_deferred, the locking is not clear
there. In gdrom_readdisk_dma() I would do:
static void gdrom_readdisk_dma(struct work_struct *work)
{
...
read_command = kzalloc(sizeof(struct packet_command), GFP_KERNEL);
if (!read_command)
probably just defer the work to some time later
spin_lock(gdrom_lock);
while (!list_empty(gdrom_deferred)) {
req = list_entry(gdrom_deferred.next, struct request,
queuelist);
list_del_init(req-queuelist);
spin_unlock(gdrom_lock);
...
spin_lock(gdrom_lock);
};
kfree(read_command);
}
That's a lot more obvious imho and doesn't suffer from potential races
or list reordering. Your read_command allocation and free for every
command also seems pretty pointless, so move that outside the loop.
What is pages doing in gdrom_request()?
Your design is also heavily geared towards there just being a single
CDROM, I'm assuming this wont be a problem given your hw (it sets a bad
example for others to follow though, lots of violations against normal
programming practice for multiple devices and smp).
--
Jens Axboe
--
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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, Dec 21 2007, Adrian McMenamin wrote: On 21/12/2007, Jens Axboe [EMAIL PROTECTED] wrote: Your design is also heavily geared towards there just being a single CDROM, I'm assuming this wont be a problem given your hw (it sets a bad example for others to follow though, lots of violations against normal programming practice for multiple devices and smp). Yes, because is there only one device and there will only ever be one device (unless you know of somebody doing Dreamcast hardware development). I understand the point you are making but adding in additional code would only diminish resource availability or slow performance on a small machine without actually delivering any better outcomes for kernel users.. Is it reaslly necessary? No it's not necessary, it was just a general reflection on how it could have been done more in style with regular drivers. -- Jens Axboe -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, December 21, 2007 2:22 pm, Jens Axboe wrote: On Fri, Dec 21 2007, Adrian McMenamin wrote: On 21/12/2007, Jens Axboe [EMAIL PROTECTED] wrote: Your design is also heavily geared towards there just being a single CDROM, I'm assuming this wont be a problem given your hw (it sets a bad example for others to follow though, lots of violations against normal programming practice for multiple devices and smp). Yes, because is there only one device and there will only ever be one device (unless you know of somebody doing Dreamcast hardware development). I understand the point you are making but adding in additional code would only diminish resource availability or slow performance on a small machine without actually delivering any better outcomes for kernel users.. Is it reaslly necessary? No it's not necessary, it was just a general reflection on how it could have been done more in style with regular drivers. I have another look at the code, though i am anxious not to add to the driver's footprint if it is not bring any real benefit. Let me see if there is a way round that. -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On 21/12/2007, Jens Axboe [EMAIL PROTECTED] wrote: Your design is also heavily geared towards there just being a single CDROM, I'm assuming this wont be a problem given your hw (it sets a bad example for others to follow though, lots of violations against normal programming practice for multiple devices and smp). Yes, because is there only one device and there will only ever be one device (unless you know of somebody doing Dreamcast hardware development). I understand the point you are making but adding in additional code would only diminish resource availability or slow performance on a small machine without actually delivering any better outcomes for kernel users.. Is it reaslly necessary? -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, Dec 21 2007, Adrian McMenamin wrote: On Fri, December 21, 2007 2:22 pm, Jens Axboe wrote: On Fri, Dec 21 2007, Adrian McMenamin wrote: On 21/12/2007, Jens Axboe [EMAIL PROTECTED] wrote: Your design is also heavily geared towards there just being a single CDROM, I'm assuming this wont be a problem given your hw (it sets a bad example for others to follow though, lots of violations against normal programming practice for multiple devices and smp). Yes, because is there only one device and there will only ever be one device (unless you know of somebody doing Dreamcast hardware development). I understand the point you are making but adding in additional code would only diminish resource availability or slow performance on a small machine without actually delivering any better outcomes for kernel users.. Is it reaslly necessary? No it's not necessary, it was just a general reflection on how it could have been done more in style with regular drivers. I have another look at the code, though i am anxious not to add to the driver's footprint if it is not bring any real benefit. Let me see if there is a way round that. Don't break your neck over it, it was nothing more than an observation :-) BTW, I saw something else - you should always use sector_div() to do divisions on a sector_t type. -- Jens Axboe -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, 2007-12-21 at 13:14 +0100, Jens Axboe wrote: What is pages doing in gdrom_request()? As the device doesn't do scatter-gather I've set blk_queue_max_segment_size to 1 and am not bothering with pages - is that wrong? -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, 2007-12-21 at 20:35 +0100, Jens Axboe wrote: backing_dev_info Sorry, I know what you mean now ... left over from an earlier experiments with read ahead. Will delete it now: didn't realise it was still lurking there) -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On Fri, Dec 21 2007, Adrian McMenamin wrote: On Fri, 2007-12-21 at 13:14 +0100, Jens Axboe wrote: What is pages doing in gdrom_request()? As the device doesn't do scatter-gather I've set blk_queue_max_segment_size to 1 and am not bothering with pages - is that wrong? I mean this: unsigned long pages; pages = rq-backing_dev_info.ra_pages; and you never use 'pages'. -- Jens Axboe -- 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/
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On 20/12/2007, Adrian McMenamin <[EMAIL PROTECTED]> wrote:
> This patch adds support for the CD Rom device (called a "GD Rom") on
> the SEGA Dreamcast.This device has a command block similar to a
> standard ATA-3 device, though implements Sega's proprietary packet
> interface - the so-called "Sega Packet Interface".
>
Fairly typically, I noticed I had chopped the final line from the
patch as soon as I had sent it.
I've also fixed a small difference in the Kconfig
diff -ruN linux-2.6-orig/drivers/block/Kconfig linux-2.6/drivers/block/Kconfig
--- linux-2.6-orig/drivers/block/Kconfig2007-12-15 22:23:47.0
+
+++ linux-2.6/drivers/block/Kconfig 2007-12-20 23:36:15.0 +
@@ -105,6 +105,18 @@
"MicroSolutions backpack protocol", "DataStor Commuter protocol"
etc.).
+config GDROM
+ tristate "SEGA Dreamcast GD-ROM drive"
+ depends on SH_DREAMCAST
+ help
+ A standard SEGA Dreamcast comes with a modified CD ROM drive called a
+ "GD-ROM" by SEGA to signify it is capable of reading special disks
+ with up to 1 GB of data. This drive will also read standard CD ROM
+ disks. Select this option to access any disks in your GD ROM drive.
+ Most users will want to say "Y" here.
+ You can also build this as a module - which will be called gdrom.ko
+
+
source "drivers/block/paride/Kconfig"
config BLK_CPQ_DA
diff -ruN linux-2.6-orig/drivers/cdrom/gdrom.c linux-2.6/drivers/cdrom/gdrom.c
--- linux-2.6-orig/drivers/cdrom/gdrom.c1970-01-01 01:00:00.0
+0100
+++ linux-2.6/drivers/cdrom/gdrom.c 2007-12-20 23:36:15.0 +
@@ -0,0 +1,810 @@
+/* GD ROM driver for the SEGA Dreamcast
+ * copyright Adrian McMenamin, 2007
+ * With thanks to Marcus Comstedt and Nathan Keynes
+ * for work in reversing PIO and DMA
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define GDROM_DEV_NAME "gdrom"
+#define GD_SESSION_OFFSET 150
+
+/* GD Rom commands */
+#define GDROM_COM_SOFTRESET 0x08
+#define GDROM_COM_EXECDIAG 0x90
+#define GDROM_COM_PACKET 0xA0
+#define GDROM_COM_IDDEV 0xA1
+
+/* GD Rom registers */
+#define GDROM_BASE_REG 0xA05F7000
+#define GDROM_ALTSTATUS_REG(GDROM_BASE_REG + 0x18)
+#define GDROM_DATA_REG (GDROM_BASE_REG + 0x80)
+#define GDROM_ERROR_REG(GDROM_BASE_REG + 0x84)
+#define GDROM_INTSEC_REG (GDROM_BASE_REG + 0x88)
+#define GDROM_SECNUM_REG (GDROM_BASE_REG + 0x8C)
+#define GDROM_BCL_REG (GDROM_BASE_REG + 0x90)
+#define GDROM_BCH_REG (GDROM_BASE_REG + 0x94)
+#define GDROM_DSEL_REG (GDROM_BASE_REG + 0x98)
+#define GDROM_STATUSCOMMAND_REG(GDROM_BASE_REG + 0x9C)
+#define GDROM_RESET_REG(GDROM_BASE_REG + 0x4E4)
+
+#define GDROM_DMA_STARTADDR_REG(GDROM_BASE_REG + 0x404)
+#define GDROM_DMA_LENGTH_REG (GDROM_BASE_REG + 0x408)
+#define GDROM_DMA_DIRECTION_REG(GDROM_BASE_REG + 0x40C)
+#define GDROM_DMA_ENABLE_REG (GDROM_BASE_REG + 0x414)
+#define GDROM_DMA_STATUS_REG (GDROM_BASE_REG + 0x418)
+#define GDROM_DMA_WAIT_REG (GDROM_BASE_REG + 0x4A0)
+#define GDROM_DMA_ACCESS_CTRL_REG (GDROM_BASE_REG + 0x4B8)
+
+#define GDROM_HARD_SECTOR 2048
+#define BLOCK_LAYER_SECTOR 512
+#define GD_TO_BLK 4
+
+static struct platform_device *pd;
+static int gdrom_major;
+static wait_queue_head_t command_queue;
+static wait_queue_head_t request_queue;
+
+static DEFINE_SPINLOCK(gdrom_lock);
+static void gdrom_readdisk_dma(struct work_struct *work);
+static DECLARE_WORK(work, gdrom_readdisk_dma);
+static LIST_HEAD(gdrom_deferred);
+
+struct gdromtoc {
+ unsigned int entry[99];
+ unsigned int first, last;
+ unsigned int leadout;
+};
+
+static struct gdrom_unit {
+ struct gendisk *disk;
+ struct cdrom_device_info *cd_info;
+ int status;
+ int pending;
+ int transfer;
+ char
Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device
On 20/12/2007, Adrian McMenamin [EMAIL PROTECTED] wrote:
This patch adds support for the CD Rom device (called a GD Rom) on
the SEGA Dreamcast.This device has a command block similar to a
standard ATA-3 device, though implements Sega's proprietary packet
interface - the so-called Sega Packet Interface.
Fairly typically, I noticed I had chopped the final line from the
patch as soon as I had sent it.
I've also fixed a small difference in the Kconfig
diff -ruN linux-2.6-orig/drivers/block/Kconfig linux-2.6/drivers/block/Kconfig
--- linux-2.6-orig/drivers/block/Kconfig2007-12-15 22:23:47.0
+
+++ linux-2.6/drivers/block/Kconfig 2007-12-20 23:36:15.0 +
@@ -105,6 +105,18 @@
MicroSolutions backpack protocol, DataStor Commuter protocol
etc.).
+config GDROM
+ tristate SEGA Dreamcast GD-ROM drive
+ depends on SH_DREAMCAST
+ help
+ A standard SEGA Dreamcast comes with a modified CD ROM drive called a
+ GD-ROM by SEGA to signify it is capable of reading special disks
+ with up to 1 GB of data. This drive will also read standard CD ROM
+ disks. Select this option to access any disks in your GD ROM drive.
+ Most users will want to say Y here.
+ You can also build this as a module - which will be called gdrom.ko
+
+
source drivers/block/paride/Kconfig
config BLK_CPQ_DA
diff -ruN linux-2.6-orig/drivers/cdrom/gdrom.c linux-2.6/drivers/cdrom/gdrom.c
--- linux-2.6-orig/drivers/cdrom/gdrom.c1970-01-01 01:00:00.0
+0100
+++ linux-2.6/drivers/cdrom/gdrom.c 2007-12-20 23:36:15.0 +
@@ -0,0 +1,810 @@
+/* GD ROM driver for the SEGA Dreamcast
+ * copyright Adrian McMenamin, 2007
+ * With thanks to Marcus Comstedt and Nathan Keynes
+ * for work in reversing PIO and DMA
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include linux/init.h
+#include linux/module.h
+#include linux/fs.h
+#include linux/kernel.h
+#include linux/list.h
+#include linux/slab.h
+#include linux/dma-mapping.h
+#include linux/cdrom.h
+#include linux/genhd.h
+#include linux/bio.h
+#include linux/blkdev.h
+#include linux/interrupt.h
+#include linux/device.h
+#include linux/wait.h
+#include linux/workqueue.h
+#include linux/platform_device.h
+#include asm/io.h
+#include asm/dma.h
+#include asm/delay.h
+#include asm/mach/dma.h
+#include asm/mach/sysasic.h
+
+#define GDROM_DEV_NAME gdrom
+#define GD_SESSION_OFFSET 150
+
+/* GD Rom commands */
+#define GDROM_COM_SOFTRESET 0x08
+#define GDROM_COM_EXECDIAG 0x90
+#define GDROM_COM_PACKET 0xA0
+#define GDROM_COM_IDDEV 0xA1
+
+/* GD Rom registers */
+#define GDROM_BASE_REG 0xA05F7000
+#define GDROM_ALTSTATUS_REG(GDROM_BASE_REG + 0x18)
+#define GDROM_DATA_REG (GDROM_BASE_REG + 0x80)
+#define GDROM_ERROR_REG(GDROM_BASE_REG + 0x84)
+#define GDROM_INTSEC_REG (GDROM_BASE_REG + 0x88)
+#define GDROM_SECNUM_REG (GDROM_BASE_REG + 0x8C)
+#define GDROM_BCL_REG (GDROM_BASE_REG + 0x90)
+#define GDROM_BCH_REG (GDROM_BASE_REG + 0x94)
+#define GDROM_DSEL_REG (GDROM_BASE_REG + 0x98)
+#define GDROM_STATUSCOMMAND_REG(GDROM_BASE_REG + 0x9C)
+#define GDROM_RESET_REG(GDROM_BASE_REG + 0x4E4)
+
+#define GDROM_DMA_STARTADDR_REG(GDROM_BASE_REG + 0x404)
+#define GDROM_DMA_LENGTH_REG (GDROM_BASE_REG + 0x408)
+#define GDROM_DMA_DIRECTION_REG(GDROM_BASE_REG + 0x40C)
+#define GDROM_DMA_ENABLE_REG (GDROM_BASE_REG + 0x414)
+#define GDROM_DMA_STATUS_REG (GDROM_BASE_REG + 0x418)
+#define GDROM_DMA_WAIT_REG (GDROM_BASE_REG + 0x4A0)
+#define GDROM_DMA_ACCESS_CTRL_REG (GDROM_BASE_REG + 0x4B8)
+
+#define GDROM_HARD_SECTOR 2048
+#define BLOCK_LAYER_SECTOR 512
+#define GD_TO_BLK 4
+
+static struct platform_device *pd;
+static int gdrom_major;
+static wait_queue_head_t command_queue;
+static wait_queue_head_t request_queue;
+
+static DEFINE_SPINLOCK(gdrom_lock);
+static void gdrom_readdisk_dma(struct work_struct *work);
+static DECLARE_WORK(work, gdrom_readdisk_dma);
+static LIST_HEAD(gdrom_deferred);
+
+struct gdromtoc {
+ unsigned
