lkml.org 
[lkml]   [2023]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRE: [PATCH v3 3/5] net: Let the active time stamping layer be selectable.
    Köry Maincent wrote:
    > From: Richard Cochran <richardcochran@gmail.com>
    >
    > Add the ETHTOOL_SET_PTP ethtool ioctl, and add checks in the ioctl and time
    > stamping paths to respect the currently selected time stamping layer.
    >
    > Add a preferred-timestamp devicetree binding to select the preferred
    > hardware timestamp layer between PHY and MAC. The choice of using
    > devicetree binding has been made as the PTP precision and quality depends
    > of external things, like adjustable clock, or the lack of a temperature
    > compensated crystal or specific features. Even if the preferred timestamp
    > is a configuration it is hardly related to the design oh the board.

    nit: oh -> of

    >
    > Signed-off-by: Richard Cochran <richardcochran@gmail.com>
    > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
    > ---
    >
    > Notes:
    > 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:
    > - Added 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
    >
    > Documentation/networking/ethtool-netlink.rst | 1 +
    > drivers/net/phy/phy_device.c | 34 ++++++++++++++++
    > include/linux/netdevice.h | 6 +++
    > include/uapi/linux/ethtool.h | 1 +
    > net/core/dev_ioctl.c | 43 ++++++++++++++++++--
    > net/core/timestamping.c | 6 +++
    > net/ethtool/common.c | 16 ++++++--
    > net/ethtool/ioctl.c | 41 ++++++++++++++-----
    > 8 files changed, 131 insertions(+), 17 deletions(-)
    >
    > +void of_set_timestamp(struct net_device *netdev, struct phy_device *phydev)
    > +{
    > + struct device_node *node = phydev->mdio.dev.of_node;
    > + const struct ethtool_ops *ops = netdev->ethtool_ops;
    > + const char *s;
    > + enum timestamping_layer ts_layer = 0;
    > +
    > + if (phy_has_hwtstamp(phydev))
    > + ts_layer = PHY_TIMESTAMPING;
    > + else if (ops->get_ts_info)
    > + ts_layer = MAC_TIMESTAMPING;
    > +
    > + if (of_property_read_string(node, "preferred-timestamp", &s))
    > + goto out;
    > +
    > + if (!s)
    > + goto out;
    > +
    > + if (phy_has_hwtstamp(phydev) && !strcmp(s, "phy"))
    > + ts_layer = PHY_TIMESTAMPING;
    > +
    > + if (ops->get_ts_info && !strcmp(s, "mac"))
    > + ts_layer = MAC_TIMESTAMPING;
    > +
    > +out:
    > + netdev->selected_timestamping_layer = ts_layer;
    > +}
    > +
    > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
    > index ba2bd604359d..d9a1c12fc43c 100644
    > --- a/include/linux/netdevice.h
    > +++ b/include/linux/netdevice.h
    > @@ -47,6 +47,7 @@
    > #include <uapi/linux/netdevice.h>
    > #include <uapi/linux/if_bonding.h>
    > #include <uapi/linux/pkt_cls.h>
    > +#include <uapi/linux/net_tstamp.h>
    > #include <linux/hashtable.h>
    > #include <linux/rbtree.h>
    > #include <net/net_trackers.h>
    > @@ -1981,6 +1982,9 @@ enum netdev_ml_priv_type {
    > *
    > * @threaded: napi threaded mode is enabled
    > *
    > + * @selected_timestamping_layer: Tracks whether the MAC or the PHY
    > + * performs packet time stamping.
    > + *
    > * @net_notifier_list: List of per-net netdev notifier block
    > * that follow this device when it is moved
    > * to another network namespace.
    > @@ -2339,6 +2343,8 @@ struct net_device {
    > unsigned wol_enabled:1;
    > unsigned threaded:1;
    >
    > + enum timestamping_layer selected_timestamping_layer;
    > +

    can perhaps be a single bit rather than an enum

    > +static int dev_hwtstamp_ioctl(struct net_device *dev,
    > + struct ifreq *ifr, unsigned int cmd)
    > +{
    > + const struct net_device_ops *ops = dev->netdev_ops;
    > + int err;
    > +
    > + err = dsa_ndo_eth_ioctl(dev, ifr, cmd);
    > + if (err == 0 || err != -EOPNOTSUPP)
    > + return err;
    > +
    > + if (!netif_device_present(dev))
    > + return -ENODEV;
    > +
    > + switch (dev->selected_timestamping_layer) {
    > + case MAC_TIMESTAMPING:
    > + if (ops->ndo_do_ioctl == phy_do_ioctl) {
    > + /* Some drivers set .ndo_do_ioctl to phy_do_ioctl. */
    > + err = -EOPNOTSUPP;
    > + } else {
    > + err = ops->ndo_eth_ioctl(dev, ifr, cmd);
    > + }
    > + break;
    > +
    > + case PHY_TIMESTAMPING:
    > + if (phy_has_hwtstamp(dev->phydev)) {
    > + err = phy_mii_ioctl(dev->phydev, ifr, cmd);
    > + } else {
    > + err = -ENODEV;
    > + WARN_ON(1);

    Please no WARN_ON on error cases that are known to be reachable
    and can be handled safely and reported to userspace.

    > + }
    > + break;
    > + }
    > +
    > + return err;
    > +}
    > +
    > diff --git a/net/ethtool/common.c b/net/ethtool/common.c
    > index 64a7e05cf2c2..e55e70bdbb3c 100644
    > --- a/net/ethtool/common.c
    > +++ b/net/ethtool/common.c
    > @@ -548,10 +548,18 @@ int __ethtool_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
    > memset(info, 0, sizeof(*info));
    > info->cmd = ETHTOOL_GET_TS_INFO;
    >
    > - if (phy_has_tsinfo(phydev))
    > - return phy_ts_info(phydev, info);
    > - if (ops->get_ts_info)
    > - return ops->get_ts_info(dev, info);
    > + switch (dev->selected_timestamping_layer) {
    > + case MAC_TIMESTAMPING:
    > + if (ops->get_ts_info)
    > + return ops->get_ts_info(dev, info);
    > + break;
    > +
    > + case PHY_TIMESTAMPING:
    > + if (phy_has_tsinfo(phydev))
    > + return phy_ts_info(phydev, info);
    > + WARN_ON(1);
    > + return -ENODEV;

    same

    > + }
    >
    > info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
    > SOF_TIMESTAMPING_SOFTWARE;
    \
     
     \ /
      Last update: 2023-03-27 00:50    [W:4.810 / U:2.348 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site