* Re: [PATCH] drivers/cdrom/cdu31c.c check_region() fix
2004-01-02 16:13 [PATCH] drivers/cdrom/cdu31c.c check_region() fix Omkhar Arasaratnam
@ 2004-01-02 19:02 ` viro
0 siblings, 0 replies; 2+ messages in thread
From: viro @ 2004-01-02 19:02 UTC (permalink / raw)
To: Omkhar Arasaratnam; +Cc: emoenke, linux-kernel, trivial, randy
On Fri, Jan 02, 2004 at 11:13:06AM -0500, Omkhar Arasaratnam wrote:
> Here is a check_region fix for cdu31a
>
> As usual feel free to poke / prod / suggest / include this patch as you see fit
It's not enough - you have
if (cdu31a_port == 0xffff) {
} else if (cdu31a_port != 0) {
...
probe on that port
if got it>
drive_found = 1;
...
} else {
...
autoprobing
...
}
if (!drive_found)
goto errout3;
if (!request_region(cdu31a_port, 4, "cdu31a"))
so at the very least you want to pull request_region() in both branches
that set drive_found. FWIW, I'd rather move it into get_drive_configuration()
- something along the lines of
--- RC0-current/drivers/cdrom/cdu31a.c Mon Sep 8 19:25:05 2003
+++ /tmp/cdu31a.c Fri Jan 2 13:58:32 2004
@@ -3203,13 +3203,16 @@
static char *load_mech[] __initdata =
{ "caddy", "tray", "pop-up", "unknown" };
-static void __init
+static int __init
get_drive_configuration(unsigned short base_io,
unsigned char res_reg[], unsigned int *res_size)
{
unsigned long retry_count;
+ if (!request_region(base_io, 4, "cdu31a"))
+ return 0;
+
/* Set the base address */
cdu31a_port = base_io;
@@ -3244,7 +3247,7 @@
/* If attention is never seen probably not a CDU31a present */
if (!is_attention()) {
res_reg[0] = 0x20;
- return;
+ goto out_err;
}
#endif
@@ -3254,11 +3257,17 @@
do_sony_cd_cmd(SONY_REQ_DRIVE_CONFIG_CMD,
NULL,
0, (unsigned char *) res_reg, res_size);
- return;
+ if (*res_size <= 2 || (res_reg[0] & 0xf0) != 0)
+ goto out_err;
+ return 1;
}
/* Return an error */
res_reg[0] = 0x20;
+out_err:
+ release_region(cdu31a_port, 4);
+ cdu31a_port = 0;
+ return 0;
}
#ifndef MODULE
@@ -3307,9 +3316,6 @@
char msg[255];
char buf[40];
int i;
- int drive_found;
- int tmp_irq;
-
/*
* According to Alex Freed (freed@europa.orion.adobe.com), this is
@@ -3323,51 +3329,32 @@
outb(0xe2, 0x9a01);
}
- drive_found = 0;
-
/* Setting the base I/O address to 0xffff will disable it. */
- if (cdu31a_port == 0xffff) {
- } else if (cdu31a_port != 0) {
- tmp_irq = cdu31a_irq; /* Need IRQ 0 because we can't sleep here. */
- cdu31a_irq = 0;
-
- get_drive_configuration(cdu31a_port,
- drive_config.exec_status,
- &res_size);
- if ((res_size > 2)
- && ((drive_config.exec_status[0] & 0xf0) == 0x00)) {
- drive_found = 1;
- }
+ if (cdu31a_port == 0xffff)
+ goto errout3;
+ if (cdu31a_port != 0) {
+ /* Need IRQ 0 because we can't sleep here. */
+ int tmp_irq = cdu31a_irq;
+ cdu31a_irq = 0;
+ if (!get_drive_configuration(cdu31a_port,
+ drive_config.exec_status,
+ &res_size))
+ goto errout3;
cdu31a_irq = tmp_irq;
} else {
cdu31a_irq = 0;
- i = 0;
- while ((cdu31a_addresses[i].base != 0)
- && (!drive_found)) {
- if (check_region(cdu31a_addresses[i].base, 4)) {
- i++;
- continue;
- }
- get_drive_configuration(cdu31a_addresses[i].base,
- drive_config.exec_status,
- &res_size);
- if ((res_size > 2)
- && ((drive_config.exec_status[0] & 0xf0) ==
- 0x00)) {
- drive_found = 1;
+ for (i = 0; cdu31a_addresses[i].base; i++) {
+ if (get_drive_configuration(cdu31a_addresses[i].base,
+ drive_config.exec_status,
+ &res_size)) {
cdu31a_irq = cdu31a_addresses[i].int_num;
- } else {
- i++;
+ break;
}
}
+ if (!cdu31a_port)
+ goto errout3;
}
-
- if (!drive_found)
- goto errout3;
-
- if (!request_region(cdu31a_port, 4, "cdu31a"))
- goto errout3;
if (register_blkdev(MAJOR_NR, "cdu31a"))
goto errout2;
^ permalink raw reply [flat|nested] 2+ messages in thread