Patch to drivers/char/synclinkmp.c against 2.6.6
to cleanup properly on errors during
driver initialization.
In particular, call pci_unregister_driver if
driver init fails. This is in response to a bug
report by Dave Jones.
Please apply.
--
Paul Fulghum
[email protected]
--- linux-2.6.6/drivers/char/synclinkmp.c 2004-06-01 15:30:02.967249331 -0500
+++ linux-2.6.6-mg1/drivers/char/synclinkmp.c 2004-06-01 15:28:31.022403976 -0500
@@ -1,5 +1,5 @@
/*
- * $Id: synclinkmp.c,v 4.19 2004/03/08 15:29:23 paulkf Exp $
+ * $Id: synclinkmp.c,v 4.20 2004/06/01 20:27:46 paulkf Exp $
*
* Device driver for Microgate SyncLink Multiport
* high speed multiprotocol serial adapter.
@@ -494,7 +494,7 @@
MODULE_PARM(dosyncppp,"1-" __MODULE_STRING(MAX_DEVICES) "i");
static char *driver_name = "SyncLink MultiPort driver";
-static char *driver_version = "$Revision: 4.19 $";
+static char *driver_version = "$Revision: 4.20 $";
static int synclinkmp_init_one(struct pci_dev *dev,const struct pci_device_id *ent);
static void synclinkmp_remove_one(struct pci_dev *dev);
@@ -3781,56 +3781,7 @@
.tiocmset = tiocmset,
};
-/* Driver initialization entry point.
- */
-
-static int __init synclinkmp_init(void)
-{
- if (break_on_load) {
- synclinkmp_get_text_ptr();
- BREAKPOINT();
- }
-
- printk("%s %s\n", driver_name, driver_version);
-
- synclinkmp_adapter_count = -1;
- pci_register_driver(&synclinkmp_pci_driver);
-
- if ( !synclinkmp_device_list ) {
- printk("%s(%d):No SyncLink devices found.\n",__FILE__,__LINE__);
- return -ENODEV;
- }
-
- serial_driver = alloc_tty_driver(synclinkmp_device_count);
- if (!serial_driver)
- return -ENOMEM;
-
- /* Initialize the tty_driver structure */
-
- serial_driver->owner = THIS_MODULE;
- serial_driver->driver_name = "synclinkmp";
- serial_driver->name = "ttySLM";
- serial_driver->major = ttymajor;
- serial_driver->minor_start = 64;
- serial_driver->type = TTY_DRIVER_TYPE_SERIAL;
- serial_driver->subtype = SERIAL_TYPE_NORMAL;
- serial_driver->init_termios = tty_std_termios;
- serial_driver->init_termios.c_cflag =
- B9600 | CS8 | CREAD | HUPCL | CLOCAL;
- serial_driver->flags = TTY_DRIVER_REAL_RAW;
- tty_set_operations(serial_driver, &ops);
- if (tty_register_driver(serial_driver) < 0)
- printk("%s(%d):Couldn't register serial driver\n",
- __FILE__,__LINE__);
-
- printk("%s %s, tty major#%d\n",
- driver_name, driver_version,
- serial_driver->major);
-
- return 0;
-}
-
-static void __exit synclinkmp_exit(void)
+static void synclinkmp_cleanup(void)
{
unsigned long flags;
int rc;
@@ -3839,10 +3790,12 @@
printk("Unloading %s %s\n", driver_name, driver_version);
- if ((rc = tty_unregister_driver(serial_driver)))
- printk("%s(%d) failed to unregister tty driver err=%d\n",
- __FILE__,__LINE__,rc);
- put_tty_driver(serial_driver);
+ if (serial_driver) {
+ if ((rc = tty_unregister_driver(serial_driver)))
+ printk("%s(%d) failed to unregister tty driver err=%d\n",
+ __FILE__,__LINE__,rc);
+ put_tty_driver(serial_driver);
+ }
info = synclinkmp_device_list;
while(info) {
@@ -3882,6 +3835,73 @@
pci_unregister_driver(&synclinkmp_pci_driver);
}
+/* Driver initialization entry point.
+ */
+
+static int __init synclinkmp_init(void)
+{
+ int rc;
+
+ if (break_on_load) {
+ synclinkmp_get_text_ptr();
+ BREAKPOINT();
+ }
+
+ printk("%s %s\n", driver_name, driver_version);
+
+ synclinkmp_adapter_count = -1;
+ rc = pci_register_driver(&synclinkmp_pci_driver);
+
+ if (rc <= 0 || !synclinkmp_device_list) {
+ printk("%s(%d):No SyncLink devices found.\n",__FILE__,__LINE__);
+ rc = -ENODEV;
+ goto error;
+ }
+
+ serial_driver = alloc_tty_driver(synclinkmp_device_count);
+ if (!serial_driver) {
+ rc = -ENOMEM;
+ goto error;
+ }
+
+ /* Initialize the tty_driver structure */
+
+ serial_driver->owner = THIS_MODULE;
+ serial_driver->driver_name = "synclinkmp";
+ serial_driver->name = "ttySLM";
+ serial_driver->major = ttymajor;
+ serial_driver->minor_start = 64;
+ serial_driver->type = TTY_DRIVER_TYPE_SERIAL;
+ serial_driver->subtype = SERIAL_TYPE_NORMAL;
+ serial_driver->init_termios = tty_std_termios;
+ serial_driver->init_termios.c_cflag =
+ B9600 | CS8 | CREAD | HUPCL | CLOCAL;
+ serial_driver->flags = TTY_DRIVER_REAL_RAW;
+ tty_set_operations(serial_driver, &ops);
+ if ((rc = tty_register_driver(serial_driver)) < 0) {
+ printk("%s(%d):Couldn't register serial driver\n",
+ __FILE__,__LINE__);
+ put_tty_driver(serial_driver);
+ serial_driver = NULL;
+ goto error;
+ }
+
+ printk("%s %s, tty major#%d\n",
+ driver_name, driver_version,
+ serial_driver->major);
+
+ return 0;
+
+error:
+ synclinkmp_cleanup();
+ return rc;
+}
+
+static void __exit synclinkmp_exit(void)
+{
+ synclinkmp_cleanup();
+}
+
module_init(synclinkmp_init);
module_exit(synclinkmp_exit);
On Tue, Jun 01, 2004 at 03:51:02PM -0500, Paul Fulghum wrote:
> In particular, call pci_unregister_driver if driver init fails. This
> is in response to a bug report by Dave Jones.
If pci_register_driver fails, the driver is not, repeat not left
registered. Therefore it must not be unregistered after failure
to register.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
On Tue, 2004-06-01 at 15:57, Russell King wrote:
> If pci_register_driver fails, the driver is not, repeat not left
> registered. Therefore it must not be unregistered after failure
> to register.
You are right. The specific problem I was trying to
fix is when no hardware is detected. I looked at other
PCI drivers (char/epca.c and net/eepro100.c) and which call
pci_unregister_driver if pci_register_driver returns <= 0
and indicates that pci_register_device returns the number
of pci devices detected. I now see that the two drivers I
looked at are broken. (bad luck that)
After looking at the source for pci_register_device(),
if no devices are detected, then it still returns 1.
I will rework the patches against synclink.c/synclinkmp.c
to only call pci_unregister_device() if init fails
(such as when no devices are detected)
*and* the call to pci_register_device() succeeds.
--
Paul Fulghum
[email protected]
On Tue, 2004-06-01 at 15:57, Russell King wrote:
> If pci_register_driver fails, the driver is not, repeat not left
> registered. Therefore it must not be unregistered after failure
> to register.
OK, here is a corrected patch that properly distinguishes
between pci_register_driver failure and the case
of finding no hardware.
--
Paul Fulghum
[email protected]
--- linux-2.6.6/drivers/char/synclinkmp.c 2004-06-02 09:07:40.495553141 -0500
+++ linux-2.6.6-mg1/drivers/char/synclinkmp.c 2004-06-02 09:08:05.720218567 -0500
@@ -1,5 +1,5 @@
/*
- * $Id: synclinkmp.c,v 4.19 2004/03/08 15:29:23 paulkf Exp $
+ * $Id: synclinkmp.c,v 4.21 2004/06/02 14:07:14 paulkf Exp $
*
* Device driver for Microgate SyncLink Multiport
* high speed multiprotocol serial adapter.
@@ -494,7 +494,7 @@
MODULE_PARM(dosyncppp,"1-" __MODULE_STRING(MAX_DEVICES) "i");
static char *driver_name = "SyncLink MultiPort driver";
-static char *driver_version = "$Revision: 4.19 $";
+static char *driver_version = "$Revision: 4.21 $";
static int synclinkmp_init_one(struct pci_dev *dev,const struct pci_device_id *ent);
static void synclinkmp_remove_one(struct pci_dev *dev);
@@ -3781,56 +3781,7 @@
.tiocmset = tiocmset,
};
-/* Driver initialization entry point.
- */
-
-static int __init synclinkmp_init(void)
-{
- if (break_on_load) {
- synclinkmp_get_text_ptr();
- BREAKPOINT();
- }
-
- printk("%s %s\n", driver_name, driver_version);
-
- synclinkmp_adapter_count = -1;
- pci_register_driver(&synclinkmp_pci_driver);
-
- if ( !synclinkmp_device_list ) {
- printk("%s(%d):No SyncLink devices found.\n",__FILE__,__LINE__);
- return -ENODEV;
- }
-
- serial_driver = alloc_tty_driver(synclinkmp_device_count);
- if (!serial_driver)
- return -ENOMEM;
-
- /* Initialize the tty_driver structure */
-
- serial_driver->owner = THIS_MODULE;
- serial_driver->driver_name = "synclinkmp";
- serial_driver->name = "ttySLM";
- serial_driver->major = ttymajor;
- serial_driver->minor_start = 64;
- serial_driver->type = TTY_DRIVER_TYPE_SERIAL;
- serial_driver->subtype = SERIAL_TYPE_NORMAL;
- serial_driver->init_termios = tty_std_termios;
- serial_driver->init_termios.c_cflag =
- B9600 | CS8 | CREAD | HUPCL | CLOCAL;
- serial_driver->flags = TTY_DRIVER_REAL_RAW;
- tty_set_operations(serial_driver, &ops);
- if (tty_register_driver(serial_driver) < 0)
- printk("%s(%d):Couldn't register serial driver\n",
- __FILE__,__LINE__);
-
- printk("%s %s, tty major#%d\n",
- driver_name, driver_version,
- serial_driver->major);
-
- return 0;
-}
-
-static void __exit synclinkmp_exit(void)
+static void synclinkmp_cleanup(void)
{
unsigned long flags;
int rc;
@@ -3839,10 +3790,12 @@
printk("Unloading %s %s\n", driver_name, driver_version);
- if ((rc = tty_unregister_driver(serial_driver)))
- printk("%s(%d) failed to unregister tty driver err=%d\n",
- __FILE__,__LINE__,rc);
- put_tty_driver(serial_driver);
+ if (serial_driver) {
+ if ((rc = tty_unregister_driver(serial_driver)))
+ printk("%s(%d) failed to unregister tty driver err=%d\n",
+ __FILE__,__LINE__,rc);
+ put_tty_driver(serial_driver);
+ }
info = synclinkmp_device_list;
while(info) {
@@ -3882,6 +3835,75 @@
pci_unregister_driver(&synclinkmp_pci_driver);
}
+/* Driver initialization entry point.
+ */
+
+static int __init synclinkmp_init(void)
+{
+ int rc;
+
+ if (break_on_load) {
+ synclinkmp_get_text_ptr();
+ BREAKPOINT();
+ }
+
+ printk("%s %s\n", driver_name, driver_version);
+
+ if ((rc = pci_register_driver(&synclinkmp_pci_driver)) < 0) {
+ printk("%s:failed to register PCI driver, error=%d\n",__FILE__,rc);
+ return rc;
+ }
+
+ if (!synclinkmp_device_list) {
+ printk("%s(%d):No SyncLink devices found.\n",__FILE__,__LINE__);
+ rc = -ENODEV;
+ goto error;
+ }
+
+ serial_driver = alloc_tty_driver(synclinkmp_device_count);
+ if (!serial_driver) {
+ rc = -ENOMEM;
+ goto error;
+ }
+
+ /* Initialize the tty_driver structure */
+
+ serial_driver->owner = THIS_MODULE;
+ serial_driver->driver_name = "synclinkmp";
+ serial_driver->name = "ttySLM";
+ serial_driver->major = ttymajor;
+ serial_driver->minor_start = 64;
+ serial_driver->type = TTY_DRIVER_TYPE_SERIAL;
+ serial_driver->subtype = SERIAL_TYPE_NORMAL;
+ serial_driver->init_termios = tty_std_termios;
+ serial_driver->init_termios.c_cflag =
+ B9600 | CS8 | CREAD | HUPCL | CLOCAL;
+ serial_driver->flags = TTY_DRIVER_REAL_RAW;
+ tty_set_operations(serial_driver, &ops);
+ if ((rc = tty_register_driver(serial_driver)) < 0) {
+ printk("%s(%d):Couldn't register serial driver\n",
+ __FILE__,__LINE__);
+ put_tty_driver(serial_driver);
+ serial_driver = NULL;
+ goto error;
+ }
+
+ printk("%s %s, tty major#%d\n",
+ driver_name, driver_version,
+ serial_driver->major);
+
+ return 0;
+
+error:
+ synclinkmp_cleanup();
+ return rc;
+}
+
+static void __exit synclinkmp_exit(void)
+{
+ synclinkmp_cleanup();
+}
+
module_init(synclinkmp_init);
module_exit(synclinkmp_exit);
On Tue, Jun 01, 2004 at 04:25:30PM -0500, Paul Fulghum wrote:
> On Tue, 2004-06-01 at 15:57, Russell King wrote:
> > If pci_register_driver fails, the driver is not, repeat not left
> > registered. Therefore it must not be unregistered after failure
> > to register.
>
> You are right. The specific problem I was trying to
> fix is when no hardware is detected. I looked at other
> PCI drivers (char/epca.c and net/eepro100.c) and which call
> pci_unregister_driver if pci_register_driver returns <= 0
> and indicates that pci_register_device returns the number
> of pci devices detected. I now see that the two drivers I
> looked at are broken. (bad luck that)
>
> After looking at the source for pci_register_device(),
> if no devices are detected, then it still returns 1.
>
> I will rework the patches against synclink.c/synclinkmp.c
> to only call pci_unregister_device() if init fails
> (such as when no devices are detected)
> *and* the call to pci_register_device() succeeds.
Don't arrange for the driver to unload if it doesn't detect anything.
2.6 has dynid support so that the user can load your driver and assign
it extra PCI vendor/device IDs at run time - which won't work if you've
forced failure when nothing is found.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
Russell King wrote:
>Don't arrange for the driver to unload if it doesn't detect anything.
>2.6 has dynid support so that the user can load your driver and assign
>it extra PCI vendor/device IDs at run time - which won't work if you've
>forced failure when nothing is found.
>
That sounds reasonable.
I'll add that change and resubmit the patches for synclink.c and
synclinkmp.c
--
Paul Fulghum
[email protected]