On Thu, Jul 12, 2018 at 03:05:06PM -0400, Bryan Whitehead wrote:
> +static int lan743x_ethtool_get_ts_info(struct net_device *netdev,
> +                                    struct ethtool_ts_info *ts_info)
> +{
> +     struct lan743x_adapter *adapter = netdev_priv(netdev);
> +
> +     ts_info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE |
> +                                SOF_TIMESTAMPING_RX_SOFTWARE |
> +                                SOF_TIMESTAMPING_SOFTWARE |
> +                                SOF_TIMESTAMPING_TX_HARDWARE |
> +                                SOF_TIMESTAMPING_RX_HARDWARE |
> +                                SOF_TIMESTAMPING_RAW_HARDWARE;
> +#ifdef CONFIG_PTP_1588_CLOCK

No need for this ifdeferry - ptp_clock_index() already returns -1 in
that case.

> +     if (adapter->ptp.ptp_clock)
> +             ts_info->phc_index = ptp_clock_index(adapter->ptp.ptp_clock);
> +     else
> +             ts_info->phc_index = -1;
> +#else
> +     ts_info->phc_index = -1;
> +#endif
> +     ts_info->tx_types = BIT(HWTSTAMP_TX_OFF) |
> +                         BIT(HWTSTAMP_TX_ON);
> +     ts_info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) |
> +                           BIT(HWTSTAMP_FILTER_ALL);
> +     return 0;
> +}
> +

> @@ -690,6 +717,7 @@ const struct ethtool_ops lan743x_ethtool_ops = {
>       .get_rxfh_indir_size = lan743x_ethtool_get_rxfh_indir_size,
>       .get_rxfh = lan743x_ethtool_get_rxfh,
>       .set_rxfh = lan743x_ethtool_set_rxfh,
> +     .get_ts_info = lan743x_ethtool_get_ts_info,
>       .get_eee = lan743x_ethtool_get_eee,
>       .set_eee = lan743x_ethtool_set_eee,
>       .get_link_ksettings = phy_ethtool_get_link_ksettings,
> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c 
> b/drivers/net/ethernet/microchip/lan743x_main.c
> index 953b581..ca9ae49 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -267,6 +267,10 @@ static void lan743x_intr_shared_isr(void *context, u32 
> int_sts, u32 flags)
>                       lan743x_intr_software_isr(adapter);
>                       int_sts &= ~INT_BIT_SW_GP_;
>               }
> +             if (int_sts & INT_BIT_1588_) {
> +                     lan743x_ptp_isr(adapter);
> +                     int_sts &= ~INT_BIT_1588_;
> +             }
>       }
>       if (int_sts)
>               lan743x_csr_write(adapter, INT_EN_CLR, int_sts);
> @@ -976,6 +980,7 @@ static void lan743x_phy_link_status_change(struct 
> net_device *netdev)
>                                              ksettings.base.duplex,
>                                              local_advertisement,
>                                              remote_advertisement);
> +             lan743x_ptp_update_latency(adapter, ksettings.base.speed);
>       }
>  }
>  
> @@ -1256,11 +1261,29 @@ static void lan743x_tx_release_desc(struct lan743x_tx 
> *tx,
>               buffer_info->dma_ptr = 0;
>               buffer_info->buffer_length = 0;
>       }
> -     if (buffer_info->skb) {
> +     if (!buffer_info->skb)
> +             goto clear_active;
> +
> +     if (!(buffer_info->flags &
> +             TX_BUFFER_INFO_FLAG_TIMESTAMP_REQUESTED)) {

Bad line break.

>               dev_kfree_skb(buffer_info->skb);
> -             buffer_info->skb = NULL;
> +             goto clear_skb;
>       }
>  
> +     if (cleanup) {
> +             lan743x_ptp_unrequest_tx_timestamp(tx->adapter);
> +             dev_kfree_skb(buffer_info->skb);
> +     } else {
> +             lan743x_ptp_tx_timestamp_skb(tx->adapter,
> +                                          buffer_info->skb,
> +                                          (buffer_info->flags &
> +                                          TX_BUFFER_INFO_FLAG_IGNORE_SYNC)
> +                                          != 0);

This is poor coding style.  Please find a better way.

> +     }
> +
> +clear_skb:
> +     buffer_info->skb = NULL;
> +
>  clear_active:
>       buffer_info->flags &= ~TX_BUFFER_INFO_FLAG_ACTIVE;
>  
> @@ -1321,10 +1344,25 @@ static int lan743x_tx_get_avail_desc(struct 
> lan743x_tx *tx)
>               return last_head - last_tail - 1;
>  }
>  
> +void lan743x_tx_set_timestamping_mode(struct lan743x_tx *tx,
> +                                   bool enable_timestamping,
> +                                   bool enable_onestep_sync)
> +{
> +     if (enable_timestamping)
> +             tx->ts_flags |= TX_TS_FLAG_TIMESTAMPING_ENABLED;
> +     else
> +             tx->ts_flags &= ~TX_TS_FLAG_TIMESTAMPING_ENABLED;
> +     if (enable_onestep_sync)
> +             tx->ts_flags |= TX_TS_FLAG_ONE_STEP_SYNC;
> +     else
> +             tx->ts_flags &= ~TX_TS_FLAG_ONE_STEP_SYNC;
> +}
> +
>  static int lan743x_tx_frame_start(struct lan743x_tx *tx,
>                                 unsigned char *first_buffer,
>                                 unsigned int first_buffer_length,
>                                 unsigned int frame_length,
> +                               bool time_stamp,
>                                 bool check_sum)
>  {
>       /* called only from within lan743x_tx_xmit_frame.
> @@ -1362,6 +1400,8 @@ static int lan743x_tx_frame_start(struct lan743x_tx *tx,
>               TX_DESC_DATA0_DTYPE_DATA_ |
>               TX_DESC_DATA0_FS_ |
>               TX_DESC_DATA0_FCS_;
> +     if (time_stamp)
> +             tx->frame_data0 |= TX_DESC_DATA0_TSE_;
>  
>       if (check_sum)
>               tx->frame_data0 |= TX_DESC_DATA0_ICE_ |
> @@ -1475,6 +1515,7 @@ static int lan743x_tx_frame_add_fragment(struct 
> lan743x_tx *tx,
>  
>  static void lan743x_tx_frame_end(struct lan743x_tx *tx,
>                                struct sk_buff *skb,
> +                              bool time_stamp,
>                                bool ignore_sync)
>  {
>       /* called only from within lan743x_tx_xmit_frame
> @@ -1492,6 +1533,8 @@ static void lan743x_tx_frame_end(struct lan743x_tx *tx,
>       tx_descriptor = &tx->ring_cpu_ptr[tx->frame_tail];
>       buffer_info = &tx->buffer_info[tx->frame_tail];
>       buffer_info->skb = skb;
> +     if (time_stamp)
> +             buffer_info->flags |= TX_BUFFER_INFO_FLAG_TIMESTAMP_REQUESTED;
>       if (ignore_sync)
>               buffer_info->flags |= TX_BUFFER_INFO_FLAG_IGNORE_SYNC;
>  
> @@ -1520,6 +1563,7 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct 
> lan743x_tx *tx,
>       unsigned int frame_length = 0;
>       unsigned int head_length = 0;
>       unsigned long irq_flags = 0;
> +     bool do_timestamp = false;
>       bool ignore_sync = false;
>       int nr_frags = 0;
>       bool gso = false;
> @@ -1541,6 +1585,16 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct 
> lan743x_tx *tx,
>       }
>  
>       /* space available, transmit skb  */
> +     if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {
> +             if (tx->ts_flags & TX_TS_FLAG_TIMESTAMPING_ENABLED) {
> +                     if (lan743x_ptp_request_tx_timestamp(tx->adapter)) {

Why not use && instead of three nested tests?

> +                             skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> +                             do_timestamp = true;
> +                             if (tx->ts_flags & TX_TS_FLAG_ONE_STEP_SYNC)
> +                                     ignore_sync = true;
> +                     }
> +             }
> +     }
>       head_length = skb_headlen(skb);
>       frame_length = skb_pagelen(skb);
>       nr_frags = skb_shinfo(skb)->nr_frags;
> @@ -1554,6 +1608,7 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct 
> lan743x_tx *tx,
>       if (lan743x_tx_frame_start(tx,
>                                  skb->data, head_length,
>                                  start_frame_length,
> +                                do_timestamp,
>                                  skb->ip_summed == CHECKSUM_PARTIAL)) {
>               dev_kfree_skb(skb);
>               goto unlock;
> @@ -1581,7 +1636,7 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct 
> lan743x_tx *tx,
>       }
>  
>  finish:
> -     lan743x_tx_frame_end(tx, skb, ignore_sync);
> +     lan743x_tx_frame_end(tx, skb, do_timestamp, ignore_sync);
>  
>  unlock:
>       spin_unlock_irqrestore(&tx->ring_lock, irq_flags);
> @@ -2410,6 +2465,8 @@ static int lan743x_netdev_close(struct net_device 
> *netdev)
>       for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++)
>               lan743x_rx_close(&adapter->rx[index]);
>  
> +     lan743x_ptp_close(adapter);
> +
>       lan743x_phy_close(adapter);
>  
>       lan743x_mac_close(adapter);
> @@ -2437,6 +2494,10 @@ static int lan743x_netdev_open(struct net_device 
> *netdev)
>       if (ret)
>               goto close_mac;
>  
> +     ret = lan743x_ptp_open(adapter);
> +     if (ret)
> +             goto close_phy;
> +
>       lan743x_rfe_open(adapter);
>  
>       for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
> @@ -2456,6 +2517,9 @@ static int lan743x_netdev_open(struct net_device 
> *netdev)
>               if (adapter->rx[index].ring_cpu_ptr)
>                       lan743x_rx_close(&adapter->rx[index]);
>       }
> +     lan743x_ptp_close(adapter);
> +
> +close_phy:
>       lan743x_phy_close(adapter);
>  
>  close_mac:
> @@ -2483,6 +2547,8 @@ static int lan743x_netdev_ioctl(struct net_device 
> *netdev,
>  {
>       if (!netif_running(netdev))
>               return -EINVAL;
> +     if (cmd == SIOCSHWTSTAMP)
> +             return lan743x_ptp_ioctl(netdev, ifr, cmd);
>       return phy_mii_ioctl(netdev->phydev, ifr, cmd);
>  }
>  
> @@ -2607,6 +2673,11 @@ static int lan743x_hardware_init(struct 
> lan743x_adapter *adapter,
>       adapter->intr.irq = adapter->pdev->irq;
>       lan743x_csr_write(adapter, INT_EN_CLR, 0xFFFFFFFF);
>       mutex_init(&adapter->dp_lock);
> +
> +     ret = lan743x_gpio_init(adapter);
> +     if (ret)
> +             return ret;
> +
>       ret = lan743x_mac_init(adapter);
>       if (ret)
>               return ret;
> @@ -2615,6 +2686,10 @@ static int lan743x_hardware_init(struct 
> lan743x_adapter *adapter,
>       if (ret)
>               return ret;
>  
> +     ret = lan743x_ptp_init(adapter);
> +     if (ret)
> +             return ret;
> +
>       lan743x_rfe_update_mac_address(adapter);
>  
>       ret = lan743x_dmac_init(adapter);

...

> diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c 
> b/drivers/net/ethernet/microchip/lan743x_ptp.c
> new file mode 100644
> index 0000000..f14565b
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
> @@ -0,0 +1,1194 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright (C) 2018 Microchip Technology Inc. */
> +
> +#include <linux/netdevice.h>
> +#include "lan743x_main.h"
> +
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/netdevice.h>
> +#include <linux/net_tstamp.h>
> +
> +#include "lan743x_ptp.h"
> +
> +/* GPIO */
> +#define LAN743X_NUMBER_OF_GPIO          (12)
> +
> +int lan743x_gpio_init(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_gpio *gpio = &adapter->gpio;
> +
> +     spin_lock_init(&gpio->gpio_lock);
> +
> +     gpio->gpio_cfg0 = 0; /* set all direction to input, data = 0 */
> +     gpio->gpio_cfg1 = 0x0FFF0000;/* disable all gpio, set to open drain */
> +     gpio->gpio_cfg2 = 0;/* set all to 1588 low polarity level */
> +     gpio->gpio_cfg3 = 0;/* disable all 1588 output */
> +     lan743x_csr_write(adapter, GPIO_CFG0, gpio->gpio_cfg0);
> +     lan743x_csr_write(adapter, GPIO_CFG1, gpio->gpio_cfg1);
> +     lan743x_csr_write(adapter, GPIO_CFG2, gpio->gpio_cfg2);
> +     lan743x_csr_write(adapter, GPIO_CFG3, gpio->gpio_cfg3);
> +
> +     return 0;
> +}
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_gpio_reserve_ptp_output(struct lan743x_adapter *adapter,
> +                                        int bit, int ptp_channel)
> +{
> +     struct lan743x_gpio *gpio = &adapter->gpio;
> +     unsigned long irq_flags = 0;
> +     int bit_mask = BIT(bit);
> +     int ret = -EBUSY;
> +
> +     spin_lock_irqsave(&gpio->gpio_lock, irq_flags);
> +
> +     if (!(gpio->used_bits & bit_mask)) {
> +             gpio->used_bits |= bit_mask;
> +             gpio->output_bits |= bit_mask;
> +             gpio->ptp_bits |= bit_mask;
> +
> +             /* set as output, and zero initial value */
> +             gpio->gpio_cfg0 |= GPIO_CFG0_GPIO_DIR_BIT_(bit);
> +             gpio->gpio_cfg0 &= ~GPIO_CFG0_GPIO_DATA_BIT_(bit);
> +             lan743x_csr_write(adapter, GPIO_CFG0, gpio->gpio_cfg0);
> +
> +             /* enable gpio , and set buffer type to push pull */
> +             gpio->gpio_cfg1 &= ~GPIO_CFG1_GPIOEN_BIT_(bit);
> +             gpio->gpio_cfg1 |= GPIO_CFG1_GPIOBUF_BIT_(bit);
> +             lan743x_csr_write(adapter, GPIO_CFG1, gpio->gpio_cfg1);
> +
> +             /* set 1588 polarity to high */
> +             gpio->gpio_cfg2 |= GPIO_CFG2_1588_POL_BIT_(bit);
> +             lan743x_csr_write(adapter, GPIO_CFG2, gpio->gpio_cfg2);
> +
> +             if (!ptp_channel) {
> +                     /* use channel A */
> +                     gpio->gpio_cfg3 &= ~GPIO_CFG3_1588_CH_SEL_BIT_(bit);
> +             } else {
> +                     /* use channel B */
> +                     gpio->gpio_cfg3 |= GPIO_CFG3_1588_CH_SEL_BIT_(bit);
> +             }
> +             gpio->gpio_cfg3 |= GPIO_CFG3_1588_OE_BIT_(bit);
> +             lan743x_csr_write(adapter, GPIO_CFG3, gpio->gpio_cfg3);
> +
> +             ret = bit;
> +     }
> +     spin_unlock_irqrestore(&gpio->gpio_lock, irq_flags);
> +     return ret;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_gpio_release(struct lan743x_adapter *adapter, int bit)
> +{
> +     struct lan743x_gpio *gpio = &adapter->gpio;
> +     unsigned long irq_flags = 0;
> +     int bit_mask = BIT(bit);
> +
> +     spin_lock_irqsave(&gpio->gpio_lock, irq_flags);
> +     if (gpio->used_bits & bit_mask) {
> +             gpio->used_bits &= ~bit_mask;
> +             if (gpio->output_bits & bit_mask) {
> +                     gpio->output_bits &= ~bit_mask;
> +
> +                     if (gpio->ptp_bits & bit_mask) {
> +                             gpio->ptp_bits &= ~bit_mask;
> +                             /* disable ptp output */
> +                             gpio->gpio_cfg3 &= ~GPIO_CFG3_1588_OE_BIT_(bit);
> +                             lan743x_csr_write(adapter, GPIO_CFG3,
> +                                               gpio->gpio_cfg3);
> +                     }
> +                     /* release gpio output */
> +
> +                     /* disable gpio */
> +                     gpio->gpio_cfg1 |= GPIO_CFG1_GPIOEN_BIT_(bit);
> +                     gpio->gpio_cfg1 &= ~GPIO_CFG1_GPIOBUF_BIT_(bit);
> +                     lan743x_csr_write(adapter, GPIO_CFG1, gpio->gpio_cfg1);
> +
> +                     /* reset back to input */
> +                     gpio->gpio_cfg0 &= ~GPIO_CFG0_GPIO_DIR_BIT_(bit);
> +                     gpio->gpio_cfg0 &= ~GPIO_CFG0_GPIO_DATA_BIT_(bit);
> +                     lan743x_csr_write(adapter, GPIO_CFG0, gpio->gpio_cfg0);
> +             }
> +     }
> +     spin_unlock_irqrestore(&gpio->gpio_lock, irq_flags);
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +/* PTP */
> +#define LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB (31249999)
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptp_reserve_event_ch(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_release_event_ch(struct lan743x_adapter *adapter,
> +                                      int event_channel);
> +#endif
> +
> +static bool lan743x_ptp_is_enabled(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_enable(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_disable(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_reset(struct lan743x_adapter *adapter);
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_get(struct lan743x_adapter *adapter,
> +                               u32 *seconds, u32 *nano_seconds,
> +                               u32 *sub_nano_seconds);
> +static int lan743x_ptp_enable_pps(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_disable_pps(struct lan743x_adapter *adapter);
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_step(struct lan743x_adapter *adapter,
> +                                s64 time_step_ns);
> +#endif /* CONFIG_PTP_1588_CLOCK */

The constant ifdef CONFIG_PTP_1588_CLOCK is poor style and
unnecessary.  Just group this code together under one ifdef, or better
yet put it into its own file.

> +static void lan743x_ptp_clock_set(struct lan743x_adapter *adapter,
> +                               u32 seconds, u32 nano_seconds,
> +                               u32 sub_nano_seconds);
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_adjfreq(struct ptp_clock_info *ptpci, s32 delta_ppb)

Please implement adjfine().

 * @adjfine:  Adjusts the frequency of the hardware clock.
 *            parameter scaled_ppm: Desired frequency offset from
 *            nominal frequency in parts per million, but with a
 *            16 bit binary fractional field.
 *
 * @adjfreq:  Adjusts the frequency of the hardware clock.
 *            This method is deprecated.  New drivers should implement
 *            the @adjfine method instead.
 *            parameter delta: Desired frequency offset from nominal frequency
 *            in parts per billion

> +{
> +     struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> +                                            ptp_clock_info);
> +     struct lan743x_adapter *adapter = container_of(ptp,
> +                                                    struct lan743x_adapter,
> +                                                    ptp);

Coding style:
        struct lan743x_adapter *adapter =
                container_of(ptp, struct lan743x_adapter, ptp);

> +     u32 lan743x_rate_adj = 0;
> +     bool positive = true;
> +     u32 u32_delta = 0;
> +     u64 u64_delta = 0;
> +
> +     if ((delta_ppb < (-LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB)) ||
> +         delta_ppb > LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB) {
> +             return -EINVAL;
> +     }
> +     if (delta_ppb > 0) {
> +             u32_delta = (u32)delta_ppb;
> +             positive = true;
> +     } else {
> +             u32_delta = (u32)(-delta_ppb);
> +             positive = false;
> +     }
> +     u64_delta = (((u64)u32_delta) * 0x800000000ULL);
> +     lan743x_rate_adj = (u32)(u64_delta / 1000000000);

You need to use the div_u64() macro here.

> +
> +     if (positive)
> +             lan743x_rate_adj |= PTP_CLOCK_RATE_ADJ_DIR_;
> +
> +     lan743x_csr_write(adapter, PTP_CLOCK_RATE_ADJ,
> +                       lan743x_rate_adj);
> +
> +     netif_info(adapter, drv, adapter->netdev,
> +                "adjfreq, delta_ppb = %d, lan743x_rate_adj = 0x%08X\n",
> +                delta_ppb, lan743x_rate_adj);

This definitely should be at the debug level, or just delete it altogether.

> +     return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_adjtime(struct ptp_clock_info *ptpci, s64 delta)
> +{
> +     struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> +                                            ptp_clock_info);
> +     struct lan743x_adapter *adapter = container_of(ptp,
> +                                                    struct lan743x_adapter,
> +                                                    ptp);

Coding style.

> +     bool enable_pps = false;
> +
> +     if (ptp->pps_event_ch >= 0) {
> +             lan743x_ptp_disable_pps(adapter);
> +             enable_pps = true;
> +     }
> +
> +     lan743x_ptp_clock_step(adapter, delta);
> +     netif_info(adapter, drv, adapter->netdev,
> +                "adjtime, delta = %lld\n", delta);

Again, debug or delete.

> +
> +     if (enable_pps)
> +             lan743x_ptp_enable_pps(adapter);
> +
> +     return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_gettime64(struct ptp_clock_info *ptpci,
> +                                struct timespec64 *ts)
> +{
> +     struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> +                                            ptp_clock_info);
> +     struct lan743x_adapter *adapter = container_of(ptp,
> +                                                    struct lan743x_adapter,
> +                                                    ptp);

Style.

> +
> +     if (ts) {
> +             u32 seconds = 0;
> +             u32 nano_seconds = 0;

Please declare stack variables at the top of the function.

> +
> +             lan743x_ptp_clock_get(adapter, &seconds, &nano_seconds, NULL);
> +             ts->tv_sec = seconds;
> +             ts->tv_nsec = nano_seconds;
> +             netif_info(adapter, drv, adapter->netdev,
> +                        "gettime = %u.%09u\n", seconds, nano_seconds);

Debug/delete

> +     } else {
> +             netif_warn(adapter, drv, adapter->netdev, "ts == NULL\n");
> +             return -EINVAL;

No need to test for 'ts'. The caller must supply a valid pointer.

> +     }
> +     return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_settime64(struct ptp_clock_info *ptpci,
> +                                const struct timespec64 *ts)
> +{
> +     struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> +                                            ptp_clock_info);
> +     struct lan743x_adapter *adapter = container_of(ptp,
> +                                                    struct lan743x_adapter,
> +                                                    ptp);
> +     bool enable_pps = false;
> +
> +     if (ptp->pps_event_ch >= 0) {
> +             lan743x_ptp_disable_pps(adapter);
> +             enable_pps = true;
> +     }
> +
> +     if (ts) {
> +             u32 seconds = 0;
> +             u32 nano_seconds = 0;
> +
> +             if (ts->tv_sec > 0xFFFFFFFFLL ||
> +                 ts->tv_sec < 0) {

Actually seconds will exceed four bytes sooner than you think.  If
your HW has a restriction, then simply keep the seconds offset in SW
in the driver, adding it in where needed (like when reading the clock
or providing time stamps).

> +                     netif_warn(adapter, drv, adapter->netdev,
> +                                "ts->tv_sec out of range, %lld\n",
> +                                ts->tv_sec);
> +                     return -EINVAL;
> +             }
> +             if (ts->tv_nsec >= 1000000000L ||
> +                 ts->tv_nsec < 0) {
> +                     netif_warn(adapter, drv, adapter->netdev,
> +                                "ts->tv_nsec out of range, %ld\n",
> +                                ts->tv_nsec);
> +                     return -EINVAL;
> +             }
> +             seconds = ts->tv_sec;
> +             nano_seconds = ts->tv_nsec;
> +             netif_info(adapter, drv, adapter->netdev,
> +                        "settime = %u.%09u\n", seconds, nano_seconds);
> +             lan743x_ptp_clock_set(adapter, seconds, nano_seconds, 0);
> +     } else {
> +             netif_warn(adapter, drv, adapter->netdev, "ts == NULL\n");
> +             return -EINVAL;
> +     }
> +
> +     if (enable_pps)
> +             lan743x_ptp_enable_pps(adapter);
> +
> +     return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptp_enable_pps(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +     u32 current_seconds = 0;
> +     u32 target_seconds = 0;
> +     u32 general_config = 0;
> +     int result = -ENODEV;
> +     int pps_bit = 0;

So this function is really *not* implementing the PTP_CLK_REQ_PPS
feature but rather the PTP_CLK_REQ_PEROUT with a period of once per
second.

PTP_CLK_REQ_PPS means placing a PPS event into the kernel's "hardpps"
subsystem by calling ptp_clock_event().

I'm sorry this isn't really documented.  I should fix that.

If you HW can output arbitrary signals, then you should implement
PTP_CLK_REQ_PEROUT.  In any case, you shouldn't advertise the
ptp_clock_info.pps capability.

> +     if (ptp->pps_event_ch >= 0) {
> +             result = 0;
> +             goto done;
> +     }
> +
> +     ptp->pps_event_ch = lan743x_ptp_reserve_event_ch(adapter);
> +     if (ptp->pps_event_ch < 0) {
> +             netif_warn(adapter, drv, adapter->netdev,
> +                        "Failed to reserve event channel for PPS\n");
> +             goto done;
> +     }
> +
> +     switch(adapter->csr.id_rev & ID_REV_ID_MASK_) {
> +     case ID_REV_ID_LAN7430_:
> +             pps_bit = 2;/* GPIO 2 is preferred on EVB LAN7430 */
> +             break;
> +     case ID_REV_ID_LAN7431_:
> +             pps_bit = 4;/* GPIO 4 is preferred on EVB LAN7431 */
> +             break;
> +     }
> +
> +     ptp->pps_gpio_bit = lan743x_gpio_reserve_ptp_output(adapter, pps_bit,
> +                                                         ptp->pps_event_ch);
> +
> +     if (ptp->pps_gpio_bit < 0) {
> +             netif_warn(adapter, drv, adapter->netdev,
> +                        "Failed to reserve gpio 0 for PPS\n");
> +             goto done;
> +     }
> +
> +     lan743x_ptp_clock_get(adapter, &current_seconds, NULL, NULL);
> +
> +     /* set the first target ahead by 2 seconds
> +      *      to make sure its not missed
> +      */
> +     target_seconds = current_seconds + 2;
> +
> +     /* set the new target */
> +     lan743x_csr_write(adapter,
> +                       PTP_CLOCK_TARGET_SEC_X(ptp->pps_event_ch),
> +                       0xFFFF0000);
> +     lan743x_csr_write(adapter,
> +                       PTP_CLOCK_TARGET_NS_X(ptp->pps_event_ch), 0);
> +
> +     general_config = lan743x_csr_read(adapter, PTP_GENERAL_CONFIG);
> +
> +     general_config &= ~(PTP_GENERAL_CONFIG_CLOCK_EVENT_X_MASK_
> +                       (ptp->pps_event_ch));
> +     general_config |= PTP_GENERAL_CONFIG_CLOCK_EVENT_X_SET_
> +                       (ptp->pps_event_ch,
> +                        PTP_GENERAL_CONFIG_CLOCK_EVENT_100US_);
> +     general_config &= ~PTP_GENERAL_CONFIG_RELOAD_ADD_X_
> +                       (ptp->pps_event_ch);
> +     lan743x_csr_write(adapter, PTP_GENERAL_CONFIG, general_config);
> +
> +     /* set the reload to one second steps */
> +     lan743x_csr_write(adapter,
> +                       PTP_CLOCK_TARGET_RELOAD_SEC_X(ptp->pps_event_ch),
> +                       1);
> +     lan743x_csr_write(adapter,
> +                       PTP_CLOCK_TARGET_RELOAD_NS_X(ptp->pps_event_ch),
> +                       0);
> +
> +     /* set the new target */
> +     lan743x_csr_write(adapter,
> +                       PTP_CLOCK_TARGET_SEC_X(ptp->pps_event_ch),
> +                       target_seconds);
> +     lan743x_csr_write(adapter,
> +                       PTP_CLOCK_TARGET_NS_X(ptp->pps_event_ch),
> +                       0);
> +     return 0;
> +
> +done:
> +     if (ptp->pps_gpio_bit >= 0) {
> +             lan743x_gpio_release(adapter, ptp->pps_gpio_bit);
> +             ptp->pps_gpio_bit = -1;
> +     }
> +     if (ptp->pps_event_ch >= 0) {
> +             lan743x_ptp_release_event_ch(adapter,
> +                                          ptp->pps_event_ch);
> +             ptp->pps_event_ch = -1;
> +     }
> +     return result;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_disable_pps(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +
> +     if (ptp->pps_gpio_bit >= 0) {
> +             lan743x_gpio_release(adapter, ptp->pps_gpio_bit);
> +             ptp->pps_gpio_bit = -1;
> +     }
> +
> +     if (ptp->pps_event_ch >= 0) {
> +             u32 general_config = 0;
> +
> +             /* set target to far in the future, effectively disabling it */
> +             lan743x_csr_write(adapter,
> +                               PTP_CLOCK_TARGET_SEC_X(ptp->pps_event_ch),
> +                               0xFFFF0000);
> +             lan743x_csr_write(adapter,
> +                               PTP_CLOCK_TARGET_NS_X(ptp->pps_event_ch), 0);
> +
> +             general_config = lan743x_csr_read(adapter, PTP_GENERAL_CONFIG);
> +             general_config |= PTP_GENERAL_CONFIG_RELOAD_ADD_X_
> +                               (ptp->pps_event_ch);
> +             lan743x_csr_write(adapter, PTP_GENERAL_CONFIG, general_config);
> +             lan743x_ptp_release_event_ch(adapter, ptp->pps_event_ch);
> +             ptp->pps_event_ch = -1;
> +     }
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_enable(struct ptp_clock_info *ptpci,
> +                             struct ptp_clock_request *request, int on)
> +{
> +     struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> +                                            ptp_clock_info);
> +     struct lan743x_adapter *adapter = container_of(ptp,
> +                                                    struct lan743x_adapter,
> +                                                    ptp);
> +
> +     if (request) {
> +             switch (request->type) {
> +             case PTP_CLK_REQ_EXTTS:
> +                     return -EINVAL;
> +             case PTP_CLK_REQ_PEROUT:
> +                     return -EINVAL;
> +             case PTP_CLK_REQ_PPS:
> +                     if (on) {
> +                             if (lan743x_ptp_enable_pps(adapter) >= 0)
> +                                     netif_info(adapter, drv,
> +                                                adapter->netdev,
> +                                                "PPS is ON\n");
> +                             else
> +                                     netif_warn(adapter, drv,
> +                                                adapter->netdev,
> +                                                "Error starting PPS\n");
> +                     } else {
> +                             lan743x_ptp_disable_pps(adapter);
> +                             netif_info(adapter, drv, adapter->netdev,
> +                                        "PPS is OFF\n");
> +                     }
> +                     break;
> +             default:
> +                     netif_err(adapter, drv, adapter->netdev,
> +                               "request->type == %d, Unknown\n",
> +                               request->type);
> +                     break;
> +             }
> +     } else {
> +             netif_err(adapter, drv, adapter->netdev, "request == NULL\n");
> +     }
> +     return 0;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +void lan743x_ptp_isr(void *context)
> +{
> +     struct lan743x_adapter *adapter = (struct lan743x_adapter *)context;
> +     struct lan743x_ptp *ptp = NULL;
> +     int enable_flag = 1;
> +     u32 ptp_int_sts = 0;
> +
> +     ptp = &adapter->ptp;
> +
> +     lan743x_csr_write(adapter, INT_EN_CLR, INT_BIT_1588_);
> +
> +     ptp_int_sts = lan743x_csr_read(adapter, PTP_INT_STS);
> +     ptp_int_sts &= lan743x_csr_read(adapter, PTP_INT_EN_SET);
> +
> +     if (ptp_int_sts & PTP_INT_BIT_TX_TS_) {
> +             tasklet_schedule(&ptp->ptp_isr_bottom_half);

Please no new tasklets.  Instead use a work queue.  If you need lower
latency, consider using ptp_schedule_worker().

> +             enable_flag = 0;/* tasklet will re-enable later */
> +     }
> +     if (ptp_int_sts & PTP_INT_BIT_TX_SWTS_ERR_) {
> +             netif_err(adapter, drv, adapter->netdev,
> +                       "PTP TX Software Timestamp Error\n");
> +             /* clear int status bit */
> +             lan743x_csr_write(adapter, PTP_INT_STS,
> +                               PTP_INT_BIT_TX_SWTS_ERR_);
> +     }
> +     if (ptp_int_sts & PTP_INT_BIT_TIMER_B_) {
> +             netif_info(adapter, drv, adapter->netdev,
> +                        "PTP TIMER B Interrupt\n");

Don't print like this from an ISR.  Or is this an error, since you
don't enable this bit?

> +             /* clear int status bit */
> +             lan743x_csr_write(adapter, PTP_INT_STS,
> +                               PTP_INT_BIT_TIMER_B_);
> +     }
> +     if (ptp_int_sts & PTP_INT_BIT_TIMER_A_) {
> +             netif_info(adapter, drv, adapter->netdev,
> +                        "PTP TIMER A Interrupt\n");
> +             /* clear int status bit */
> +             lan743x_csr_write(adapter, PTP_INT_STS,
> +                               PTP_INT_BIT_TIMER_A_);
> +     }
> +
> +     if (enable_flag) {
> +             /* re-enable isr */
> +             lan743x_csr_write(adapter, INT_EN_SET, INT_BIT_1588_);
> +     }
> +}
> +
> +static void lan743x_ptp_tx_ts_complete(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +     int i;
> +     int c;

Put same types on one line:
        int c, i;

> +
> +     spin_lock_bh(&ptp->tx_ts_lock);
> +     c = ptp->tx_ts_skb_queue_size;
> +
> +     if (c > ptp->tx_ts_queue_size)
> +             c = ptp->tx_ts_queue_size;
> +     if (c <= 0)
> +             goto done;
> +
> +     for (i = 0; i < c; i++) {
> +             bool ignore_sync = ((ptp->tx_ts_ignore_sync_queue &
> +                                 BIT(i)) != 0);
> +             struct sk_buff *skb = ptp->tx_ts_skb_queue[i];
> +             u32 nseconds = ptp->tx_ts_nseconds_queue[i];
> +             u32 seconds = ptp->tx_ts_seconds_queue[i];
> +             u32 header = ptp->tx_ts_header_queue[i];
> +             struct skb_shared_hwtstamps tstamps;

Locals to top of function please.

> +             memset(&tstamps, 0, sizeof(tstamps));
> +             tstamps.hwtstamp = ktime_set(seconds, nseconds);
> +             if (!ignore_sync ||
> +                 ((header & PTP_TX_MSG_HEADER_MSG_TYPE_) !=
> +                 PTP_TX_MSG_HEADER_MSG_TYPE_SYNC_))
> +                     skb_tstamp_tx(skb, &tstamps);
> +
> +             dev_kfree_skb(skb);
> +
> +             ptp->tx_ts_skb_queue[i] = NULL;
> +             ptp->tx_ts_seconds_queue[i] = 0;
> +             ptp->tx_ts_nseconds_queue[i] = 0;
> +             ptp->tx_ts_header_queue[i] = 0;
> +     }
> +
> +     /* shift queue */
> +     ptp->tx_ts_ignore_sync_queue >>= c;
> +     for (i = c; i < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS; i++) {
> +             ptp->tx_ts_skb_queue[i - c] = ptp->tx_ts_skb_queue[i];
> +             ptp->tx_ts_seconds_queue[i - c] = ptp->tx_ts_seconds_queue[i];
> +             ptp->tx_ts_nseconds_queue[i - c] = ptp->tx_ts_nseconds_queue[i];
> +             ptp->tx_ts_header_queue[i - c] = ptp->tx_ts_header_queue[i];
> +
> +             ptp->tx_ts_skb_queue[i] = NULL;
> +             ptp->tx_ts_seconds_queue[i] = 0;
> +             ptp->tx_ts_nseconds_queue[i] = 0;
> +             ptp->tx_ts_header_queue[i] = 0;
> +     }
> +     ptp->tx_ts_skb_queue_size -= c;
> +     ptp->tx_ts_queue_size -= c;
> +done:
> +     ptp->pending_tx_timestamps -= c;
> +     spin_unlock_bh(&ptp->tx_ts_lock);
> +}
> +
> +static void lan743x_ptp_tx_ts_enqueue_skb(struct lan743x_adapter *adapter,
> +                                       struct sk_buff *skb, bool ignore_sync)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +
> +     spin_lock_bh(&ptp->tx_ts_lock);
> +     if (ptp->tx_ts_skb_queue_size < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS) {
> +             ptp->tx_ts_skb_queue[ptp->tx_ts_skb_queue_size] = skb;
> +             if (ignore_sync)
> +                     ptp->tx_ts_ignore_sync_queue |=
> +                             BIT(ptp->tx_ts_skb_queue_size);
> +             ptp->tx_ts_skb_queue_size++;
> +     } else {
> +             /* this should never happen, so long as the tx channel
> +              * calls and honors the result from
> +              * lan743x_ptp_request_tx_timestamp
> +              */
> +             netif_err(adapter, drv, adapter->netdev,
> +                       "tx ts skb queue overflow\n");
> +             dev_kfree_skb(skb);
> +     }
> +     spin_unlock_bh(&ptp->tx_ts_lock);
> +}
> +
> +static void lan743x_ptp_tx_ts_enqueue_ts(struct lan743x_adapter *adapter,
> +                                      u32 seconds, u32 nano_seconds,
> +                                      u32 header)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +
> +     spin_lock_bh(&ptp->tx_ts_lock);
> +     if (ptp->tx_ts_queue_size < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS) {
> +             ptp->tx_ts_seconds_queue[ptp->tx_ts_queue_size] = seconds;
> +             ptp->tx_ts_nseconds_queue[ptp->tx_ts_queue_size] = nano_seconds;
> +             ptp->tx_ts_header_queue[ptp->tx_ts_queue_size] = header;
> +             ptp->tx_ts_queue_size++;
> +     } else {
> +             netif_err(adapter, drv, adapter->netdev,
> +                       "tx ts queue overflow\n");
> +     }
> +     spin_unlock_bh(&ptp->tx_ts_lock);
> +}
> +
> +static void lan743x_ptp_isr_bottom_half(unsigned long param)
> +{
> +     struct lan743x_adapter *adapter = (struct lan743x_adapter *)param;
> +     bool new_timestamp_available = false;
> +
> +     while (lan743x_csr_read(adapter, PTP_INT_STS) & PTP_INT_BIT_TX_TS_) {

As a sanity check, you should break this loop using a counter.

> +             u32 cap_info = lan743x_csr_read(adapter, PTP_CAP_INFO);
> +
> +             if (PTP_CAP_INFO_TX_TS_CNT_GET_(cap_info) > 0) {
> +                     u32 seconds = lan743x_csr_read(adapter,
> +                             PTP_TX_EGRESS_SEC);
> +                     u32 nsec = lan743x_csr_read(adapter, PTP_TX_EGRESS_NS);
> +                     u32 cause = (nsec &
> +                                  PTP_TX_EGRESS_NS_CAPTURE_CAUSE_MASK_);
> +                     u32 header = lan743x_csr_read(adapter,
> +                                                   PTP_TX_MSG_HEADER);
> +
> +                     if (cause == PTP_TX_EGRESS_NS_CAPTURE_CAUSE_SW_) {
> +                             nsec &= PTP_TX_EGRESS_NS_TS_NS_MASK_;
> +                             lan743x_ptp_tx_ts_enqueue_ts(adapter,
> +                                                          seconds, nsec,
> +                                                          header);
> +                             new_timestamp_available = true;
> +                     } else if (cause ==
> +                             PTP_TX_EGRESS_NS_CAPTURE_CAUSE_AUTO_) {
> +                             netif_err(adapter, drv, adapter->netdev,
> +                                       "Auto capture cause not supported\n");
> +                     } else {
> +                             netif_warn(adapter, drv, adapter->netdev,
> +                                        "unknown tx timestamp capture 
> cause\n");
> +                     }
> +             } else {
> +                     netif_warn(adapter, drv, adapter->netdev,
> +                                "TX TS INT but no TX TS CNT\n");
> +             }
> +             lan743x_csr_write(adapter, PTP_INT_STS, PTP_INT_BIT_TX_TS_);
> +     }
> +
> +     if (new_timestamp_available)
> +             lan743x_ptp_tx_ts_complete(adapter);
> +
> +     lan743x_csr_write(adapter, INT_EN_SET, INT_BIT_1588_);
> +}
> +
> +static void lan743x_ptp_sync_to_system_clock(struct lan743x_adapter *adapter)
> +{
> +     struct timeval tv;
> +
> +     memset(&tv, 0, sizeof(tv));
> +     do_gettimeofday(&tv);

Use the TAI clock instead.

> +     lan743x_ptp_clock_set(adapter, tv.tv_sec, tv.tv_usec * 1000, 0);
> +}
> +
> +void lan743x_ptp_update_latency(struct lan743x_adapter *adapter,
> +                             u32 link_speed)
> +{
> +     switch (link_speed) {
> +     case 10:
> +             lan743x_csr_write(adapter, PTP_LATENCY,
> +                               PTP_LATENCY_TX_SET_(0) |
> +                               PTP_LATENCY_RX_SET_(0));
> +             break;
> +     case 100:
> +             lan743x_csr_write(adapter, PTP_LATENCY,
> +                               PTP_LATENCY_TX_SET_(181) |
> +                               PTP_LATENCY_RX_SET_(594));
> +             break;
> +     case 1000:
> +             lan743x_csr_write(adapter, PTP_LATENCY,
> +                               PTP_LATENCY_TX_SET_(30) |
> +                               PTP_LATENCY_RX_SET_(525));
> +             break;
> +     }
> +}
> +
> +int lan743x_ptp_init(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +
> +     mutex_init(&ptp->command_lock);
> +     spin_lock_init(&ptp->tx_ts_lock);
> +     tasklet_init(&ptp->ptp_isr_bottom_half,
> +                  lan743x_ptp_isr_bottom_half, (unsigned long)adapter);
> +     tasklet_disable(&ptp->ptp_isr_bottom_half);
> +     ptp->used_event_ch = 0;
> +     ptp->pps_event_ch = -1;
> +     ptp->pps_gpio_bit = -1;
> +     return 0;
> +}
> +
> +int lan743x_ptp_open(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +     int ret = -ENODEV;
> +     u32 temp;
> +
> +     lan743x_ptp_reset(adapter);
> +     lan743x_ptp_sync_to_system_clock(adapter);
> +     temp = lan743x_csr_read(adapter, PTP_TX_MOD2);
> +     temp |= PTP_TX_MOD2_TX_PTP_CLR_UDPV4_CHKSUM_;
> +     lan743x_csr_write(adapter, PTP_TX_MOD2, temp);
> +     lan743x_ptp_enable(adapter);
> +     tasklet_enable(&ptp->ptp_isr_bottom_half);
> +     lan743x_csr_write(adapter, INT_EN_SET, INT_BIT_1588_);
> +     lan743x_csr_write(adapter, PTP_INT_EN_SET,
> +                       PTP_INT_BIT_TX_SWTS_ERR_ | PTP_INT_BIT_TX_TS_);
> +     ptp->flags |= PTP_FLAG_ISR_ENABLED;
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +     snprintf(ptp->pin_config[0].name, 32, "lan743x_ptp_pin_0");
> +     ptp->pin_config[0].index = 0;
> +     ptp->pin_config[0].func = PTP_PF_PEROUT;
> +     ptp->pin_config[0].chan = 0;
> +
> +     ptp->ptp_clock_info.owner = THIS_MODULE;
> +     snprintf(ptp->ptp_clock_info.name, 16, "%pm",
> +              adapter->netdev->dev_addr);
> +     ptp->ptp_clock_info.max_adj = LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB;
> +     ptp->ptp_clock_info.n_alarm = 0;
> +     ptp->ptp_clock_info.n_ext_ts = 0;
> +     ptp->ptp_clock_info.n_per_out = 0;
> +     ptp->ptp_clock_info.n_pins = 0;
> +     ptp->ptp_clock_info.pps = 1;
> +     ptp->ptp_clock_info.pin_config = NULL;
> +     ptp->ptp_clock_info.adjfreq = lan743x_ptpci_adjfreq;
> +     ptp->ptp_clock_info.adjtime = lan743x_ptpci_adjtime;
> +     ptp->ptp_clock_info.gettime64 = lan743x_ptpci_gettime64;
> +     ptp->ptp_clock_info.getcrosststamp = NULL;
> +     ptp->ptp_clock_info.settime64 = lan743x_ptpci_settime64;
> +     ptp->ptp_clock_info.enable = lan743x_ptpci_enable;
> +     ptp->ptp_clock_info.verify = NULL;
> +
> +     ptp->ptp_clock = ptp_clock_register(&ptp->ptp_clock_info,
> +                                         &adapter->pdev->dev);
> +
> +     if (IS_ERR(ptp->ptp_clock)) {
> +             netif_err(adapter, ifup, adapter->netdev,
> +                       "ptp_clock_register failed\n");
> +             goto done;
> +     }
> +     ptp->flags |= PTP_FLAG_PTP_CLOCK_REGISTERED;
> +     netif_info(adapter, ifup, adapter->netdev,
> +                "successfully registered ptp clock\n");
> +#endif
> +
> +     return 0;
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +done:
> +     lan743x_ptp_close(adapter);
> +     return ret;
> +#endif
> +}
> +
> +void lan743x_ptp_close(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +     int index;
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +     if (ptp->flags & PTP_FLAG_PTP_CLOCK_REGISTERED) {
> +             ptp_clock_unregister(ptp->ptp_clock);
> +             ptp->ptp_clock = NULL;
> +             ptp->flags &= ~PTP_FLAG_PTP_CLOCK_REGISTERED;
> +             netif_info(adapter, drv, adapter->netdev,
> +                        "ptp clock unregister\n");
> +     }
> +#endif
> +
> +     if (ptp->flags & PTP_FLAG_ISR_ENABLED) {
> +             lan743x_csr_write(adapter, PTP_INT_EN_CLR,
> +                               PTP_INT_BIT_TX_SWTS_ERR_ |
> +                               PTP_INT_BIT_TX_TS_);
> +             lan743x_csr_write(adapter, INT_EN_CLR, INT_BIT_1588_);
> +             tasklet_disable(&ptp->ptp_isr_bottom_half);
> +             ptp->flags &= ~PTP_FLAG_ISR_ENABLED;
> +     }
> +
> +     /* clean up pending timestamp requests */
> +     lan743x_ptp_tx_ts_complete(adapter);
> +     spin_lock_bh(&ptp->tx_ts_lock);
> +     for (index = 0;
> +             index < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS;
> +             index++) {
> +             struct sk_buff *skb = ptp->tx_ts_skb_queue[index];
> +
> +             if (skb)
> +                     dev_kfree_skb(skb);
> +             ptp->tx_ts_skb_queue[index] = NULL;
> +             ptp->tx_ts_seconds_queue[index] = 0;
> +             ptp->tx_ts_nseconds_queue[index] = 0;
> +     }
> +     ptp->tx_ts_skb_queue_size = 0;
> +     ptp->tx_ts_queue_size = 0;
> +     ptp->pending_tx_timestamps = 0;
> +     spin_unlock_bh(&ptp->tx_ts_lock);
> +
> +     lan743x_ptp_disable(adapter);
> +}
> +
> +void lan743x_ptp_set_sync_ts_insert(struct lan743x_adapter *adapter,
> +                                 bool ts_insert_enable)
> +{
> +     u32 ptp_tx_mod = lan743x_csr_read(adapter, PTP_TX_MOD);
> +
> +     if (ts_insert_enable)
> +             ptp_tx_mod |= PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_;
> +     else
> +             ptp_tx_mod &= ~PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_;
> +
> +     lan743x_csr_write(adapter, PTP_TX_MOD, ptp_tx_mod);
> +}
> +
> +static bool lan743x_ptp_is_enabled(struct lan743x_adapter *adapter)
> +{
> +     if (lan743x_csr_read(adapter, PTP_CMD_CTL) & PTP_CMD_CTL_PTP_ENABLE_)
> +             return true;
> +     return false;
> +}
> +
> +static void lan743x_ptp_wait_till_cmd_done(struct lan743x_adapter *adapter,
> +                                        u32 bit_mask)
> +{
> +     int timeout = 1000;
> +     u32 data = 0;
> +
> +     while (timeout &&
> +            (data = (lan743x_csr_read(adapter, PTP_CMD_CTL) &
> +            bit_mask))) {
> +             usleep_range(1000, 20000);
> +             timeout--;
> +     }
> +     if (data) {
> +             netif_err(adapter, drv, adapter->netdev,
> +                       "timeout waiting for cmd to be done, cmd = 0x%08X\n",
> +                       bit_mask);
> +     }
> +}
> +
> +static void lan743x_ptp_enable(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +
> +     mutex_lock(&ptp->command_lock);
> +
> +     if (lan743x_ptp_is_enabled(adapter)) {
> +             netif_warn(adapter, drv, adapter->netdev,
> +                        "PTP already enabled\n");
> +             goto done;
> +     }
> +     lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_ENABLE_);
> +done:
> +     mutex_unlock(&ptp->command_lock);
> +}
> +
> +static void lan743x_ptp_disable(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +
> +     mutex_lock(&ptp->command_lock);
> +     if (!lan743x_ptp_is_enabled(adapter)) {
> +             netif_warn(adapter, drv, adapter->netdev,
> +                        "PTP already disabled\n");
> +             goto done;
> +     }
> +     lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_DISABLE_);
> +     lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_ENABLE_);
> +done:
> +     mutex_unlock(&ptp->command_lock);
> +}
> +
> +static void lan743x_ptp_reset(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +
> +     mutex_lock(&ptp->command_lock);
> +
> +     if (lan743x_ptp_is_enabled(adapter)) {
> +             netif_err(adapter, drv, adapter->netdev,
> +                       "Attempting reset while enabled\n");
> +             goto done;
> +     }
> +
> +     lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_RESET_);
> +     lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_RESET_);
> +done:
> +     mutex_unlock(&ptp->command_lock);
> +}
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptp_reserve_event_ch(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +     int result = -ENODEV;
> +     int index = 0;
> +
> +     mutex_lock(&ptp->command_lock);
> +     for (index = 0; index < LAN743X_PTP_NUMBER_OF_EVENT_CHANNELS; index++) {
> +             if (!(test_bit(index, &ptp->used_event_ch))) {
> +                     ptp->used_event_ch |= BIT(index);
> +                     result = index;
> +                     break;
> +             }
> +     }
> +     mutex_unlock(&ptp->command_lock);
> +     return result;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_release_event_ch(struct lan743x_adapter *adapter,
> +                                      int event_channel)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +
> +     mutex_lock(&ptp->command_lock);
> +     if (test_bit(event_channel, &ptp->used_event_ch)) {
> +             ptp->used_event_ch &= ~BIT(event_channel);
> +     } else {
> +             netif_warn(adapter, drv, adapter->netdev,
> +                        "attempted release on a not used event_channel = 
> %d\n",
> +                        event_channel);
> +     }
> +     mutex_unlock(&ptp->command_lock);
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_get(struct lan743x_adapter *adapter,
> +                               u32 *seconds, u32 *nano_seconds,
> +                               u32 *sub_nano_seconds)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +
> +     mutex_lock(&ptp->command_lock);
> +
> +     lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_CLOCK_READ_);
> +     lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_CLOCK_READ_);
> +
> +     if (seconds)
> +             (*seconds) = lan743x_csr_read(adapter, PTP_CLOCK_SEC);
> +
> +     if (nano_seconds)
> +             (*nano_seconds) = lan743x_csr_read(adapter, PTP_CLOCK_NS);
> +
> +     if (sub_nano_seconds)
> +             (*sub_nano_seconds) =
> +             lan743x_csr_read(adapter, PTP_CLOCK_SUBNS);
> +
> +     mutex_unlock(&ptp->command_lock);
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +static void lan743x_ptp_clock_set(struct lan743x_adapter *adapter,
> +                               u32 seconds, u32 nano_seconds,
> +                               u32 sub_nano_seconds)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +
> +     mutex_lock(&ptp->command_lock);
> +
> +     lan743x_csr_write(adapter, PTP_CLOCK_SEC, seconds);
> +     lan743x_csr_write(adapter, PTP_CLOCK_NS, nano_seconds);
> +     lan743x_csr_write(adapter, PTP_CLOCK_SUBNS, sub_nano_seconds);
> +
> +     lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_CLOCK_LOAD_);
> +     lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_CLOCK_LOAD_);
> +     mutex_unlock(&ptp->command_lock);
> +}
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_step(struct lan743x_adapter *adapter,
> +                                s64 time_step_ns)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +     u64 abs_time_step_ns = 0;
> +     u32 nano_seconds = 0;
> +     s32 seconds = 0;
> +
> +     if (time_step_ns >  15000000000LL) {
> +             /* convert to clock set */
> +             u32 nano_seconds = 0;
> +             u32 seconds = 0;
> +
> +             lan743x_ptp_clock_get(adapter, &seconds, &nano_seconds, NULL);
> +             seconds += (time_step_ns / 1000000000LL);

Use the macro.

> +             nano_seconds += (time_step_ns % 1000000000LL);

Actually, use div_u64_rem() to avoid the % operator.

> +             if (nano_seconds >= 1000000000) {

How can this test be true?

> +                     seconds++;
> +                     nano_seconds -= 1000000000;
> +             }
> +             lan743x_ptp_clock_set(adapter, seconds, nano_seconds, 0);
> +             return;
> +     } else if (time_step_ns < -15000000000LL) {
> +             /* convert to clock set */
> +             u32 nano_seconds_step = 0;
> +             u32 nano_seconds = 0;
> +             u32 seconds = 0;

Ugh.  Now you have these defined twice.  Move them to the top, please.

> +             time_step_ns = -time_step_ns;
> +
> +             lan743x_ptp_clock_get(adapter, &seconds, &nano_seconds, NULL);
> +             seconds -= (time_step_ns / 1000000000LL);
> +             nano_seconds_step = (time_step_ns % 1000000000LL);

Use division macro for 64 bit.

> +             if (nano_seconds < nano_seconds_step) {
> +                     seconds--;
> +                     nano_seconds += 1000000000;
> +             }
> +             nano_seconds -= nano_seconds_step;
> +             lan743x_ptp_clock_set(adapter, seconds, nano_seconds, 0);
> +             return;
> +     }
> +
> +     /* do clock step */
> +
> +     if (time_step_ns >= 0) {
> +             abs_time_step_ns = (u64)(time_step_ns);
> +             seconds = (s32)(abs_time_step_ns / 1000000000);
> +             nano_seconds = (u32)(abs_time_step_ns % 1000000000);
> +     } else {
> +             abs_time_step_ns = (u64)(-time_step_ns);
> +             seconds = -((s32)(abs_time_step_ns / 1000000000));
> +             nano_seconds = (u32)(abs_time_step_ns % 1000000000);
> +             if (nano_seconds > 0) {
> +                     /* subtracting nano seconds is not allowed
> +                      * convert to subtracting from seconds,
> +                      * and adding to nanoseconds
> +                      */
> +                     seconds--;
> +                     nano_seconds = (1000000000 - nano_seconds);
> +             }
> +     }
> +
> +     if (nano_seconds > 0) {
> +             /* add 8 ns to cover the likely normal increment */
> +             nano_seconds += 8;
> +     }
> +
> +     if (nano_seconds >= 1000000000) {
> +             /* carry into seconds */
> +             seconds++;
> +             nano_seconds -= 1000000000;
> +     }
> +
> +     while (seconds) {
> +             mutex_lock(&ptp->command_lock);
> +             if (seconds > 0) {
> +                     u32 adjustment_value = (u32)seconds;
> +
> +                     if (adjustment_value > 0xF)
> +                             adjustment_value = 0xF;
> +                     lan743x_csr_write(adapter, PTP_CLOCK_STEP_ADJ,
> +                                       PTP_CLOCK_STEP_ADJ_DIR_ |
> +                                       adjustment_value);
> +                     seconds -= ((s32)adjustment_value);
> +             } else {
> +                     u32 adjustment_value = (u32)(-seconds);
> +
> +                     if (adjustment_value > 0xF)
> +                             adjustment_value = 0xF;
> +                     lan743x_csr_write(adapter, PTP_CLOCK_STEP_ADJ,
> +                                       adjustment_value);
> +                     seconds += ((s32)adjustment_value);
> +             }
> +             lan743x_csr_write(adapter, PTP_CMD_CTL,
> +                               PTP_CMD_CTL_PTP_CLOCK_STEP_SEC_);
> +             lan743x_ptp_wait_till_cmd_done(adapter,
> +                                            PTP_CMD_CTL_PTP_CLOCK_STEP_SEC_);
> +             mutex_unlock(&ptp->command_lock);
> +     }
> +     if (nano_seconds) {
> +             mutex_lock(&ptp->command_lock);
> +             lan743x_csr_write(adapter, PTP_CLOCK_STEP_ADJ,
> +                               PTP_CLOCK_STEP_ADJ_DIR_ |
> +                               (nano_seconds &
> +                               PTP_CLOCK_STEP_ADJ_VALUE_MASK_));
> +             lan743x_csr_write(adapter, PTP_CMD_CTL,
> +                               PTP_CMD_CTL_PTP_CLK_STP_NSEC_);
> +             lan743x_ptp_wait_till_cmd_done(adapter,
> +                                            PTP_CMD_CTL_PTP_CLK_STP_NSEC_);
> +             mutex_unlock(&ptp->command_lock);
> +     }
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +bool lan743x_ptp_request_tx_timestamp(struct lan743x_adapter *adapter)
> +{
> +     struct lan743x_ptp *ptp = &adapter->ptp;
> +     bool result = false;
> +
> +     spin_lock_bh(&ptp->tx_ts_lock);
> +     if (ptp->pending_tx_timestamps < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS) {
> +             ptp->pending_tx_timestamps++;
> +             result = true;/* request granted */

Avoid tail comments please.

> +     }
> +     spin_unlock_bh(&ptp->tx_ts_lock);
> +     return result;
> +}

Thanks,
Richard

Reply via email to