[PATCH 0/8] Convert even more drivers to set_phys_id

[PATCH 0/8] Convert even more drivers to set_phys_id

[Stephen Hemminger]

This converts most drivers to the new set_phys_id ethtool ops.
The three remaining drivers do things that are harder to convert since
they all use a mailbox model. These should all compile, but not
tested.

[PATCH 1/8] vxge: convert to set_phys_id

[Stephen Hemminger]

[-- Attachment #1: vxge-set-phys-id.patch --]
[-- Type: text/plain, Size: 1784 bytes --]

Also fix up incorrect docbook comment

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/vxge/vxge-ethtool.c	2011-04-04 11:56:50.004060423 -0700
+++ b/drivers/net/vxge/vxge-ethtool.c	2011-04-04 11:58:47.045259090 -0700
@@ -134,22 +134,29 @@ static void vxge_ethtool_gregs(struct ne
 /**
  * vxge_ethtool_idnic - To physically identify the nic on the system.
  * @dev : device pointer.
- * @id : pointer to the structure with identification parameters given by
- * ethtool.
+ * @state : requested LED state
  *
  * Used to physically identify the NIC on the system.
- * The Link LED will blink for a time specified by the user.
- * Return value:
  * 0 on success
  */
-static int vxge_ethtool_idnic(struct net_device *dev, u32 data)
+static int vxge_ethtool_idnic(struct net_device *dev,
+			      enum ethtool_phys_id_state state)
 {
 	struct vxgedev *vdev = netdev_priv(dev);
 	struct __vxge_hw_device *hldev = vdev->devh;
 
-	vxge_hw_device_flick_link_led(hldev, VXGE_FLICKER_ON);
-	msleep_interruptible(data ? (data * HZ) : VXGE_MAX_FLICKER_TIME);
-	vxge_hw_device_flick_link_led(hldev, VXGE_FLICKER_OFF);
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		vxge_hw_device_flick_link_led(hldev, VXGE_FLICKER_ON);
+		break;
+
+	case ETHTOOL_ID_INACTIVE:
+		vxge_hw_device_flick_link_led(hldev, VXGE_FLICKER_OFF);
+		break;
+
+	default:
+		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -1183,7 +1190,7 @@ static const struct ethtool_ops vxge_eth
 	.get_tso		= ethtool_op_get_tso,
 	.set_tso		= vxge_ethtool_op_set_tso,
 	.get_strings		= vxge_ethtool_get_strings,
-	.phys_id		= vxge_ethtool_idnic,
+	.set_phys_id		= vxge_ethtool_idnic,
 	.get_sset_count		= vxge_ethtool_get_sset_count,
 	.get_ethtool_stats	= vxge_get_ethtool_stats,
 	.set_flags		= vxge_set_flags,

Re: [PATCH 1/8] vxge: convert to set_phys_id

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 04 Apr 2011 14:06:35 -0700

> Also fix up incorrect docbook comment
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.

[PATCH 2/8] bnx2: convert to set_phys_id

[Stephen Hemminger]

[-- Attachment #1: bnx2-set-phys.patch --]
[-- Type: text/plain, Size: 2840 bytes --]

In this case, need to add element to device private to hold
original led state.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
 drivers/net/bnx2.c |   58 +++++++++++++++++++++++++----------------------------
 drivers/net/bnx2.h |    1 
 2 files changed, 29 insertions(+), 30 deletions(-)

--- a/drivers/net/bnx2.c	2011-04-04 12:00:28.358308398 -0700
+++ b/drivers/net/bnx2.c	2011-04-04 12:05:33.909298263 -0700
@@ -7495,41 +7495,39 @@ bnx2_get_ethtool_stats(struct net_device
 }
 
 static int
-bnx2_phys_id(struct net_device *dev, u32 data)
+bnx2_set_phys_id(struct net_device *dev, enum ethtool_phys_id_state state)
 {
 	struct bnx2 *bp = netdev_priv(dev);
-	int i;
-	u32 save;
 
-	bnx2_set_power_state(bp, PCI_D0);
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		bnx2_set_power_state(bp, PCI_D0);
 
-	if (data == 0)
-		data = 2;
+		bp->leds_save = REG_RD(bp, BNX2_MISC_CFG);
+		REG_WR(bp, BNX2_MISC_CFG, BNX2_MISC_CFG_LEDMODE_MAC);
+		return -EINVAL;
 
-	save = REG_RD(bp, BNX2_MISC_CFG);
-	REG_WR(bp, BNX2_MISC_CFG, BNX2_MISC_CFG_LEDMODE_MAC);
-
-	for (i = 0; i < (data * 2); i++) {
-		if ((i % 2) == 0) {
-			REG_WR(bp, BNX2_EMAC_LED, BNX2_EMAC_LED_OVERRIDE);
-		}
-		else {
-			REG_WR(bp, BNX2_EMAC_LED, BNX2_EMAC_LED_OVERRIDE |
-				BNX2_EMAC_LED_1000MB_OVERRIDE |
-				BNX2_EMAC_LED_100MB_OVERRIDE |
-				BNX2_EMAC_LED_10MB_OVERRIDE |
-				BNX2_EMAC_LED_TRAFFIC_OVERRIDE |
-				BNX2_EMAC_LED_TRAFFIC);
-		}
-		msleep_interruptible(500);
-		if (signal_pending(current))
-			break;
+	case ETHTOOL_ID_ON:
+		REG_WR(bp, BNX2_EMAC_LED, BNX2_EMAC_LED_OVERRIDE |
+		       BNX2_EMAC_LED_1000MB_OVERRIDE |
+		       BNX2_EMAC_LED_100MB_OVERRIDE |
+		       BNX2_EMAC_LED_10MB_OVERRIDE |
+		       BNX2_EMAC_LED_TRAFFIC_OVERRIDE |
+		       BNX2_EMAC_LED_TRAFFIC);
+		break;
+
+	case ETHTOOL_ID_OFF:
+		REG_WR(bp, BNX2_EMAC_LED, BNX2_EMAC_LED_OVERRIDE);
+		break;
+
+	case ETHTOOL_ID_INACTIVE:
+		REG_WR(bp, BNX2_EMAC_LED, 0);
+		REG_WR(bp, BNX2_MISC_CFG, bp->leds_save);
+
+		if (!netif_running(dev))
+			bnx2_set_power_state(bp, PCI_D3hot);
+		break;
 	}
-	REG_WR(bp, BNX2_EMAC_LED, 0);
-	REG_WR(bp, BNX2_MISC_CFG, save);
-
-	if (!netif_running(dev))
-		bnx2_set_power_state(bp, PCI_D3hot);
 
 	return 0;
 }
@@ -7602,7 +7600,7 @@ static const struct ethtool_ops bnx2_eth
 	.set_tso		= bnx2_set_tso,
 	.self_test		= bnx2_self_test,
 	.get_strings		= bnx2_get_strings,
-	.phys_id		= bnx2_phys_id,
+	.set_phys_id		= bnx2_set_phys_id,
 	.get_ethtool_stats	= bnx2_get_ethtool_stats,
 	.get_sset_count		= bnx2_get_sset_count,
 	.set_flags		= bnx2_set_flags,
--- a/drivers/net/bnx2.h	2011-04-04 12:03:55.332429815 -0700
+++ b/drivers/net/bnx2.h	2011-04-04 12:04:59.388992279 -0700
@@ -6922,6 +6922,7 @@ struct bnx2 {
 	u8			num_tx_rings;
 	u8			num_rx_rings;
 
+	u32 			leds_save;
 	u32			idle_chk_status_idx;
 
 #ifdef BCM_CNIC

Re: [PATCH 2/8] bnx2: convert to set_phys_id

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 04 Apr 2011 14:06:36 -0700

> In this case, need to add element to device private to hold
> original led state.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.

[PATCH 3/8] bnx2x: convert to set_phys_id

[Stephen Hemminger]

[-- Attachment #1: bnx2x-set-phys.patch --]
[-- Type: text/plain, Size: 2138 bytes --]

Also cleanup error codes to no lie about things that driver doesn't
support. If device is down report -EAGAIN (same as Broadcom), and 
if port doesn't blink then error as well.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/bnx2x/bnx2x_ethtool.c	2011-04-04 12:54:28.927607342 -0700
+++ b/drivers/net/bnx2x/bnx2x_ethtool.c	2011-04-04 12:55:43.108429058 -0700
@@ -2097,36 +2097,38 @@ static void bnx2x_get_ethtool_stats(stru
 	}
 }
 
-static int bnx2x_phys_id(struct net_device *dev, u32 data)
+static int bnx2x_set_phys_id(struct net_device *dev,
+			     enum ethtool_phys_id_state state)
 {
 	struct bnx2x *bp = netdev_priv(dev);
-	int i;
 
 	if (!netif_running(dev))
-		return 0;
+		return -EAGAIN;
 
 	if (!bp->port.pmf)
-		return 0;
+		return -EOPNOTSUPP;
 
-	if (data == 0)
-		data = 2;
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		return -EINVAL;
 
-	for (i = 0; i < (data * 2); i++) {
-		if ((i % 2) == 0)
-			bnx2x_set_led(&bp->link_params, &bp->link_vars,
-				      LED_MODE_OPER, SPEED_1000);
-		else
-			bnx2x_set_led(&bp->link_params, &bp->link_vars,
-				      LED_MODE_OFF, 0);
+	case ETHTOOL_ID_ON:
+		bnx2x_set_led(&bp->link_params, &bp->link_vars,
+			      LED_MODE_OPER, SPEED_1000);
+		break;
+
+	case ETHTOOL_ID_OFF:
+		bnx2x_set_led(&bp->link_params, &bp->link_vars,
+			      LED_MODE_OFF, 0);
 
-		msleep_interruptible(500);
-		if (signal_pending(current))
-			break;
-	}
+		break;
 
-	if (bp->link_vars.link_up)
-		bnx2x_set_led(&bp->link_params, &bp->link_vars, LED_MODE_OPER,
-			      bp->link_vars.line_speed);
+	case ETHTOOL_ID_INACTIVE:
+		if (bp->link_vars.link_up)
+			bnx2x_set_led(&bp->link_params, &bp->link_vars,
+				      LED_MODE_OPER,
+				      bp->link_vars.line_speed);
+	}
 
 	return 0;
 }
@@ -2218,7 +2220,7 @@ static const struct ethtool_ops bnx2x_et
 	.self_test		= bnx2x_self_test,
 	.get_sset_count		= bnx2x_get_sset_count,
 	.get_strings		= bnx2x_get_strings,
-	.phys_id		= bnx2x_phys_id,
+	.set_phys_id		= bnx2x_set_phys_id,
 	.get_ethtool_stats	= bnx2x_get_ethtool_stats,
 	.get_rxnfc		= bnx2x_get_rxnfc,
 	.get_rxfh_indir		= bnx2x_get_rxfh_indir,

Re: [PATCH 3/8] bnx2x: convert to set_phys_id

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 04 Apr 2011 14:06:37 -0700

> Also cleanup error codes to no lie about things that driver doesn't
> support. If device is down report -EAGAIN (same as Broadcom), and 
> if port doesn't blink then error as well.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.

[PATCH 4/8] s2io: convert to set_phys_id

[Stephen Hemminger]

[-- Attachment #1: s2io-set-phys.patch --]
[-- Type: text/plain, Size: 4664 bytes --]

Convert to new ethtool set physical id model. Remove no longer used
timer, and fix docbook comment.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/s2io.c	2011-04-04 12:56:26.680911740 -0700
+++ b/drivers/net/s2io.c	2011-04-04 13:12:34.591276022 -0700
@@ -5484,53 +5484,50 @@ static void s2io_ethtool_gregs(struct ne
 	}
 }
 
-/**
- *  s2io_phy_id  - timer function that alternates adapter LED.
- *  @data : address of the private member of the device structure, which
- *  is a pointer to the s2io_nic structure, provided as an u32.
- * Description: This is actually the timer function that alternates the
- * adapter LED bit of the adapter control bit to set/reset every time on
- * invocation. The timer is set for 1/2 a second, hence tha NIC blinks
- *  once every second.
+/*
+ *  s2io_set_led - control NIC led
  */
-static void s2io_phy_id(unsigned long data)
+static void s2io_set_led(struct s2io_nic *sp, bool on)
 {
-	struct s2io_nic *sp = (struct s2io_nic *)data;
 	struct XENA_dev_config __iomem *bar0 = sp->bar0;
-	u64 val64 = 0;
-	u16 subid;
+	u16 subid = sp->pdev->subsystem_device;
+	u64 val64;
 
-	subid = sp->pdev->subsystem_device;
 	if ((sp->device_type == XFRAME_II_DEVICE) ||
 	    ((subid & 0xFF) >= 0x07)) {
 		val64 = readq(&bar0->gpio_control);
-		val64 ^= GPIO_CTRL_GPIO_0;
+		if (on)
+			val64 |= GPIO_CTRL_GPIO_0;
+		else
+			val64 &= ~GPIO_CTRL_GPIO_0;
+
 		writeq(val64, &bar0->gpio_control);
 	} else {
 		val64 = readq(&bar0->adapter_control);
-		val64 ^= ADAPTER_LED_ON;
+		if (on)
+			val64 |= ADAPTER_LED_ON;
+		else
+			val64 &= ~ADAPTER_LED_ON;
+
 		writeq(val64, &bar0->adapter_control);
 	}
 
-	mod_timer(&sp->id_timer, jiffies + HZ / 2);
 }
 
 /**
- * s2io_ethtool_idnic - To physically identify the nic on the system.
- * @sp : private member of the device structure, which is a pointer to the
- * s2io_nic structure.
- * @id : pointer to the structure with identification parameters given by
- * ethtool.
+ * s2io_ethtool_set_led - To physically identify the nic on the system.
+ * @dev : network device
+ * @state: led setting
+ *
  * Description: Used to physically identify the NIC on the system.
  * The Link LED will blink for a time specified by the user for
  * identification.
  * NOTE: The Link has to be Up to be able to blink the LED. Hence
  * identification is possible only if it's link is up.
- * Return value:
- * int , returns 0 on success
  */
 
-static int s2io_ethtool_idnic(struct net_device *dev, u32 data)
+static int s2io_ethtool_set_led(struct net_device *dev,
+				enum ethtool_phys_id_state state)
 {
 	u64 val64 = 0, last_gpio_ctrl_val;
 	struct s2io_nic *sp = netdev_priv(dev);
@@ -5543,24 +5540,27 @@ static int s2io_ethtool_idnic(struct net
 		val64 = readq(&bar0->adapter_control);
 		if (!(val64 & ADAPTER_CNTL_EN)) {
 			pr_err("Adapter Link down, cannot blink LED\n");
-			return -EFAULT;
+			return -EAGAIN;
 		}
 	}
-	if (sp->id_timer.function == NULL) {
-		init_timer(&sp->id_timer);
-		sp->id_timer.function = s2io_phy_id;
-		sp->id_timer.data = (unsigned long)sp;
-	}
-	mod_timer(&sp->id_timer, jiffies);
-	if (data)
-		msleep_interruptible(data * HZ);
-	else
-		msleep_interruptible(MAX_FLICKER_TIME);
-	del_timer_sync(&sp->id_timer);
 
-	if (CARDS_WITH_FAULTY_LINK_INDICATORS(sp->device_type, subid)) {
-		writeq(last_gpio_ctrl_val, &bar0->gpio_control);
-		last_gpio_ctrl_val = readq(&bar0->gpio_control);
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		return -EINVAL;
+
+	case ETHTOOL_ID_ON:
+		s2io_set_led(sp, true);
+		break;
+
+	case ETHTOOL_ID_OFF:
+		s2io_set_led(sp, false);
+		break;
+
+	case ETHTOOL_ID_INACTIVE:
+		if (CARDS_WITH_FAULTY_LINK_INDICATORS(sp->device_type, subid)) {
+			writeq(last_gpio_ctrl_val, &bar0->gpio_control);
+			last_gpio_ctrl_val = readq(&bar0->gpio_control);
+		}
 	}
 
 	return 0;
@@ -6776,7 +6776,7 @@ static const struct ethtool_ops netdev_e
 	.set_ufo = ethtool_op_set_ufo,
 	.self_test = s2io_ethtool_test,
 	.get_strings = s2io_ethtool_get_strings,
-	.phys_id = s2io_ethtool_idnic,
+	.set_phys_id = s2io_ethtool_set_led,
 	.get_ethtool_stats = s2io_get_ethtool_stats,
 	.get_sset_count = s2io_get_sset_count,
 };
--- a/drivers/net/s2io.h	2011-04-04 13:05:25.570692566 -0700
+++ b/drivers/net/s2io.h	2011-04-04 13:05:29.762736762 -0700
@@ -893,9 +893,6 @@ struct s2io_nic {
 	u16 all_multi_pos;
 	u16 promisc_flg;
 
-	/*  Id timer, used to blink NIC to physically identify NIC. */
-	struct timer_list id_timer;
-
 	/*  Restart timer, used to restart NIC if the device is stuck and
 	 *  a schedule task that will set the correct Link state once the
 	 *  NIC's PHY has stabilized after a state change.

Re: [PATCH 4/8] s2io: convert to set_phys_id

[Ben Hutchings]

On Mon, 2011-04-04 at 14:06 -0700, Stephen Hemminger wrote:
> plain text document attachment (s2io-set-phys.patch)
> Convert to new ethtool set physical id model. Remove no longer used
> timer, and fix docbook comment.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> 
> --- a/drivers/net/s2io.c	2011-04-04 12:56:26.680911740 -0700
> +++ b/drivers/net/s2io.c	2011-04-04 13:12:34.591276022 -0700
> @@ -5484,53 +5484,50 @@ static void s2io_ethtool_gregs(struct ne
>  	}
>  }
>  
> -/**
> - *  s2io_phy_id  - timer function that alternates adapter LED.
> - *  @data : address of the private member of the device structure, which
> - *  is a pointer to the s2io_nic structure, provided as an u32.
> - * Description: This is actually the timer function that alternates the
> - * adapter LED bit of the adapter control bit to set/reset every time on
> - * invocation. The timer is set for 1/2 a second, hence tha NIC blinks
> - *  once every second.
> +/*
> + *  s2io_set_led - control NIC led
>   */
> -static void s2io_phy_id(unsigned long data)
> +static void s2io_set_led(struct s2io_nic *sp, bool on)
>  {
> -	struct s2io_nic *sp = (struct s2io_nic *)data;
>  	struct XENA_dev_config __iomem *bar0 = sp->bar0;
> -	u64 val64 = 0;
> -	u16 subid;
> +	u16 subid = sp->pdev->subsystem_device;
> +	u64 val64;
>  
> -	subid = sp->pdev->subsystem_device;
>  	if ((sp->device_type == XFRAME_II_DEVICE) ||
>  	    ((subid & 0xFF) >= 0x07)) {
>  		val64 = readq(&bar0->gpio_control);
> -		val64 ^= GPIO_CTRL_GPIO_0;
> +		if (on)
> +			val64 |= GPIO_CTRL_GPIO_0;
> +		else
> +			val64 &= ~GPIO_CTRL_GPIO_0;
> +
>  		writeq(val64, &bar0->gpio_control);
>  	} else {
>  		val64 = readq(&bar0->adapter_control);
> -		val64 ^= ADAPTER_LED_ON;
> +		if (on)
> +			val64 |= ADAPTER_LED_ON;
> +		else
> +			val64 &= ~ADAPTER_LED_ON;
> +
>  		writeq(val64, &bar0->adapter_control);
>  	}
>  
> -	mod_timer(&sp->id_timer, jiffies + HZ / 2);
>  }
>  
>  /**
> - * s2io_ethtool_idnic - To physically identify the nic on the system.
> - * @sp : private member of the device structure, which is a pointer to the
> - * s2io_nic structure.
> - * @id : pointer to the structure with identification parameters given by
> - * ethtool.
> + * s2io_ethtool_set_led - To physically identify the nic on the system.
> + * @dev : network device
> + * @state: led setting
> + *
>   * Description: Used to physically identify the NIC on the system.
>   * The Link LED will blink for a time specified by the user for
>   * identification.
>   * NOTE: The Link has to be Up to be able to blink the LED. Hence
>   * identification is possible only if it's link is up.
> - * Return value:
> - * int , returns 0 on success
>   */
>  
> -static int s2io_ethtool_idnic(struct net_device *dev, u32 data)
> +static int s2io_ethtool_set_led(struct net_device *dev,
> +				enum ethtool_phys_id_state state)
>  {
>  	u64 val64 = 0, last_gpio_ctrl_val;
>  	struct s2io_nic *sp = netdev_priv(dev);
> @@ -5543,24 +5540,27 @@ static int s2io_ethtool_idnic(struct net
>  		val64 = readq(&bar0->adapter_control);
>  		if (!(val64 & ADAPTER_CNTL_EN)) {
>  			pr_err("Adapter Link down, cannot blink LED\n");
> -			return -EFAULT;
> +			return -EAGAIN;
>  		}
>  	}
> -	if (sp->id_timer.function == NULL) {
> -		init_timer(&sp->id_timer);
> -		sp->id_timer.function = s2io_phy_id;
> -		sp->id_timer.data = (unsigned long)sp;
> -	}
> -	mod_timer(&sp->id_timer, jiffies);
> -	if (data)
> -		msleep_interruptible(data * HZ);
> -	else
> -		msleep_interruptible(MAX_FLICKER_TIME);
> -	del_timer_sync(&sp->id_timer);
>  
> -	if (CARDS_WITH_FAULTY_LINK_INDICATORS(sp->device_type, subid)) {
> -		writeq(last_gpio_ctrl_val, &bar0->gpio_control);
> -		last_gpio_ctrl_val = readq(&bar0->gpio_control);
> +	switch (state) {
> +	case ETHTOOL_ID_ACTIVE:
> +		return -EINVAL;
> +
> +	case ETHTOOL_ID_ON:
> +		s2io_set_led(sp, true);
> +		break;
> +
> +	case ETHTOOL_ID_OFF:
> +		s2io_set_led(sp, false);
> +		break;
> +
> +	case ETHTOOL_ID_INACTIVE:
> +		if (CARDS_WITH_FAULTY_LINK_INDICATORS(sp->device_type, subid)) {
> +			writeq(last_gpio_ctrl_val, &bar0->gpio_control);
> +			last_gpio_ctrl_val = readq(&bar0->gpio_control);
[...]

I think last_gpio_ctrl_val needs to be moved to struct s2io_nic and
initialised only in the ETHTOOL_ID_ACTIVE case.

Ben.

-- 
Ben Hutchings, Senior Software 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.

Re: [PATCH 4/8] s2io: convert to set_phys_id

[Stephen Hemminger]

		last_gpio_ctrl_val = readq(&bar0->gpio_control);
> [...]
> 
> I think last_gpio_ctrl_val needs to be moved to struct s2io_nic and
> initialised only in the ETHTOOL_ID_ACTIVE case.

Strange there is a value already there and unused?
   sp->adapt_ctrl_org

[PATCH 4/8] s2io: convert to set_phys_id (v2)

[Stephen Hemminger]

Convert to new ethtool set physical id model. Remove no longer used
timer, and fix docbook comment.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
v2 - store last gpio value in unused part of s2io_nic

--- a/drivers/net/s2io.c	2011-04-04 18:02:15.842923454 -0700
+++ b/drivers/net/s2io.c	2011-04-04 18:06:53.070404512 -0700
@@ -5484,83 +5484,79 @@ static void s2io_ethtool_gregs(struct ne
 	}
 }
 
-/**
- *  s2io_phy_id  - timer function that alternates adapter LED.
- *  @data : address of the private member of the device structure, which
- *  is a pointer to the s2io_nic structure, provided as an u32.
- * Description: This is actually the timer function that alternates the
- * adapter LED bit of the adapter control bit to set/reset every time on
- * invocation. The timer is set for 1/2 a second, hence tha NIC blinks
- *  once every second.
+/*
+ *  s2io_set_led - control NIC led
  */
-static void s2io_phy_id(unsigned long data)
+static void s2io_set_led(struct s2io_nic *sp, bool on)
 {
-	struct s2io_nic *sp = (struct s2io_nic *)data;
 	struct XENA_dev_config __iomem *bar0 = sp->bar0;
-	u64 val64 = 0;
-	u16 subid;
+	u16 subid = sp->pdev->subsystem_device;
+	u64 val64;
 
-	subid = sp->pdev->subsystem_device;
 	if ((sp->device_type == XFRAME_II_DEVICE) ||
 	    ((subid & 0xFF) >= 0x07)) {
 		val64 = readq(&bar0->gpio_control);
-		val64 ^= GPIO_CTRL_GPIO_0;
+		if (on)
+			val64 |= GPIO_CTRL_GPIO_0;
+		else
+			val64 &= ~GPIO_CTRL_GPIO_0;
+
 		writeq(val64, &bar0->gpio_control);
 	} else {
 		val64 = readq(&bar0->adapter_control);
-		val64 ^= ADAPTER_LED_ON;
+		if (on)
+			val64 |= ADAPTER_LED_ON;
+		else
+			val64 &= ~ADAPTER_LED_ON;
+
 		writeq(val64, &bar0->adapter_control);
 	}
 
-	mod_timer(&sp->id_timer, jiffies + HZ / 2);
 }
 
 /**
- * s2io_ethtool_idnic - To physically identify the nic on the system.
- * @sp : private member of the device structure, which is a pointer to the
- * s2io_nic structure.
- * @id : pointer to the structure with identification parameters given by
- * ethtool.
+ * s2io_ethtool_set_led - To physically identify the nic on the system.
+ * @dev : network device
+ * @state: led setting
+ *
  * Description: Used to physically identify the NIC on the system.
  * The Link LED will blink for a time specified by the user for
  * identification.
  * NOTE: The Link has to be Up to be able to blink the LED. Hence
  * identification is possible only if it's link is up.
- * Return value:
- * int , returns 0 on success
  */
 
-static int s2io_ethtool_idnic(struct net_device *dev, u32 data)
+static int s2io_ethtool_set_led(struct net_device *dev,
+				enum ethtool_phys_id_state state)
 {
-	u64 val64 = 0, last_gpio_ctrl_val;
 	struct s2io_nic *sp = netdev_priv(dev);
 	struct XENA_dev_config __iomem *bar0 = sp->bar0;
-	u16 subid;
+	u16 subid = sp->pdev->subsystem_device;
 
-	subid = sp->pdev->subsystem_device;
-	last_gpio_ctrl_val = readq(&bar0->gpio_control);
 	if ((sp->device_type == XFRAME_I_DEVICE) && ((subid & 0xFF) < 0x07)) {
-		val64 = readq(&bar0->adapter_control);
+		u64 val64 = readq(&bar0->adapter_control);
 		if (!(val64 & ADAPTER_CNTL_EN)) {
 			pr_err("Adapter Link down, cannot blink LED\n");
-			return -EFAULT;
+			return -EAGAIN;
 		}
 	}
-	if (sp->id_timer.function == NULL) {
-		init_timer(&sp->id_timer);
-		sp->id_timer.function = s2io_phy_id;
-		sp->id_timer.data = (unsigned long)sp;
-	}
-	mod_timer(&sp->id_timer, jiffies);
-	if (data)
-		msleep_interruptible(data * HZ);
-	else
-		msleep_interruptible(MAX_FLICKER_TIME);
-	del_timer_sync(&sp->id_timer);
 
-	if (CARDS_WITH_FAULTY_LINK_INDICATORS(sp->device_type, subid)) {
-		writeq(last_gpio_ctrl_val, &bar0->gpio_control);
-		last_gpio_ctrl_val = readq(&bar0->gpio_control);
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		sp->adapt_ctrl_org = readq(&bar0->gpio_control);
+		return -EINVAL;
+
+	case ETHTOOL_ID_ON:
+		s2io_set_led(sp, true);
+		break;
+
+	case ETHTOOL_ID_OFF:
+		s2io_set_led(sp, false);
+		break;
+
+	case ETHTOOL_ID_INACTIVE:
+		if (CARDS_WITH_FAULTY_LINK_INDICATORS(sp->device_type, subid))
+			writeq(sp->adapt_ctrl_org, &bar0->gpio_control);
 	}
 
 	return 0;
@@ -6776,7 +6772,7 @@ static const struct ethtool_ops netdev_e
 	.set_ufo = ethtool_op_set_ufo,
 	.self_test = s2io_ethtool_test,
 	.get_strings = s2io_ethtool_get_strings,
-	.phys_id = s2io_ethtool_idnic,
+	.set_phys_id = s2io_ethtool_set_led,
 	.get_ethtool_stats = s2io_get_ethtool_stats,
 	.get_sset_count = s2io_get_sset_count,
 };
--- a/drivers/net/s2io.h	2011-04-04 18:02:15.878923843 -0700
+++ b/drivers/net/s2io.h	2011-04-04 18:06:40.518243198 -0700
@@ -893,9 +893,6 @@ struct s2io_nic {
 	u16 all_multi_pos;
 	u16 promisc_flg;
 
-	/*  Id timer, used to blink NIC to physically identify NIC. */
-	struct timer_list id_timer;
-
 	/*  Restart timer, used to restart NIC if the device is stuck and
 	 *  a schedule task that will set the correct Link state once the
 	 *  NIC's PHY has stabilized after a state change.

Re: [PATCH 4/8] s2io: convert to set_phys_id

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 04 Apr 2011 14:06:38 -0700

> Convert to new ethtool set physical id model. Remove no longer used
> timer, and fix docbook comment.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.

[PATCH 5/8] --- drivers/net/niu.c | 33 ++++++++++++++++++--------------- drivers/net/niu.h | 1 + 2 files changed, 19 insertions(+), 15 deletions(-)

[Stephen Hemminger]

[-- Attachment #1: niu-set-phys.patch --]
[-- Type: text/plain, Size: 1710 bytes --]

--- a/drivers/net/niu.c	2011-04-04 13:15:46.805340164 -0700
+++ b/drivers/net/niu.c	2011-04-04 13:19:17.551610508 -0700
@@ -7888,28 +7888,31 @@ static void niu_force_led(struct niu *np
 	nw64_mac(reg, val);
 }
 
-static int niu_phys_id(struct net_device *dev, u32 data)
+static int niu_set_phys_id(struct net_device *dev,
+			   enum ethtool_phys_id_state state)
+
 {
 	struct niu *np = netdev_priv(dev);
-	u64 orig_led_state;
-	int i;
 
 	if (!netif_running(dev))
 		return -EAGAIN;
 
-	if (data == 0)
-		data = 2;
-
-	orig_led_state = niu_led_state_save(np);
-	for (i = 0; i < (data * 2); i++) {
-		int on = ((i % 2) == 0);
-
-		niu_force_led(np, on);
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		np->orig_led_state = niu_led_state_save(np);
+		return -EINVAL;
+
+	case ETHTOOL_ID_ON:
+		niu_force_led(np, 1);
+		break;
+
+	case ETHTOOL_ID_OFF:
+		niu_force_led(np, 0);
+		break;
 
-		if (msleep_interruptible(500))
-			break;
+	case ETHTOOL_ID_INACTIVE:
+		niu_led_state_restore(np, np->orig_led_state);
 	}
-	niu_led_state_restore(np, orig_led_state);
 
 	return 0;
 }
@@ -7932,7 +7935,7 @@ static const struct ethtool_ops niu_etht
 	.get_strings		= niu_get_strings,
 	.get_sset_count		= niu_get_sset_count,
 	.get_ethtool_stats	= niu_get_ethtool_stats,
-	.phys_id		= niu_phys_id,
+	.set_phys_id		= niu_set_phys_id,
 	.get_rxnfc		= niu_get_nfc,
 	.set_rxnfc		= niu_set_nfc,
 	.set_flags		= niu_set_flags,
--- a/drivers/net/niu.h	2011-04-04 13:16:37.445885085 -0700
+++ b/drivers/net/niu.h	2011-04-04 13:17:27.034418897 -0700
@@ -3279,6 +3279,7 @@ struct niu {
 	unsigned long			xpcs_off;
 
 	struct timer_list		timer;
+	u64				orig_led_state;
 	const struct niu_phy_ops	*phy_ops;
 	int				phy_addr;
 

[PATCH 5/8] niu: convert to new ethtool set_phys_id

[Stephen Hemminger]

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
Sorry for mangled version..

 drivers/net/niu.c |   33 ++++++++++++++++++---------------
 drivers/net/niu.h |    1 +
 2 files changed, 19 insertions(+), 15 deletions(-)

--- a/drivers/net/niu.c	2011-04-04 13:15:46.805340164 -0700
+++ b/drivers/net/niu.c	2011-04-04 13:19:17.551610508 -0700
@@ -7888,28 +7888,31 @@ static void niu_force_led(struct niu *np
 	nw64_mac(reg, val);
 }
 
-static int niu_phys_id(struct net_device *dev, u32 data)
+static int niu_set_phys_id(struct net_device *dev,
+			   enum ethtool_phys_id_state state)
+
 {
 	struct niu *np = netdev_priv(dev);
-	u64 orig_led_state;
-	int i;
 
 	if (!netif_running(dev))
 		return -EAGAIN;
 
-	if (data == 0)
-		data = 2;
-
-	orig_led_state = niu_led_state_save(np);
-	for (i = 0; i < (data * 2); i++) {
-		int on = ((i % 2) == 0);
-
-		niu_force_led(np, on);
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		np->orig_led_state = niu_led_state_save(np);
+		return -EINVAL;
+
+	case ETHTOOL_ID_ON:
+		niu_force_led(np, 1);
+		break;
+
+	case ETHTOOL_ID_OFF:
+		niu_force_led(np, 0);
+		break;
 
-		if (msleep_interruptible(500))
-			break;
+	case ETHTOOL_ID_INACTIVE:
+		niu_led_state_restore(np, np->orig_led_state);
 	}
-	niu_led_state_restore(np, orig_led_state);
 
 	return 0;
 }
@@ -7932,7 +7935,7 @@ static const struct ethtool_ops niu_etht
 	.get_strings		= niu_get_strings,
 	.get_sset_count		= niu_get_sset_count,
 	.get_ethtool_stats	= niu_get_ethtool_stats,
-	.phys_id		= niu_phys_id,
+	.set_phys_id		= niu_set_phys_id,
 	.get_rxnfc		= niu_get_nfc,
 	.set_rxnfc		= niu_set_nfc,
 	.set_flags		= niu_set_flags,
--- a/drivers/net/niu.h	2011-04-04 13:16:37.445885085 -0700
+++ b/drivers/net/niu.h	2011-04-04 13:17:27.034418897 -0700
@@ -3279,6 +3279,7 @@ struct niu {
 	unsigned long			xpcs_off;
 
 	struct timer_list		timer;
+	u64				orig_led_state;
 	const struct niu_phy_ops	*phy_ops;
 	int				phy_addr;
 

Re: [PATCH 5/8] niu: convert to new ethtool set_phys_id

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 4 Apr 2011 15:31:19 -0700

> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.

[PATCH 6/8] benet: convert to set_phys_id

[Stephen Hemminger]

[-- Attachment #1: benet-set-phys.patch --]
[-- Type: text/plain, Size: 2410 bytes --]

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


---
 drivers/net/benet/be.h         |    1 
 drivers/net/benet/be_ethtool.c |   42 ++++++++++++++++++++++-------------------
 2 files changed, 24 insertions(+), 19 deletions(-)

--- a/drivers/net/benet/be.h	2011-04-04 13:27:59.285137429 -0700
+++ b/drivers/net/benet/be.h	2011-04-04 13:31:08.107089776 -0700
@@ -313,6 +313,7 @@ struct be_adapter {
 	char fw_ver[FW_VER_LEN];
 	u32 if_handle;		/* Used to configure filtering */
 	u32 pmac_id;		/* MAC addr handle used by BE card */
+	u32 beacon_state;	/* for set_phys_id */
 
 	bool eeh_err;
 	bool link_up;
--- a/drivers/net/benet/be_ethtool.c	2011-04-04 13:24:55.943254754 -0700
+++ b/drivers/net/benet/be_ethtool.c	2011-04-04 13:32:35.043991789 -0700
@@ -526,29 +526,33 @@ be_set_pauseparam(struct net_device *net
 }
 
 static int
-be_phys_id(struct net_device *netdev, u32 data)
+be_set_phys_id(struct net_device *netdev,
+	       enum ethtool_phys_id_state state)
 {
 	struct be_adapter *adapter = netdev_priv(netdev);
-	int status;
-	u32 cur;
-
-	be_cmd_get_beacon_state(adapter, adapter->hba_port_num, &cur);
-
-	if (cur == BEACON_STATE_ENABLED)
-		return 0;
-
-	if (data < 2)
-		data = 2;
 
-	status = be_cmd_set_beacon_state(adapter, adapter->hba_port_num, 0, 0,
-			BEACON_STATE_ENABLED);
-	set_current_state(TASK_INTERRUPTIBLE);
-	schedule_timeout(data*HZ);
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		be_cmd_get_beacon_state(adapter, adapter->hba_port_num,
+					&adapter->beacon_state);
+		return -EINVAL;
 
-	status = be_cmd_set_beacon_state(adapter, adapter->hba_port_num, 0, 0,
-			BEACON_STATE_DISABLED);
+	case ETHTOOL_ID_ON:
+		be_cmd_set_beacon_state(adapter, adapter->hba_port_num, 0, 0,
+					BEACON_STATE_ENABLED);
+		break;
+
+	case ETHTOOL_ID_OFF:
+		be_cmd_set_beacon_state(adapter, adapter->hba_port_num, 0, 0,
+					BEACON_STATE_DISABLED);
+		break;
+
+	case ETHTOOL_ID_INACTIVE:
+		be_cmd_set_beacon_state(adapter, adapter->hba_port_num, 0, 0,
+					adapter->beacon_state);
+	}
 
-	return status;
+	return 0;
 }
 
 static bool
@@ -753,7 +757,7 @@ const struct ethtool_ops be_ethtool_ops
 	.get_tso = ethtool_op_get_tso,
 	.set_tso = ethtool_op_set_tso,
 	.get_strings = be_get_stat_strings,
-	.phys_id = be_phys_id,
+	.set_phys_id = be_set_phys_id,
 	.get_sset_count = be_get_sset_count,
 	.get_ethtool_stats = be_get_ethtool_stats,
 	.get_regs_len = be_get_reg_len,

RE: [PATCH 6/8] benet: convert to set_phys_id

[Ajit.Khaparde]

_______________________________________
From: Stephen Hemminger [shemminger@vyatta.com]
Sent: Monday, April 04, 2011 4:06 PM
To: David S. Miller; Ben Hutchings; Perla, Sathya; Seetharaman, Subramanian; Khaparde, Ajit
Cc: netdev@vger.kernel.org
Subject: [PATCH 6/8] benet: convert to set_phys_id

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


Thanks Stephen. You finished the work, while I was in the middle of the same.

I have picked this patch for testing. I guess the ethtool patches will be re-spun.
So I will probably wait for that before I send an update.

-Ajit

Re: [PATCH 6/8] benet: convert to set_phys_id

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 04 Apr 2011 14:06:40 -0700

> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.

[PATCH 7/8] pcnet32: convert to set_phys_id

[Stephen Hemminger]

[-- Attachment #1: pcnet32-set_phys.patch --]
[-- Type: text/plain, Size: 4165 bytes --]

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/net/pcnet32.c	2011-04-04 13:33:41.144686505 -0700
+++ b/drivers/net/pcnet32.c	2011-04-04 13:41:13.545491008 -0700
@@ -295,12 +295,14 @@ struct pcnet32_private {
 	struct net_device	*next;
 	struct mii_if_info	mii_if;
 	struct timer_list	watchdog_timer;
-	struct timer_list	blink_timer;
 	u32			msg_enable;	/* debug message level */
 
 	/* each bit indicates an available PHY */
 	u32			phymask;
 	unsigned short		chip_version;	/* which variant this is */
+
+	/* saved registers during ethtool blink */
+	u16 			save_regs[4];
 };
 
 static int pcnet32_probe_pci(struct pci_dev *, const struct pci_device_id *);
@@ -324,8 +326,6 @@ static void pcnet32_restart(struct net_d
 static void pcnet32_ethtool_test(struct net_device *dev,
 				 struct ethtool_test *eth_test, u64 * data);
 static int pcnet32_loopback_test(struct net_device *dev, uint64_t * data1);
-static int pcnet32_phys_id(struct net_device *dev, u32 data);
-static void pcnet32_led_blink_callback(struct net_device *dev);
 static int pcnet32_get_regs_len(struct net_device *dev);
 static void pcnet32_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 			     void *ptr);
@@ -1022,7 +1022,8 @@ clean_up:
 	return rc;
 }				/* end pcnet32_loopback_test  */
 
-static void pcnet32_led_blink_callback(struct net_device *dev)
+static int pcnet32_set_phys_id(struct net_device *dev,
+			       enum ethtool_phys_id_state state)
 {
 	struct pcnet32_private *lp = netdev_priv(dev);
 	struct pcnet32_access *a = &lp->a;
@@ -1030,50 +1031,31 @@ static void pcnet32_led_blink_callback(s
 	unsigned long flags;
 	int i;
 
-	spin_lock_irqsave(&lp->lock, flags);
-	for (i = 4; i < 8; i++)
-		a->write_bcr(ioaddr, i, a->read_bcr(ioaddr, i) ^ 0x4000);
-	spin_unlock_irqrestore(&lp->lock, flags);
-
-	mod_timer(&lp->blink_timer, PCNET32_BLINK_TIMEOUT);
-}
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		/* Save the current value of the bcrs */
+		spin_lock_irqsave(&lp->lock, flags);
+		for (i = 4; i < 8; i++)
+			lp->save_regs[i - 4] = a->read_bcr(ioaddr, i);
+		spin_unlock_irqrestore(&lp->lock, flags);
+		return -EINVAL;
 
-static int pcnet32_phys_id(struct net_device *dev, u32 data)
-{
-	struct pcnet32_private *lp = netdev_priv(dev);
-	struct pcnet32_access *a = &lp->a;
-	ulong ioaddr = dev->base_addr;
-	unsigned long flags;
-	int i, regs[4];
+	case ETHTOOL_ID_ON:
+	case ETHTOOL_ID_OFF:
+		/* Blink the led */
+		spin_lock_irqsave(&lp->lock, flags);
+		for (i = 4; i < 8; i++)
+			a->write_bcr(ioaddr, i, a->read_bcr(ioaddr, i) ^ 0x4000);
+		spin_unlock_irqrestore(&lp->lock, flags);
+		break;
 
-	if (!lp->blink_timer.function) {
-		init_timer(&lp->blink_timer);
-		lp->blink_timer.function = (void *)pcnet32_led_blink_callback;
-		lp->blink_timer.data = (unsigned long)dev;
+	case ETHTOOL_ID_INACTIVE:
+		/* Restore the original value of the bcrs */
+		spin_lock_irqsave(&lp->lock, flags);
+		for (i = 4; i < 8; i++)
+			a->write_bcr(ioaddr, i, lp->save_regs[i - 4]);
+		spin_unlock_irqrestore(&lp->lock, flags);
 	}
-
-	/* Save the current value of the bcrs */
-	spin_lock_irqsave(&lp->lock, flags);
-	for (i = 4; i < 8; i++)
-		regs[i - 4] = a->read_bcr(ioaddr, i);
-	spin_unlock_irqrestore(&lp->lock, flags);
-
-	mod_timer(&lp->blink_timer, jiffies);
-	set_current_state(TASK_INTERRUPTIBLE);
-
-	/* AV: the limit here makes no sense whatsoever */
-	if ((!data) || (data > (u32) (MAX_SCHEDULE_TIMEOUT / HZ)))
-		data = (u32) (MAX_SCHEDULE_TIMEOUT / HZ);
-
-	msleep_interruptible(data * 1000);
-	del_timer_sync(&lp->blink_timer);
-
-	/* Restore the original value of the bcrs */
-	spin_lock_irqsave(&lp->lock, flags);
-	for (i = 4; i < 8; i++)
-		a->write_bcr(ioaddr, i, regs[i - 4]);
-	spin_unlock_irqrestore(&lp->lock, flags);
-
 	return 0;
 }
 
@@ -1450,7 +1432,7 @@ static const struct ethtool_ops pcnet32_
 	.set_ringparam		= pcnet32_set_ringparam,
 	.get_strings		= pcnet32_get_strings,
 	.self_test		= pcnet32_ethtool_test,
-	.phys_id		= pcnet32_phys_id,
+	.set_phys_id		= pcnet32_set_phys_id,
 	.get_regs_len		= pcnet32_get_regs_len,
 	.get_regs		= pcnet32_get_regs,
 	.get_sset_count		= pcnet32_get_sset_count,

Re: [PATCH 7/8] pcnet32: convert to set_phys_id

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 04 Apr 2011 14:06:41 -0700

> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.

[PATCH 8/8] ewrk3: convert to set_phys_id

[Stephen Hemminger]

[-- Attachment #1: ewrk3-set-phys.patch --]
[-- Type: text/plain, Size: 2472 bytes --]

Keep orginal locking and error handling.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/ewrk3.c	2011-04-04 13:41:41.717791151 -0700
+++ b/drivers/net/ewrk3.c	2011-04-04 13:46:55.425138028 -0700
@@ -1604,55 +1604,51 @@ static u32 ewrk3_get_link(struct net_dev
 	return !(cmr & CMR_LINK);
 }
 
-static int ewrk3_phys_id(struct net_device *dev, u32 data)
+static int ewrk3_set_phys_id(struct net_device *dev,
+			     enum ethtool_phys_id_state state)
 {
 	struct ewrk3_private *lp = netdev_priv(dev);
 	unsigned long iobase = dev->base_addr;
 	unsigned long flags;
 	u8 cr;
-	int count;
-
-	/* Toggle LED 4x per second */
-	count = data << 2;
 
 	spin_lock_irqsave(&lp->hw_lock, flags);
 
-	/* Bail if a PHYS_ID is already in progress */
-	if (lp->led_mask == 0) {
-		spin_unlock_irqrestore(&lp->hw_lock, flags);
-		return -EBUSY;
-	}
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		/* Bail if a PHYS_ID is already in progress */
+		if (lp->led_mask == 0) {
+			spin_unlock_irqrestore(&lp->hw_lock, flags);
+			return -EBUSY;
+		}
 
-	/* Prevent ISR from twiddling the LED */
-	lp->led_mask = 0;
+		/* Prevent ISR from twiddling the LED */
+		lp->led_mask = 0;
+		spin_unlock_irqrestore(&lp->hw_lock, flags);
+		return -EINVAL;
 
-	while (count--) {
+	case ETHTOOL_ID_ON:
+	case ETHTOOL_ID_OFF:
 		/* Toggle the LED */
 		cr = inb(EWRK3_CR);
 		outb(cr ^ CR_LED, EWRK3_CR);
+		break;
 
-		/* Wait a little while */
-		spin_unlock_irqrestore(&lp->hw_lock, flags);
-		msleep(250);
-		spin_lock_irqsave(&lp->hw_lock, flags);
-
-		/* Exit if we got a signal */
-		if (signal_pending(current))
-			break;
+	case ETHTOOL_ID_INACTIVE:
+		lp->led_mask = CR_LED;
+		cr = inb(EWRK3_CR);
+		outb(cr & ~CR_LED, EWRK3_CR);
 	}
-
-	lp->led_mask = CR_LED;
-	cr = inb(EWRK3_CR);
-	outb(cr & ~CR_LED, EWRK3_CR);
 	spin_unlock_irqrestore(&lp->hw_lock, flags);
-	return signal_pending(current) ? -ERESTARTSYS : 0;
+
+	return 0;
 }
 
 static const struct ethtool_ops ethtool_ops_203 = {
 	.get_drvinfo = ewrk3_get_drvinfo,
 	.get_settings = ewrk3_get_settings,
 	.set_settings = ewrk3_set_settings,
-	.phys_id = ewrk3_phys_id,
+	.set_phys_id = ewrk3_set_phys_id,
 };
 
 static const struct ethtool_ops ethtool_ops = {
@@ -1660,7 +1656,7 @@ static const struct ethtool_ops ethtool_
 	.get_settings = ewrk3_get_settings,
 	.set_settings = ewrk3_set_settings,
 	.get_link = ewrk3_get_link,
-	.phys_id = ewrk3_phys_id,
+	.set_phys_id = ewrk3_set_phys_id,
 };
 
 /*

Re: [PATCH 8/8] ewrk3: convert to set_phys_id

[Ben Hutchings]

On Mon, 2011-04-04 at 14:06 -0700, Stephen Hemminger wrote:
> Keep orginal locking and error handling.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> 
> --- a/drivers/net/ewrk3.c	2011-04-04 13:41:41.717791151 -0700
> +++ b/drivers/net/ewrk3.c	2011-04-04 13:46:55.425138028 -0700
> @@ -1604,55 +1604,51 @@ static u32 ewrk3_get_link(struct net_dev
>  	return !(cmr & CMR_LINK);
>  }
>  
> -static int ewrk3_phys_id(struct net_device *dev, u32 data)
> +static int ewrk3_set_phys_id(struct net_device *dev,
> +			     enum ethtool_phys_id_state state)
>  {
>  	struct ewrk3_private *lp = netdev_priv(dev);
>  	unsigned long iobase = dev->base_addr;
>  	unsigned long flags;
>  	u8 cr;
> -	int count;
> -
> -	/* Toggle LED 4x per second */
> -	count = data << 2;
>  
>  	spin_lock_irqsave(&lp->hw_lock, flags);
>  
> -	/* Bail if a PHYS_ID is already in progress */
> -	if (lp->led_mask == 0) {
> -		spin_unlock_irqrestore(&lp->hw_lock, flags);
> -		return -EBUSY;
> -	}
> +	switch (state) {
> +	case ETHTOOL_ID_ACTIVE:
> +		/* Bail if a PHYS_ID is already in progress */
> +		if (lp->led_mask == 0) {
> +			spin_unlock_irqrestore(&lp->hw_lock, flags);
> +			return -EBUSY;
> +		}
[...]

This can never happen.  Well, actually it can with the first version of
my patch, but I'll fix that. :-)

Ben.

-- 
Ben Hutchings, Senior Software 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.

Re: [PATCH 8/8] ewrk3: convert to set_phys_id

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 04 Apr 2011 14:06:42 -0700

> Keep orginal locking and error handling.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Please remove the "in progress" handling as indicated in feedback
sent for this patch.

Thanks.

Re: [PATCH 8/8] ewrk3: convert to set_phys_id

[Stephen Hemminger]

Use ethtool infrastructure for blinking, which is now does
locking at higher level.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/ewrk3.c	2011-04-06 14:50:45.905902923 -0700
+++ b/drivers/net/ewrk3.c	2011-04-06 14:56:00.249078160 -0700
@@ -1604,55 +1604,47 @@ static u32 ewrk3_get_link(struct net_dev
 	return !(cmr & CMR_LINK);
 }
 
-static int ewrk3_phys_id(struct net_device *dev, u32 data)
+static int ewrk3_set_phys_id(struct net_device *dev,
+			     enum ethtool_phys_id_state state)
 {
 	struct ewrk3_private *lp = netdev_priv(dev);
 	unsigned long iobase = dev->base_addr;
-	unsigned long flags;
 	u8 cr;
-	int count;
 
-	/* Toggle LED 4x per second */
-	count = data << 2;
+	spin_lock_irq(&lp->hw_lock);
 
-	spin_lock_irqsave(&lp->hw_lock, flags);
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		/* Prevent ISR from twiddling the LED */
+		lp->led_mask = 0;
+		spin_unlock_irq(&lp->hw_lock);
+		return -EINVAL;
 
-	/* Bail if a PHYS_ID is already in progress */
-	if (lp->led_mask == 0) {
-		spin_unlock_irqrestore(&lp->hw_lock, flags);
-		return -EBUSY;
-	}
-
-	/* Prevent ISR from twiddling the LED */
-	lp->led_mask = 0;
+	case ETHTOOL_ID_ON:
+		cr = inb(EWRK3_CR);
+		outb(cr | CR_LED, EWRK3_CR);
+		break;
 
-	while (count--) {
-		/* Toggle the LED */
+	case ETHTOOL_ID_OFF:
 		cr = inb(EWRK3_CR);
-		outb(cr ^ CR_LED, EWRK3_CR);
+		outb(cr & ~CR_LED, EWRK3_CR);
+		break;
 
-		/* Wait a little while */
-		spin_unlock_irqrestore(&lp->hw_lock, flags);
-		msleep(250);
-		spin_lock_irqsave(&lp->hw_lock, flags);
-
-		/* Exit if we got a signal */
-		if (signal_pending(current))
-			break;
+	case ETHTOOL_ID_INACTIVE:
+		lp->led_mask = CR_LED;
+		cr = inb(EWRK3_CR);
+		outb(cr & ~CR_LED, EWRK3_CR);
 	}
+	spin_unlock_irq(&lp->hw_lock);
 
-	lp->led_mask = CR_LED;
-	cr = inb(EWRK3_CR);
-	outb(cr & ~CR_LED, EWRK3_CR);
-	spin_unlock_irqrestore(&lp->hw_lock, flags);
-	return signal_pending(current) ? -ERESTARTSYS : 0;
+	return 0;
 }
 
 static const struct ethtool_ops ethtool_ops_203 = {
 	.get_drvinfo = ewrk3_get_drvinfo,
 	.get_settings = ewrk3_get_settings,
 	.set_settings = ewrk3_set_settings,
-	.phys_id = ewrk3_phys_id,
+	.set_phys_id = ewrk3_set_phys_id,
 };
 
 static const struct ethtool_ops ethtool_ops = {
@@ -1660,7 +1652,7 @@ static const struct ethtool_ops ethtool_
 	.get_settings = ewrk3_get_settings,
 	.set_settings = ewrk3_set_settings,
 	.get_link = ewrk3_get_link,
-	.phys_id = ewrk3_phys_id,
+	.set_phys_id = ewrk3_set_phys_id,
 };
 
 /*

Re: [PATCH 8/8] ewrk3: convert to set_phys_id

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 6 Apr 2011 14:58:36 -0700

> Use ethtool infrastructure for blinking, which is now does
> locking at higher level.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.