[ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Rene Herman]

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

Hi Takashi.

And finally, unregister on probe failure for sound/drivers,
sound/arm/sa11xx-uda1341.c and sound/ppc/powermac.c.

   sound/arm/sa11xx-uda1341.c    |   14 +++++++++-----
   sound/drivers/dummy.c         |    4 ++++
   sound/drivers/mpu401/mpu401.c |    4 ++++
   sound/drivers/mtpav.c         |   14 +++++++++-----
   sound/drivers/serial-u16550.c |    4 ++++
   sound/drivers/virmidi.c       |    4 ++++
   sound/ppc/powermac.c          |   14 +++++++++-----
   7 files changed, 43 insertions(+), 15 deletions(-)

Signed-off-by: Rene Herman <rene.herman@keyaccess.nl>


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

Index: local/sound/arm/sa11xx-uda1341.c
===================================================================
--- local.orig/sound/arm/sa11xx-uda1341.c	2006-03-20 06:53:29.000000000 +0100
+++ local/sound/arm/sa11xx-uda1341.c	2006-04-13 03:14:49.000000000 +0200
@@ -984,11 +984,15 @@ static int __init sa11xx_uda1341_init(vo
 	if ((err = platform_driver_register(&sa11xx_uda1341_driver)) < 0)
 		return err;
 	device = platform_device_register_simple(SA11XX_UDA1341_DRIVER, -1, NULL, 0);
-	if (IS_ERR(device)) {
-		platform_driver_unregister(&sa11xx_uda1341_driver);
-		return PTR_ERR(device);
-	}
-	return 0;
+	if (!IS_ERR(device)) {
+		if (platform_get_drvdata(device))
+			return 0;
+		platform_device_unregister(device);
+		err = -ENODEV
+	} else
+		err = PTR_ERR(device);
+	platform_driver_unregister(&sa11xx_uda1341_driver);
+	return err;
 }
 
 static void __exit sa11xx_uda1341_exit(void)
Index: local/sound/drivers/dummy.c
===================================================================
--- local.orig/sound/drivers/dummy.c	2006-04-13 03:11:35.000000000 +0200
+++ local/sound/drivers/dummy.c	2006-04-13 03:14:49.000000000 +0200
@@ -677,6 +677,10 @@ static int __init alsa_card_dummy_init(v
 							 i, NULL, 0);
 		if (IS_ERR(device))
 			continue;
+		if (!platform_get_drvdata(device)) {
+			platform_device_unregister(device);
+			continue;
+		}
 		devices[i] = device;
 		cards++;
 	}
Index: local/sound/drivers/mpu401/mpu401.c
===================================================================
--- local.orig/sound/drivers/mpu401/mpu401.c	2006-04-13 03:11:35.000000000 +0200
+++ local/sound/drivers/mpu401/mpu401.c	2006-04-13 03:14:49.000000000 +0200
@@ -252,6 +252,10 @@ static int __init alsa_card_mpu401_init(
 							 i, NULL, 0);
 		if (IS_ERR(device))
 			continue;
+		if (!platform_get_drvdata(device)) {
+			platform_device_unregister(device);
+			continue;
+		}
 		platform_devices[i] = device;
 		devices++;
 	}
Index: local/sound/drivers/mtpav.c
===================================================================
--- local.orig/sound/drivers/mtpav.c	2006-03-20 06:53:29.000000000 +0100
+++ local/sound/drivers/mtpav.c	2006-04-13 03:14:49.000000000 +0200
@@ -770,11 +770,15 @@ static int __init alsa_card_mtpav_init(v
 		return err;
 
 	device = platform_device_register_simple(SND_MTPAV_DRIVER, -1, NULL, 0);
-	if (IS_ERR(device)) {
-		platform_driver_unregister(&snd_mtpav_driver);
-		return PTR_ERR(device);
-	}
-	return 0;
+	if (!IS_ERR(device)) {
+		if (platform_get_drvdata(device))
+			return 0;
+		platform_device_unregister(device);
+		err = -ENODEV;
+	} else
+		err = PTR_ERR(device);
+	platform_driver_unregister(&snd_mtpav_driver);
+	return err;
 }
 
 static void __exit alsa_card_mtpav_exit(void)
Index: local/sound/drivers/serial-u16550.c
===================================================================
--- local.orig/sound/drivers/serial-u16550.c	2006-04-13 03:11:35.000000000 +0200
+++ local/sound/drivers/serial-u16550.c	2006-04-13 03:14:49.000000000 +0200
@@ -997,6 +997,10 @@ static int __init alsa_card_serial_init(
 							 i, NULL, 0);
 		if (IS_ERR(device))
 			continue;
+		if (!platform_get_drvdata(device)) {
+			platform_device_unregister(device);
+			continue;
+		}
 		devices[i] = device;
 		cards++;
 	}
Index: local/sound/drivers/virmidi.c
===================================================================
--- local.orig/sound/drivers/virmidi.c	2006-04-13 03:11:35.000000000 +0200
+++ local/sound/drivers/virmidi.c	2006-04-13 03:14:49.000000000 +0200
@@ -171,6 +171,10 @@ static int __init alsa_card_virmidi_init
 							 i, NULL, 0);
 		if (IS_ERR(device))
 			continue;
+		if (!platform_get_drvdata(device)) {
+			platform_device_unregister(device);
+			continue;
+		}
 		devices[i] = device;
 		cards++;
 	}
Index: local/sound/ppc/powermac.c
===================================================================
--- local.orig/sound/ppc/powermac.c	2006-03-20 06:53:29.000000000 +0100
+++ local/sound/ppc/powermac.c	2006-04-13 03:14:49.000000000 +0200
@@ -188,11 +188,15 @@ static int __init alsa_card_pmac_init(vo
 	if ((err = platform_driver_register(&snd_pmac_driver)) < 0)
 		return err;
 	device = platform_device_register_simple(SND_PMAC_DRIVER, -1, NULL, 0);
-	if (IS_ERR(device)) {
-		platform_driver_unregister(&snd_pmac_driver);
-		return PTR_ERR(device);
-	}
-	return 0;
+	if (!IS_ERR(device)) {
+		if (platform_get_drvdata(device))
+			return 0;
+		platform_device_unregister(device);
+		err = -ENODEV;
+	} else
+		err = PTR_ERR(device);
+	platform_driver_unregister(&snd_pmac_driver);
+	return err;
 
 }
 

Re: [ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Ingo Oeser]

Hi Rene,

On Thursday, 13. April 2006 03:46, you wrote:
> And finally, unregister on probe failure for sound/drivers,
> sound/arm/sa11xx-uda1341.c and sound/ppc/powermac.c.
> 
>    sound/arm/sa11xx-uda1341.c    |   14 +++++++++-----
>    sound/drivers/dummy.c         |    4 ++++
>    sound/drivers/mpu401/mpu401.c |    4 ++++
>    sound/drivers/mtpav.c         |   14 +++++++++-----
>    sound/drivers/serial-u16550.c |    4 ++++
>    sound/drivers/virmidi.c       |    4 ++++
>    sound/ppc/powermac.c          |   14 +++++++++-----
>    7 files changed, 43 insertions(+), 15 deletions(-)

This is inserting lots of duplicate and control heavy code.
It looks like the same pattern and is using just platform_xxx funxtions.

Any counter-examples with a different pattern, which you converted in
a different manner?

Wouldn't it be more useful to do these checks in 
platform_register_simple() and return the proper error value there?

Or just create a small helper, if this have to be done seperate.

Greg, what do you think here?


Regards

Ingo Oeser

Re: [ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Russell King]

On Thu, Apr 13, 2006 at 11:26:34AM +0200, Ingo Oeser wrote:
> Hi Rene,
> Wouldn't it be more useful to do these checks in 
> platform_register_simple() and return the proper error value there?

Firstly, platform_device_register_simple() is going away.

Secondly, it's inappropriate to force this weirdo ALSA specific behaviour
on the rest of the platform device users.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Rene Herman]

Ingo Oeser wrote:

Why did you remove alsa-devel from the CC?

> This is inserting lots of duplicate and control heavy code. It looks 
> like the same pattern and is using just platform_xxx funxtions.
> 
> Any counter-examples with a different pattern, which you converted in
> a different manner?

Not lots really... for all of them it's basically:

if (!platform_get_drvdata(device)) {
	platform_device_unregister(device);
	continue;
}

in the driver's main init loop. In this batch there were three drivers 
that both did not support more than one device and passed the error on 
up meaning it looks a bit a bit more clumsy than that but when moving 
these drivers from the platform_device_register_simple() interface this 
code will be revisited again; making them support more devices can also 
happen longer term.

> Wouldn't it be more useful to do these checks in 
> platform_register_simple() and return the proper error value there?

That interface is going away. I checked where ALSA used them due to that 
in fact (the same patches for sound/isa are already in ALSA and have 
been submitted for -stable as well).

Yes, it would be better if the driver model supplied this functionality 
itself. The problem is that an error return from the platform probe() 
method is not being honoured/passed up by the driver model so that we 
don't get a chance to say "no, the hardware's not here, go away!".

Not honouring/passing up probe() method error returns, not even -ENODEV, 
makes some sense for discoverable busses such as PCI where you at least 
have a driver independent bus_id sitting in /sys/devices/pci* that you 
can later echo into /sys/bus/pci/drivers/*/bind to make the driver bind 
to a device, but not much sense for the platform bus. Platform devices 
only "exist" (in /sys/devices/platform) due to the driver creating them 
itself and keeping them after failing a probe means that directory 
becomes an enumeration of the drivers we loaded, rather than a view of 
what's present in the system.

The driver model crowd did not seem exceedingly interested in the 
problem though:

http://marc.theaimsgroup.com/?l=linux-kernel&m=114417829014332&w=2

It _is_ ofcourse an option to not care about /sys/devices/platform, and 
ALSA could do that as well longer term. This was discussed:

http://marc.theaimsgroup.com/?l=linux-kernel&m=114442720631596&w=2

(marc seems to be not so good at keeping threads intact. reply at: 
http://marc.theaimsgroup.com/?l=linux-kernel&m=114469016131522&w=2)

and for now, we'll keep the old behaviour of not loading on probe 
failure, using drvdata() as a private success flag set from probe(). 
Longer term, we can revisit this.

Most importantly though, you replied to the one submitted for -stable. 
For -stable, this is certainly the way to go. ALSA drivers always failed 
to load on probe() failure before they were even using the platform 
driver interface (which was new to 2.6.16) and things like the alsaconf 
script rely on this. For -stable, it's a simple bugfix therefore.

> Or just create a small helper, if this have to be done seperate.

That would be going way overboard for the three lines as stated in the 
beginning. Certainly when they might/will go again in the future...

Rene.

Re: [ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Russell King]

On Thu, Apr 13, 2006 at 04:05:33PM +0200, Rene Herman wrote:
> Not honouring/passing up probe() method error returns, not even -ENODEV, 
> makes some sense for discoverable busses such as PCI where you at least 
> have a driver independent bus_id sitting in /sys/devices/pci* that you 
> can later echo into /sys/bus/pci/drivers/*/bind to make the driver bind 
> to a device, but not much sense for the platform bus. Platform devices 
> only "exist" (in /sys/devices/platform) due to the driver creating them 
> itself and keeping them after failing a probe means that directory 
> becomes an enumeration of the drivers we loaded, rather than a view of 
> what's present in the system.

Incorrect.  In some circumstances, they may be created by architecture
support code, and might be created and destroyed dynamically by
architecture support code.

> The driver model crowd did not seem exceedingly interested in the 
> problem though:
> 
> http://marc.theaimsgroup.com/?l=linux-kernel&m=114417829014332&w=2

Incorrect summary.  The ALSA use model of the driver model doesn't fit
with the driver model use model.  It's not that we're not interested
in it - it's that it's perverted to the way driver model folk intend
the subsystem to work, and the way that platform devices are used on
some architectures.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Rene Herman]

Russell King wrote:

> On Thu, Apr 13, 2006 at 04:05:33PM +0200, Rene Herman wrote:

>> Not honouring/passing up probe() method error returns, not even -ENODEV, 
>> makes some sense for discoverable busses such as PCI where you at least 
>> have a driver independent bus_id sitting in /sys/devices/pci* that you 
>> can later echo into /sys/bus/pci/drivers/*/bind to make the driver bind 
>> to a device, but not much sense for the platform bus. Platform devices 
>> only "exist" (in /sys/devices/platform) due to the driver creating them 
>> itself and keeping them after failing a probe means that directory 
>> becomes an enumeration of the drivers we loaded, rather than a view of 
>> what's present in the system.
> 
> Incorrect.  In some circumstances, they may be created by architecture
> support code, and might be created and destroyed dynamically by
> architecture support code.

Okay, thanks, that's relevant information. Please explain though what's 
incorrect about the fact that for these ISA devices on the plain old PC, 
with nothing other than the driver available to probe for them, just 
keeping them registered after failing a probe turns 
/sys/devices/platform into a view of "what drivers did we load".

>> The driver model crowd did not seem exceedingly interested in the 
>> problem though:
>>
>> http://marc.theaimsgroup.com/?l=linux-kernel&m=114417829014332&w=2
> 
> Incorrect summary.  The ALSA use model of the driver model doesn't fit
> with the driver model use model.  It's not that we're not interested
> in it - it's that it's perverted to the way driver model folk intend
> the subsystem to work, and the way that platform devices are used on
> some architectures.

And I take it that interest is reflected in getting a grand total of 0 
comments from anyone on my own feeble attempts to suggest things in that 
thread such as the settable flag that would make the driver model pass 
up the error return from probe when set, or having an additional 
.discover method, or ..

M'kay. I believe there's one clean way out of this. We could add an "isa 
bus", where the _user_ would first need to setup the hardware from 
userspace by echoing values into sysfs. Say, something like:

echo -n foo		>/sys/devices/isa0/new
echo -n "io 0x220"	>/sys/devices/isa0/foo/resources
echo -n "irq 5"		>/sys/devices/isa0/foo/resources
echo -n "dma 1"		>/sys/devices/isa0/foo/resources

and so on. The type of resources would be modelled after ISA-PnP (and to 
make them more equal, you could do multiple device id's per bus id, card 
id's, in PNP lingo, but this principle at least)

The driver would them request id "foo" as an agreed upon ID (snd-sb8 
would request "sb8", say) or we could pass in the id as a module parameter.

I actually think this would be Great. Comments? Pitchforks?

 From the driver's standpoint, there would not be a difference with 
ISA-PnP anymore other than ISA-PnP also providing for the possibilty to 
change them -- something which ALSA also uses, but which it should 
probably not; has annoyed me for some time. We could then in fact also 
integrate this into ISA-PnP itself, using PnP-like device IDs and all, 
so that from the driver's standpoint, it _is_ always speaking to an 
ISA-PnP device.

Rene.

Re: [ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Russell King]

On Thu, Apr 13, 2006 at 06:17:49PM +0200, Rene Herman wrote:
> Russell King wrote:
> 
> >On Thu, Apr 13, 2006 at 04:05:33PM +0200, Rene Herman wrote:
> 
> >>Not honouring/passing up probe() method error returns, not even -ENODEV, 
> >>makes some sense for discoverable busses such as PCI where you at least 
> >>have a driver independent bus_id sitting in /sys/devices/pci* that you 
> >>can later echo into /sys/bus/pci/drivers/*/bind to make the driver bind 
> >>to a device, but not much sense for the platform bus. Platform devices 
> >>only "exist" (in /sys/devices/platform) due to the driver creating them 
> >>itself and keeping them after failing a probe means that directory 
> >>becomes an enumeration of the drivers we loaded, rather than a view of 
> >>what's present in the system.
> >
> >Incorrect.  In some circumstances, they may be created by architecture
> >support code, and might be created and destroyed dynamically by
> >architecture support code.
> 
> Okay, thanks, that's relevant information. Please explain though what's 
> incorrect about the fact that for these ISA devices on the plain old PC, 
> with nothing other than the driver available to probe for them, just 
> keeping them registered after failing a probe turns 
> /sys/devices/platform into a view of "what drivers did we load".

If a driver for an ISA device only wants to register a device and driver
if the hardware exists, it needs to handle behaviour itself and not force
such behaviour on the upper layers (which is what you're arguing for.)

> >>The driver model crowd did not seem exceedingly interested in the 
> >>problem though:
> >>
> >>http://marc.theaimsgroup.com/?l=linux-kernel&m=114417829014332&w=2
> >
> >Incorrect summary.  The ALSA use model of the driver model doesn't fit
> >with the driver model use model.  It's not that we're not interested
> >in it - it's that it's perverted to the way driver model folk intend
> >the subsystem to work, and the way that platform devices are used on
> >some architectures.
> 
> And I take it that interest is reflected in getting a grand total of 0 
> comments from anyone on my own feeble attempts to suggest things in that 
> thread such as the settable flag that would make the driver model pass 
> up the error return from probe when set, or having an additional 
> .discover method, or ..

I never commented on it because I'm not specifically a driver model
person - I just happen to be one of the major users of platform devices,
though I have put forward some changes to the platform driver model to
facilitate getting rid of quite frankly some extremely poor and buggy
driver code (and fixed up that crap code in the process.)

Greg is the maintainer of the driver model, and it's Greg who needs to
comment on changes to the way it works.

> M'kay. I believe there's one clean way out of this. We could add an "isa 
> bus", where the _user_ would first need to setup the hardware from 
> userspace by echoing values into sysfs. Say, something like:

Maybe this is the best solution for ISA devices - they do appear to
have differing semantics at the probe level from platform devices.
Maybe this "discovery" should be part of the bus matching method, prior
to the driver probe method being called?  With an ISA bus type, you can
certainly arrange for that to happen without changing existing driver
model behaviour.

In addition, you can check whether the driver was bound by checking
whether dev->driver is non-NULL.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Rene Herman]

Russell King wrote:

> On Thu, Apr 13, 2006 at 06:17:49PM +0200, Rene Herman wrote:

>> Okay, thanks, that's relevant information. Please explain though
>> what's incorrect about the fact that for these ISA devices on the
>> plain old PC, with nothing other than the driver available to probe
>> for them, just keeping them registered after failing a probe turns
>> /sys/devices/platform into a view of "what drivers did we load".
> 
> If a driver for an ISA device only wants to register a device and
> driver if the hardware exists, it needs to handle behaviour itself
> and not force such behaviour on the upper layers (which is what
> you're arguing for.)

Nono, please note I'm arguing for nothing of the sort. The original 
patch to bus_add_device() to pass up the probe() return was submitted 
with just a "if I do this, things work as I expect. is it correct?" 
question attached. Given that everything uses that same code, it wasn't 
correct. What I am arguing for is that it would be good if the driver 
model provided me the _option_ to fail a registration if the driver 
tells it there are no devices. ie, the flag that I could set that would 
make the driver model interpret an ENODEV from probe() to really mean NODEV.

The current work-around of using drvdata() as a success flag is exactly 
what you say -- ALSA doing it all by itself. This thread specifically 
only started due to Ingo Oeser suggesting that work-around would go into 
platform_device_register_simple()...

>> M'kay. I believe there's one clean way out of this. We could add an "isa 
>> bus", where the _user_ would first need to setup the hardware from 
>> userspace by echoing values into sysfs. Say, something like:
> 
> Maybe this is the best solution for ISA devices - they do appear to
> have differing semantics at the probe level from platform devices.
> Maybe this "discovery" should be part of the bus matching method, prior
> to the driver probe method being called?  With an ISA bus type, you can
> certainly arrange for that to happen without changing existing driver
> model behaviour.

I can try and see if I can come up with something sensible I guess. Will 
need time though...

Takashi: anyways, these patches are good to go. Already saw the ISA 
driver ones present in 1.0.11-rc5. I by the way do not see them in the 
ALSA CVS at http://cvs.sourceforge.net/viewcvs.py/alsa/. How's that?

Rene.

Re: [ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Greg KH]

On Thu, Apr 13, 2006 at 06:05:50PM +0100, Russell King wrote:
> On Thu, Apr 13, 2006 at 06:17:49PM +0200, Rene Herman wrote:
> > Russell King wrote:
> > 
> > >On Thu, Apr 13, 2006 at 04:05:33PM +0200, Rene Herman wrote:
> > 
> > >>Not honouring/passing up probe() method error returns, not even -ENODEV, 
> > >>makes some sense for discoverable busses such as PCI where you at least 
> > >>have a driver independent bus_id sitting in /sys/devices/pci* that you 
> > >>can later echo into /sys/bus/pci/drivers/*/bind to make the driver bind 
> > >>to a device, but not much sense for the platform bus. Platform devices 
> > >>only "exist" (in /sys/devices/platform) due to the driver creating them 
> > >>itself and keeping them after failing a probe means that directory 
> > >>becomes an enumeration of the drivers we loaded, rather than a view of 
> > >>what's present in the system.
> > >
> > >Incorrect.  In some circumstances, they may be created by architecture
> > >support code, and might be created and destroyed dynamically by
> > >architecture support code.
> > 
> > Okay, thanks, that's relevant information. Please explain though what's 
> > incorrect about the fact that for these ISA devices on the plain old PC, 
> > with nothing other than the driver available to probe for them, just 
> > keeping them registered after failing a probe turns 
> > /sys/devices/platform into a view of "what drivers did we load".
> 
> If a driver for an ISA device only wants to register a device and driver
> if the hardware exists, it needs to handle behaviour itself and not force
> such behaviour on the upper layers (which is what you're arguing for.)
> 
> > >>The driver model crowd did not seem exceedingly interested in the 
> > >>problem though:
> > >>
> > >>http://marc.theaimsgroup.com/?l=linux-kernel&m=114417829014332&w=2
> > >
> > >Incorrect summary.  The ALSA use model of the driver model doesn't fit
> > >with the driver model use model.  It's not that we're not interested
> > >in it - it's that it's perverted to the way driver model folk intend
> > >the subsystem to work, and the way that platform devices are used on
> > >some architectures.
> > 
> > And I take it that interest is reflected in getting a grand total of 0 
> > comments from anyone on my own feeble attempts to suggest things in that 
> > thread such as the settable flag that would make the driver model pass 
> > up the error return from probe when set, or having an additional 
> > .discover method, or ..
> 
> I never commented on it because I'm not specifically a driver model
> person - I just happen to be one of the major users of platform devices,
> though I have put forward some changes to the platform driver model to
> facilitate getting rid of quite frankly some extremely poor and buggy
> driver code (and fixed up that crap code in the process.)
> 
> Greg is the maintainer of the driver model, and it's Greg who needs to
> comment on changes to the way it works.

Hm, I didn't see any patches sent to comment on :)

Anyway, no, no wierd flags to make the core pass stuff up on probe,
that's just a mess.

I still really don't understand why these ALSA drivers are so unlike any
of the zillion other drivers we have in the kernel that work just fine
today.  But to be fair, I've only been barely paying attention to this
thread.  Once the bad driver core patch was dropped from my tree, I've
been off doing other things...

> > M'kay. I believe there's one clean way out of this. We could add an "isa 
> > bus", where the _user_ would first need to setup the hardware from 
> > userspace by echoing values into sysfs. Say, something like:
> 
> Maybe this is the best solution for ISA devices - they do appear to
> have differing semantics at the probe level from platform devices.
> Maybe this "discovery" should be part of the bus matching method, prior
> to the driver probe method being called?  With an ISA bus type, you can
> certainly arrange for that to happen without changing existing driver
> model behaviour.

I know that Adam Belay had some work for ISA devices along these lines.
You might want to ask him, as he understands the issues involved here.

thanks,

greg k-h

Re: [ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Rene Herman]

Greg KH wrote:

> I still really don't understand why these ALSA drivers are so unlike
> any of the zillion other drivers we have in the kernel that work just
> fine today.

It's nothing to do with ALSA, only with ISA -- ALSA is just one of the 
few remaining serious ISA users. Have said this a number of times now, 
so apologies to anyone who _is_ following this. I promise it's going to 
be the last time. The issue:

Historically, ALSA ISA drivers failed to load upon not finding their 
hardware at load time, same as most ISA drivers. When 2.6.16 switched 
them to use of the platform driver interface this behaviour was 
inadvertently changed due to the platform driver interface not passing 
up the probe() return. Things such as the "alsaconf" configuration 
script actualy rely on the non-load behaviour and current submissions to 
-stable simply use drvdata() being !NULL as a private success flag to 
restore that behaviour. This is okay.

Longer term (than -stable) it could ofcourse be better to follow the 
model from saner busses such as PCI more closely -- we could just load 
whether or not a device was bound to. The difference though is that a 
PCI device has a life all by itself by virtue of its _bus_ knowing that 
it's present (it has an entry in /sys/devices/pci*, without any device 
specific driver loaded) while these old ISA devices only exist in the 
driver model by virtue of a driver creating them as a platform device 
because it might want to drive them. If we keep them registered even 
after failing a probe, then /sys/devices/platform turns into an 
enumeration of the drivers we loaded, not a view of what's present in 
the system.

(and there is no point in keeping the driver loaded when we don't keep 
the devices registered as well -- you couldn't do anything with that 
driver, it would just be sitting there taking up memory)

Problem therefore: /sys/devices pollution. Note that ALSA supports some 
20 (non-pnp-only) ISA drivers and imagine loading them all, with all of 
them creating one or more devices in /sys/devices/platform, with none of 
the actual hardware present. I consider this not nice, not from the view 
of ALSA, but because I've been told that one of the reasons for /sys was 
this nice view of "what's in this system" that /sys/devices would provide.

The answer needed -- if you _do_ think it's okay to just have those 
devices present in /sys/devices/platform without the hardware existing, 
then I do not have a problem. I'll just suggest to keep them registered 
and be done with it (*).

If on the other hand you agree it's not nice, I'll try and look into 
what a seperate isa_bus could provide.

Rene.

(*) Note by the way I do not necesarily qualify as an ALSA person. I 
only ran into this when I submitted a new ALSA ISA driver for 2.6.16+. 
Takashi Iwai is being CCed on all these messages though, and he hasn't 
yet protested anything I've been saying, so I assume he's okay with my 
worries at least...

Re: [ALSA STABLE 3/3] a few more -- unregister platform device again if probe was unsuccessful

[Takashi Iwai]

At Fri, 14 Apr 2006 01:12:17 +0200,
Rene Herman wrote:
> 
> (*) Note by the way I do not necesarily qualify as an ALSA person. I 
> only ran into this when I submitted a new ALSA ISA driver for 2.6.16+. 
> Takashi Iwai is being CCed on all these messages though, and he hasn't 
> yet protested anything I've been saying, so I assume he's okay with my 
> worries at least...

Acked ;)  I already took Rene's latest patches to ALSA tree.

I know the current behavior is somewhat out of standard, but would
like to keep it _right now_ for compatibility and usability reasons.

OTOH, it would make sense to keep the driver struct if the ALSA module
parameters are writable after loading.  Then the user have another
chance to retry the probe with different parameters.  Anyway, it's not
for 2.6.17.


Takashi