Re: [PATCH net-next v2] net: dsa: mv88e6xxx: extend PTP gettime function to read system clock

[Andrew Lunn]

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