Re: [PATCH net-next v3 4/4] net: fec: add support for PTP system timestamping for MDIO devices
flat view
From: Vladimir Oltean <olteanv@gmail.com>
Date: 2019-08-21 10:28:24
Also in:
lkml
On Tue, 20 Aug 2019 at 11:49, Hubert Feurstein [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Hubert Feurstein <redacted> 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. The ptp_read_system_*ts functions already check the ptp_sts pointer. Signed-off-by: Hubert Feurstein <redacted> --- drivers/net/ethernet/freescale/fec_main.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index c01d3ec3e9af..dd1253683ac0 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c@@ -1815,10 +1815,12 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum, reinit_completion(&fep->mdio_done); /* start a write op */ + ptp_read_system_prets(bus->ptp_sts); writel(FEC_MMFR_ST | FEC_MMFR_OP_WRITE | FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) | FEC_MMFR_TA | FEC_MMFR_DATA(value), fep->hwp + FEC_MII_DATA); + ptp_read_system_postts(bus->ptp_sts);
How do you know the core will not service an interrupt here? Why are you not disabling (postponing) local interrupts after this critical section? (which you were in the RFC) If the argument is that you didn't notice any issue with phc2sys -N 5, that's not a good argument. "Unlikely for a condition to happen" does not mean deterministic. Here is an example of the system servicing an interrupt during the transmission of a 12-byte SPI transfer (proof that they can occur anywhere where they aren't disabled): https://drive.google.com/file/d/1rUZpfkBKHJGwQN4orFUWks_5i70wn-mj/view?usp=sharing
quoted hunk ↗ jump to hunk
/* wait for end of transfer */ time_left = wait_for_completion_timeout(&fep->mdio_done,@@ -1956,7 +1958,7 @@ static int fec_enet_mii_init(struct platform_device *pdev) struct fec_enet_private *fep = netdev_priv(ndev); struct device_node *node; int err = -ENXIO; - u32 mii_speed, holdtime; + u32 mii_speed, mii_period, holdtime; /* * The i.MX28 dual fec interfaces are not equal.@@ -1993,6 +1995,7 @@ static int fec_enet_mii_init(struct platform_device *pdev) * document. */ mii_speed = DIV_ROUND_UP(clk_get_rate(fep->clk_ipg), 5000000); + mii_period = div_u64((u64)mii_speed * 2 * NSEC_PER_SEC, clk_get_rate(fep->clk_ipg)); if (fep->quirks & FEC_QUIRK_ENET_MAC) mii_speed--; if (mii_speed > 63) {@@ -2034,6 +2037,8 @@ static int fec_enet_mii_init(struct platform_device *pdev) pdev->name, fep->dev_id + 1); fep->mii_bus->priv = fep; fep->mii_bus->parent = &pdev->dev; + fep->mii_bus->flags = MII_BUS_F_PTP_STS_SUPPORTED; + fep->mii_bus->ptp_sts_offset = 32 * mii_period; node = of_get_child_by_name(pdev->dev.of_node, "mdio"); err = of_mdiobus_register(fep->mii_bus, node); --2.22.1
Regards, -Vladimir