Date:   Sat, 4 Mar 2023 16:43:47 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     =?iso-8859-1?Q?K=F6ry?= Maincent <kory.maincent@bootlin.com>
Cc:     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: <011d63c3-e3ff-4b67-8ab7-d39f541c7b31@lunn.ch>
References: <20230303164248.499286-1-kory.maincent@bootlin.com>
 <20230303164248.499286-4-kory.maincent@bootlin.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230303164248.499286-4-kory.maincent@bootlin.com>
Precedence: bulk

On Fri, Mar 03, 2023 at 05:42:40PM +0100, K?ry Maincent wrote:
> From: Richard Cochran <richardcochran@gmail.com>
> 
> Make the sysfs knob writable, and add checks in the ioctl and time
> stamping paths to respect the currently selected time stamping layer.

Although it probably works, i think the ioctl code is ugly.

I think it would be better to pull the IOCTL code into the PTP object
interface. Add an ioctl member to struct ptp_clock_info. The PTP core
can then directly call into the PTP object.

You now have a rather odd semantic that calling the .ndo_eth_ioctl
means operate on the MAC PTP. If you look at net_device_ops, i don't
think any of the other members have this semantic. They all look at
the netdev as a whole, and ask the netdev to do something, without
caring what level it operates at. So a PTP ioctl should operate on
'the' PTP of the netdev, whichever that might be, MAC or PHY.

Clearly, it is a bigger change, you need to touch every MAC driver
with PTP support, but at the end, you have a cleaner architecture.

     Andrew