Takashi Iwai wrote:

>> then the ID is used as is and everything seems fine. I assume that 
>> either the space check could go or that a passed in ID should be 
>> subjected to it as well? (that function also deletes all !isalnum's from 
>> the shortname such as <underscore> which, again, is fine when passed in 
>> manually).
> 
> The space check is there to retrieve the second word from the
> shortname, because the length of id string is much shorter.  In most
> cases, you likely have names like "Vendor DeviceXXXX".  Then only
> DeviceXXX is extracted.  If it matters, pass a string without space in
> the shortname field.

Okay I guess. No, "FM" is fine by me...

>> +static int __devinit snd_adlib_probe(struct platform_device *device)
> (snip)
>> +	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_card_free(card);
>> +		return -EINVAL;
>> +	}
> 
> -EBUSY would be more suitable (although it's ignored later).

Yes.

>> +	card->private_free = snd_adlib_free;
>> +
>> +	if (snd_opl3_create(card, port[i], port[i] + 2, OPL3_HW_AUTO, 1, &opl3) < 0) {
>> +		snd_printk(KERN_ERR DRV_NAME ": could not create OPL\n");
>> +		snd_card_free(card);
>> +		return -EINVAL;
>> +	}
> 
> Better to keep the original error value?

This is a problem unfortunately. I haven't yet regenerated the patch to 
continue on IS_ERR that I sent a few days ago against -mm as you asked 
due to the issue of driver_probe_device() specifically ignoring -ENODEV 
and -ENXIO. I've looked, and both of these are values ALSA internals can 
and do return so if we just propagate the value up, we might propagate 
an -ENODEV or -ENXIO; platform_driver_register_simple() would then 
return success and let the driver load.

 From the perspective of these ISA drivers, deleting the -ENODEV | 
-ENXIO check from driver_probe_device() would be best. And I guess 
making it dependent on hotpluggability of a bus is best all around, but 
this is waiting for comment from greg.

> Ditto.  Maybe better to have a single error exit with snd_card_free()?

Attached is a version that simply jumps to a return -EINVAL, which would 
also be the easiest way to fixup the others, at least for -stable. If 
you disagree though, in the next message I'll sent a version that does 
register the error, but for now just turns them into -EINVAL before 
returning.

Given that the specific error is ignored anyway I prefer this first one 
myself but both are fine by me. I'll use the method you prefer with the 
others as well.

Rene