Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 19 Jun 2023 10:58:31 +0200
From:   Horatiu Vultur <horatiu.vultur@microchip.com>
To:     "Radu Pirea (NXP OSS)" <radu-nicolae.pirea@oss.nxp.com>
CC:     <andrew@lunn.ch>, <hkallweit1@gmail.com>, <linux@armlinux.org.uk>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <richardcochran@gmail.com>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <sebastian.tobuschat@nxp.com>
Subject: Re: [PATCH net-next v1 14/14] net: phy: nxp-c45-tja11xx: timestamp
 reading workaround for TJA1120
Message-ID: <20230619085831.dnzg2i5mqysc6r3r@soft-dev3-1>
References: <20230616135323.98215-1-radu-nicolae.pirea@oss.nxp.com>
 <20230616135323.98215-15-radu-nicolae.pirea@oss.nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
In-Reply-To: <20230616135323.98215-15-radu-nicolae.pirea@oss.nxp.com>
Precedence: bulk

The 06/16/2023 16:53, Radu Pirea (NXP OSS) wrote:

Hi Radu,

> 
> On TJA1120 engineering samples, the new timestamp is stuck in the FIFO.
> If the MORE_TS bit is set and the VALID bit is not set, we know that we
> have a timestamp in the FIFO but not in the buffer.
> 
> To move the new timestamp in the buffer registers, the current
> timestamp(which is invalid) is unlocked by writing any of the buffer
> registers.

Shouldn't this be split and merged in patch 9 and patch 10?
As those two patches introduced this functions with issues.

> 
> Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@oss.nxp.com>
> ---
>  drivers/net/phy/nxp-c45-tja11xx.c | 31 ++++++++++++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
> index 0d22eb7534dc..3543c8fe099c 100644
> --- a/drivers/net/phy/nxp-c45-tja11xx.c
> +++ b/drivers/net/phy/nxp-c45-tja11xx.c
> @@ -532,15 +532,30 @@ static bool nxp_c45_get_extts(struct nxp_c45_phy *priv,
>  static bool tja1120_get_extts(struct nxp_c45_phy *priv,
>                               struct timespec64 *extts)
>  {
> +       const struct nxp_c45_regmap *regmap = nxp_c45_get_regmap(priv->phydev);
> +       bool more_ts;
>         bool valid;
>         u16 reg;
> 
> +       reg = phy_read_mmd(priv->phydev, MDIO_MMD_VEND1,
> +                          regmap->vend1_ext_trg_ctrl);
> +       more_ts = !!(reg & TJA1120_MORE_TS);
> +
>         reg = phy_read_mmd(priv->phydev, MDIO_MMD_VEND1,
>                            TJA1120_VEND1_PTP_TRIG_DATA_S);
>         valid = !!(reg & TJA1120_TS_VALID);
>         if (valid)
>                 return nxp_c45_get_extts(priv, extts);
> 
> +       /* Bug workaround for TJA1120 enegineering samples: move the new
> +        * timestamp from the FIFO to the buffer.
> +        */
> +       if (more_ts) {
> +               phy_write_mmd(priv->phydev, MDIO_MMD_VEND1,
> +                             regmap->vend1_ext_trg_ctrl, RING_DONE);
> +               return nxp_c45_get_extts(priv, extts);
> +       }
> +
>         return valid;
>  }
> 
> @@ -588,15 +603,25 @@ static bool tja1120_get_hwtxts(struct nxp_c45_phy *priv,
>                                struct nxp_c45_hwts *hwts)
>  {
>         struct phy_device *phydev = priv->phydev;
> +       bool more_ts;
>         bool valid;
>         u16 reg;
> 
>         mutex_lock(&priv->ptp_lock);
> +       reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, TJA1120_EGRESS_TS_END);
> +       more_ts = !!(reg & TJA1120_MORE_TS);
>         reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, TJA1120_EGRESS_TS_DATA_S);
>         valid = !!(reg & TJA1120_TS_VALID);
> -       if (!valid)
> -               goto tja1120_get_hwtxts_out;
> -
> +       if (!valid) {
> +               if (!more_ts)
> +                       goto tja1120_get_hwtxts_out;
> +               /* Bug workaround for TJA1120 enegineering samples: move the
> +                * new timestamp from the FIFO to the buffer.
> +                */
> +               phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1,
> +                                  TJA1120_EGRESS_TS_END, TJA1120_TS_VALID);
> +               valid = true;
> +       }
>         nxp_c45_read_egress_ts(priv, hwts);
>         phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, TJA1120_EGRESS_TS_DATA_S,
>                            TJA1120_TS_VALID);
> --
> 2.34.1
> 
> 

-- 
/Horatiu