Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

net: cpsw: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

New timestamping API was introduced in commit 66f7223039c0 ("net: add
NDOs for configuring hardware timestamping") from kernel v6.6. It is
time to convert the two cpsw drivers to the new API, so that the
ndo_eth_ioctl() path can be removed completely.

The cpsw_hwtstamp_get() and cpsw_hwtstamp_set() methods (and their shim
definitions, for the case where CONFIG_TI_CPTS is not enabled) must have
their prototypes adjusted.

These methods are used by two drivers (cpsw and cpsw_new), with vastly
different configurations:
- cpsw has two operating modes:
- "dual EMAC" - enabled through the "dual_emac" device tree property -
creates one net_device per EMAC / slave interface (but there is no
bridging offload)
- "switch mode" - default - there is a single net_device, with two
EMACs/slaves behind it (and switching between them happens
unbeknownst to the network stack).
- cpsw_new always registers one net_device for each EMAC which doesn't
have status = "disabled". In terms of switching, it has two modes:
- "dual EMAC": default, no switching between ports, no switchdev
offload.
- "switch mode": enabled through the "switch_mode" devlink parameter,
offloads the Linux bridge through switchdev

Essentially, in 3 out of 4 operating modes, there is a bijective
relation between the net_device and the slave. Timestamping can thus be
configured on individual slaves. But in the "switch mode" of the cpsw
driver, ndo_eth_ioctl() targets a single slave, designated using the
"active_slave" device tree property.

To deal with these different cases, the common portion of the drivers,
cpsw_priv.c, has the cpsw_slave_index() function pointer, set to
separate, identically named cpsw_slave_index_priv() by the 2 drivers.

This is all relevant because cpsw_ndo_ioctl() has the old-style
phy_has_hwtstamp() logic which lets the PHY handle the timestamping
ioctls. Normally, that logic should be obsoleted by the more complex
logic in the core, which permits dynamically selecting the timestamp
provider - see dev_set_hwtstamp_phylib().

But I have doubts as to how this works for the "switch mode" of the dual
EMAC driver, because the core logic only engages if the PHY is visible
through ndev->phydev (this is set by phy_attach_direct()).

In cpsw.c, we have:
cpsw_ndo_open()
-> for_each_slave(priv, cpsw_slave_open, priv); // continues on errors
-> of_phy_connect()
-> phy_connect_direct()
-> phy_attach_direct()
OR
-> phy_connect()
-> phy_connect_direct()
-> phy_attach_direct()

The problem for "switch mode" is that the behavior of phy_attach_direct()
called twice in a row for the same net_device (once for each slave) is
probably undefined.

For sure it will overwrite dev->phydev. I don't see any explicit error
checks for this case, and even if there were, the for_each_slave() call
makes them non-fatal to cpsw_ndo_open() anyway.

I have no idea what is the extent to which this provides a usable
result, but the point is: only the last attached PHY will be visible
in dev->phydev, and this may well be a different PHY than
cpsw->slaves[slave_no].phy for the "active_slave".

In dual EMAC mode, as well as in cpsw_new, this should not be a problem.
I don't know whether PHY timestamping is a use case for the cpsw "switch
mode" as well, and I hope that there isn't, because for the sake of
simplicity, I've decided to deliberately break that functionality, by
refusing all PHY timestamping. Keeping it would mean blocking the old
API from ever being removed. In the new dev_set_hwtstamp_phylib() API,
it is not possible to operate on a phylib PHY other than dev->phydev,
and I would very much prefer not adding that much complexity for bizarre
driver decisions.

Final point about the cpsw_hwtstamp_get() conversion: we don't need to
propagate the unnecessary "config.flags = 0;", because dev_get_hwtstamp()
provides a zero-initialized struct kernel_hwtstamp_config.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Link: https://patch.msgid.link/20250512114422.4176010-1-vladimir.oltean@nxp.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Vladimir Oltean and committed by
Jakub Kicinski
36d9b542 265e1d5c

+37 -28
+5
drivers/net/ethernet/ti/cpsw.c
··· 1174 1174 .ndo_setup_tc = cpsw_ndo_setup_tc, 1175 1175 .ndo_bpf = cpsw_ndo_bpf, 1176 1176 .ndo_xdp_xmit = cpsw_ndo_xdp_xmit, 1177 + .ndo_hwtstamp_get = cpsw_hwtstamp_get, 1178 + .ndo_hwtstamp_set = cpsw_hwtstamp_set, 1177 1179 }; 1178 1180 1179 1181 static void cpsw_get_drvinfo(struct net_device *ndev, ··· 1648 1646 ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_CTAG_RX; 1649 1647 ndev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT | 1650 1648 NETDEV_XDP_ACT_NDO_XMIT; 1649 + /* Hijack PHY timestamping requests in order to block them */ 1650 + if (!cpsw->data.dual_emac) 1651 + ndev->see_all_hwtstamp_requests = true; 1651 1652 1652 1653 ndev->netdev_ops = &cpsw_netdev_ops; 1653 1654 ndev->ethtool_ops = &cpsw_ethtool_ops;
+2
drivers/net/ethernet/ti/cpsw_new.c
··· 1147 1147 .ndo_bpf = cpsw_ndo_bpf, 1148 1148 .ndo_xdp_xmit = cpsw_ndo_xdp_xmit, 1149 1149 .ndo_get_port_parent_id = cpsw_get_port_parent_id, 1150 + .ndo_hwtstamp_get = cpsw_hwtstamp_get, 1151 + .ndo_hwtstamp_set = cpsw_hwtstamp_set, 1150 1152 }; 1151 1153 1152 1154 static void cpsw_get_drvinfo(struct net_device *ndev,
+25 -28
drivers/net/ethernet/ti/cpsw_priv.c
··· 614 614 writel_relaxed(ETH_P_8021Q, &cpsw->regs->vlan_ltype); 615 615 } 616 616 617 - static int cpsw_hwtstamp_set(struct net_device *dev, struct ifreq *ifr) 617 + int cpsw_hwtstamp_set(struct net_device *dev, 618 + struct kernel_hwtstamp_config *cfg, 619 + struct netlink_ext_ack *extack) 618 620 { 619 621 struct cpsw_priv *priv = netdev_priv(dev); 620 622 struct cpsw_common *cpsw = priv->cpsw; 621 - struct hwtstamp_config cfg; 623 + 624 + /* This will only execute if dev->see_all_hwtstamp_requests is set */ 625 + if (cfg->source != HWTSTAMP_SOURCE_NETDEV) { 626 + NL_SET_ERR_MSG_MOD(extack, 627 + "Switch mode only supports MAC timestamping"); 628 + return -EOPNOTSUPP; 629 + } 622 630 623 631 if (cpsw->version != CPSW_VERSION_1 && 624 632 cpsw->version != CPSW_VERSION_2 && 625 633 cpsw->version != CPSW_VERSION_3) 626 634 return -EOPNOTSUPP; 627 635 628 - if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg))) 629 - return -EFAULT; 630 - 631 - if (cfg.tx_type != HWTSTAMP_TX_OFF && cfg.tx_type != HWTSTAMP_TX_ON) 636 + if (cfg->tx_type != HWTSTAMP_TX_OFF && cfg->tx_type != HWTSTAMP_TX_ON) 632 637 return -ERANGE; 633 638 634 - switch (cfg.rx_filter) { 639 + switch (cfg->rx_filter) { 635 640 case HWTSTAMP_FILTER_NONE: 636 641 priv->rx_ts_enabled = 0; 637 642 break; ··· 656 651 case HWTSTAMP_FILTER_PTP_V2_SYNC: 657 652 case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ: 658 653 priv->rx_ts_enabled = HWTSTAMP_FILTER_PTP_V2_EVENT; 659 - cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT; 654 + cfg->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT; 660 655 break; 661 656 default: 662 657 return -ERANGE; 663 658 } 664 659 665 - priv->tx_ts_enabled = cfg.tx_type == HWTSTAMP_TX_ON; 660 + priv->tx_ts_enabled = cfg->tx_type == HWTSTAMP_TX_ON; 666 661 667 662 switch (cpsw->version) { 668 663 case CPSW_VERSION_1: ··· 676 671 WARN_ON(1); 677 672 } 678 673 679 - return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0; 674 + return 0; 680 675 } 681 676 682 - static int cpsw_hwtstamp_get(struct net_device *dev, struct ifreq *ifr) 677 + int cpsw_hwtstamp_get(struct net_device *dev, 678 + struct kernel_hwtstamp_config *cfg) 683 679 { 684 680 struct cpsw_common *cpsw = ndev_to_cpsw(dev); 685 681 struct cpsw_priv *priv = netdev_priv(dev); 686 - struct hwtstamp_config cfg; 687 682 688 683 if (cpsw->version != CPSW_VERSION_1 && 689 684 cpsw->version != CPSW_VERSION_2 && 690 685 cpsw->version != CPSW_VERSION_3) 691 686 return -EOPNOTSUPP; 692 687 693 - cfg.flags = 0; 694 - cfg.tx_type = priv->tx_ts_enabled ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF; 695 - cfg.rx_filter = priv->rx_ts_enabled; 688 + cfg->tx_type = priv->tx_ts_enabled ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF; 689 + cfg->rx_filter = priv->rx_ts_enabled; 696 690 697 - return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0; 691 + return 0; 698 692 } 699 693 #else 700 - static int cpsw_hwtstamp_get(struct net_device *dev, struct ifreq *ifr) 694 + int cpsw_hwtstamp_get(struct net_device *dev, 695 + struct kernel_hwtstamp_config *cfg) 701 696 { 702 697 return -EOPNOTSUPP; 703 698 } 704 699 705 - static int cpsw_hwtstamp_set(struct net_device *dev, struct ifreq *ifr) 700 + int cpsw_hwtstamp_set(struct net_device *dev, 701 + struct kernel_hwtstamp_config *cfg, 702 + struct netlink_ext_ack *extack) 706 703 { 707 704 return -EOPNOTSUPP; 708 705 } ··· 721 714 return -EINVAL; 722 715 723 716 phy = cpsw->slaves[slave_no].phy; 724 - 725 - if (!phy_has_hwtstamp(phy)) { 726 - switch (cmd) { 727 - case SIOCSHWTSTAMP: 728 - return cpsw_hwtstamp_set(dev, req); 729 - case SIOCGHWTSTAMP: 730 - return cpsw_hwtstamp_get(dev, req); 731 - } 732 - } 733 - 734 717 if (phy) 735 718 return phy_mii_ioctl(phy, req, cmd); 736 719
+5
drivers/net/ethernet/ti/cpsw_priv.h
··· 469 469 void cpsw_cbs_resume(struct cpsw_slave *slave, struct cpsw_priv *priv); 470 470 void cpsw_mqprio_resume(struct cpsw_slave *slave, struct cpsw_priv *priv); 471 471 void cpsw_qos_clsflower_resume(struct cpsw_priv *priv); 472 + int cpsw_hwtstamp_get(struct net_device *dev, 473 + struct kernel_hwtstamp_config *cfg); 474 + int cpsw_hwtstamp_set(struct net_device *dev, 475 + struct kernel_hwtstamp_config *cfg, 476 + struct netlink_ext_ack *extack); 472 477 473 478 /* ethtool */ 474 479 u32 cpsw_get_msglevel(struct net_device *ndev);