[PATCH net-next v5 00/16] net: Make timestamping selectable

[PATCH net-next v5 00/16] net: Make timestamping selectable

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Up until now, there was no way to let the user select the layer at
which time stamping occurs. The stack assumed that PHY time stamping
is always preferred, but some MAC/PHY combinations were buggy.

This series updates the default MAC/PHY default timestamping and aims to
allow the user to select the desired layer administratively.

Changes in v2:
- Move selected_timestamping_layer variable of the concerned patch.
- Use sysfs_streq instead of strmcmp.
- Use the PHY timestamp only if available.

Changes in v3:
- Expose the PTP choice to ethtool instead of sysfs.
  You can test it with the ethtool source on branch feature_ptp of:
  https://github.com/kmaincent/ethtool
- Added a devicetree binding to select the preferred timestamp.

Changes in v4:
- Move on to ethtool netlink instead of ioctl.
- Add a netdev notifier to allow packet trapping by the MAC in case of PHY
  time stamping.
- Add a PHY whitelist to not break the old PHY default time-stamping
  preference API.

Change in v5:
- Update to ndo_hwstamp_get/set. This bring several new patches.
- Add few patches to make the glue.
- Convert macb to ndo_hwstamp_get/set.
- Add netlink specs description of new ethtool commands.
- Removed netdev notifier.
- Split the patches that expose the timestamping to userspace to separate
  the core and ethtool development.
- Add description of software timestamping.
- Convert PHYs hwtstamp callback to use kernel_hwtstamp_config.

Kory Maincent (15):
  net: Convert PHYs hwtstamp callback to use kernel_hwtstamp_config
  net: phy: Remove the call to phy_mii_ioctl in phy_hwstamp_get/set
  net: macb: Convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  net: Make dev_set_hwtstamp_phylib accessible
  net_tstamp: Add TIMESTAMPING SOFTWARE and HARDWARE mask
  net: phy: micrel: fix ts_info value in case of no phc
  net: ethtool: Add a command to expose current time stamping layer
  netlink: specs: Introduce new netlink command to get current timestamp
  net: ethtool: Add a command to list available time stamping layers
  netlink: specs: Introduce new netlink command to list available time
    stamping layers
  net: Replace hwtstamp_source by timestamping layer
  net: Change the API of PHY default timestamp to MAC
  net: ethtool: ts: Update GET_TS to reply the current selected
    timestamp
  net ethtool: net: Let the active time stamping layer be selectable
  netlink: specs: Introduce time stamping set command

Richard Cochran (1):
  net: ethtool: Refactor identical get_ts_info implementations.

 Documentation/netlink/specs/ethtool.yaml      |  57 +++++
 Documentation/networking/ethtool-netlink.rst  |  63 ++++++
 drivers/net/bonding/bond_main.c               |  27 +--
 drivers/net/ethernet/cadence/macb.h           |  15 +-
 drivers/net/ethernet/cadence/macb_main.c      |  42 +++-
 drivers/net/ethernet/cadence/macb_ptp.c       |  28 +--
 .../ethernet/microchip/lan966x/lan966x_main.c |   6 +-
 drivers/net/macvlan.c                         |  14 +-
 drivers/net/phy/bcm-phy-ptp.c                 |  15 +-
 drivers/net/phy/dp83640.c                     |  24 +-
 drivers/net/phy/micrel.c                      |  44 ++--
 drivers/net/phy/mscc/mscc_ptp.c               |  18 +-
 drivers/net/phy/nxp-c45-tja11xx.c             |  17 +-
 drivers/net/phy/phy.c                         |  28 ++-
 drivers/net/phy/phy_device.c                  |  68 ++++++
 drivers/ptp/ptp_ines.c                        |  16 +-
 include/linux/ethtool.h                       |   8 +
 include/linux/mii_timestamper.h               |   4 +-
 include/linux/net_tstamp.h                    |  11 +-
 include/linux/netdevice.h                     |   8 +
 include/linux/phy.h                           |   6 +-
 include/uapi/linux/ethtool_netlink.h          |  29 +++
 include/uapi/linux/net_tstamp.h               |  22 ++
 net/8021q/vlan_dev.c                          |  15 +-
 net/core/dev.c                                |   3 +
 net/core/dev_ioctl.c                          |  42 ++--
 net/core/timestamping.c                       |   9 +
 net/ethtool/Makefile                          |   2 +-
 net/ethtool/common.c                          |  22 +-
 net/ethtool/common.h                          |   1 +
 net/ethtool/netlink.c                         |  28 +++
 net/ethtool/netlink.h                         |   4 +
 net/ethtool/ts.c                              | 210 ++++++++++++++++++
 33 files changed, 707 insertions(+), 199 deletions(-)
 create mode 100644 net/ethtool/ts.c

-- 
2.25.1

[PATCH net-next v5 01/16] net: Convert PHYs hwtstamp callback to use kernel_hwtstamp_config

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

The PHYs hwtstamp callback are still getting the timestamp config from
ifreq and using copy_from/to_user.
Get rid of these functions by using timestamp configuration in parameter.
This also allow to move on to kernel_hwtstamp_config and be similar to
net devices using the new ndo_hwstamp_get/set.

This adds the possibility to manipulate the timestamp configuration
from the kernel which was not possible with the copy_from/to_user.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 drivers/net/phy/bcm-phy-ptp.c     | 15 +++++-------
 drivers/net/phy/dp83640.c         | 24 +++++++++----------
 drivers/net/phy/micrel.c          | 38 ++++++++++++++-----------------
 drivers/net/phy/mscc/mscc_ptp.c   | 18 +++++++--------
 drivers/net/phy/nxp-c45-tja11xx.c | 17 ++++++--------
 drivers/net/phy/phy.c             | 21 +++++++++++++++--
 drivers/ptp/ptp_ines.c            | 16 ++++++-------
 include/linux/mii_timestamper.h   |  4 +++-
 include/linux/phy.h               |  6 +++--
 9 files changed, 82 insertions(+), 77 deletions(-)

diff --git a/drivers/net/phy/bcm-phy-ptp.c b/drivers/net/phy/bcm-phy-ptp.c
index ef00d6163061..0beae8f81ffa 100644
--- a/drivers/net/phy/bcm-phy-ptp.c
+++ b/drivers/net/phy/bcm-phy-ptp.c
@@ -782,16 +782,13 @@ static void bcm_ptp_txtstamp(struct mii_timestamper *mii_ts,
 }
 
 static int bcm_ptp_hwtstamp(struct mii_timestamper *mii_ts,
-			    struct ifreq *ifr)
+			    struct kernel_hwtstamp_config *cfg,
+			    struct netlink_ext_ack *extack)
 {
 	struct bcm_ptp_private *priv = mii2priv(mii_ts);
-	struct hwtstamp_config cfg;
 	u16 mode, ctrl;
 
-	if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
-		return -EFAULT;
-
-	switch (cfg.rx_filter) {
+	switch (cfg->rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
 		priv->hwts_rx = false;
 		break;
@@ -804,14 +801,14 @@ static int bcm_ptp_hwtstamp(struct mii_timestamper *mii_ts,
 	case HWTSTAMP_FILTER_PTP_V2_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
-		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
+		cfg->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
 		priv->hwts_rx = true;
 		break;
 	default:
 		return -ERANGE;
 	}
 
-	priv->tx_type = cfg.tx_type;
+	priv->tx_type = cfg->tx_type;
 
 	ctrl  = priv->hwts_rx ? SLICE_RX_EN : 0;
 	ctrl |= priv->tx_type != HWTSTAMP_TX_OFF ? SLICE_TX_EN : 0;
@@ -840,7 +837,7 @@ static int bcm_ptp_hwtstamp(struct mii_timestamper *mii_ts,
 	/* purge existing data */
 	skb_queue_purge(&priv->tx_queue);
 
-	return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
+	return 0;
 }
 
 static int bcm_ptp_ts_info(struct mii_timestamper *mii_ts,
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 2657be7cc049..5c42c47dc564 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -1207,22 +1207,20 @@ static irqreturn_t dp83640_handle_interrupt(struct phy_device *phydev)
 	return IRQ_HANDLED;
 }
 
-static int dp83640_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
+static int dp83640_hwtstamp(struct mii_timestamper *mii_ts,
+			    struct kernel_hwtstamp_config *cfg,
+			    struct netlink_ext_ack *extack)
 {
 	struct dp83640_private *dp83640 =
 		container_of(mii_ts, struct dp83640_private, mii_ts);
-	struct hwtstamp_config cfg;
 	u16 txcfg0, rxcfg0;
 
-	if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
-		return -EFAULT;
-
-	if (cfg.tx_type < 0 || cfg.tx_type > HWTSTAMP_TX_ONESTEP_SYNC)
+	if (cfg->tx_type < 0 || cfg->tx_type > HWTSTAMP_TX_ONESTEP_SYNC)
 		return -ERANGE;
 
-	dp83640->hwts_tx_en = cfg.tx_type;
+	dp83640->hwts_tx_en = cfg->tx_type;
 
-	switch (cfg.rx_filter) {
+	switch (cfg->rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
 		dp83640->hwts_rx_en = 0;
 		dp83640->layer = 0;
@@ -1234,7 +1232,7 @@ static int dp83640_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 		dp83640->hwts_rx_en = 1;
 		dp83640->layer = PTP_CLASS_L4;
 		dp83640->version = PTP_CLASS_V1;
-		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
+		cfg->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
@@ -1242,7 +1240,7 @@ static int dp83640_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 		dp83640->hwts_rx_en = 1;
 		dp83640->layer = PTP_CLASS_L4;
 		dp83640->version = PTP_CLASS_V2;
-		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
+		cfg->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
@@ -1250,7 +1248,7 @@ static int dp83640_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 		dp83640->hwts_rx_en = 1;
 		dp83640->layer = PTP_CLASS_L2;
 		dp83640->version = PTP_CLASS_V2;
-		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
+		cfg->rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_SYNC:
@@ -1258,7 +1256,7 @@ static int dp83640_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 		dp83640->hwts_rx_en = 1;
 		dp83640->layer = PTP_CLASS_L4 | PTP_CLASS_L2;
 		dp83640->version = PTP_CLASS_V2;
-		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
+		cfg->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
 		break;
 	default:
 		return -ERANGE;
@@ -1292,7 +1290,7 @@ static int dp83640_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 
 	mutex_unlock(&dp83640->clock->extreg_lock);
 
-	return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
+	return 0;
 }
 
 static void rx_timestamp_work(struct work_struct *work)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 927d3d54658e..43d072b53839 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -2373,24 +2373,22 @@ static void lan8814_flush_fifo(struct phy_device *phydev, bool egress)
 	lanphy_read_page_reg(phydev, 5, PTP_TSU_INT_STS);
 }
 
-static int lan8814_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
+static int lan8814_hwtstamp(struct mii_timestamper *mii_ts,
+			    struct kernel_hwtstamp_config *config,
+			    struct netlink_ext_ack *extack)
 {
 	struct kszphy_ptp_priv *ptp_priv =
 			  container_of(mii_ts, struct kszphy_ptp_priv, mii_ts);
 	struct phy_device *phydev = ptp_priv->phydev;
 	struct lan8814_shared_priv *shared = phydev->shared->priv;
 	struct lan8814_ptp_rx_ts *rx_ts, *tmp;
-	struct hwtstamp_config config;
 	int txcfg = 0, rxcfg = 0;
 	int pkt_ts_enable;
 
-	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
-		return -EFAULT;
+	ptp_priv->hwts_tx_type = config->tx_type;
+	ptp_priv->rx_filter = config->rx_filter;
 
-	ptp_priv->hwts_tx_type = config.tx_type;
-	ptp_priv->rx_filter = config.rx_filter;
-
-	switch (config.rx_filter) {
+	switch (config->rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
 		ptp_priv->layer = 0;
 		ptp_priv->version = 0;
@@ -2436,13 +2434,13 @@ static int lan8814_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 		lanphy_write_page_reg(ptp_priv->phydev, 5, PTP_TX_MOD,
 				      PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_);
 
-	if (config.rx_filter != HWTSTAMP_FILTER_NONE)
+	if (config->rx_filter != HWTSTAMP_FILTER_NONE)
 		lan8814_config_ts_intr(ptp_priv->phydev, true);
 	else
 		lan8814_config_ts_intr(ptp_priv->phydev, false);
 
 	mutex_lock(&shared->shared_lock);
-	if (config.rx_filter != HWTSTAMP_FILTER_NONE)
+	if (config->rx_filter != HWTSTAMP_FILTER_NONE)
 		shared->ref++;
 	else
 		shared->ref--;
@@ -2466,7 +2464,7 @@ static int lan8814_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 	lan8814_flush_fifo(ptp_priv->phydev, false);
 	lan8814_flush_fifo(ptp_priv->phydev, true);
 
-	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ? -EFAULT : 0;
+	return 0;
 }
 
 static void lan8814_txtstamp(struct mii_timestamper *mii_ts,
@@ -3681,21 +3679,19 @@ static void lan8841_ptp_enable_processing(struct kszphy_ptp_priv *ptp_priv,
 #define LAN8841_PTP_TX_TIMESTAMP_EN		443
 #define LAN8841_PTP_TX_MOD			445
 
-static int lan8841_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
+static int lan8841_hwtstamp(struct mii_timestamper *mii_ts,
+			    struct kernel_hwtstamp_config *config,
+			    struct netlink_ext_ack *extack)
 {
 	struct kszphy_ptp_priv *ptp_priv = container_of(mii_ts, struct kszphy_ptp_priv, mii_ts);
 	struct phy_device *phydev = ptp_priv->phydev;
-	struct hwtstamp_config config;
 	int txcfg = 0, rxcfg = 0;
 	int pkt_ts_enable;
 
-	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
-		return -EFAULT;
+	ptp_priv->hwts_tx_type = config->tx_type;
+	ptp_priv->rx_filter = config->rx_filter;
 
-	ptp_priv->hwts_tx_type = config.tx_type;
-	ptp_priv->rx_filter = config.rx_filter;
-
-	switch (config.rx_filter) {
+	switch (config->rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
 		ptp_priv->layer = 0;
 		ptp_priv->version = 0;
@@ -3749,13 +3745,13 @@ static int lan8841_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 
 	/* Now enable/disable the timestamping */
 	lan8841_ptp_enable_processing(ptp_priv,
-				      config.rx_filter != HWTSTAMP_FILTER_NONE);
+				      config->rx_filter != HWTSTAMP_FILTER_NONE);
 
 	skb_queue_purge(&ptp_priv->tx_queue);
 
 	lan8841_ptp_flush_fifo(ptp_priv);
 
-	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ? -EFAULT : 0;
+	return 0;
 }
 
 static bool lan8841_rxtstamp(struct mii_timestamper *mii_ts,
diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
index cf728bfd83e2..eb0b032cb613 100644
--- a/drivers/net/phy/mscc/mscc_ptp.c
+++ b/drivers/net/phy/mscc/mscc_ptp.c
@@ -1045,19 +1045,17 @@ static void vsc85xx_ts_reset_fifo(struct phy_device *phydev)
 			     val);
 }
 
-static int vsc85xx_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
+static int vsc85xx_hwtstamp(struct mii_timestamper *mii_ts,
+			    struct kernel_hwtstamp_config *cfg,
+			    struct netlink_ext_ack *extack)
 {
 	struct vsc8531_private *vsc8531 =
 		container_of(mii_ts, struct vsc8531_private, mii_ts);
 	struct phy_device *phydev = vsc8531->ptp->phydev;
-	struct hwtstamp_config cfg;
 	bool one_step = false;
 	u32 val;
 
-	if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
-		return -EFAULT;
-
-	switch (cfg.tx_type) {
+	switch (cfg->tx_type) {
 	case HWTSTAMP_TX_ONESTEP_SYNC:
 		one_step = true;
 		break;
@@ -1069,9 +1067,9 @@ static int vsc85xx_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 		return -ERANGE;
 	}
 
-	vsc8531->ptp->tx_type = cfg.tx_type;
+	vsc8531->ptp->tx_type = cfg->tx_type;
 
-	switch (cfg.rx_filter) {
+	switch (cfg->rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
@@ -1084,7 +1082,7 @@ static int vsc85xx_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 		return -ERANGE;
 	}
 
-	vsc8531->ptp->rx_filter = cfg.rx_filter;
+	vsc8531->ptp->rx_filter = cfg->rx_filter;
 
 	mutex_lock(&vsc8531->ts_lock);
 
@@ -1132,7 +1130,7 @@ static int vsc85xx_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 	vsc8531->ptp->configured = 1;
 	mutex_unlock(&vsc8531->ts_lock);
 
-	return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
+	return 0;
 }
 
 static int vsc85xx_ts_info(struct mii_timestamper *mii_ts,
diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
index 7ab080ff02df..416484ea6eb3 100644
--- a/drivers/net/phy/nxp-c45-tja11xx.c
+++ b/drivers/net/phy/nxp-c45-tja11xx.c
@@ -1022,24 +1022,21 @@ static bool nxp_c45_rxtstamp(struct mii_timestamper *mii_ts,
 }
 
 static int nxp_c45_hwtstamp(struct mii_timestamper *mii_ts,
-			    struct ifreq *ifreq)
+			    struct kernel_hwtstamp_config *config,
+			    struct netlink_ext_ack *extack)
 {
 	struct nxp_c45_phy *priv = container_of(mii_ts, struct nxp_c45_phy,
 						mii_ts);
 	struct phy_device *phydev = priv->phydev;
 	const struct nxp_c45_phy_data *data;
-	struct hwtstamp_config cfg;
 
-	if (copy_from_user(&cfg, ifreq->ifr_data, sizeof(cfg)))
-		return -EFAULT;
-
-	if (cfg.tx_type < 0 || cfg.tx_type > HWTSTAMP_TX_ON)
+	if (cfg->tx_type < 0 || cfg->tx_type > HWTSTAMP_TX_ON)
 		return -ERANGE;
 
 	data = nxp_c45_get_data(phydev);
-	priv->hwts_tx = cfg.tx_type;
+	priv->hwts_tx = cfg->tx_type;
 
-	switch (cfg.rx_filter) {
+	switch (cfg->rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
 		priv->hwts_rx = 0;
 		break;
@@ -1047,7 +1044,7 @@ static int nxp_c45_hwtstamp(struct mii_timestamper *mii_ts,
 	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
 		priv->hwts_rx = 1;
-		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
+		cfg->rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
 		break;
 	default:
 		return -ERANGE;
@@ -1074,7 +1071,7 @@ static int nxp_c45_hwtstamp(struct mii_timestamper *mii_ts,
 		nxp_c45_clear_reg_field(phydev, &data->regmap->irq_egr_ts_en);
 
 nxp_c45_no_ptp_irq:
-	return copy_to_user(ifreq->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
+	return 0;
 }
 
 static int nxp_c45_ts_info(struct mii_timestamper *mii_ts,
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index a5fa077650e8..d058316666ba 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -325,9 +325,13 @@ EXPORT_SYMBOL(phy_ethtool_ksettings_get);
 int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
 {
 	struct mii_ioctl_data *mii_data = if_mii(ifr);
+	struct kernel_hwtstamp_config kernel_cfg;
+	struct netlink_ext_ack extack = {};
 	u16 val = mii_data->val_in;
 	bool change_autoneg = false;
+	struct hwtstamp_config cfg;
 	int prtad, devad;
+	int ret;
 
 	switch (cmd) {
 	case SIOCGMIIPHY:
@@ -411,8 +415,21 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
 		return 0;
 
 	case SIOCSHWTSTAMP:
-		if (phydev->mii_ts && phydev->mii_ts->hwtstamp)
-			return phydev->mii_ts->hwtstamp(phydev->mii_ts, ifr);
+		if (phydev->mii_ts && phydev->mii_ts->hwtstamp) {
+			if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
+				return -EFAULT;
+
+			hwtstamp_config_to_kernel(&kernel_cfg, &cfg);
+			ret = phydev->mii_ts->hwtstamp(phydev->mii_ts, &kernel_cfg, &extack);
+			if (ret)
+				return ret;
+
+			hwtstamp_config_from_kernel(&cfg, &kernel_cfg);
+			if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)))
+				return -EFAULT;
+
+			return 0;
+		}
 		fallthrough;
 
 	default:
diff --git a/drivers/ptp/ptp_ines.c b/drivers/ptp/ptp_ines.c
index ed215b458183..1d2940a78455 100644
--- a/drivers/ptp/ptp_ines.c
+++ b/drivers/ptp/ptp_ines.c
@@ -328,17 +328,15 @@ static u64 ines_find_txts(struct ines_port *port, struct sk_buff *skb)
 	return ns;
 }
 
-static int ines_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
+static int ines_hwtstamp(struct mii_timestamper *mii_ts,
+			 struct kernel_hwtstamp_config *cfg,
+			 struct netlink_ext_ack *extack)
 {
 	struct ines_port *port = container_of(mii_ts, struct ines_port, mii_ts);
 	u32 cm_one_step = 0, port_conf, ts_stat_rx, ts_stat_tx;
-	struct hwtstamp_config cfg;
 	unsigned long flags;
 
-	if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
-		return -EFAULT;
-
-	switch (cfg.tx_type) {
+	switch (cfg->tx_type) {
 	case HWTSTAMP_TX_OFF:
 		ts_stat_tx = 0;
 		break;
@@ -353,7 +351,7 @@ static int ines_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 		return -ERANGE;
 	}
 
-	switch (cfg.rx_filter) {
+	switch (cfg->rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
 		ts_stat_rx = 0;
 		break;
@@ -372,7 +370,7 @@ static int ines_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 	case HWTSTAMP_FILTER_PTP_V2_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
 		ts_stat_rx = TS_ENABLE;
-		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
+		cfg->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
 		break;
 	default:
 		return -ERANGE;
@@ -393,7 +391,7 @@ static int ines_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 
 	spin_unlock_irqrestore(&port->lock, flags);
 
-	return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
+	return 0;
 }
 
 static void ines_link_state(struct mii_timestamper *mii_ts,
diff --git a/include/linux/mii_timestamper.h b/include/linux/mii_timestamper.h
index fa940bbaf8ae..26b04f73f214 100644
--- a/include/linux/mii_timestamper.h
+++ b/include/linux/mii_timestamper.h
@@ -9,6 +9,7 @@
 #include <linux/device.h>
 #include <linux/ethtool.h>
 #include <linux/skbuff.h>
+#include <linux/net_tstamp.h>
 
 struct phy_device;
 
@@ -51,7 +52,8 @@ struct mii_timestamper {
 			 struct sk_buff *skb, int type);
 
 	int  (*hwtstamp)(struct mii_timestamper *mii_ts,
-			 struct ifreq *ifreq);
+			 struct kernel_hwtstamp_config *kernel_config,
+			 struct netlink_ext_ack *extack);
 
 	void (*link_state)(struct mii_timestamper *mii_ts,
 			   struct phy_device *phydev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3cc52826f18e..e5f1f41e399c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1560,9 +1560,11 @@ static inline bool phy_has_txtstamp(struct phy_device *phydev)
 	return phydev && phydev->mii_ts && phydev->mii_ts->txtstamp;
 }
 
-static inline int phy_hwtstamp(struct phy_device *phydev, struct ifreq *ifr)
+static inline int phy_hwtstamp(struct phy_device *phydev,
+			       struct kernel_hwtstamp_config *cfg,
+			       struct netlink_ext_ack *extack)
 {
-	return phydev->mii_ts->hwtstamp(phydev->mii_ts, ifr);
+	return phydev->mii_ts->hwtstamp(phydev->mii_ts, cfg, extack);
 }
 
 static inline bool phy_rxtstamp(struct phy_device *phydev, struct sk_buff *skb,
-- 
2.25.1

[PATCH net-next v5 02/16] net: phy: Remove the call to phy_mii_ioctl in phy_hwstamp_get/set

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

__phy_hwtstamp_set function were calling the phy_mii_ioctl function
which will then use the ifreq pointer to call the hwtstamp callback.
Now that ifreq has been removed from the hwstamp callback parameters
it seems more logical to not go through the phy_mii_ioctl function and pass
directly kernel_hwtstamp_config parameter to the hwtstamp callback.

Lets do the same for __phy_hwtstamp_get function and return directly
EOPNOTSUPP as SIOCGHWTSTAMP is not supported for now for the PHYs.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 drivers/net/phy/phy.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index d058316666ba..3376e58e2b88 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -486,7 +486,7 @@ int __phy_hwtstamp_get(struct phy_device *phydev,
 	if (!phydev)
 		return -ENODEV;
 
-	return phy_mii_ioctl(phydev, config->ifr, SIOCGHWTSTAMP);
+	return -EOPNOTSUPP;
 }
 
 /**
@@ -503,7 +503,10 @@ int __phy_hwtstamp_set(struct phy_device *phydev,
 	if (!phydev)
 		return -ENODEV;
 
-	return phy_mii_ioctl(phydev, config->ifr, SIOCSHWTSTAMP);
+	if (phydev->mii_ts && phydev->mii_ts->hwtstamp)
+		return phydev->mii_ts->hwtstamp(phydev->mii_ts, config, extack);
+
+	return -EOPNOTSUPP;
 }
 
 /**
-- 
2.25.1

[PATCH net-next v5 03/16] net: ethtool: Refactor identical get_ts_info implementations.

[Köry Maincent]

From: Richard Cochran <richardcochran@gmail.com>

The vlan, macvlan and the bonding drivers call their "real" device driver
in order to report the time stamping capabilities.  Provide a core
ethtool helper function to avoid copy/paste in the stack.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Change in v5:
- Fixe typo
---
 drivers/net/bonding/bond_main.c | 27 ++-------------------------
 drivers/net/macvlan.c           | 14 +-------------
 include/linux/ethtool.h         |  8 ++++++++
 net/8021q/vlan_dev.c            | 15 +--------------
 net/ethtool/common.c            |  6 ++++++
 5 files changed, 18 insertions(+), 52 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ed7212e61c54..18af563d20b2 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5763,29 +5763,12 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
 	rcu_read_unlock();
 
 	if (real_dev) {
-		ops = real_dev->ethtool_ops;
-		phydev = real_dev->phydev;
-
-		if (phy_has_tsinfo(phydev)) {
-			ret = phy_ts_info(phydev, info);
-			goto out;
-		} else if (ops->get_ts_info) {
-			ret = ops->get_ts_info(real_dev, info);
-			goto out;
-		}
+		ret = ethtool_get_ts_info_by_layer(real_dev, info);
 	} else {
 		/* Check if all slaves support software tx timestamping */
 		rcu_read_lock();
 		bond_for_each_slave_rcu(bond, slave, iter) {
-			ret = -1;
-			ops = slave->dev->ethtool_ops;
-			phydev = slave->dev->phydev;
-
-			if (phy_has_tsinfo(phydev))
-				ret = phy_ts_info(phydev, &ts_info);
-			else if (ops->get_ts_info)
-				ret = ops->get_ts_info(slave->dev, &ts_info);
-
+			ret = ethtool_get_ts_info_by_layer(slave->dev, &ts_info);
 			if (!ret && (ts_info.so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE)) {
 				sw_tx_support = true;
 				continue;
@@ -5797,15 +5780,9 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
 		rcu_read_unlock();
 	}
 
-	ret = 0;
-	info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
-				SOF_TIMESTAMPING_SOFTWARE;
 	if (sw_tx_support)
 		info->so_timestamping |= SOF_TIMESTAMPING_TX_SOFTWARE;
 
-	info->phc_index = -1;
-
-out:
 	dev_put(real_dev);
 	return ret;
 }
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 02bd201bc7e5..759406fbaea8 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1086,20 +1086,8 @@ static int macvlan_ethtool_get_ts_info(struct net_device *dev,
 				       struct ethtool_ts_info *info)
 {
 	struct net_device *real_dev = macvlan_dev_real_dev(dev);
-	const struct ethtool_ops *ops = real_dev->ethtool_ops;
-	struct phy_device *phydev = real_dev->phydev;
 
-	if (phy_has_tsinfo(phydev)) {
-		return phy_ts_info(phydev, info);
-	} else if (ops->get_ts_info) {
-		return ops->get_ts_info(real_dev, info);
-	} else {
-		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
-			SOF_TIMESTAMPING_SOFTWARE;
-		info->phc_index = -1;
-	}
-
-	return 0;
+	return ethtool_get_ts_info_by_layer(real_dev, info);
 }
 
 static netdev_features_t macvlan_fix_features(struct net_device *dev,
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 62b61527bcc4..1159daac776e 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -1043,6 +1043,14 @@ static inline int ethtool_mm_frag_size_min_to_add(u32 val_min, u32 *val_add,
 	return -EINVAL;
 }
 
+/**
+ * ethtool_get_ts_info_by_layer - Obtains time stamping capabilities from the MAC or PHY layer.
+ * @dev: pointer to net_device structure
+ * @info: buffer to hold the result
+ * Returns zero on success, non-zero otherwise.
+ */
+int ethtool_get_ts_info_by_layer(struct net_device *dev, struct ethtool_ts_info *info);
+
 /**
  * ethtool_sprintf - Write formatted string to ethtool string data
  * @data: Pointer to start of string to update
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 2a7f1b15714a..407b2335f091 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -702,20 +702,7 @@ static int vlan_ethtool_get_ts_info(struct net_device *dev,
 				    struct ethtool_ts_info *info)
 {
 	const struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
-	const struct ethtool_ops *ops = vlan->real_dev->ethtool_ops;
-	struct phy_device *phydev = vlan->real_dev->phydev;
-
-	if (phy_has_tsinfo(phydev)) {
-		return phy_ts_info(phydev, info);
-	} else if (ops->get_ts_info) {
-		return ops->get_ts_info(vlan->real_dev, info);
-	} else {
-		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
-			SOF_TIMESTAMPING_SOFTWARE;
-		info->phc_index = -1;
-	}
-
-	return 0;
+	return ethtool_get_ts_info_by_layer(vlan->real_dev, info);
 }
 
 static void vlan_dev_get_stats64(struct net_device *dev,
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index f5598c5f50de..e2315e24d695 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -661,6 +661,12 @@ int ethtool_get_phc_vclocks(struct net_device *dev, int **vclock_index)
 }
 EXPORT_SYMBOL(ethtool_get_phc_vclocks);
 
+int ethtool_get_ts_info_by_layer(struct net_device *dev, struct ethtool_ts_info *info)
+{
+	return __ethtool_get_ts_info(dev, info);
+}
+EXPORT_SYMBOL(ethtool_get_ts_info_by_layer);
+
 const struct ethtool_phy_ops *ethtool_phy_ops;
 
 void ethtool_set_ethtool_phy_ops(const struct ethtool_phy_ops *ops)
-- 
2.25.1

[PATCH net-next v5 04/16] net: macb: Convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

The hardware timestamping through ndo_eth_ioctl() is going away.
Convert the macb driver to the new API before that can be removed.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h      | 15 ++++++---
 drivers/net/ethernet/cadence/macb_main.c | 42 +++++++++++++++++++-----
 drivers/net/ethernet/cadence/macb_ptp.c  | 28 ++++++----------
 3 files changed, 53 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 78c972bb1d96..aa5700ac9c00 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1165,9 +1165,10 @@ struct macb_ptp_info {
 	int (*get_ts_info)(struct net_device *dev,
 			   struct ethtool_ts_info *info);
 	int (*get_hwtst)(struct net_device *netdev,
-			 struct ifreq *ifr);
+			 struct kernel_hwtstamp_config *tstamp_config);
 	int (*set_hwtst)(struct net_device *netdev,
-			 struct ifreq *ifr, int cmd);
+			 struct kernel_hwtstamp_config *tstamp_config,
+			 struct netlink_ext_ack *extack);
 };
 
 struct macb_pm_data {
@@ -1314,7 +1315,7 @@ struct macb {
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_clock_info;
 	struct tsu_incr tsu_incr;
-	struct hwtstamp_config tstamp_config;
+	struct kernel_hwtstamp_config tstamp_config;
 
 	/* RX queue filer rule set*/
 	struct ethtool_rx_fs_list rx_fs_list;
@@ -1363,8 +1364,12 @@ static inline void gem_ptp_do_rxstamp(struct macb *bp, struct sk_buff *skb, stru
 
 	gem_ptp_rxstamp(bp, skb, desc);
 }
-int gem_get_hwtst(struct net_device *dev, struct ifreq *rq);
-int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd);
+
+int gem_get_hwtst(struct net_device *dev,
+		  struct kernel_hwtstamp_config *tstamp_config);
+int gem_set_hwtst(struct net_device *dev,
+		  struct kernel_hwtstamp_config *tstamp_config,
+		  struct netlink_ext_ack *extack);
 #else
 static inline void gem_ptp_init(struct net_device *ndev) { }
 static inline void gem_ptp_remove(struct net_device *ndev) { }
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index cebae0f418f2..898debfd4db3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3773,18 +3773,38 @@ static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!netif_running(dev))
 		return -EINVAL;
 
-	if (bp->ptp_info) {
-		switch (cmd) {
-		case SIOCSHWTSTAMP:
-			return bp->ptp_info->set_hwtst(dev, rq, cmd);
-		case SIOCGHWTSTAMP:
-			return bp->ptp_info->get_hwtst(dev, rq);
-		}
-	}
-
 	return phylink_mii_ioctl(bp->phylink, rq, cmd);
 }
 
+static int macb_hwtstamp_get(struct net_device *dev,
+			     struct kernel_hwtstamp_config *cfg)
+{
+	struct macb *bp = netdev_priv(dev);
+
+	if (!netif_running(dev))
+		return -EINVAL;
+
+	if (!bp->ptp_info)
+		return -EOPNOTSUPP;
+
+	return bp->ptp_info->get_hwtst(dev, cfg);
+}
+
+static int macb_hwtstamp_set(struct net_device *dev,
+			     struct kernel_hwtstamp_config *cfg,
+			     struct netlink_ext_ack *extack)
+{
+	struct macb *bp = netdev_priv(dev);
+
+	if (!netif_running(dev))
+		return -EINVAL;
+
+	if (!bp->ptp_info)
+		return -EOPNOTSUPP;
+
+	return bp->ptp_info->set_hwtst(dev, cfg, extack);
+}
+
 static inline void macb_set_txcsum_feature(struct macb *bp,
 					   netdev_features_t features)
 {
@@ -3884,6 +3904,8 @@ static const struct net_device_ops macb_netdev_ops = {
 #endif
 	.ndo_set_features	= macb_set_features,
 	.ndo_features_check	= macb_features_check,
+	.ndo_hwtstamp_set	= macb_hwtstamp_set,
+	.ndo_hwtstamp_get	= macb_hwtstamp_get,
 };
 
 /* Configure peripheral capabilities according to device tree
@@ -4539,6 +4561,8 @@ static const struct net_device_ops at91ether_netdev_ops = {
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= at91ether_poll_controller,
 #endif
+	.ndo_hwtstamp_set	= macb_hwtstamp_set,
+	.ndo_hwtstamp_get	= macb_hwtstamp_get,
 };
 
 static int at91ether_clk_init(struct platform_device *pdev, struct clk **pclk,
diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index 51d26fa190d7..a63bf29c4fa8 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -374,19 +374,16 @@ static int gem_ptp_set_ts_mode(struct macb *bp,
 	return 0;
 }
 
-int gem_get_hwtst(struct net_device *dev, struct ifreq *rq)
+int gem_get_hwtst(struct net_device *dev,
+		  struct kernel_hwtstamp_config *tstamp_config)
 {
-	struct hwtstamp_config *tstamp_config;
 	struct macb *bp = netdev_priv(dev);
 
-	tstamp_config = &bp->tstamp_config;
+	*tstamp_config = bp->tstamp_config;
 	if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0)
 		return -EOPNOTSUPP;
 
-	if (copy_to_user(rq->ifr_data, tstamp_config, sizeof(*tstamp_config)))
-		return -EFAULT;
-	else
-		return 0;
+	return 0;
 }
 
 static void gem_ptp_set_one_step_sync(struct macb *bp, u8 enable)
@@ -401,22 +398,18 @@ static void gem_ptp_set_one_step_sync(struct macb *bp, u8 enable)
 		macb_writel(bp, NCR, reg_val & ~MACB_BIT(OSSMODE));
 }
 
-int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
+int gem_set_hwtst(struct net_device *dev,
+		  struct kernel_hwtstamp_config *tstamp_config,
+		  struct netlink_ext_ack *extack)
 {
 	enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
 	enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
-	struct hwtstamp_config *tstamp_config;
 	struct macb *bp = netdev_priv(dev);
 	u32 regval;
 
-	tstamp_config = &bp->tstamp_config;
 	if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0)
 		return -EOPNOTSUPP;
 
-	if (copy_from_user(tstamp_config, ifr->ifr_data,
-			   sizeof(*tstamp_config)))
-		return -EFAULT;
-
 	switch (tstamp_config->tx_type) {
 	case HWTSTAMP_TX_OFF:
 		break;
@@ -463,12 +456,11 @@ int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
 		return -ERANGE;
 	}
 
+	bp->tstamp_config = *tstamp_config;
+
 	if (gem_ptp_set_ts_mode(bp, tx_bd_control, rx_bd_control) != 0)
 		return -ERANGE;
 
-	if (copy_to_user(ifr->ifr_data, tstamp_config, sizeof(*tstamp_config)))
-		return -EFAULT;
-	else
-		return 0;
+	return 0;
 }
 
-- 
2.25.1

[PATCH net-next v5 05/16] net: Make dev_set_hwtstamp_phylib accessible

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Make the dev_set_hwtstamp_phylib function accessible in prevision to use
it from ethtool to reset the tstamp current configuration.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 include/linux/netdevice.h | 3 +++
 net/core/dev_ioctl.c      | 6 +++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e070a4540fba..b9d0411836db 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3922,6 +3922,9 @@ int generic_hwtstamp_get_lower(struct net_device *dev,
 int generic_hwtstamp_set_lower(struct net_device *dev,
 			       struct kernel_hwtstamp_config *kernel_cfg,
 			       struct netlink_ext_ack *extack);
+int dev_set_hwtstamp_phylib(struct net_device *dev,
+			    struct kernel_hwtstamp_config *cfg,
+			    struct netlink_ext_ack *extack);
 int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *userdata);
 unsigned int dev_get_flags(const struct net_device *);
 int __dev_change_flags(struct net_device *dev, unsigned int flags,
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index b46aedc36939..342a667858ac 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -322,9 +322,9 @@ static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
  * frames and not forward them), it must set IFF_SEE_ALL_HWTSTAMP_REQUESTS in
  * dev->priv_flags.
  */
-static int dev_set_hwtstamp_phylib(struct net_device *dev,
-				   struct kernel_hwtstamp_config *cfg,
-				   struct netlink_ext_ack *extack)
+int dev_set_hwtstamp_phylib(struct net_device *dev,
+			    struct kernel_hwtstamp_config *cfg,
+			    struct netlink_ext_ack *extack)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
 	bool phy_ts = phy_has_hwtstamp(dev->phydev);
-- 
2.25.1

[PATCH net-next v5 06/16] net_tstamp: Add TIMESTAMPING SOFTWARE and HARDWARE mask

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Timestamping software or hardware flags are often used as a group,
therefore adding these masks will easier future use.

I did not use SOF_TIMESTAMPING_SYS_HARDWARE flag as it is deprecated and
not use at all.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 include/uapi/linux/net_tstamp.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index a2c66b3d7f0f..df8091998c8d 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -48,6 +48,14 @@ enum {
 					 SOF_TIMESTAMPING_TX_SCHED | \
 					 SOF_TIMESTAMPING_TX_ACK)
 
+#define SOF_TIMESTAMPING_SOFTWARE_MASK	(SOF_TIMESTAMPING_RX_SOFTWARE | \
+					 SOF_TIMESTAMPING_TX_SOFTWARE | \
+					 SOF_TIMESTAMPING_SOFTWARE)
+
+#define SOF_TIMESTAMPING_HARDWARE_MASK	(SOF_TIMESTAMPING_RX_HARDWARE | \
+					 SOF_TIMESTAMPING_TX_HARDWARE | \
+					 SOF_TIMESTAMPING_RAW_HARDWARE)
+
 /**
  * struct so_timestamping - SO_TIMESTAMPING parameter
  *
-- 
2.25.1

[PATCH net-next v5 07/16] net: phy: micrel: fix ts_info value in case of no phc

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

In case of no phc we should not return SOFTWARE TIMESTAMPING flags as we do
not know the netdev supports of timestamping.
Remove it from the lan8841_ts_info and simply return 0.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

This patch is not tested but it seems consistent to me.
---
 drivers/net/phy/micrel.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 43d072b53839..4c115e55ffc0 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -3607,12 +3607,8 @@ static int lan8841_ts_info(struct mii_timestamper *mii_ts,
 
 	info->phc_index = ptp_priv->ptp_clock ?
 				ptp_clock_index(ptp_priv->ptp_clock) : -1;
-	if (info->phc_index == -1) {
-		info->so_timestamping |= SOF_TIMESTAMPING_TX_SOFTWARE |
-					 SOF_TIMESTAMPING_RX_SOFTWARE |
-					 SOF_TIMESTAMPING_SOFTWARE;
+	if (info->phc_index == -1)
 		return 0;
-	}
 
 	info->so_timestamping = SOF_TIMESTAMPING_TX_HARDWARE |
 				SOF_TIMESTAMPING_RX_HARDWARE |
-- 
2.25.1

[PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Time stamping on network packets may happen either in the MAC or in
the PHY, but not both.  In preparation for making the choice
selectable, expose both the current and available layers via ethtool.

In accordance with the kernel implementation as it stands, the current
layer will always read as "phy" when a PHY time stamping device is
present. Future patches will allow changing the current layer
administratively.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

---
Changes in v2:
- Move the introduction of selected_timestamping_layer variable in next
  patch.

Changes in v3:
- Move on to ethtool instead of syfs

Changes in v4:
- Move on to netlink ethtool instead of ioctl. I am not familiar with
  netlink so there might be some code that does not follow the good code
  practice.

Changes in v5:
- Rename timestamping layers.
- Set a default value of ts_layer in __ethtool_get_ts_info function.
- Separate TS_GET and TS_LIST_GET ethtool command in two separate patches.
- Update documentation.
---
 Documentation/networking/ethtool-netlink.rst | 23 ++++++
 include/uapi/linux/ethtool_netlink.h         | 14 ++++
 include/uapi/linux/net_tstamp.h              | 14 ++++
 net/ethtool/Makefile                         |  2 +-
 net/ethtool/common.h                         |  1 +
 net/ethtool/netlink.c                        | 10 +++
 net/ethtool/netlink.h                        |  2 +
 net/ethtool/ts.c                             | 78 ++++++++++++++++++++
 8 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 net/ethtool/ts.c

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 2540c70952ff..644b3b764044 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -225,6 +225,7 @@ Userspace to kernel:
   ``ETHTOOL_MSG_RSS_GET``               get RSS settings
   ``ETHTOOL_MSG_MM_GET``                get MAC merge layer state
   ``ETHTOOL_MSG_MM_SET``                set MAC merge layer parameters
+  ``ETHTOOL_MSG_TS_GET``                get current timestamping
   ===================================== =================================
 
 Kernel to userspace:
@@ -268,6 +269,7 @@ Kernel to userspace:
   ``ETHTOOL_MSG_PSE_GET_REPLY``            PSE parameters
   ``ETHTOOL_MSG_RSS_GET_REPLY``            RSS settings
   ``ETHTOOL_MSG_MM_GET_REPLY``             MAC merge layer status
+  ``ETHTOOL_MSG_TS_GET_REPLY``             current timestamping
   ======================================== =================================
 
 ``GET`` requests are sent by userspace applications to retrieve device
@@ -1994,6 +1996,26 @@ The attributes are propagated to the driver through the following structure:
 .. kernel-doc:: include/linux/ethtool.h
     :identifiers: ethtool_mm_cfg
 
+TS_GET
+======
+
+Gets current timestamping.
+
+Request contents:
+
+  =================================  ======  ====================
+  ``ETHTOOL_A_TS_HEADER``            nested  request header
+  =================================  ======  ====================
+
+Kernel response contents:
+
+  =======================  ======  ==============================
+  ``ETHTOOL_A_TS_HEADER``  nested  reply header
+  ``ETHTOOL_A_TS_LAYER``   u32     current timestamping
+  =======================  ======  ==============================
+
+This command get the current timestamp layer.
+
 Request translation
 ===================
 
@@ -2100,4 +2122,5 @@ are netlink only.
   n/a                                 ``ETHTOOL_MSG_PLCA_GET_STATUS``
   n/a                                 ``ETHTOOL_MSG_MM_GET``
   n/a                                 ``ETHTOOL_MSG_MM_SET``
+  n/a                                 ``ETHTOOL_MSG_TS_GET``
   =================================== =====================================
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 73e2c10dc2cc..cb51136328cf 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -57,6 +57,7 @@ enum {
 	ETHTOOL_MSG_PLCA_GET_STATUS,
 	ETHTOOL_MSG_MM_GET,
 	ETHTOOL_MSG_MM_SET,
+	ETHTOOL_MSG_TS_GET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -109,6 +110,7 @@ enum {
 	ETHTOOL_MSG_PLCA_NTF,
 	ETHTOOL_MSG_MM_GET_REPLY,
 	ETHTOOL_MSG_MM_NTF,
+	ETHTOOL_MSG_TS_GET_REPLY,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
@@ -975,6 +977,18 @@ enum {
 	ETHTOOL_A_MM_MAX = (__ETHTOOL_A_MM_CNT - 1)
 };
 
+/* TS LAYER */
+
+enum {
+	ETHTOOL_A_TS_UNSPEC,
+	ETHTOOL_A_TS_HEADER,			/* nest - _A_HEADER_* */
+	ETHTOOL_A_TS_LAYER,			/* u32 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_TS_CNT,
+	ETHTOOL_A_TS_MAX = (__ETHTOOL_A_TS_CNT - 1)
+};
+
 /* generic netlink info */
 #define ETHTOOL_GENL_NAME "ethtool"
 #define ETHTOOL_GENL_VERSION 1
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index df8091998c8d..33ff8e989dbe 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -13,6 +13,20 @@
 #include <linux/types.h>
 #include <linux/socket.h>   /* for SO_TIMESTAMPING */
 
+/*
+ * Hardware layer of the TIMESTAMPING provider
+ * New description layer should have the NETDEV_TIMESTAMPING or
+ * PHYLIB_TIMESTAMPING bit set to know which API to use for timestamping.
+ */
+enum {
+	NO_TIMESTAMPING = 0,
+	NETDEV_TIMESTAMPING = (1 << 0),
+	PHYLIB_TIMESTAMPING = (1 << 1),
+	SOFTWARE_TIMESTAMPING = (1 << 2) | (1 << 0),
+
+	__TIMESTAMPING_COUNT,
+};
+
 /* SO_TIMESTAMPING flags */
 enum {
 	SOF_TIMESTAMPING_TX_HARDWARE = (1<<0),
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 504f954a1b28..4ea64c080639 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -8,4 +8,4 @@ ethtool_nl-y	:= netlink.o bitset.o strset.o linkinfo.o linkmodes.o rss.o \
 		   linkstate.o debug.o wol.o features.o privflags.o rings.o \
 		   channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \
 		   tunnels.o fec.o eeprom.o stats.o phc_vclocks.o mm.o \
-		   module.o pse-pd.o plca.o mm.o
+		   module.o pse-pd.o plca.o mm.o ts.o
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index 28b8aaaf9bcb..a264b635f7d3 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -35,6 +35,7 @@ extern const char wol_mode_names[][ETH_GSTRING_LEN];
 extern const char sof_timestamping_names[][ETH_GSTRING_LEN];
 extern const char ts_tx_type_names[][ETH_GSTRING_LEN];
 extern const char ts_rx_filter_names[][ETH_GSTRING_LEN];
+extern const char ts_layer_names[][ETH_GSTRING_LEN];
 extern const char udp_tunnel_type_names[][ETH_GSTRING_LEN];
 
 int __ethtool_get_link(struct net_device *dev);
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 3bbd5afb7b31..561c0931d055 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -306,6 +306,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_PLCA_GET_STATUS]	= &ethnl_plca_status_request_ops,
 	[ETHTOOL_MSG_MM_GET]		= &ethnl_mm_request_ops,
 	[ETHTOOL_MSG_MM_SET]		= &ethnl_mm_request_ops,
+	[ETHTOOL_MSG_TS_GET]		= &ethnl_ts_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -1128,6 +1129,15 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.policy = ethnl_mm_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_mm_set_policy) - 1,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_TS_GET,
+		.doit	= ethnl_default_doit,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
+		.policy = ethnl_ts_get_policy,
+		.maxattr = ARRAY_SIZE(ethnl_ts_get_policy) - 1,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 9a333a8d04c1..1e6085198acc 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -395,6 +395,7 @@ extern const struct ethnl_request_ops ethnl_rss_request_ops;
 extern const struct ethnl_request_ops ethnl_plca_cfg_request_ops;
 extern const struct ethnl_request_ops ethnl_plca_status_request_ops;
 extern const struct ethnl_request_ops ethnl_mm_request_ops;
+extern const struct ethnl_request_ops ethnl_ts_request_ops;
 
 extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1];
 extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1];
@@ -441,6 +442,7 @@ extern const struct nla_policy ethnl_plca_set_cfg_policy[ETHTOOL_A_PLCA_MAX + 1]
 extern const struct nla_policy ethnl_plca_get_status_policy[ETHTOOL_A_PLCA_HEADER + 1];
 extern const struct nla_policy ethnl_mm_get_policy[ETHTOOL_A_MM_HEADER + 1];
 extern const struct nla_policy ethnl_mm_set_policy[ETHTOOL_A_MM_MAX + 1];
+extern const struct nla_policy ethnl_ts_get_policy[ETHTOOL_A_TS_HEADER + 1];
 
 int ethnl_set_features(struct sk_buff *skb, struct genl_info *info);
 int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info);
diff --git a/net/ethtool/ts.c b/net/ethtool/ts.c
new file mode 100644
index 000000000000..cd33f057ee48
--- /dev/null
+++ b/net/ethtool/ts.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/net_tstamp.h>
+#include <linux/phy.h>
+
+#include "netlink.h"
+#include "common.h"
+#include "bitset.h"
+
+struct ts_req_info {
+	struct ethnl_req_info		base;
+};
+
+struct ts_reply_data {
+	struct ethnl_reply_data		base;
+	u32				ts_layer;
+};
+
+#define TS_REPDATA(__reply_base) \
+	container_of(__reply_base, struct ts_reply_data, base)
+
+/* TS_GET */
+const struct nla_policy ethnl_ts_get_policy[] = {
+	[ETHTOOL_A_TS_HEADER]		=
+		NLA_POLICY_NESTED(ethnl_header_policy),
+};
+
+static int ts_prepare_data(const struct ethnl_req_info *req_base,
+			   struct ethnl_reply_data *reply_base,
+			   const struct genl_info *info)
+{
+	struct ts_reply_data *data = TS_REPDATA(reply_base);
+	struct net_device *dev = reply_base->dev;
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	int ret;
+
+	ret = ethnl_ops_begin(dev);
+	if (ret < 0)
+		return ret;
+
+	if (phy_has_tsinfo(dev->phydev))
+		data->ts_layer = PHYLIB_TIMESTAMPING;
+	else if (ops->get_ts_info)
+		data->ts_layer = NETDEV_TIMESTAMPING;
+	else
+		data->ts_layer = NO_TIMESTAMPING;
+
+	ethnl_ops_complete(dev);
+
+	return ret;
+}
+
+static int ts_reply_size(const struct ethnl_req_info *req_base,
+			 const struct ethnl_reply_data *reply_base)
+{
+	return nla_total_size(sizeof(u32));
+}
+
+static int ts_fill_reply(struct sk_buff *skb,
+			 const struct ethnl_req_info *req_base,
+			 const struct ethnl_reply_data *reply_base)
+{
+	struct ts_reply_data *data = TS_REPDATA(reply_base);
+
+	return nla_put_u32(skb, ETHTOOL_A_TS_LAYER, data->ts_layer);
+}
+
+const struct ethnl_request_ops ethnl_ts_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_TS_GET,
+	.reply_cmd		= ETHTOOL_MSG_TS_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_TS_HEADER,
+	.req_info_size		= sizeof(struct ts_req_info),
+	.reply_data_size	= sizeof(struct ts_reply_data),
+
+	.prepare_data		= ts_prepare_data,
+	.reply_size		= ts_reply_size,
+	.fill_reply		= ts_fill_reply,
+};
-- 
2.25.1

[PATCH net-next v5 09/16] netlink: specs: Introduce new netlink command to get current timestamp

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Add a new commands allowing to get the current time stamping on a
netdevice's link.

Example usage :
./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do ts-get \
	     --json '{"header":{"dev-name":"eth0"}}'
{'header': {'dev-index': 3, 'dev-name': 'eth0'}, 'ts-layer': 1}

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 Documentation/netlink/specs/ethtool.yaml | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 837b565577ca..49ee028e97ca 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -942,6 +942,16 @@ attribute-sets:
       -
         name: burst-tmr
         type: u32
+  -
+    name: ts
+    attributes:
+      -
+        name: header
+        type: nest
+        nested-attributes: header
+      -
+        name: ts-layer
+        type: u32
 
 operations:
   enum-model: directional
@@ -1692,3 +1702,17 @@ operations:
       name: mm-ntf
       doc: Notification for change in MAC Merge configuration.
       notify: mm-get
+    -
+      name: ts-get
+      doc: Get current timestamp
+
+      attribute-set: ts
+
+      do:
+        request:
+          attributes:
+            - header
+        reply:
+          attributes: &ts
+            - header
+            - ts-layer
-- 
2.25.1

[PATCH net-next v5 10/16] net: ethtool: Add a command to list available time stamping layers

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Introduce a new netlink message that lists all available time stamping
layers on a given interface.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 Documentation/networking/ethtool-netlink.rst | 23 +++++++
 include/uapi/linux/ethtool_netlink.h         | 14 ++++
 net/ethtool/netlink.c                        | 10 +++
 net/ethtool/netlink.h                        |  1 +
 net/ethtool/ts.c                             | 72 ++++++++++++++++++++
 5 files changed, 120 insertions(+)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 644b3b764044..963a5aacac8d 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -226,6 +226,7 @@ Userspace to kernel:
   ``ETHTOOL_MSG_MM_GET``                get MAC merge layer state
   ``ETHTOOL_MSG_MM_SET``                set MAC merge layer parameters
   ``ETHTOOL_MSG_TS_GET``                get current timestamping
+  ``ETHTOOL_MSG_TS_LIST_GET``            list available timestampings
   ===================================== =================================
 
 Kernel to userspace:
@@ -270,6 +271,7 @@ Kernel to userspace:
   ``ETHTOOL_MSG_RSS_GET_REPLY``            RSS settings
   ``ETHTOOL_MSG_MM_GET_REPLY``             MAC merge layer status
   ``ETHTOOL_MSG_TS_GET_REPLY``             current timestamping
+  ``ETHTOOL_MSG_TS_LIST_GET_REPLY``        available timestampings
   ======================================== =================================
 
 ``GET`` requests are sent by userspace applications to retrieve device
@@ -2016,6 +2018,26 @@ Kernel response contents:
 
 This command get the current timestamp layer.
 
+TS_LIST_GET
+==========
+
+Get the list of available timestampings.
+
+Request contents:
+
+  =================================  ======  ====================
+  ``ETHTOOL_A_TS_HEADER``            nested  request header
+  =================================  ======  ====================
+
+Kernel response contents:
+
+  ===========================  ======  ==============================
+  ``ETHTOOL_A_TS_HEADER``      nested  reply header
+  ``ETHTOOL_A_TS_LIST_LAYER``  binary  available timestampings
+  ===========================  ======  ==============================
+
+This command lists all the possible timestamp layer available.
+
 Request translation
 ===================
 
@@ -2122,5 +2144,6 @@ are netlink only.
   n/a                                 ``ETHTOOL_MSG_PLCA_GET_STATUS``
   n/a                                 ``ETHTOOL_MSG_MM_GET``
   n/a                                 ``ETHTOOL_MSG_MM_SET``
+  n/a                                 ``ETHTOOL_MSG_TSLIST_GET``
   n/a                                 ``ETHTOOL_MSG_TS_GET``
   =================================== =====================================
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index cb51136328cf..62b885d44d06 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -58,6 +58,7 @@ enum {
 	ETHTOOL_MSG_MM_GET,
 	ETHTOOL_MSG_MM_SET,
 	ETHTOOL_MSG_TS_GET,
+	ETHTOOL_MSG_TS_LIST_GET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -111,6 +112,7 @@ enum {
 	ETHTOOL_MSG_MM_GET_REPLY,
 	ETHTOOL_MSG_MM_NTF,
 	ETHTOOL_MSG_TS_GET_REPLY,
+	ETHTOOL_MSG_TS_LIST_GET_REPLY,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
@@ -989,6 +991,18 @@ enum {
 	ETHTOOL_A_TS_MAX = (__ETHTOOL_A_TS_CNT - 1)
 };
 
+/* TS LIST LAYER */
+
+enum {
+	ETHTOOL_A_TS_LIST_UNSPEC,
+	ETHTOOL_A_TS_LIST_HEADER,			/* nest - _A_HEADER_* */
+	ETHTOOL_A_TS_LIST_LAYER,		/* array, u32 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_TS_LIST_CNT,
+	ETHTOOL_A_TS_LIST_MAX = (__ETHTOOL_A_TS_LIST_CNT - 1)
+};
+
 /* generic netlink info */
 #define ETHTOOL_GENL_NAME "ethtool"
 #define ETHTOOL_GENL_VERSION 1
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 561c0931d055..842c9db1531f 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -307,6 +307,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_MM_GET]		= &ethnl_mm_request_ops,
 	[ETHTOOL_MSG_MM_SET]		= &ethnl_mm_request_ops,
 	[ETHTOOL_MSG_TS_GET]		= &ethnl_ts_request_ops,
+	[ETHTOOL_MSG_TS_LIST_GET]	= &ethnl_ts_list_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -1138,6 +1139,15 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.policy = ethnl_ts_get_policy,
 		.maxattr = ARRAY_SIZE(ethnl_ts_get_policy) - 1,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_TS_LIST_GET,
+		.doit	= ethnl_default_doit,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
+		.policy = ethnl_ts_get_policy,
+		.maxattr = ARRAY_SIZE(ethnl_ts_get_policy) - 1,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 1e6085198acc..ea8c312db3af 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -396,6 +396,7 @@ extern const struct ethnl_request_ops ethnl_plca_cfg_request_ops;
 extern const struct ethnl_request_ops ethnl_plca_status_request_ops;
 extern const struct ethnl_request_ops ethnl_mm_request_ops;
 extern const struct ethnl_request_ops ethnl_ts_request_ops;
+extern const struct ethnl_request_ops ethnl_ts_list_request_ops;
 
 extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1];
 extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1];
diff --git a/net/ethtool/ts.c b/net/ethtool/ts.c
index cd33f057ee48..d52b9800dc3b 100644
--- a/net/ethtool/ts.c
+++ b/net/ethtool/ts.c
@@ -76,3 +76,75 @@ const struct ethnl_request_ops ethnl_ts_request_ops = {
 	.reply_size		= ts_reply_size,
 	.fill_reply		= ts_fill_reply,
 };
+
+/* TS_LIST_GET */
+struct ts_list_reply_data {
+	struct ethnl_reply_data		base;
+	u32				ts_layer[__TIMESTAMPING_COUNT];
+	u8				num_ts;
+};
+
+#define TS_LIST_REPDATA(__reply_base) \
+	container_of(__reply_base, struct ts_list_reply_data, base)
+
+static int ts_list_prepare_data(const struct ethnl_req_info *req_base,
+				struct ethnl_reply_data *reply_base,
+				const struct genl_info *info)
+{
+	struct ts_list_reply_data *data = TS_LIST_REPDATA(reply_base);
+	struct net_device *dev = reply_base->dev;
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct ethtool_ts_info ts_info = {0};
+	int ret, i = 0;
+
+	ret = ethnl_ops_begin(dev);
+	if (ret < 0)
+		return ret;
+
+	if (phy_has_tsinfo(dev->phydev))
+		data->ts_layer[i++] = PHYLIB_TIMESTAMPING;
+	if (ops->get_ts_info) {
+		ops->get_ts_info(dev, &ts_info);
+		if (ts_info.so_timestamping &
+		    SOF_TIMESTAMPING_HARDWARE_MASK)
+			data->ts_layer[i++] = NETDEV_TIMESTAMPING;
+
+		if (ts_info.so_timestamping &
+		    SOF_TIMESTAMPING_SOFTWARE_MASK)
+			data->ts_layer[i++] = SOFTWARE_TIMESTAMPING;
+	}
+
+	data->num_ts = i;
+	ethnl_ops_complete(dev);
+
+	return ret;
+}
+
+static int ts_list_reply_size(const struct ethnl_req_info *req_base,
+			      const struct ethnl_reply_data *reply_base)
+{
+	struct ts_list_reply_data *data = TS_LIST_REPDATA(reply_base);
+
+	return nla_total_size(sizeof(u32)) * data->num_ts;
+}
+
+static int ts_list_fill_reply(struct sk_buff *skb,
+			      const struct ethnl_req_info *req_base,
+			 const struct ethnl_reply_data *reply_base)
+{
+	struct ts_list_reply_data *data = TS_LIST_REPDATA(reply_base);
+
+	return nla_put(skb, ETHTOOL_A_TS_LIST_LAYER, sizeof(u32) * data->num_ts, data->ts_layer);
+}
+
+const struct ethnl_request_ops ethnl_ts_list_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_TS_LIST_GET,
+	.reply_cmd		= ETHTOOL_MSG_TS_LIST_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_TS_HEADER,
+	.req_info_size		= sizeof(struct ts_req_info),
+	.reply_data_size	= sizeof(struct ts_list_reply_data),
+
+	.prepare_data		= ts_list_prepare_data,
+	.reply_size		= ts_list_reply_size,
+	.fill_reply		= ts_list_fill_reply,
+};
-- 
2.25.1

[PATCH net-next v5 11/16] netlink: specs: Introduce new netlink command to list available time stamping layers

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Add a new commands allowing to list available time stamping layers on a
netdevice's link.

Example usage :
./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema \
	     --do ts-list-get \
	     --json '{"header":{"dev-name":"eth0"}}'
{'header': {'dev-index': 3, 'dev-name': 'eth0'},
 'ts-list-layer': b'\x01\x00\x00\x00\x05\x00\x00\x00'}

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 Documentation/netlink/specs/ethtool.yaml | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 49ee028e97ca..81ed8e5f2f55 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -952,6 +952,16 @@ attribute-sets:
       -
         name: ts-layer
         type: u32
+  -
+    name: ts-list
+    attributes:
+      -
+        name: header
+        type: nest
+        nested-attributes: header
+      -
+        name: ts-list-layer
+        type: binary
 
 operations:
   enum-model: directional
@@ -1716,3 +1726,17 @@ operations:
           attributes: &ts
             - header
             - ts-layer
+    -
+      name: ts-list-get
+      doc: Get list of timestamp devices available on an interface
+
+      attribute-set: ts-list
+
+      do:
+        request:
+          attributes:
+            - header
+        reply:
+          attributes:
+            - header
+            - ts-list-layer
-- 
2.25.1

[PATCH net-next v5 12/16] net: Replace hwtstamp_source by timestamping layer

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Replace hwtstamp_source which is only used by the kernel_hwtstamp_config
structure by the more widely use timestamp_layer structure. This is done
to prepare the support of selectable timestamping source.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 drivers/net/ethernet/microchip/lan966x/lan966x_main.c |  6 +++---
 include/linux/net_tstamp.h                            | 11 +++--------
 net/core/dev_ioctl.c                                  |  2 +-
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
index 8e4101628fbd..83c1177469e2 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
@@ -470,15 +470,15 @@ static int lan966x_port_hwtstamp_set(struct net_device *dev,
 	struct lan966x_port *port = netdev_priv(dev);
 	int err;
 
-	if (cfg->source != HWTSTAMP_SOURCE_NETDEV &&
-	    cfg->source != HWTSTAMP_SOURCE_PHYLIB)
+	if (cfg->source != NETDEV_TIMESTAMPING &&
+	    cfg->source != PHYLIB_TIMESTAMPING)
 		return -EOPNOTSUPP;
 
 	err = lan966x_ptp_setup_traps(port, cfg);
 	if (err)
 		return err;
 
-	if (cfg->source == HWTSTAMP_SOURCE_NETDEV) {
+	if (cfg->source == NETDEV_TIMESTAMPING) {
 		if (!port->lan966x->ptp)
 			return -EOPNOTSUPP;
 
diff --git a/include/linux/net_tstamp.h b/include/linux/net_tstamp.h
index eb01c37e71e0..2c1af19d5421 100644
--- a/include/linux/net_tstamp.h
+++ b/include/linux/net_tstamp.h
@@ -5,11 +5,6 @@
 
 #include <uapi/linux/net_tstamp.h>
 
-enum hwtstamp_source {
-	HWTSTAMP_SOURCE_NETDEV,
-	HWTSTAMP_SOURCE_PHYLIB,
-};
-
 /**
  * struct kernel_hwtstamp_config - Kernel copy of struct hwtstamp_config
  *
@@ -20,8 +15,8 @@ enum hwtstamp_source {
  *	a legacy implementation of a lower driver
  * @copied_to_user: request was passed to a legacy implementation which already
  *	copied the ioctl request back to user space
- * @source: indication whether timestamps should come from the netdev or from
- *	an attached phylib PHY
+ * @source: indication whether timestamps should come from software, the netdev
+ *	or from an attached phylib PHY
  *
  * Prefer using this structure for in-kernel processing of hardware
  * timestamping configuration, over the inextensible struct hwtstamp_config
@@ -33,7 +28,7 @@ struct kernel_hwtstamp_config {
 	int rx_filter;
 	struct ifreq *ifr;
 	bool copied_to_user;
-	enum hwtstamp_source source;
+	u32 source;
 };
 
 static inline void hwtstamp_config_to_kernel(struct kernel_hwtstamp_config *kernel_cfg,
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 342a667858ac..45cc1ea9b195 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -332,7 +332,7 @@ int dev_set_hwtstamp_phylib(struct net_device *dev,
 	bool changed = false;
 	int err;
 
-	cfg->source = phy_ts ? HWTSTAMP_SOURCE_PHYLIB : HWTSTAMP_SOURCE_NETDEV;
+	cfg->source = phy_ts ? PHYLIB_TIMESTAMPING : NETDEV_TIMESTAMPING;
 
 	if (phy_ts && (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)) {
 		err = ops->ndo_hwtstamp_get(dev, &old_cfg);
-- 
2.25.1

[PATCH net-next v5 13/16] net: Change the API of PHY default timestamp to MAC

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Change the API to select MAC default time stamping instead of the PHY.
Indeed the PHY is closer to the wire therefore theoretically it have less
delay than the MAC timestamping but the reality is different. Due to lower
time stamping clock frequency, latency in the MDIO bus and no PHC hardware
synchronization between different PHY, the PHY PTP is often less precise
than the MAC. The exception is for PHY designed specially for PTP case but
these board are not very widespread. For not breaking the compatibility I
introduce an allowlist to reference all current PHYs that support
time stamping and let them keep the old API behavior.

The phy_set_timestamp function is called at each call of phy_attach_direct.
In case of MAC driver using phylink this function is called when the
interface is turned up. Then if the interface goes down and up again the
last choice of timestamp will be overwritten by the default choice.
A solution could be to cache the timestamp status but it can bring other
issues. In case of SFP, if we change the module, it doesn't make sense to
blindly re-set the timestamp back to PHY, if the new module has a PHY with
mediocre timestamping capabilities.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Changes in v5:
- Extract the API change in this patch.
- Rename whitelist to allowlist.
- Set NETDEV_TIMESTAMPING in register_netdevice function.
- Add software timestamping case description in ts_info.
---
 drivers/net/phy/phy_device.c | 68 ++++++++++++++++++++++++++++++++++++
 include/linux/netdevice.h    |  5 +++
 net/core/dev.c               |  3 ++
 net/core/dev_ioctl.c         | 36 +++++++++++--------
 net/core/timestamping.c      |  9 +++++
 net/ethtool/common.c         | 16 +++++++--
 6 files changed, 121 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2ce74593d6e4..2d5a6d57acb3 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1411,6 +1411,68 @@ int phy_sfp_probe(struct phy_device *phydev,
 }
 EXPORT_SYMBOL(phy_sfp_probe);
 
+/* An allowlist for PHYs selected as default timesetamping.
+ * Its use is to keep compatibility with old PTP API which is selecting
+ * these PHYs as default timestamping.
+ * The new API is selecting the MAC as default timestamping.
+ */
+const char * const phy_timestamping_allowlist[] = {
+	"Broadcom BCM5411",
+	"Broadcom BCM5421",
+	"Broadcom BCM54210E",
+	"Broadcom BCM5461",
+	"Broadcom BCM54612E",
+	"Broadcom BCM5464",
+	"Broadcom BCM5481",
+	"Broadcom BCM54810",
+	"Broadcom BCM54811",
+	"Broadcom BCM5482",
+	"Broadcom BCM50610",
+	"Broadcom BCM50610M",
+	"Broadcom BCM57780",
+	"Broadcom BCM5395",
+	"Broadcom BCM53125",
+	"Broadcom BCM53128",
+	"Broadcom BCM89610",
+	"NatSemi DP83640",
+	"Microchip LAN8841 Gigabit PHY",
+	"Microchip INDY Gigabit Quad PHY",
+	"Microsemi GE VSC856X SyncE",
+	"Microsemi GE VSC8575 SyncE",
+	"Microsemi GE VSC8582 SyncE",
+	"Microsemi GE VSC8584 SyncE",
+	"NXP C45 TJA1103",
+	NULL,
+};
+
+/**
+ * phy_set_timestamp - set the default selected timestamping device
+ * @dev: Pointer to net_device
+ * @phydev: Pointer to phy_device
+ *
+ * This is used to set default timestamping device taking into account
+ * the new API choice, which is selecting the timestamping from MAC by
+ * default.
+ */
+static void phy_set_timestamp(struct net_device *dev, struct phy_device *phydev)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	int i;
+
+	/* Backward compatibility to old timestamping API */
+	for (i = 0; phy_timestamping_allowlist[i]; i++) {
+		if (!strcmp(phy_timestamping_allowlist[i],
+			    phydev->drv->name)) {
+			if (phy_has_tsinfo(phydev))
+				dev->ts_layer = PHYLIB_TIMESTAMPING;
+			return;
+		}
+	}
+
+	if (phy_has_tsinfo(phydev) && !ops->get_ts_info)
+		dev->ts_layer = PHYLIB_TIMESTAMPING;
+}
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -1484,6 +1546,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 
 	phydev->phy_link_change = phy_link_change;
 	if (dev) {
+		phy_set_timestamp(dev, phydev);
 		phydev->attached_dev = dev;
 		dev->phydev = phydev;
 
@@ -1794,6 +1857,7 @@ EXPORT_SYMBOL_GPL(devm_phy_package_join);
 void phy_detach(struct phy_device *phydev)
 {
 	struct net_device *dev = phydev->attached_dev;
+	const struct ethtool_ops *ops = dev->ethtool_ops;
 	struct module *ndev_owner = NULL;
 	struct mii_bus *bus;
 
@@ -1812,6 +1876,10 @@ void phy_detach(struct phy_device *phydev)
 
 	phy_suspend(phydev);
 	if (dev) {
+		if (ops->get_ts_info)
+			dev->ts_layer = NETDEV_TIMESTAMPING;
+		else
+			dev->ts_layer = NO_TIMESTAMPING;
 		phydev->attached_dev->phydev = NULL;
 		phydev->attached_dev = NULL;
 	}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b9d0411836db..4e1d01120511 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -47,6 +47,7 @@
 #include <uapi/linux/if_bonding.h>
 #include <uapi/linux/pkt_cls.h>
 #include <uapi/linux/netdev.h>
+#include <uapi/linux/net_tstamp.h>
 #include <linux/hashtable.h>
 #include <linux/rbtree.h>
 #include <net/net_trackers.h>
@@ -2054,6 +2055,8 @@ enum netdev_ml_priv_type {
  *
  *	@dpll_pin: Pointer to the SyncE source pin of a DPLL subsystem,
  *		   where the clock is recovered.
+ *	@ts_layer:	Tracks which network device
+ *			performs packet	time stamping.
  *
  *	FIXME: cleanup struct net_device such that network protocol info
  *	moves out.
@@ -2415,6 +2418,8 @@ struct net_device {
 #if IS_ENABLED(CONFIG_DPLL)
 	struct dpll_pin		*dpll_pin;
 #endif
+
+	u32			ts_layer;
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 606a366cc209..1d4890dee114 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10204,6 +10204,9 @@ int register_netdevice(struct net_device *dev)
 	    dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
 		rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL, 0, NULL);
 
+	if (dev->ethtool_ops->get_ts_info)
+		dev->ts_layer = NETDEV_TIMESTAMPING;
+
 out:
 	return ret;
 
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 45cc1ea9b195..20009462fa24 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -259,9 +259,7 @@ static int dev_eth_ioctl(struct net_device *dev,
  * @dev: Network device
  * @cfg: Timestamping configuration structure
  *
- * Helper for enforcing a common policy that phylib timestamping, if available,
- * should take precedence in front of hardware timestamping provided by the
- * netdev.
+ * Helper for calling the selected hardware provider timestamping.
  *
  * Note: phy_mii_ioctl() only handles SIOCSHWTSTAMP (not SIOCGHWTSTAMP), and
  * there only exists a phydev->mii_ts->hwtstamp() method. So this will return
@@ -271,10 +269,14 @@ static int dev_eth_ioctl(struct net_device *dev,
 static int dev_get_hwtstamp_phylib(struct net_device *dev,
 				   struct kernel_hwtstamp_config *cfg)
 {
-	if (phy_has_hwtstamp(dev->phydev))
+	u32 ts_layer = dev->ts_layer;
+
+	if (ts_layer & PHYLIB_TIMESTAMPING)
 		return phy_hwtstamp_get(dev->phydev, cfg);
+	else if (ts_layer & NETDEV_TIMESTAMPING)
+		return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
 
-	return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
+	return -EOPNOTSUPP;
 }
 
 static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
@@ -315,9 +317,8 @@ static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
  * @cfg: Timestamping configuration structure
  * @extack: Netlink extended ack message structure, for error reporting
  *
- * Helper for enforcing a common policy that phylib timestamping, if available,
- * should take precedence in front of hardware timestamping provided by the
- * netdev. If the netdev driver needs to perform specific actions even for PHY
+ * Helper for calling the selected hardware provider timestamping.
+ * If the netdev driver needs to perform specific actions even for PHY
  * timestamping to work properly (a switch port must trap the timestamped
  * frames and not forward them), it must set IFF_SEE_ALL_HWTSTAMP_REQUESTS in
  * dev->priv_flags.
@@ -327,20 +328,26 @@ int dev_set_hwtstamp_phylib(struct net_device *dev,
 			    struct netlink_ext_ack *extack)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
-	bool phy_ts = phy_has_hwtstamp(dev->phydev);
 	struct kernel_hwtstamp_config old_cfg = {};
+	u32 ts_layer = dev->ts_layer;
 	bool changed = false;
 	int err;
 
-	cfg->source = phy_ts ? PHYLIB_TIMESTAMPING : NETDEV_TIMESTAMPING;
+	cfg->source = ts_layer;
+
+	if (!(ts_layer & PHYLIB_TIMESTAMPING) &&
+	    !(ts_layer & NETDEV_TIMESTAMPING))
+		return -EOPNOTSUPP;
 
-	if (phy_ts && (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)) {
+	if ((ts_layer & PHYLIB_TIMESTAMPING) &&
+	    (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)) {
 		err = ops->ndo_hwtstamp_get(dev, &old_cfg);
 		if (err)
 			return err;
 	}
 
-	if (!phy_ts || (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)) {
+	if ((ts_layer & NETDEV_TIMESTAMPING) ||
+	    (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)) {
 		err = ops->ndo_hwtstamp_set(dev, cfg, extack);
 		if (err) {
 			if (extack->_msg)
@@ -349,10 +356,11 @@ int dev_set_hwtstamp_phylib(struct net_device *dev,
 		}
 	}
 
-	if (phy_ts && (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS))
+	if ((ts_layer & PHYLIB_TIMESTAMPING) &&
+	    (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS))
 		changed = kernel_hwtstamp_config_changed(&old_cfg, cfg);
 
-	if (phy_ts) {
+	if (ts_layer & PHYLIB_TIMESTAMPING) {
 		err = phy_hwtstamp_set(dev->phydev, cfg, extack);
 		if (err) {
 			if (changed)
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index 04840697fe79..4638b2fb0dbc 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -21,6 +21,7 @@ static unsigned int classify(const struct sk_buff *skb)
 
 void skb_clone_tx_timestamp(struct sk_buff *skb)
 {
+	u32 ts_layer = skb->dev->ts_layer;
 	struct mii_timestamper *mii_ts;
 	struct sk_buff *clone;
 	unsigned int type;
@@ -28,6 +29,9 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
 	if (!skb->sk)
 		return;
 
+	if (!(ts_layer & PHYLIB_TIMESTAMPING))
+		return;
+
 	type = classify(skb);
 	if (type == PTP_CLASS_NONE)
 		return;
@@ -46,10 +50,15 @@ bool skb_defer_rx_timestamp(struct sk_buff *skb)
 {
 	struct mii_timestamper *mii_ts;
 	unsigned int type;
+	u32 ts_layer;
 
 	if (!skb->dev || !skb->dev->phydev || !skb->dev->phydev->mii_ts)
 		return false;
 
+	ts_layer = skb->dev->ts_layer;
+	if (!(ts_layer & PHYLIB_TIMESTAMPING))
+		return false;
+
 	if (skb_headroom(skb) < ETH_HLEN)
 		return false;
 
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index e2315e24d695..54a2acc20af0 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -633,13 +633,25 @@ int __ethtool_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
 {
 	const struct ethtool_ops *ops = dev->ethtool_ops;
 	struct phy_device *phydev = dev->phydev;
+	u32 ts_layer = dev->ts_layer;
+	int ret;
 
 	memset(info, 0, sizeof(*info));
 	info->cmd = ETHTOOL_GET_TS_INFO;
 
-	if (phy_has_tsinfo(phydev))
+	if (ts_layer == SOFTWARE_TIMESTAMPING) {
+		ret = ops->get_ts_info(dev, info);
+		if (ret)
+			return ret;
+		info->so_timestamping &= ~SOF_TIMESTAMPING_HARDWARE_MASK;
+		info->phc_index = -1;
+		return ret;
+	}
+
+	if (ts_layer & PHYLIB_TIMESTAMPING)
 		return phy_ts_info(phydev, info);
-	if (ops->get_ts_info)
+
+	if (ts_layer & NETDEV_TIMESTAMPING)
 		return ops->get_ts_info(dev, info);
 
 	info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
-- 
2.25.1

[PATCH net-next v5 14/16] net: ethtool: ts: Update GET_TS to reply the current selected timestamp

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

As the default selected timestamp API change we have to change also the
timestamp return by ethtool. This patch return now the current selected
timestamp.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 net/ethtool/ts.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/net/ethtool/ts.c b/net/ethtool/ts.c
index d52b9800dc3b..11041a16de5b 100644
--- a/net/ethtool/ts.c
+++ b/net/ethtool/ts.c
@@ -31,19 +31,13 @@ static int ts_prepare_data(const struct ethnl_req_info *req_base,
 {
 	struct ts_reply_data *data = TS_REPDATA(reply_base);
 	struct net_device *dev = reply_base->dev;
-	const struct ethtool_ops *ops = dev->ethtool_ops;
 	int ret;
 
 	ret = ethnl_ops_begin(dev);
 	if (ret < 0)
 		return ret;
 
-	if (phy_has_tsinfo(dev->phydev))
-		data->ts_layer = PHYLIB_TIMESTAMPING;
-	else if (ops->get_ts_info)
-		data->ts_layer = NETDEV_TIMESTAMPING;
-	else
-		data->ts_layer = NO_TIMESTAMPING;
+	data->ts_layer = dev->ts_layer;
 
 	ethnl_ops_complete(dev);
 
-- 
2.25.1

[PATCH net-next v5 15/16] net ethtool: net: Let the active time stamping layer be selectable

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Now that the current timestamp is saved in a variable lets add the
ETHTOOL_MSG_TS_SET ethtool netlink socket to make it selectable.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Changes in v2:
- Move selected_timestamping_layer introduction in this patch.
- Replace strmcmp by sysfs_streq.
- Use the PHY timestamp only if available.

Changes in v3:
- Add a devicetree binding to select the preferred timestamp
- Replace the way to select timestamp through ethtool instead of sysfs
You can test it with the ethtool source on branch feature_ptp of:
https://github.com/kmaincent/ethtool

Changes in v4:
- Change the API to select MAC default time stamping instead of the PHY.
- Add a whitelist to no break the old timestamp PHY default preferences
  for current PHY.
- Replace the ethtool ioctl by netlink.
- Add a netdev notifier to allow network device to create trap on PTP
  packets. Not tested as it need to change the lan966x driver that
  implement packet traps. I will do after the hwtstamp management change
  to NDOs.

Changes in v5:
- Remove the netdev notifier added in v4.
- Extract the default timestamp API change in another patch.
---
 Documentation/networking/ethtool-netlink.rst | 17 +++++
 include/uapi/linux/ethtool_netlink.h         |  1 +
 net/ethtool/netlink.c                        |  8 +++
 net/ethtool/netlink.h                        |  1 +
 net/ethtool/ts.c                             | 66 ++++++++++++++++++++
 5 files changed, 93 insertions(+)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 963a5aacac8d..eb7f8592986b 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -227,6 +227,7 @@ Userspace to kernel:
   ``ETHTOOL_MSG_MM_SET``                set MAC merge layer parameters
   ``ETHTOOL_MSG_TS_GET``                get current timestamping
   ``ETHTOOL_MSG_TS_LIST_GET``            list available timestampings
+  ``ETHTOOL_MSG_TS_SET``                set current timestamping
   ===================================== =================================
 
 Kernel to userspace:
@@ -2038,6 +2039,21 @@ Kernel response contents:
 
 This command lists all the possible timestamp layer available.
 
+TS_SET
+======
+
+Modify the selected timestamping.
+
+Request contents:
+
+  =======================  ======  ===================
+  ``ETHTOOL_A_TS_HEADER``  nested  reply header
+  ``ETHTOOL_A_TS_LAYER``   u32     timestamping
+  =======================  ======  ===================
+
+This command set the timestamping with one that should be listed by the
+TSLIST_GET command.
+
 Request translation
 ===================
 
@@ -2146,4 +2162,5 @@ are netlink only.
   n/a                                 ``ETHTOOL_MSG_MM_SET``
   n/a                                 ``ETHTOOL_MSG_TSLIST_GET``
   n/a                                 ``ETHTOOL_MSG_TS_GET``
+  n/a                                 ``ETHTOOL_MSG_TS_SET``
   =================================== =====================================
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 62b885d44d06..df6c4fcc62c1 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -59,6 +59,7 @@ enum {
 	ETHTOOL_MSG_MM_SET,
 	ETHTOOL_MSG_TS_GET,
 	ETHTOOL_MSG_TS_LIST_GET,
+	ETHTOOL_MSG_TS_SET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 842c9db1531f..8322bf71f80d 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -308,6 +308,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_MM_SET]		= &ethnl_mm_request_ops,
 	[ETHTOOL_MSG_TS_GET]		= &ethnl_ts_request_ops,
 	[ETHTOOL_MSG_TS_LIST_GET]	= &ethnl_ts_list_request_ops,
+	[ETHTOOL_MSG_TS_SET]		= &ethnl_ts_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -1148,6 +1149,13 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.policy = ethnl_ts_get_policy,
 		.maxattr = ARRAY_SIZE(ethnl_ts_get_policy) - 1,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_TS_SET,
+		.flags	= GENL_UNS_ADMIN_PERM,
+		.doit	= ethnl_default_set_doit,
+		.policy = ethnl_ts_set_policy,
+		.maxattr = ARRAY_SIZE(ethnl_ts_set_policy) - 1,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index ea8c312db3af..8fedf234b824 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -444,6 +444,7 @@ extern const struct nla_policy ethnl_plca_get_status_policy[ETHTOOL_A_PLCA_HEADE
 extern const struct nla_policy ethnl_mm_get_policy[ETHTOOL_A_MM_HEADER + 1];
 extern const struct nla_policy ethnl_mm_set_policy[ETHTOOL_A_MM_MAX + 1];
 extern const struct nla_policy ethnl_ts_get_policy[ETHTOOL_A_TS_HEADER + 1];
+extern const struct nla_policy ethnl_ts_set_policy[ETHTOOL_A_TS_MAX + 1];
 
 int ethnl_set_features(struct sk_buff *skb, struct genl_info *info);
 int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info);
diff --git a/net/ethtool/ts.c b/net/ethtool/ts.c
index 11041a16de5b..f77297965e03 100644
--- a/net/ethtool/ts.c
+++ b/net/ethtool/ts.c
@@ -59,6 +59,69 @@ static int ts_fill_reply(struct sk_buff *skb,
 	return nla_put_u32(skb, ETHTOOL_A_TS_LAYER, data->ts_layer);
 }
 
+/* TS_SET */
+const struct nla_policy ethnl_ts_set_policy[] = {
+	[ETHTOOL_A_TS_HEADER]	= NLA_POLICY_NESTED(ethnl_header_policy),
+	[ETHTOOL_A_TS_LAYER]	= NLA_POLICY_RANGE(NLA_U32, 0,
+						   __TIMESTAMPING_COUNT - 1)
+};
+
+static int ethnl_set_ts_validate(struct ethnl_req_info *req_info,
+				 struct genl_info *info)
+{
+	struct nlattr **tb = info->attrs;
+	const struct net_device_ops *ops = req_info->dev->netdev_ops;
+
+	if (!tb[ETHTOOL_A_TS_LAYER])
+		return 0;
+
+	if (!ops->ndo_hwtstamp_set)
+		return -EOPNOTSUPP;
+
+	return 1;
+}
+
+static int ethnl_set_ts(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	struct net_device *dev = req_info->dev;
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct kernel_hwtstamp_config config = {0};
+	struct nlattr **tb = info->attrs;
+	bool mod = false;
+	u32 ts_layer;
+	int ret;
+
+	ts_layer = dev->ts_layer;
+	ethnl_update_u32(&ts_layer, tb[ETHTOOL_A_TS_LAYER], &mod);
+
+	if (!mod)
+		return 0;
+
+	if (ts_layer & NETDEV_TIMESTAMPING && !ops->get_ts_info) {
+		NL_SET_ERR_MSG_ATTR(info->extack, tb[ETHTOOL_A_TS_LAYER],
+				    "this device cannot support timestamping");
+		return -EINVAL;
+	}
+
+	if (ts_layer & PHYLIB_TIMESTAMPING && !phy_has_tsinfo(dev->phydev)) {
+		NL_SET_ERR_MSG_ATTR(info->extack, tb[ETHTOOL_A_TS_LAYER],
+				    "this device cannot support timestamping");
+		return -EINVAL;
+	}
+
+	/* Disable time stamping in the current layer. */
+	if (netif_device_present(dev) &&
+	    dev->ts_layer & (PHYLIB_TIMESTAMPING | NETDEV_TIMESTAMPING)) {
+		ret = dev_set_hwtstamp_phylib(dev, &config, info->extack);
+		if (ret < 0)
+			return ret;
+	}
+
+	dev->ts_layer = ts_layer;
+
+	return 1;
+}
+
 const struct ethnl_request_ops ethnl_ts_request_ops = {
 	.request_cmd		= ETHTOOL_MSG_TS_GET,
 	.reply_cmd		= ETHTOOL_MSG_TS_GET_REPLY,
@@ -69,6 +132,9 @@ const struct ethnl_request_ops ethnl_ts_request_ops = {
 	.prepare_data		= ts_prepare_data,
 	.reply_size		= ts_reply_size,
 	.fill_reply		= ts_fill_reply,
+
+	.set_validate		= ethnl_set_ts_validate,
+	.set			= ethnl_set_ts,
 };
 
 /* TS_LIST_GET */
-- 
2.25.1

[PATCH net-next v5 16/16] netlink: specs: Introduce time stamping set command

[Köry Maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

Add a new commands allowing to set the time stamping.

Example usage :
./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema \
	     --do ts-list-get \
	     --json '{"header":{"dev-name":"eth0"}}'
{'header': {'dev-index': 3, 'dev-name': 'eth0'},
 'ts-list-layer': b'\x02\x00\x00\x00\x01\x00\x00\x00\x05\x00\x00\x00'}

./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do ts-set \
	     --json '{"header":{"dev-name":"eth0"}, "ts-layer":5}'
none

./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do ts-get \
	     --json '{"header":{"dev-name":"eth0"}}'
{'header': {'dev-index': 3, 'dev-name': 'eth0'}, 'ts-layer': 5}

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 Documentation/netlink/specs/ethtool.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 81ed8e5f2f55..cfee4bf32128 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -1740,3 +1740,12 @@ operations:
           attributes:
             - header
             - ts-list-layer
+    -
+      name: ts-set
+      doc: Set the timestamp device
+
+      attribute-set: ts
+
+      do:
+        request:
+          attributes: *ts
-- 
2.25.1

Re: [PATCH net-next v5 03/16] net: ethtool: Refactor identical get_ts_info implementations.

[kernel test robot]

Hi Köry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/K-ry-Maincent/net-Convert-PHYs-hwtstamp-callback-to-use-kernel_hwtstamp_config/20231009-235451
base:   net-next/main
patch link:    https://lore.kernel.org/r/20231009155138.86458-4-kory.maincent%40bootlin.com
patch subject: [PATCH net-next v5 03/16] net: ethtool: Refactor identical get_ts_info implementations.
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231010/202310100344.QG4Jg301-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231010/202310100344.QG4Jg301-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310100344.QG4Jg301-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/bonding/bond_main.c: In function 'bond_ethtool_get_ts_info':
>> drivers/net/bonding/bond_main.c:5755:28: warning: unused variable 'phydev' [-Wunused-variable]
    5755 |         struct phy_device *phydev;
         |                            ^~~~~~
>> drivers/net/bonding/bond_main.c:5752:35: warning: unused variable 'ops' [-Wunused-variable]
    5752 |         const struct ethtool_ops *ops;
         |                                   ^~~


vim +/phydev +5755 drivers/net/bonding/bond_main.c

217df670d9a4da Jay Vosburgh    2005-09-26  5746  
94dd016ae538b1 Hangbin Liu     2021-11-30  5747  static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
94dd016ae538b1 Hangbin Liu     2021-11-30  5748  				    struct ethtool_ts_info *info)
94dd016ae538b1 Hangbin Liu     2021-11-30  5749  {
94dd016ae538b1 Hangbin Liu     2021-11-30  5750  	struct bonding *bond = netdev_priv(bond_dev);
980f0799a15c75 Hangbin Liu     2023-04-18  5751  	struct ethtool_ts_info ts_info;
94dd016ae538b1 Hangbin Liu     2021-11-30 @5752  	const struct ethtool_ops *ops;
94dd016ae538b1 Hangbin Liu     2021-11-30  5753  	struct net_device *real_dev;
980f0799a15c75 Hangbin Liu     2023-04-18  5754  	bool sw_tx_support = false;
94dd016ae538b1 Hangbin Liu     2021-11-30 @5755  	struct phy_device *phydev;
980f0799a15c75 Hangbin Liu     2023-04-18  5756  	struct list_head *iter;
980f0799a15c75 Hangbin Liu     2023-04-18  5757  	struct slave *slave;
9b80ccda233fa6 Hangbin Liu     2022-05-19  5758  	int ret = 0;
94dd016ae538b1 Hangbin Liu     2021-11-30  5759  
9b80ccda233fa6 Hangbin Liu     2022-05-19  5760  	rcu_read_lock();
94dd016ae538b1 Hangbin Liu     2021-11-30  5761  	real_dev = bond_option_active_slave_get_rcu(bond);
9b80ccda233fa6 Hangbin Liu     2022-05-19  5762  	dev_hold(real_dev);
9b80ccda233fa6 Hangbin Liu     2022-05-19  5763  	rcu_read_unlock();
9b80ccda233fa6 Hangbin Liu     2022-05-19  5764  
94dd016ae538b1 Hangbin Liu     2021-11-30  5765  	if (real_dev) {
59b068fe2f41f9 Richard Cochran 2023-10-09  5766  		ret = ethtool_get_ts_info_by_layer(real_dev, info);
980f0799a15c75 Hangbin Liu     2023-04-18  5767  	} else {
980f0799a15c75 Hangbin Liu     2023-04-18  5768  		/* Check if all slaves support software tx timestamping */
980f0799a15c75 Hangbin Liu     2023-04-18  5769  		rcu_read_lock();
980f0799a15c75 Hangbin Liu     2023-04-18  5770  		bond_for_each_slave_rcu(bond, slave, iter) {
59b068fe2f41f9 Richard Cochran 2023-10-09  5771  			ret = ethtool_get_ts_info_by_layer(slave->dev, &ts_info);
980f0799a15c75 Hangbin Liu     2023-04-18  5772  			if (!ret && (ts_info.so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE)) {
980f0799a15c75 Hangbin Liu     2023-04-18  5773  				sw_tx_support = true;
980f0799a15c75 Hangbin Liu     2023-04-18  5774  				continue;
980f0799a15c75 Hangbin Liu     2023-04-18  5775  			}
980f0799a15c75 Hangbin Liu     2023-04-18  5776  
980f0799a15c75 Hangbin Liu     2023-04-18  5777  			sw_tx_support = false;
980f0799a15c75 Hangbin Liu     2023-04-18  5778  			break;
980f0799a15c75 Hangbin Liu     2023-04-18  5779  		}
980f0799a15c75 Hangbin Liu     2023-04-18  5780  		rcu_read_unlock();
94dd016ae538b1 Hangbin Liu     2021-11-30  5781  	}
94dd016ae538b1 Hangbin Liu     2021-11-30  5782  
980f0799a15c75 Hangbin Liu     2023-04-18  5783  	if (sw_tx_support)
980f0799a15c75 Hangbin Liu     2023-04-18  5784  		info->so_timestamping |= SOF_TIMESTAMPING_TX_SOFTWARE;
980f0799a15c75 Hangbin Liu     2023-04-18  5785  
9b80ccda233fa6 Hangbin Liu     2022-05-19  5786  	dev_put(real_dev);
9b80ccda233fa6 Hangbin Liu     2022-05-19  5787  	return ret;
94dd016ae538b1 Hangbin Liu     2021-11-30  5788  }
94dd016ae538b1 Hangbin Liu     2021-11-30  5789  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v5 01/16] net: Convert PHYs hwtstamp callback to use kernel_hwtstamp_config

[Florian Fainelli]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> The PHYs hwtstamp callback are still getting the timestamp config from
> ifreq and using copy_from/to_user.
> Get rid of these functions by using timestamp configuration in parameter.
> This also allow to move on to kernel_hwtstamp_config and be similar to
> net devices using the new ndo_hwstamp_get/set.
> 
> This adds the possibility to manipulate the timestamp configuration
> from the kernel which was not possible with the copy_from/to_user.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net-next v5 02/16] net: phy: Remove the call to phy_mii_ioctl in phy_hwstamp_get/set

[Florian Fainelli]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> __phy_hwtstamp_set function were calling the phy_mii_ioctl function
> which will then use the ifreq pointer to call the hwtstamp callback.
> Now that ifreq has been removed from the hwstamp callback parameters
> it seems more logical to not go through the phy_mii_ioctl function and pass
> directly kernel_hwtstamp_config parameter to the hwtstamp callback.
> 
> Lets do the same for __phy_hwtstamp_get function and return directly
> EOPNOTSUPP as SIOCGHWTSTAMP is not supported for now for the PHYs.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net-next v5 03/16] net: ethtool: Refactor identical get_ts_info implementations.

[Florian Fainelli]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Richard Cochran <richardcochran@gmail.com>
> 
> The vlan, macvlan and the bonding drivers call their "real" device driver
> in order to report the time stamping capabilities.  Provide a core
> ethtool helper function to avoid copy/paste in the stack.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

With the unused variables spotted by the kbuild test robot fixed:

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net-next v5 04/16] net: macb: Convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Florian Fainelli]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> The hardware timestamping through ndo_eth_ioctl() is going away.
> Convert the macb driver to the new API before that can be removed.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net-next v5 05/16] net: Make dev_set_hwtstamp_phylib accessible

[Florian Fainelli]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> Make the dev_set_hwtstamp_phylib function accessible in prevision to use
> it from ethtool to reset the tstamp current configuration.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
>   include/linux/netdevice.h | 3 +++
>   net/core/dev_ioctl.c      | 6 +++---
>   2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index e070a4540fba..b9d0411836db 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3922,6 +3922,9 @@ int generic_hwtstamp_get_lower(struct net_device *dev,
>   int generic_hwtstamp_set_lower(struct net_device *dev,
>   			       struct kernel_hwtstamp_config *kernel_cfg,
>   			       struct netlink_ext_ack *extack);
> +int dev_set_hwtstamp_phylib(struct net_device *dev,
> +			    struct kernel_hwtstamp_config *cfg,
> +			    struct netlink_ext_ack *extack);
>   int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *userdata);
>   unsigned int dev_get_flags(const struct net_device *);
>   int __dev_change_flags(struct net_device *dev, unsigned int flags,
> diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
> index b46aedc36939..342a667858ac 100644
> --- a/net/core/dev_ioctl.c
> +++ b/net/core/dev_ioctl.c
> @@ -322,9 +322,9 @@ static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
>    * frames and not forward them), it must set IFF_SEE_ALL_HWTSTAMP_REQUESTS in
>    * dev->priv_flags.
>    */
> -static int dev_set_hwtstamp_phylib(struct net_device *dev,
> -				   struct kernel_hwtstamp_config *cfg,
> -				   struct netlink_ext_ack *extack)
> +int dev_set_hwtstamp_phylib(struct net_device *dev,
> +			    struct kernel_hwtstamp_config *cfg,
> +			    struct netlink_ext_ack *extack)
>   {
>   	const struct net_device_ops *ops = dev->netdev_ops;
>   	bool phy_ts = phy_has_hwtstamp(dev->phydev);

Missing EXPORT_SYMBOL_GPL() here?
-- 
Florian

Re: [PATCH net-next v5 06/16] net_tstamp: Add TIMESTAMPING SOFTWARE and HARDWARE mask

[Florian Fainelli]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> Timestamping software or hardware flags are often used as a group,
> therefore adding these masks will easier future use.
> 
> I did not use SOF_TIMESTAMPING_SYS_HARDWARE flag as it is deprecated and
> not use at all.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Maybe you can squash this patch where it actually gets used in patch 10?
-- 
Florian

Re: [PATCH net-next v5 07/16] net: phy: micrel: fix ts_info value in case of no phc

[Florian Fainelli]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> In case of no phc we should not return SOFTWARE TIMESTAMPING flags as we do
> not know the netdev supports of timestamping.

There seems to be a word missing, maybe:

as we do not know whether the netdev supports timestamping.

> Remove it from the lan8841_ts_info and simply return 0.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
> 
> This patch is not tested but it seems consistent to me.

This does look correct to me as well.
-- 
Florian

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 1517 bytes --]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> Time stamping on network packets may happen either in the MAC or in
> the PHY, but not both.  In preparation for making the choice
> selectable, expose both the current and available layers via ethtool.
> 
> In accordance with the kernel implementation as it stands, the current
> layer will always read as "phy" when a PHY time stamping device is
> present. Future patches will allow changing the current layer
> administratively.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> 
> ---

[snip]

> +/*
> + * Hardware layer of the TIMESTAMPING provider
> + * New description layer should have the NETDEV_TIMESTAMPING or
> + * PHYLIB_TIMESTAMPING bit set to know which API to use for timestamping.

If we are talking about hardware layers, then we shall use either 
PHY_TIMESTAMPING or MAC_TIMESTAMPING. PHYLIB is the sub-subsystem to 
deal with Ethernet PHYs, and netdev is the object through which we 
represent network devices, so they are not even quite describing similar 
things. If you go with the {PHY,MAC}_TIMESTAMPING suggestion, then I 
could see how we could somewhat easily add PCS_TIMESTAMPING for instance.

> + */
> +enum {
> +	NO_TIMESTAMPING = 0,
> +	NETDEV_TIMESTAMPING = (1 << 0),
> +	PHYLIB_TIMESTAMPING = (1 << 1),
> +	SOFTWARE_TIMESTAMPING = (1 << 2) | (1 << 0),

Why do we have to set NETDEV_TIMESTAMPING here, or is this a round-about 
way of enumerating 0, 1, 2 and 3?
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH net-next v5 09/16] netlink: specs: Introduce new netlink command to get current timestamp

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> Add a new commands allowing to get the current time stamping on a
> netdevice's link.
> 
> Example usage :
> ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do ts-get \
> 	     --json '{"header":{"dev-name":"eth0"}}'
> {'header': {'dev-index': 3, 'dev-name': 'eth0'}, 'ts-layer': 1}
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

This is small enough you could probably fold this patch into patch 8.
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH net-next v5 11/16] netlink: specs: Introduce new netlink command to list available time stamping layers

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> Add a new commands allowing to list available time stamping layers on a
> netdevice's link.
> 
> Example usage :
> ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema \
> 	     --do ts-list-get \
> 	     --json '{"header":{"dev-name":"eth0"}}'
> {'header': {'dev-index': 3, 'dev-name': 'eth0'},
>   'ts-list-layer': b'\x01\x00\x00\x00\x05\x00\x00\x00'}
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Likewise you could fold this into patch 10.
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH net-next v5 12/16] net: Replace hwtstamp_source by timestamping layer

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 855 bytes --]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> Replace hwtstamp_source which is only used by the kernel_hwtstamp_config
> structure by the more widely use timestamp_layer structure. This is done
> to prepare the support of selectable timestamping source.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
[snip]
>    * Prefer using this structure for in-kernel processing of hardware
>    * timestamping configuration, over the inextensible struct hwtstamp_config
> @@ -33,7 +28,7 @@ struct kernel_hwtstamp_config {
>   	int rx_filter;
>   	struct ifreq *ifr;
>   	bool copied_to_user;
> -	enum hwtstamp_source source;

It would be kind of nice to keep using the enum internally such that we 
can catch unhandled values, which the compiler is usually quite good at 
figuring.
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH net-next v5 15/16] net ethtool: net: Let the active time stamping layer be selectable

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 2905 bytes --]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> Now that the current timestamp is saved in a variable lets add the
> ETHTOOL_MSG_TS_SET ethtool netlink socket to make it selectable.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---

[snip]

> +static int ethnl_set_ts_validate(struct ethnl_req_info *req_info,
> +				 struct genl_info *info)
> +{
> +	struct nlattr **tb = info->attrs;
> +	const struct net_device_ops *ops = req_info->dev->netdev_ops;
> +
> +	if (!tb[ETHTOOL_A_TS_LAYER])
> +		return 0;
> +
> +	if (!ops->ndo_hwtstamp_set)
> +		return -EOPNOTSUPP;

I would check for this first, in all likelihood this is what most 
drivers currently do not support, no need to event de-reference the 
array of attributes.

> +
> +	return 1;
> +}
> +
> +static int ethnl_set_ts(struct ethnl_req_info *req_info, struct genl_info *info)
> +{
> +	struct net_device *dev = req_info->dev;
> +	const struct ethtool_ops *ops = dev->ethtool_ops;
> +	struct kernel_hwtstamp_config config = {0};
> +	struct nlattr **tb = info->attrs;
> +	bool mod = false;
> +	u32 ts_layer;
> +	int ret;
> +
> +	ts_layer = dev->ts_layer;
> +	ethnl_update_u32(&ts_layer, tb[ETHTOOL_A_TS_LAYER], &mod);
> +
> +	if (!mod)
> +		return 0;
> +
> +	if (ts_layer & NETDEV_TIMESTAMPING && !ops->get_ts_info) {
> +		NL_SET_ERR_MSG_ATTR(info->extack, tb[ETHTOOL_A_TS_LAYER],
> +				    "this device cannot support timestamping");

Maybe expand the extended ack with "this devices does not support 
MAC-based timestamping"

> +		return -EINVAL;
> +	}
> +
> +	if (ts_layer & PHYLIB_TIMESTAMPING && !phy_has_tsinfo(dev->phydev)) {
> +		NL_SET_ERR_MSG_ATTR(info->extack, tb[ETHTOOL_A_TS_LAYER],
> +				    "this device cannot support timestamping");

Likewise, detail which kind of timestamping is not supported.

> +		return -EINVAL;
> +	}
> +
> +	/* Disable time stamping in the current layer. */
> +	if (netif_device_present(dev) &&
> +	    dev->ts_layer & (PHYLIB_TIMESTAMPING | NETDEV_TIMESTAMPING)) {
> +		ret = dev_set_hwtstamp_phylib(dev, &config, info->extack);

Can we still land in this function even if no changes to the 
timestamping configuration has been made? If so, would suggest first 
getting the current configuration and compare it with the user-supplied 
configuration if there are no changes, return.

> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	dev->ts_layer = ts_layer;
> +
> +	return 1;
> +}
> +
>   const struct ethnl_request_ops ethnl_ts_request_ops = {
>   	.request_cmd		= ETHTOOL_MSG_TS_GET,
>   	.reply_cmd		= ETHTOOL_MSG_TS_GET_REPLY,
> @@ -69,6 +132,9 @@ const struct ethnl_request_ops ethnl_ts_request_ops = {
>   	.prepare_data		= ts_prepare_data,
>   	.reply_size		= ts_reply_size,
>   	.fill_reply		= ts_fill_reply,
> +
> +	.set_validate		= ethnl_set_ts_validate,
> +	.set			= ethnl_set_ts,
>   };
>   
>   /* TS_LIST_GET */

-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH net-next v5 16/16] netlink: specs: Introduce time stamping set command

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> Add a new commands allowing to set the time stamping.
> 
> Example usage :
> ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema \
> 	     --do ts-list-get \
> 	     --json '{"header":{"dev-name":"eth0"}}'
> {'header': {'dev-index': 3, 'dev-name': 'eth0'},
>   'ts-list-layer': b'\x02\x00\x00\x00\x01\x00\x00\x00\x05\x00\x00\x00'}
> 
> ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do ts-set \
> 	     --json '{"header":{"dev-name":"eth0"}, "ts-layer":5}'
> none
> 
> ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do ts-get \
> 	     --json '{"header":{"dev-name":"eth0"}}'
> {'header': {'dev-index': 3, 'dev-name': 'eth0'}, 'ts-layer': 5}
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Likewise, probably better folded into patch 15.
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH net-next v5 13/16] net: Change the API of PHY default timestamp to MAC

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 3986 bytes --]

On 10/9/23 08:51, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> Change the API to select MAC default time stamping instead of the PHY.
> Indeed the PHY is closer to the wire therefore theoretically it have less
> delay than the MAC timestamping but the reality is different.

s/have/has/ or you need to make the subject plural.

> Due to lower
> time stamping clock frequency, latency in the MDIO bus and no PHC hardware
> synchronization between different PHY, the PHY PTP is often less precise
> than the MAC. 

-> different PHYs

> The exception is for PHY designed specially for PTP case but
> these board are not very widespread. 

s/board/devices/ maybe?

> For not breaking the compatibility I
> introduce an allowlist to reference all current PHYs that support
> time stamping and let them keep the old API behavior.
> 
> The phy_set_timestamp function is called at each call of phy_attach_direct.
> In case of MAC driver using phylink this function is called when the
> interface is turned up. Then if the interface goes down and up again the
> last choice of timestamp will be overwritten by the default choice.
> A solution could be to cache the timestamp status but it can bring other
> issues. In case of SFP, if we change the module, it doesn't make sense to
> blindly re-set the timestamp back to PHY, if the new module has a PHY with
> mediocre timestamping capabilities.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
> 
> Changes in v5:
> - Extract the API change in this patch.
> - Rename whitelist to allowlist.
> - Set NETDEV_TIMESTAMPING in register_netdevice function.
> - Add software timestamping case description in ts_info.
> ---
>   drivers/net/phy/phy_device.c | 68 ++++++++++++++++++++++++++++++++++++
>   include/linux/netdevice.h    |  5 +++
>   net/core/dev.c               |  3 ++
>   net/core/dev_ioctl.c         | 36 +++++++++++--------
>   net/core/timestamping.c      |  9 +++++
>   net/ethtool/common.c         | 16 +++++++--
>   6 files changed, 121 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 2ce74593d6e4..2d5a6d57acb3 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1411,6 +1411,68 @@ int phy_sfp_probe(struct phy_device *phydev,
>   }
>   EXPORT_SYMBOL(phy_sfp_probe);
>   
> +/* An allowlist for PHYs selected as default timesetamping.
> + * Its use is to keep compatibility with old PTP API which is selecting
> + * these PHYs as default timestamping.
> + * The new API is selecting the MAC as default timestamping.
> + */
> +const char * const phy_timestamping_allowlist[] = {
> +	"Broadcom BCM5411",
> +	"Broadcom BCM5421",
> +	"Broadcom BCM54210E",
> +	"Broadcom BCM5461",
> +	"Broadcom BCM54612E",
> +	"Broadcom BCM5464",
> +	"Broadcom BCM5481",
> +	"Broadcom BCM54810",
> +	"Broadcom BCM54811",
> +	"Broadcom BCM5482",
> +	"Broadcom BCM50610",
> +	"Broadcom BCM50610M",
> +	"Broadcom BCM57780",
> +	"Broadcom BCM5395",
> +	"Broadcom BCM53125",
> +	"Broadcom BCM53128",
> +	"Broadcom BCM89610",

The list of Broadcom PHYs that you have is too broad, if you look at 
drivers/net/phy/bcm-phy-ptp.c only PHY_ID_BCM54210E is actually 
supported as of now.

The allowlist is not maintainable using the PHY device name, especially 
not without having a shared header file that is used by both 
phy_device.c and the individual PHY device driver. You are guaranteed 
that a name change on one side can be missed on the other.

Using PHY OUIs can be a tad complicated with C45 PHYs, so rather, I 
would suggest that each of those PHY device drivers be responsible for 
overidding the timestamping selection, you could even consider inventing 
a specific PHYLIB_TIMESTAMPING_LEGACY and then issue a warning within 
the PHY library to encourage a change (if this is even relevant).

>   #endif
> +
> +	u32			ts_layer;

Likewise, would be preferable to use an enum type.
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH net-next v5 05/16] net: Make dev_set_hwtstamp_phylib accessible

[Köry Maincent]

On Mon, 9 Oct 2023 14:09:29 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> > -static int dev_set_hwtstamp_phylib(struct net_device *dev,
> > -				   struct kernel_hwtstamp_config *cfg,
> > -				   struct netlink_ext_ack *extack)
> > +int dev_set_hwtstamp_phylib(struct net_device *dev,
> > +			    struct kernel_hwtstamp_config *cfg,
> > +			    struct netlink_ext_ack *extack)
> >   {
> >   	const struct net_device_ops *ops = dev->netdev_ops;
> >   	bool phy_ts = phy_has_hwtstamp(dev->phydev);  
> 
> Missing EXPORT_SYMBOL_GPL() here?

True. Will be fixed in next version.

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Köry Maincent]

On Mon, 9 Oct 2023 14:20:02 -0700
Florian Fainelli <florian.fainelli@broadcom.com> wrote:

Hello Florian,
Thanks for your review!

> > +/*
> > + * Hardware layer of the TIMESTAMPING provider
> > + * New description layer should have the NETDEV_TIMESTAMPING or
> > + * PHYLIB_TIMESTAMPING bit set to know which API to use for timestamping.  
> 
> If we are talking about hardware layers, then we shall use either 
> PHY_TIMESTAMPING or MAC_TIMESTAMPING. PHYLIB is the sub-subsystem to 
> deal with Ethernet PHYs, and netdev is the object through which we 
> represent network devices, so they are not even quite describing similar 
> things. If you go with the {PHY,MAC}_TIMESTAMPING suggestion, then I 
> could see how we could somewhat easily add PCS_TIMESTAMPING for instance.

I am indeed talking about hardware layers but I updated the name to use NETDEV
and PHYLIB timestamping for a reason. It is indeed only PHY or MAC timestamping
for now but it may be expanded in the future to theoretically to 7 layers of
timestamps possible. Also there may be several possible timestamp within a MAC
device precision vs volume.
See the thread of my last version that talk about it:
https://lore.kernel.org/netdev/20230511203646.ihljeknxni77uu5j@skbuf/

All these possibles timestamps go through exclusively the netdev API or the
phylib API. Even the software timestamping is done in the netdev driver,
therefore it goes through the netdev API and then should have the
NETDEV_TIMESTAMPING bit set.

> > + */
> > +enum {
> > +	NO_TIMESTAMPING = 0,
> > +	NETDEV_TIMESTAMPING = (1 << 0),
> > +	PHYLIB_TIMESTAMPING = (1 << 1),
> > +	SOFTWARE_TIMESTAMPING = (1 << 2) | (1 << 0),  
> 
> Why do we have to set NETDEV_TIMESTAMPING here, or is this a round-about 
> way of enumerating 0, 1, 2 and 3?

I answered you above the software timestamping should have the
NETDEV_TIMESTAMPING bit set as it is done from the net device driver.

What I was thinking is that all the new timestamping should have
NETDEV_TIMESTAMPING or PHYLIB_TIMESTAMPING set to know which API to pass
through.
Like we could add these in the future:
MAC_DMA_TIMESTAMPING = (2 << 2) | (1 >> 0),
MAC_PRECISION_TIMESTAMPING = (3 << 2) | (1 >> 0),
...
PHY_SFP_TIMESTAMPING = (2 << 2) | (1 << 1),
...


Or maybe do you prefer to use defines like this:
# define NETDEV_TIMESTAMPING (1 << 0)
# define PHYLIB_TIMESTAMPING (1 << 1)

enum {
	NO_TIMESTAMPING = 0,
	MAC_TIMESTAMPING = NETDEV_TIMESTAMPING,
	PHY_TIMESTAMPING = PHYLIB_TIMESTAMPING,
	SOFTWARE_TIMESTAMPING = (1 << 2) | NETDEV_TIMESTAMPING,
	...
	MAC_DMA_TIMESTAMPING = (2 << 2) | NETDEV_TIMESTAMPING,
	MAC_PRECISION_TIMESTAMPING = (3 << 2) | NETDEV_TIMESTAMPING,

or other idea?

Regards,

Re: [PATCH net-next v5 15/16] net ethtool: net: Let the active time stamping layer be selectable

[Köry Maincent]

On Mon, 9 Oct 2023 14:28:38 -0700
Florian Fainelli <florian.fainelli@broadcom.com> wrote:

> > +static int ethnl_set_ts_validate(struct ethnl_req_info *req_info,
> > +				 struct genl_info *info)
> > +{
> > +	struct nlattr **tb = info->attrs;
> > +	const struct net_device_ops *ops = req_info->dev->netdev_ops;
> > +
> > +	if (!tb[ETHTOOL_A_TS_LAYER])
> > +		return 0;
> > +
> > +	if (!ops->ndo_hwtstamp_set)
> > +		return -EOPNOTSUPP;  
> 
> I would check for this first, in all likelihood this is what most 
> drivers currently do not support, no need to event de-reference the 
> array of attributes.

Indeed seems more logical.

> > +static int ethnl_set_ts(struct ethnl_req_info *req_info, struct genl_info
> > *info) +{
> > +	struct net_device *dev = req_info->dev;
> > +	const struct ethtool_ops *ops = dev->ethtool_ops;
> > +	struct kernel_hwtstamp_config config = {0};
> > +	struct nlattr **tb = info->attrs;
> > +	bool mod = false;
> > +	u32 ts_layer;
> > +	int ret;
> > +
> > +	ts_layer = dev->ts_layer;

> > +
> > +	if (ts_layer & NETDEV_TIMESTAMPING && !ops->get_ts_info) {
> > +		NL_SET_ERR_MSG_ATTR(info->extack, tb[ETHTOOL_A_TS_LAYER],
> > +				    "this device cannot support
> > timestamping");  
> 
> Maybe expand the extended ack with "this devices does not support 
> MAC-based timestamping"

Ok.

> > +	/* Disable time stamping in the current layer. */
> > +	if (netif_device_present(dev) &&
> > +	    dev->ts_layer & (PHYLIB_TIMESTAMPING | NETDEV_TIMESTAMPING)) {
> > +		ret = dev_set_hwtstamp_phylib(dev, &config, info->extack);
> >  
> 
> Can we still land in this function even if no changes to the 
> timestamping configuration has been made? 

We land in this function every time we change the timestamp from a valid
one.  

> If so, would suggest first 
> getting the current configuration and compare it with the user-supplied 
> configuration if there are no changes, return.

It is already done at the beginning of the function:
> > +	ethnl_update_u32(&ts_layer, tb[ETHTOOL_A_TS_LAYER], &mod);
> > +
> > +	if (!mod)
> > +		return 0;

Re: [PATCH net-next v5 09/16] netlink: specs: Introduce new netlink command to get current timestamp

[Köry Maincent]

On Mon, 9 Oct 2023 14:21:42 -0700
Florian Fainelli <florian.fainelli@broadcom.com> wrote:

> On 10/9/23 08:51, Köry Maincent wrote:
> > From: Kory Maincent <kory.maincent@bootlin.com>
> > 
> > Add a new commands allowing to get the current time stamping on a
> > netdevice's link.
> > 
> > Example usage :
> > ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do ts-get \
> > 	     --json '{"header":{"dev-name":"eth0"}}'
> > {'header': {'dev-index': 3, 'dev-name': 'eth0'}, 'ts-layer': 1}
> > 
> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>  
> 
> This is small enough you could probably fold this patch into patch 8.

I like having it separate. Indeed the ynl tool does not have a proper usage
documentation. I took quite some times to me to understand how use it
especially with bitset. Using the commit messages to add examples like that
would have help me a lot in the process.
I could also squash the example in the previous commit message but then it
become more noisy.
What do you think?

Re: [PATCH net-next v5 01/16] net: Convert PHYs hwtstamp callback to use kernel_hwtstamp_config

[Simon Horman]

...

> diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
> index 7ab080ff02df..416484ea6eb3 100644
> --- a/drivers/net/phy/nxp-c45-tja11xx.c
> +++ b/drivers/net/phy/nxp-c45-tja11xx.c
> @@ -1022,24 +1022,21 @@ static bool nxp_c45_rxtstamp(struct mii_timestamper *mii_ts,
>  }
>  
>  static int nxp_c45_hwtstamp(struct mii_timestamper *mii_ts,
> -			    struct ifreq *ifreq)
> +			    struct kernel_hwtstamp_config *config,
> +			    struct netlink_ext_ack *extack)
>  {
>  	struct nxp_c45_phy *priv = container_of(mii_ts, struct nxp_c45_phy,
>  						mii_ts);
>  	struct phy_device *phydev = priv->phydev;
>  	const struct nxp_c45_phy_data *data;
> -	struct hwtstamp_config cfg;
>  
> -	if (copy_from_user(&cfg, ifreq->ifr_data, sizeof(cfg)))
> -		return -EFAULT;
> -
> -	if (cfg.tx_type < 0 || cfg.tx_type > HWTSTAMP_TX_ON)
> +	if (cfg->tx_type < 0 || cfg->tx_type > HWTSTAMP_TX_ON)

Hi Köry,

cfg is removed from this function by this patch, but is used here.

>  		return -ERANGE;
>  
>  	data = nxp_c45_get_data(phydev);
> -	priv->hwts_tx = cfg.tx_type;
> +	priv->hwts_tx = cfg->tx_type;
>  
> -	switch (cfg.rx_filter) {
> +	switch (cfg->rx_filter) {
>  	case HWTSTAMP_FILTER_NONE:
>  		priv->hwts_rx = 0;
>  		break;
> @@ -1047,7 +1044,7 @@ static int nxp_c45_hwtstamp(struct mii_timestamper *mii_ts,
>  	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
>  	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
>  		priv->hwts_rx = 1;
> -		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
> +		cfg->rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
>  		break;
>  	default:
>  		return -ERANGE;
> @@ -1074,7 +1071,7 @@ static int nxp_c45_hwtstamp(struct mii_timestamper *mii_ts,
>  		nxp_c45_clear_reg_field(phydev, &data->regmap->irq_egr_ts_en);
>  
>  nxp_c45_no_ptp_irq:
> -	return copy_to_user(ifreq->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
> +	return 0;
>  }
>  

...

Re: [PATCH net-next v5 13/16] net: Change the API of PHY default timestamp to MAC

[Simon Horman]

On Mon, Oct 09, 2023 at 05:51:35PM +0200, Köry Maincent wrote:

Hi Köry,

some minor feedback from my side.

...

> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 2ce74593d6e4..2d5a6d57acb3 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1411,6 +1411,68 @@ int phy_sfp_probe(struct phy_device *phydev,
>  }
>  EXPORT_SYMBOL(phy_sfp_probe);
>  
> +/* An allowlist for PHYs selected as default timesetamping.
> + * Its use is to keep compatibility with old PTP API which is selecting
> + * these PHYs as default timestamping.
> + * The new API is selecting the MAC as default timestamping.
> + */
> +const char * const phy_timestamping_allowlist[] = {

Should this be static?

As flagged by Sparse.

> +	"Broadcom BCM5411",
> +	"Broadcom BCM5421",
> +	"Broadcom BCM54210E",
> +	"Broadcom BCM5461",
> +	"Broadcom BCM54612E",
> +	"Broadcom BCM5464",
> +	"Broadcom BCM5481",
> +	"Broadcom BCM54810",
> +	"Broadcom BCM54811",
> +	"Broadcom BCM5482",
> +	"Broadcom BCM50610",
> +	"Broadcom BCM50610M",
> +	"Broadcom BCM57780",
> +	"Broadcom BCM5395",
> +	"Broadcom BCM53125",
> +	"Broadcom BCM53128",
> +	"Broadcom BCM89610",
> +	"NatSemi DP83640",
> +	"Microchip LAN8841 Gigabit PHY",
> +	"Microchip INDY Gigabit Quad PHY",
> +	"Microsemi GE VSC856X SyncE",
> +	"Microsemi GE VSC8575 SyncE",
> +	"Microsemi GE VSC8582 SyncE",
> +	"Microsemi GE VSC8584 SyncE",
> +	"NXP C45 TJA1103",
> +	NULL,
> +};
> +
> +/**
> + * phy_set_timestamp - set the default selected timestamping device
> + * @dev: Pointer to net_device
> + * @phydev: Pointer to phy_device
> + *
> + * This is used to set default timestamping device taking into account
> + * the new API choice, which is selecting the timestamping from MAC by
> + * default.
> + */

...

> @@ -1484,6 +1546,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
>  
>  	phydev->phy_link_change = phy_link_change;
>  	if (dev) {
> +		phy_set_timestamp(dev, phydev);
>  		phydev->attached_dev = dev;
>  		dev->phydev = phydev;
>  
> @@ -1794,6 +1857,7 @@ EXPORT_SYMBOL_GPL(devm_phy_package_join);
>  void phy_detach(struct phy_device *phydev)
>  {
>  	struct net_device *dev = phydev->attached_dev;
> +	const struct ethtool_ops *ops = dev->ethtool_ops;

Elsewhere in this function it is assumed that dev may be NULL.
But here it is dereferenced unconditionally.

As flagged by Smatch.

>  	struct module *ndev_owner = NULL;
>  	struct mii_bus *bus;
>  
> @@ -1812,6 +1876,10 @@ void phy_detach(struct phy_device *phydev)
>  
>  	phy_suspend(phydev);
>  	if (dev) {
> +		if (ops->get_ts_info)
> +			dev->ts_layer = NETDEV_TIMESTAMPING;
> +		else
> +			dev->ts_layer = NO_TIMESTAMPING;
>  		phydev->attached_dev->phydev = NULL;
>  		phydev->attached_dev = NULL;
>  	}

...

Re: [PATCH net-next v5 01/16] net: Convert PHYs hwtstamp callback to use kernel_hwtstamp_config

[Köry Maincent]

On Tue, 10 Oct 2023 17:37:47 +0200
Simon Horman <horms@kernel.org> wrote:

> ...
> 
> > diff --git a/drivers/net/phy/nxp-c45-tja11xx.c
> > b/drivers/net/phy/nxp-c45-tja11xx.c index 7ab080ff02df..416484ea6eb3 100644
> > --- a/drivers/net/phy/nxp-c45-tja11xx.c
> > +++ b/drivers/net/phy/nxp-c45-tja11xx.c
> > @@ -1022,24 +1022,21 @@ static bool nxp_c45_rxtstamp(struct mii_timestamper
> > *mii_ts, }
> >  
> >  static int nxp_c45_hwtstamp(struct mii_timestamper *mii_ts,
> > -			    struct ifreq *ifreq)
> > +			    struct kernel_hwtstamp_config *config,
> > +			    struct netlink_ext_ack *extack)
> >  {
> >  	struct nxp_c45_phy *priv = container_of(mii_ts, struct nxp_c45_phy,
> >  						mii_ts);
> >  	struct phy_device *phydev = priv->phydev;
> >  	const struct nxp_c45_phy_data *data;
> > -	struct hwtstamp_config cfg;
> >  
> > -	if (copy_from_user(&cfg, ifreq->ifr_data, sizeof(cfg)))
> > -		return -EFAULT;
> > -
> > -	if (cfg.tx_type < 0 || cfg.tx_type > HWTSTAMP_TX_ON)
> > +	if (cfg->tx_type < 0 || cfg->tx_type > HWTSTAMP_TX_ON)  
> 
> Hi Köry,
> 
> cfg is removed from this function by this patch, but is used here.

Thanks for your review.
Indeed there is a mistake here. It will be fixed it next version.

Re: [PATCH net-next v5 03/16] net: ethtool: Refactor identical get_ts_info implementations.

[Jay Vosburgh]

Köry Maincent <kory.maincent@bootlin.com> wrote:

>From: Richard Cochran <richardcochran@gmail.com>
>
>The vlan, macvlan and the bonding drivers call their "real" device driver
>in order to report the time stamping capabilities.  Provide a core
>ethtool helper function to avoid copy/paste in the stack.
>
>Signed-off-by: Richard Cochran <richardcochran@gmail.com>
>Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

	For the bonding portion:

Reviewed-by: Jay Vosburgh <jay.vosburgh@canonical.com>


>---
>
>Change in v5:
>- Fixe typo
>---
> drivers/net/bonding/bond_main.c | 27 ++-------------------------
> drivers/net/macvlan.c           | 14 +-------------
> include/linux/ethtool.h         |  8 ++++++++
> net/8021q/vlan_dev.c            | 15 +--------------
> net/ethtool/common.c            |  6 ++++++
> 5 files changed, 18 insertions(+), 52 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index ed7212e61c54..18af563d20b2 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -5763,29 +5763,12 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
> 	rcu_read_unlock();
> 
> 	if (real_dev) {
>-		ops = real_dev->ethtool_ops;
>-		phydev = real_dev->phydev;
>-
>-		if (phy_has_tsinfo(phydev)) {
>-			ret = phy_ts_info(phydev, info);
>-			goto out;
>-		} else if (ops->get_ts_info) {
>-			ret = ops->get_ts_info(real_dev, info);
>-			goto out;
>-		}
>+		ret = ethtool_get_ts_info_by_layer(real_dev, info);
> 	} else {
> 		/* Check if all slaves support software tx timestamping */
> 		rcu_read_lock();
> 		bond_for_each_slave_rcu(bond, slave, iter) {
>-			ret = -1;
>-			ops = slave->dev->ethtool_ops;
>-			phydev = slave->dev->phydev;
>-
>-			if (phy_has_tsinfo(phydev))
>-				ret = phy_ts_info(phydev, &ts_info);
>-			else if (ops->get_ts_info)
>-				ret = ops->get_ts_info(slave->dev, &ts_info);
>-
>+			ret = ethtool_get_ts_info_by_layer(slave->dev, &ts_info);
> 			if (!ret && (ts_info.so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE)) {
> 				sw_tx_support = true;
> 				continue;
>@@ -5797,15 +5780,9 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
> 		rcu_read_unlock();
> 	}
> 
>-	ret = 0;
>-	info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
>-				SOF_TIMESTAMPING_SOFTWARE;
> 	if (sw_tx_support)
> 		info->so_timestamping |= SOF_TIMESTAMPING_TX_SOFTWARE;
> 
>-	info->phc_index = -1;
>-
>-out:
> 	dev_put(real_dev);
> 	return ret;
> }
>diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>index 02bd201bc7e5..759406fbaea8 100644
>--- a/drivers/net/macvlan.c
>+++ b/drivers/net/macvlan.c
>@@ -1086,20 +1086,8 @@ static int macvlan_ethtool_get_ts_info(struct net_device *dev,
> 				       struct ethtool_ts_info *info)
> {
> 	struct net_device *real_dev = macvlan_dev_real_dev(dev);
>-	const struct ethtool_ops *ops = real_dev->ethtool_ops;
>-	struct phy_device *phydev = real_dev->phydev;
> 
>-	if (phy_has_tsinfo(phydev)) {
>-		return phy_ts_info(phydev, info);
>-	} else if (ops->get_ts_info) {
>-		return ops->get_ts_info(real_dev, info);
>-	} else {
>-		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
>-			SOF_TIMESTAMPING_SOFTWARE;
>-		info->phc_index = -1;
>-	}
>-
>-	return 0;
>+	return ethtool_get_ts_info_by_layer(real_dev, info);
> }
> 
> static netdev_features_t macvlan_fix_features(struct net_device *dev,
>diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>index 62b61527bcc4..1159daac776e 100644
>--- a/include/linux/ethtool.h
>+++ b/include/linux/ethtool.h
>@@ -1043,6 +1043,14 @@ static inline int ethtool_mm_frag_size_min_to_add(u32 val_min, u32 *val_add,
> 	return -EINVAL;
> }
> 
>+/**
>+ * ethtool_get_ts_info_by_layer - Obtains time stamping capabilities from the MAC or PHY layer.
>+ * @dev: pointer to net_device structure
>+ * @info: buffer to hold the result
>+ * Returns zero on success, non-zero otherwise.
>+ */
>+int ethtool_get_ts_info_by_layer(struct net_device *dev, struct ethtool_ts_info *info);
>+
> /**
>  * ethtool_sprintf - Write formatted string to ethtool string data
>  * @data: Pointer to start of string to update
>diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
>index 2a7f1b15714a..407b2335f091 100644
>--- a/net/8021q/vlan_dev.c
>+++ b/net/8021q/vlan_dev.c
>@@ -702,20 +702,7 @@ static int vlan_ethtool_get_ts_info(struct net_device *dev,
> 				    struct ethtool_ts_info *info)
> {
> 	const struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
>-	const struct ethtool_ops *ops = vlan->real_dev->ethtool_ops;
>-	struct phy_device *phydev = vlan->real_dev->phydev;
>-
>-	if (phy_has_tsinfo(phydev)) {
>-		return phy_ts_info(phydev, info);
>-	} else if (ops->get_ts_info) {
>-		return ops->get_ts_info(vlan->real_dev, info);
>-	} else {
>-		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
>-			SOF_TIMESTAMPING_SOFTWARE;
>-		info->phc_index = -1;
>-	}
>-
>-	return 0;
>+	return ethtool_get_ts_info_by_layer(vlan->real_dev, info);
> }
> 
> static void vlan_dev_get_stats64(struct net_device *dev,
>diff --git a/net/ethtool/common.c b/net/ethtool/common.c
>index f5598c5f50de..e2315e24d695 100644
>--- a/net/ethtool/common.c
>+++ b/net/ethtool/common.c
>@@ -661,6 +661,12 @@ int ethtool_get_phc_vclocks(struct net_device *dev, int **vclock_index)
> }
> EXPORT_SYMBOL(ethtool_get_phc_vclocks);
> 
>+int ethtool_get_ts_info_by_layer(struct net_device *dev, struct ethtool_ts_info *info)
>+{
>+	return __ethtool_get_ts_info(dev, info);
>+}
>+EXPORT_SYMBOL(ethtool_get_ts_info_by_layer);
>+
> const struct ethtool_phy_ops *ethtool_phy_ops;
> 
> void ethtool_set_ethtool_phy_ops(const struct ethtool_phy_ops *ops)
>-- 
>2.25.1
>
>

Re: [PATCH net-next v5 13/16] net: Change the API of PHY default timestamp to MAC

[kernel test robot]

Hi Köry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/K-ry-Maincent/net-Convert-PHYs-hwtstamp-callback-to-use-kernel_hwtstamp_config/20231009-235451
base:   net-next/main
patch link:    https://lore.kernel.org/r/20231009155138.86458-14-kory.maincent%40bootlin.com
patch subject: [PATCH net-next v5 13/16] net: Change the API of PHY default timestamp to MAC
config: i386-randconfig-063-20231012 (https://download.01.org/0day-ci/archive/20231013/202310130914.kppQjKFp-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231013/202310130914.kppQjKFp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310130914.kppQjKFp-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/phy/phy_device.c:1419:12: sparse: sparse: symbol 'phy_timestamping_allowlist' was not declared. Should it be static?

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Jakub Kicinski]

On Tue, 10 Oct 2023 10:23:43 +0200 Köry Maincent wrote:
> > > +/*
> > > + * Hardware layer of the TIMESTAMPING provider
> > > + * New description layer should have the NETDEV_TIMESTAMPING or
> > > + * PHYLIB_TIMESTAMPING bit set to know which API to use for timestamping.    
> > 
> > If we are talking about hardware layers, then we shall use either 
> > PHY_TIMESTAMPING or MAC_TIMESTAMPING. PHYLIB is the sub-subsystem to 
> > deal with Ethernet PHYs, and netdev is the object through which we 
> > represent network devices, so they are not even quite describing similar 
> > things. If you go with the {PHY,MAC}_TIMESTAMPING suggestion, then I 
> > could see how we could somewhat easily add PCS_TIMESTAMPING for instance.  
> 
> I am indeed talking about hardware layers but I updated the name to use NETDEV
> and PHYLIB timestamping for a reason. It is indeed only PHY or MAC timestamping
> for now but it may be expanded in the future to theoretically to 7 layers of
> timestamps possible. Also there may be several possible timestamp within a MAC
> device precision vs volume.
> See the thread of my last version that talk about it:
> https://lore.kernel.org/netdev/20230511203646.ihljeknxni77uu5j@skbuf/
> 
> All these possibles timestamps go through exclusively the netdev API or the
> phylib API. Even the software timestamping is done in the netdev driver,
> therefore it goes through the netdev API and then should have the
> NETDEV_TIMESTAMPING bit set.

Netdev vs phylib is an implementation detail of Linux.
I'm also surprised that you changed this.

> > > + */
> > > +enum {
> > > +	NO_TIMESTAMPING = 0,
> > > +	NETDEV_TIMESTAMPING = (1 << 0),
> > > +	PHYLIB_TIMESTAMPING = (1 << 1),
> > > +	SOFTWARE_TIMESTAMPING = (1 << 2) | (1 << 0),    
> > 
> > Why do we have to set NETDEV_TIMESTAMPING here, or is this a round-about 
> > way of enumerating 0, 1, 2 and 3?  
> 
> I answered you above the software timestamping should have the
> NETDEV_TIMESTAMPING bit set as it is done from the net device driver.
> 
> What I was thinking is that all the new timestamping should have
> NETDEV_TIMESTAMPING or PHYLIB_TIMESTAMPING set to know which API to pass
> through.
> Like we could add these in the future:
> MAC_DMA_TIMESTAMPING = (2 << 2) | (1 >> 0),
> MAC_PRECISION_TIMESTAMPING = (3 << 2) | (1 >> 0),
> ...
> PHY_SFP_TIMESTAMPING = (2 << 2) | (1 << 1),
> ...

What is "PRECISION"? DMA is a separate block like MAC and PHY.

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Andrew Lunn]

> > All these possibles timestamps go through exclusively the netdev API or the
> > phylib API. Even the software timestamping is done in the netdev driver,
> > therefore it goes through the netdev API and then should have the
> > NETDEV_TIMESTAMPING bit set.
> 
> Netdev vs phylib is an implementation detail of Linux.
> I'm also surprised that you changed this.
> 
> > > > + */
> > > > +enum {
> > > > +	NO_TIMESTAMPING = 0,
> > > > +	NETDEV_TIMESTAMPING = (1 << 0),
> > > > +	PHYLIB_TIMESTAMPING = (1 << 1),
> > > > +	SOFTWARE_TIMESTAMPING = (1 << 2) | (1 << 0),    

Just emphasising Jakubs point here. phylib is an implementation
detail, in that the MAC driver might be using firmware to drive its
PHY, and that firmware can do a timestamp in the PHY. The API being
defined here should be independent of the implementation details. So
it probably should be MAC_TIMESTAMPING and PHY_TIMESTAMPING, and leave
it to the driver to decide if its PHYLIB doing the actual work, or
firmware.

	Andrew

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Vladimir Oltean]

On Fri, Oct 13, 2023 at 09:00:20AM -0700, Jakub Kicinski wrote:
> On Tue, 10 Oct 2023 10:23:43 +0200 Köry Maincent wrote:
> > > Why do we have to set NETDEV_TIMESTAMPING here, or is this a round-about 
> > > way of enumerating 0, 1, 2 and 3?  
> > 
> > I answered you above the software timestamping should have the
> > NETDEV_TIMESTAMPING bit set as it is done from the net device driver.
> > 
> > What I was thinking is that all the new timestamping should have
> > NETDEV_TIMESTAMPING or PHYLIB_TIMESTAMPING set to know which API to pass
> > through.
> > Like we could add these in the future:
> > MAC_DMA_TIMESTAMPING = (2 << 2) | (1 >> 0),
> > MAC_PRECISION_TIMESTAMPING = (3 << 2) | (1 >> 0),
> > ...
> > PHY_SFP_TIMESTAMPING = (2 << 2) | (1 << 1),
> > ...
> 
> What is "PRECISION"? DMA is a separate block like MAC and PHY.

If DMA is a separate block like MAC and PHY, can it have its own PHC
device, and the ethtool UAPI only lists the timestamping-capable PHCs
for one NIC, and is able to select between them? Translation between the
UAPI-visible PHC index and MAC, DMA, phylib PHY, other PHY etc can then
be done by the kernel as needed.

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Jakub Kicinski]

On Fri, 13 Oct 2023 19:14:46 +0300 Vladimir Oltean wrote:
> > What is "PRECISION"? DMA is a separate block like MAC and PHY.  
> 
> If DMA is a separate block like MAC and PHY, can it have its own PHC
> device, and the ethtool UAPI only lists the timestamping-capable PHCs
> for one NIC, and is able to select between them? 

Possibly, I guess. There are some devices which use generic (i.e.
modeled by Linux as separate struct device) DMA controllers to read 
out packets from "MAC" FIFOs. In practice I'm not sure if any of those
DMA controllers has time stamping capabilities.

> Translation between the UAPI-visible PHC index and MAC, DMA, phylib
> PHY, other PHY etc can then be done by the kernel as needed.

Translation by the kernel at which point?

IMHO it'd indeed be clearer for the user to have an ability to read 
the PHC for SOF_..._DMA via ETHTOOL_MSG_TS_LIST_GET_REPLY as a separate
entry, rather than e.g. assume that DMA uses the same PHC as MAC.

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Vladimir Oltean]

On Fri, Oct 13, 2023 at 09:30:56AM -0700, Jakub Kicinski wrote:
> On Fri, 13 Oct 2023 19:14:46 +0300 Vladimir Oltean wrote:
> > > What is "PRECISION"? DMA is a separate block like MAC and PHY.  
> > 
> > If DMA is a separate block like MAC and PHY, can it have its own PHC
> > device, and the ethtool UAPI only lists the timestamping-capable PHCs
> > for one NIC, and is able to select between them? 
> 
> Possibly, I guess. There are some devices which use generic (i.e.
> modeled by Linux as separate struct device) DMA controllers to read 
> out packets from "MAC" FIFOs. In practice I'm not sure if any of those
> DMA controllers has time stamping capabilities.

The answer is not completely satisfactory, I guess. My proposal would
only work if the common denominator for a hardware timestamp provider
could be modeled as a struct ptp_clock, like we do for MAC and phylib
PHYs already, and we call ptp_clock_index() to get the phc_index that
serves as the UAPI token for it.

> > Translation between the UAPI-visible PHC index and MAC, DMA, phylib
> > PHY, other PHY etc can then be done by the kernel as needed.
> 
> Translation by the kernel at which point?

The gist of what I'm proposing is for the core ethtool netlink message
handler to get just the phc_index as an attribute. No other information
as to what it represents. Not that it's netdev, DMA, phylib PHY or whatnot.

The ethtool kernel code would iterate through the stuff registered in
the system for the netdev, calling get_ts_info() or phy_ts_info() on it,
until it finds something which populates struct ethtool_ts_info ::
phc_index with the phc_index retrieved from netlink.

Then, ethtool just talks with the timestamper that matched that phc_index.

Same idea would be applied for the command that lists all timestamping
layers for a netdev. Check get_ts_info(), phy_ts_info(dev->phydev), and
can be extended in the future.

> IMHO it'd indeed be clearer for the user to have an ability to read 
> the PHC for SOF_..._DMA via ETHTOOL_MSG_TS_LIST_GET_REPLY as a separate
> entry, rather than e.g. assume that DMA uses the same PHC as MAC.

I'm not really sure what you're referring to, with SOF_..._DMA.
The DMA, if presented as a PHC as I am proposing, would play the role of
the hardware timestamp provider (think SOF_TIMESTAMPING_TX_HARDWARE |
SOF_TIMESTAMPING_RX_HARDWARE), so there will be no driver-visible
special socket option flags for DMA timestamping.

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Jakub Kicinski]

On Fri, 13 Oct 2023 20:09:03 +0300 Vladimir Oltean wrote:
> > > Translation between the UAPI-visible PHC index and MAC, DMA, phylib
> > > PHY, other PHY etc can then be done by the kernel as needed.  
> > 
> > Translation by the kernel at which point?  
> 
> The gist of what I'm proposing is for the core ethtool netlink message
> handler to get just the phc_index as an attribute. No other information
> as to what it represents. Not that it's netdev, DMA, phylib PHY or whatnot.
> 
> The ethtool kernel code would iterate through the stuff registered in
> the system for the netdev, calling get_ts_info() or phy_ts_info() on it,
> until it finds something which populates struct ethtool_ts_info ::
> phc_index with the phc_index retrieved from netlink.
> 
> Then, ethtool just talks with the timestamper that matched that phc_index.
> 
> Same idea would be applied for the command that lists all timestamping
> layers for a netdev. Check get_ts_info(), phy_ts_info(dev->phydev), and
> can be extended in the future.

I see, that could work. The user would then dig around sysfs to figure
out which PHC has what characteristics?

> > IMHO it'd indeed be clearer for the user to have an ability to read 
> > the PHC for SOF_..._DMA via ETHTOOL_MSG_TS_LIST_GET_REPLY as a separate
> > entry, rather than e.g. assume that DMA uses the same PHC as MAC.  
> 
> I'm not really sure what you're referring to, with SOF_..._DMA.
> The DMA, if presented as a PHC as I am proposing, would play the role of
> the hardware timestamp provider (think SOF_TIMESTAMPING_TX_HARDWARE |
> SOF_TIMESTAMPING_RX_HARDWARE), so there will be no driver-visible
> special socket option flags for DMA timestamping.

Each packet may want different timestamp tho, especially on Tx it
should be fairly easy for socket to request to get "real" MAC stamps,
while most get cheaper DMA stamps. Currently some drivers run flow
matching to find PTP packets and automatically give them better quality
timestamps :(

Even if at the config level we use PHCs we need to translate that into
some SKBTX_* bit, don't we?

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Vladimir Oltean]

On Fri, Oct 13, 2023 at 10:46:06AM -0700, Jakub Kicinski wrote:
> On Fri, 13 Oct 2023 20:09:03 +0300 Vladimir Oltean wrote:
> > > > Translation between the UAPI-visible PHC index and MAC, DMA, phylib
> > > > PHY, other PHY etc can then be done by the kernel as needed.  
> > > 
> > > Translation by the kernel at which point?  
> > 
> > The gist of what I'm proposing is for the core ethtool netlink message
> > handler to get just the phc_index as an attribute. No other information
> > as to what it represents. Not that it's netdev, DMA, phylib PHY or whatnot.
> > 
> > The ethtool kernel code would iterate through the stuff registered in
> > the system for the netdev, calling get_ts_info() or phy_ts_info() on it,
> > until it finds something which populates struct ethtool_ts_info ::
> > phc_index with the phc_index retrieved from netlink.
> > 
> > Then, ethtool just talks with the timestamper that matched that phc_index.
> > 
> > Same idea would be applied for the command that lists all timestamping
> > layers for a netdev. Check get_ts_info(), phy_ts_info(dev->phydev), and
> > can be extended in the future.
> 
> I see, that could work. The user would then dig around sysfs to figure
> out which PHC has what characteristics?

Yup. /sys/class/ptp/ptp<N>/ gives you everything else you need to know
about the PHC index that's configured as the active timestamper for this
netdev. It's just that (and I need to stress this again) the
timestamping-capable DMA blocks that you're talking about, but I've
never seen, should be able to fully implement a ptp_clock, with their
own clock operations and friends.

> > > IMHO it'd indeed be clearer for the user to have an ability to read 
> > > the PHC for SOF_..._DMA via ETHTOOL_MSG_TS_LIST_GET_REPLY as a separate
> > > entry, rather than e.g. assume that DMA uses the same PHC as MAC.  
> > 
> > I'm not really sure what you're referring to, with SOF_..._DMA.
> > The DMA, if presented as a PHC as I am proposing, would play the role of
> > the hardware timestamp provider (think SOF_TIMESTAMPING_TX_HARDWARE |
> > SOF_TIMESTAMPING_RX_HARDWARE), so there will be no driver-visible
> > special socket option flags for DMA timestamping.
> 
> Each packet may want different timestamp tho, especially on Tx it
> should be fairly easy for socket to request to get "real" MAC stamps,
> while most get cheaper DMA stamps. Currently some drivers run flow
> matching to find PTP packets and automatically give them better quality
> timestamps :(
> 
> Even if at the config level we use PHCs we need to translate that into
> some SKBTX_* bit, don't we?

I think Richard had something to say about that being wishful thinking:
https://lore.kernel.org/netdev/ZGw46hrpiqCVNeXS@hoboy.vegasvil.org/

On RX I'm not sure how you'd know in advance if the packet is going to
be routed to a socket that wants DMA or MAC timestamps. And having a
socket with hardware timestamps from one provider in one direction, and
another provider in the other direction, is.... not sane as a kernel API?

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Jakub Kicinski]

On Fri, 13 Oct 2023 20:56:01 +0300 Vladimir Oltean wrote:
> > > I'm not really sure what you're referring to, with SOF_..._DMA.
> > > The DMA, if presented as a PHC as I am proposing, would play the role of
> > > the hardware timestamp provider (think SOF_TIMESTAMPING_TX_HARDWARE |
> > > SOF_TIMESTAMPING_RX_HARDWARE), so there will be no driver-visible
> > > special socket option flags for DMA timestamping.  
> > 
> > Each packet may want different timestamp tho, especially on Tx it
> > should be fairly easy for socket to request to get "real" MAC stamps,
> > while most get cheaper DMA stamps. Currently some drivers run flow
> > matching to find PTP packets and automatically give them better quality
> > timestamps :(
> > 
> > Even if at the config level we use PHCs we need to translate that into
> > some SKBTX_* bit, don't we?  
> 
> I think Richard had something to say about that being wishful thinking:
> https://lore.kernel.org/netdev/ZGw46hrpiqCVNeXS@hoboy.vegasvil.org/

🤷️

> On RX I'm not sure how you'd know in advance if the packet is going to
> be routed to a socket that wants DMA or MAC timestamps. And having a
> socket with hardware timestamps from one provider in one direction, and
> another provider in the other direction, is.... not sane as a kernel API?

For DC NICs all the timestamps are based on the same PHC. The use case
is to get MAC/precise stamps for PTP and DMA/rough stamps for TCP CC.

Agreed that stamps from different PHCs in different directions would 
be weird.

Thinking about it again we want the ability to configure the rx filter
per stamping point. MAC stamps PTP frames and DMA stamps all the rest.
For Tx we need to know whether app wanted DMA or MAC stamp (more
or less whether it's PTP again, without running a classifier on the
packet).

Re: [PATCH net-next v5 10/16] net: ethtool: Add a command to list available time stamping layers

[kernel test robot]

Hi Köry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/K-ry-Maincent/net-Convert-PHYs-hwtstamp-callback-to-use-kernel_hwtstamp_config/20231009-235451
base:   net-next/main
patch link:    https://lore.kernel.org/r/20231009155138.86458-11-kory.maincent%40bootlin.com
patch subject: [PATCH net-next v5 10/16] net: ethtool: Add a command to list available time stamping layers
reproduce: (https://download.01.org/0day-ci/archive/20231014/202310140615.H4ByVgnr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310140615.H4ByVgnr-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Documentation/networking/ethtool-netlink.rst:2022: WARNING: Title underline too short.

vim +2022 Documentation/networking/ethtool-netlink.rst

  2020	
  2021	TS_LIST_GET
> 2022	==========
  2023	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Köry Maincent]

On Fri, 13 Oct 2023 18:11:19 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > > All these possibles timestamps go through exclusively the netdev API or
> > > the phylib API. Even the software timestamping is done in the netdev
> > > driver, therefore it goes through the netdev API and then should have the
> > > NETDEV_TIMESTAMPING bit set.  
> > 
> > Netdev vs phylib is an implementation detail of Linux.
> > I'm also surprised that you changed this.
> >   
> > > > > + */
> > > > > +enum {
> > > > > +	NO_TIMESTAMPING = 0,
> > > > > +	NETDEV_TIMESTAMPING = (1 << 0),
> > > > > +	PHYLIB_TIMESTAMPING = (1 << 1),
> > > > > +	SOFTWARE_TIMESTAMPING = (1 << 2) | (1 << 0),      
> 
> Just emphasising Jakubs point here. phylib is an implementation
> detail, in that the MAC driver might be using firmware to drive its
> PHY, and that firmware can do a timestamp in the PHY. The API being
> defined here should be independent of the implementation details. So
> it probably should be MAC_TIMESTAMPING and PHY_TIMESTAMPING, and leave
> it to the driver to decide if its PHYLIB doing the actual work, or
> firmware.

That is one reason why I moved to NETDEV_TIMESTAMPING, we don't know if it will
really be the MAC that does the timestamping, as the firmware could ask the PHY
to does it, but it surely goes though the netdev driver.

> Netdev vs phylib is an implementation detail of Linux.
> I'm also surprised that you changed this.

This is the main reason I changed this. This is Linux implementation purpose to
know whether it should go through netdev or phylib, and then each of these
drivers could use other timestamps which are hardware related.

As I have answered to Florian maybe you prefer to separate the Linux
implementation detail and the hardware timestamping like this:

> Or maybe do you prefer to use defines like this:
> # define NETDEV_TIMESTAMPING (1 << 0)
> # define PHYLIB_TIMESTAMPING (1 << 1)
> 
> enum {
> 	NO_TIMESTAMPING = 0,
> 	MAC_TIMESTAMPING = NETDEV_TIMESTAMPING,
> 	PHY_TIMESTAMPING = PHYLIB_TIMESTAMPING,
> 	SOFTWARE_TIMESTAMPING = (1 << 2) | NETDEV_TIMESTAMPING,
> 	...
> 	MAC_DMA_TIMESTAMPING = (2 << 2) | NETDEV_TIMESTAMPING,
> 	MAC_PRECISION_TIMESTAMPING = (3 << 2) | NETDEV_TIMESTAMPING,
> };


> > The gist of what I'm proposing is for the core ethtool netlink message
> > handler to get just the phc_index as an attribute. No other information
> > as to what it represents. Not that it's netdev, DMA, phylib PHY or whatnot.
> > 
> > The ethtool kernel code would iterate through the stuff registered in
> > the system for the netdev, calling get_ts_info() or phy_ts_info() on it,
> > until it finds something which populates struct ethtool_ts_info ::
> > phc_index with the phc_index retrieved from netlink.
> > 
> > Then, ethtool just talks with the timestamper that matched that phc_index.
> > 
> > Same idea would be applied for the command that lists all timestamping
> > layers for a netdev. Check get_ts_info(), phy_ts_info(dev->phydev), and
> > can be extended in the future.  
> 
> I see, that could work. The user would then dig around sysfs to figure
> out which PHC has what characteristics?

I am not an expert but there are net drivers that enable
SOF_TIMESTAMPING_TX/RX/RAW_HARDWARE without phc. In that case we won't ever be
able to enter the get_ts_info() with you proposition.
Still I am wondering why hardware timestamping capabilities can be enabled
without phc.

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Jakub Kicinski]

On Mon, 16 Oct 2023 12:41:34 +0200 Köry Maincent wrote:
> > Netdev vs phylib is an implementation detail of Linux.
> > I'm also surprised that you changed this.  
> 
> This is the main reason I changed this. This is Linux implementation purpose to
> know whether it should go through netdev or phylib, and then each of these
> drivers could use other timestamps which are hardware related.

For an integrated design there's 90% chance the stamping is done 
by the MAC. Even if it isn't there's no difference between PHY
and MAC in terms of quality.

But there is a big difference between MAC/PHY and DMA which would
both fall under NETDEV?

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Köry Maincent]

On Mon, 16 Oct 2023 07:22:04 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Mon, 16 Oct 2023 12:41:34 +0200 Köry Maincent wrote:
> > > Netdev vs phylib is an implementation detail of Linux.
> > > I'm also surprised that you changed this.    
> > 
> > This is the main reason I changed this. This is Linux implementation
> > purpose to know whether it should go through netdev or phylib, and then
> > each of these drivers could use other timestamps which are hardware
> > related.  
> 
> For an integrated design there's 90% chance the stamping is done 
> by the MAC. Even if it isn't there's no difference between PHY
> and MAC in terms of quality.

Ok, but there might be quality difference in case of several timestamp
configuration done in the MAC. Like the timestamping precision vs frequency
precision. In that case how ethtool would tell the driver to switch between
them?

My solution could work for this case by simply adding new values to the enum:

enum {
	NETDEV_TIMESTAMPING = (1 << 0),
	PHYLIB_TIMESTAMPING = (1 << 1),
	MAC_TS_PRECISION = (1 << 2)|(1 << 0),
	MAC_FREQ_PRECISION = (2 << 2)|(1 << 0),
}

Automatically Linux will go through the netdev implementation and could pass
the enum value to the netdev driver.

> But there is a big difference between MAC/PHY and DMA which would
> both fall under NETDEV?

Currently there is no DMA timestamping support right? And I suppose it fill fall
under the net device management?

In that case we will have MAC and DMA under netdev and PHY under phylib and
we won't have to do anything more than this timestamping management patch: 
https://lore.kernel.org/netdev/20231009155138.86458-14-kory.maincent@bootlin.com/

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Jakub Kicinski]

On Mon, 16 Oct 2023 17:00:27 +0200 Köry Maincent wrote:
> On Mon, 16 Oct 2023 07:22:04 -0700
> Jakub Kicinski <kuba@kernel.org> wrote:
> > > This is the main reason I changed this. This is Linux implementation
> > > purpose to know whether it should go through netdev or phylib, and then
> > > each of these drivers could use other timestamps which are hardware
> > > related.    
> > 
> > For an integrated design there's 90% chance the stamping is done 
> > by the MAC. Even if it isn't there's no difference between PHY
> > and MAC in terms of quality.  
> 
> Ok, but there might be quality difference in case of several timestamp
> configuration done in the MAC. Like the timestamping precision vs frequency
> precision. In that case how ethtool would tell the driver to switch between
> them?

What's the reason for timestamp precision differences?
My understanding so far was the the differences come from:
 1. different stamping device (i.e. separate "piece of silicon",
    accessed over a different bus, with different PHC etc.)
 2. different stamping point (MAC vs DMA)

I don't think any "integrated" device would support stamps which
differ under category 1.

> My solution could work for this case by simply adding new values to the enum:
> 
> enum {
> 	NETDEV_TIMESTAMPING = (1 << 0),
> 	PHYLIB_TIMESTAMPING = (1 << 1),
> 	MAC_TS_PRECISION = (1 << 2)|(1 << 0),
> 	MAC_FREQ_PRECISION = (2 << 2)|(1 << 0),
> }
> 
> Automatically Linux will go through the netdev implementation and could pass
> the enum value to the netdev driver.

We can add multiple fields to netlink. Why use the magic encoding?

> > But there is a big difference between MAC/PHY and DMA which would
> > both fall under NETDEV?  
> 
> Currently there is no DMA timestamping support right?

Kinda. Some devices pass DMA stamps as "HW stamps", and pretend they
are "good enough". But yes, there's no distinction at API level.

> And I suppose it fill fall under the net device management?

Yes.

> In that case we will have MAC and DMA under netdev and PHY under phylib and
> we won't have to do anything more than this timestamping management patch: 
> https://lore.kernel.org/netdev/20231009155138.86458-14-kory.maincent@bootlin.com/

Maybe we should start with a doc describing what APIs are at play,
what questions they answer, and what hard use cases we have.

I'm not opposed to the ethool API reporting just the differences
from my point 1. (in the first paragraph). But then we shouldn't
call that "layer", IMO, but device or source or such.

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Köry Maincent]

On Mon, 16 Oct 2023 08:43:46 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Mon, 16 Oct 2023 17:00:27 +0200 Köry Maincent wrote:
> > On Mon, 16 Oct 2023 07:22:04 -0700
> > Jakub Kicinski <kuba@kernel.org> wrote:  
>
> > Ok, but there might be quality difference in case of several timestamp
> > configuration done in the MAC. Like the timestamping precision vs frequency
> > precision. In that case how ethtool would tell the driver to switch between
> > them?  
> 
> What's the reason for timestamp precision differences?
> My understanding so far was the the differences come from:
>  1. different stamping device (i.e. separate "piece of silicon",
>     accessed over a different bus, with different PHC etc.)
>  2. different stamping point (MAC vs DMA)
> 
> I don't think any "integrated" device would support stamps which
> differ under category 1.

It was a case reported by Maxime on v3:
https://lore.kernel.org/netdev/20230324112541.0b3dd38a@pc-7.home/ 


> > My solution could work for this case by simply adding new values to the
> > enum:
> > 
> > enum {
> > 	NETDEV_TIMESTAMPING = (1 << 0),
> > 	PHYLIB_TIMESTAMPING = (1 << 1),
> > 	MAC_TS_PRECISION = (1 << 2)|(1 << 0),
> > 	MAC_FREQ_PRECISION = (2 << 2)|(1 << 0),
> > }
> > 
> > Automatically Linux will go through the netdev implementation and could pass
> > the enum value to the netdev driver.  
> 
> We can add multiple fields to netlink. Why use the magic encoding?

To simplify the Linux code to go under either netdev or phylib implementation
without needing describing all the enum possibility in the condition:
if (ts_layer & PHYLIB_TIMESTAMPING)
...
if (ts_layer & NETDEV_TIMESTAMPING)
...

We also could add "is_phylib" and "is_netdev" functions with a simple switch
case in it, but we have to be careful to always update these functions when new
enum values will appear.

> 
> > > But there is a big difference between MAC/PHY and DMA which would
> > > both fall under NETDEV?    
> > 
> > Currently there is no DMA timestamping support right?  
> 
> Kinda. Some devices pass DMA stamps as "HW stamps", and pretend they
> are "good enough". But yes, there's no distinction at API level.

Ok. I did suppose this when writing my last reply.

> > In that case we will have MAC and DMA under netdev and PHY under phylib and
> > we won't have to do anything more than this timestamping management patch: 
> > https://lore.kernel.org/netdev/20231009155138.86458-14-kory.maincent@bootlin.com/
> >  
> 
> Maybe we should start with a doc describing what APIs are at play,
> what questions they answer, and what hard use cases we have.
> 
> I'm not opposed to the ethool API reporting just the differences
> from my point 1. (in the first paragraph). But then we shouldn't
> call that "layer", IMO, but device or source or such.

I am open to change the naming to fit the best for our current and future usage.
If we take into account the Maxime case of several timestamps on a device then
maybe source could work.

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Jakub Kicinski]

On Mon, 16 Oct 2023 18:23:07 +0200 Köry Maincent wrote:
> > What's the reason for timestamp precision differences?
> > My understanding so far was the the differences come from:
> >  1. different stamping device (i.e. separate "piece of silicon",
> >     accessed over a different bus, with different PHC etc.)
> >  2. different stamping point (MAC vs DMA)
> > 
> > I don't think any "integrated" device would support stamps which
> > differ under category 1.  
> 
> It was a case reported by Maxime on v3:
> https://lore.kernel.org/netdev/20230324112541.0b3dd38a@pc-7.home/ 

IMHO this talks about how clock control/disciplining works which
is a somewhat independent topic of timestamping.

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Jacob Keller]

On 10/16/2023 10:03 AM, Jakub Kicinski wrote:
> On Mon, 16 Oct 2023 18:23:07 +0200 Köry Maincent wrote:
>>> What's the reason for timestamp precision differences?
>>> My understanding so far was the the differences come from:
>>>  1. different stamping device (i.e. separate "piece of silicon",
>>>     accessed over a different bus, with different PHC etc.)
>>>  2. different stamping point (MAC vs DMA)
>>>
>>> I don't think any "integrated" device would support stamps which
>>> differ under category 1.  
>>
>> It was a case reported by Maxime on v3:
>> https://lore.kernel.org/netdev/20230324112541.0b3dd38a@pc-7.home/ 
> 
> IMHO this talks about how clock control/disciplining works which
> is a somewhat independent topic of timestamping.

The thread in question mentions that the device has two modes, one which
has higher precision for the timestamps, and one which has better
precision on frequency adjustments. I don't know the details for why the
hardware has this behavior, but being able to switch between the two
timestamp modes has value as described by the thread.

I'm not sure how to represent that in such an API because both modes
seem to capture the timestamp at the MAC.

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Richard Cochran]

On Mon, Oct 16, 2023 at 12:41:34PM +0200, Köry Maincent wrote:

> Still I am wondering why hardware timestamping capabilities can be enabled
> without phc.

There is hardware that simply provides time values on frames from a
free running clock, and that clock cannot be read, set, or adjusted.

So the time stamps only relate to other time stamps from the same
device.  That might be used for performance analysis.

Thanks,
Richard

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Köry Maincent]

On Mon, 16 Oct 2023 16:50:01 -0700
Richard Cochran <richardcochran@gmail.com> wrote:

> On Mon, Oct 16, 2023 at 12:41:34PM +0200, Köry Maincent wrote:
> 
> > Still I am wondering why hardware timestamping capabilities can be enabled
> > without phc.  
> 
> There is hardware that simply provides time values on frames from a
> free running clock, and that clock cannot be read, set, or adjusted.
> 
> So the time stamps only relate to other time stamps from the same
> device.  That might be used for performance analysis.

Ok, thanks you for the information.

Köry

Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

[Köry Maincent]

On Mon, 16 Oct 2023 16:03:13 -0700
Jacob Keller <jacob.e.keller@intel.com> wrote:

> On 10/16/2023 10:03 AM, Jakub Kicinski wrote:
> > On Mon, 16 Oct 2023 18:23:07 +0200 Köry Maincent wrote:  
> >>> What's the reason for timestamp precision differences?
> >>> My understanding so far was the the differences come from:
> >>>  1. different stamping device (i.e. separate "piece of silicon",
> >>>     accessed over a different bus, with different PHC etc.)
> >>>  2. different stamping point (MAC vs DMA)
> >>>
> >>> I don't think any "integrated" device would support stamps which
> >>> differ under category 1.    
> >>
> >> It was a case reported by Maxime on v3:
> >> https://lore.kernel.org/netdev/20230324112541.0b3dd38a@pc-7.home/   
> > 
> > IMHO this talks about how clock control/disciplining works which
> > is a somewhat independent topic of timestamping.  
> 
> The thread in question mentions that the device has two modes, one which
> has higher precision for the timestamps, and one which has better
> precision on frequency adjustments. I don't know the details for why the
> hardware has this behavior, but being able to switch between the two
> timestamp modes has value as described by the thread.
> 
> I'm not sure how to represent that in such an API because both modes
> seem to capture the timestamp at the MAC.

After some thought, indeed moving back to MAC/PHY_TIMESTAMPING seems better.
This case of several timestamp modes in MAC is currently only for the special
stmmac case.
We could support it the same way we could support multiPHY by saving the
source id of the timestamp like this in net_device:
struct {
	enum ts_layer layer,
	int source_id,
} ts

Re: [PATCH net-next v5 01/16] net: Convert PHYs hwtstamp callback to use kernel_hwtstamp_config

[kernel test robot]

Hi Köry,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/K-ry-Maincent/net-Convert-PHYs-hwtstamp-callback-to-use-kernel_hwtstamp_config/20231009-235451
base:   net-next/main
patch link:    https://lore.kernel.org/r/20231009155138.86458-2-kory.maincent%40bootlin.com
patch subject: [PATCH net-next v5 01/16] net: Convert PHYs hwtstamp callback to use kernel_hwtstamp_config
config: csky-randconfig-002-20231020 (https://download.01.org/0day-ci/archive/20231021/202310210416.0nNTYS2E-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231021/202310210416.0nNTYS2E-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310210416.0nNTYS2E-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/net/phy/nxp-c45-tja11xx.c: In function 'nxp_c45_hwtstamp':
>> drivers/net/phy/nxp-c45-tja11xx.c:1033:13: error: 'cfg' undeclared (first use in this function)
    1033 |         if (cfg->tx_type < 0 || cfg->tx_type > HWTSTAMP_TX_ON)
         |             ^~~
   drivers/net/phy/nxp-c45-tja11xx.c:1033:13: note: each undeclared identifier is reported only once for each function it appears in


vim +/cfg +1033 drivers/net/phy/nxp-c45-tja11xx.c

  1023	
  1024	static int nxp_c45_hwtstamp(struct mii_timestamper *mii_ts,
  1025				    struct kernel_hwtstamp_config *config,
  1026				    struct netlink_ext_ack *extack)
  1027	{
  1028		struct nxp_c45_phy *priv = container_of(mii_ts, struct nxp_c45_phy,
  1029							mii_ts);
  1030		struct phy_device *phydev = priv->phydev;
  1031		const struct nxp_c45_phy_data *data;
  1032	
> 1033		if (cfg->tx_type < 0 || cfg->tx_type > HWTSTAMP_TX_ON)
  1034			return -ERANGE;
  1035	
  1036		data = nxp_c45_get_data(phydev);
  1037		priv->hwts_tx = cfg->tx_type;
  1038	
  1039		switch (cfg->rx_filter) {
  1040		case HWTSTAMP_FILTER_NONE:
  1041			priv->hwts_rx = 0;
  1042			break;
  1043		case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
  1044		case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
  1045		case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
  1046			priv->hwts_rx = 1;
  1047			cfg->rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
  1048			break;
  1049		default:
  1050			return -ERANGE;
  1051		}
  1052	
  1053		if (priv->hwts_rx || priv->hwts_tx) {
  1054			phy_write_mmd(phydev, MDIO_MMD_VEND1,
  1055				      data->regmap->vend1_event_msg_filt,
  1056				      EVENT_MSG_FILT_ALL);
  1057			data->ptp_enable(phydev, true);
  1058		} else {
  1059			phy_write_mmd(phydev, MDIO_MMD_VEND1,
  1060				      data->regmap->vend1_event_msg_filt,
  1061				      EVENT_MSG_FILT_NONE);
  1062			data->ptp_enable(phydev, false);
  1063		}
  1064	
  1065		if (nxp_c45_poll_txts(priv->phydev))
  1066			goto nxp_c45_no_ptp_irq;
  1067	
  1068		if (priv->hwts_tx)
  1069			nxp_c45_set_reg_field(phydev, &data->regmap->irq_egr_ts_en);
  1070		else
  1071			nxp_c45_clear_reg_field(phydev, &data->regmap->irq_egr_ts_en);
  1072	
  1073	nxp_c45_no_ptp_irq:
  1074		return 0;
  1075	}
  1076	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki