Received-SPF: pass (google.com: domain of linux-kernel+bounces-69158-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
References: <20240216-feature_ptp_netnext-v8-0-510f42f444fb@bootlin.com>
 <20240216-feature_ptp_netnext-v8-6-510f42f444fb@bootlin.com>
User-agent: mu4e 1.10.8; emacs 28.2
From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
To: Kory Maincent <kory.maincent@bootlin.com>
Cc: 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>, Jakub Kicinski <kuba@kernel.org>, 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>, Vladimir
 Oltean <vladimir.oltean@nxp.com>, 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 RFC net-next v8 06/13] net: Add struct
 kernel_ethtool_ts_info
Date: Fri, 16 Feb 2024 10:27:32 -0800
In-reply-to: <20240216-feature_ptp_netnext-v8-6-510f42f444fb@bootlin.com>
Message-ID: <87frxsgt4i.fsf@nvidia.com>
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?eBazfm7itDGr4mVJFVyww7HvVQIcjZPGWl2y/HNPlTRxSoXL4JXfN4jEXS0T?=
 =?us-ascii?Q?49tMjV2/YOv16tkwPHEjGh2Ebaa9M5Z4LCdodnLLyWLNHz9SsuPaDbr5kuo6?=
 =?us-ascii?Q?gjqnmA7qQKjWNxIKB9aVlpenOMYr5w4p0xsz6oLbodu29zu2cpLKlUqn4cvy?=
 =?us-ascii?Q?rtwvwYXYG7gYtOcUjyLwEVQJmJQT9C0mcg/MyHsMMBctubYoCb+KYcx+mLt5?=
 =?us-ascii?Q?85YQoOY+XbYaU0vtlw5kdKWsk0m5E+oRN2FfBAdIu9f36nQbu9x3+dWHeF1M?=
 =?us-ascii?Q?2bR+5MWoNCab1HikKpx8/Dgy7anvhoa9nZyuLEw8nl4hKgubrubTfzbEhgzt?=
 =?us-ascii?Q?QIPAwOPeD9XBrMy1F2Oum9N1vyQFKwwI+yB3uw9II2mhl0eXr1BVV7QQUP7o?=
 =?us-ascii?Q?MiVssZBIzsNSBRIGkH08fK3H2+wUOUsZTtxelu+mNYyHzjI+MJ+aGFe1Zxz6?=
 =?us-ascii?Q?xrM2/dBBvsSiE8YN4qz/kQEjJd+DTA76oWf912wFCoBSZ+hEswV7xl8Wt/w7?=
 =?us-ascii?Q?35zjVKsk+397cqxaMoHnib+R2mugTuN5d4Snz48bUk4EOEbZzSvqsGN7Cndu?=
 =?us-ascii?Q?j2IQdXv5HFMVBlQb8t+MLTYZ465jOKLACfDnilw14fxIpNnUDfceIGFq3AXc?=
 =?us-ascii?Q?Cgdmp/JDqh81OyY2H+Wg9oZvKHbliIOF0dGX5h/dgD2+S7ecRMMhjH8ZqkrI?=
 =?us-ascii?Q?bRzrl98NxQCFyAyP75H0QHZZcSjJ46TqZlL1x9vsgTZWHVHSBFujXsi4UEuD?=
 =?us-ascii?Q?okQA8YWoJtPfQMROtokDUNg6GHK6yAPLYAWuAfSvb0kxlIw9y/lnjsyGMwSY?=
 =?us-ascii?Q?UjE6usfXCVX6DaaoVGsPVutSpYhpdIWB7lnCEpdXmy5dUnka29ceEauJU+9n?=
 =?us-ascii?Q?Yeee0f1E/no50gcDBzviowXnV9eglkUWSxolWAnqW3yHH2Ix16H59wpS/MGq?=
 =?us-ascii?Q?15tGVFQCUSSRM5QWooyCpTqf6RLsAYKpcxtFLVhUmGQfDmYf3gmmeF9ZB7Fc?=
 =?us-ascii?Q?s05iCxguukDG6IjnEaA5GEVxixtgiuwgsqnO5RNq7jVPd+tEYgY1HWhO2mPm?=
 =?us-ascii?Q?50NJjP/o8+JOrzJRJfAoBiBzuEMtZvfaYu4XQp3dSDytxyvl25IMQScA+4Je?=
 =?us-ascii?Q?+oWmXHZnqKCVsx7pZZDEUQCo6Juvny7KHJMfYir/rgcyvzdTEm2sAgrxasB+?=
 =?us-ascii?Q?lDUOv+bumhTBf/JvT8mJF5A8KnV7dd6nv97sK7SUB8aWESCzI/mwcyEb5Vk6?=
 =?us-ascii?Q?BwYnmJgzUBtxAg+zAJ1OZnzmj2p9gG2v8G5GAblTP5z8prVqQp6GcWy9ReXx?=
 =?us-ascii?Q?OnFMNnMQtnhoG6wFWyvztdelHKmsFZUS/TWRIhIz3xPpjbN/a2Z2iNzTY3Bz?=
 =?us-ascii?Q?p1Im+0neKE8eOziOjFPrMN2LglAOKTC/fRFJDLbko9tsIt6jFw/AvOmlDBEP?=
 =?us-ascii?Q?4xUBIxhmavFjT4GKvdkWlrEAUYuJG557iXqpkv4y7xuIWCCapOw9xqOlfKpo?=
 =?us-ascii?Q?lPajcfo3MaYBJdWiZxIKNrKYBWKrx04kSfJxYc05By+2as8JaTQk/x6c766U?=
 =?us-ascii?Q?KRy7PsNzD7Z/tj9Qv6Bclduw2qhqlghVWjQ0kG8mtNG5t5Gf9tC1U8PH+8Pc?=
 =?us-ascii?Q?rQ=3D=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ab299b75-130d-48c2-8daf-08dc2f1d3920
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB2743.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Feb 2024 18:29:33.9894
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: enNP7yuO4gG07BWXQ+7Qh/GfpVZB9glDaytMB5xpjMl23cX0UymcmFxocdJAtvy+sG6ObzBZn9wzFIZtsTPr1w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB6139

On Fri, 16 Feb, 2024 16:52:24 +0100 Kory Maincent <kory.maincent@bootlin.com> wrote:
> In prevision to add new UAPI for hwtstamp we will be limited to the struct
> ethtool_ts_info that is currently passed in fixed binary format through the
> ETHTOOL_GET_TS_INFO ethtool ioctl. It would be good if new kernel code
> already started operating on an extensible kernel variant of that
> structure, similar in concept to struct kernel_hwtstamp_config vs struct
> hwtstamp_config.
>
> Since struct ethtool_ts_info is in include/uapi/linux/ethtool.h, here
> we introduce the kernel-only structure in include/linux/ethtool.h.
> The manual copy is then made in the function called by ETHTOOL_GET_TS_INFO.
>
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
>  drivers/net/bonding/bond_main.c                    |  4 ++--
>  drivers/net/can/dev/dev.c                          |  2 +-
>  drivers/net/can/peak_canfd/peak_canfd.c            |  2 +-
>  drivers/net/can/usb/gs_usb.c                       |  2 +-
>  drivers/net/can/usb/peak_usb/pcan_usb_core.c       |  2 +-
>  drivers/net/can/usb/peak_usb/pcan_usb_core.h       |  2 +-
>  drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c    |  2 +-
>  drivers/net/dsa/hirschmann/hellcreek_hwtstamp.h    |  2 +-
>  drivers/net/dsa/microchip/ksz_ptp.c                |  2 +-
>  drivers/net/dsa/microchip/ksz_ptp.h                |  2 +-
>  drivers/net/dsa/mv88e6xxx/hwtstamp.c               |  2 +-
>  drivers/net/dsa/mv88e6xxx/hwtstamp.h               |  4 ++--
>  drivers/net/dsa/ocelot/felix.c                     |  2 +-
>  drivers/net/dsa/sja1105/sja1105_ptp.c              |  2 +-
>  drivers/net/dsa/sja1105/sja1105_ptp.h              |  2 +-
>  drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c       |  2 +-
>  .../net/ethernet/aquantia/atlantic/aq_ethtool.c    |  2 +-
>  .../net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c    |  2 +-
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c  |  2 +-
>  drivers/net/ethernet/broadcom/tg3.c                |  2 +-
>  drivers/net/ethernet/cadence/macb.h                |  2 +-
>  drivers/net/ethernet/cadence/macb_main.c           |  4 ++--
>  drivers/net/ethernet/cavium/liquidio/lio_ethtool.c |  2 +-
>  .../net/ethernet/cavium/thunder/nicvf_ethtool.c    |  2 +-
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c |  2 +-
>  drivers/net/ethernet/cisco/enic/enic_ethtool.c     |  2 +-
>  drivers/net/ethernet/engleder/tsnep_ethtool.c      |  2 +-
>  drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c |  2 +-
>  .../net/ethernet/freescale/dpaa2/dpaa2-ethtool.c   |  2 +-
>  .../net/ethernet/freescale/enetc/enetc_ethtool.c   |  2 +-
>  drivers/net/ethernet/freescale/fec_main.c          |  2 +-
>  drivers/net/ethernet/freescale/gianfar_ethtool.c   |  2 +-
>  .../net/ethernet/fungible/funeth/funeth_ethtool.c  |  2 +-
>  drivers/net/ethernet/hisilicon/hns3/hnae3.h        |  2 +-
>  drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c |  2 +-
>  .../net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.c |  2 +-
>  .../net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h |  2 +-
>  drivers/net/ethernet/intel/e1000e/ethtool.c        |  2 +-
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |  2 +-
>  drivers/net/ethernet/intel/ice/ice_ethtool.c       |  2 +-
>  drivers/net/ethernet/intel/igb/igb_ethtool.c       |  2 +-
>  drivers/net/ethernet/intel/igc/igc_ethtool.c       |  2 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c   |  2 +-
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c    |  2 +-
>  .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c  |  2 +-
>  drivers/net/ethernet/mellanox/mlx4/en_ethtool.c    |  2 +-
>  drivers/net/ethernet/mellanox/mlx5/core/en.h       |  2 +-
>  .../net/ethernet/mellanox/mlx5/core/en_ethtool.c   |  4 ++--
>  .../ethernet/mellanox/mlx5/core/ipoib/ethtool.c    |  2 +-
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  2 +-
>  .../net/ethernet/mellanox/mlxsw/spectrum_ethtool.c |  2 +-
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c |  4 ++--
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h | 10 ++++-----
>  drivers/net/ethernet/microchip/lan743x_ethtool.c   |  2 +-
>  .../ethernet/microchip/lan966x/lan966x_ethtool.c   |  2 +-
>  .../net/ethernet/microchip/sparx5/sparx5_ethtool.c |  2 +-
>  drivers/net/ethernet/mscc/ocelot_net.c             |  2 +-
>  drivers/net/ethernet/mscc/ocelot_ptp.c             |  2 +-
>  .../net/ethernet/pensando/ionic/ionic_ethtool.c    |  2 +-
>  drivers/net/ethernet/qlogic/qede/qede_ethtool.c    |  2 +-
>  drivers/net/ethernet/qlogic/qede/qede_ptp.c        |  2 +-
>  drivers/net/ethernet/qlogic/qede/qede_ptp.h        |  2 +-
>  drivers/net/ethernet/renesas/ravb_main.c           |  2 +-
>  drivers/net/ethernet/renesas/rswitch.c             |  2 +-
>  drivers/net/ethernet/sfc/ethtool.c                 |  2 +-
>  drivers/net/ethernet/sfc/falcon/nic.h              |  2 +-
>  drivers/net/ethernet/sfc/ptp.c                     |  2 +-
>  drivers/net/ethernet/sfc/ptp.h                     |  5 +++--
>  drivers/net/ethernet/sfc/siena/ethtool.c           |  2 +-
>  drivers/net/ethernet/sfc/siena/ptp.c               |  2 +-
>  drivers/net/ethernet/sfc/siena/ptp.h               |  4 ++--
>  .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   |  2 +-
>  drivers/net/ethernet/ti/am65-cpsw-ethtool.c        |  2 +-
>  drivers/net/ethernet/ti/cpsw_ethtool.c             |  4 ++--
>  drivers/net/ethernet/ti/cpsw_priv.h                |  2 +-
>  drivers/net/ethernet/ti/icssg/icssg_ethtool.c      |  2 +-
>  drivers/net/ethernet/ti/netcp_ethss.c              |  4 ++--
>  drivers/net/ethernet/xscale/ixp4xx_eth.c           |  2 +-
>  drivers/net/macvlan.c                              |  2 +-
>  drivers/net/netdevsim/ethtool.c                    |  2 +-
>  drivers/net/phy/bcm-phy-ptp.c                      |  2 +-
>  drivers/net/phy/dp83640.c                          |  2 +-
>  drivers/net/phy/marvell_ptp.c                      |  2 +-
>  drivers/net/phy/micrel.c                           |  4 ++--
>  drivers/net/phy/mscc/mscc_ptp.c                    |  2 +-
>  drivers/net/phy/nxp-c45-tja11xx.c                  |  2 +-
>  drivers/ptp/ptp_ines.c                             |  2 +-
>  drivers/s390/net/qeth_ethtool.c                    |  2 +-
>  include/linux/can/dev.h                            |  2 +-
>  include/linux/ethtool.h                            | 25 +++++++++++++++++++---
>  include/linux/mii_timestamper.h                    |  2 +-
>  include/linux/phy.h                                |  2 +-
>  include/net/dsa.h                                  |  2 +-
>  include/soc/mscc/ocelot.h                          |  2 +-
>  net/8021q/vlan_dev.c                               |  2 +-
>  net/dsa/user.c                                     |  2 +-
>  net/ethtool/common.c                               |  6 +++---
>  net/ethtool/common.h                               |  2 +-
>  net/ethtool/ioctl.c                                | 12 +++++++++--
>  net/ethtool/tsinfo.c                               |  6 +++---
>  net/sched/sch_taprio.c                             |  2 +-
>  101 files changed, 150 insertions(+), 122 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index b90c33607594..f74190a8d9ee 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -18,6 +18,7 @@
>  #include <linux/if_ether.h>
>  #include <linux/netlink.h>
>  #include <uapi/linux/ethtool.h>
> +#include <uapi/linux/net_tstamp.h>
>  
>  struct compat_ethtool_rx_flow_spec {
>  	u32		flow_type;
> @@ -644,6 +645,22 @@ struct ethtool_rxfh_param {
>  	u8	input_xfrm;
>  };
>  
> +/**
> + * struct kernel_ethtool_ts_info - kernel copy of struct ethtool_ts_info
> + * @cmd: command number = %ETHTOOL_GET_TS_INFO
> + * @so_timestamping: bit mask of the sum of the supported SO_TIMESTAMPING flags
> + * @phc_index: device index of the associated PHC, or -1 if there is none
> + * @tx_types: bit mask of the supported hwtstamp_tx_types enumeration values
> + * @rx_filters: bit mask of the supported hwtstamp_rx_filters enumeration values
> + */
> +struct kernel_ethtool_ts_info {
> +	u32 cmd;
> +	u32 so_timestamping;
> +	int phc_index;
> +	enum hwtstamp_tx_types tx_types;

I might just be terrible at reading code, but isn't this what
tx_reserved in ethtool_ts_info is for? I feel like using one of the
reserved memspaces for this enum removes the need to make
kernel_ethtool_ts_info? I might be missing something obvious however.

> +	enum hwtstamp_rx_filters rx_filters;
> +};

--
Thanks,

Rahul Rameshbabu