[ALSA 1/2] continue on IS_ERR from platform device registration

[ALSA 1/2] continue on IS_ERR from platform device registration

[Rene Herman]

[-- Attachment #1: Type: text/plain, Size: 1503 bytes --]

Rene Herman wrote:

> Yes. I don't see a significantly cleaner solution then than the
> slightly hackish "using drvdata as a private success flag" that I
> posted before. Example patch versus snd_adlib attached again. This
> seems to work well.
> 
> Takashi: do you agree? If the probe() method return is not going to
> be propagated up, there are few other options.
> 
> (Continuing the loop on IS_ERR(device) is then also a bit
> questionable again as the IS_ERR then signifies an eror in the bowels
> of the device model, but I feel it's still the correct thing to do)

If you do agree, here's both patches generated against 2.6.17-rc1-mm1.
First, the continue on IS_ERR one again.

   ad1848/ad1848.c       |   14 ++++----------
   cmi8330.c             |   14 ++++----------
   cs423x/cs4231.c       |   14 ++++----------
   cs423x/cs4236.c       |   14 ++++----------
   es1688/es1688.c       |   14 ++++----------
   es18xx.c              |   14 ++++----------
   gus/gusclassic.c      |   14 ++++----------
   gus/gusextreme.c      |   14 ++++----------
   gus/gusmax.c          |   14 ++++----------
   gus/interwave.c       |   14 ++++----------
   opl3sa2.c             |   14 ++++----------
   sb/sb16.c             |   14 ++++----------
   sb/sb8.c              |   14 ++++----------
   sgalaxy.c             |   14 ++++----------
   sscape.c              |   14 ++++----------
   wavefront/wavefront.c |   14 ++++----------
   16 files changed, 64 insertions(+), 160 deletions(-)

Rene.



[-- Attachment #2: alsa_platform_iserr.diff --]
[-- Type: text/plain, Size: 14844 bytes --]

Index: mm/sound/isa/ad1848/ad1848.c
===================================================================
--- mm.orig/sound/isa/ad1848/ad1848.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/ad1848/ad1848.c	2006-04-06 02:45:56.000000000 +0200
@@ -193,10 +193,8 @@ static int __init alsa_card_ad1848_init(
 			continue;
 		device = platform_device_register_simple(SND_AD1848_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -204,14 +202,10 @@ static int __init alsa_card_ad1848_init(
 #ifdef MODULE
 		printk(KERN_ERR "AD1848 soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_ad1848_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_ad1848_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_ad1848_exit(void)
Index: mm/sound/isa/cmi8330.c
===================================================================
--- mm.orig/sound/isa/cmi8330.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/cmi8330.c	2006-04-06 02:45:56.000000000 +0200
@@ -699,10 +699,8 @@ static int __init alsa_card_cmi8330_init
 			continue;
 		device = platform_device_register_simple(CMI8330_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		cards++;
 	}
@@ -719,14 +717,10 @@ static int __init alsa_card_cmi8330_init
 #ifdef MODULE
 		snd_printk(KERN_ERR "CMI8330 not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_cmi8330_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_cmi8330_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_cmi8330_exit(void)
Index: mm/sound/isa/cs423x/cs4231.c
===================================================================
--- mm.orig/sound/isa/cs423x/cs4231.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/cs423x/cs4231.c	2006-04-06 02:45:56.000000000 +0200
@@ -209,10 +209,8 @@ static int __init alsa_card_cs4231_init(
 			continue;
 		device = platform_device_register_simple(SND_CS4231_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -220,14 +218,10 @@ static int __init alsa_card_cs4231_init(
 #ifdef MODULE
 		printk(KERN_ERR "CS4231 soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_cs4231_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_cs4231_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_cs4231_exit(void)
Index: mm/sound/isa/cs423x/cs4236.c
===================================================================
--- mm.orig/sound/isa/cs423x/cs4236.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/cs423x/cs4236.c	2006-04-06 02:45:56.000000000 +0200
@@ -780,10 +780,8 @@ static int __init alsa_card_cs423x_init(
 			continue;
 		device = platform_device_register_simple(CS423X_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		snd_cs423x_devices++;
 	}
@@ -802,14 +800,10 @@ static int __init alsa_card_cs423x_init(
 #ifdef MODULE
 		printk(KERN_ERR IDENT " soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_cs423x_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_cs423x_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_cs423x_exit(void)
Index: mm/sound/isa/es1688/es1688.c
===================================================================
--- mm.orig/sound/isa/es1688/es1688.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/es1688/es1688.c	2006-04-06 02:45:56.000000000 +0200
@@ -213,10 +213,8 @@ static int __init alsa_card_es1688_init(
 			continue;
 		device = platform_device_register_simple(ES1688_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -224,14 +222,10 @@ static int __init alsa_card_es1688_init(
 #ifdef MODULE
 		printk(KERN_ERR "ESS AudioDrive ES1688 soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_es1688_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_es1688_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_es1688_exit(void)
Index: mm/sound/isa/gus/gusclassic.c
===================================================================
--- mm.orig/sound/isa/gus/gusclassic.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/gus/gusclassic.c	2006-04-06 02:45:56.000000000 +0200
@@ -253,10 +253,8 @@ static int __init alsa_card_gusclassic_i
 			continue;
 		device = platform_device_register_simple(GUSCLASSIC_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -264,14 +262,10 @@ static int __init alsa_card_gusclassic_i
 #ifdef MODULE
 		printk(KERN_ERR "GUS Classic soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_gusclassic_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_gusclassic_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_gusclassic_exit(void)
Index: mm/sound/isa/gus/gusextreme.c
===================================================================
--- mm.orig/sound/isa/gus/gusextreme.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/gus/gusextreme.c	2006-04-06 02:45:56.000000000 +0200
@@ -363,10 +363,8 @@ static int __init alsa_card_gusextreme_i
 			continue;
 		device = platform_device_register_simple(GUSEXTREME_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -374,14 +372,10 @@ static int __init alsa_card_gusextreme_i
 #ifdef MODULE
 		printk(KERN_ERR "GUS Extreme soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_gusextreme_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_gusextreme_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_gusextreme_exit(void)
Index: mm/sound/isa/gus/gusmax.c
===================================================================
--- mm.orig/sound/isa/gus/gusmax.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/gus/gusmax.c	2006-04-06 02:45:56.000000000 +0200
@@ -390,10 +390,8 @@ static int __init alsa_card_gusmax_init(
 			continue;
 		device = platform_device_register_simple(GUSMAX_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -401,14 +399,10 @@ static int __init alsa_card_gusmax_init(
 #ifdef MODULE
 		printk(KERN_ERR "GUS MAX soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_gusmax_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_gusmax_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_gusmax_exit(void)
Index: mm/sound/isa/gus/interwave.c
===================================================================
--- mm.orig/sound/isa/gus/interwave.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/gus/interwave.c	2006-04-06 02:45:56.000000000 +0200
@@ -947,10 +947,8 @@ static int __init alsa_card_interwave_in
 #endif
 		device = platform_device_register_simple(INTERWAVE_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		cards++;
 	}
@@ -966,14 +964,10 @@ static int __init alsa_card_interwave_in
 #ifdef MODULE
 		printk(KERN_ERR "InterWave soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_interwave_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_interwave_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_interwave_exit(void)
Index: mm/sound/isa/opl3sa2.c
===================================================================
--- mm.orig/sound/isa/opl3sa2.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/opl3sa2.c	2006-04-06 02:45:56.000000000 +0200
@@ -962,10 +962,8 @@ static int __init alsa_card_opl3sa2_init
 #endif
 		device = platform_device_register_simple(OPL3SA2_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		snd_opl3sa2_devices++;
 	}
@@ -983,14 +981,10 @@ static int __init alsa_card_opl3sa2_init
 #ifdef MODULE
 		snd_printk(KERN_ERR "Yamaha OPL3-SA soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_opl3sa2_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_opl3sa2_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_opl3sa2_exit(void)
Index: mm/sound/isa/sb/sb16.c
===================================================================
--- mm.orig/sound/isa/sb/sb16.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/sb/sb16.c	2006-04-06 02:45:56.000000000 +0200
@@ -721,10 +721,8 @@ static int __init alsa_card_sb16_init(vo
 			continue;
 		device = platform_device_register_simple(SND_SB16_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		cards++;
 	}
@@ -746,14 +744,10 @@ static int __init alsa_card_sb16_init(vo
 		snd_printk(KERN_ERR "In case, if you have AWE card, try snd-sbawe module\n");
 #endif
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_sb16_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_sb16_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_sb16_exit(void)
Index: mm/sound/isa/sb/sb8.c
===================================================================
--- mm.orig/sound/isa/sb/sb8.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/sb/sb8.c	2006-04-06 02:45:56.000000000 +0200
@@ -264,10 +264,8 @@ static int __init alsa_card_sb8_init(voi
 			continue;
 		device = platform_device_register_simple(SND_SB8_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -275,14 +273,10 @@ static int __init alsa_card_sb8_init(voi
 #ifdef MODULE
 		snd_printk(KERN_ERR "Sound Blaster soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_sb8_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_sb8_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_sb8_exit(void)
Index: mm/sound/isa/sgalaxy.c
===================================================================
--- mm.orig/sound/isa/sgalaxy.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/sgalaxy.c	2006-04-06 02:45:56.000000000 +0200
@@ -366,10 +366,8 @@ static int __init alsa_card_sgalaxy_init
 			continue;
 		device = platform_device_register_simple(SND_SGALAXY_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -377,14 +375,10 @@ static int __init alsa_card_sgalaxy_init
 #ifdef MODULE
 		snd_printk(KERN_ERR "Sound Galaxy soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_sgalaxy_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_sgalaxy_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_sgalaxy_exit(void)
Index: mm/sound/isa/wavefront/wavefront.c
===================================================================
--- mm.orig/sound/isa/wavefront/wavefront.c	2006-04-06 02:16:26.000000000 +0200
+++ mm/sound/isa/wavefront/wavefront.c	2006-04-06 02:45:56.000000000 +0200
@@ -722,10 +722,8 @@ static int __init alsa_card_wavefront_in
 #endif
 		device = platform_device_register_simple(WAVEFRONT_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		cards++;
 	}
@@ -742,14 +740,10 @@ static int __init alsa_card_wavefront_in
 #ifdef MODULE
 		printk (KERN_ERR "No WaveFront cards found or devices busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_wavefront_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_wavefront_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_wavefront_exit(void)
Index: mm/sound/isa/es18xx.c
===================================================================
--- mm.orig/sound/isa/es18xx.c	2006-04-06 02:28:33.000000000 +0200
+++ mm/sound/isa/es18xx.c	2006-04-06 02:45:56.000000000 +0200
@@ -2392,10 +2392,8 @@ static int __init alsa_card_es18xx_init(
 			continue;
 		device = platform_device_register_simple(ES18XX_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+	       		continue;
 		platform_devices[i] = device;
 		cards++;
 	}
@@ -2412,14 +2410,10 @@ static int __init alsa_card_es18xx_init(
 #ifdef MODULE
 		snd_printk(KERN_ERR "ESS AudioDrive ES18xx soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_es18xx_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_es18xx_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_es18xx_exit(void)
Index: mm/sound/isa/sscape.c
===================================================================
--- mm.orig/sound/isa/sscape.c	2006-04-06 02:08:30.000000000 +0200
+++ mm/sound/isa/sscape.c	2006-04-06 02:52:33.000000000 +0200
@@ -1427,8 +1427,8 @@ static int __init sscape_manual_probe(vo
 		    dma[i] == SNDRV_AUTO_DMA) {
 			printk(KERN_INFO
 			       "sscape: insufficient parameters, need IO, IRQ, MPU-IRQ and DMA\n");
-			ret = -ENXIO;
-			goto errout;
+			sscape_unregister_all();
+			return -ENXIO;
 		}
 
 		/*
@@ -1436,17 +1436,11 @@ static int __init sscape_manual_probe(vo
 		 */
 		device = platform_device_register_simple(SSCAPE_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			ret = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 	}
 	return 0;
-
- errout:
-	sscape_unregister_all();
-	return ret;
 }
 
 static void sscape_exit(void)

Re: [ALSA 1/2] continue on IS_ERR from platform device registration

[Takashi Iwai]

At Thu, 06 Apr 2006 04:08:34 +0200,
Rene Herman wrote:
> 
> Rene Herman wrote:
> 
> > Yes. I don't see a significantly cleaner solution then than the
> > slightly hackish "using drvdata as a private success flag" that I
> > posted before. Example patch versus snd_adlib attached again. This
> > seems to work well.
> > 
> > Takashi: do you agree? If the probe() method return is not going to
> > be propagated up, there are few other options.
> > 
> > (Continuing the loop on IS_ERR(device) is then also a bit
> > questionable again as the IS_ERR then signifies an eror in the bowels
> > of the device model, but I feel it's still the correct thing to do)
> 
> If you do agree, here's both patches generated against 2.6.17-rc1-mm1.
> First, the continue on IS_ERR one again.

Well, I'm not so confident that it's the way to go.

The problem is that currently the ALSA ISA driver wants to refuse
loading (and modprobe returns an error) when no devices are found at
loading time.  On 2.2/2.4 kernels, PCI drivers also show the same
behavior, but it was changed for a good reason.

Then, shouldn't be the ISA drivers changed to follow the same style?
That is, we keep pnp_driver and platform_driver regardless probe calls
succeeded or not.  They can be, in theory, later bound/activated over
sysfs.

At least it would make the code more simpler, I guess.


Takashi

Re: [ALSA 1/2] continue on IS_ERR from platform device registration

[Rene Herman]

[-- Attachment #1: Type: text/plain, Size: 4658 bytes --]

Takashi Iwai wrote:

[ unregistering platform devices again when no device found ]

> Well, I'm not so confident that it's the way to go.

Okay... Added Greg back to the CC. Don't know if he wants to comment or not.

> The problem is that currently the ALSA ISA driver wants to refuse 
> loading (and modprobe returns an error) when no devices are found at 
> loading time.  On 2.2/2.4 kernels, PCI drivers also show the same 
> behavior, but it was changed for a good reason.
> 
> Then, shouldn't be the ISA drivers changed to follow the same style? 
> That is, we keep pnp_driver and platform_driver regardless probe
> calls succeeded or not.  They can be, in theory, later
> bound/activated over sysfs.

The thing I'm stumbling over is the non (generic) discoverability of ISA 
versus busses such as ISA-PnP and PCI which makes for a big conceptual 
difference.

A PnP/PCI device has a life all by itself by virtue of its bus knowing 
that it's present. For one, the device will be present in /sys/devices/ 
without any specific driver for the device loaded yet. A platform device 
on the other hand only "exists" by virtue of a driver creating it 
because it might want to drive it. If we keep it registered even after 
failing a probe, then /sys/devices/platform turns into a view of "what 
drivers did we load" rather then "what's present in this system". As far 
as I'm aware, that latter view of /sys/devices was at one time the idea. 
Just imagine loading all ISA drivers for an appreciation of the amount 
of pollution to this view always keeping the devices registered does.

However!

I must say I wasn't aware that ALSA PCI devices at the moment load 
without devices. Wasn't there even an oft used ALSA configuration script 
out there that worked by loading all drivers and checking which ones stuck?

I just checked the behaviour and yes, the drivers load. Most importantly 
to the analogy, even without any of the supported IDs present.

I have a 125d:1978 (ESS Canyon3D) at PCI 0000\:00\:08\:0. If I remove 
the 125d:1978 device ID from snd-es1968, then yes, snd-es1968 still 
loads. With the ID still not present, a:

echo -n 0000\:00\:08.0 >"/sys/bus/pci/drivers/ES1968 (ESS Maestro)/bind"

runs into a bug somewhere it seems. The xterm I'm doing that in now 
hangs (but is killable). Doing:

echo "125d 1978" >"/sys/bus/pci/drivers/ES1968 (ESS Maestro)/new_id"

instead does work fine, and after this binding and unbinding work fine 
as well again, which means there at least seems to be some point.

In the analogy, given that the PCI driver loads even without any of its 
IDs present means a platform_driver loading without anything present as 
well isn't, from the view of the driver model, much of a difference 
after all. PnP doesn't have a "new_id" field, but could have, and I 
therefore in fact agree by now that it's best to follow PCI's lead here.

> At least it would make the code more simpler, I guess.

Not significantly. We'd need to seperate the probe() method a bit, so 
that the things we need before a bind _could_ ever succeed were in the 
main loop and would make the driver skip device registration if not 
fullfilled. For example, if with the snd-adlib driver no port= is given, 
then attempted binds would simply tell you "please specify port" over 
and over again, something which can only be solved by unloading and 
reloading the driver, this time specifying the port. This is still a 
difference due to non-discoverability, and I feel this should still make 
the driver fail to load.

For snd-adlib, this would look like the attached patch. I'm also being 
more verbose about which bus_id is failing. The !cards printk() is gone, 
as it is no longer a matter of "device not found or busy" (and there 
will be a printk() from the actual error spot anyway) and changed the 
-ENODEV there to -EINVAL.

It works. "modprobe snd-adlib" fails to load, and "modprobe snd-adlib 
port=0x388" succeeds, with or without probe failures. I can for example 
now do:

# modprobe snd-cs4236		// which claims 0x388
# modprobe snd-adlib port=0x388	// which has it load
# modprobe -r snd-cs4236 && modprobe snd-cs4236 fm_port=0

which enables the OPL at 0x388, but leaves it alone, so that I can then:

# echo -n snd_adlib.0 >/sys/bus/platform/drivers/snd_adlib/bind

to bind the snd_adlib driver to it. Which is... erm... great fun I guess.

(I am by the way being so specific about what I'm doing with these sysfs 
things because until Dmitry pointed them out to me in the precursor 
discussion, I was't even aware of bind/unbind. Want to make sure I 
understand how to operate it all).

How do you feel about this one?

Rene.


[-- Attachment #2: adlib-platform_keep.diff --]
[-- Type: text/plain, Size: 3430 bytes --]

Index: local/sound/isa/adlib.c
===================================================================
--- local.orig/sound/isa/adlib.c	2006-04-06 23:10:52.000000000 +0200
+++ local/sound/isa/adlib.c	2006-04-07 01:42:11.000000000 +0200
@@ -43,25 +43,19 @@ static int __devinit snd_adlib_probe(str
 	struct snd_card *card;
 	struct snd_opl3 *opl3;
 
-	int error;
-	int i = device->id;
-
-	if (port[i] == SNDRV_AUTO_PORT) {
-		snd_printk(KERN_ERR DRV_NAME ": please specify port\n");
-		error = -EINVAL;
-		goto out0;
-	}
+	int error, i = device->id;
+	char *bus_id = device->dev.bus_id;
 
 	card = snd_card_new(index[i], id[i], THIS_MODULE, 0);
 	if (!card) {
-		snd_printk(KERN_ERR DRV_NAME ": could not create card\n");
+		snd_printk(KERN_ERR "%s: could not create card\n", bus_id);
 		error = -EINVAL;
 		goto out0;
 	}
 
 	card->private_data = request_region(port[i], 4, CRD_NAME);
 	if (!card->private_data) {
-		snd_printk(KERN_ERR DRV_NAME ": could not grab ports\n");
+		snd_printk(KERN_ERR "%s: could not grab ports\n", bus_id);
 		error = -EBUSY;
 		goto out1;
 	}
@@ -69,13 +63,13 @@ static int __devinit snd_adlib_probe(str
 
 	error = snd_opl3_create(card, port[i], port[i] + 2, OPL3_HW_AUTO, 1, &opl3);
 	if (error < 0) {
-		snd_printk(KERN_ERR DRV_NAME ": could not create OPL\n");
+		snd_printk(KERN_ERR "%s: could not create OPL\n", bus_id);
 		goto out1;
 	}
 
 	error = snd_opl3_hwdep_new(opl3, 0, 0, NULL);
 	if (error < 0) {
-		snd_printk(KERN_ERR DRV_NAME ": could not create FM\n");
+		snd_printk(KERN_ERR "%s: could not create FM\n", bus_id);
 		goto out1;
 	}
 
@@ -87,7 +81,7 @@ static int __devinit snd_adlib_probe(str
 
 	error = snd_card_register(card);
 	if (error < 0) {
-		snd_printk(KERN_ERR DRV_NAME ": could not register card\n");
+		snd_printk(KERN_ERR "%s: could not register card\n", bus_id);
 		goto out1;
 	}
 
@@ -95,8 +89,7 @@ static int __devinit snd_adlib_probe(str
 	return 0;
 
 out1:	snd_card_free(card);
- out0:	error = -EINVAL; /* FIXME: should be the original error code */
-	return error;
+out0:	return error;
 }
 
 static int __devexit snd_adlib_remove(struct platform_device *device)
@@ -109,7 +102,6 @@ static int __devexit snd_adlib_remove(st
 static struct platform_driver snd_adlib_driver = {
 	.probe		= snd_adlib_probe,
 	.remove		= __devexit_p(snd_adlib_remove),
-
 	.driver		= {
 		.name	= DRV_NAME
 	}
@@ -119,9 +111,10 @@ static int __init alsa_card_adlib_init(v
 {
 	int i, cards;
 
-	if (platform_driver_register(&snd_adlib_driver) < 0) {
+	i = platform_driver_register(&snd_adlib_driver);
+	if (i < 0) {
 		snd_printk(KERN_ERR DRV_NAME ": could not register driver\n");
-		return -ENODEV;
+		return i;
 	}
 
 	for (cards = 0, i = 0; i < SNDRV_CARDS; i++) {
@@ -130,21 +123,26 @@ static int __init alsa_card_adlib_init(v
 		if (!enable[i])
 			continue;
 
+		if (port[i] == SNDRV_AUTO_PORT) {
+			snd_printk(KERN_ERR DRV_NAME ": please specify port for card %d\n", i);
+			continue;
+		}
+
 		device = platform_device_register_simple(DRV_NAME, i, NULL, 0);
-		if (IS_ERR(device))
+		if (IS_ERR(device)) {
+			snd_printk(KERN_ERR DRV_NAME ": could not register device for card %d\n", i);
 			continue;
+		}
 
 		devices[i] = device;
 		cards++;
 	}
 
 	if (!cards) {
-#ifdef MODULE
-		printk(KERN_ERR CRD_NAME " soundcard not found or device busy\n");
-#endif
 		platform_driver_unregister(&snd_adlib_driver);
-		return -ENODEV;
+		return -EINVAL;
 	}
+
 	return 0;
 }
 

Re: [Alsa-devel] Re: [ALSA 1/2] continue on IS_ERR from platform device registration

[Takashi Iwai]

At Fri, 07 Apr 2006 18:26:26 +0200,
Rene Herman wrote:
> 
> I must say I wasn't aware that ALSA PCI devices at the moment load 
> without devices. Wasn't there even an oft used ALSA configuration script 
> out there that worked by loading all drivers and checking which ones stuck?

Not for PCI.  alsaconf checks the return status of modprobe for ISA
non-PnP devices, though, so we would fix it once if the probe model
were changed.

(snip)
> In the analogy, given that the PCI driver loads even without any of its 
> IDs present means a platform_driver loading without anything present as 
> well isn't, from the view of the driver model, much of a difference 
> after all. PnP doesn't have a "new_id" field, but could have, and I 
> therefore in fact agree by now that it's best to follow PCI's lead here.

Exactly, that's the reason I suggested.  To keep the behavior of
driver consistent.

OTOH, I'm not 100% for that change, because I know that it will break
the old behavior as mentioned in the above.  Such a breakage isn't
always a good buy against just-for-cleanness or consistency...

> > At least it would make the code more simpler, I guess.
> 
> Not significantly.

Well, you can remove the whole

	if (!cards)
		... unregister...

in the probe callback if we follow the pci driver model, i.e. modprobe
doesn't return an error if no device is found at the moment.

> We'd need to seperate the probe() method a bit, so 
> that the things we need before a bind _could_ ever succeed were in the 
> main loop and would make the driver skip device registration if not 
> fullfilled. For example, if with the snd-adlib driver no port= is given, 
> then attempted binds would simply tell you "please specify port" over 
> and over again, something which can only be solved by unloading and 
> reloading the driver, this time specifying the port. This is still a 
> difference due to non-discoverability, and I feel this should still make 
> the driver fail to load.

Hm, surely it's not so intuitive in the case of ISA devices.  Maybe
it'd be better to keep the current behavior:  probe() returns an error
if no device is found at loading...


Takashi

Re: [Alsa-devel] Re: [ALSA 1/2] continue on IS_ERR from platform device registration

[Rene Herman]

Takashi Iwai wrote:

> Hm, surely it's not so intuitive in the case of ISA devices.  Maybe 
> it'd be better to keep the current behavior:  probe() returns an
> error if no device is found at loading...

Okay. It's always possible to revisit the isssue later. Keeping the old
behaviour is what the already submitted patches did. probe() returning
an error is not enough; the issue was/is that that error is not
being passed up. Using drvdata as a private success flag as submitted
works fine fortunately; all drivers do a platform_set_drvdata(device,
card) just before returning success from probe().

I'll repost them following this message. Have been generated against
2.6.17-rc1-mm2.

Will also post them (and the !enable[i] patch) against 2.6.16.2 for
-stable. You already acked the !enable[i] and continue-on-is_err patches
for -stable, the third is the same unregister patch, restoring the old
fail-to-load behaviour also for -stable.

Considering that one of the rules for -stable is that fixes also need to 
be present in upstream trees, could you relay the three patches for 
-stable yourself?

Rene.

[ALSA 1/2] continue on IS_ERR from platform device registration

[Rene Herman]

[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]

Hi Takashi.

Continue with the next one on error from device registration. Against
2.6.17-rc1-mm2.

This would seem the correct thing to do, even if it's not the probe()
error that we're getting.

    sound/isa/ad1848/ad1848.c       |   14 ++++----------
    sound/isa/cmi8330.c             |   14 ++++----------
    sound/isa/cs423x/cs4231.c       |   14 ++++----------
    sound/isa/cs423x/cs4236.c       |   14 ++++----------
    sound/isa/es1688/es1688.c       |   14 ++++----------
    sound/isa/es18xx.c              |   14 ++++----------
    sound/isa/gus/gusclassic.c      |   14 ++++----------
    sound/isa/gus/gusextreme.c      |   14 ++++----------
    sound/isa/gus/gusmax.c          |   14 ++++----------
    sound/isa/gus/interwave.c       |   14 ++++----------
    sound/isa/opl3sa2.c             |   14 ++++----------
    sound/isa/sb/sb16.c             |   14 ++++----------
    sound/isa/sb/sb8.c              |   14 ++++----------
    sound/isa/sgalaxy.c             |   14 ++++----------
    sound/isa/sscape.c              |   14 ++++----------
    sound/isa/wavefront/wavefront.c |   14 ++++----------
    16 files changed, 64 insertions(+), 160 deletions(-)

Rene




[-- Attachment #2: alsa_platform_iserr.diff --]
[-- Type: text/plain, Size: 14845 bytes --]

Index: mm/sound/isa/ad1848/ad1848.c
===================================================================
--- mm.orig/sound/isa/ad1848/ad1848.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/ad1848/ad1848.c	2006-04-10 23:44:32.000000000 +0200
@@ -193,10 +193,8 @@ static int __init alsa_card_ad1848_init(
 			continue;
 		device = platform_device_register_simple(SND_AD1848_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -204,14 +202,10 @@ static int __init alsa_card_ad1848_init(
 #ifdef MODULE
 		printk(KERN_ERR "AD1848 soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_ad1848_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_ad1848_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_ad1848_exit(void)
Index: mm/sound/isa/cmi8330.c
===================================================================
--- mm.orig/sound/isa/cmi8330.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/cmi8330.c	2006-04-10 23:44:32.000000000 +0200
@@ -699,10 +699,8 @@ static int __init alsa_card_cmi8330_init
 			continue;
 		device = platform_device_register_simple(CMI8330_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		cards++;
 	}
@@ -719,14 +717,10 @@ static int __init alsa_card_cmi8330_init
 #ifdef MODULE
 		snd_printk(KERN_ERR "CMI8330 not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_cmi8330_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_cmi8330_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_cmi8330_exit(void)
Index: mm/sound/isa/cs423x/cs4231.c
===================================================================
--- mm.orig/sound/isa/cs423x/cs4231.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/cs423x/cs4231.c	2006-04-10 23:44:32.000000000 +0200
@@ -209,10 +209,8 @@ static int __init alsa_card_cs4231_init(
 			continue;
 		device = platform_device_register_simple(SND_CS4231_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -220,14 +218,10 @@ static int __init alsa_card_cs4231_init(
 #ifdef MODULE
 		printk(KERN_ERR "CS4231 soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_cs4231_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_cs4231_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_cs4231_exit(void)
Index: mm/sound/isa/cs423x/cs4236.c
===================================================================
--- mm.orig/sound/isa/cs423x/cs4236.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/cs423x/cs4236.c	2006-04-10 23:44:32.000000000 +0200
@@ -780,10 +780,8 @@ static int __init alsa_card_cs423x_init(
 			continue;
 		device = platform_device_register_simple(CS423X_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		snd_cs423x_devices++;
 	}
@@ -802,14 +800,10 @@ static int __init alsa_card_cs423x_init(
 #ifdef MODULE
 		printk(KERN_ERR IDENT " soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_cs423x_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_cs423x_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_cs423x_exit(void)
Index: mm/sound/isa/es1688/es1688.c
===================================================================
--- mm.orig/sound/isa/es1688/es1688.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/es1688/es1688.c	2006-04-10 23:44:32.000000000 +0200
@@ -213,10 +213,8 @@ static int __init alsa_card_es1688_init(
 			continue;
 		device = platform_device_register_simple(ES1688_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -224,14 +222,10 @@ static int __init alsa_card_es1688_init(
 #ifdef MODULE
 		printk(KERN_ERR "ESS AudioDrive ES1688 soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_es1688_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_es1688_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_es1688_exit(void)
Index: mm/sound/isa/gus/gusclassic.c
===================================================================
--- mm.orig/sound/isa/gus/gusclassic.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/gus/gusclassic.c	2006-04-10 23:44:32.000000000 +0200
@@ -253,10 +253,8 @@ static int __init alsa_card_gusclassic_i
 			continue;
 		device = platform_device_register_simple(GUSCLASSIC_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -264,14 +262,10 @@ static int __init alsa_card_gusclassic_i
 #ifdef MODULE
 		printk(KERN_ERR "GUS Classic soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_gusclassic_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_gusclassic_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_gusclassic_exit(void)
Index: mm/sound/isa/gus/gusextreme.c
===================================================================
--- mm.orig/sound/isa/gus/gusextreme.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/gus/gusextreme.c	2006-04-10 23:44:32.000000000 +0200
@@ -363,10 +363,8 @@ static int __init alsa_card_gusextreme_i
 			continue;
 		device = platform_device_register_simple(GUSEXTREME_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -374,14 +372,10 @@ static int __init alsa_card_gusextreme_i
 #ifdef MODULE
 		printk(KERN_ERR "GUS Extreme soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_gusextreme_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_gusextreme_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_gusextreme_exit(void)
Index: mm/sound/isa/gus/gusmax.c
===================================================================
--- mm.orig/sound/isa/gus/gusmax.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/gus/gusmax.c	2006-04-10 23:44:32.000000000 +0200
@@ -390,10 +390,8 @@ static int __init alsa_card_gusmax_init(
 			continue;
 		device = platform_device_register_simple(GUSMAX_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -401,14 +399,10 @@ static int __init alsa_card_gusmax_init(
 #ifdef MODULE
 		printk(KERN_ERR "GUS MAX soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_gusmax_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_gusmax_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_gusmax_exit(void)
Index: mm/sound/isa/gus/interwave.c
===================================================================
--- mm.orig/sound/isa/gus/interwave.c	2006-04-10 23:44:24.000000000 +0200
+++ mm/sound/isa/gus/interwave.c	2006-04-10 23:44:32.000000000 +0200
@@ -947,10 +947,8 @@ static int __init alsa_card_interwave_in
 #endif
 		device = platform_device_register_simple(INTERWAVE_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		cards++;
 	}
@@ -966,14 +964,10 @@ static int __init alsa_card_interwave_in
 #ifdef MODULE
 		printk(KERN_ERR "InterWave soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_interwave_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_interwave_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_interwave_exit(void)
Index: mm/sound/isa/opl3sa2.c
===================================================================
--- mm.orig/sound/isa/opl3sa2.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/opl3sa2.c	2006-04-10 23:44:32.000000000 +0200
@@ -962,10 +962,8 @@ static int __init alsa_card_opl3sa2_init
 #endif
 		device = platform_device_register_simple(OPL3SA2_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		snd_opl3sa2_devices++;
 	}
@@ -983,14 +981,10 @@ static int __init alsa_card_opl3sa2_init
 #ifdef MODULE
 		snd_printk(KERN_ERR "Yamaha OPL3-SA soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_opl3sa2_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_opl3sa2_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_opl3sa2_exit(void)
Index: mm/sound/isa/sb/sb16.c
===================================================================
--- mm.orig/sound/isa/sb/sb16.c	2006-04-10 23:44:24.000000000 +0200
+++ mm/sound/isa/sb/sb16.c	2006-04-10 23:44:32.000000000 +0200
@@ -721,10 +721,8 @@ static int __init alsa_card_sb16_init(vo
 			continue;
 		device = platform_device_register_simple(SND_SB16_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		cards++;
 	}
@@ -746,14 +744,10 @@ static int __init alsa_card_sb16_init(vo
 		snd_printk(KERN_ERR "In case, if you have AWE card, try snd-sbawe module\n");
 #endif
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_sb16_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_sb16_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_sb16_exit(void)
Index: mm/sound/isa/sb/sb8.c
===================================================================
--- mm.orig/sound/isa/sb/sb8.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/sb/sb8.c	2006-04-10 23:44:32.000000000 +0200
@@ -264,10 +264,8 @@ static int __init alsa_card_sb8_init(voi
 			continue;
 		device = platform_device_register_simple(SND_SB8_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -275,14 +273,10 @@ static int __init alsa_card_sb8_init(voi
 #ifdef MODULE
 		snd_printk(KERN_ERR "Sound Blaster soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_sb8_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_sb8_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_sb8_exit(void)
Index: mm/sound/isa/sgalaxy.c
===================================================================
--- mm.orig/sound/isa/sgalaxy.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/sgalaxy.c	2006-04-10 23:44:32.000000000 +0200
@@ -366,10 +366,8 @@ static int __init alsa_card_sgalaxy_init
 			continue;
 		device = platform_device_register_simple(SND_SGALAXY_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		devices[i] = device;
 		cards++;
 	}
@@ -377,14 +375,10 @@ static int __init alsa_card_sgalaxy_init
 #ifdef MODULE
 		snd_printk(KERN_ERR "Sound Galaxy soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_sgalaxy_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_sgalaxy_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_sgalaxy_exit(void)
Index: mm/sound/isa/wavefront/wavefront.c
===================================================================
--- mm.orig/sound/isa/wavefront/wavefront.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/wavefront/wavefront.c	2006-04-10 23:44:32.000000000 +0200
@@ -722,10 +722,8 @@ static int __init alsa_card_wavefront_in
 #endif
 		device = platform_device_register_simple(WAVEFRONT_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 		cards++;
 	}
@@ -742,14 +740,10 @@ static int __init alsa_card_wavefront_in
 #ifdef MODULE
 		printk (KERN_ERR "No WaveFront cards found or devices busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_wavefront_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_wavefront_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_wavefront_exit(void)
Index: mm/sound/isa/es18xx.c
===================================================================
--- mm.orig/sound/isa/es18xx.c	2006-04-10 23:44:24.000000000 +0200
+++ mm/sound/isa/es18xx.c	2006-04-10 23:44:32.000000000 +0200
@@ -2392,10 +2392,8 @@ static int __init alsa_card_es18xx_init(
 			continue;
 		device = platform_device_register_simple(ES18XX_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			err = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+	       		continue;
 		platform_devices[i] = device;
 		cards++;
 	}
@@ -2412,14 +2410,10 @@ static int __init alsa_card_es18xx_init(
 #ifdef MODULE
 		snd_printk(KERN_ERR "ESS AudioDrive ES18xx soundcard not found or device busy\n");
 #endif
-		err = -ENODEV;
-		goto errout;
+		snd_es18xx_unregister_all();
+		return -ENODEV;
 	}
 	return 0;
-
- errout:
-	snd_es18xx_unregister_all();
-	return err;
 }
 
 static void __exit alsa_card_es18xx_exit(void)
Index: mm/sound/isa/sscape.c
===================================================================
--- mm.orig/sound/isa/sscape.c	2006-04-10 23:43:38.000000000 +0200
+++ mm/sound/isa/sscape.c	2006-04-10 23:44:32.000000000 +0200
@@ -1427,8 +1427,8 @@ static int __init sscape_manual_probe(vo
 		    dma[i] == SNDRV_AUTO_DMA) {
 			printk(KERN_INFO
 			       "sscape: insufficient parameters, need IO, IRQ, MPU-IRQ and DMA\n");
-			ret = -ENXIO;
-			goto errout;
+			sscape_unregister_all();
+			return -ENXIO;
 		}
 
 		/*
@@ -1436,17 +1436,11 @@ static int __init sscape_manual_probe(vo
 		 */
 		device = platform_device_register_simple(SSCAPE_DRIVER,
 							 i, NULL, 0);
-		if (IS_ERR(device)) {
-			ret = PTR_ERR(device);
-			goto errout;
-		}
+		if (IS_ERR(device))
+			continue;
 		platform_devices[i] = device;
 	}
 	return 0;
-
- errout:
-	sscape_unregister_all();
-	return ret;
 }
 
 static void sscape_exit(void)

Re: [Alsa-devel] Re: [ALSA 1/2] continue on IS_ERR from platform device registration

[Takashi Iwai]

At Tue, 11 Apr 2006 01:10:59 +0200,
Rene Herman wrote:
> 
> Will also post them (and the !enable[i] patch) against 2.6.16.2 for
> -stable. You already acked the !enable[i] and continue-on-is_err patches
> for -stable, the third is the same unregister patch, restoring the old
> fail-to-load behaviour also for -stable.
> 
> Considering that one of the rules for -stable is that fixes also need to 
> be present in upstream trees, could you relay the three patches for 
> -stable yourself?

Yes, but only if you give a valid signed-off-by line at this time ;)


Takashi