This series of patches removes the assumption that pnp_register_driver()
returns the number of devices claimed. Returning the count is unreliable
because devices may be hot-plugged in the future. (Many devices don't
support hot-plug, of course, but PNP in general does.)
This changes the convention to "zero for success, or a negative error
value," which matches pci_register_driver(), acpi_bus_register_driver(),
and platform_driver_register().
If drivers need to know the number of devices, they can count calls
to their .probe() methods.
Remove the assumption that pnp_register_driver() returns the number of
devices claimed.
parport_pc_init() does nothing with "count", so remove it. Then nobody
uses the return value of parport_pc_find_ports(), so make it void.
Finally, update pnp_register_driver() usage.
Signed-off-by: Bjorn Helgaas <[email protected]>
Index: work-mm4/drivers/parport/parport_pc.c
===================================================================
--- work-mm4.orig/drivers/parport/parport_pc.c 2006-03-01 15:37:16.000000000 -0700
+++ work-mm4/drivers/parport/parport_pc.c 2006-03-02 12:41:33.000000000 -0700
@@ -3126,9 +3126,9 @@
* autoirq is PARPORT_IRQ_NONE, PARPORT_IRQ_AUTO, or PARPORT_IRQ_PROBEONLY
* autodma is PARPORT_DMA_NONE or PARPORT_DMA_AUTO
*/
-static int __init parport_pc_find_ports (int autoirq, int autodma)
+static void __init parport_pc_find_ports (int autoirq, int autodma)
{
- int count = 0, r;
+ int count = 0, err;
#ifdef CONFIG_PARPORT_PC_SUPERIO
detect_and_report_winbond ();
@@ -3140,23 +3140,17 @@
/* PnP ports, skip detection if SuperIO already found them */
if (!count) {
- r = pnp_register_driver (&parport_pc_pnp_driver);
- if (r >= 0) {
+ err = pnp_register_driver (&parport_pc_pnp_driver);
+ if (!err)
pnp_registered_parport = 1;
- count += r;
- }
}
/* ISA ports and whatever (see asm/parport.h). */
- count += parport_pc_find_nonpci_ports (autoirq, autodma);
-
- r = pci_register_driver (&parport_pc_pci_driver);
- if (r)
- return r;
- pci_registered_parport = 1;
- count += 1;
+ parport_pc_find_nonpci_ports (autoirq, autodma);
- return count;
+ err = pci_register_driver (&parport_pc_pci_driver);
+ if (!err)
+ pci_registered_parport = 1;
}
/*
@@ -3381,8 +3375,6 @@
static int __init parport_pc_init(void)
{
- int count = 0;
-
if (parse_parport_params())
return -EINVAL;
@@ -3395,12 +3387,11 @@
break;
if ((io_hi[i]) == PARPORT_IOHI_AUTO)
io_hi[i] = 0x400 + io[i];
- if (parport_pc_probe_port(io[i], io_hi[i],
- irqval[i], dmaval[i], NULL))
- count++;
+ parport_pc_probe_port(io[i], io_hi[i],
+ irqval[i], dmaval[i], NULL);
}
} else
- count += parport_pc_find_ports (irqval[0], dmaval[0]);
+ parport_pc_find_ports (irqval[0], dmaval[0]);
return 0;
}
Remove the assumption that pnp_register_driver() returns the number of
devices claimed.
Signed-off-by: Bjorn Helgaas <[email protected]>
Index: work-mm4/sound/drivers/mpu401/mpu401.c
===================================================================
--- work-mm4.orig/sound/drivers/mpu401/mpu401.c 2006-02-01 16:24:33.000000000 -0700
+++ work-mm4/sound/drivers/mpu401/mpu401.c 2006-02-14 16:50:34.000000000 -0700
@@ -59,7 +59,8 @@
MODULE_PARM_DESC(irq, "IRQ # for MPU-401 device.");
static struct platform_device *platform_devices[SNDRV_CARDS];
-static int pnp_registered = 0;
+static int pnp_registered;
+static unsigned int snd_mpu401_devices;
static int snd_mpu401_create(int dev, struct snd_card **rcard)
{
@@ -197,6 +198,7 @@
}
snd_card_set_dev(card, &pnp_dev->dev);
pnp_set_drvdata(pnp_dev, card);
+ snd_mpu401_devices++;
++dev;
return 0;
}
@@ -234,12 +236,11 @@
static int __init alsa_card_mpu401_init(void)
{
- int i, err, devices;
+ int i, err;
if ((err = platform_driver_register(&snd_mpu401_driver)) < 0)
return err;
- devices = 0;
for (i = 0; i < SNDRV_CARDS && enable[i]; i++) {
struct platform_device *device;
#ifdef CONFIG_PNP
@@ -253,14 +254,13 @@
goto errout;
}
platform_devices[i] = device;
- devices++;
+ snd_mpu401_devices++;
}
- if ((err = pnp_register_driver(&snd_mpu401_pnp_driver)) >= 0) {
+ err = pnp_register_driver(&snd_mpu401_pnp_driver);
+ if (!err)
pnp_registered = 1;
- devices += err;
- }
- if (!devices) {
+ if (!snd_mpu401_devices) {
#ifdef MODULE
printk(KERN_ERR "MPU-401 device not found or device busy\n");
#endif
Remove the assumption that pnp_register_driver() returns the number of
devices claimed.
Signed-off-by: Bjorn Helgaas <[email protected]>
Index: work-mm4/sound/isa/cs423x/cs4236.c
===================================================================
--- work-mm4.orig/sound/isa/cs423x/cs4236.c 2006-02-22 09:55:41.000000000 -0700
+++ work-mm4/sound/isa/cs423x/cs4236.c 2006-02-22 10:05:14.000000000 -0700
@@ -133,6 +133,7 @@
static int pnp_registered;
#endif
#endif /* CONFIG_PNP */
+static unsigned int snd_cs423x_devices;
struct snd_card_cs4236 {
struct snd_cs4231 *chip;
@@ -564,7 +565,7 @@
snd_card_free(card);
return err;
}
-
+
platform_set_drvdata(pdev, card);
return 0;
}
@@ -650,6 +651,7 @@
}
pnp_set_drvdata(pdev, card);
dev++;
+ snd_cs423x_devices++;
return 0;
}
@@ -713,6 +715,7 @@
}
pnp_set_card_drvdata(pcard, card);
dev++;
+ snd_cs423x_devices++;
return 0;
}
@@ -721,7 +724,7 @@
snd_card_free(pnp_get_card_drvdata(pcard));
pnp_set_card_drvdata(pcard, NULL);
}
-
+
#ifdef CONFIG_PM
static int snd_cs423x_pnpc_suspend(struct pnp_card_link *pcard, pm_message_t state)
{
@@ -766,7 +769,7 @@
static int __init alsa_card_cs423x_init(void)
{
- int i, err, cards = 0;
+ int i, err;
if ((err = platform_driver_register(&cs423x_nonpnp_driver)) < 0)
return err;
@@ -782,24 +785,20 @@
goto errout;
}
platform_devices[i] = device;
- cards++;
+ snd_cs423x_devices++;
}
#ifdef CONFIG_PNP
#ifdef CS4232
- i = pnp_register_driver(&cs4232_pnp_driver);
- if (i >= 0) {
+ err = pnp_register_driver(&cs4232_pnp_driver);
+ if (!err)
pnp_registered = 1;
- cards += i;
- }
#endif
- i = pnp_register_card_driver(&cs423x_pnpc_driver);
- if (i >= 0) {
+ err = pnp_register_card_driver(&cs423x_pnpc_driver);
+ if (!err)
pnpc_registered = 1;
- cards += i;
- }
#endif /* CONFIG_PNP */
- if (!cards) {
+ if (!snd_cs423x_devices) {
#ifdef MODULE
printk(KERN_ERR IDENT " soundcard not found or device busy\n");
#endif
Remove the assumption that pnp_register_driver() returns the number of
devices claimed.
Signed-off-by: Bjorn Helgaas <[email protected]>
Index: work-mm4/sound/isa/opl3sa2.c
===================================================================
--- work-mm4.orig/sound/isa/opl3sa2.c 2006-02-01 16:24:54.000000000 -0700
+++ work-mm4/sound/isa/opl3sa2.c 2006-02-15 10:11:17.000000000 -0700
@@ -95,6 +95,7 @@
static int pnp_registered;
static int pnpc_registered;
#endif
+static unsigned int snd_opl3sa2_devices;
/* control ports */
#define OPL3SA2_PM_CTRL 0x01
@@ -760,6 +761,7 @@
}
pnp_set_drvdata(pdev, card);
dev++;
+ snd_opl3sa2_devices++;
return 0;
}
@@ -826,6 +828,7 @@
}
pnp_set_card_drvdata(pcard, card);
dev++;
+ snd_opl3sa2_devices++;
return 0;
}
@@ -944,7 +947,7 @@
static int __init alsa_card_opl3sa2_init(void)
{
- int i, err, cards = 0;
+ int i, err;
if ((err = platform_driver_register(&snd_opl3sa2_nonpnp_driver)) < 0)
return err;
@@ -962,23 +965,19 @@
goto errout;
}
platform_devices[i] = device;
- cards++;
+ snd_opl3sa2_devices++;
}
#ifdef CONFIG_PNP
err = pnp_register_driver(&opl3sa2_pnp_driver);
- if (err >= 0) {
+ if (!err)
pnp_registered = 1;
- cards += err;
- }
err = pnp_register_card_driver(&opl3sa2_pnpc_driver);
- if (err >= 0) {
+ if (!err)
pnpc_registered = 1;
- cards += err;
- }
#endif
- if (!cards) {
+ if (!snd_opl3sa2_devices) {
#ifdef MODULE
snd_printk(KERN_ERR "Yamaha OPL3-SA soundcard not found or device busy\n");
#endif
Remove the assumption that pnp_register_driver() returns the number of
devices claimed.
Signed-off-by: Bjorn Helgaas <[email protected]>
Index: work-mm4/drivers/input/serio/i8042-x86ia64io.h
===================================================================
--- work-mm4.orig/drivers/input/serio/i8042-x86ia64io.h 2006-03-02 12:40:45.000000000 -0700
+++ work-mm4/drivers/input/serio/i8042-x86ia64io.h 2006-03-02 12:45:21.000000000 -0700
@@ -192,7 +192,9 @@
#include <linux/pnp.h>
static int i8042_pnp_kbd_registered;
+static unsigned int i8042_pnp_kbd_devices;
static int i8042_pnp_aux_registered;
+static unsigned int i8042_pnp_aux_devices;
static int i8042_pnp_command_reg;
static int i8042_pnp_data_reg;
@@ -219,6 +221,7 @@
strncat(i8042_pnp_kbd_name, pnp_dev_name(dev), sizeof(i8042_pnp_kbd_name));
}
+ i8042_pnp_kbd_devices++;
return 0;
}
@@ -239,6 +242,7 @@
strncat(i8042_pnp_aux_name, pnp_dev_name(dev), sizeof(i8042_pnp_aux_name));
}
+ i8042_pnp_aux_devices++;
return 0;
}
@@ -287,21 +291,23 @@
static int __init i8042_pnp_init(void)
{
- int result_kbd = 0, result_aux = 0;
char kbd_irq_str[4] = { 0 }, aux_irq_str[4] = { 0 };
+ int err;
if (i8042_nopnp) {
printk(KERN_INFO "i8042: PNP detection disabled\n");
return 0;
}
- if ((result_kbd = pnp_register_driver(&i8042_pnp_kbd_driver)) >= 0)
+ err = pnp_register_driver(&i8042_pnp_kbd_driver);
+ if (!err)
i8042_pnp_kbd_registered = 1;
- if ((result_aux = pnp_register_driver(&i8042_pnp_aux_driver)) >= 0)
+ err = pnp_register_driver(&i8042_pnp_aux_driver);
+ if (!err)
i8042_pnp_aux_registered = 1;
- if (result_kbd <= 0 && result_aux <= 0) {
+ if (!i8042_pnp_kbd_devices && !i8042_pnp_aux_devices) {
i8042_pnp_exit();
#if defined(__ia64__)
return -ENODEV;
@@ -311,24 +317,24 @@
#endif
}
- if (result_kbd > 0)
+ if (i8042_pnp_kbd_devices)
snprintf(kbd_irq_str, sizeof(kbd_irq_str),
"%d", i8042_pnp_kbd_irq);
- if (result_aux > 0)
+ if (i8042_pnp_aux_devices)
snprintf(aux_irq_str, sizeof(aux_irq_str),
"%d", i8042_pnp_aux_irq);
printk(KERN_INFO "PNP: PS/2 Controller [%s%s%s] at %#x,%#x irq %s%s%s\n",
- i8042_pnp_kbd_name, (result_kbd > 0 && result_aux > 0) ? "," : "",
+ i8042_pnp_kbd_name, (i8042_pnp_kbd_devices && i8042_pnp_aux_devices) ? "," : "",
i8042_pnp_aux_name,
i8042_pnp_data_reg, i8042_pnp_command_reg,
- kbd_irq_str, (result_kbd > 0 && result_aux > 0) ? "," : "",
+ kbd_irq_str, (i8042_pnp_kbd_devices && i8042_pnp_aux_devices) ? "," : "",
aux_irq_str);
#if defined(__ia64__)
- if (result_kbd <= 0)
+ if (!i8042_pnp_kbd_devices)
i8042_nokbd = 1;
- if (result_aux <= 0)
+ if (!i8042_pnp_aux_devices)
i8042_noaux = 1;
#endif
Remove the assumption that pnp_register_driver() returns the number of
devices claimed.
Signed-off-by: Bjorn Helgaas <[email protected]>
Index: work-mm4/drivers/input/gameport/ns558.c
===================================================================
--- work-mm4.orig/drivers/input/gameport/ns558.c 2006-03-02 12:40:45.000000000 -0700
+++ work-mm4/drivers/input/gameport/ns558.c 2006-03-02 12:43:58.000000000 -0700
@@ -256,9 +256,10 @@
static int __init ns558_init(void)
{
- int i = 0;
+ int i = 0, err;
- if (pnp_register_driver(&ns558_pnp_driver) >= 0)
+ err = pnp_register_driver(&ns558_pnp_driver);
+ if (!err)
pnp_registered = 1;
/*
Remove the assumption that pnp_register_driver() returns the number of
devices claimed. Returning the count is unreliable because devices may
be hot-plugged in the future.
This changes the convention to "zero for success, or a negative error
value," which matches pci_register_driver(), acpi_bus_register_driver(),
and platform_driver_register().
Signed-off-by: Bjorn Helgaas <[email protected]>
Index: work-mm4/drivers/pnp/driver.c
===================================================================
--- work-mm4.orig/drivers/pnp/driver.c 2006-03-02 12:40:45.000000000 -0700
+++ work-mm4/drivers/pnp/driver.c 2006-03-02 12:46:49.000000000 -0700
@@ -201,31 +201,14 @@
.resume = pnp_bus_resume,
};
-
-static int count_devices(struct device * dev, void * c)
-{
- int * count = c;
- (*count)++;
- return 0;
-}
-
int pnp_register_driver(struct pnp_driver *drv)
{
- int count;
-
pnp_dbg("the driver '%s' has been registered", drv->name);
drv->driver.name = drv->name;
drv->driver.bus = &pnp_bus_type;
- count = driver_register(&drv->driver);
-
- /* get the number of initial matches */
- if (count >= 0){
- count = 0;
- driver_for_each_device(&drv->driver, NULL, &count, count_devices);
- }
- return count;
+ return driver_register(&drv->driver);
}
void pnp_unregister_driver(struct pnp_driver *drv)
Index: work-mm4/drivers/pnp/card.c
===================================================================
--- work-mm4.orig/drivers/pnp/card.c 2006-03-02 12:40:45.000000000 -0700
+++ work-mm4/drivers/pnp/card.c 2006-03-02 12:46:49.000000000 -0700
@@ -47,7 +47,7 @@
{
dev->card_link = NULL;
}
-
+
static void card_remove_first(struct pnp_dev * dev)
{
struct pnp_card_driver * drv = to_pnp_card_driver(dev->driver);
@@ -373,7 +373,7 @@
int pnp_register_card_driver(struct pnp_card_driver * drv)
{
- int count;
+ int error;
struct list_head *pos, *temp;
drv->link.name = drv->name;
@@ -384,21 +384,19 @@
drv->link.suspend = drv->suspend ? card_suspend : NULL;
drv->link.resume = drv->resume ? card_resume : NULL;
- count = pnp_register_driver(&drv->link);
- if (count < 0)
- return count;
+ error = pnp_register_driver(&drv->link);
+ if (error < 0)
+ return error;
spin_lock(&pnp_lock);
list_add_tail(&drv->global_list, &pnp_card_drivers);
spin_unlock(&pnp_lock);
- count = 0;
-
list_for_each_safe(pos,temp,&pnp_cards){
struct pnp_card *card = list_entry(pos, struct pnp_card, global_list);
- count += card_probe(card,drv);
+ card_probe(card,drv);
}
- return count;
+ return 0;
}
/**
Index: work-mm4/Documentation/pnp.txt
===================================================================
--- work-mm4.orig/Documentation/pnp.txt 2006-03-02 12:40:45.000000000 -0700
+++ work-mm4/Documentation/pnp.txt 2006-03-02 12:48:01.000000000 -0700
@@ -115,6 +115,9 @@
pnp_register_driver
- adds a PnP driver to the Plug and Play Layer
- this includes driver model integration
+- returns zero for success or a negative error number for failure; count
+ calls to the .add() method if you need to know how many devices bind to
+ the driver
pnp_unregister_driver
- removes a PnP driver from the Plug and Play Layer
Remove the assumption that pnp_register_driver() returns the number of
devices claimed.
Signed-off-by: Bjorn Helgaas <[email protected]>
Index: work-mm4/sound/oss/cs4232.c
===================================================================
--- work-mm4.orig/sound/oss/cs4232.c 2006-01-02 20:21:10.000000000 -0700
+++ work-mm4/sound/oss/cs4232.c 2006-02-15 15:44:34.000000000 -0700
@@ -360,6 +360,8 @@
static int __initdata synthirq = -1;
static int __initdata isapnp = 1;
+static unsigned int cs4232_devices;
+
MODULE_DESCRIPTION("CS4232 based soundcard driver");
MODULE_AUTHOR("Hannu Savolainen, Paul Barton-Davis");
MODULE_LICENSE("GPL");
@@ -421,6 +423,7 @@
return -ENODEV;
}
pnp_set_drvdata(dev,isapnpcfg);
+ cs4232_devices++;
return 0;
}
@@ -455,10 +458,11 @@
#endif
cfg.irq = -1;
- if (isapnp &&
- (pnp_register_driver(&cs4232_driver) > 0)
- )
- return 0;
+ if (isapnp) {
+ pnp_register_driver(&cs4232_driver);
+ if (cs4232_devices)
+ return 0;
+ }
if(io==-1||irq==-1||dma==-1)
{
@@ -503,7 +507,8 @@
int ints[7];
/* If we have isapnp cards, no need for options */
- if (pnp_register_driver(&cs4232_driver) > 0)
+ pnp_register_driver(&cs4232_driver);
+ if (cs4232_devices)
return 1;
str = get_options(str, ARRAY_SIZE(ints), ints);
Remove the assumption that pnp_register_driver() returns the number of
devices claimed.
Signed-off-by: Bjorn Helgaas <[email protected]>
Index: work-mm4/drivers/net/irda/nsc-ircc.c
===================================================================
--- work-mm4.orig/drivers/net/irda/nsc-ircc.c 2006-02-22 09:55:50.000000000 -0700
+++ work-mm4/drivers/net/irda/nsc-ircc.c 2006-02-22 10:06:53.000000000 -0700
@@ -207,7 +207,7 @@
/* Register with PnP subsystem to detect disable ports */
ret = pnp_register_driver(&nsc_ircc_pnp_driver);
- if (ret >= 0)
+ if (!ret)
pnp_registered = 1;
ret = -ENODEV;
On Thu, Mar 02, 2006 at 04:09:37PM -0700, Bjorn Helgaas wrote:
> Remove the assumption that pnp_register_driver() returns the number of
> devices claimed.
>
> Signed-off-by: Bjorn Helgaas <[email protected]>
Wouldn't a diff like
--- a/drivers/input/gameport/ns558.c 2006-03-02 12:40:45.000000000 -0700
+++ b/drivers/input/gameport/ns558.c 2006-03-02 12:43:58.000000000 -0700
@@ -258,7 +256,7 @@
{
int i = 0;
- if (pnp_register_driver(&ns558_pnp_driver) >= 0)
+ if (!pnp_register_driver(&ns558_pnp_driver))
pnp_registered = 1;
/*
be enough? The err variable isn't used anywhere else.
> Index: work-mm4/drivers/input/gameport/ns558.c
> ===================================================================
> --- work-mm4.orig/drivers/input/gameport/ns558.c 2006-03-02 12:40:45.000000000 -0700
> +++ work-mm4/drivers/input/gameport/ns558.c 2006-03-02 12:43:58.000000000 -0700
> @@ -256,9 +256,10 @@
>
> static int __init ns558_init(void)
> {
> - int i = 0;
> + int i = 0, err;
>
> - if (pnp_register_driver(&ns558_pnp_driver) >= 0)
> + err = pnp_register_driver(&ns558_pnp_driver);
> + if (!err)
> pnp_registered = 1;
>
> /*
>
>
--
Vojtech Pavlik
Director SuSE Labs
On Friday 03 March 2006 05:55, Vojtech Pavlik wrote:
> On Thu, Mar 02, 2006 at 04:09:37PM -0700, Bjorn Helgaas wrote:
> > Remove the assumption that pnp_register_driver() returns the number of
> > devices claimed.
> >
> > Signed-off-by: Bjorn Helgaas <[email protected]>
>
> Wouldn't a diff like
>
> --- a/drivers/input/gameport/ns558.c 2006-03-02 12:40:45.000000000 -0700
> +++ b/drivers/input/gameport/ns558.c 2006-03-02 12:43:58.000000000 -0700
> @@ -258,7 +256,7 @@
> {
> int i = 0;
>
> - if (pnp_register_driver(&ns558_pnp_driver) >= 0)
> + if (!pnp_register_driver(&ns558_pnp_driver))
> pnp_registered = 1;
>
> /*
>
> be enough? The err variable isn't used anywhere else.
Yup, that'd be fine, and in fact my first version of the patch
did just that. I introduced "err" because most of the other
drivers I had to touch used "err," and it seemed a bit clearer to me
because it gives an extra hint that pnp_register_driver() returns
only an error code.
But I don't care either way, so say the word, and I'll send
Andrew your patch to replace mine.
Bjorn
> > Index: work-mm4/drivers/input/gameport/ns558.c
> > ===================================================================
> > --- work-mm4.orig/drivers/input/gameport/ns558.c 2006-03-02 12:40:45.000000000 -0700
> > +++ work-mm4/drivers/input/gameport/ns558.c 2006-03-02 12:43:58.000000000 -0700
> > @@ -256,9 +256,10 @@
> >
> > static int __init ns558_init(void)
> > {
> > - int i = 0;
> > + int i = 0, err;
> >
> > - if (pnp_register_driver(&ns558_pnp_driver) >= 0)
> > + err = pnp_register_driver(&ns558_pnp_driver);
> > + if (!err)
> > pnp_registered = 1;
> >
> > /*
> >
> >
>
On Thursday 02 March 2006 18:09, Bjorn Helgaas wrote:
> Remove the assumption that pnp_register_driver() returns the number of
> devices claimed.
>
Applied, thank you Bjorn.
--
Dmitry
On Thu, Mar 02, 2006 at 04:01:27PM -0700, Bjorn Helgaas wrote:
> This series of patches removes the assumption that pnp_register_driver()
> returns the number of devices claimed. Returning the count is unreliable
> because devices may be hot-plugged in the future. (Many devices don't
> support hot-plug, of course, but PNP in general does.)
>
> This changes the convention to "zero for success, or a negative error
> value," which matches pci_register_driver(), acpi_bus_register_driver(),
> and platform_driver_register().
>
> If drivers need to know the number of devices, they can count calls
> to their .probe() methods.
Andrew, I'd appreciate if you add this to mainline when it's convenient to
make non-critical changes.
Thanks,
Adam