[PATCH 0/7] CCISS cleanups

[PATCH 0/7] CCISS cleanups

[Bjorn Helgaas]

This is a series of minor cleanups to the cciss (HP Smart Array) driver:

  1  disable device before returning failure
  2  claim all resources the device decodes, not just I/O ports
  3  print more useful identification when driver claims device
  4  remove intermediate #define for ARRAY_SIZE
  5  fix spelling errors
  6  unparenthesize "return" statements
  7  Lindent (warning, huge diff, but changes whitespace only)
  
They're in order of usefulness.

[PATCH 1/7] CCISS: disable device when returning failure

[Bjorn Helgaas]

If something fails after we call pci_enable_device(), we should call
pci_disable_device() before returning the failure.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: rc5-mm3/drivers/block/cciss.c
===================================================================
--- rc5-mm3.orig/drivers/block/cciss.c	2006-06-14 14:29:40.000000000 -0600
+++ rc5-mm3/drivers/block/cciss.c	2006-06-14 14:36:03.000000000 -0600
@@ -2744,7 +2744,7 @@
 	__u64 cfg_offset;
 	__u32 cfg_base_addr;
 	__u64 cfg_base_addr_index;
-	int i;
+	int i, err;
 
 	/* check to see if controller has been disabled */
 	/* BEFORE trying to enable it */
@@ -2752,13 +2752,14 @@
 	if(!(command & 0x02))
 	{
 		printk(KERN_WARNING "cciss: controller appears to be disabled\n");
-		return(-1);
+		return -ENODEV;
 	}
 
-	if (pci_enable_device(pdev))
+	err = pci_enable_device(pdev);
+	if (err)
 	{
 		printk(KERN_ERR "cciss: Unable to Enable PCI device\n");
-		return( -1);
+		return err;
 	}
 
 	subsystem_vendor_id = pdev->subsystem_vendor;
@@ -2824,7 +2825,8 @@
 	}
 	if (scratchpad != CCISS_FIRMWARE_READY) {
 		printk(KERN_WARNING "cciss: Board not ready.  Timed out.\n");
-		return -1;
+		err = -ENODEV;
+		goto err_out_disable_pdev;
 	}
 
 	/* get the address index number */
@@ -2841,7 +2843,8 @@
 	if (cfg_base_addr_index == -1) {
 		printk(KERN_WARNING "cciss: Cannot find cfg_base_addr_index\n");
 		release_io_mem(c);
-		return -1;
+		err = -ENODEV;
+		goto err_out_disable_pdev;
 	}
 
 	cfg_offset = readl(c->vaddr + SA5_CTMEM_OFFSET);
@@ -2868,7 +2871,8 @@
 		printk(KERN_WARNING "cciss: Sorry, I don't know how"
 			" to access the Smart Array controller %08lx\n", 
 				(unsigned long)board_id);
-		return -1;
+		err = -ENODEV;
+		goto err_out_disable_pdev;
 	}
 	if (  (readb(&c->cfgtable->Signature[0]) != 'C') ||
 	      (readb(&c->cfgtable->Signature[1]) != 'I') ||
@@ -2876,7 +2880,8 @@
 	      (readb(&c->cfgtable->Signature[3]) != 'S') )
 	{
 		printk("Does not appear to be a valid CISS config table\n");
-		return -1;
+		err = -ENODEV;
+		goto err_out_disable_pdev;
 	}
 
 #ifdef CONFIG_X86
@@ -2920,10 +2925,14 @@
 	{
 		printk(KERN_WARNING "cciss: unable to get board into"
 					" simple mode\n");
-		return -1;
+		err = -ENODEV;
+		goto err_out_disable_pdev;
 	}
 	return 0;
 
+err_out_disable_pdev:
+	pci_disable_device(pdev);
+	return err;
 }
 
 /* 

Re: [PATCH 1/7] CCISS: disable device when returning failure

[Jeff Garzik]

Bjorn Helgaas wrote:
> If something fails after we call pci_enable_device(), we should call
> pci_disable_device() before returning the failure.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

ACK patches 1-2

RE: [PATCH 1/7] CCISS: disable device when returning failure

[Miller, Mike (OS Dev)]

 

> -----Original Message-----
> From: Helgaas, Bjorn 
> Sent: Wednesday, June 14, 2006 6:09 PM
> To: Miller, Mike (OS Dev)
> Cc: ISS StorageDev; linux-kernel@vger.kernel.org; Andrew Morton
> Subject: [PATCH 1/7] CCISS: disable device when returning failure
> 
> If something fails after we call pci_enable_device(), we should call
> pci_disable_device() before returning the failure.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Acked-by: Mike Miller <mike.miller@hp.com>

> 
> Index: rc5-mm3/drivers/block/cciss.c
> ===================================================================
> --- rc5-mm3.orig/drivers/block/cciss.c	2006-06-14 
> 14:29:40.000000000 -0600
> +++ rc5-mm3/drivers/block/cciss.c	2006-06-14 
> 14:36:03.000000000 -0600
> @@ -2744,7 +2744,7 @@
>  	__u64 cfg_offset;
>  	__u32 cfg_base_addr;
>  	__u64 cfg_base_addr_index;
> -	int i;
> +	int i, err;
>  
>  	/* check to see if controller has been disabled */
>  	/* BEFORE trying to enable it */
> @@ -2752,13 +2752,14 @@
>  	if(!(command & 0x02))
>  	{
>  		printk(KERN_WARNING "cciss: controller appears 
> to be disabled\n");
> -		return(-1);
> +		return -ENODEV;
>  	}
>  
> -	if (pci_enable_device(pdev))
> +	err = pci_enable_device(pdev);
> +	if (err)
>  	{
>  		printk(KERN_ERR "cciss: Unable to Enable PCI device\n");
> -		return( -1);
> +		return err;
>  	}
>  
>  	subsystem_vendor_id = pdev->subsystem_vendor; @@ 
> -2824,7 +2825,8 @@
>  	}
>  	if (scratchpad != CCISS_FIRMWARE_READY) {
>  		printk(KERN_WARNING "cciss: Board not ready.  
> Timed out.\n");
> -		return -1;
> +		err = -ENODEV;
> +		goto err_out_disable_pdev;
>  	}
>  
>  	/* get the address index number */
> @@ -2841,7 +2843,8 @@
>  	if (cfg_base_addr_index == -1) {
>  		printk(KERN_WARNING "cciss: Cannot find 
> cfg_base_addr_index\n");
>  		release_io_mem(c);
> -		return -1;
> +		err = -ENODEV;
> +		goto err_out_disable_pdev;
>  	}
>  
>  	cfg_offset = readl(c->vaddr + SA5_CTMEM_OFFSET); @@ 
> -2868,7 +2871,8 @@
>  		printk(KERN_WARNING "cciss: Sorry, I don't know how"
>  			" to access the Smart Array controller 
> %08lx\n", 
>  				(unsigned long)board_id);
> -		return -1;
> +		err = -ENODEV;
> +		goto err_out_disable_pdev;
>  	}
>  	if (  (readb(&c->cfgtable->Signature[0]) != 'C') ||
>  	      (readb(&c->cfgtable->Signature[1]) != 'I') || @@ 
> -2876,7 +2880,8 @@
>  	      (readb(&c->cfgtable->Signature[3]) != 'S') )
>  	{
>  		printk("Does not appear to be a valid CISS 
> config table\n");
> -		return -1;
> +		err = -ENODEV;
> +		goto err_out_disable_pdev;
>  	}
>  
>  #ifdef CONFIG_X86
> @@ -2920,10 +2925,14 @@
>  	{
>  		printk(KERN_WARNING "cciss: unable to get board into"
>  					" simple mode\n");
> -		return -1;
> +		err = -ENODEV;
> +		goto err_out_disable_pdev;
>  	}
>  	return 0;
>  
> +err_out_disable_pdev:
> +	pci_disable_device(pdev);
> +	return err;
>  }
>  
>  /* 
> 

[PATCH 2/7] CCISS: request all PCI resources

[Bjorn Helgaas]

We should call pci_request_regions() to claim all resources the device
decodes.  Previously, we claimed only the I/O port range.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: rc5-mm3/drivers/block/cciss.c
===================================================================
--- rc5-mm3.orig/drivers/block/cciss.c	2006-06-14 14:37:49.000000000 -0600
+++ rc5-mm3/drivers/block/cciss.c	2006-06-14 14:44:03.000000000 -0600
@@ -2638,16 +2638,6 @@
 }
 #endif /* CCISS_DEBUG */ 
 
-static void release_io_mem(ctlr_info_t *c)
-{
-	/* if IO mem was not protected do nothing */
-	if( c->io_mem_addr == 0)
-		return;
-	release_region(c->io_mem_addr, c->io_mem_length);
-	c->io_mem_addr = 0;
-	c->io_mem_length = 0;
-}
-
 static int find_PCI_BAR_index(struct pci_dev *pdev,
 				unsigned long pci_bar_addr)
 {
@@ -2762,36 +2752,18 @@
 		return err;
 	}
 
+	err = pci_request_regions(pdev, "cciss");
+	if (err) {
+		printk(KERN_ERR "cciss: Cannot obtain PCI resources, "
+			"aborting\n");
+		goto err_out_disable_pdev;
+	}
+
 	subsystem_vendor_id = pdev->subsystem_vendor;
 	subsystem_device_id = pdev->subsystem_device;
 	board_id = (((__u32) (subsystem_device_id << 16) & 0xffff0000) |
 					subsystem_vendor_id);
 
-	/* search for our IO range so we can protect it */
-	for(i=0; i<DEVICE_COUNT_RESOURCE; i++)
-	{
-		/* is this an IO range */ 
-		if( pci_resource_flags(pdev, i) & 0x01 ) {
-			c->io_mem_addr = pci_resource_start(pdev, i);
-			c->io_mem_length = pci_resource_end(pdev, i) -
-				pci_resource_start(pdev, i) +1;
-#ifdef CCISS_DEBUG
-			printk("IO value found base_addr[%d] %lx %lx\n", i,
-				c->io_mem_addr, c->io_mem_length);
-#endif /* CCISS_DEBUG */
-			/* register the IO range */ 
-			if(!request_region( c->io_mem_addr,
-                                        c->io_mem_length, "cciss"))
-			{
-				printk(KERN_WARNING "cciss I/O memory range already in use addr=%lx length=%ld\n",
-				c->io_mem_addr, c->io_mem_length);
-				c->io_mem_addr= 0;
-				c->io_mem_length = 0;
-			} 
-			break;
-		}
-	}
-
 #ifdef CCISS_DEBUG
 	printk("command = %x\n", command);
 	printk("irq = %x\n", pdev->irq);
@@ -2826,7 +2798,7 @@
 	if (scratchpad != CCISS_FIRMWARE_READY) {
 		printk(KERN_WARNING "cciss: Board not ready.  Timed out.\n");
 		err = -ENODEV;
-		goto err_out_disable_pdev;
+		goto err_out_free_res;
 	}
 
 	/* get the address index number */
@@ -2842,9 +2814,8 @@
 #endif /* CCISS_DEBUG */
 	if (cfg_base_addr_index == -1) {
 		printk(KERN_WARNING "cciss: Cannot find cfg_base_addr_index\n");
-		release_io_mem(c);
 		err = -ENODEV;
-		goto err_out_disable_pdev;
+		goto err_out_free_res;
 	}
 
 	cfg_offset = readl(c->vaddr + SA5_CTMEM_OFFSET);
@@ -2872,7 +2843,7 @@
 			" to access the Smart Array controller %08lx\n", 
 				(unsigned long)board_id);
 		err = -ENODEV;
-		goto err_out_disable_pdev;
+		goto err_out_free_res;
 	}
 	if (  (readb(&c->cfgtable->Signature[0]) != 'C') ||
 	      (readb(&c->cfgtable->Signature[1]) != 'I') ||
@@ -2881,7 +2852,7 @@
 	{
 		printk("Does not appear to be a valid CISS config table\n");
 		err = -ENODEV;
-		goto err_out_disable_pdev;
+		goto err_out_free_res;
 	}
 
 #ifdef CONFIG_X86
@@ -2926,10 +2897,13 @@
 		printk(KERN_WARNING "cciss: unable to get board into"
 					" simple mode\n");
 		err = -ENODEV;
-		goto err_out_disable_pdev;
+		goto err_out_free_res;
 	}
 	return 0;
 
+err_out_free_res:
+	pci_release_regions(pdev);
+
 err_out_disable_pdev:
 	pci_disable_device(pdev);
 	return err;
@@ -3276,7 +3250,6 @@
 clean2:
 	unregister_blkdev(hba[i]->major, hba[i]->devname);
 clean1:
-	release_io_mem(hba[i]);
 	hba[i]->busy_initializing = 0;
 	free_hba(i);
 	return(-1);
@@ -3322,7 +3295,6 @@
                 pci_disable_msi(hba[i]->pdev);
 #endif /* CONFIG_PCI_MSI */
 
-	pci_set_drvdata(pdev, NULL);
 	iounmap(hba[i]->vaddr);
 	cciss_unregister_scsi(i);  /* unhook from SCSI subsystem */
 	unregister_blkdev(hba[i]->major, hba[i]->devname);
@@ -3349,7 +3321,9 @@
 #ifdef CONFIG_CISS_SCSI_TAPE
 	kfree(hba[i]->scsi_rejects.complete);
 #endif
- 	release_io_mem(hba[i]);
+ 	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
 	free_hba(i);
 }	
 
Index: rc5-mm3/drivers/block/cciss.h
===================================================================
--- rc5-mm3.orig/drivers/block/cciss.h	2006-03-19 22:53:29.000000000 -0700
+++ rc5-mm3/drivers/block/cciss.h	2006-06-14 14:45:00.000000000 -0600
@@ -60,8 +60,6 @@
 	__u32	board_id;
 	void __iomem *vaddr;
 	unsigned long paddr;
-	unsigned long io_mem_addr;
-	unsigned long io_mem_length;
 	CfgTable_struct __iomem *cfgtable;
 	int	interrupts_enabled;
 	int	major;

RE: [PATCH 2/7] CCISS: request all PCI resources

[Miller, Mike (OS Dev)]

 

> -----Original Message-----
> From: Helgaas, Bjorn 
> Sent: Wednesday, June 14, 2006 6:10 PM
> To: Miller, Mike (OS Dev)
> Cc: ISS StorageDev; linux-kernel@vger.kernel.org; Andrew Morton
> Subject: [PATCH 2/7] CCISS: request all PCI resources
> 
> We should call pci_request_regions() to claim all resources 
> the device decodes.  Previously, we claimed only the I/O port range.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Acked-by: Mike Miller <mike.miller@hp.com>

> 
> Index: rc5-mm3/drivers/block/cciss.c
> ===================================================================
> --- rc5-mm3.orig/drivers/block/cciss.c	2006-06-14 
> 14:37:49.000000000 -0600
> +++ rc5-mm3/drivers/block/cciss.c	2006-06-14 
> 14:44:03.000000000 -0600
> @@ -2638,16 +2638,6 @@
>  }
>  #endif /* CCISS_DEBUG */ 
>  
> -static void release_io_mem(ctlr_info_t *c) -{
> -	/* if IO mem was not protected do nothing */
> -	if( c->io_mem_addr == 0)
> -		return;
> -	release_region(c->io_mem_addr, c->io_mem_length);
> -	c->io_mem_addr = 0;
> -	c->io_mem_length = 0;
> -}
> -
>  static int find_PCI_BAR_index(struct pci_dev *pdev,
>  				unsigned long pci_bar_addr)
>  {
> @@ -2762,36 +2752,18 @@
>  		return err;
>  	}
>  
> +	err = pci_request_regions(pdev, "cciss");
> +	if (err) {
> +		printk(KERN_ERR "cciss: Cannot obtain PCI resources, "
> +			"aborting\n");
> +		goto err_out_disable_pdev;
> +	}
> +
>  	subsystem_vendor_id = pdev->subsystem_vendor;
>  	subsystem_device_id = pdev->subsystem_device;
>  	board_id = (((__u32) (subsystem_device_id << 16) & 0xffff0000) |
>  					subsystem_vendor_id);
>  
> -	/* search for our IO range so we can protect it */
> -	for(i=0; i<DEVICE_COUNT_RESOURCE; i++)
> -	{
> -		/* is this an IO range */ 
> -		if( pci_resource_flags(pdev, i) & 0x01 ) {
> -			c->io_mem_addr = pci_resource_start(pdev, i);
> -			c->io_mem_length = pci_resource_end(pdev, i) -
> -				pci_resource_start(pdev, i) +1;
> -#ifdef CCISS_DEBUG
> -			printk("IO value found base_addr[%d] 
> %lx %lx\n", i,
> -				c->io_mem_addr, c->io_mem_length);
> -#endif /* CCISS_DEBUG */
> -			/* register the IO range */ 
> -			if(!request_region( c->io_mem_addr,
> -                                        c->io_mem_length, "cciss"))
> -			{
> -				printk(KERN_WARNING "cciss I/O 
> memory range already in use addr=%lx length=%ld\n",
> -				c->io_mem_addr, c->io_mem_length);
> -				c->io_mem_addr= 0;
> -				c->io_mem_length = 0;
> -			} 
> -			break;
> -		}
> -	}
> -
>  #ifdef CCISS_DEBUG
>  	printk("command = %x\n", command);
>  	printk("irq = %x\n", pdev->irq);
> @@ -2826,7 +2798,7 @@
>  	if (scratchpad != CCISS_FIRMWARE_READY) {
>  		printk(KERN_WARNING "cciss: Board not ready.  
> Timed out.\n");
>  		err = -ENODEV;
> -		goto err_out_disable_pdev;
> +		goto err_out_free_res;
>  	}
>  
>  	/* get the address index number */
> @@ -2842,9 +2814,8 @@
>  #endif /* CCISS_DEBUG */
>  	if (cfg_base_addr_index == -1) {
>  		printk(KERN_WARNING "cciss: Cannot find 
> cfg_base_addr_index\n");
> -		release_io_mem(c);
>  		err = -ENODEV;
> -		goto err_out_disable_pdev;
> +		goto err_out_free_res;
>  	}
>  
>  	cfg_offset = readl(c->vaddr + SA5_CTMEM_OFFSET); @@ 
> -2872,7 +2843,7 @@
>  			" to access the Smart Array controller 
> %08lx\n", 
>  				(unsigned long)board_id);
>  		err = -ENODEV;
> -		goto err_out_disable_pdev;
> +		goto err_out_free_res;
>  	}
>  	if (  (readb(&c->cfgtable->Signature[0]) != 'C') ||
>  	      (readb(&c->cfgtable->Signature[1]) != 'I') || @@ 
> -2881,7 +2852,7 @@
>  	{
>  		printk("Does not appear to be a valid CISS 
> config table\n");
>  		err = -ENODEV;
> -		goto err_out_disable_pdev;
> +		goto err_out_free_res;
>  	}
>  
>  #ifdef CONFIG_X86
> @@ -2926,10 +2897,13 @@
>  		printk(KERN_WARNING "cciss: unable to get board into"
>  					" simple mode\n");
>  		err = -ENODEV;
> -		goto err_out_disable_pdev;
> +		goto err_out_free_res;
>  	}
>  	return 0;
>  
> +err_out_free_res:
> +	pci_release_regions(pdev);
> +
>  err_out_disable_pdev:
>  	pci_disable_device(pdev);
>  	return err;
> @@ -3276,7 +3250,6 @@
>  clean2:
>  	unregister_blkdev(hba[i]->major, hba[i]->devname);
>  clean1:
> -	release_io_mem(hba[i]);
>  	hba[i]->busy_initializing = 0;
>  	free_hba(i);
>  	return(-1);
> @@ -3322,7 +3295,6 @@
>                  pci_disable_msi(hba[i]->pdev);  #endif /* 
> CONFIG_PCI_MSI */
>  
> -	pci_set_drvdata(pdev, NULL);
>  	iounmap(hba[i]->vaddr);
>  	cciss_unregister_scsi(i);  /* unhook from SCSI subsystem */
>  	unregister_blkdev(hba[i]->major, hba[i]->devname); @@ 
> -3349,7 +3321,9 @@  #ifdef CONFIG_CISS_SCSI_TAPE
>  	kfree(hba[i]->scsi_rejects.complete);
>  #endif
> - 	release_io_mem(hba[i]);
> + 	pci_release_regions(pdev);
> +	pci_disable_device(pdev);
> +	pci_set_drvdata(pdev, NULL);
>  	free_hba(i);
>  }	
>  
> Index: rc5-mm3/drivers/block/cciss.h
> ===================================================================
> --- rc5-mm3.orig/drivers/block/cciss.h	2006-03-19 
> 22:53:29.000000000 -0700
> +++ rc5-mm3/drivers/block/cciss.h	2006-06-14 
> 14:45:00.000000000 -0600
> @@ -60,8 +60,6 @@
>  	__u32	board_id;
>  	void __iomem *vaddr;
>  	unsigned long paddr;
> -	unsigned long io_mem_addr;
> -	unsigned long io_mem_length;
>  	CfgTable_struct __iomem *cfgtable;
>  	int	interrupts_enabled;
>  	int	major;
> 

[PATCH 3/7] CCISS: announce cciss%d devices with PCI address/IRQ/DAC info

[Bjorn Helgaas]

We already print "cciss: using DAC cycles" or similar for every
adapter found: why not just identify the device we're talking
about and include other useful information?

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: rc6-mm2/drivers/block/cciss.c
===================================================================
--- rc6-mm2.orig/drivers/block/cciss.c	2006-06-14 16:18:20.000000000 -0600
+++ rc6-mm2/drivers/block/cciss.c	2006-06-14 16:18:29.000000000 -0600
@@ -3086,11 +3086,8 @@
 	int i;
 	int j;
 	int rc;
+	int dac;
 
-	printk(KERN_DEBUG "cciss: Device 0x%x has been found at"
-			" bus %d dev %d func %d\n",
-		pdev->device, pdev->bus->number, PCI_SLOT(pdev->devfn),
-			PCI_FUNC(pdev->devfn));
 	i = alloc_cciss_hba();
 	if(i < 0)
 		return (-1);
@@ -3106,11 +3103,11 @@
 
 	/* configure PCI DMA stuff */
 	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK))
-		printk("cciss: using DAC cycles\n");
+		dac = 1;
 	else if (!pci_set_dma_mask(pdev, DMA_32BIT_MASK))
-		printk("cciss: not using DAC cycles\n");
+		dac = 0;
 	else {
-		printk("cciss: no suitable DMA available\n");
+		printk(KERN_ERR "cciss: no suitable DMA available\n");
 		goto clean1;
 	}
 
@@ -3141,6 +3138,11 @@
 			hba[i]->intr[SIMPLE_MODE_INT], hba[i]->devname);
 		goto clean2;
 	}
+
+	printk(KERN_INFO "%s: <0x%x> at PCI %s IRQ %d%s using DAC\n",
+		hba[i]->devname, pdev->device, pci_name(pdev),
+		hba[i]->intr[SIMPLE_MODE_INT], dac ? "" : " not");
+
 	hba[i]->cmd_pool_bits = kmalloc(((NR_CMDS+BITS_PER_LONG-1)/BITS_PER_LONG)*sizeof(unsigned long), GFP_KERNEL);
 	hba[i]->cmd_pool = (CommandList_struct *)pci_alloc_consistent(
 		hba[i]->pdev, NR_CMDS * sizeof(CommandList_struct), 

Re: [PATCH 3/7] CCISS: announce cciss%d devices with PCI address/IRQ/DAC info

[Jeff Garzik]

Bjorn Helgaas wrote:
> We already print "cciss: using DAC cycles" or similar for every
> adapter found: why not just identify the device we're talking
> about and include other useful information?
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> 
> Index: rc6-mm2/drivers/block/cciss.c
> ===================================================================
> --- rc6-mm2.orig/drivers/block/cciss.c	2006-06-14 16:18:20.000000000 -0600
> +++ rc6-mm2/drivers/block/cciss.c	2006-06-14 16:18:29.000000000 -0600
> @@ -3086,11 +3086,8 @@
>  	int i;
>  	int j;
>  	int rc;
> +	int dac;
>  
> -	printk(KERN_DEBUG "cciss: Device 0x%x has been found at"
> -			" bus %d dev %d func %d\n",
> -		pdev->device, pdev->bus->number, PCI_SLOT(pdev->devfn),
> -			PCI_FUNC(pdev->devfn));
>  	i = alloc_cciss_hba();
>  	if(i < 0)
>  		return (-1);
> @@ -3106,11 +3103,11 @@
>  
>  	/* configure PCI DMA stuff */
>  	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK))
> -		printk("cciss: using DAC cycles\n");
> +		dac = 1;
>  	else if (!pci_set_dma_mask(pdev, DMA_32BIT_MASK))
> -		printk("cciss: not using DAC cycles\n");
> +		dac = 0;
>  	else {
> -		printk("cciss: no suitable DMA available\n");
> +		printk(KERN_ERR "cciss: no suitable DMA available\n");
>  		goto clean1;
>  	}
>  
> @@ -3141,6 +3138,11 @@
>  			hba[i]->intr[SIMPLE_MODE_INT], hba[i]->devname);
>  		goto clean2;
>  	}
> +
> +	printk(KERN_INFO "%s: <0x%x> at PCI %s IRQ %d%s using DAC\n",
> +		hba[i]->devname, pdev->device, pci_name(pdev),
> +		hba[i]->intr[SIMPLE_MODE_INT], dac ? "" : " not");

Although this patch is correct, I would consider using dev_printk() 
rather than referencing pci_name() in printk() arguments.

	Jeff

RE: [PATCH 3/7] CCISS: announce cciss%d devices with PCI address/IRQ/DAC info

[Miller, Mike (OS Dev)]

 

> -----Original Message-----
> From: Helgaas, Bjorn 
> Sent: Wednesday, June 14, 2006 6:10 PM
> To: Miller, Mike (OS Dev)
> Cc: ISS StorageDev; linux-kernel@vger.kernel.org; Andrew Morton
> Subject: [PATCH 3/7] CCISS: announce cciss%d devices with PCI 
> address/IRQ/DAC info
> 
> We already print "cciss: using DAC cycles" or similar for 
> every adapter found: why not just identify the device we're 
> talking about and include other useful information?
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Acked-by: Mike Miller <mike.miller@hp.com>

> 
> Index: rc6-mm2/drivers/block/cciss.c
> ===================================================================
> --- rc6-mm2.orig/drivers/block/cciss.c	2006-06-14 
> 16:18:20.000000000 -0600
> +++ rc6-mm2/drivers/block/cciss.c	2006-06-14 
> 16:18:29.000000000 -0600
> @@ -3086,11 +3086,8 @@
>  	int i;
>  	int j;
>  	int rc;
> +	int dac;
>  
> -	printk(KERN_DEBUG "cciss: Device 0x%x has been found at"
> -			" bus %d dev %d func %d\n",
> -		pdev->device, pdev->bus->number, PCI_SLOT(pdev->devfn),
> -			PCI_FUNC(pdev->devfn));
>  	i = alloc_cciss_hba();
>  	if(i < 0)
>  		return (-1);
> @@ -3106,11 +3103,11 @@
>  
>  	/* configure PCI DMA stuff */
>  	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK))
> -		printk("cciss: using DAC cycles\n");
> +		dac = 1;
>  	else if (!pci_set_dma_mask(pdev, DMA_32BIT_MASK))
> -		printk("cciss: not using DAC cycles\n");
> +		dac = 0;
>  	else {
> -		printk("cciss: no suitable DMA available\n");
> +		printk(KERN_ERR "cciss: no suitable DMA available\n");
>  		goto clean1;
>  	}
>  
> @@ -3141,6 +3138,11 @@
>  			hba[i]->intr[SIMPLE_MODE_INT], hba[i]->devname);
>  		goto clean2;
>  	}
> +
> +	printk(KERN_INFO "%s: <0x%x> at PCI %s IRQ %d%s using DAC\n",
> +		hba[i]->devname, pdev->device, pci_name(pdev),
> +		hba[i]->intr[SIMPLE_MODE_INT], dac ? "" : " not");
> +
>  	hba[i]->cmd_pool_bits = 
> kmalloc(((NR_CMDS+BITS_PER_LONG-1)/BITS_PER_LONG)*sizeof(unsig
> ned long), GFP_KERNEL);
>  	hba[i]->cmd_pool = (CommandList_struct *)pci_alloc_consistent(
>  		hba[i]->pdev, NR_CMDS * sizeof(CommandList_struct), 
> 

[PATCH 4/7] CCISS: use ARRAY_SIZE without intermediates

[Bjorn Helgaas]

It's easier to verify loop bounds if the array name is mentioned
the for() statement that steps through the array.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: rc5-mm3/drivers/block/cciss.c
===================================================================
--- rc5-mm3.orig/drivers/block/cciss.c	2006-06-14 15:15:28.000000000 -0600
+++ rc5-mm3/drivers/block/cciss.c	2006-06-14 15:16:13.000000000 -0600
@@ -104,8 +104,6 @@
 };
 MODULE_DEVICE_TABLE(pci, cciss_pci_device_id);
 
-#define NR_PRODUCTS ARRAY_SIZE(products)
-
 /*  board_id = Subsystem Device ID & Vendor ID
  *  product = Marketing Name for the board
  *  access = Address of the struct of function pointers 
@@ -2831,14 +2829,14 @@
 	print_cfg_table(c->cfgtable);
 #endif /* CCISS_DEBUG */
 
-	for(i=0; i<NR_PRODUCTS; i++) {
+	for(i=0; i<ARRAY_SIZE(products); i++) {
 		if (board_id == products[i].board_id) {
 			c->product_name = products[i].product_name;
 			c->access = *(products[i].access);
 			break;
 		}
 	}
-	if (i == NR_PRODUCTS) {
+	if (i == ARRAY_SIZE(products)) {
 		printk(KERN_WARNING "cciss: Sorry, I don't know how"
 			" to access the Smart Array controller %08lx\n", 
 				(unsigned long)board_id);

Re: [PATCH 4/7] CCISS: use ARRAY_SIZE without intermediates

[Jeff Garzik]

Bjorn Helgaas wrote:
> It's easier to verify loop bounds if the array name is mentioned
> the for() statement that steps through the array.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

ACK patches 4-7

RE: [PATCH 4/7] CCISS: use ARRAY_SIZE without intermediates

[Miller, Mike (OS Dev)]

 

> -----Original Message-----
> From: Helgaas, Bjorn 
> Sent: Wednesday, June 14, 2006 6:11 PM
> To: Miller, Mike (OS Dev)
> Cc: ISS StorageDev; linux-kernel@vger.kernel.org; Andrew Morton
> Subject: [PATCH 4/7] CCISS: use ARRAY_SIZE without intermediates
> 
> It's easier to verify loop bounds if the array name is 
> mentioned the for() statement that steps through the array.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Acked-by: Mike Miller <mike.miller@hp.com>

> 
> Index: rc5-mm3/drivers/block/cciss.c
> ===================================================================
> --- rc5-mm3.orig/drivers/block/cciss.c	2006-06-14 
> 15:15:28.000000000 -0600
> +++ rc5-mm3/drivers/block/cciss.c	2006-06-14 
> 15:16:13.000000000 -0600
> @@ -104,8 +104,6 @@
>  };
>  MODULE_DEVICE_TABLE(pci, cciss_pci_device_id);
>  
> -#define NR_PRODUCTS ARRAY_SIZE(products)
> -
>  /*  board_id = Subsystem Device ID & Vendor ID
>   *  product = Marketing Name for the board
>   *  access = Address of the struct of function pointers @@ 
> -2831,14 +2829,14 @@
>  	print_cfg_table(c->cfgtable);
>  #endif /* CCISS_DEBUG */
>  
> -	for(i=0; i<NR_PRODUCTS; i++) {
> +	for(i=0; i<ARRAY_SIZE(products); i++) {
>  		if (board_id == products[i].board_id) {
>  			c->product_name = products[i].product_name;
>  			c->access = *(products[i].access);
>  			break;
>  		}
>  	}
> -	if (i == NR_PRODUCTS) {
> +	if (i == ARRAY_SIZE(products)) {
>  		printk(KERN_WARNING "cciss: Sorry, I don't know how"
>  			" to access the Smart Array controller 
> %08lx\n", 
>  				(unsigned long)board_id);
> 

[PATCH 5/7] CCISS: fix a few spelling errors

[Bjorn Helgaas]

Fix a few spelling errors.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: rc5-mm3/drivers/block/cciss.c
===================================================================
--- rc5-mm3.orig/drivers/block/cciss.c	2006-06-14 15:16:13.000000000 -0600
+++ rc5-mm3/drivers/block/cciss.c	2006-06-14 15:16:44.000000000 -0600
@@ -129,7 +129,7 @@
 	{ 0x3215103C, "Smart Array E200i", &SA5_access},
 };
 
-/* How long to wait (in millesconds) for board to go into simple mode */
+/* How long to wait (in milliseconds) for board to go into simple mode */
 #define MAX_CONFIG_WAIT 30000 
 #define MAX_IOCTL_CONFIG_WAIT 1000
 
@@ -1117,7 +1117,7 @@
  *
  * Right now I'm using the getgeometry() function to do this, but this
  * function should probably be finer grained and allow you to revalidate one
- * particualar logical volume (instead of all of them on a particular
+ * particular logical volume (instead of all of them on a particular
  * controller).
  */
 static int revalidate_allvol(ctlr_info_t *host)
@@ -1260,7 +1260,7 @@
 		return;
 
 
-	/* Get information about the disk and modify the driver sturcture */
+	/* Get information about the disk and modify the driver structure */
 	size_buff = kmalloc(sizeof( ReadCapdata_struct), GFP_KERNEL);
         if (size_buff == NULL)
 		goto mem_msg;
@@ -1335,7 +1335,7 @@
 }
 
 /* This function will add and remove logical drives from the Logical
- * drive array of the controller and maintain persistancy of ordering
+ * drive array of the controller and maintain persistency of ordering
  * so that mount points are preserved until the next reboot.  This allows
  * for the removal of logical drives in the middle of the drive array
  * without a re-ordering of those drives.
@@ -1482,7 +1482,7 @@
  * clear_all = This flag determines whether or not the disk information
  *             is going to be completely cleared out and the highest_lun
  *             reset.  Sometimes we want to clear out information about
- *             the disk in preperation for re-adding it.  In this case
+ *             the disk in preparation for re-adding it.  In this case
  *             the highest_lun should be left unchanged and the LunID
  *             should not be cleared.
 */
@@ -2597,7 +2597,7 @@
 	return IRQ_HANDLED;
 }
 /* 
- *  We cannot read the structure directly, for portablity we must use 
+ *  We cannot read the structure directly, for portability we must use 
  *   the io functions.
  *   This is for debug only. 
  */
@@ -2620,9 +2620,9 @@
 				readl(&(tb->TransportActive)));
 	printk("   Requested transport Method = 0x%x\n", 
 			readl(&(tb->HostWrite.TransportRequest)));
-	printk("   Coalese Interrupt Delay = 0x%x\n", 
+	printk("   Coalesce Interrupt Delay = 0x%x\n", 
 			readl(&(tb->HostWrite.CoalIntDelay)));
-	printk("   Coalese Interrupt Count = 0x%x\n", 
+	printk("   Coalesce Interrupt Count = 0x%x\n", 
 			readl(&(tb->HostWrite.CoalIntCount)));
 	printk("   Max outstanding commands = 0x%d\n", 
 			readl(&(tb->CmdsOutMax)));

RE: [PATCH 5/7] CCISS: fix a few spelling errors

[Miller, Mike (OS Dev)]

 

> -----Original Message-----
> From: Helgaas, Bjorn 
> Sent: Wednesday, June 14, 2006 6:12 PM
> To: Miller, Mike (OS Dev)
> Cc: ISS StorageDev; linux-kernel@vger.kernel.org; Andrew Morton
> Subject: [PATCH 5/7] CCISS: fix a few spelling errors
> 
> Fix a few spelling errors.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Acked-by: Mike Miller <mike.miller@hp.com>

> 
> Index: rc5-mm3/drivers/block/cciss.c
> ===================================================================
> --- rc5-mm3.orig/drivers/block/cciss.c	2006-06-14 
> 15:16:13.000000000 -0600
> +++ rc5-mm3/drivers/block/cciss.c	2006-06-14 
> 15:16:44.000000000 -0600
> @@ -129,7 +129,7 @@
>  	{ 0x3215103C, "Smart Array E200i", &SA5_access},  };
>  
> -/* How long to wait (in millesconds) for board to go into 
> simple mode */
> +/* How long to wait (in milliseconds) for board to go into 
> simple mode 
> +*/
>  #define MAX_CONFIG_WAIT 30000
>  #define MAX_IOCTL_CONFIG_WAIT 1000
>  
> @@ -1117,7 +1117,7 @@
>   *
>   * Right now I'm using the getgeometry() function to do 
> this, but this
>   * function should probably be finer grained and allow you 
> to revalidate one
> - * particualar logical volume (instead of all of them on a particular
> + * particular logical volume (instead of all of them on a particular
>   * controller).
>   */
>  static int revalidate_allvol(ctlr_info_t *host) @@ -1260,7 +1260,7 @@
>  		return;
>  
>  
> -	/* Get information about the disk and modify the driver 
> sturcture */
> +	/* Get information about the disk and modify the driver 
> structure */
>  	size_buff = kmalloc(sizeof( ReadCapdata_struct), GFP_KERNEL);
>          if (size_buff == NULL)
>  		goto mem_msg;
> @@ -1335,7 +1335,7 @@
>  }
>  
>  /* This function will add and remove logical drives from the Logical
> - * drive array of the controller and maintain persistancy of ordering
> + * drive array of the controller and maintain persistency of ordering
>   * so that mount points are preserved until the next reboot. 
>  This allows
>   * for the removal of logical drives in the middle of the drive array
>   * without a re-ordering of those drives.
> @@ -1482,7 +1482,7 @@
>   * clear_all = This flag determines whether or not the disk 
> information
>   *             is going to be completely cleared out and the 
> highest_lun
>   *             reset.  Sometimes we want to clear out 
> information about
> - *             the disk in preperation for re-adding it.  In 
> this case
> + *             the disk in preparation for re-adding it.  In 
> this case
>   *             the highest_lun should be left unchanged and the LunID
>   *             should not be cleared.
>  */
> @@ -2597,7 +2597,7 @@
>  	return IRQ_HANDLED;
>  }
>  /*
> - *  We cannot read the structure directly, for portablity we 
> must use 
> + *  We cannot read the structure directly, for portability 
> we must use
>   *   the io functions.
>   *   This is for debug only. 
>   */
> @@ -2620,9 +2620,9 @@
>  				readl(&(tb->TransportActive)));
>  	printk("   Requested transport Method = 0x%x\n", 
>  			readl(&(tb->HostWrite.TransportRequest)));
> -	printk("   Coalese Interrupt Delay = 0x%x\n", 
> +	printk("   Coalesce Interrupt Delay = 0x%x\n", 
>  			readl(&(tb->HostWrite.CoalIntDelay)));
> -	printk("   Coalese Interrupt Count = 0x%x\n", 
> +	printk("   Coalesce Interrupt Count = 0x%x\n", 
>  			readl(&(tb->HostWrite.CoalIntCount)));
>  	printk("   Max outstanding commands = 0x%d\n", 
>  			readl(&(tb->CmdsOutMax)));
> 

[PATCH 6/7] CCISS: remove parens around return values

[Bjorn Helgaas]

Typical Linux style is "return -EINVAL", not "return(-EINVAL)".

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: rc6-mm2/drivers/block/cciss.c
===================================================================
--- rc6-mm2.orig/drivers/block/cciss.c	2006-06-14 16:18:37.000000000 -0600
+++ rc6-mm2/drivers/block/cciss.c	2006-06-14 16:18:39.000000000 -0600
@@ -677,7 +677,7 @@
 		pciinfo.board_id = host->board_id;
 		if (copy_to_user(argp, &pciinfo,  sizeof( cciss_pci_info_struct )))
 			return  -EFAULT;
-		return(0);
+		return 0;
 	}	
 	case CCISS_GETINTINFO:
 	{
@@ -687,7 +687,7 @@
 		intinfo.count = readl(&host->cfgtable->HostWrite.CoalIntCount);
 		if (copy_to_user(argp, &intinfo, sizeof( cciss_coalint_struct )))
 			return -EFAULT;
-                return(0);
+                return 0;
         }
 	case CCISS_SETINTINFO:
         {
@@ -703,7 +703,7 @@
 
 		{
 //			printk("cciss_ioctl: delay and count cannot be 0\n");
-			return( -EINVAL);
+			return -EINVAL;
 		}
 		spin_lock_irqsave(CCISS_LOCK(ctlr), flags);
 		/* Update the field, and then ring the doorbell */ 
@@ -723,7 +723,7 @@
 		spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
 		if (i >= MAX_IOCTL_CONFIG_WAIT)
 			return -EAGAIN;
-                return(0);
+                return 0;
         }
 	case CCISS_GETNODENAME:
         {
@@ -735,7 +735,7 @@
 			NodeName[i] = readb(&host->cfgtable->ServerName[i]);
                 if (copy_to_user(argp, NodeName, sizeof( NodeName_type)))
                 	return  -EFAULT;
-                return(0);
+                return 0;
         }
 	case CCISS_SETNODENAME:
 	{
@@ -767,7 +767,7 @@
 		spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
 		if (i >= MAX_IOCTL_CONFIG_WAIT)
 			return -EAGAIN;
-                return(0);
+                return 0;
         }
 
 	case CCISS_GETHEARTBEAT:
@@ -778,7 +778,7 @@
                 heartbeat = readl(&host->cfgtable->HeartBeat);
                 if (copy_to_user(argp, &heartbeat, sizeof( Heartbeat_type)))
                 	return -EFAULT;
-                return(0);
+                return 0;
         }
 	case CCISS_GETBUSTYPES:
         {
@@ -788,7 +788,7 @@
                 BusTypes = readl(&host->cfgtable->BusTypes);
                 if (copy_to_user(argp, &BusTypes, sizeof( BusTypes_type) ))
                 	return  -EFAULT;
-                return(0);
+                return 0;
         }
 	case CCISS_GETFIRMVER:
         {
@@ -799,7 +799,7 @@
 
                 if (copy_to_user(argp, firmware, sizeof( FirmwareVer_type)))
                 	return -EFAULT;
-                return(0);
+                return 0;
         }
         case CCISS_GETDRIVVER:
         {
@@ -809,7 +809,7 @@
 
                 if (copy_to_user(argp, &DriverVer, sizeof( DriverVer_type) ))
                 	return -EFAULT;
-                return(0);
+                return 0;
         }
 
 	case CCISS_REVALIDVOLS:
@@ -826,7 +826,7 @@
  		if (copy_to_user(argp, &luninfo,
  				sizeof(LogvolInfo_struct)))
  			return -EFAULT;
- 		return(0);
+ 		return 0;
  	}
 	case CCISS_DEREGDISK:
 		return rebuild_lun_table(host, disk);
@@ -934,7 +934,7 @@
 		{
 			kfree(buff);
 			cmd_free(host, c, 0);
-			return( -EFAULT);	
+			return -EFAULT;	
 		} 	
 
 		if (iocommand.Request.Type.Direction == XFER_READ)
@@ -949,7 +949,7 @@
                 }
                 kfree(buff);
 		cmd_free(host, c, 0);
-                return(0);
+                return 0;
 	} 
 	case CCISS_BIG_PASSTHRU: {
 		BIG_IOCTL_Command_struct *ioc;
@@ -1101,7 +1101,7 @@
 		}
 		kfree(buff_size);
 		kfree(ioc);
-		return(status);
+		return status;
 	}
 	default:
 		return -ENOTTY;
@@ -1546,7 +1546,7 @@
 
 	drv->LunID = 0;
 	}
-	return(0);
+	return 0;
 }
 
 static int fill_cmd(CommandList_struct *c, __u8 cmd, int ctlr, void *buff,
@@ -1639,7 +1639,7 @@
 		default:
 			printk(KERN_WARNING
 				"cciss%d:  Unknown Command 0x%c\n", ctlr, cmd);
-			return(IO_ERROR);
+			return IO_ERROR;
 		}
 	} else if (cmd_type == TYPE_MSG) {
 		switch (cmd) {
@@ -1807,7 +1807,7 @@
 	pci_unmap_single( h->pdev, (dma_addr_t) buff_dma_handle.val,
 			c->SG[0].Len, PCI_DMA_BIDIRECTIONAL);
 	cmd_free(h, c, 0);
-        return(return_status);
+        return return_status;
 
 }
 static void cciss_geometry_inquiry(int ctlr, int logvol,
@@ -1942,7 +1942,7 @@
 		if (done == FIFO_EMPTY)
 			schedule_timeout_uninterruptible(1);
 		else
-			return (done);
+			return done;
 	}
 	/* Invalid address to tell caller we ran out of time */
 	return 1;
@@ -2019,7 +2019,7 @@
 
 	if ((c = cmd_alloc(info_p, 1)) == NULL) {
 		printk(KERN_WARNING "cciss: unable to get memory");
-		return(IO_ERROR);
+		return IO_ERROR;
 	}
 	status = fill_cmd(c, cmd, ctlr, buff, size, use_unit_num,
 		log_unit, page_code, scsi3addr, cmd_type);
@@ -2154,7 +2154,7 @@
 		do_cciss_intr(0, info_p, NULL);
 #endif
 	cmd_free(info_p, c, 1);
-	return (status);
+	return status;
 } 
 /*
  * Map (physical) PCI mem into (virtual) kernel space
@@ -3088,7 +3088,7 @@
 
 	i = alloc_cciss_hba();
 	if(i < 0)
-		return (-1);
+		return -1;
 
 	hba[i]->busy_initializing = 1;
 
@@ -3230,7 +3230,7 @@
 		add_disk(disk);
 	}
 
-	return(1);
+	return 1;
 
 clean4:
 #ifdef CONFIG_CISS_SCSI_TAPE
@@ -3252,7 +3252,7 @@
 clean1:
 	hba[i]->busy_initializing = 0;
 	free_hba(i);
-	return(-1);
+	return -1;
 }
 
 static void __devexit cciss_remove_one (struct pci_dev *pdev)

RE: [PATCH 6/7] CCISS: remove parens around return values

[Miller, Mike (OS Dev)]

 

> -----Original Message-----
> From: Helgaas, Bjorn 
> Sent: Wednesday, June 14, 2006 6:13 PM
> To: Miller, Mike (OS Dev)
> Cc: ISS StorageDev; linux-kernel@vger.kernel.org; Andrew Morton
> Subject: [PATCH 6/7] CCISS: remove parens around return values
> 
> Typical Linux style is "return -EINVAL", not "return(-EINVAL)".
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Acked-by: Mike Miller <mike.miller@hp.com>

> 
> Index: rc6-mm2/drivers/block/cciss.c
> ===================================================================
> --- rc6-mm2.orig/drivers/block/cciss.c	2006-06-14 
> 16:18:37.000000000 -0600
> +++ rc6-mm2/drivers/block/cciss.c	2006-06-14 
> 16:18:39.000000000 -0600
> @@ -677,7 +677,7 @@
>  		pciinfo.board_id = host->board_id;
>  		if (copy_to_user(argp, &pciinfo,  sizeof( 
> cciss_pci_info_struct )))
>  			return  -EFAULT;
> -		return(0);
> +		return 0;
>  	}	
>  	case CCISS_GETINTINFO:
>  	{
> @@ -687,7 +687,7 @@
>  		intinfo.count = 
> readl(&host->cfgtable->HostWrite.CoalIntCount);
>  		if (copy_to_user(argp, &intinfo, sizeof( 
> cciss_coalint_struct )))
>  			return -EFAULT;
> -                return(0);
> +                return 0;
>          }
>  	case CCISS_SETINTINFO:
>          {
> @@ -703,7 +703,7 @@
>  
>  		{
>  //			printk("cciss_ioctl: delay and count 
> cannot be 0\n");
> -			return( -EINVAL);
> +			return -EINVAL;
>  		}
>  		spin_lock_irqsave(CCISS_LOCK(ctlr), flags);
>  		/* Update the field, and then ring the doorbell 
> */ @@ -723,7 +723,7 @@
>  		spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
>  		if (i >= MAX_IOCTL_CONFIG_WAIT)
>  			return -EAGAIN;
> -                return(0);
> +                return 0;
>          }
>  	case CCISS_GETNODENAME:
>          {
> @@ -735,7 +735,7 @@
>  			NodeName[i] = 
> readb(&host->cfgtable->ServerName[i]);
>                  if (copy_to_user(argp, NodeName, sizeof( 
> NodeName_type)))
>                  	return  -EFAULT;
> -                return(0);
> +                return 0;
>          }
>  	case CCISS_SETNODENAME:
>  	{
> @@ -767,7 +767,7 @@
>  		spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
>  		if (i >= MAX_IOCTL_CONFIG_WAIT)
>  			return -EAGAIN;
> -                return(0);
> +                return 0;
>          }
>  
>  	case CCISS_GETHEARTBEAT:
> @@ -778,7 +778,7 @@
>                  heartbeat = readl(&host->cfgtable->HeartBeat);
>                  if (copy_to_user(argp, &heartbeat, sizeof( 
> Heartbeat_type)))
>                  	return -EFAULT;
> -                return(0);
> +                return 0;
>          }
>  	case CCISS_GETBUSTYPES:
>          {
> @@ -788,7 +788,7 @@
>                  BusTypes = readl(&host->cfgtable->BusTypes);
>                  if (copy_to_user(argp, &BusTypes, sizeof( 
> BusTypes_type) ))
>                  	return  -EFAULT;
> -                return(0);
> +                return 0;
>          }
>  	case CCISS_GETFIRMVER:
>          {
> @@ -799,7 +799,7 @@
>  
>                  if (copy_to_user(argp, firmware, sizeof( 
> FirmwareVer_type)))
>                  	return -EFAULT;
> -                return(0);
> +                return 0;
>          }
>          case CCISS_GETDRIVVER:
>          {
> @@ -809,7 +809,7 @@
>  
>                  if (copy_to_user(argp, &DriverVer, sizeof( 
> DriverVer_type) ))
>                  	return -EFAULT;
> -                return(0);
> +                return 0;
>          }
>  
>  	case CCISS_REVALIDVOLS:
> @@ -826,7 +826,7 @@
>   		if (copy_to_user(argp, &luninfo,
>   				sizeof(LogvolInfo_struct)))
>   			return -EFAULT;
> - 		return(0);
> + 		return 0;
>   	}
>  	case CCISS_DEREGDISK:
>  		return rebuild_lun_table(host, disk); @@ -934,7 
> +934,7 @@
>  		{
>  			kfree(buff);
>  			cmd_free(host, c, 0);
> -			return( -EFAULT);	
> +			return -EFAULT;	
>  		} 	
>  
>  		if (iocommand.Request.Type.Direction == 
> XFER_READ) @@ -949,7 +949,7 @@
>                  }
>                  kfree(buff);
>  		cmd_free(host, c, 0);
> -                return(0);
> +                return 0;
>  	} 
>  	case CCISS_BIG_PASSTHRU: {
>  		BIG_IOCTL_Command_struct *ioc;
> @@ -1101,7 +1101,7 @@
>  		}
>  		kfree(buff_size);
>  		kfree(ioc);
> -		return(status);
> +		return status;
>  	}
>  	default:
>  		return -ENOTTY;
> @@ -1546,7 +1546,7 @@
>  
>  	drv->LunID = 0;
>  	}
> -	return(0);
> +	return 0;
>  }
>  
>  static int fill_cmd(CommandList_struct *c, __u8 cmd, int 
> ctlr, void *buff, @@ -1639,7 +1639,7 @@
>  		default:
>  			printk(KERN_WARNING
>  				"cciss%d:  Unknown Command 
> 0x%c\n", ctlr, cmd);
> -			return(IO_ERROR);
> +			return IO_ERROR;
>  		}
>  	} else if (cmd_type == TYPE_MSG) {
>  		switch (cmd) {
> @@ -1807,7 +1807,7 @@
>  	pci_unmap_single( h->pdev, (dma_addr_t) buff_dma_handle.val,
>  			c->SG[0].Len, PCI_DMA_BIDIRECTIONAL);
>  	cmd_free(h, c, 0);
> -        return(return_status);
> +        return return_status;
>  
>  }
>  static void cciss_geometry_inquiry(int ctlr, int logvol, @@ 
> -1942,7 +1942,7 @@
>  		if (done == FIFO_EMPTY)
>  			schedule_timeout_uninterruptible(1);
>  		else
> -			return (done);
> +			return done;
>  	}
>  	/* Invalid address to tell caller we ran out of time */
>  	return 1;
> @@ -2019,7 +2019,7 @@
>  
>  	if ((c = cmd_alloc(info_p, 1)) == NULL) {
>  		printk(KERN_WARNING "cciss: unable to get memory");
> -		return(IO_ERROR);
> +		return IO_ERROR;
>  	}
>  	status = fill_cmd(c, cmd, ctlr, buff, size, use_unit_num,
>  		log_unit, page_code, scsi3addr, cmd_type); @@ 
> -2154,7 +2154,7 @@
>  		do_cciss_intr(0, info_p, NULL);
>  #endif
>  	cmd_free(info_p, c, 1);
> -	return (status);
> +	return status;
>  }
>  /*
>   * Map (physical) PCI mem into (virtual) kernel space @@ 
> -3088,7 +3088,7 @@
>  
>  	i = alloc_cciss_hba();
>  	if(i < 0)
> -		return (-1);
> +		return -1;
>  
>  	hba[i]->busy_initializing = 1;
>  
> @@ -3230,7 +3230,7 @@
>  		add_disk(disk);
>  	}
>  
> -	return(1);
> +	return 1;
>  
>  clean4:
>  #ifdef CONFIG_CISS_SCSI_TAPE
> @@ -3252,7 +3252,7 @@
>  clean1:
>  	hba[i]->busy_initializing = 0;
>  	free_hba(i);
> -	return(-1);
> +	return -1;
>  }
>  
>  static void __devexit cciss_remove_one (struct pci_dev *pdev)
> 

Re: [PATCH 7/7] CCISS: run through Lindent

[Bjorn Helgaas]

On Wednesday 14 June 2006 17:13, Bjorn Helgaas wrote:
> cciss is full of inconsistent style ("for (" vs. "for(", lines that
> end with whitespace, lines beginning with a mix of spaces & tabs, etc).
> 
> This patch changes only whitespace.

This patch was too big for the linux-kernel list.   You can get
the whole thing here:

$ wget ftp://ftp.kernel.org/pub/linux/kernel/people/helgaas/cciss-indent

Re: [PATCH 0/7] CCISS cleanups

[Laura Garcia]

Hi Bjorn,

	Reading cciss and cpqarray driver code, I've noticed that both have very similar structure so, could it be useful to merge both drivers in only one?


Best regards.


"Bjorn Helgaas" <bjorn.helgaas@hp.com> wrote:

> From: Bjorn Helgaas <bjorn.helgaas@hp.com>
> Date: 15-jun-2006 1:07
> Subject: [PATCH 0/7] CCISS cleanups
> To: Mike Miller <mike.miller@hp.com>
> Cc: iss_storagedev@hp.com, linux-kernel@vger.kernel.org, Andrew Morton
> <akpm@osdl.org>
> 
> 
> This is a series of minor cleanups to the cciss (HP Smart Array) driver:
> 
>   1  disable device before returning failure
>   2  claim all resources the device decodes, not just I/O ports
>   3  print more useful identification when driver claims device
>   4  remove intermediate #define for ARRAY_SIZE
>   5  fix spelling errors
>   6  unparenthesize "return" statements
>   7  Lindent (warning, huge diff, but changes whitespace only)
> 
> They're in order of usefulness.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/7] CCISS cleanups

[Jeff Garzik]

Laura Garcia wrote:
> Hi Bjorn,
> 
> 	Reading cciss and cpqarray driver code, I've noticed that both have very similar structure so, could it be useful to merge both drivers in only one?

Given that cpqarray hardware is not actively appearing on the market, it 
may be best to just leave that driver alone.

	Jeff

[PATCH 8/7] CCISS: tidy up product table indentation

[Bjorn Helgaas]

Make each one fit on a line so it's easier to read.  I re-ordered
COMPAQ_CISSC/0x4091, which was out of order.  I double-checked these,
but it would be good if you'd also check them to make sure I didn't
miss any.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: rc6-mm2/drivers/block/cciss.c
===================================================================
--- rc6-mm2.orig/drivers/block/cciss.c	2006-06-15 10:47:49.000000000 -0600
+++ rc6-mm2/drivers/block/cciss.c	2006-06-15 10:55:48.000000000 -0600
@@ -64,42 +64,24 @@
 
 /* define the PCI info for the cards we can control */
 static const struct pci_device_id cciss_pci_device_id[] = {
-	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISS,
-	 0x0E11, 0x4070, 0, 0, 0},
-	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSB,
-	 0x0E11, 0x4080, 0, 0, 0},
-	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSB,
-	 0x0E11, 0x4082, 0, 0, 0},
-	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSB,
-	 0x0E11, 0x4083, 0, 0, 0},
-	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSC,
-	 0x0E11, 0x409A, 0, 0, 0},
-	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSC,
-	 0x0E11, 0x409B, 0, 0, 0},
-	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSC,
-	 0x0E11, 0x409C, 0, 0, 0},
-	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSC,
-	 0x0E11, 0x409D, 0, 0, 0},
-	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSC,
-	 0x0E11, 0x4091, 0, 0, 0},
-	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSA,
-	 0x103C, 0x3225, 0, 0, 0},
-	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSC,
-	 0x103c, 0x3223, 0, 0, 0},
-	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSC,
-	 0x103c, 0x3234, 0, 0, 0},
-	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSC,
-	 0x103c, 0x3235, 0, 0, 0},
-	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSD,
-	 0x103c, 0x3211, 0, 0, 0},
-	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSD,
-	 0x103c, 0x3212, 0, 0, 0},
-	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSD,
-	 0x103c, 0x3213, 0, 0, 0},
-	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSD,
-	 0x103c, 0x3214, 0, 0, 0},
-	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSD,
-	 0x103c, 0x3215, 0, 0, 0},
+	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISS,  0x0E11, 0x4070},
+	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSB, 0x0E11, 0x4080},
+	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSB, 0x0E11, 0x4082},
+	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSB, 0x0E11, 0x4083},
+	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSC, 0x0E11, 0x4091},
+	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSC, 0x0E11, 0x409A},
+	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSC, 0x0E11, 0x409B},
+	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSC, 0x0E11, 0x409C},
+	{PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSC, 0x0E11, 0x409D},
+	{PCI_VENDOR_ID_HP,     PCI_DEVICE_ID_HP_CISSA,     0x103C, 0x3225},
+	{PCI_VENDOR_ID_HP,     PCI_DEVICE_ID_HP_CISSC,     0x103C, 0x3223},
+	{PCI_VENDOR_ID_HP,     PCI_DEVICE_ID_HP_CISSC,     0x103C, 0x3234},
+	{PCI_VENDOR_ID_HP,     PCI_DEVICE_ID_HP_CISSC,     0x103C, 0x3235},
+	{PCI_VENDOR_ID_HP,     PCI_DEVICE_ID_HP_CISSD,     0x103C, 0x3211},
+	{PCI_VENDOR_ID_HP,     PCI_DEVICE_ID_HP_CISSD,     0x103C, 0x3212},
+	{PCI_VENDOR_ID_HP,     PCI_DEVICE_ID_HP_CISSD,     0x103C, 0x3213},
+	{PCI_VENDOR_ID_HP,     PCI_DEVICE_ID_HP_CISSD,     0x103C, 0x3214},
+	{PCI_VENDOR_ID_HP,     PCI_DEVICE_ID_HP_CISSD,     0x103C, 0x3215},
 	{0,}
 };