Date:   Fri, 10 Mar 2023 15:04:36 +0100
From:   =?UTF-8?B?S8O2cnk=?= Maincent <kory.maincent@bootlin.com>
To:     Michael Walle <michael@walle.cc>
Cc:     Horatiu Vultur <horatiu.vultur@microchip.com>,
        Vladimir Oltean <vladimir.oltean@nxp.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>, netdev@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-omap@vger.kernel.org,
        Maxime Chevallier <maxime.chevallier@bootlin.com>,
        Richard Cochran <richardcochran@gmail.com>,
        thomas.petazzoni@bootlin.com, Russell King <linux@armlinux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Jay Vosburgh <j.vosburgh@gmail.com>,
        Veaceslav Falico <vfalico@gmail.com>,
        Andy Gospodarek <andy@greyhouse.net>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        UNGLinuxDriver@microchip.com, Minghao Chi <chi.minghao@zte.com.cn>,
        Jie Wang <wangjie125@huawei.com>,
        Oleksij Rempel <linux@rempel-privat.de>,
        Sean Anderson <sean.anderson@seco.com>,
        "Gustavo A. R. Silva" <gustavoars@kernel.org>,
        Wolfram Sang <wsa+renesas@sang-engineering.com>,
        Alexander Lobakin <alexandr.lobakin@intel.com>,
        Marco Bonelli <marco@mebeim.net>
Subject: Re: [PATCH v3 3/5] net: Let the active time stamping layer be
 selectable.
Message-ID: <20230310150436.40ed168d@kmaincent-XPS-13-7390>
In-Reply-To: <0d2304a9bc276a0d321629108cf8febd@walle.cc>
References: <20230308135936.761794-1-kory.maincent@bootlin.com>
        <20230308135936.761794-1-kory.maincent@bootlin.com>
        <20230308135936.761794-4-kory.maincent@bootlin.com>
        <20230308135936.761794-4-kory.maincent@bootlin.com>
        <20230308230321.liw3v255okrhxg6s@skbuf>
        <20230310114852.3cef643d@kmaincent-XPS-13-7390>
        <20230310113533.l7flaoli7y3bmlnr@skbuf>
        <b4ebfd3770ffa5ad1233d2b5e79499ee@walle.cc>
        <20230310131529.6bahmi4obryy5dsx@soft-dev3-1>
        <0d2304a9bc276a0d321629108cf8febd@walle.cc>
Organization: bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, 10 Mar 2023 14:34:07 +0100
Michael Walle <michael@walle.cc> wrote:

> >> > There is a slight inconvenience caused by the fact that there are
> >> > already PHY drivers using PHY timestamping, and those may have been
> >> > introduced into deployments with PHY timestamping. We cannot change =
the
> >> > default behavior for those either. There are 5 such PHY drivers today
> >> > (I've grepped for mii_timestamper in drivers/net/phy).
> >> >
> >> > I would suggest that the kernel implements a short whitelist of 5
> >> > entries containing PHY driver names, which are compared against
> >> > netdev->phydev->drv->name (with the appropriate NULL pointer checks).
> >> > Matches will default to PHY timestamping. Otherwise, the new default
> >> > will be to keep the behavior as if PHY timestamping doesn't exist
> >> > (MAC still provides the timestamps), and the user needs to select the
> >> > PHY as the timestamping source explicitly.
> >> >
> >> > Thoughts? =20
> >>=20
> >> While I agree in principle (I have suggested to make MAC timestamping
> >> the default before), I see a problem with the recent LAN8814 PHY
> >> timestamping support, which will likely be released with 6.3. That
> >> would now switch the timestamping to PHY timestamping for our board
> >> (arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-8g.dts). I could
> >> argue that is a regression for our board iff NETWORK_PHY_TIMESTAMPING
> >> is enabled. Honestly, I don't know how to proceed here and haven't
> >> tried to replicate the regression due to limited time. Assuming,
> >> that I can show it is a regression, what would be the solution then,
> >> reverting the commit? Horatiu, any ideas?

Adding this whitelist will add some PHY driver specific name in the phy API
core.
Will it be accepted? Is it not better to add a "legacy_default_timestamping"
boolean in the phy_device struct and set it for these 5 PHY drivers?
Then move on the default behavior to MAC default timestamping on the otehr
cases.
=20
> >> I digress from the original problem a bit. But if there would be such
> >> a whitelist, I'd propose that it won't contain the lan8814 driver. =20
> >=20
> > I don't have anything against having a whitelist the PHY driver names. =
=20
>=20
> Yeah, but my problem right now is, that if this discussion won't find
> any good solution, the lan8814 phy timestamping will find it's way
> into an official kernel and then it is really hard to undo things.

Yes and we need to find a solution as the issue will raise at each new PHY =
PTP
support.

K=C3=B6ry