Re: [PATCH net-next 1/3] net: mdio: add support for passing a PTP system timestamp to the mii_bus driver

[Andrew Lunn <andrew@lunn.ch>]

On Fri, Aug 16, 2019 at 06:31:55PM +0200, Hubert Feurstein wrote:
> In order to improve the synchronisation precision of phc2sys (from
> the linuxptp project) for devices like switches which are attached
> to the MDIO bus, it is necessary the get the system timestamps as
> close as possible to the access which causes the PTP timestamp
> register to be snapshotted in the switch hardware. Usually this is
> triggered by an MDIO write access, the snapshotted timestamp is then
> transferred by several MDIO reads.
> 
> This patch adds the required infrastructure to solve the problem described
> above.
> 
> Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
> ---
>  drivers/net/phy/mdio_bus.c | 105 +++++++++++++++++++++++++++++++++++++
>  include/linux/mdio.h       |   7 +++
>  include/linux/phy.h        |  25 +++++++++
>  3 files changed, 137 insertions(+)
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index bd04fe762056..167a21f267fa 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -34,6 +34,7 @@
>  #include <linux/phy.h>
>  #include <linux/io.h>
>  #include <linux/uaccess.h>
> +#include <linux/ptp_clock_kernel.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/mdio.h>
> @@ -697,6 +698,110 @@ int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
>  }
>  EXPORT_SYMBOL(mdiobus_write);
>  
> +/**
> + * __mdiobus_write_sts - Unlocked version of the mdiobus_write_sts function
> + * @bus: the mii_bus struct
> + * @addr: the phy address
> + * @regnum: register number to write
> + * @val: value to write to @regnum
> + * @sts: the ptp system timestamp
> + *
> + * Write a MDIO bus register and request the MDIO bus driver to take the
> + * system timestamps when sts-pointer is valid. When the bus driver doesn't
> + * support this, the timestamps are taken in this function instead.
> + *
> + * In order to improve the synchronisation precision of phc2sys (from
> + * the linuxptp project) for devices like switches which are attached
> + * to the MDIO bus, it is necessary the get the system timestamps as
> + * close as possible to the access which causes the PTP timestamp
> + * register to be snapshotted in the switch hardware. Usually this is
> + * triggered by an MDIO write access, the snapshotted timestamp is then
> + * transferred by several MDIO reads.
> + *
> + * Caller must hold the mdio bus lock.
> + *
> + * NOTE: MUST NOT be called from interrupt context.
> + */
> +int __mdiobus_write_sts(struct mii_bus *bus, int addr, u32 regnum, u16 val,
> +			struct ptp_system_timestamp *sts)
> +{
> +	int retval;
> +
> +	WARN_ON_ONCE(!mutex_is_locked(&bus->mdio_lock));
> +
> +	if (!bus->ptp_sts_supported)
> +		ptp_read_system_prets(sts);

How expensive is ptp_read_system_prets()? My original suggestion was
to unconditionally call it here, and then let the driver overwrite it
if it supports finer grained time stamping. MDIO is slow, so as long
as ptp_read_system_prets() is not too expensive, i prefer KISS.

   Andrew