Patchwork [v2,net-next,5/8] qlcnic: Cleanup of structure qlcnic_hardware_context

login
register
mail settings
Submitter Jitendra Kalsaria
Date June 22, 2013, 8:12 a.m.
Message ID <1371888727-16422-6-git-send-email-jitendra.kalsaria@qlogic.com>
Download mbox | patch
Permalink /patch/253361/
State Accepted
Delegated to: David Miller
Headers show

Comments

Jitendra Kalsaria - June 22, 2013, 8:12 a.m.
From: Pratik Pujar <pratik.pujar@qlogic.com>

Signed-off-by: Pratik Pujar <pratik.pujar@qlogic.com>
Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h       |    2 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c    |    3 ++-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c  |    4 ++--
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c |    2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)
Joe Perches - June 22, 2013, 10:53 a.m.
On Sat, 2013-06-22 at 04:12 -0400, Jitendra Kalsaria wrote:
> From: Pratik Pujar <pratik.pujar@qlogic.com>

This isn't really a cleanup.
It's allowing more capabilities.

> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
[]
> @@ -449,7 +449,7 @@ struct qlcnic_hardware_context {
>  	u16 max_pci_func;
>  
>  	u32 capabilities;
> -	u32 capabilities2;
> +	u32 extra_capability[3];

Also, it looks like it's used as a bitfield.

I suggest instead using an array of ulongs and
something like tg3's use of tg3_flag_set|tg3_flag_clear.


--
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
Pratik Pujar - June 24, 2013, 6:17 p.m.
>-----Original Message-----
>From: Joe Perches [mailto:joe@perches.com]
>Sent: 22 June 2013 16:24
>To: Jitendra Kalsaria
>Cc: David Miller; netdev; Sony Chacko; Shahed Shaikh; Dept-NX Linux NIC
>Driver; Pratik Pujar
>Subject: Re: [PATCH v2 net-next 5/8] qlcnic: Cleanup of structure
>qlcnic_hardware_context
>
>On Sat, 2013-06-22 at 04:12 -0400, Jitendra Kalsaria wrote:
>> From: Pratik Pujar <pratik.pujar@qlogic.com>
>
>This isn't really a cleanup.
>It's allowing more capabilities.

The driver was dealing with extra capabilities but separate variables were used for these. Now instead of using separate variables, clubbed in a single array. 

>
>> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
>> b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
>[]
>> @@ -449,7 +449,7 @@ struct qlcnic_hardware_context {
>>  	u16 max_pci_func;
>>
>>  	u32 capabilities;
>> -	u32 capabilities2;
>> +	u32 extra_capability[3];
>
>Also, it looks like it's used as a bitfield.
>
>I suggest instead using an array of ulongs and something like tg3's use of
>tg3_flag_set|tg3_flag_clear.
>
>

The extra_capability is read-only field. Driver does not perform any set/clear operations on this.

Thanks,
Pratik.

--
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
Joe Perches - June 24, 2013, 6:19 p.m.
On Mon, 2013-06-24 at 18:17 +0000, Pratik Pujar wrote:
> >-----Original Message-----
> >From: Joe Perches [mailto:joe@perches.com]
> >Sent: 22 June 2013 16:24
> >To: Jitendra Kalsaria
> >Cc: David Miller; netdev; Sony Chacko; Shahed Shaikh; Dept-NX Linux NIC
> >Driver; Pratik Pujar
> >Subject: Re: [PATCH v2 net-next 5/8] qlcnic: Cleanup of structure
> >qlcnic_hardware_context
> >
> >On Sat, 2013-06-22 at 04:12 -0400, Jitendra Kalsaria wrote:
> >> From: Pratik Pujar <pratik.pujar@qlogic.com>
> >
> >This isn't really a cleanup.
> >It's allowing more capabilities.
> 
> The driver was dealing with extra capabilities but separate variables
> were used for these. Now instead of using separate variables, clubbed
> in a single array. 

What was 2 is now 3 no?

> >> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
> >> b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
> >[]
> >> @@ -449,7 +449,7 @@ struct qlcnic_hardware_context {
> >>  	u16 max_pci_func;
> >>
> >>  	u32 capabilities;
> >> -	u32 capabilities2;
> >> +	u32 extra_capability[3];


--
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
Pratik Pujar - June 25, 2013, 5:04 a.m.
>-----Original Message-----
>From: Joe Perches [mailto:joe@perches.com]
>Sent: 24 June 2013 23:49
>To: Pratik Pujar
>Cc: Jitendra Kalsaria; David Miller; netdev; Sony Chacko; Shahed Shaikh; Dept-
>NX Linux NIC Driver
>Subject: Re: [PATCH v2 net-next 5/8] qlcnic: Cleanup of structure
>qlcnic_hardware_context
>
>On Mon, 2013-06-24 at 18:17 +0000, Pratik Pujar wrote:
>> >-----Original Message-----
>> >From: Joe Perches [mailto:joe@perches.com]
>> >Sent: 22 June 2013 16:24
>> >To: Jitendra Kalsaria
>> >Cc: David Miller; netdev; Sony Chacko; Shahed Shaikh; Dept-NX Linux
>> >NIC Driver; Pratik Pujar
>> >Subject: Re: [PATCH v2 net-next 5/8] qlcnic: Cleanup of structure
>> >qlcnic_hardware_context
>> >
>> >On Sat, 2013-06-22 at 04:12 -0400, Jitendra Kalsaria wrote:
>> >> From: Pratik Pujar <pratik.pujar@qlogic.com>
>> >
>> >This isn't really a cleanup.
>> >It's allowing more capabilities.
>>
>> The driver was dealing with extra capabilities but separate variables
>> were used for these. Now instead of using separate variables, clubbed
>> in a single array.
>
>What was 2 is now 3 no?
>

Yes, adapter chip exposes extra capabilities through 3 words. Driver uses only 1, as others are reserved as of now. So array extra_capability[3] is a provision for future use of these extra capabilities. 
I will change the description to structure cleanup + provision for future use of extra capabilities. 

>> >> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
>> >> b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
>> >[]
>> >> @@ -449,7 +449,7 @@ struct qlcnic_hardware_context {
>> >>  	u16 max_pci_func;
>> >>
>> >>  	u32 capabilities;
>> >> -	u32 capabilities2;
>> >> +	u32 extra_capability[3];
>
>

Thanks,
Pratik.

--
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 cc2c2c1..5694d59 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -449,7 +449,7 @@  struct qlcnic_hardware_context {
 	u16 max_pci_func;
 
 	u32 capabilities;
-	u32 capabilities2;
+	u32 extra_capability[3];
 	u32 temp;
 	u32 int_vec_bit;
 	u32 fw_hal_version;
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
index e7f305d..9fcbfd4 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
@@ -786,7 +786,8 @@  int qlcnic_82xx_config_hw_lro(struct qlcnic_adapter *adapter, int enable)
 	word = 0;
 	if (enable) {
 		word = QLCNIC_ENABLE_IPV4_LRO | QLCNIC_NO_DEST_IPV4_CHECK;
-		if (adapter->ahw->capabilities2 & QLCNIC_FW_CAP2_HW_LRO_IPV6)
+		if (adapter->ahw->extra_capability[0] &
+		    QLCNIC_FW_CAP2_HW_LRO_IPV6)
 			word |= QLCNIC_ENABLE_IPV6_LRO |
 				QLCNIC_NO_DEST_IPV6_CHECK;
 	}
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 8e1453a..3963e78 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -994,7 +994,7 @@  qlcnic_initialize_nic(struct qlcnic_adapter *adapter)
 	if (adapter->ahw->capabilities & QLCNIC_FW_CAPABILITY_MORE_CAPS) {
 		u32 temp;
 		temp = QLCRD32(adapter, CRB_FW_CAPABILITIES_2);
-		adapter->ahw->capabilities2 = temp;
+		adapter->ahw->extra_capability[0] = temp;
 	}
 	adapter->ahw->max_mac_filters = nic_info.max_mac_filters;
 	adapter->ahw->max_mtu = nic_info.max_mtu;
@@ -1486,7 +1486,7 @@  static void qlcnic_get_lro_mss_capability(struct qlcnic_adapter *adapter)
 	u32 capab = 0;
 
 	if (qlcnic_82xx_check(adapter)) {
-		if (adapter->ahw->capabilities2 &
+		if (adapter->ahw->extra_capability[0] &
 		    QLCNIC_FW_CAPABILITY_2_LRO_MAX_TCP_SEG)
 			adapter->flags |= QLCNIC_FW_LRO_MSS_CAP;
 	} else {
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
index 7ec030a..10ed82b 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
@@ -168,7 +168,7 @@  static int qlcnic_82xx_store_beacon(struct qlcnic_adapter *adapter,
 	if (err)
 		return err;
 
-	if ((ahw->capabilities2 & QLCNIC_FW_CAPABILITY_2_BEACON)) {
+	if (ahw->extra_capability[0] & QLCNIC_FW_CAPABILITY_2_BEACON) {
 		err = qlcnic_get_beacon_state(adapter, &h_beacon_state);
 		if (!err) {
 			dev_info(&adapter->pdev->dev,