Hi Andrew, Am Mo., 5. Aug. 2019 um 15:58 Uhr schrieb Andrew Lunn <and...@lunn.ch>: > > On Mon, Aug 05, 2019 at 10:26:42AM +0200, Hubert Feurstein wrote: > > From: Hubert Feurstein <h.feurst...@gmail.com> > > Hi Hubert > > In your RFC patch, there was some interesting numbers. Can you provide > numbers of just this patch? How much of an improvement does it make? > > Your RFC patch pushed these ptp_read_system_{pre|post}ts() calls down > into the MDIO driver. This patch is much less invasive, but i wonder > how much a penalty you paid?
I mentioned the numbers already in my RFC mail. Without this patch a quick test-run gave me this result: Min: -17829 ns Max: 21694 ns StdDev: 8520 ns Count: 127 With this patch applied, the results improved: Min: -12144 ns Max: 10891 ns StdDev: 4046,71 ns Count: 112 I know, the sample count for the statistics (only 112 samples!) is not really big. However, even with this low number of samples I already got too high min / max values. > > Did you also try moving these calls into global2_avb.c, around the one > write that really matters? > > It was speculated that the jitter comes from contention on the mdio > bus lock. Did you investigate this? If you can prove this true, one > thing to try is to take the mdio bus lock in the mv88e6xxx driver, > take the start timestamp, call __mdiobus_write(), and then the end > timestamp. The bus contention is then outside your time snapshot. > I've tested this now: diff --git a/drivers/net/dsa/mv88e6xxx/smi.c b/drivers/net/dsa/mv88e6xxx/smi.c index 801fd4abba5a..fc7f9318df52 100644 --- a/drivers/net/dsa/mv88e6xxx/smi.c +++ b/drivers/net/dsa/mv88e6xxx/smi.c @@ -40,12 +40,27 @@ static int mv88e6xxx_smi_direct_read(struct mv88e6xxx_chip *chip, return 0; } +static int mv88e6xxx_mdiobus_write_nested(struct mv88e6xxx_chip *chip, int addr, u32 regnum, u16 val) +{ + int err; + + BUG_ON(in_interrupt()); + + mutex_lock_nested(&chip->bus->mdio_lock, MDIO_MUTEX_NESTED); + ptp_read_system_prets(chip->ptp_sts); + err = __mdiobus_write(chip->bus, addr, regnum, val); + ptp_read_system_postts(chip->ptp_sts); + mutex_unlock(&chip->bus->mdio_lock); + + return err; +} + static int mv88e6xxx_smi_direct_write(struct mv88e6xxx_chip *chip, int dev, int reg, u16 data) { int ret; - ret = mdiobus_write_nested_ptp(chip->bus, dev, reg, data, chip->ptp_sts); + ret = mv88e6xxx_mdiobus_write_nested(chip, dev, reg, data); if (ret < 0) return ret; The result was: Min: -8052 Max: 9988 StdDev: 2490.17 Count: 3592 It got improved, but you still have the unpredictable latencies caused by the mdio_done-completion (=> wait_for_completion_timeout) in imx_fec. > We could even think about adding a mdiobus_write variant which does > all this. I'm sure other DSA drivers would find it useful, if it > really does help. > > Andrew Hubert