[PATCH 5/8] aacraid: error return checking

[PATCH 5/8] aacraid: error return checking

[Mark Haverkamp]

Received from Mark Salyzyn from Adaptec

This patch adds some additional error return checking and error return
value propagation during initialization. Also, the deprecation of
pci_module_init with pci_register_driver along with the change in return
values.

Applies to the scsi-misc-2.6 git tree.

Signed-off-by: Mark Haverkamp <markh@osdl.org>



Index: scsi-misc-aac-2/drivers/scsi/aacraid/linit.c
===================================================================
--- scsi-misc-aac-2.orig/drivers/scsi/aacraid/linit.c	2005-09-08 11:32:27.000000000 -0700
+++ scsi-misc-aac-2/drivers/scsi/aacraid/linit.c	2005-09-08 11:32:30.000000000 -0700
@@ -748,7 +748,8 @@
 		unique_id++;
 	}
 
-	if (pci_enable_device(pdev))
+	error = pci_enable_device(pdev);
+	if (error)
 		goto out;
 
 	if (pci_set_dma_mask(pdev, 0xFFFFFFFFULL) || 
@@ -801,7 +802,9 @@
 			goto out_free_fibs;
 
 	aac->maximum_num_channels = aac_drivers[index].channels;
-	aac_get_adapter_info(aac);
+	error = aac_get_adapter_info(aac);
+	if (error < 0)
+		goto out_deinit;
 
 	/*
  	 * Lets override negotiations and drop the maximum SG limit to 34
@@ -929,9 +932,14 @@
 	printk(KERN_INFO "Adaptec %s driver (%s)\n",
 	  AAC_DRIVERNAME, aac_driver_version);
 
-	error = pci_module_init(&aac_pci_driver);
-	if (error)
+	error = pci_register_driver(&aac_pci_driver);
+	if (error < 0 || list_empty(&aac_devices)) {
+		if (error >= 0) {
+			pci_unregister_driver(&aac_pci_driver);
+			error = -ENODEV;
+		}
 		return error;
+	}
 
 	aac_cfg_major = register_chrdev( 0, "aac", &aac_cfg_fops);
 	if (aac_cfg_major < 0) {

-- 
Mark Haverkamp <markh@osdl.org>

Re: [PATCH 5/8] aacraid: error return checking

[Christoph Hellwig]

> @@ -929,9 +932,14 @@
>  	printk(KERN_INFO "Adaptec %s driver (%s)\n",
>  	  AAC_DRIVERNAME, aac_driver_version);
>  
> -	error = pci_module_init(&aac_pci_driver);
> -	if (error)
> +	error = pci_register_driver(&aac_pci_driver);
> +	if (error < 0 || list_empty(&aac_devices)) {
> +		if (error >= 0) {
> +			pci_unregister_driver(&aac_pci_driver);
> +			error = -ENODEV;
> +		}
>  		return error;

The list_empty check is wrong.  I driver should stay loaded even if
no devices have been found.

RE: [PATCH 5/8] aacraid: error return checking

[Salyzyn, Mark]

Christoph Hellwig [mailto:hch@infradead.org] writes:
>> @@ -929,9 +932,14 @@
>>  	printk(KERN_INFO "Adaptec %s driver (%s)\n",
>>  	  AAC_DRIVERNAME, aac_driver_version);
>>  
>> -	error = pci_module_init(&aac_pci_driver);
>> -	if (error)
>> +	error = pci_register_driver(&aac_pci_driver);
>> +	if (error < 0 || list_empty(&aac_devices)) {
>> +		if (error >= 0) {
>> +			pci_unregister_driver(&aac_pci_driver);
>> +			error = -ENODEV;
>> +		}
>>  		return error;
>The list_empty check is wrong.  I driver should stay loaded even if no
devices have been found.

We get complains about a driver module loaded with no associated
hardware. There are system configurations that probe for installed
hardware by loading modules, they expect only the functional driver
modules to load. What purpose is there in a driver module that is loaded
with no hardware?

Sincerely -- Mark Salyzyn

Re: [PATCH 5/8] aacraid: error return checking

[Christoph Hellwig]

On Mon, Sep 12, 2005 at 07:19:56AM -0400, Salyzyn, Mark wrote:
> We get complains about a driver module loaded with no associated
> hardware. There are system configurations that probe for installed
> hardware by loading modules, they expect only the functional driver
> modules to load. What purpose is there in a driver module that is loaded
> with no hardware?

Allow for hot-plug devices to be hanled without further user-space invention.
Your above probing method will have the same effect on every modern driver.