diff mbox

[NEXT,2/2] netxen: support for GbE port settings

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

Commit Message

amit salecha March 11, 2011, 9:50 a.m. UTC
From: Sony Chacko <sony.chacko@qlogic.com>

o Allow setting speed and auto negotiation parameters for GbE ports.
o Log an error message to indicate duplex setting is not supported in
  the hardware currently.

Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/netxen/netxen_nic.h         |    6 ++-
 drivers/net/netxen/netxen_nic_ctx.c     |   15 +++++++
 drivers/net/netxen/netxen_nic_ethtool.c |   69 ++++++++++--------------------
 3 files changed, 43 insertions(+), 47 deletions(-)

Comments

David Miller March 14, 2011, 10:15 p.m. UTC | #1
From: Amit Kumar Salecha <amit.salecha@qlogic.com>
Date: Fri, 11 Mar 2011 01:50:03 -0800

> From: Sony Chacko <sony.chacko@qlogic.com>
> 
> o Allow setting speed and auto negotiation parameters for GbE ports.
> o Log an error message to indicate duplex setting is not supported in
>   the hardware currently.
> 
> Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>

The kernel log is not the place to transmit this information.

Please work with Ben Hutchings who is working for ways to report
this to the user in the proper location, via ethtool.
--
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
Ben Hutchings March 14, 2011, 10:30 p.m. UTC | #2
On Mon, 2011-03-14 at 15:15 -0700, David Miller wrote:
> From: Amit Kumar Salecha <amit.salecha@qlogic.com>
> Date: Fri, 11 Mar 2011 01:50:03 -0800
> 
> > From: Sony Chacko <sony.chacko@qlogic.com>
> > 
> > o Allow setting speed and auto negotiation parameters for GbE ports.
> > o Log an error message to indicate duplex setting is not supported in
> >   the hardware currently.
> > 
> > Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
> > Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
> 
> The kernel log is not the place to transmit this information.
> 
> Please work with Ben Hutchings who is working for ways to report
> this to the user in the proper location, via ethtool.

I am?  Well, patches welcome, at least.

Ben.
David Miller March 14, 2011, 10:32 p.m. UTC | #3
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 14 Mar 2011 22:30:22 +0000

> On Mon, 2011-03-14 at 15:15 -0700, David Miller wrote:
>> From: Amit Kumar Salecha <amit.salecha@qlogic.com>
>> Date: Fri, 11 Mar 2011 01:50:03 -0800
>> 
>> > From: Sony Chacko <sony.chacko@qlogic.com>
>> > 
>> > o Allow setting speed and auto negotiation parameters for GbE ports.
>> > o Log an error message to indicate duplex setting is not supported in
>> >   the hardware currently.
>> > 
>> > Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
>> > Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
>> 
>> The kernel log is not the place to transmit this information.
>> 
>> Please work with Ben Hutchings who is working for ways to report
>> this to the user in the proper location, via ethtool.
> 
> I am?  Well, patches welcome, at least.

You were discussing a scheme, I think with Intel folks, about letting
such changes through then making ethtool re-read the settings and see
which changes were not integrated by the driver.

Then you'd emit a diagnostic from ethtool over stderr to report any
such discrepencies.
--
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
Ben Hutchings March 14, 2011, 10:44 p.m. UTC | #4
On Mon, 2011-03-14 at 15:32 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Mon, 14 Mar 2011 22:30:22 +0000
> 
> > On Mon, 2011-03-14 at 15:15 -0700, David Miller wrote:
> >> From: Amit Kumar Salecha <amit.salecha@qlogic.com>
> >> Date: Fri, 11 Mar 2011 01:50:03 -0800
> >> 
> >> > From: Sony Chacko <sony.chacko@qlogic.com>
> >> > 
> >> > o Allow setting speed and auto negotiation parameters for GbE ports.
> >> > o Log an error message to indicate duplex setting is not supported in
> >> >   the hardware currently.
> >> > 
> >> > Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
> >> > Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
> >> 
> >> The kernel log is not the place to transmit this information.
> >> 
> >> Please work with Ben Hutchings who is working for ways to report
> >> this to the user in the proper location, via ethtool.
> > 
> > I am?  Well, patches welcome, at least.
> 
> You were discussing a scheme, I think with Intel folks, about letting
> such changes through then making ethtool re-read the settings and see
> which changes were not integrated by the driver.
> 
> Then you'd emit a diagnostic from ethtool over stderr to report any
> such discrepencies.

Only for offload settings, where the API makes it clear which flags the
user is deliberately setting and the kernel or driver can reasonably fix
up others.  For link settings, that isn't clear, and drivers should
simply reject invalid combinations.

Ben.
David Miller March 14, 2011, 10:54 p.m. UTC | #5
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 14 Mar 2011 22:44:08 +0000

> For link settings, that isn't clear, and drivers should simply
> reject invalid combinations.

Ok, makes sense.

Amit please update your patch to silently reject link setting attempts
that are unsupported by the device.

Thanks.
--
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/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index a113805..d7299f1 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -739,7 +739,8 @@  struct netxen_recv_context {
 #define NX_CDRP_CMD_READ_PEXQ_PARAMETERS	0x0000001c
 #define NX_CDRP_CMD_GET_LIC_CAPABILITIES	0x0000001d
 #define NX_CDRP_CMD_READ_MAX_LRO_PER_BOARD	0x0000001e
-#define NX_CDRP_CMD_MAX				0x0000001f
+#define NX_CDRP_CMD_CONFIG_GBE_PORT		0x0000001f
+#define NX_CDRP_CMD_MAX				0x00000020
 
 #define NX_RCODE_SUCCESS		0
 #define NX_RCODE_NO_HOST_MEM		1
@@ -1054,6 +1055,7 @@  typedef struct {
 #define NX_FW_CAPABILITY_BDG			(1 << 8)
 #define NX_FW_CAPABILITY_FVLANTX		(1 << 9)
 #define NX_FW_CAPABILITY_HW_LRO			(1 << 10)
+#define NX_FW_CAPABILITY_GBE_LINK_CFG		(1 << 11)
 
 /* module types */
 #define LINKEVENT_MODULE_NOT_PRESENT			1
@@ -1349,6 +1351,8 @@  void netxen_advert_link_change(struct netxen_adapter *adapter, int linkup);
 void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *);
 void netxen_pci_camqm_write_2M(struct netxen_adapter *, u64, u64);
 
+int nx_fw_cmd_set_gbe_port(struct netxen_adapter *adapter,
+				u32 speed, u32 duplex, u32 autoneg);
 int nx_fw_cmd_set_mtu(struct netxen_adapter *adapter, int mtu);
 int netxen_nic_change_mtu(struct net_device *netdev, int new_mtu);
 int netxen_config_hw_lro(struct netxen_adapter *adapter, int enable);
diff --git a/drivers/net/netxen/netxen_nic_ctx.c b/drivers/net/netxen/netxen_nic_ctx.c
index f7d06cb..f16966a 100644
--- a/drivers/net/netxen/netxen_nic_ctx.c
+++ b/drivers/net/netxen/netxen_nic_ctx.c
@@ -112,6 +112,21 @@  nx_fw_cmd_set_mtu(struct netxen_adapter *adapter, int mtu)
 	return 0;
 }
 
+int
+nx_fw_cmd_set_gbe_port(struct netxen_adapter *adapter,
+			u32 speed, u32 duplex, u32 autoneg)
+{
+
+	return netxen_issue_cmd(adapter,
+				adapter->ahw.pci_func,
+				NXHAL_VERSION,
+				speed,
+				duplex,
+				autoneg,
+				NX_CDRP_CMD_CONFIG_GBE_PORT);
+
+}
+
 static int
 nx_fw_cmd_create_rx_ctx(struct netxen_adapter *adapter)
 {
diff --git a/drivers/net/netxen/netxen_nic_ethtool.c b/drivers/net/netxen/netxen_nic_ethtool.c
index 587498e..481f331 100644
--- a/drivers/net/netxen/netxen_nic_ethtool.c
+++ b/drivers/net/netxen/netxen_nic_ethtool.c
@@ -214,7 +214,6 @@  skip:
 			check_sfp_module = netif_running(dev) &&
 				adapter->has_link_events;
 		} else {
-			ecmd->autoneg = AUTONEG_ENABLE;
 			ecmd->supported |= (SUPPORTED_TP |SUPPORTED_Autoneg);
 			ecmd->advertising |=
 				(ADVERTISED_TP | ADVERTISED_Autoneg);
@@ -252,53 +251,31 @@  static int
 netxen_nic_set_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
 {
 	struct netxen_adapter *adapter = netdev_priv(dev);
-	__u32 status;
+	int ret;
 
-	/* read which mode */
-	if (adapter->ahw.port_type == NETXEN_NIC_GBE) {
-		/* autonegotiation */
-		if (adapter->phy_write &&
-		    adapter->phy_write(adapter,
-				       NETXEN_NIU_GB_MII_MGMT_ADDR_AUTONEG,
-				       ecmd->autoneg) != 0)
-			return -EIO;
-		else
-			adapter->link_autoneg = ecmd->autoneg;
+	if (adapter->ahw.port_type != NETXEN_NIC_GBE)
+		return -EOPNOTSUPP;
 
-		if (adapter->phy_read &&
-		    adapter->phy_read(adapter,
-				      NETXEN_NIU_GB_MII_MGMT_ADDR_PHY_STATUS,
-				      &status) != 0)
-			return -EIO;
+	if (!(adapter->capabilities & NX_FW_CAPABILITY_GBE_LINK_CFG)) {
+		netdev_info(dev, "Firmware upgrade required to "
+				"support this operation\n");
+		return -EOPNOTSUPP;
+	}
 
-		/* speed */
-		switch (ecmd->speed) {
-		case SPEED_10:
-			netxen_set_phy_speed(status, 0);
-			break;
-		case SPEED_100:
-			netxen_set_phy_speed(status, 1);
-			break;
-		case SPEED_1000:
-			netxen_set_phy_speed(status, 2);
-			break;
-		}
-		/* set duplex mode */
-		if (ecmd->duplex == DUPLEX_HALF)
-			netxen_clear_phy_duplex(status);
-		if (ecmd->duplex == DUPLEX_FULL)
-			netxen_set_phy_duplex(status);
-		if (adapter->phy_write &&
-		    adapter->phy_write(adapter,
-				       NETXEN_NIU_GB_MII_MGMT_ADDR_PHY_STATUS,
-				       *((int *)&status)) != 0)
-			return -EIO;
-		else {
-			adapter->link_speed = ecmd->speed;
-			adapter->link_duplex = ecmd->duplex;
-		}
-	} else
+	ret = nx_fw_cmd_set_gbe_port(adapter, ecmd->speed, ecmd->duplex,
+				     ecmd->autoneg);
+	if (ret == NX_RCODE_NOT_SUPPORTED && ecmd->duplex == DUPLEX_HALF) {
+		netdev_info(dev, "Speed and autoneg mode settings supported, "
+			    "half duplex mode not supported\n");
 		return -EOPNOTSUPP;
+	} else if (ret) {
+		netdev_info(dev, "Setting speed, duplex or autoneg failed\n");
+		return -EIO;
+	}
+
+	adapter->link_speed = ecmd->speed;
+	adapter->link_duplex = ecmd->duplex;
+	adapter->link_autoneg = ecmd->autoneg;
 
 	if (!netif_running(dev))
 		return 0;