Patchwork [net-next,5/7] qlcnic: Rename the IRQ description.

login
register
mail settings
Submitter Jitendra Kalsaria
Date April 24, 2013, 10:42 p.m.
Message ID <1366843365-2787-6-git-send-email-jitendra.kalsaria@qlogic.com>
Download mbox | patch
Permalink /patch/239335/
State Accepted
Delegated to: David Miller
Headers show

Comments

Jitendra Kalsaria - April 24, 2013, 10:42 p.m.
From: Himanshu Madhani <himanshu.madhani@qlogic.com>

Here's what modified vectors will look like
in the /proc/interrupts

        MSIx             	INTx
-----------------------------------------
83xx    qlcnic[MB]
	qlcnic-ethX[Rx0]
	qlcnic-ethX[Rx1]
	..
	qlcnic-ethX[RxN]
	qlcnic-ethx[Tx0]        qlcnic[MB+Tx0+Rx0]

82xx    qlcnic-ethX[Rx0]
	qlcnic-ethX[Rx1]
	..
	qlcnic-ethX[Tx0+RxN]    qlcnic-ethX[Tx0+Rx0]

Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
Signed-off-by: Shahed Shaikh <shahed.shaikh@qlogic.com>
Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h      |    4 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |   23 +++++++++++++++------
 2 files changed, 18 insertions(+), 9 deletions(-)
Ben Hutchings - April 25, 2013, 12:05 a.m.
On Wed, 2013-04-24 at 18:42 -0400, Jitendra Kalsaria wrote:
> From: Himanshu Madhani <himanshu.madhani@qlogic.com>
> 
> Here's what modified vectors will look like
> in the /proc/interrupts
> 
>         MSIx             	INTx
> -----------------------------------------
> 83xx    qlcnic[MB]
> 	qlcnic-ethX[Rx0]
> 	qlcnic-ethX[Rx1]
> 	..
> 	qlcnic-ethX[RxN]
> 	qlcnic-ethx[Tx0]        qlcnic[MB+Tx0+Rx0]
> 
> 82xx    qlcnic-ethX[Rx0]
> 	qlcnic-ethX[Rx1]
> 	..
> 	qlcnic-ethX[Tx0+RxN]    qlcnic-ethX[Tx0+Rx0]
[...]

Many other multiqueue drivers (including your own qlge) use these
formats for IRQ handler names:

RX queue:   <device>-rx-<index>
TX queue:   <device>-tx-<index>
Queue pair: <device>-<index>

Please use similar formats.

Ben.
Jitendra Kalsaria - April 25, 2013, 11:11 p.m.
On 4/24/13 5:05 PM, "Ben Hutchings" <bhutchings@solarflare.com> wrote:

>On Wed, 2013-04-24 at 18:42 -0400, Jitendra Kalsaria wrote:
>> From: Himanshu Madhani <himanshu.madhani@qlogic.com>
>> 
>> Here's what modified vectors will look like
>> in the /proc/interrupts
>> 
>>         MSIx             	INTx
>> -----------------------------------------
>> 83xx    qlcnic[MB]
>> 	qlcnic-ethX[Rx0]
>> 	qlcnic-ethX[Rx1]
>> 	..
>> 	qlcnic-ethX[RxN]
>> 	qlcnic-ethx[Tx0]        qlcnic[MB+Tx0+Rx0]
>> 
>> 82xx    qlcnic-ethX[Rx0]
>> 	qlcnic-ethX[Rx1]
>> 	..
>> 	qlcnic-ethX[Tx0+RxN]    qlcnic-ethX[Tx0+Rx0]
>[...]
>
>Many other multiqueue drivers (including your own qlge) use these
>formats for IRQ handler names:

Thanks for the review. We will change the format in a future submission
according to your suggestion and similar to what other multiqueue drivers
are using.
>
>RX queue:   <device>-rx-<index>
>TX queue:   <device>-tx-<index>
>Queue pair: <device>-<index>

We will like to add the qlcnic prefix to the above format, as it makes it
easier for an end user to identify which driver these entries belongs to,
without having to run addition command.
This is how the format will look like:

       MSIx                    INTx
               -----------------------------------------
83xx   qlcnic-MB
       qlcnic-ethX-rx-0
       qlcnic-ethX-rx-1
       ..
       qlcnic-ethX-rx-N
       qlcnic-ethx-tx-0        qlcnic-MB-tx-0-rx-0
 
82xx   qlcnic-ethX-rx-0
       qlcnic-ethX-rx-1
       ..
       qlcnic-ethX-tx-0-rx-N   qlcnic-ethX-tx-0-rx-0

Thanks,

   Jiten
>
>Please use similar formats.
>
>Ben.
>
>-- 
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>
>


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 25, 2013, 11:21 p.m.
From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Date: Thu, 25 Apr 2013 23:11:05 +0000

> We will like to add the qlcnic prefix to the above format, as it makes it
> easier for an end user to identify which driver these entries belongs to,
> without having to run addition command.

This is illogical, and wrong.

By being different, you are making it MORE DIFFICULT for users because
they have to specially accomodate your unique conventions.

Please follow what other drivers do.

If you disagree with what the convention is, start a discussion about
changing it, rather than doing whatever you feel like doing in your
drivers.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jitendra Kalsaria - April 26, 2013, 12:35 a.m.
On 4/25/13 4:21 PM, "David Miller" <davem@davemloft.net> wrote:

>From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
>Date: Thu, 25 Apr 2013 23:11:05 +0000
>
>> We will like to add the qlcnic prefix to the above format, as it makes
>>it
>> easier for an end user to identify which driver these entries belongs
>>to,
>> without having to run addition command.
>
>This is illogical, and wrong.
>
>By being different, you are making it MORE DIFFICULT for users because
>they have to specially accomodate your unique conventions.
>
>Please follow what other drivers do.
>
>If you disagree with what the convention is, start a discussion about
>changing it, rather than doing whatever you feel like doing in your
>drivers.

We will follow the convention and make the changes in a future submission.


Thanks,
    Jiten


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 767c683..b2206db 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -519,13 +519,13 @@  struct qlcnic_host_sds_ring {
 	int irq;
 
 	dma_addr_t phys_addr;
-	char name[IFNAMSIZ+4];
+	char name[IFNAMSIZ + 12];
 } ____cacheline_internodealigned_in_smp;
 
 struct qlcnic_host_tx_ring {
 	int irq;
 	void __iomem *crb_intr_mask;
-	char name[IFNAMSIZ+4];
+	char name[IFNAMSIZ + 12];
 	u16 ctx_id;
 	u32 producer;
 	u32 sw_consumer;
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 4e0bcb1..1310b7b 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -1287,7 +1287,7 @@  qlcnic_request_irq(struct qlcnic_adapter *adapter)
 	irq_handler_t handler;
 	struct qlcnic_host_sds_ring *sds_ring;
 	struct qlcnic_host_tx_ring *tx_ring;
-	int err, ring;
+	int err, ring, num_sds_rings;
 
 	unsigned long flags = 0;
 	struct net_device *netdev = adapter->netdev;
@@ -1318,10 +1318,20 @@  qlcnic_request_irq(struct qlcnic_adapter *adapter)
 		if (qlcnic_82xx_check(adapter) ||
 		    (qlcnic_83xx_check(adapter) &&
 		     (adapter->flags & QLCNIC_MSIX_ENABLED))) {
-			for (ring = 0; ring < adapter->max_sds_rings; ring++) {
+			num_sds_rings = adapter->max_sds_rings;
+			for (ring = 0; ring < num_sds_rings; ring++) {
 				sds_ring = &recv_ctx->sds_rings[ring];
-				snprintf(sds_ring->name, sizeof(int) + IFNAMSIZ,
-					 "%s[%d]", netdev->name, ring);
+				if (qlcnic_82xx_check(adapter) &&
+				    (ring == (num_sds_rings - 1)))
+					snprintf(sds_ring->name,
+						 sizeof(sds_ring->name),
+						 "qlcnic-%s[Tx0+Rx%d]",
+						 netdev->name, ring);
+				else
+					snprintf(sds_ring->name,
+						 sizeof(sds_ring->name),
+						 "qlcnic-%s[Rx%d]",
+						 netdev->name, ring);
 				err = request_irq(sds_ring->irq, handler, flags,
 						  sds_ring->name, sds_ring);
 				if (err)
@@ -1335,9 +1345,8 @@  qlcnic_request_irq(struct qlcnic_adapter *adapter)
 			for (ring = 0; ring < adapter->max_drv_tx_rings;
 			     ring++) {
 				tx_ring = &adapter->tx_ring[ring];
-				snprintf(tx_ring->name, sizeof(int) + IFNAMSIZ,
-					 "%s[%d]", netdev->name,
-					 adapter->max_sds_rings + ring);
+				snprintf(tx_ring->name, sizeof(tx_ring->name),
+					 "qlcnic-%s[Tx%d]", netdev->name, ring);
 				err = request_irq(tx_ring->irq, handler, flags,
 						  tx_ring->name, tx_ring);
 				if (err)