linux-kernel - Re: [PATCH] SH/Dreamcast - add support for GD-Rom CDROM drive on SEGA Dreamcast

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080115002958.GA27597@linux-sh.org>
Date:	Tue, 15 Jan 2008 09:29:58 +0900
From:	Paul Mundt <lethal@...ux-sh.org>
To:	Adrian McMenamin <adrian@...golddream.dyndns.info>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Jens Axboe <jens.axboe@...cle.com>,
	linux-sh <linux-sh@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] SH/Dreamcast - add support for GD-Rom CDROM drive on SEGA Dreamcast

On Mon, Jan 14, 2008 at 11:17:15PM +0000, Adrian McMenamin wrote:
> +/* 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)
> +

Almost all of these are whitespace damaged. checkpatch doesn't seem to
catch them, but that's checkpatch's problem.

If your editor has no way of flagging whitespace damage in a visible
fashion, you may wish to consider replacing it with something that isn't
crap.

With vim you can use something like 'let c_space_errors=1' to see these.

> +static bool gdrom_data_request(void)
> +{
> + 	return (ctrl_inb(GDROM_ALTSTATUS_REG) & 0x88) == 8;
> +}
> +
Andrew first pointed this out, and this is still broken. Additionally,
checkpatch _does_ complain about this:

ERROR: use tabs not spaces
#202: FILE: drivers/cdrom/gdrom.c:146:
+ ^Ireturn (ctrl_inb(GDROM_ALTSTATUS_REG) & 0x88) == 8;$

ERROR: use tabs not spaces
#230: FILE: drivers/cdrom/gdrom.c:174:
+ ^I * been hit by a serious failure - but we'll$

ERROR: use tabs not spaces
#231: FILE: drivers/cdrom/gdrom.c:175:
+ ^I * try to return a sense key even so */$

ERROR: use tabs not spaces
#497: FILE: drivers/cdrom/gdrom.c:441:
+ ^I * the sense key if possible */$

ERROR: use tabs not spaces
#517: FILE: drivers/cdrom/gdrom.c:461:
+ ^I^Iprintk(KERN_ERR "GDROM: Unknown sense key: %d\n", sense_key);$

ERROR: use tabs not spaces
#680: FILE: drivers/cdrom/gdrom.c:624:
+ ^I^I * before handling ending the request */$

ERROR: use tabs not spaces
#692: FILE: drivers/cdrom/gdrom.c:636:
+ ^I * and then schedule workqueue */$

ERROR: use tabs not spaces
#764: FILE: drivers/cdrom/gdrom.c:708:
+ ^I * Bits 31 - 16 security: 0x8843$

ERROR: use tabs not spaces
#765: FILE: drivers/cdrom/gdrom.c:709:
+ ^I * Bits 15 and 7 reserved (0)$

ERROR: use tabs not spaces
#766: FILE: drivers/cdrom/gdrom.c:710:
+ ^I * Bits 14 - 8 start of transfer range in 1 MB blocks OR'ed with 0x80$

ERROR: trailing whitespace
#767: FILE: drivers/cdrom/gdrom.c:711:
+ ^I * Bits 6 - 0 end of transfer range in 1 MB blocks OR'ed with 0x80 $

ERROR: use tabs not spaces
#767: FILE: drivers/cdrom/gdrom.c:711:
+ ^I * Bits 6 - 0 end of transfer range in 1 MB blocks OR'ed with 0x80 $

ERROR: use tabs not spaces
#768: FILE: drivers/cdrom/gdrom.c:712:
+ ^I * (0x40 | 0x80) = start range at 0x0C000000$

ERROR: use tabs not spaces
#769: FILE: drivers/cdrom/gdrom.c:713:
+ ^I * (0x7F | 0x80) = end range at 0x0FFFFFFF */$

WARNING: line over 80 characters
#873: FILE: drivers/cdrom/gdrom.c:817:
+	printk(KERN_WARNING "GDROM: Could not probe for device - error is 0x%X\n", err);

All of these seem to stem from the fact that your editor is misconfigured
or broken. Please fix all of these and resubmit.

> +static int __devinit probe_gdrom(struct platform_device *devptr)
> +{
[snip]

> +}
> +
> +static int remove_gdrom(struct platform_device *devptr)
> +{
> +	flush_scheduled_work();
> +	blk_cleanup_queue(gd.gdrom_rq);
> +	free_irq(HW_EVENT_GDROM_CMD, &gd);
> +	free_irq(HW_EVENT_GDROM_DMA, &gd);
> +	del_gendisk(gd.disk);
> +	if (gdrom_major)
> +		unregister_blkdev(gdrom_major, GDROM_DEV_NAME);
> +	return unregister_cdrom(gd.cd_info);
> +}
> +
> +static struct platform_driver gdrom_driver = {
> +	.probe = probe_gdrom,
> +	.remove = remove_gdrom,
> +	.driver = {
> +			.name = GDROM_DEV_NAME,
> +	},
> +};
> +
Well, you're half-way there. You've got probe_gdrom() as __devinit, now
remove_gdrom() needs to be __devexit, and you can wrap around it with
__devexit_p() in the gdrom_driver definition.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/