lkml.org 
[lkml]   [2020]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 08/10] net: eth: altera: add support for ptp and timestamping
    On Wed, Jul 08, 2020 at 03:23:59PM +0800, Ooi, Joyce wrote:

    > @@ -222,6 +223,32 @@ static void tse_get_regs(struct net_device *dev, struct ethtool_regs *regs,
    > buf[i] = csrrd32(priv->mac_dev, i * 4);
    > }
    >
    > +static int tse_get_ts_info(struct net_device *dev,
    > + struct ethtool_ts_info *info)
    > +{
    > + struct altera_tse_private *priv = netdev_priv(dev);
    > +
    > + if (priv->ptp_enable) {
    > + if (priv->ptp_priv.ptp_clock)
    > + info->phc_index =
    > + ptp_clock_index(priv->ptp_priv.ptp_clock);

    Need to handle case where priv->ptp_priv.ptp_clock == NULL.

    > + info->so_timestamping = SOF_TIMESTAMPING_TX_HARDWARE |
    > + SOF_TIMESTAMPING_RX_HARDWARE |
    > + SOF_TIMESTAMPING_RAW_HARDWARE;
    > +
    > + info->tx_types = (1 << HWTSTAMP_TX_OFF) |
    > + (1 << HWTSTAMP_TX_ON);

    No need to break statement. This fits nicely on one line.

    > +
    > + info->rx_filters = (1 << HWTSTAMP_FILTER_NONE) |
    > + (1 << HWTSTAMP_FILTER_ALL);
    > +
    > + return 0;
    > + } else {

    No need for else block.

    > + return ethtool_op_get_ts_info(dev, info);
    > + }
    > +}
    > +
    > static const struct ethtool_ops tse_ethtool_ops = {
    > .get_drvinfo = tse_get_drvinfo,
    > .get_regs_len = tse_reglen,


    > @@ -1309,6 +1324,83 @@ static int tse_shutdown(struct net_device *dev)
    > return 0;
    > }
    >
    > +/* ioctl to configure timestamping */
    > +static int tse_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
    > +{
    > + struct altera_tse_private *priv = netdev_priv(dev);
    > + struct hwtstamp_config config;

    Need to check here for phy_has_hwtstamp() and pass through to PHY
    layer if true.

    > +
    > + if (!netif_running(dev))
    > + return -EINVAL;
    > +
    > + if (!priv->ptp_enable) {
    > + netdev_alert(priv->dev, "Timestamping not supported");
    > + return -EOPNOTSUPP;
    > + }
    > +
    > + if (cmd == SIOCSHWTSTAMP) {
    > + if (copy_from_user(&config, ifr->ifr_data,
    > + sizeof(struct hwtstamp_config)))
    > + return -EFAULT;
    > +
    > + if (config.flags)
    > + return -EINVAL;
    > +
    > + switch (config.tx_type) {
    > + case HWTSTAMP_TX_OFF:
    > + priv->hwts_tx_en = 0;
    > + break;
    > + case HWTSTAMP_TX_ON:
    > + priv->hwts_tx_en = 1;
    > + break;
    > + default:
    > + return -ERANGE;
    > + }
    > +
    > + switch (config.rx_filter) {
    > + case HWTSTAMP_FILTER_NONE:
    > + priv->hwts_rx_en = 0;
    > + config.rx_filter = HWTSTAMP_FILTER_NONE;
    > + break;
    > + default:
    > + priv->hwts_rx_en = 1;
    > + config.rx_filter = HWTSTAMP_FILTER_ALL;
    > + break;
    > + }
    > +
    > + if (copy_to_user(ifr->ifr_data, &config,
    > + sizeof(struct hwtstamp_config)))
    > + return -EFAULT;
    > + else
    > + return 0;
    > + }
    > +
    > + if (cmd == SIOCGHWTSTAMP) {
    > + config.flags = 0;
    > +
    > + if (priv->hwts_tx_en)
    > + config.tx_type = HWTSTAMP_TX_ON;
    > + else
    > + config.tx_type = HWTSTAMP_TX_OFF;
    > +
    > + if (priv->hwts_rx_en)
    > + config.rx_filter = HWTSTAMP_FILTER_ALL;
    > + else
    > + config.rx_filter = HWTSTAMP_FILTER_NONE;
    > +
    > + if (copy_to_user(ifr->ifr_data, &config,
    > + sizeof(struct hwtstamp_config)))
    > + return -EFAULT;
    > + else
    > + return 0;
    > + }
    > +
    > + if (!dev->phydev)
    > + return -EINVAL;
    > +
    > + return phy_mii_ioctl(dev->phydev, ifr, cmd);
    > +}
    > +
    > static struct net_device_ops altera_tse_netdev_ops = {
    > .ndo_open = tse_open,
    > .ndo_stop = tse_shutdown,


    > @@ -1568,6 +1661,27 @@ static int altera_tse_probe(struct platform_device *pdev)
    > netdev_err(ndev, "Cannot attach to PHY (error: %d)\n", ret);
    > goto err_init_phy;
    > }
    > +
    > + priv->ptp_enable = of_property_read_bool(pdev->dev.of_node,
    > + "altr,has-ptp");

    The name "ptp_enable" is a poor choice. It sounds like something that
    can be enabled at run time. Suggest "has_ptp" instead.

    > + dev_info(&pdev->dev, "PTP Enable: %d\n", priv->ptp_enable);
    > +
    > + if (priv->ptp_enable) {
    > + /* MAP PTP */
    > + ret = intel_fpga_tod_probe(pdev, &priv->ptp_priv);
    > + if (ret) {
    > + dev_err(&pdev->dev, "cannot map PTP\n");
    > + goto err_init_phy;
    > + }
    > + ret = intel_fpga_tod_register(&priv->ptp_priv,
    > + priv->device);
    > + if (ret) {
    > + dev_err(&pdev->dev, "Failed to register PTP clock\n");
    > + ret = -ENXIO;
    > + goto err_init_phy;
    > + }
    > + }
    > +
    > return 0;
    >
    > err_init_phy:


    > +/* Initialize PTP control block registers */
    > +int intel_fpga_tod_init(struct intel_fpga_tod_private *priv)
    > +{
    > + struct timespec64 now;
    > + int ret = 0;
    > +
    > + ret = intel_fpga_tod_adjust_fine(&priv->ptp_clock_ops, 0l);

    Why clobber a learned frequency offset here? If user space closes
    then re-opens, then it expects the old frequency to be preserved.

    It is fine to set this to zero when the driver loads, but not after.

    > + if (ret != 0)
    > + goto out;
    > +
    > + /* Initialize the hardware clock to the system time */
    > + ktime_get_real_ts64(&now);

    Please initialize to zero instead, as some people prefer it that way.

    (But only the first time when the driver loads!)

    > + intel_fpga_tod_set_time(&priv->ptp_clock_ops, &now);
    > +
    > + spin_lock_init(&priv->tod_lock);
    > +
    > +out:
    > + return ret;
    > +}

    Thanks,
    Richard

    \
     
     \ /
      Last update: 2020-07-09 13:32    [W:4.919 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site