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.
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;
}
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
