[PATCH v1.1 2/16 update 3] arcmsr: Adding code to support MSI-X interrupt

[PATCH v1.1 2/16 update 3] arcmsr: Adding code to support MSI-X interrupt

[ching]

From: Ching<ching2048@areca.com.tw>

Adding code to support MSI-X interrupt.

This update has made change by Tomas' and Alexander's suggestion.

Signed-off-by: Ching<ching2048@areca.com.tw>
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
--- a/drivers/scsi/arcmsr/arcmsr.h	2014-04-28 16:02:46.000000000 +0800
+++ b/drivers/scsi/arcmsr/arcmsr.h	2014-05-06 15:24:36.000000000 +0800
@@ -64,6 +64,7 @@ struct device_attribute;
 #define ARCMSR_MAX_HBB_POSTQUEUE						264
 #define ARCMSR_MAX_XFER_LEN							0x26000 /* 152K */
 #define ARCMSR_CDB_SG_PAGE_LENGTH						256 
+#define ARCMST_NUM_MSIX_VECTORS		4
 #ifndef PCI_DEVICE_ID_ARECA_1880
 #define PCI_DEVICE_ID_ARECA_1880 0x1880
  #endif
@@ -508,6 +509,7 @@ struct AdapterControlBlock
 	struct pci_dev *		pdev;
 	struct Scsi_Host *		host;
 	unsigned long			vir2phy_offset;
+	struct msix_entry	entries[ARCMST_NUM_MSIX_VECTORS];
 	/* Offset is used in making arc cdb physical to virtual calculations */
 	uint32_t			outbound_int_enable;
 	uint32_t			cdb_phyaddr_hi32;
@@ -544,6 +546,8 @@ struct AdapterControlBlock
 	/* iop init */
 	#define ACB_F_ABORT				0x0200
 	#define ACB_F_FIRMWARE_TRAP           		0x0400
+	#define ACB_F_MSI_ENABLED		0x1000
+	#define ACB_F_MSIX_ENABLED		0x2000
 	struct CommandControlBlock *			pccb_pool[ARCMSR_MAX_FREECCB_NUM];
 	/* used for memory free */
 	struct list_head		ccb_free_list;
@@ -594,6 +598,7 @@ struct AdapterControlBlock
 				#define	FW_DEADLOCK	0x0010
 	atomic_t 			rq_map_token;
 	atomic_t			ante_token_value;
+	int		msix_vector_count;
 };/* HW_DEVICE_EXTENSION */
 /*
 *******************************************************************************
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c	2014-04-28 15:58:34.000000000 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c	2014-05-07 18:42:40.000000000 +0800
@@ -603,6 +603,60 @@ static void arcmsr_message_isr_bh_fn(str
 	}
 }
 
+static int
+arcmsr_request_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
+{
+	int	i, j, r;
+	struct msix_entry entries[ARCMST_NUM_MSIX_VECTORS];
+
+	if (!pci_find_capability(pdev, PCI_CAP_ID_MSIX))
+		goto msi_int;
+	for (i = 0; i < ARCMST_NUM_MSIX_VECTORS; i++)
+		entries[i].entry = i;
+	r = pci_enable_msix_range(pdev, entries, 1, ARCMST_NUM_MSIX_VECTORS);
+	if (r < 0)
+		goto msi_int;
+	acb->msix_vector_count = r;
+	for (i = 0; i < r; i++) {
+		if (request_irq(entries[i].vector,
+			arcmsr_do_interrupt, 0, "arcmsr", acb)) {
+			pr_warn("arcmsr%d: request_irq =%d failed!\n",
+				acb->host->host_no, pdev->irq);
+			for (j = 0 ; j < i ; j++)
+				free_irq(entries[j].vector, acb);
+			pci_disable_msix(pdev);
+			goto msi_int;
+		}
+		acb->entries[i] = entries[i];
+	}
+	acb->acb_flags |= ACB_F_MSIX_ENABLED;
+	pr_info("arcmsr%d: msi-x enabled\n", acb->host->host_no);
+	return ARC_SUCCESS;
+msi_int:
+	if (!pci_find_capability(pdev, PCI_CAP_ID_MSI))
+		goto legacy_int;
+	if (pci_enable_msi_range(pdev, 1, 1) < 0)
+		goto legacy_int;
+	if (request_irq(pdev->irq, arcmsr_do_interrupt,
+		IRQF_SHARED, "arcmsr", acb)) {
+		pr_warn("arcmsr%d: request_irq =%d failed!\n",
+			acb->host->host_no, pdev->irq);
+		pci_disable_msi(pdev);
+		goto legacy_int;
+	}
+	acb->acb_flags |= ACB_F_MSI_ENABLED;
+	pr_info("arcmsr%d: msi enabled\n", acb->host->host_no);
+	return ARC_SUCCESS;
+legacy_int:
+	if (request_irq(pdev->irq, arcmsr_do_interrupt,
+		IRQF_SHARED, "arcmsr", acb)) {
+		pr_warn("arcmsr%d: request_irq = %d failed!\n",
+			acb->host->host_no, pdev->irq);
+		return ARC_FAILURE;
+	}
+	return ARC_SUCCESS;
+}
+
 static int arcmsr_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct Scsi_Host *host;
@@ -667,16 +721,13 @@ static int arcmsr_probe(struct pci_dev *
 	if(error){
 		goto free_hbb_mu;
 	}
-	arcmsr_iop_init(acb);
 	error = scsi_add_host(host, &pdev->dev);
 	if(error){
 		goto RAID_controller_stop;
 	}
-	error = request_irq(pdev->irq, arcmsr_do_interrupt, IRQF_SHARED, "arcmsr", acb);
-	if(error){
+	if (arcmsr_request_irq(pdev, acb) == ARC_FAILURE)
 		goto scsi_host_remove;
-	}
-	host->irq = pdev->irq;
+	arcmsr_iop_init(acb);
     	scsi_scan_host(host);
 	INIT_WORK(&acb->arcmsr_do_message_isr_bh, arcmsr_message_isr_bh_fn);
 	atomic_set(&acb->rq_map_token, 16);
@@ -710,6 +761,21 @@ pci_disable_dev:
 	return -ENODEV;
 }
 
+static void arcmsr_free_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
+{
+	int i;
+
+	if (acb->acb_flags & ACB_F_MSI_ENABLED) {
+		free_irq(pdev->irq, acb);
+		pci_disable_msi(pdev);
+	} else if (acb->acb_flags & ACB_F_MSIX_ENABLED) {
+		for (i = 0; i < acb->msix_vector_count; i++)
+			free_irq(acb->entries[i].vector, acb);
+		pci_disable_msix(pdev);
+	} else
+		free_irq(pdev->irq, acb);
+}
+
 static uint8_t arcmsr_abort_hba_allcmd(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
@@ -992,6 +1058,7 @@ static void arcmsr_done4abort_postqueue(
 	}
 	}
 }
+
 static void arcmsr_remove(struct pci_dev *pdev)
 {
 	struct Scsi_Host *host = pci_get_drvdata(pdev);
@@ -1029,7 +1096,7 @@ static void arcmsr_remove(struct pci_dev
 			}
 		}
 	}
-	free_irq(pdev->irq, acb);
+	arcmsr_free_irq(pdev, acb);
 	arcmsr_free_ccb_pool(acb);
 	arcmsr_free_hbb_mu(acb);
 	arcmsr_unmap_pciregion(acb);
@@ -1045,6 +1112,7 @@ static void arcmsr_shutdown(struct pci_d
 		(struct AdapterControlBlock *)host->hostdata;
 	del_timer_sync(&acb->eternal_timer);
 	arcmsr_disable_outbound_ints(acb);
+	arcmsr_free_irq(pdev, acb);
 	flush_work(&acb->arcmsr_do_message_isr_bh);
 	arcmsr_stop_adapter_bgrb(acb);
 	arcmsr_flush_adapter_cache(acb);
@@ -2516,8 +2584,6 @@ static int arcmsr_iop_confirm(struct Ada
 	case ACB_ADAPTER_TYPE_A: {
 		if (cdb_phyaddr_hi32 != 0) {
 			struct MessageUnit_A __iomem *reg = acb->pmuA;
-			uint32_t intmask_org;
-			intmask_org = arcmsr_disable_outbound_ints(acb);
 			writel(ARCMSR_SIGNATURE_SET_CONFIG, \
 						&reg->message_rwbuffer[0]);
 			writel(cdb_phyaddr_hi32, &reg->message_rwbuffer[1]);
@@ -2529,7 +2595,6 @@ static int arcmsr_iop_confirm(struct Ada
 				acb->host->host_no);
 				return 1;
 			}
-			arcmsr_enable_outbound_ints(acb, intmask_org);
 		}
 		}
 		break;
@@ -2539,8 +2604,6 @@ static int arcmsr_iop_confirm(struct Ada
 		uint32_t __iomem *rwbuffer;
 
 		struct MessageUnit_B *reg = acb->pmuB;
-		uint32_t intmask_org;
-		intmask_org = arcmsr_disable_outbound_ints(acb);
 		reg->postq_index = 0;
 		reg->doneq_index = 0;
 		writel(ARCMSR_MESSAGE_SET_POST_WINDOW, reg->drv2iop_doorbell);
@@ -2569,7 +2632,6 @@ static int arcmsr_iop_confirm(struct Ada
 			return 1;
 		}
 		arcmsr_hbb_enable_driver_mode(acb);
-		arcmsr_enable_outbound_ints(acb, intmask_org);
 		}
 		break;
 	case ACB_ADAPTER_TYPE_C: {

Re: [PATCH v1.1 2/16 update 3] arcmsr: Adding code to support MSI-X interrupt

[Tomas Henzl]

On 05/07/2014 01:52 PM, ching wrote:
> From: Ching<ching2048@areca.com.tw>
>
> Adding code to support MSI-X interrupt.
>
> This update has made change by Tomas' and Alexander's suggestion.
>
> Signed-off-by: Ching<ching2048@areca.com.tw>
> ---
>
> diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
> --- a/drivers/scsi/arcmsr/arcmsr.h	2014-04-28 16:02:46.000000000 +0800
> +++ b/drivers/scsi/arcmsr/arcmsr.h	2014-05-06 15:24:36.000000000 +0800
> @@ -64,6 +64,7 @@ struct device_attribute;
>  #define ARCMSR_MAX_HBB_POSTQUEUE						264
>  #define ARCMSR_MAX_XFER_LEN							0x26000 /* 152K */
>  #define ARCMSR_CDB_SG_PAGE_LENGTH						256 
> +#define ARCMST_NUM_MSIX_VECTORS		4
>  #ifndef PCI_DEVICE_ID_ARECA_1880
>  #define PCI_DEVICE_ID_ARECA_1880 0x1880
>   #endif
> @@ -508,6 +509,7 @@ struct AdapterControlBlock
>  	struct pci_dev *		pdev;
>  	struct Scsi_Host *		host;
>  	unsigned long			vir2phy_offset;
> +	struct msix_entry	entries[ARCMST_NUM_MSIX_VECTORS];
>  	/* Offset is used in making arc cdb physical to virtual calculations */
>  	uint32_t			outbound_int_enable;
>  	uint32_t			cdb_phyaddr_hi32;
> @@ -544,6 +546,8 @@ struct AdapterControlBlock
>  	/* iop init */
>  	#define ACB_F_ABORT				0x0200
>  	#define ACB_F_FIRMWARE_TRAP           		0x0400
> +	#define ACB_F_MSI_ENABLED		0x1000
> +	#define ACB_F_MSIX_ENABLED		0x2000
>  	struct CommandControlBlock *			pccb_pool[ARCMSR_MAX_FREECCB_NUM];
>  	/* used for memory free */
>  	struct list_head		ccb_free_list;
> @@ -594,6 +598,7 @@ struct AdapterControlBlock
>  				#define	FW_DEADLOCK	0x0010
>  	atomic_t 			rq_map_token;
>  	atomic_t			ante_token_value;
> +	int		msix_vector_count;
>  };/* HW_DEVICE_EXTENSION */
>  /*
>  *******************************************************************************
> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c	2014-04-28 15:58:34.000000000 +0800
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c	2014-05-07 18:42:40.000000000 +0800
> @@ -603,6 +603,60 @@ static void arcmsr_message_isr_bh_fn(str
>  	}
>  }
>  
> +static int
> +arcmsr_request_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
> +{
> +	int	i, j, r;
> +	struct msix_entry entries[ARCMST_NUM_MSIX_VECTORS];
> +
> +	if (!pci_find_capability(pdev, PCI_CAP_ID_MSIX))
> +		goto msi_int;
> +	for (i = 0; i < ARCMST_NUM_MSIX_VECTORS; i++)
> +		entries[i].entry = i;
> +	r = pci_enable_msix_range(pdev, entries, 1, ARCMST_NUM_MSIX_VECTORS);
> +	if (r < 0)
> +		goto msi_int;
> +	acb->msix_vector_count = r;
> +	for (i = 0; i < r; i++) {
> +		if (request_irq(entries[i].vector,
> +			arcmsr_do_interrupt, 0, "arcmsr", acb)) {
> +			pr_warn("arcmsr%d: request_irq =%d failed!\n",
> +				acb->host->host_no, pdev->irq);
> +			for (j = 0 ; j < i ; j++)
> +				free_irq(entries[j].vector, acb);
> +			pci_disable_msix(pdev);
> +			goto msi_int;
> +		}
> +		acb->entries[i] = entries[i];
> +	}
> +	acb->acb_flags |= ACB_F_MSIX_ENABLED;
> +	pr_info("arcmsr%d: msi-x enabled\n", acb->host->host_no);
> +	return ARC_SUCCESS;
> +msi_int:
> +	if (!pci_find_capability(pdev, PCI_CAP_ID_MSI))
> +		goto legacy_int;
> +	if (pci_enable_msi_range(pdev, 1, 1) < 0)
> +		goto legacy_int;
> +	if (request_irq(pdev->irq, arcmsr_do_interrupt,
> +		IRQF_SHARED, "arcmsr", acb)) {
> +		pr_warn("arcmsr%d: request_irq =%d failed!\n",
> +			acb->host->host_no, pdev->irq);
> +		pci_disable_msi(pdev);
> +		goto legacy_int;
> +	}
> +	acb->acb_flags |= ACB_F_MSI_ENABLED;
> +	pr_info("arcmsr%d: msi enabled\n", acb->host->host_no);
> +	return ARC_SUCCESS;
> +legacy_int:
> +	if (request_irq(pdev->irq, arcmsr_do_interrupt,
> +		IRQF_SHARED, "arcmsr", acb)) {
> +		pr_warn("arcmsr%d: request_irq = %d failed!\n",
> +			acb->host->host_no, pdev->irq);
> +		return ARC_FAILURE;
> +	}
> +	return ARC_SUCCESS;
> +}
> +
>  static int arcmsr_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
>  	struct Scsi_Host *host;
> @@ -667,16 +721,13 @@ static int arcmsr_probe(struct pci_dev *
>  	if(error){
>  		goto free_hbb_mu;
>  	}
> -	arcmsr_iop_init(acb);
>  	error = scsi_add_host(host, &pdev->dev);
>  	if(error){
>  		goto RAID_controller_stop;
Hi Ching,
you have moved the arcmsr_iop_init so you probably
should accordingly rewrite the error path handling at the end of this function
I don't know if it is a problem when you call arcmsr_stop_adapter_bgrb
without a starting it before
Also when you allocate the irq you could free the irq in out_free_sysfs:  
When you change all this you could consider moving the scsi_scan_host
more towards the end of that function so it's called after everything else 
is initiated, that can be made in another patch later.
-tomas
>
>
>
>
>
>  	}
> -	error = request_irq(pdev->irq, arcmsr_do_interrupt, IRQF_SHARED, "arcmsr", acb);
> -	if(error){
> +	if (arcmsr_request_irq(pdev, acb) == ARC_FAILURE)
>  		goto scsi_host_remove;
> -	}
> -	host->irq = pdev->irq;
> +	arcmsr_iop_init(acb);
>      	scsi_scan_host(host);
>  	INIT_WORK(&acb->arcmsr_do_message_isr_bh, arcmsr_message_isr_bh_fn);
>  	atomic_set(&acb->rq_map_token, 16);
> @@ -710,6 +761,21 @@ pci_disable_dev:
>  	return -ENODEV;
>  }
>  
> +static void arcmsr_free_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
> +{
> +	int i;
> +
> +	if (acb->acb_flags & ACB_F_MSI_ENABLED) {
> +		free_irq(pdev->irq, acb);
> +		pci_disable_msi(pdev);
> +	} else if (acb->acb_flags & ACB_F_MSIX_ENABLED) {
> +		for (i = 0; i < acb->msix_vector_count; i++)
> +			free_irq(acb->entries[i].vector, acb);
> +		pci_disable_msix(pdev);
> +	} else
> +		free_irq(pdev->irq, acb);
> +}
> +
>  static uint8_t arcmsr_abort_hba_allcmd(struct AdapterControlBlock *acb)
>  {
>  	struct MessageUnit_A __iomem *reg = acb->pmuA;
> @@ -992,6 +1058,7 @@ static void arcmsr_done4abort_postqueue(
>  	}
>  	}
>  }
> +
>  static void arcmsr_remove(struct pci_dev *pdev)
>  {
>  	struct Scsi_Host *host = pci_get_drvdata(pdev);
> @@ -1029,7 +1096,7 @@ static void arcmsr_remove(struct pci_dev
>  			}
>  		}
>  	}
> -	free_irq(pdev->irq, acb);
> +	arcmsr_free_irq(pdev, acb);
>  	arcmsr_free_ccb_pool(acb);
>  	arcmsr_free_hbb_mu(acb);
>  	arcmsr_unmap_pciregion(acb);
> @@ -1045,6 +1112,7 @@ static void arcmsr_shutdown(struct pci_d
>  		(struct AdapterControlBlock *)host->hostdata;
>  	del_timer_sync(&acb->eternal_timer);
>  	arcmsr_disable_outbound_ints(acb);
> +	arcmsr_free_irq(pdev, acb);
>  	flush_work(&acb->arcmsr_do_message_isr_bh);
>  	arcmsr_stop_adapter_bgrb(acb);
>  	arcmsr_flush_adapter_cache(acb);
> @@ -2516,8 +2584,6 @@ static int arcmsr_iop_confirm(struct Ada
>  	case ACB_ADAPTER_TYPE_A: {
>  		if (cdb_phyaddr_hi32 != 0) {
>  			struct MessageUnit_A __iomem *reg = acb->pmuA;
> -			uint32_t intmask_org;
> -			intmask_org = arcmsr_disable_outbound_ints(acb);
>  			writel(ARCMSR_SIGNATURE_SET_CONFIG, \
>  						&reg->message_rwbuffer[0]);
>  			writel(cdb_phyaddr_hi32, &reg->message_rwbuffer[1]);
> @@ -2529,7 +2595,6 @@ static int arcmsr_iop_confirm(struct Ada
>  				acb->host->host_no);
>  				return 1;
>  			}
> -			arcmsr_enable_outbound_ints(acb, intmask_org);
>  		}
>  		}
>  		break;
> @@ -2539,8 +2604,6 @@ static int arcmsr_iop_confirm(struct Ada
>  		uint32_t __iomem *rwbuffer;
>  
>  		struct MessageUnit_B *reg = acb->pmuB;
> -		uint32_t intmask_org;
> -		intmask_org = arcmsr_disable_outbound_ints(acb);
>  		reg->postq_index = 0;
>  		reg->doneq_index = 0;
>  		writel(ARCMSR_MESSAGE_SET_POST_WINDOW, reg->drv2iop_doorbell);
> @@ -2569,7 +2632,6 @@ static int arcmsr_iop_confirm(struct Ada
>  			return 1;
>  		}
>  		arcmsr_hbb_enable_driver_mode(acb);
> -		arcmsr_enable_outbound_ints(acb, intmask_org);
>  		}
>  		break;
>  	case ACB_ADAPTER_TYPE_C: {
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1.1 2/16 update 3] arcmsr: Adding code to support MSI-X interrupt

[ching]

Hi Tomas,

Thanks for your suggestion.
I will add a new patch 17/17 at last.
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c	2014-05-08 17:45:34.000000000 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c	2014-05-08 17:45:52.000000000 +0800
@@ -778,12 +778,11 @@ static int arcmsr_probe(struct pci_dev *
 	}
 	error = scsi_add_host(host, &pdev->dev);
 	if(error){
-		goto RAID_controller_stop;
+		goto free_ccb_pool;
 	}
 	if (arcmsr_request_irq(pdev, acb) == ARC_FAILURE)
 		goto scsi_host_remove;
 	arcmsr_iop_init(acb);
-    	scsi_scan_host(host);
 	INIT_WORK(&acb->arcmsr_do_message_isr_bh, arcmsr_message_isr_bh_fn);
 	atomic_set(&acb->rq_map_token, 16);
 	atomic_set(&acb->ante_token_value, 16);
@@ -795,13 +794,14 @@ static int arcmsr_probe(struct pci_dev *
 	add_timer(&acb->eternal_timer);
 	if(arcmsr_alloc_sysfs_attr(acb))
 		goto out_free_sysfs;
+    	scsi_scan_host(host);
 	return 0;
 out_free_sysfs:
-scsi_host_remove:
-	scsi_remove_host(host);
-RAID_controller_stop:
 	arcmsr_stop_adapter_bgrb(acb);
 	arcmsr_flush_adapter_cache(acb);
+scsi_host_remove:
+	scsi_remove_host(host);
+free_ccb_pool:
 	arcmsr_free_ccb_pool(acb);
 free_hbb_mu:
 	arcmsr_free_mu(acb);



On Wed, 2014-05-07 at 16:31 +0200, Tomas Henzl wrote:
> On 05/07/2014 01:52 PM, ching wrote:
> > From: Ching<ching2048@areca.com.tw>
> >
> > Adding code to support MSI-X interrupt.
> >
> > This update has made change by Tomas' and Alexander's suggestion.
> >
> > Signed-off-by: Ching<ching2048@areca.com.tw>
> > ---
> >
> > diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
> > --- a/drivers/scsi/arcmsr/arcmsr.h	2014-04-28 16:02:46.000000000 +0800
> > +++ b/drivers/scsi/arcmsr/arcmsr.h	2014-05-06 15:24:36.000000000 +0800
> > @@ -64,6 +64,7 @@ struct device_attribute;
> >  #define ARCMSR_MAX_HBB_POSTQUEUE						264
> >  #define ARCMSR_MAX_XFER_LEN							0x26000 /* 152K */
> >  #define ARCMSR_CDB_SG_PAGE_LENGTH						256 
> > +#define ARCMST_NUM_MSIX_VECTORS		4
> >  #ifndef PCI_DEVICE_ID_ARECA_1880
> >  #define PCI_DEVICE_ID_ARECA_1880 0x1880
> >   #endif
> > @@ -508,6 +509,7 @@ struct AdapterControlBlock
> >  	struct pci_dev *		pdev;
> >  	struct Scsi_Host *		host;
> >  	unsigned long			vir2phy_offset;
> > +	struct msix_entry	entries[ARCMST_NUM_MSIX_VECTORS];
> >  	/* Offset is used in making arc cdb physical to virtual calculations */
> >  	uint32_t			outbound_int_enable;
> >  	uint32_t			cdb_phyaddr_hi32;
> > @@ -544,6 +546,8 @@ struct AdapterControlBlock
> >  	/* iop init */
> >  	#define ACB_F_ABORT				0x0200
> >  	#define ACB_F_FIRMWARE_TRAP           		0x0400
> > +	#define ACB_F_MSI_ENABLED		0x1000
> > +	#define ACB_F_MSIX_ENABLED		0x2000
> >  	struct CommandControlBlock *			pccb_pool[ARCMSR_MAX_FREECCB_NUM];
> >  	/* used for memory free */
> >  	struct list_head		ccb_free_list;
> > @@ -594,6 +598,7 @@ struct AdapterControlBlock
> >  				#define	FW_DEADLOCK	0x0010
> >  	atomic_t 			rq_map_token;
> >  	atomic_t			ante_token_value;
> > +	int		msix_vector_count;
> >  };/* HW_DEVICE_EXTENSION */
> >  /*
> >  *******************************************************************************
> > diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> > --- a/drivers/scsi/arcmsr/arcmsr_hba.c	2014-04-28 15:58:34.000000000 +0800
> > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c	2014-05-07 18:42:40.000000000 +0800
> > @@ -603,6 +603,60 @@ static void arcmsr_message_isr_bh_fn(str
> >  	}
> >  }
> >  
> > +static int
> > +arcmsr_request_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
> > +{
> > +	int	i, j, r;
> > +	struct msix_entry entries[ARCMST_NUM_MSIX_VECTORS];
> > +
> > +	if (!pci_find_capability(pdev, PCI_CAP_ID_MSIX))
> > +		goto msi_int;
> > +	for (i = 0; i < ARCMST_NUM_MSIX_VECTORS; i++)
> > +		entries[i].entry = i;
> > +	r = pci_enable_msix_range(pdev, entries, 1, ARCMST_NUM_MSIX_VECTORS);
> > +	if (r < 0)
> > +		goto msi_int;
> > +	acb->msix_vector_count = r;
> > +	for (i = 0; i < r; i++) {
> > +		if (request_irq(entries[i].vector,
> > +			arcmsr_do_interrupt, 0, "arcmsr", acb)) {
> > +			pr_warn("arcmsr%d: request_irq =%d failed!\n",
> > +				acb->host->host_no, pdev->irq);
> > +			for (j = 0 ; j < i ; j++)
> > +				free_irq(entries[j].vector, acb);
> > +			pci_disable_msix(pdev);
> > +			goto msi_int;
> > +		}
> > +		acb->entries[i] = entries[i];
> > +	}
> > +	acb->acb_flags |= ACB_F_MSIX_ENABLED;
> > +	pr_info("arcmsr%d: msi-x enabled\n", acb->host->host_no);
> > +	return ARC_SUCCESS;
> > +msi_int:
> > +	if (!pci_find_capability(pdev, PCI_CAP_ID_MSI))
> > +		goto legacy_int;
> > +	if (pci_enable_msi_range(pdev, 1, 1) < 0)
> > +		goto legacy_int;
> > +	if (request_irq(pdev->irq, arcmsr_do_interrupt,
> > +		IRQF_SHARED, "arcmsr", acb)) {
> > +		pr_warn("arcmsr%d: request_irq =%d failed!\n",
> > +			acb->host->host_no, pdev->irq);
> > +		pci_disable_msi(pdev);
> > +		goto legacy_int;
> > +	}
> > +	acb->acb_flags |= ACB_F_MSI_ENABLED;
> > +	pr_info("arcmsr%d: msi enabled\n", acb->host->host_no);
> > +	return ARC_SUCCESS;
> > +legacy_int:
> > +	if (request_irq(pdev->irq, arcmsr_do_interrupt,
> > +		IRQF_SHARED, "arcmsr", acb)) {
> > +		pr_warn("arcmsr%d: request_irq = %d failed!\n",
> > +			acb->host->host_no, pdev->irq);
> > +		return ARC_FAILURE;
> > +	}
> > +	return ARC_SUCCESS;
> > +}
> > +
> >  static int arcmsr_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> >  {
> >  	struct Scsi_Host *host;
> > @@ -667,16 +721,13 @@ static int arcmsr_probe(struct pci_dev *
> >  	if(error){
> >  		goto free_hbb_mu;
> >  	}
> > -	arcmsr_iop_init(acb);
> >  	error = scsi_add_host(host, &pdev->dev);
> >  	if(error){
> >  		goto RAID_controller_stop;
> Hi Ching,
> you have moved the arcmsr_iop_init so you probably
> should accordingly rewrite the error path handling at the end of this function
> I don't know if it is a problem when you call arcmsr_stop_adapter_bgrb
> without a starting it before
> Also when you allocate the irq you could free the irq in out_free_sysfs:  
> When you change all this you could consider moving the scsi_scan_host
> more towards the end of that function so it's called after everything else 
> is initiated, that can be made in another patch later.
> -tomas
> >
> >
> >
> >
> >
> >  	}
> > -	error = request_irq(pdev->irq, arcmsr_do_interrupt, IRQF_SHARED, "arcmsr", acb);
> > -	if(error){
> > +	if (arcmsr_request_irq(pdev, acb) == ARC_FAILURE)
> >  		goto scsi_host_remove;
> > -	}
> > -	host->irq = pdev->irq;
> > +	arcmsr_iop_init(acb);
> >      	scsi_scan_host(host);
> >  	INIT_WORK(&acb->arcmsr_do_message_isr_bh, arcmsr_message_isr_bh_fn);
> >  	atomic_set(&acb->rq_map_token, 16);
> > @@ -710,6 +761,21 @@ pci_disable_dev:
> >  	return -ENODEV;
> >  }
> >  
> > +static void arcmsr_free_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
> > +{
> > +	int i;
> > +
> > +	if (acb->acb_flags & ACB_F_MSI_ENABLED) {
> > +		free_irq(pdev->irq, acb);
> > +		pci_disable_msi(pdev);
> > +	} else if (acb->acb_flags & ACB_F_MSIX_ENABLED) {
> > +		for (i = 0; i < acb->msix_vector_count; i++)
> > +			free_irq(acb->entries[i].vector, acb);
> > +		pci_disable_msix(pdev);
> > +	} else
> > +		free_irq(pdev->irq, acb);
> > +}
> > +
> >  static uint8_t arcmsr_abort_hba_allcmd(struct AdapterControlBlock *acb)
> >  {
> >  	struct MessageUnit_A __iomem *reg = acb->pmuA;
> > @@ -992,6 +1058,7 @@ static void arcmsr_done4abort_postqueue(
> >  	}
> >  	}
> >  }
> > +
> >  static void arcmsr_remove(struct pci_dev *pdev)
> >  {
> >  	struct Scsi_Host *host = pci_get_drvdata(pdev);
> > @@ -1029,7 +1096,7 @@ static void arcmsr_remove(struct pci_dev
> >  			}
> >  		}
> >  	}
> > -	free_irq(pdev->irq, acb);
> > +	arcmsr_free_irq(pdev, acb);
> >  	arcmsr_free_ccb_pool(acb);
> >  	arcmsr_free_hbb_mu(acb);
> >  	arcmsr_unmap_pciregion(acb);
> > @@ -1045,6 +1112,7 @@ static void arcmsr_shutdown(struct pci_d
> >  		(struct AdapterControlBlock *)host->hostdata;
> >  	del_timer_sync(&acb->eternal_timer);
> >  	arcmsr_disable_outbound_ints(acb);
> > +	arcmsr_free_irq(pdev, acb);
> >  	flush_work(&acb->arcmsr_do_message_isr_bh);
> >  	arcmsr_stop_adapter_bgrb(acb);
> >  	arcmsr_flush_adapter_cache(acb);
> > @@ -2516,8 +2584,6 @@ static int arcmsr_iop_confirm(struct Ada
> >  	case ACB_ADAPTER_TYPE_A: {
> >  		if (cdb_phyaddr_hi32 != 0) {
> >  			struct MessageUnit_A __iomem *reg = acb->pmuA;
> > -			uint32_t intmask_org;
> > -			intmask_org = arcmsr_disable_outbound_ints(acb);
> >  			writel(ARCMSR_SIGNATURE_SET_CONFIG, \
> >  						&reg->message_rwbuffer[0]);
> >  			writel(cdb_phyaddr_hi32, &reg->message_rwbuffer[1]);
> > @@ -2529,7 +2595,6 @@ static int arcmsr_iop_confirm(struct Ada
> >  				acb->host->host_no);
> >  				return 1;
> >  			}
> > -			arcmsr_enable_outbound_ints(acb, intmask_org);
> >  		}
> >  		}
> >  		break;
> > @@ -2539,8 +2604,6 @@ static int arcmsr_iop_confirm(struct Ada
> >  		uint32_t __iomem *rwbuffer;
> >  
> >  		struct MessageUnit_B *reg = acb->pmuB;
> > -		uint32_t intmask_org;
> > -		intmask_org = arcmsr_disable_outbound_ints(acb);
> >  		reg->postq_index = 0;
> >  		reg->doneq_index = 0;
> >  		writel(ARCMSR_MESSAGE_SET_POST_WINDOW, reg->drv2iop_doorbell);
> > @@ -2569,7 +2632,6 @@ static int arcmsr_iop_confirm(struct Ada
> >  			return 1;
> >  		}
> >  		arcmsr_hbb_enable_driver_mode(acb);
> > -		arcmsr_enable_outbound_ints(acb, intmask_org);
> >  		}
> >  		break;
> >  	case ACB_ADAPTER_TYPE_C: {
> >
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v1.1 2/16 update 3] arcmsr: Adding code to support MSI-X interrupt

[Tomas Henzl]

On 05/08/2014 01:37 PM, ching wrote:
> Hi Tomas,
>
> Thanks for your suggestion.
> I will add a new patch 17/17 at last.

This additional patch was meant for moving the scsi_scan_host, because it's
nor related to the ms-x interrupts. Probably  the maintainer will prefer
a single patch which changes msi-x at once, I don't care.
To the patch below - you should also free the allocated irqs as I wrote previously.

Cheers,
Tomas

> ---
>
> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c	2014-05-08 17:45:34.000000000 +0800
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c	2014-05-08 17:45:52.000000000 +0800
> @@ -778,12 +778,11 @@ static int arcmsr_probe(struct pci_dev *
>  	}
>  	error = scsi_add_host(host, &pdev->dev);
>  	if(error){
> -		goto RAID_controller_stop;
> +		goto free_ccb_pool;
>  	}
>  	if (arcmsr_request_irq(pdev, acb) == ARC_FAILURE)
>  		goto scsi_host_remove;
>  	arcmsr_iop_init(acb);
> -    	scsi_scan_host(host);
>  	INIT_WORK(&acb->arcmsr_do_message_isr_bh, arcmsr_message_isr_bh_fn);
>  	atomic_set(&acb->rq_map_token, 16);
>  	atomic_set(&acb->ante_token_value, 16);
> @@ -795,13 +794,14 @@ static int arcmsr_probe(struct pci_dev *
>  	add_timer(&acb->eternal_timer);
>  	if(arcmsr_alloc_sysfs_attr(acb))
>  		goto out_free_sysfs;
> +    	scsi_scan_host(host);
>  	return 0;
>  out_free_sysfs:
> -scsi_host_remove:
> -	scsi_remove_host(host);
> -RAID_controller_stop:
>  	arcmsr_stop_adapter_bgrb(acb);
>  	arcmsr_flush_adapter_cache(acb);
> +scsi_host_remove:
> +	scsi_remove_host(host);
> +free_ccb_pool:
>  	arcmsr_free_ccb_pool(acb);
>  free_hbb_mu:
>  	arcmsr_free_mu(acb);
>
>
>
> On Wed, 2014-05-07 at 16:31 +0200, Tomas Henzl wrote:
>> On 05/07/2014 01:52 PM, ching wrote:
>>> From: Ching<ching2048@areca.com.tw>
>>>
>>> Adding code to support MSI-X interrupt.
>>>
>>> This update has made change by Tomas' and Alexander's suggestion.
>>>
>>> Signed-off-by: Ching<ching2048@areca.com.tw>
>>> ---
>>>
>>> diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
>>> --- a/drivers/scsi/arcmsr/arcmsr.h	2014-04-28 16:02:46.000000000 +0800
>>> +++ b/drivers/scsi/arcmsr/arcmsr.h	2014-05-06 15:24:36.000000000 +0800
>>> @@ -64,6 +64,7 @@ struct device_attribute;
>>>  #define ARCMSR_MAX_HBB_POSTQUEUE						264
>>>  #define ARCMSR_MAX_XFER_LEN							0x26000 /* 152K */
>>>  #define ARCMSR_CDB_SG_PAGE_LENGTH						256 
>>> +#define ARCMST_NUM_MSIX_VECTORS		4
>>>  #ifndef PCI_DEVICE_ID_ARECA_1880
>>>  #define PCI_DEVICE_ID_ARECA_1880 0x1880
>>>   #endif
>>> @@ -508,6 +509,7 @@ struct AdapterControlBlock
>>>  	struct pci_dev *		pdev;
>>>  	struct Scsi_Host *		host;
>>>  	unsigned long			vir2phy_offset;
>>> +	struct msix_entry	entries[ARCMST_NUM_MSIX_VECTORS];
>>>  	/* Offset is used in making arc cdb physical to virtual calculations */
>>>  	uint32_t			outbound_int_enable;
>>>  	uint32_t			cdb_phyaddr_hi32;
>>> @@ -544,6 +546,8 @@ struct AdapterControlBlock
>>>  	/* iop init */
>>>  	#define ACB_F_ABORT				0x0200
>>>  	#define ACB_F_FIRMWARE_TRAP           		0x0400
>>> +	#define ACB_F_MSI_ENABLED		0x1000
>>> +	#define ACB_F_MSIX_ENABLED		0x2000
>>>  	struct CommandControlBlock *			pccb_pool[ARCMSR_MAX_FREECCB_NUM];
>>>  	/* used for memory free */
>>>  	struct list_head		ccb_free_list;
>>> @@ -594,6 +598,7 @@ struct AdapterControlBlock
>>>  				#define	FW_DEADLOCK	0x0010
>>>  	atomic_t 			rq_map_token;
>>>  	atomic_t			ante_token_value;
>>> +	int		msix_vector_count;
>>>  };/* HW_DEVICE_EXTENSION */
>>>  /*
>>>  *******************************************************************************
>>> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
>>> --- a/drivers/scsi/arcmsr/arcmsr_hba.c	2014-04-28 15:58:34.000000000 +0800
>>> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c	2014-05-07 18:42:40.000000000 +0800
>>> @@ -603,6 +603,60 @@ static void arcmsr_message_isr_bh_fn(str
>>>  	}
>>>  }
>>>  
>>> +static int
>>> +arcmsr_request_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
>>> +{
>>> +	int	i, j, r;
>>> +	struct msix_entry entries[ARCMST_NUM_MSIX_VECTORS];
>>> +
>>> +	if (!pci_find_capability(pdev, PCI_CAP_ID_MSIX))
>>> +		goto msi_int;
>>> +	for (i = 0; i < ARCMST_NUM_MSIX_VECTORS; i++)
>>> +		entries[i].entry = i;
>>> +	r = pci_enable_msix_range(pdev, entries, 1, ARCMST_NUM_MSIX_VECTORS);
>>> +	if (r < 0)
>>> +		goto msi_int;
>>> +	acb->msix_vector_count = r;
>>> +	for (i = 0; i < r; i++) {
>>> +		if (request_irq(entries[i].vector,
>>> +			arcmsr_do_interrupt, 0, "arcmsr", acb)) {
>>> +			pr_warn("arcmsr%d: request_irq =%d failed!\n",
>>> +				acb->host->host_no, pdev->irq);
>>> +			for (j = 0 ; j < i ; j++)
>>> +				free_irq(entries[j].vector, acb);
>>> +			pci_disable_msix(pdev);
>>> +			goto msi_int;
>>> +		}
>>> +		acb->entries[i] = entries[i];
>>> +	}
>>> +	acb->acb_flags |= ACB_F_MSIX_ENABLED;
>>> +	pr_info("arcmsr%d: msi-x enabled\n", acb->host->host_no);
>>> +	return ARC_SUCCESS;
>>> +msi_int:
>>> +	if (!pci_find_capability(pdev, PCI_CAP_ID_MSI))
>>> +		goto legacy_int;
>>> +	if (pci_enable_msi_range(pdev, 1, 1) < 0)
>>> +		goto legacy_int;
>>> +	if (request_irq(pdev->irq, arcmsr_do_interrupt,
>>> +		IRQF_SHARED, "arcmsr", acb)) {
>>> +		pr_warn("arcmsr%d: request_irq =%d failed!\n",
>>> +			acb->host->host_no, pdev->irq);
>>> +		pci_disable_msi(pdev);
>>> +		goto legacy_int;
>>> +	}
>>> +	acb->acb_flags |= ACB_F_MSI_ENABLED;
>>> +	pr_info("arcmsr%d: msi enabled\n", acb->host->host_no);
>>> +	return ARC_SUCCESS;
>>> +legacy_int:
>>> +	if (request_irq(pdev->irq, arcmsr_do_interrupt,
>>> +		IRQF_SHARED, "arcmsr", acb)) {
>>> +		pr_warn("arcmsr%d: request_irq = %d failed!\n",
>>> +			acb->host->host_no, pdev->irq);
>>> +		return ARC_FAILURE;
>>> +	}
>>> +	return ARC_SUCCESS;
>>> +}
>>> +
>>>  static int arcmsr_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>>  {
>>>  	struct Scsi_Host *host;
>>> @@ -667,16 +721,13 @@ static int arcmsr_probe(struct pci_dev *
>>>  	if(error){
>>>  		goto free_hbb_mu;
>>>  	}
>>> -	arcmsr_iop_init(acb);
>>>  	error = scsi_add_host(host, &pdev->dev);
>>>  	if(error){
>>>  		goto RAID_controller_stop;
>> Hi Ching,
>> you have moved the arcmsr_iop_init so you probably
>> should accordingly rewrite the error path handling at the end of this function
>> I don't know if it is a problem when you call arcmsr_stop_adapter_bgrb
>> without a starting it before
>> Also when you allocate the irq you could free the irq in out_free_sysfs:  
>> When you change all this you could consider moving the scsi_scan_host
>> more towards the end of that function so it's called after everything else 
>> is initiated, that can be made in another patch later.
>> -tomas
>>>
>>>
>>>
>>>
>>>  	}
>>> -	error = request_irq(pdev->irq, arcmsr_do_interrupt, IRQF_SHARED, "arcmsr", acb);
>>> -	if(error){
>>> +	if (arcmsr_request_irq(pdev, acb) == ARC_FAILURE)
>>>  		goto scsi_host_remove;
>>> -	}
>>> -	host->irq = pdev->irq;
>>> +	arcmsr_iop_init(acb);
>>>      	scsi_scan_host(host);
>>>  	INIT_WORK(&acb->arcmsr_do_message_isr_bh, arcmsr_message_isr_bh_fn);
>>>  	atomic_set(&acb->rq_map_token, 16);
>>> @@ -710,6 +761,21 @@ pci_disable_dev:
>>>  	return -ENODEV;
>>>  }
>>>  
>>> +static void arcmsr_free_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
>>> +{
>>> +	int i;
>>> +
>>> +	if (acb->acb_flags & ACB_F_MSI_ENABLED) {
>>> +		free_irq(pdev->irq, acb);
>>> +		pci_disable_msi(pdev);
>>> +	} else if (acb->acb_flags & ACB_F_MSIX_ENABLED) {
>>> +		for (i = 0; i < acb->msix_vector_count; i++)
>>> +			free_irq(acb->entries[i].vector, acb);
>>> +		pci_disable_msix(pdev);
>>> +	} else
>>> +		free_irq(pdev->irq, acb);
>>> +}
>>> +
>>>  static uint8_t arcmsr_abort_hba_allcmd(struct AdapterControlBlock *acb)
>>>  {
>>>  	struct MessageUnit_A __iomem *reg = acb->pmuA;
>>> @@ -992,6 +1058,7 @@ static void arcmsr_done4abort_postqueue(
>>>  	}
>>>  	}
>>>  }
>>> +
>>>  static void arcmsr_remove(struct pci_dev *pdev)
>>>  {
>>>  	struct Scsi_Host *host = pci_get_drvdata(pdev);
>>> @@ -1029,7 +1096,7 @@ static void arcmsr_remove(struct pci_dev
>>>  			}
>>>  		}
>>>  	}
>>> -	free_irq(pdev->irq, acb);
>>> +	arcmsr_free_irq(pdev, acb);
>>>  	arcmsr_free_ccb_pool(acb);
>>>  	arcmsr_free_hbb_mu(acb);
>>>  	arcmsr_unmap_pciregion(acb);
>>> @@ -1045,6 +1112,7 @@ static void arcmsr_shutdown(struct pci_d
>>>  		(struct AdapterControlBlock *)host->hostdata;
>>>  	del_timer_sync(&acb->eternal_timer);
>>>  	arcmsr_disable_outbound_ints(acb);
>>> +	arcmsr_free_irq(pdev, acb);
>>>  	flush_work(&acb->arcmsr_do_message_isr_bh);
>>>  	arcmsr_stop_adapter_bgrb(acb);
>>>  	arcmsr_flush_adapter_cache(acb);
>>> @@ -2516,8 +2584,6 @@ static int arcmsr_iop_confirm(struct Ada
>>>  	case ACB_ADAPTER_TYPE_A: {
>>>  		if (cdb_phyaddr_hi32 != 0) {
>>>  			struct MessageUnit_A __iomem *reg = acb->pmuA;
>>> -			uint32_t intmask_org;
>>> -			intmask_org = arcmsr_disable_outbound_ints(acb);
>>>  			writel(ARCMSR_SIGNATURE_SET_CONFIG, \
>>>  						&reg->message_rwbuffer[0]);
>>>  			writel(cdb_phyaddr_hi32, &reg->message_rwbuffer[1]);
>>> @@ -2529,7 +2595,6 @@ static int arcmsr_iop_confirm(struct Ada
>>>  				acb->host->host_no);
>>>  				return 1;
>>>  			}
>>> -			arcmsr_enable_outbound_ints(acb, intmask_org);
>>>  		}
>>>  		}
>>>  		break;
>>> @@ -2539,8 +2604,6 @@ static int arcmsr_iop_confirm(struct Ada
>>>  		uint32_t __iomem *rwbuffer;
>>>  
>>>  		struct MessageUnit_B *reg = acb->pmuB;
>>> -		uint32_t intmask_org;
>>> -		intmask_org = arcmsr_disable_outbound_ints(acb);
>>>  		reg->postq_index = 0;
>>>  		reg->doneq_index = 0;
>>>  		writel(ARCMSR_MESSAGE_SET_POST_WINDOW, reg->drv2iop_doorbell);
>>> @@ -2569,7 +2632,6 @@ static int arcmsr_iop_confirm(struct Ada
>>>  			return 1;
>>>  		}
>>>  		arcmsr_hbb_enable_driver_mode(acb);
>>> -		arcmsr_enable_outbound_ints(acb, intmask_org);
>>>  		}
>>>  		break;
>>>  	case ACB_ADAPTER_TYPE_C: {
>>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1.1 2/16 update 3] arcmsr: Adding code to support MSI-X interrupt

[ching]

Hi Tomas,

Thanks for your remind again.
The patch 17/17 will be as following.
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c	2014-05-08 17:45:34.000000000 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c	2014-05-12 10:50:28.000000000 +0800
@@ -778,12 +778,11 @@ static int arcmsr_probe(struct pci_dev *
 	}
 	error = scsi_add_host(host, &pdev->dev);
 	if(error){
-		goto RAID_controller_stop;
+		goto free_ccb_pool;
 	}
 	if (arcmsr_request_irq(pdev, acb) == ARC_FAILURE)
 		goto scsi_host_remove;
 	arcmsr_iop_init(acb);
-    	scsi_scan_host(host);
 	INIT_WORK(&acb->arcmsr_do_message_isr_bh, arcmsr_message_isr_bh_fn);
 	atomic_set(&acb->rq_map_token, 16);
 	atomic_set(&acb->ante_token_value, 16);
@@ -795,13 +794,17 @@ static int arcmsr_probe(struct pci_dev *
 	add_timer(&acb->eternal_timer);
 	if(arcmsr_alloc_sysfs_attr(acb))
 		goto out_free_sysfs;
+    	scsi_scan_host(host);
 	return 0;
 out_free_sysfs:
-scsi_host_remove:
-	scsi_remove_host(host);
-RAID_controller_stop:
+	del_timer_sync(&acb->eternal_timer);
+	flush_scheduled_work();
 	arcmsr_stop_adapter_bgrb(acb);
 	arcmsr_flush_adapter_cache(acb);
+	arcmsr_free_irq(pdev, acb);
+scsi_host_remove:
+	scsi_remove_host(host);
+free_ccb_pool:
 	arcmsr_free_ccb_pool(acb);
 free_hbb_mu:
 	arcmsr_free_mu(acb);



On Fri, 2014-05-09 at 13:15 +0200, Tomas Henzl wrote:
> On 05/08/2014 01:37 PM, ching wrote:
> > Hi Tomas,
> >
> > Thanks for your suggestion.
> > I will add a new patch 17/17 at last.
> 
> This additional patch was meant for moving the scsi_scan_host, because it's
> nor related to the ms-x interrupts. Probably  the maintainer will prefer
> a single patch which changes msi-x at once, I don't care.
> To the patch below - you should also free the allocated irqs as I wrote previously.
> 
> Cheers,
> Tomas
> 

Re: [PATCH v1.1 2/16 update 3] arcmsr: Adding code to support MSI-X interrupt

[Tomas Henzl]

On 05/12/2014 01:55 PM, ching wrote:
> Hi Tomas,
>
> Thanks for your remind again.
> The patch 17/17 will be as following.

Hi Ching,
I think we have messed up this thread a bit. Please repost the 2/16 with fixes related only to msi-x,
this is the original 2/26 patch update with fixes directly related to it - free_irq + related fixes to error path.
This should create a clear situation so James knows what he should take

For the other changes - moving scsi_scan_host + flush_work etc. either create a 17/16 patch
or  wait until the 16/16 set is taken and then post a new patch.

 out_free_sysfs:
-scsi_host_remove:
-	scsi_remove_host(host);
-RAID_controller_stop:
+	del_timer_sync(&acb->eternal_timer);
+	flush_scheduled_work();

Btw, isn't flush_work(&acb->arcmsr_d..  better here?
and if so the block looks almost identical to what is in arcmsr_shutdown,
can we use it instead?

Thanks,
Tomas

>  	arcmsr_stop_adapter_bgrb(acb);
>  	arcmsr_flush_adapter_cache(acb);
> +	arcmsr_free_irq(pdev, acb);
> +scsi_host_remove:
> +	scsi_remove_host(host);
> +free_ccb_pool:
>  	arcmsr_free_ccb_pool(acb);
>  free_hbb_mu:
>  	arcmsr_free_mu(acb);
>
>
>
> On Fri, 2014-05-09 at 13:15 +0200, Tomas Henzl wrote:
>> On 05/08/2014 01:37 PM, ching wrote:
>>> Hi Tomas,
>>>
>>> Thanks for your suggestion.
>>> I will add a new patch 17/17 at last.
>> This additional patch was meant for moving the scsi_scan_host, because it's
>> nor related to the ms-x interrupts. Probably  the maintainer will prefer
>> a single patch which changes msi-x at once, I don't care.
>> To the patch below - you should also free the allocated irqs as I wrote previously.
>>
>> Cheers,
>> Tomas
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1.1 2/16 update 3] arcmsr: Adding code to support MSI-X interrupt

[ching]

Hi Tomas,

I will repost patch1 to patch17.
Although, patch3 to patch16 are not change, but patch2 has been updated.
So, the patch line number are shifted in patch3 till patch16.

On Mon, 2014-05-12 at 15:01 +0200, Tomas Henzl wrote:
> On 05/12/2014 01:55 PM, ching wrote:
> > Hi Tomas,
> >
> > Thanks for your remind again.
> > The patch 17/17 will be as following.
> 
> Hi Ching,
> I think we have messed up this thread a bit. Please repost the 2/16 with fixes related only to msi-x,
> this is the original 2/26 patch update with fixes directly related to it - free_irq + related fixes to error path.
> This should create a clear situation so James knows what he should take
> 
> For the other changes - moving scsi_scan_host + flush_work etc. either create a 17/16 patch
> or  wait until the 16/16 set is taken and then post a new patch.
> 
>  out_free_sysfs:
> -scsi_host_remove:
> -	scsi_remove_host(host);
> -RAID_controller_stop:
> +	del_timer_sync(&acb->eternal_timer);
> +	flush_scheduled_work();
> 
> Btw, isn't flush_work(&acb->arcmsr_d..  better here?
Agree.
> and if so the block looks almost identical to what is in arcmsr_shutdown,
> can we use it instead?
I do not prefer to call shutdown at here.
> 
> Thanks,
> Tomas
> 
> >  	arcmsr_stop_adapter_bgrb(acb);
> >  	arcmsr_flush_adapter_cache(acb);
> > +	arcmsr_free_irq(pdev, acb);
> > +scsi_host_remove:
> > +	scsi_remove_host(host);
> > +free_ccb_pool:
> >  	arcmsr_free_ccb_pool(acb);
> >  free_hbb_mu:
> >  	arcmsr_free_mu(acb);
> >
> >
> >
> > On Fri, 2014-05-09 at 13:15 +0200, Tomas Henzl wrote:
> >> On 05/08/2014 01:37 PM, ching wrote:
> >>> Hi Tomas,
> >>>
> >>> Thanks for your suggestion.
> >>> I will add a new patch 17/17 at last.
> >> This additional patch was meant for moving the scsi_scan_host, because it's
> >> nor related to the ms-x interrupts. Probably  the maintainer will prefer
> >> a single patch which changes msi-x at once, I don't care.
> >> To the patch below - you should also free the allocated irqs as I wrote previously.
> >>
> >> Cheers,
> >> Tomas
> >>
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>