Date:   Sat, 4 Mar 2023 16:04:28 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc:     =?iso-8859-1?Q?K=F6ry?= Maincent <kory.maincent@bootlin.com>,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        linux-omap@vger.kernel.org, Michael Walle <michael@walle.cc>,
        Richard Cochran <richardcochran@gmail.com>,
        thomas.petazzoni@bootlin.com, Jay Vosburgh <j.vosburgh@gmail.com>,
        Veaceslav Falico <vfalico@gmail.com>,
        Andy Gospodarek <andy@greyhouse.net>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Joakim Zhang <qiangqing.zhang@nxp.com>,
        Vladimir Oltean <vladimir.oltean@nxp.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        UNGLinuxDriver@microchip.com,
        Grygorii Strashko <grygorii.strashko@ti.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        Minghao Chi <chi.minghao@zte.com.cn>,
        Jie Wang <wangjie125@huawei.com>,
        Guangbin Huang <huangguangbin2@huawei.com>,
        Wolfram Sang <wsa+renesas@sang-engineering.com>,
        Wang Yufen <wangyufen@huawei.com>,
        Alexandru Tachici <alexandru.tachici@analog.com>,
        Oleksij Rempel <linux@rempel-privat.de>
Subject: Re: [PATCH v2 3/4] net: Let the active time stamping layer be
 selectable.
Message-ID: <ZANd/LE8Jk+0gPdI@lunn.ch>
References: <20230303164248.499286-1-kory.maincent@bootlin.com>
 <20230303164248.499286-4-kory.maincent@bootlin.com>
 <640289d5ef54c_cc8e2087a@willemb.c.googlers.com.notmuch>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <640289d5ef54c_cc8e2087a@willemb.c.googlers.com.notmuch>
Precedence: bulk

> Should setting netdev->selected_timestamping_layer be limited to
> choices that the device supports?
> 
> At a higher level, this series assumes that any timestamp not through
> phydev is a MAC timestamp. I don't think that is necessarily true for
> all devices. Some may timestamp at the phy, but not expose a phydev.
> This is a somewhat pedantic point. I understand that the purpose of
> the series is to select from among two sets of APIs.

Network drivers tend to fall into one of two classes.

1) Linux drives the whole hardware, MAC, PCS, PHY, SPF cage, LEDs etc.

2) Linux drives just the MAC, and the rest is hidden away by firmware.

For this to work, the MAC API should be sufficient to configure and
get status information for things which are hidden away from Linux.
An example of this is the ethtool .get_link_ksettings, which mostly
deals with PHY settings. Those which have linux controlling the
hardware call phy_ethtool_get_link_ksettings to get phylib to do the
work, where as if the hardware is hidden away, calls into the firmware
are made to implement the API.

When we are talking about time stamping, i assume you are talking
about firmware driver the lower level hardware. I can see two ways
this could go:

1) The MAC driver registers two timestamping devices with the core,
one for the MAC and another for the PHY. In that case, all we need is
the registration API to include some sort of indicator what layer this
time stamper works at. The core can then expose to user space that
there are two, and mux kAPI calls to one or the other.

2) Despite the hardware having two stampers, it only exposes one to
the PTP core. Firmware driven hardware already has intimate knowledge
of the hardware, since it has to have firmware to drive the hardware,
so it should be able to say which is the better stamper. So it just
exposes that one.

So i think the proposed API does work for firmware driven stampers,
but we might need to extend ptp_caps to include a level indication,
MAC, bump in the wire, PHY, etc.

     Andrew