Messages in this thread | | | Date | Mon, 5 Aug 2019 19:40:51 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH net-next v2] net: dsa: mv88e6xxx: extend PTP gettime function to read system clock |
| |
> +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.
O.K. So lets think about a more generic solution we can use inside the mdio bus driver. I don't know if adding an sts pointer to struct device will be accepted. But adding one to struct mii_bus should be O.K. It can be assigned to once the mdio_lock is taken, to avoid race conditions. Add mdio_ptp_read_system_prets(bus) and mdio_ptp_read_system_postts(bus) which the bus driver can use.
We also need a fallback in case the bus driver does not use them, so something like:
mdiobus_write_sts(...) { int retval;
BUG_ON(in_interrupt());
mutex_lock(&bus->mdio_lock); bus->sts = sts; sts->post_ts = 0;
ktime_get_real_ts64(&sts->pre_ts);
retval = __mdiobus_write(bus, addr, regnum, val);
if (!sts->post_ts) ktime_get_real_ts64(sts->post_ts)
bus->sts = NULL; mutex_unlock(&bus->mdio_lock);
return retval; }
So at worse case, we get the time around the whole write operation, but the MDIO bus driver can overwrite the pre_ts and set post_ts, using mdio_ptp_read_system_prets(bus) and mdio_ptp_read_system_postts(bus).
A similar scheme could be implemented to SPI devices, if the SPI maintainer would accepted a sts pointer in the SPI bus driver structure.
Andrew
| |