Based on previous RFCs from Maxim Georgiev:
https://lore.kernel.org/netdev/[email protected]/
this series attempts to introduce new API for the hardware timestamping
control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
I don't have any board with phylib hardware timestamping, so I would
appreciate testing (especially on lan966x, the most intricate
conversion). I was, however, able to test netdev level timestamping,
because I also have some more unsubmitted conversions in progress:
https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
I hope that the concerns expressed in the comments of previous series
were addressed, and that Köry Maincent's series:
https://lore.kernel.org/netdev/[email protected]/
can make progress in parallel with the conversion of the rest of drivers.
Maxim Georgiev (5):
net: add NDOs for configuring hardware timestamping
net: add hwtstamping helpers for stackable net devices
net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
Vladimir Oltean (7):
net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
net: fec: delete fec_ptp_disable_hwts()
net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
net: transfer rtnl_lock() requirement from
ethtool_set_ethtool_phy_ops() to caller
net: phy: provide phylib stubs for hardware timestamping operations
net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from
converted drivers
drivers/net/bonding/bond_main.c | 105 ++++++----
drivers/net/ethernet/freescale/fec.h | 6 +-
drivers/net/ethernet/freescale/fec_main.c | 56 +++---
drivers/net/ethernet/freescale/fec_ptp.c | 43 ++--
.../ethernet/microchip/lan966x/lan966x_main.c | 58 +++---
.../ethernet/microchip/lan966x/lan966x_main.h | 12 +-
.../ethernet/microchip/lan966x/lan966x_ptp.c | 34 ++--
.../ethernet/microchip/sparx5/sparx5_main.h | 9 +-
.../ethernet/microchip/sparx5/sparx5_netdev.c | 35 ++--
.../ethernet/microchip/sparx5/sparx5_ptp.c | 24 ++-
drivers/net/macvlan.c | 34 ++--
drivers/net/phy/Makefile | 2 +
drivers/net/phy/phy.c | 34 ++++
drivers/net/phy/phy_device.c | 26 ++-
include/linux/net_tstamp.h | 30 +++
include/linux/netdevice.h | 25 +++
include/linux/phy.h | 7 +
net/8021q/vlan_dev.c | 27 ++-
net/core/dev_ioctl.c | 184 +++++++++++++++++-
net/ethtool/common.c | 3 +-
20 files changed, 544 insertions(+), 210 deletions(-)
--
2.34.1
net/core/dev_ioctl.c (built-in code) will want to call phy_mii_ioctl()
for hardware timestamping purposes. This is not directly possible,
because phy_mii_ioctl() is a symbol provided under CONFIG_PHYLIB.
Do something similar to what was done in DSA in commit 5a17818682cf
("net: dsa: replace NETDEV_PRE_CHANGE_HWTSTAMP notifier with a stub"),
and arrange some indirect calls to phy_mii_ioctl() through a stub
structure containing function pointers, that's provided by phylib as
built-in even when CONFIG_PHYLIB=m, and which phy_init() populates at
runtime (module insertion).
Note: maybe the ownership of the ethtool_phy_ops singleton is backwards,
and the methods exposed by that should be later merged into phylib_stubs.
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v8:
- Patch is new
drivers/net/phy/Makefile | 2 ++
drivers/net/phy/phy.c | 34 ++++++++++++++++++++++++++++++++++
drivers/net/phy/phy_device.c | 18 ++++++++++++++++++
include/linux/phy.h | 7 +++++++
4 files changed, 61 insertions(+)
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 2fe51ea83bab..5b5b0d300220 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -14,6 +14,8 @@ endif
# dedicated loadable module, so we bundle them all together into libphy.ko
ifdef CONFIG_PHYLIB
libphy-y += $(mdio-bus-y)
+# the stubs are built-in whenever PHYLIB is built-in or module
+obj-y += stubs.o
else
obj-$(CONFIG_MDIO_DEVICE) += mdio-bus.o
endif
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index bdf00b2b2c1d..8aec8e83038c 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -455,6 +455,40 @@ int phy_do_ioctl_running(struct net_device *dev, struct ifreq *ifr, int cmd)
}
EXPORT_SYMBOL(phy_do_ioctl_running);
+/**
+ * __phy_hwtstamp_get - Get hardware timestamping configuration from PHY
+ *
+ * @phydev: the PHY device structure
+ * @config: structure holding the timestamping configuration
+ *
+ * Query the PHY device for its current hardware timestamping configuration.
+ */
+int __phy_hwtstamp_get(struct phy_device *phydev,
+ struct kernel_hwtstamp_config *config)
+{
+ if (!phydev)
+ return -ENODEV;
+
+ return phy_mii_ioctl(phydev, config->ifr, SIOCGHWTSTAMP);
+}
+
+/**
+ * __phy_hwtstamp_set - Modify PHY hardware timestamping configuration
+ *
+ * @phydev: the PHY device structure
+ * @config: structure holding the timestamping configuration
+ * @extack: netlink extended ack structure, for error reporting
+ */
+int __phy_hwtstamp_set(struct phy_device *phydev,
+ struct kernel_hwtstamp_config *config,
+ struct netlink_ext_ack *extack)
+{
+ if (!phydev)
+ return -ENODEV;
+
+ return phy_mii_ioctl(phydev, config->ifr, SIOCSHWTSTAMP);
+}
+
/**
* phy_queue_state_machine - Trigger the state machine to run soon
*
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index ab53d10f1844..08c162b7e6be 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -27,6 +27,7 @@
#include <linux/of.h>
#include <linux/netdevice.h>
#include <linux/phy.h>
+#include <linux/phylib_stubs.h>
#include <linux/phy_led_triggers.h>
#include <linux/pse-pd/pse.h>
#include <linux/property.h>
@@ -3448,12 +3449,28 @@ static const struct ethtool_phy_ops phy_ethtool_phy_ops = {
.start_cable_test_tdr = phy_start_cable_test_tdr,
};
+static const struct phylib_stubs __phylib_stubs = {
+ .hwtstamp_get = __phy_hwtstamp_get,
+ .hwtstamp_set = __phy_hwtstamp_set,
+};
+
+static void phylib_register_stubs(void)
+{
+ phylib_stubs = &__phylib_stubs;
+}
+
+static void phylib_unregister_stubs(void)
+{
+ phylib_stubs = NULL;
+}
+
static int __init phy_init(void)
{
int rc;
rtnl_lock();
ethtool_set_ethtool_phy_ops(&phy_ethtool_phy_ops);
+ phylib_register_stubs();
rtnl_unlock();
rc = mdio_bus_init();
@@ -3483,6 +3500,7 @@ static void __exit phy_exit(void)
mdio_bus_exit();
rtnl_lock();
ethtool_set_ethtool_phy_ops(NULL);
+ phylib_unregister_stubs();
rtnl_unlock();
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 11c1e91563d4..6710508e8c97 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -298,6 +298,7 @@ static inline const char *phy_modes(phy_interface_t interface)
#define MII_BUS_ID_SIZE 61
struct device;
+struct kernel_hwtstamp_config;
struct phylink;
struct sfp_bus;
struct sfp_upstream_ops;
@@ -1954,6 +1955,12 @@ int phy_ethtool_set_plca_cfg(struct phy_device *phydev,
int phy_ethtool_get_plca_status(struct phy_device *phydev,
struct phy_plca_status *plca_st);
+int __phy_hwtstamp_get(struct phy_device *phydev,
+ struct kernel_hwtstamp_config *config);
+int __phy_hwtstamp_set(struct phy_device *phydev,
+ struct kernel_hwtstamp_config *config,
+ struct netlink_ext_ack *extack);
+
static inline int phy_package_read(struct phy_device *phydev, u32 regnum)
{
struct phy_package_shared *shared = phydev->shared;
--
2.34.1
On Mon, Jul 17, 2023 at 06:27:08PM +0300, Vladimir Oltean wrote:
> net/core/dev_ioctl.c (built-in code) will want to call phy_mii_ioctl()
> for hardware timestamping purposes. This is not directly possible,
> because phy_mii_ioctl() is a symbol provided under CONFIG_PHYLIB.
>
> Do something similar to what was done in DSA in commit 5a17818682cf
> ("net: dsa: replace NETDEV_PRE_CHANGE_HWTSTAMP notifier with a stub"),
> and arrange some indirect calls to phy_mii_ioctl() through a stub
> structure containing function pointers, that's provided by phylib as
> built-in even when CONFIG_PHYLIB=m, and which phy_init() populates at
> runtime (module insertion).
>
> Note: maybe the ownership of the ethtool_phy_ops singleton is backwards,
> and the methods exposed by that should be later merged into phylib_stubs.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
I should use "git add" on new files more often...
Annex to this patch:
diff --git a/drivers/net/phy/stubs.c b/drivers/net/phy/stubs.c
new file mode 100644
index 000000000000..06498de2d16a
--- /dev/null
+++ b/drivers/net/phy/stubs.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Stubs for PHY library functionality called by the core network stack.
+ * These are necessary because CONFIG_PHYLIB can be a module, and built-in
+ * code cannot directly call symbols exported by modules.
+ */
+#include <net/dsa_stubs.h>
+
+const struct phylib_stubs *phylib_stubs;
+EXPORT_SYMBOL_GPL(phylib_stubs);
diff --git a/include/linux/phylib_stubs.h b/include/linux/phylib_stubs.h
new file mode 100644
index 000000000000..1279f48c8a70
--- /dev/null
+++ b/include/linux/phylib_stubs.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Stubs for the Network PHY library
+ */
+
+#include <linux/rtnetlink.h>
+
+struct kernel_hwtstamp_config;
+struct netlink_ext_ack;
+struct phy_device;
+
+#if IS_ENABLED(CONFIG_PHYLIB)
+
+extern const struct phylib_stubs *phylib_stubs;
+
+struct phylib_stubs {
+ int (*hwtstamp_get)(struct phy_device *phydev,
+ struct kernel_hwtstamp_config *config);
+ int (*hwtstamp_set)(struct phy_device *phydev,
+ struct kernel_hwtstamp_config *config,
+ struct netlink_ext_ack *extack);
+};
+
+static inline int phy_hwtstamp_get(struct phy_device *phydev,
+ struct kernel_hwtstamp_config *config)
+{
+ /* phylib_register_stubs() and phylib_unregister_stubs()
+ * also run under rtnl_lock().
+ */
+ ASSERT_RTNL();
+
+ if (!phylib_stubs)
+ return -EOPNOTSUPP;
+
+ return phylib_stubs->hwtstamp_get(phydev, config);
+}
+
+static inline int phy_hwtstamp_set(struct phy_device *phydev,
+ struct kernel_hwtstamp_config *config,
+ struct netlink_ext_ack *extack)
+{
+ /* phylib_register_stubs() and phylib_unregister_stubs()
+ * also run under rtnl_lock().
+ */
+ ASSERT_RTNL();
+
+ if (!phylib_stubs)
+ return -EOPNOTSUPP;
+
+ return phylib_stubs->hwtstamp_set(phydev, config, extack);
+}
+
+#else
+
+static inline int phy_hwtstamp_get(struct phy_device *phydev,
+ struct kernel_hwtstamp_config *config)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int phy_hwtstamp_set(struct phy_device *phydev,
+ struct kernel_hwtstamp_config *config,
+ struct netlink_ext_ack *extack)
+{
+ return -EOPNOTSUPP;
+}
+
+#endif
On Mon, Jul 17, 2023 at 06:26:57PM +0300, Vladimir Oltean wrote:
> Based on previous RFCs from Maxim Georgiev:
> https://lore.kernel.org/netdev/[email protected]/
>
> this series attempts to introduce new API for the hardware timestamping
> control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
>
> I don't have any board with phylib hardware timestamping, so I would
> appreciate testing (especially on lan966x, the most intricate
> conversion). I was, however, able to test netdev level timestamping,
> because I also have some more unsubmitted conversions in progress:
>
> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
>
> I hope that the concerns expressed in the comments of previous series
> were addressed, and that K?ry Maincent's series:
> https://lore.kernel.org/netdev/[email protected]/
> can make progress in parallel with the conversion of the rest of drivers.
I'll be waiting until the end of today for more feedback, then I'll
resend this with all the pieces actually added to the commit (and with
include/linux/phylib_stubs.h part of the ETHERNET PHY LIBRARY entry in
MAINTAINERS).
pw-bot: cr