diff mbox

[1/2] net: dsa: mv88e6xxx: sleep in _mv88e6xxx_stats_wait

Message ID 1971016402.292095.1437232999941.JavaMail.zimbra@savoirfairelinux.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Vivien Didelot July 18, 2015, 3:23 p.m. UTC
Hi all,

----- On Jul 18, 2015, at 10:58 AM, Andrew Lunn andrew@lunn.ch wrote:

>> Good point. The timeout is most definitely quite large and for sure on
>> the safe side. It might make sense to add some statistics gathering to
>> see how long the maximum observed delay actually is.
> 
> Hi All
> 
> Statistics are something which can be used a lot, i bursts and
> interactivily. ATU, VTU etc, are much less often used. So different
> delays might be justified.
> 
> I agree about doing some statistics gathering to determine actual
> delays needed.
> 
>        Andrew

What do you think about something like this?

Thanks,
-v

--
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

Comments

Andrew Lunn July 18, 2015, 3:28 p.m. UTC | #1
On Sat, Jul 18, 2015 at 11:23:19AM -0400, Vivien Didelot wrote:
> Hi all,
> 
> ----- On Jul 18, 2015, at 10:58 AM, Andrew Lunn andrew@lunn.ch wrote:
> 
> >> Good point. The timeout is most definitely quite large and for sure on
> >> the safe side. It might make sense to add some statistics gathering to
> >> see how long the maximum observed delay actually is.
> > 
> > Hi All
> > 
> > Statistics are something which can be used a lot, i bursts and
> > interactivily. ATU, VTU etc, are much less often used. So different
> > delays might be justified.
> > 
> > I agree about doing some statistics gathering to determine actual
> > delays needed.
> > 
> >        Andrew
> 
> What do you think about something like this?

Hi Vivien

Lets get some actually statistics first. I would suggest for testing
you make _mv88e6xxx_wait() a busy loop and time how long it actually
takes for different busy bits to clear. We should also test it on
different families.

	  Andrew
--
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/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 4f3701f..6471807 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -563,9 +563,10 @@  static bool mv88e6xxx_6352_family(struct dsa_switch *ds)
 
 /* Must be called with SMI lock held */
 static int _mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset,
-			   u16 mask)
+			   u16 mask, unsigned int msecs)
 {
 	unsigned long timeout = jiffies + HZ / 10;
+	unsigned long usecs = msecs * 1000;
 
 	while (time_before(jiffies, timeout)) {
 		int ret;
@@ -576,7 +577,8 @@  static int _mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset,
 		if (!(ret & mask))
 			return 0;
 
-		usleep_range(1000, 2000);
+		if (usecs)
+			usleep_range(usecs, usecs + 1000);
 	}
 	return -ETIMEDOUT;
 }
@@ -585,7 +587,7 @@  static int _mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset,
 static int _mv88e6xxx_stats_wait(struct dsa_switch *ds)
 {
 	return _mv88e6xxx_wait(ds, REG_GLOBAL, GLOBAL_STATS_OP,
-			       GLOBAL_STATS_OP_BUSY);
+			       GLOBAL_STATS_OP_BUSY, 0);
 }
 
 /* Must be called with SMI mutex held */
@@ -872,13 +874,14 @@  error:
 }
 #endif /* CONFIG_NET_DSA_HWMON */
 
-static int mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset, u16 mask)
+static int mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset, u16 mask,
+			  unsigned int msecs)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
 	mutex_lock(&ps->smi_mutex);
-	ret = _mv88e6xxx_wait(ds, reg, offset, mask);
+	ret = _mv88e6xxx_wait(ds, reg, offset, mask, msecs);
 	mutex_unlock(&ps->smi_mutex);
 
 	return ret;
@@ -887,33 +890,33 @@  static int mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset, u16 mask)
 static int _mv88e6xxx_phy_wait(struct dsa_switch *ds)
 {
 	return _mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_SMI_OP,
-			       GLOBAL2_SMI_OP_BUSY);
+			       GLOBAL2_SMI_OP_BUSY, 1);
 }
 
 int mv88e6xxx_eeprom_load_wait(struct dsa_switch *ds)
 {
 	return mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_EEPROM_OP,
-			      GLOBAL2_EEPROM_OP_LOAD);
+			      GLOBAL2_EEPROM_OP_LOAD, 1);
 }
 
 int mv88e6xxx_eeprom_busy_wait(struct dsa_switch *ds)
 {
 	return mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_EEPROM_OP,
-			      GLOBAL2_EEPROM_OP_BUSY);
+			      GLOBAL2_EEPROM_OP_BUSY, 1);
 }
 
 /* Must be called with SMI lock held */
 static int _mv88e6xxx_atu_wait(struct dsa_switch *ds)
 {
 	return _mv88e6xxx_wait(ds, REG_GLOBAL, GLOBAL_ATU_OP,
-			       GLOBAL_ATU_OP_BUSY);
+			       GLOBAL_ATU_OP_BUSY, 1);
 }
 
 /* Must be called with SMI lock held */
 static int _mv88e6xxx_scratch_wait(struct dsa_switch *ds)
 {
 	return _mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_SCRATCH_MISC,
-			       GLOBAL2_SCRATCH_BUSY);
+			       GLOBAL2_SCRATCH_BUSY, 1);
 }
 
 /* Must be called with SMI mutex held */