Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Mon, 20 Nov 2023 12:52:55 +0200
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     =?utf-8?B?S8O2cnk=?= Maincent <kory.maincent@bootlin.com>
Cc:     Jakub Kicinski <kuba@kernel.org>,
        Florian Fainelli <florian.fainelli@broadcom.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@broadcom.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Richard Cochran <richardcochran@gmail.com>,
        Radu Pirea <radu-nicolae.pirea@oss.nxp.com>,
        Jay Vosburgh <j.vosburgh@gmail.com>,
        Andy Gospodarek <andy@greyhouse.net>,
        Nicolas Ferre <nicolas.ferre@microchip.com>,
        Claudiu Beznea <claudiu.beznea@tuxon.dev>,
        Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Horatiu Vultur <horatiu.vultur@microchip.com>,
        UNGLinuxDriver@microchip.com, Simon Horman <horms@kernel.org>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-doc@vger.kernel.org,
        Maxime Chevallier <maxime.chevallier@bootlin.com>
Subject: Re: [PATCH net-next v7 15/16] net: ethtool: ts: Let the active time
 stamping layer be selectable
Message-ID: <20231120105255.cgbart5amkg4efaz@skbuf>
References: <20231114-feature_ptp_netnext-v7-0-472e77951e40@bootlin.com>
 <20231114-feature_ptp_netnext-v7-15-472e77951e40@bootlin.com>
 <20231118183433.30ca1d1a@kernel.org>
 <20231120104439.15bfdd09@kmaincent-XPS-13-7390>
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231120104439.15bfdd09@kmaincent-XPS-13-7390>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?l+h7SPV5/XWn5N5xEnk4KVS19Y9Viwpg9RbuegzDlRIdR929E95GChpOfT?=
 =?iso-8859-1?Q?rCqHrK0y/4+rl+ya558Rua2FBCAwCeDNVYrrofp0liw/zxVNiXU9ZpbBBB?=
 =?iso-8859-1?Q?Q1K5YUlN1HSLYdpbxk8IDfZJabzjPpMuuvqjXQ1DvdqYb261vmPgTG8W8Q?=
 =?iso-8859-1?Q?iSfxj0PUDliPVG+hQaRvEDvRMoq5vKHRCOxCg/q9Gyuh5USbSDaoLEVy54?=
 =?iso-8859-1?Q?bkr/CP6Y2mJ3q4jZubauC2nDGVqZ0oGVMlvkiY34j4uor80wqUFBJO6Qfe?=
 =?iso-8859-1?Q?gdrpGLo/Na2S2TocwU6Ybc/PIpGnb+Dm6KVsUx1fVZpryfEYBxABrI86gj?=
 =?iso-8859-1?Q?2UWC5SWTX9WaCOm1gGB7eq0IMoMuyFgrbC9VqfGj2FKH2PqRIpdQLOWiWQ?=
 =?iso-8859-1?Q?wNjjK8tUqD2gwEYuPYa8mmbZyIDC3hnYYCUqNprSfgRb5y9QAuy7/d/vIT?=
 =?iso-8859-1?Q?48pIJiCNzvgs0zfpJMrEYUVNmb+aBw6IWQoNK+RopXQJNw72ShAoO0ZIEG?=
 =?iso-8859-1?Q?cIxO3ti9+znmZho7s/9ZPtduTa8Zjn+ykFz1EWkXJVymyyy3hfV1tV0RI0?=
 =?iso-8859-1?Q?MAYDetrxFDmasA+GH4dBlUrMg4yKVr095hRn6EFLqcpyHYIqpWtYmIfrRJ?=
 =?iso-8859-1?Q?cR0lS5sHpbcobQA0a1vO6FS4AmVJ6/HpUlHl6gKg8h077JshRJvzfX11D/?=
 =?iso-8859-1?Q?92i9LOT58+j/ZMDmuNYlk6anLC92ShEipTLFJ+u+KJqUCxhwvZJv514Ki0?=
 =?iso-8859-1?Q?So+tw8pw1P4MZCCu3qYIITQKjdlupQM4mH+W55mgJrKaJkpc13L4LGYykW?=
 =?iso-8859-1?Q?fOtYOPIG9CywFa4SJZNgG695I94hOEGfKVTOTo0Hrk4Bd9+JM2/CahhDq5?=
 =?iso-8859-1?Q?De6ghXBIv/Ha9Hgk5PuzZZ5mpf8rHSYYpQykWWzFRCaE7EZEQlZ7xiEbE3?=
 =?iso-8859-1?Q?GTLglnBXFaCRJFv+nCvBh7xCJpGVFUHdKc2ovpvMKkrE4QZZ2D549X5hHA?=
 =?iso-8859-1?Q?p70HUjzRVbR0wesbLlhBP53DrfXSZSebmN5nqSqJ4Zzrd7nMAdtwSPfGXf?=
 =?iso-8859-1?Q?71jDpbd7aPzk2Rw6eMqFRVOESnBKlCsVpzSu8V2Q5oFbOWXVwCm2yaxsPI?=
 =?iso-8859-1?Q?RA2/15OrvqPSEIo8qUm2IPFehRmJU3GHM1O4nisq7u4b0jWZkR1W5wZk22?=
 =?iso-8859-1?Q?mqLxvRb32bZ9LNDub9WGg9nRBbRcLHpqZwA5WpXgd5fqgxLPjHPbjDu0pE?=
 =?iso-8859-1?Q?b3M2GC9zwsaJ2B/B0EWdklh8CFFbvagM6NR8LSfJ1yFQiJrKufKCNyp5I0?=
 =?iso-8859-1?Q?8njHCVs4bmDHfOHu8lGbfwwMd8r5KdT2HugvLL3hCB7qgBOLrWxfr0K+SC?=
 =?iso-8859-1?Q?qScNAsyMDWHEFs0f5QafMf12VycXdDvSVXx4iTNXevD4pwAJrF6vztVMHe?=
 =?iso-8859-1?Q?wk+AY8ZnfYEe8PkAuhuJFpYHzEX6Ra+GBm7w7w1GdXbCdZ8MeA7J38aJC/?=
 =?iso-8859-1?Q?JyOCTCPLoXvgl3/8oQVRdOYw5aQJle+bsxTnPAZ5yCP94p5huJtG8iSmaz?=
 =?iso-8859-1?Q?wx+dqeDKCoQ3qNz2Q7bxX3HLCvRdMSuS8+JlRj1lrqsIXmQDO1EILBpJ8V?=
 =?iso-8859-1?Q?137082akMUjh+spg96fs+lKti8FgELMw4pXaY4pxeeCw4ZpoGEwDT9GA?=
 =?iso-8859-1?Q?=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3758b8cc-08bc-4d58-124b-08dbe9b6dd2c
X-MS-Exchange-CrossTenant-AuthSource: AM0PR04MB6452.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Nov 2023 10:53:00.9915
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2ME+/HgL1+P++uIZG1fY5TpBvC+H9mNiL6jejay5IP4gqLvOfDsDM+S4WwwG3jg/Pj++vvkhlhrqdu2ZD73y0g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB9122
X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED,
	HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,
	SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Mon, 20 Nov 2023 02:53:34 -0800 (PST)

Hi K?ry,

On Mon, Nov 20, 2023 at 10:44:39AM +0100, K?ry Maincent wrote:
> On Sat, 18 Nov 2023 18:34:33 -0800
> Jakub Kicinski <kuba@kernel.org> wrote:
> 
> > On Tue, 14 Nov 2023 12:28:43 +0100 Kory Maincent wrote:
> > > +	if (!tb[ETHTOOL_A_TS_LAYER])
> > > +		return 0;  
> > 
> > GENL_REQ_ATTR_CHECK(), not sure why anyone would issue this command
> > without any useful attr.
> > 
> > > +	/* Disable time stamping in the current layer. */
> > > +	if (netif_device_present(dev) &&
> > > +	    (dev->ts_layer == PHY_TIMESTAMPING ||
> > > +	    dev->ts_layer == MAC_TIMESTAMPING)) {
> > > +		ret = dev_set_hwtstamp_phylib(dev, &config, info->extack);
> > > +		if (ret < 0)
> > > +			return ret;  
> > 
> > So you only support PHYLIB?
> > 
> > The semantics need to be better documented :(
> 
> Yes as we don't really know how each MAC deal with the timestamping
> before ndo_hwstamp_get/set. Using phylib only allows us to be sure these NDO are
> implemented and the management of timestamping is coherent in the MAC. Also It
> will push people to move on to these NDOs.
> 
> Ok I will add documentation.
> 
> -- 
> K?ry Maincent, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com/

When Jakub says "the semantics need to be better documented", I'm also
thinking of a different direction.

From what I understand, Maxime is working on representing multiple
phylib PHYs in the UAPI:
https://patchwork.kernel.org/project/netdevbpf/cover/20231117162323.626979-1-maxime.chevallier@bootlin.com/

Does your UAPI proposal make it possible in any way to select
timestamping in phylib PHY A rather than PHY B? Or do you think it is
extensible to support that, somehow?