[PATCH NEXT 0/2]qlcnic: minor fixes

[PATCH NEXT 0/2]qlcnic: minor fixes

[Amit Kumar Salecha]

Hi,
 Series of 2 minor fixes, apply them on net-next.
 Accidentally 2nd patch sent twice, please ignore.

-Amit

[PATCH NEXT 1/2] qlcnic: fix type of module parameters

[Amit Kumar Salecha]

o Module parameters auto_fw_reset, use_msi, use_msi_x, qlcnic_mac_learn,
  and load_fw_file should be of type bool not int.
o All module parameters should have qlcnic prefix.
o Remove unnecessary macro for value "1".

Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/qlcnic/qlcnic.h      |    1 -
 drivers/net/qlcnic/qlcnic_main.c |   46 +++++++++++++++++++-------------------
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index 44e316f..fa7f794 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -867,7 +867,6 @@ struct qlcnic_nic_intr_coalesce {
 #define LINKEVENT_LINKSPEED_MBPS	0
 #define LINKEVENT_LINKSPEED_ENCODED	1
 
-#define AUTO_FW_RESET_ENABLED	0x01
 /* firmware response header:
  *	63:58 - message type
  *	57:56 - owner
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 37c04b4..3fd878c 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -30,25 +30,26 @@ static const char qlcnic_driver_string[] = "QLogic 1/10 GbE "
 	"Converged/Intelligent Ethernet Driver v" QLCNIC_LINUX_VERSIONID;
 
 static struct workqueue_struct *qlcnic_wq;
-static int qlcnic_mac_learn;
-module_param(qlcnic_mac_learn, int, 0444);
+static bool qlcnic_mac_learn;
+module_param(qlcnic_mac_learn, bool, 0444);
 MODULE_PARM_DESC(qlcnic_mac_learn, "Mac Filter (0=disabled, 1=enabled)");
 
-static int use_msi = 1;
-module_param(use_msi, int, 0444);
-MODULE_PARM_DESC(use_msi, "MSI interrupt (0=disabled, 1=enabled");
+static bool qlcnic_use_msi = 1;
+module_param(qlcnic_use_msi, bool, 0444);
+MODULE_PARM_DESC(qlcnic_use_msi, "MSI interrupt (0=disabled, 1=enabled");
 
-static int use_msi_x = 1;
-module_param(use_msi_x, int, 0444);
-MODULE_PARM_DESC(use_msi_x, "MSI-X interrupt (0=disabled, 1=enabled");
+static bool qlcnic_use_msi_x = 1;
+module_param(qlcnic_use_msi_x, bool, 0444);
+MODULE_PARM_DESC(qlcnic_use_msi_x, "MSI-X interrupt (0=disabled, 1=enabled");
 
-static int auto_fw_reset = AUTO_FW_RESET_ENABLED;
-module_param(auto_fw_reset, int, 0644);
-MODULE_PARM_DESC(auto_fw_reset, "Auto firmware reset (0=disabled, 1=enabled");
+static bool qlcnic_auto_fw_reset = 1;
+module_param(qlcnic_auto_fw_reset, bool, 0644);
+MODULE_PARM_DESC(qlcnic_auto_fw_reset,
+		 "Auto firmware reset (0=disabled, 1=enabled");
 
-static int load_fw_file;
-module_param(load_fw_file, int, 0444);
-MODULE_PARM_DESC(load_fw_file, "Load firmware from (0=flash, 1=file");
+static bool qlcnic_load_fw_file;
+module_param(qlcnic_load_fw_file, bool, 0444);
+MODULE_PARM_DESC(qlcnic_load_fw_file, "Load firmware from (0=flash, 1=file");
 
 static int qlcnic_config_npars;
 module_param(qlcnic_config_npars, int, 0444);
@@ -404,7 +405,7 @@ qlcnic_setup_intr(struct qlcnic_adapter *adapter)
 		/* fall through for msi */
 	}
 
-	if (use_msi && !pci_enable_msi(pdev)) {
+	if (qlcnic_use_msi && !pci_enable_msi(pdev)) {
 		adapter->flags |= QLCNIC_MSI_ENABLED;
 		adapter->tgt_status_reg = qlcnic_get_ioaddr(adapter,
 				msi_tgt_status[adapter->ahw.pci_func]);
@@ -658,8 +659,8 @@ qlcnic_check_options(struct qlcnic_adapter *adapter)
 		adapter->max_rxd = MAX_RCV_DESCRIPTORS_1G;
 	}
 
-	adapter->msix_supported = !!use_msi_x;
-	adapter->rss_supported = !!use_msi_x;
+	adapter->msix_supported = qlcnic_use_msi_x;
+	adapter->rss_supported = qlcnic_use_msi_x;
 
 	adapter->num_txd = MAX_CMD_DESCRIPTORS;
 
@@ -972,7 +973,7 @@ qlcnic_start_firmware(struct qlcnic_adapter *adapter)
 	else if (!err)
 		goto check_fw_status;
 
-	if (load_fw_file)
+	if (qlcnic_load_fw_file)
 		qlcnic_request_firmware(adapter);
 	else {
 		err = qlcnic_check_flash_fw_ver(adapter);
@@ -2959,8 +2960,7 @@ qlcnic_check_health(struct qlcnic_adapter *adapter)
 		if (adapter->need_fw_reset)
 			goto detach;
 
-		if (adapter->reset_context &&
-		    auto_fw_reset == AUTO_FW_RESET_ENABLED) {
+		if (adapter->reset_context && qlcnic_auto_fw_reset) {
 			qlcnic_reset_hw_context(adapter);
 			adapter->netdev->trans_start = jiffies;
 		}
@@ -2973,7 +2973,7 @@ qlcnic_check_health(struct qlcnic_adapter *adapter)
 
 	qlcnic_dev_request_reset(adapter);
 
-	if ((auto_fw_reset == AUTO_FW_RESET_ENABLED))
+	if (qlcnic_auto_fw_reset)
 		clear_bit(__QLCNIC_FW_ATTACHED, &adapter->state);
 
 	dev_info(&netdev->dev, "firmware hang detected\n");
@@ -2982,8 +2982,8 @@ detach:
 	adapter->dev_state = (state == QLCNIC_DEV_NEED_QUISCENT) ? state :
 		QLCNIC_DEV_NEED_RESET;
 
-	if ((auto_fw_reset == AUTO_FW_RESET_ENABLED) &&
-		!test_and_set_bit(__QLCNIC_RESETTING, &adapter->state)) {
+	if (qlcnic_auto_fw_reset &&
+	    !test_and_set_bit(__QLCNIC_RESETTING, &adapter->state)) {
 
 		qlcnic_schedule_work(adapter, qlcnic_detach_work, 0);
 		QLCDB(adapter, DRV, "fw recovery scheduled.\n");
-- 
1.7.3.2

Re: [PATCH NEXT 1/2] qlcnic: fix type of module parameters

[David Miller]

From: Amit Kumar Salecha <amit.salecha@qlogic.com>
Date: Mon, 21 Feb 2011 03:58:33 -0800

> o Module parameters auto_fw_reset, use_msi, use_msi_x, qlcnic_mac_learn,
>   and load_fw_file should be of type bool not int.
> o All module parameters should have qlcnic prefix.
> o Remove unnecessary macro for value "1".
> 
> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>

You must not change module parameter names on a whim, as this will
break scripts.

When you create a module parameter in your driver, you are creating
something users will use, and therefore an API.  You therefore
cannot change it without breaking stuff.

This is yet another reason I scream at anyone who adds module
parameters to network drivers, they are always "the wrong thing
to do".

None of these values you guys have module parameter for in the
qlcnic driver are providing facilities that are really qlcnic
specific at all.

MSI-X enablement, firmware loading controls, etc.  All of this
stuff is generic and would be potentially necessary in any device
driver, not just qlcnic's.

Therefore generic facilities are where this stuff should be
implemented, instead of in driver specific module parameters.

I will not apply these patches, sorry.

[PATCH NEXT 2/2] qlcnic: Remove validation for max tx and max rx queues

[Amit Kumar Salecha]

From: Rajesh Borundia <rajesh.borundia@qlogic.com>

Max rx queues and tx queues are governed by fimware.
So driver should not validate these values.

Signed-off-by: Rajesh Borundia <rajesh.borundia@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/qlcnic/qlcnic.h      |    4 ----
 drivers/net/qlcnic/qlcnic_main.c |    6 ++----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index fa7f794..dc44564 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -1132,14 +1132,10 @@ struct qlcnic_eswitch {
 #define MAX_BW			100	/* % of link speed */
 #define MAX_VLAN_ID		4095
 #define MIN_VLAN_ID		2
-#define MAX_TX_QUEUES		1
-#define MAX_RX_QUEUES		4
 #define DEFAULT_MAC_LEARN	1
 
 #define IS_VALID_VLAN(vlan)	(vlan >= MIN_VLAN_ID && vlan < MAX_VLAN_ID)
 #define IS_VALID_BW(bw)		(bw <= MAX_BW)
-#define IS_VALID_TX_QUEUES(que)	(que > 0 && que <= MAX_TX_QUEUES)
-#define IS_VALID_RX_QUEUES(que)	(que > 0 && que <= MAX_RX_QUEUES)
 
 struct qlcnic_pci_func_cfg {
 	u16	func_type;
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 3fd878c..92e6c62 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -3654,10 +3654,8 @@ validate_npar_config(struct qlcnic_adapter *adapter,
 		if (adapter->npars[pci_func].type != QLCNIC_TYPE_NIC)
 			return QL_STATUS_INVALID_PARAM;
 
-		if (!IS_VALID_BW(np_cfg[i].min_bw)
-				|| !IS_VALID_BW(np_cfg[i].max_bw)
-				|| !IS_VALID_RX_QUEUES(np_cfg[i].max_rx_queues)
-				|| !IS_VALID_TX_QUEUES(np_cfg[i].max_tx_queues))
+		if (!IS_VALID_BW(np_cfg[i].min_bw) ||
+		    !IS_VALID_BW(np_cfg[i].max_bw))
 			return QL_STATUS_INVALID_PARAM;
 	}
 	return 0;
-- 
1.7.3.2