[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180713033135.dfatbghlpeswm7g6@localhost>
Date: Thu, 12 Jul 2018 20:31:35 -0700
From: Richard Cochran <richardcochran@...il.com>
To: Bryan Whitehead <Bryan.Whitehead@...rochip.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org,
UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH v2 net-next 9/9] lan743x: Add PTP support
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, ¤t_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
Powered by blists - more mailing lists