linux-kernel - Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device

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] [day] [month] [year] [list]

Message-Id: <200712220544.25102.vapier@gentoo.org>
Date:	Sat, 22 Dec 2007 05:44:24 -0500
From:	Mike Frysinger <vapier@...too.org>
To:	"Adrian McMenamin" <lkmladrian@...il.com>
Cc:	linux-kernel@...r.kernel.org, linux-sh@...r.kernel.org,
	"Paul Mundt" <lethal@...ux-sh.org>, axboe@...nel.dk
Subject: Re: [PATCH - SH/Dreamcast] Add support for GD-Rom device

On Thursday 20 December 2007, Adrian McMenamin wrote:
> On 20/12/2007, Adrian McMenamin <lkmladrian@...il.com> 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

Download attachment "signature.asc " of type "application/pgp-signature" (828 bytes)