diff mbox

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

Message ID 1298289514-15671-2-git-send-email-amit.salecha@qlogic.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

amit salecha Feb. 21, 2011, 11:58 a.m. UTC 
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(-)

Comments

David Miller Feb. 22, 2011, 6:31 p.m. UTC | #1 
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.
--
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
diff mbox

Patch

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");