[net-next-2.6 RFC PATCH v2 00/13] ethtool: allow custom interval for

[net-next-2.6 RFC PATCH v2 00/13] ethtool: allow custom interval for

[Bruce Allan]

physical identification

The following series changes the recently added ethtool set_phys_id
functions to allow drivers to provide a frequency at which to cycle
through an on/off identifier via software if/when the capability is
not provided by hardware.

---

Bruce Allan (13):
      tg3: set ethtool set_phys_id on/off cycle frequency to 1/sec
      sky2: set ethtool set_phys_id on/off cycle frequency to 1/sec
      skge: set ethtool set_phys_id on/off cycle frequency to 2/sec
      sfc: set ethtool set_phys_id on/off cycle frequency to 1/sec
      s2io: set ethtool set_phys_id on/off cycle frequency to 1/sec
      pcnet32: set ethtool set_phys_id on/off cycle frequency to 2/sec
      niu: set ethtool set_phys_id on/off cycle frequency to 1/sec
      ewrk3: set ethtool set_phys_id on/off cycle frequency to 2/sec
      cxgb3: set ethtool set_phys_id on/off cycle frequency to 1/sec
      bnx2x: set ethtool set_phys_id on/off cycle frequency to 1/sec
      bnx2: set ethtool set_phys_id on/off cycle frequency to 1/sec
      benet: set ethtool set_phys_id on/off cycle frequency to 1/sec
      ethtool: allow custom interval for physical identification


 drivers/net/benet/be_ethtool.c    |    2 +-
 drivers/net/bnx2.c                |    2 +-
 drivers/net/bnx2x/bnx2x_ethtool.c |    2 +-
 drivers/net/cxgb3/cxgb3_main.c    |    2 +-
 drivers/net/ewrk3.c               |    2 +-
 drivers/net/niu.c                 |    2 +-
 drivers/net/pcnet32.c             |    2 +-
 drivers/net/s2io.c                |    2 +-
 drivers/net/sfc/ethtool.c         |    6 +++---
 drivers/net/skge.c                |    2 +-
 drivers/net/sky2.c                |    2 +-
 drivers/net/tg3.c                 |    2 +-
 include/linux/ethtool.h           |    6 ++++--
 net/core/ethtool.c                |   13 ++++++++-----
 14 files changed, 26 insertions(+), 21 deletions(-)

-- 
Thanks,
Bruce.

[net-next-2.6 RFC PATCH v2 01/13] ethtool: allow custom interval for physical identification

[Bruce Allan]

When physical identification of an adapter is done by toggling the
mechanism on and off through software utilizing the set_phys_id operation,
it is done with a fixed duration for both on and off states.  Some drivers
may want to set a custom duration for the on/off intervals.  This patch
changes the API so the return code from the driver's entry point when it
is called with ETHTOOL_ID_ACTIVE can specify the frequency at which to
cycle the on/off states.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Ben Hutchings <bhutchings@solarflare.com>
---

 include/linux/ethtool.h |    6 ++++--
 net/core/ethtool.c      |   13 ++++++++-----
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 12cfbd0..6191a84 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -770,8 +770,10 @@ bool ethtool_invalid_flags(struct net_device *dev, u32 data, u32 supported);
  *	attached to it.  The implementation may update the indicator
  *	asynchronously or synchronously, but in either case it must return
  *	quickly.  It is initially called with the argument %ETHTOOL_ID_ACTIVE,
- *	and must either activate asynchronous updates or return -%EINVAL.
- *	If it returns -%EINVAL then it will be called again at intervals with
+ *	and must either activate asynchronous updates and return zero, return
+ *	a negative error or return a positive frequency for synchronous
+ *	indication (e.g. 1 for one on/off cycle per second).  If it returns
+ *	a frequency then it will be called again at intervals with the
  *	argument %ETHTOOL_ID_ON or %ETHTOOL_ID_OFF and should set the state of
  *	the indicator accordingly.  Finally, it is called with the argument
  *	%ETHTOOL_ID_INACTIVE and must deactivate the indicator.  Returns a
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 43ef09f..2dd7bde 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1640,7 +1640,7 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
 		return dev->ethtool_ops->phys_id(dev, id.data);
 
 	rc = dev->ethtool_ops->set_phys_id(dev, ETHTOOL_ID_ACTIVE);
-	if (rc && rc != -EINVAL)
+	if (rc < 0)
 		return rc;
 
 	/* Drop the RTNL lock while waiting, but prevent reentry or
@@ -1655,23 +1655,26 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
 		schedule_timeout_interruptible(
 			id.data ? (id.data * HZ) : MAX_SCHEDULE_TIMEOUT);
 	} else {
-		/* Driver expects to be called periodically */
+		/* Driver expects to be called using the frequency in rc */
+		int i = 0, interval = (HZ / (rc * 2));
+
 		do {
 			rtnl_lock();
 			rc = dev->ethtool_ops->set_phys_id(dev, ETHTOOL_ID_ON);
 			rtnl_unlock();
 			if (rc)
 				break;
-			schedule_timeout_interruptible(HZ / 2);
+			schedule_timeout_interruptible(interval);
 
 			rtnl_lock();
 			rc = dev->ethtool_ops->set_phys_id(dev, ETHTOOL_ID_OFF);
 			rtnl_unlock();
 			if (rc)
 				break;
-			schedule_timeout_interruptible(HZ / 2);
+			schedule_timeout_interruptible(interval);
 		} while (!signal_pending(current) &&
-			 (id.data == 0 || --id.data != 0));
+			 (id.data == 0 ||
+			  (++i * 2 * interval) < (id.data * HZ)));
 	}
 
 	rtnl_lock();

[net-next-2.6 RFC PATCH v2 02/13] benet: set ethtool set_phys_id on/off cycle frequency to 1/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Sathya Perla <sathya.perla@emulex.com>
Cc: Subbu Seetharaman <subbu.seetharaman@emulex.com>
Cc: Ajit Khaparde <ajit.khaparde@emulex.com>
---

 drivers/net/benet/be_ethtool.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/benet/be_ethtool.c b/drivers/net/benet/be_ethtool.c
index 96f5502..80226e4 100644
--- a/drivers/net/benet/be_ethtool.c
+++ b/drivers/net/benet/be_ethtool.c
@@ -516,7 +516,7 @@ be_set_phys_id(struct net_device *netdev,
 	case ETHTOOL_ID_ACTIVE:
 		be_cmd_get_beacon_state(adapter, adapter->hba_port_num,
 					&adapter->beacon_state);
-		return -EINVAL;
+		return 1;	/* cycle on/off once per second */
 
 	case ETHTOOL_ID_ON:
 		be_cmd_set_beacon_state(adapter, adapter->hba_port_num, 0, 0,

[net-next-2.6 RFC PATCH v2 03/13] bnx2: set ethtool set_phys_id on/off cycle frequency to 1/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Michael Chan <mchan@broadcom.com>
---

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

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 0a52079..bf729ee 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -7473,7 +7473,7 @@ bnx2_set_phys_id(struct net_device *dev, enum ethtool_phys_id_state state)
 
 		bp->leds_save = REG_RD(bp, BNX2_MISC_CFG);
 		REG_WR(bp, BNX2_MISC_CFG, BNX2_MISC_CFG_LEDMODE_MAC);
-		return -EINVAL;
+		return 1;	/* cycle on/off once per second */
 
 	case ETHTOOL_ID_ON:
 		REG_WR(bp, BNX2_EMAC_LED, BNX2_EMAC_LED_OVERRIDE |

[net-next-2.6 RFC PATCH v2 04/13] bnx2x: set ethtool set_phys_id on/off cycle frequency to 1/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Eilon Greenstein <eilong@broadcom.com>
---

 drivers/net/bnx2x/bnx2x_ethtool.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
index ad7d91e..0a5e88d 100644
--- a/drivers/net/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/bnx2x/bnx2x_ethtool.c
@@ -2025,7 +2025,7 @@ static int bnx2x_set_phys_id(struct net_device *dev,
 
 	switch (state) {
 	case ETHTOOL_ID_ACTIVE:
-		return -EINVAL;
+		return 1;	/* cycle on/off once per second */
 
 	case ETHTOOL_ID_ON:
 		bnx2x_set_led(&bp->link_params, &bp->link_vars,

[net-next-2.6 RFC PATCH v2 05/13] cxgb3: set ethtool set_phys_id on/off cycle frequency to 1/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Divy Le Ray <divy@chelsio.com>
---

 drivers/net/cxgb3/cxgb3_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/cxgb3/cxgb3_main.c b/drivers/net/cxgb3/cxgb3_main.c
index 802c7a7..a087e06 100644
--- a/drivers/net/cxgb3/cxgb3_main.c
+++ b/drivers/net/cxgb3/cxgb3_main.c
@@ -1757,7 +1757,7 @@ static int set_phys_id(struct net_device *dev,
 
 	switch (state) {
 	case ETHTOOL_ID_ACTIVE:
-		return -EINVAL;
+		return 1;	/* cycle on/off once per second */
 
 	case ETHTOOL_ID_OFF:
 		t3_set_reg_field(adapter, A_T3DBG_GPIO_EN, F_GPIO0_OUT_VAL, 0);

[net-next-2.6 RFC PATCH v2 06/13] ewrk3: set ethtool set_phys_id on/off cycle frequency to 2/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Not tested.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
---

 drivers/net/ewrk3.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ewrk3.c b/drivers/net/ewrk3.c
index c7ce443..17b6027 100644
--- a/drivers/net/ewrk3.c
+++ b/drivers/net/ewrk3.c
@@ -1618,7 +1618,7 @@ static int ewrk3_set_phys_id(struct net_device *dev,
 		/* Prevent ISR from twiddling the LED */
 		lp->led_mask = 0;
 		spin_unlock_irq(&lp->hw_lock);
-		return -EINVAL;
+		return 2;	/* cycle on/off twice per second */
 
 	case ETHTOOL_ID_ON:
 		cr = inb(EWRK3_CR);

[net-next-2.6 RFC PATCH v2 07/13] niu: set ethtool set_phys_id on/off cycle frequency to 1/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
---

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

diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index 3fa1e9c..ea2272f 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -7896,7 +7896,7 @@ static int niu_set_phys_id(struct net_device *dev,
 	switch (state) {
 	case ETHTOOL_ID_ACTIVE:
 		np->orig_led_state = niu_led_state_save(np);
-		return -EINVAL;
+		return 1;	/* cycle on/off once per second */
 
 	case ETHTOOL_ID_ON:
 		niu_force_led(np, 1);

[net-next-2.6 RFC PATCH v2 08/13] pcnet32: set ethtool set_phys_id on/off cycle frequency to 2/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Don Fry <pcnet32@frontier.com>
---

 drivers/net/pcnet32.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
index e89afb9..0a1efba 100644
--- a/drivers/net/pcnet32.c
+++ b/drivers/net/pcnet32.c
@@ -1038,7 +1038,7 @@ static int pcnet32_set_phys_id(struct net_device *dev,
 		for (i = 4; i < 8; i++)
 			lp->save_regs[i - 4] = a->read_bcr(ioaddr, i);
 		spin_unlock_irqrestore(&lp->lock, flags);
-		return -EINVAL;
+		return 2;	/* cycle on/off twice per second */
 
 	case ETHTOOL_ID_ON:
 	case ETHTOOL_ID_OFF:

[net-next-2.6 RFC PATCH v2 09/13] s2io: set ethtool set_phys_id on/off cycle frequency to 1/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Jon Mason <jdmason@kudzu.us>
---

 drivers/net/s2io.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 2d5cc61..2302d97 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -5541,7 +5541,7 @@ static int s2io_ethtool_set_led(struct net_device *dev,
 	switch (state) {
 	case ETHTOOL_ID_ACTIVE:
 		sp->adapt_ctrl_org = readq(&bar0->gpio_control);
-		return -EINVAL;
+		return 1;	/* cycle on/off once per second */
 
 	case ETHTOOL_ID_ON:
 		s2io_set_led(sp, true);

[net-next-2.6 RFC PATCH v2 10/13] sfc: set ethtool set_phys_id on/off cycle frequency to 1/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Also fixed the compile warning re. "mode" may be used uninitialized.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
Cc: Steve Hodgson <shodgson@solarflare.com>
Cc: Ben Hutchings <bhutchings@solarflare.com>
---

 drivers/net/sfc/ethtool.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sfc/ethtool.c b/drivers/net/sfc/ethtool.c
index 644f7c1..5d8468f 100644
--- a/drivers/net/sfc/ethtool.c
+++ b/drivers/net/sfc/ethtool.c
@@ -182,7 +182,7 @@ static int efx_ethtool_phys_id(struct net_device *net_dev,
 			       enum ethtool_phys_id_state state)
 {
 	struct efx_nic *efx = netdev_priv(net_dev);
-	enum efx_led_mode mode;
+	enum efx_led_mode mode = EFX_LED_DEFAULT;
 
 	switch (state) {
 	case ETHTOOL_ID_ON:
@@ -194,8 +194,8 @@ static int efx_ethtool_phys_id(struct net_device *net_dev,
 	case ETHTOOL_ID_INACTIVE:
 		mode = EFX_LED_DEFAULT;
 		break;
-	default:
-		return -EINVAL;
+	case ETHTOOL_ID_ACTIVE:
+		return 1;	/* cycle on/off once per second */
 	}
 
 	efx->type->set_id_led(efx, mode);

[net-next-2.6 RFC PATCH v2 11/13] skge: set ethtool set_phys_id on/off cycle frequency to 2/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Stephen Hemminger <shemminger@linux-foundation.org>
---

 drivers/net/skge.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/skge.c b/drivers/net/skge.c
index 310dcbc..176d784 100644
--- a/drivers/net/skge.c
+++ b/drivers/net/skge.c
@@ -753,7 +753,7 @@ static int skge_set_phys_id(struct net_device *dev,
 
 	switch (state) {
 	case ETHTOOL_ID_ACTIVE:
-		return -EINVAL;
+		return 2;	/* cycle on/off twice per second */
 
 	case ETHTOOL_ID_ON:
 		skge_led(skge, LED_MODE_TST);

[net-next-2.6 RFC PATCH v2 12/13] sky2: set ethtool set_phys_id on/off cycle frequency to 1/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Stephen Hemminger <shemminger@linux-foundation.org>
---

 drivers/net/sky2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index a4b8fe5..c8d0451 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -3813,7 +3813,7 @@ static int sky2_set_phys_id(struct net_device *dev,
 
 	switch (state) {
 	case ETHTOOL_ID_ACTIVE:
-		return -EINVAL;
+		return 1;	/* cycle on/off once per second */
 	case ETHTOOL_ID_INACTIVE:
 		sky2_led(sky2, MO_LED_NORM);
 		break;

[net-next-2.6 RFC PATCH v2 13/13] tg3: set ethtool set_phys_id on/off cycle frequency to 1/sec

[Bruce Allan]

Physical identification frequency based on how it was done prior to the
introduction of set_phys_id.  Compile tested only.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Matt Carlson <mcarlson@broadcom.com>
Cc: Michael Chan <mchan@broadcom.com>
---

 drivers/net/tg3.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 9d7defc..7c1a9dd 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -10292,7 +10292,7 @@ static int tg3_set_phys_id(struct net_device *dev,
 
 	switch (state) {
 	case ETHTOOL_ID_ACTIVE:
-		return -EINVAL;
+		return 1;	/* cycle on/off once per second */
 
 	case ETHTOOL_ID_ON:
 		tw32(MAC_LED_CTRL, LED_CTRL_LNKLED_OVERRIDE |

Re: [net-next-2.6 RFC PATCH v2 00/13] ethtool: allow custom interval for

[Ben Hutchings]

On Wed, 2011-04-13 at 12:58 -0700, Bruce Allan wrote:
> physical identification
> 
> The following series changes the recently added ethtool set_phys_id
> functions to allow drivers to provide a frequency at which to cycle
> through an on/off identifier via software if/when the capability is
> not provided by hardware.
[...]

The first patch leaves all the drivers broken temporarily.  Since the
change in each driver is trivial, I think you can squash this all into
one patch.

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: [net-next-2.6 RFC PATCH v2 01/13] ethtool: allow custom interval for physical identification

[Ben Hutchings]

On Wed, 2011-04-13 at 12:58 -0700, Bruce Allan wrote:
> When physical identification of an adapter is done by toggling the
> mechanism on and off through software utilizing the set_phys_id operation,
> it is done with a fixed duration for both on and off states.  Some drivers
> may want to set a custom duration for the on/off intervals.  This patch
> changes the API so the return code from the driver's entry point when it
> is called with ETHTOOL_ID_ACTIVE can specify the frequency at which to
> cycle the on/off states.
[...]
> @@ -1655,23 +1655,26 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
>  		schedule_timeout_interruptible(
>  			id.data ? (id.data * HZ) : MAX_SCHEDULE_TIMEOUT);
>  	} else {
> -		/* Driver expects to be called periodically */
> +		/* Driver expects to be called using the frequency in rc */
> +		int i = 0, interval = (HZ / (rc * 2));
> +
>  		do {
>  			rtnl_lock();
>  			rc = dev->ethtool_ops->set_phys_id(dev, ETHTOOL_ID_ON);
>  			rtnl_unlock();
>  			if (rc)
>  				break;
> -			schedule_timeout_interruptible(HZ / 2);
> +			schedule_timeout_interruptible(interval);
>  
>  			rtnl_lock();
>  			rc = dev->ethtool_ops->set_phys_id(dev, ETHTOOL_ID_OFF);
>  			rtnl_unlock();
>  			if (rc)
>  				break;
> -			schedule_timeout_interruptible(HZ / 2);
> +			schedule_timeout_interruptible(interval);
>  		} while (!signal_pending(current) &&
> -			 (id.data == 0 || --id.data != 0));
> +			 (id.data == 0 ||
> +			  (++i * 2 * interval) < (id.data * HZ)));
[...]

I'm sure there ought to be a clearer way to do this, and to avoid any
weird effects from integer overflow in the multiplication.  How about
using an inner loop for each second:

		/* Driver expects to be called at twice the frequency in rc */
		int n = rc * 2, i, interval = HZ / n;

		do {
			i = n;
			do {
	 			rtnl_lock();
 				rc = dev->ethtool_ops->set_phys_id(
					dev, (i & 1) ? ETHTOOL_ID_OFF : ETHTOOL_ID_ON);
	 			rtnl_unlock();
 				if (rc)
 					break;
				schedule_timeout_interruptible(interval);
			} while (!signal_pending(current) && --i != 0);
 		} while (!signal_pending(current) &&
			 (id.data == 0 || --id.data != 0));

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: [net-next-2.6 RFC PATCH v2 00/13] ethtool: allow custom interval for

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 13 Apr 2011 21:11:04 +0100

> On Wed, 2011-04-13 at 12:58 -0700, Bruce Allan wrote:
>> physical identification
>> 
>> The following series changes the recently added ethtool set_phys_id
>> functions to allow drivers to provide a frequency at which to cycle
>> through an on/off identifier via software if/when the capability is
>> not provided by hardware.
> [...]
> 
> The first patch leaves all the drivers broken temporarily.  Since the
> change in each driver is trivial, I think you can squash this all into
> one patch.

Agreed.

Re: [net-next-2.6 RFC PATCH v2 12/13] sky2: set ethtool set_phys_id on/off cycle frequency to 1/sec

[Stephen Hemminger]

On Wed, 13 Apr 2011 12:59:49 -0700
Bruce Allan <bruce.w.allan@intel.com> wrote:

> Physical identification frequency based on how it was done prior to the
> introduction of set_phys_id.  Compile tested only.
> 
> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Stephen Hemminger <shemminger@linux-foundation.org>

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

Assume same for skge

RE: [net-next-2.6 RFC PATCH v2 01/13] ethtool: allow custom interval for physical identification

[Allan, Bruce W]

>-----Original Message-----
>From: Ben Hutchings [mailto:bhutchings@solarflare.com]
>Sent: Wednesday, April 13, 2011 1:25 PM
>To: Allan, Bruce W
>Cc: netdev@vger.kernel.org
>Subject: Re: [net-next-2.6 RFC PATCH v2 01/13] ethtool: allow custom interval
>for physical identification
>
>I'm sure there ought to be a clearer way to do this, and to avoid any
>weird effects from integer overflow in the multiplication.  How about
>using an inner loop for each second:
>
>		/* Driver expects to be called at twice the frequency in rc */
>		int n = rc * 2, i, interval = HZ / n;
>
>		do {
>			i = n;
>			do {
>	 			rtnl_lock();
> 				rc = dev->ethtool_ops->set_phys_id(
>					dev, (i & 1) ? ETHTOOL_ID_OFF : ETHTOOL_ID_ON);
>	 			rtnl_unlock();
> 				if (rc)
> 					break;
>				schedule_timeout_interruptible(interval);
>			} while (!signal_pending(current) && --i != 0);
> 		} while (!signal_pending(current) &&
>			 (id.data == 0 || --id.data != 0));
>
>Ben.

OK, if that is clearer to you...v3 forthcoming.

Thanks,
Bruce.

RE: [net-next-2.6 RFC PATCH v2 01/13] ethtool: allow custom interval for physical identification

[Ben Hutchings]

On Wed, 2011-04-13 at 15:39 -0700, Allan, Bruce W wrote:
> 
> >-----Original Message-----
> >From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> >Sent: Wednesday, April 13, 2011 1:25 PM
> >To: Allan, Bruce W
> >Cc: netdev@vger.kernel.org
> >Subject: Re: [net-next-2.6 RFC PATCH v2 01/13] ethtool: allow custom interval
> >for physical identification
> >
> >I'm sure there ought to be a clearer way to do this, and to avoid any
> >weird effects from integer overflow in the multiplication.  How about
> >using an inner loop for each second:
> >
> >		/* Driver expects to be called at twice the frequency in rc */
> >		int n = rc * 2, i, interval = HZ / n;
> >

		/* Count down seconds */
> >		do {
			/* Count down iterations per second */
> >			i = n;
> >			do {
> >	 			rtnl_lock();
> > 				rc = dev->ethtool_ops->set_phys_id(
> >					dev, (i & 1) ? ETHTOOL_ID_OFF : ETHTOOL_ID_ON);
> >	 			rtnl_unlock();
> > 				if (rc)
> > 					break;
> >				schedule_timeout_interruptible(interval);
> >			} while (!signal_pending(current) && --i != 0);
> > 		} while (!signal_pending(current) &&
> >			 (id.data == 0 || --id.data != 0));
> >
> >Ben.
> 
> OK, if that is clearer to you...v3 forthcoming.

I guess it wouldn't hurt to add comemnts too.  Would you agree that it's
clear with the additions above?

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: [net-next-2.6 RFC PATCH v2 01/13] ethtool: allow custom interval for physical identification

[Allan, Bruce W]

>-----Original Message-----
>From: Ben Hutchings [mailto:bhutchings@solarflare.com]
>Sent: Wednesday, April 13, 2011 3:45 PM
>To: Allan, Bruce W
>Cc: netdev@vger.kernel.org
>Subject: RE: [net-next-2.6 RFC PATCH v2 01/13] ethtool: allow custom interval
>for physical identification
>
>On Wed, 2011-04-13 at 15:39 -0700, Allan, Bruce W wrote:
>>
>> >-----Original Message-----
>> >From: Ben Hutchings [mailto:bhutchings@solarflare.com]
>> >Sent: Wednesday, April 13, 2011 1:25 PM
>> >To: Allan, Bruce W
>> >Cc: netdev@vger.kernel.org
>> >Subject: Re: [net-next-2.6 RFC PATCH v2 01/13] ethtool: allow custom interval
>> >for physical identification
>> >
>> >I'm sure there ought to be a clearer way to do this, and to avoid any
>> >weird effects from integer overflow in the multiplication.  How about
>> >using an inner loop for each second:
>> >
>> >		/* Driver expects to be called at twice the frequency in rc */
>> >		int n = rc * 2, i, interval = HZ / n;
>> >
>
>		/* Count down seconds */
>> >		do {
>			/* Count down iterations per second */
>> >			i = n;
>> >			do {
>> >	 			rtnl_lock();
>> > 				rc = dev->ethtool_ops->set_phys_id(
>> >					dev, (i & 1) ? ETHTOOL_ID_OFF : ETHTOOL_ID_ON);
>> >	 			rtnl_unlock();
>> > 				if (rc)
>> > 					break;
>> >				schedule_timeout_interruptible(interval);
>> >			} while (!signal_pending(current) && --i != 0);
>> > 		} while (!signal_pending(current) &&
>> >			 (id.data == 0 || --id.data != 0));
>> >
>> >Ben.
>>
>> OK, if that is clearer to you...v3 forthcoming.
>
>I guess it wouldn't hurt to add comemnts too.  Would you agree that it's
>clear with the additions above?
>
>Ben.

Sure, makes sense to me.

Thanks,
Bruce.

Re: [net-next-2.6 RFC PATCH v2 08/13] pcnet32: set ethtool set_phys_id on/off cycle frequency to 2/sec

[Don Fry]

> Date: Wed, 13 Apr 2011 12:59:27 -0700
> 
> Physical identification frequency based on how it was done prior to the
> introduction of set_phys_id.  Compile tested only.
> 
> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Don Fry <pcnet32@frontier.com>

Acked-by: Don Fry <pcnet32@frontier.com>