On Sun, Dec 23, 2007 at 07:43:24PM +0000, Adrian McMenamin wrote:
> This patch adds support for the CD-Rom (the so-called "GD-Rom") on the
> SEGA Dreamcast.
>
Looks quite a bit better. Just a few minor things.
> +static void gdrom_spicommand(void *spi_string, int buflen)
> +{
> + short *cmd = spi_string;
> + /* ensure IRQ_WAIT is set */
> + ctrl_outb(0x08, GDROM_ALTSTATUS_REG);
> + /* specify how many bytes we expect back */
> + ctrl_outb(buflen & 0xFF, GDROM_BCL_REG);
> + ctrl_outb((buflen >> 8) & 0xFF, GDROM_BCH_REG);
> + /* other parameters */
> + ctrl_outb(0, GDROM_INTSEC_REG);
> + ctrl_outb(0, GDROM_SECNUM_REG);
> + ctrl_outb(0, GDROM_ERROR_REG);
> + /* Wait until we can go */
> + gdrom_wait_clrbusy();
> + ctrl_outb(GDROM_COM_PACKET, GDROM_STATUSCOMMAND_REG);
> + while ((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x88) != 8)
> + ; /* wait for DRQ to be set to 1 */
cpu_relax()
> +static int gdrom_get_last_session(struct cdrom_device_info *cd_info,
> struct cdrom_multisession *ms_info)
> +{
> + struct gdromtoc *tocA, *tocB;
> + int fentry, lentry, track, data, tocuse;
> + int err = -ENOMEM;
> + tocA = kzalloc(sizeof(struct gdromtoc), GFP_KERNEL);
> + if (!tocA)
> + goto exit;
> + tocB = kzalloc(sizeof(struct gdromtoc), GFP_KERNEL);
> + if (!tocB)
> + goto clean_tocA;
> + tocuse = 1;
> + err = gdrom_readtoc_cmd(tocB, 1);
> + if (err) {
> + tocuse = 0;
> + err = gdrom_readtoc_cmd(tocA, 0);
> + if (err) {
> + err = -ENXIO;
> + printk(KERN_INFO "Could not get CD table of
> contents\n");
> + goto clean_tocB;
> + }
> + } else
> + printk(KERN_DEBUG "Disk is GDROM\n");
> +
> + kfree(gd.toc);
> +
> + if (tocuse) {
> + gd.toc = tocB;
> + kfree(tocA);
> + } else {
> + gd.toc = tocA;
> + kfree(tocB);
> + }
tocA and tocB seem to be useless, as you're not using them for anything
but gd.toc assignment. If you're catching errors from gdrom_readtoc_cmd()
directly, then just pass in gd.toc in both cases, then you can skip all
of this temporary state stuff entirely.
> + sense_key = sense[1] & 0x0F;
> + switch (sense_key){
> + case 0xB:
> + printk(KERN_INFO "GDROM: Command aborted\n");
> + break;
> + case 0x7:
> + printk(KERN_INFO "GDROM: Data protection error\n");
> + break;
> + case 0x6:
> + printk(KERN_NOTICE "GDROM: Unit needs attention - possible disk
> switch\n");
> + break;
> + case 0x5:
> + printk(KERN_INFO "GDROM: Illegal request - command has
> failed\n");
> + break;
> + case 0x4:
> + printk(KERN_CRIT "GDROM: Hardware error\n");
> + break;
> + case 0x3:
> + printk(KERN_WARNING "GDROM: Disk read error\n");
> + break;
> + case 0x2:
> + printk(KERN_INFO "GDROM: Not ready\n");
> + break;
> + case 0x1:
> + printk(KERN_DEBUG "GDROM: Recovered from error\n");
> + break;
> + case 0x0:
> + /*success - stay silent*/
> + break;
> +
> + default:
> + printk(KERN_INFO "GDROM: Unknown error\n");
> + }
These are all pretty close together, maybe it would be cleaner to throw
these strings in to an array and just use the sense_key as the array
index. Some other drivers do this already.
--
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/
