Message ID | 1496413690-22826-1-git-send-email-rafalo@cadence.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Fri, Jun 02, 2017 at 03:28:10PM +0100, Rafal Ozieblo wrote: > +int gem_get_hwtst(struct net_device *dev, struct ifreq *rq) > +{ > + struct macb *bp = netdev_priv(dev); > + struct hwtstamp_config *tstamp_config = &bp->tstamp_config; > + > + if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0) > + return -EFAULT; Segmentation fault? Really? ... > +int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd) > +{ > + struct macb *bp = netdev_priv(dev); > + struct hwtstamp_config *tstamp_config = &bp->tstamp_config; > + enum macb_bd_control tx_bd_control = TSTAMP_DISABLED; > + enum macb_bd_control rx_bd_control = TSTAMP_DISABLED; > + u32 regval; > + > + if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0) > + return -EFAULT; here again Thanks, Richard
> From: Richard Cochran [mailto:richardcochran@gmail.com] > Sent: 4 czerwca 2017 22:55 > To: Rafal Ozieblo <rafalo@cadence.com> > Cc: David Miller <davem@davemloft.net>; nicolas.ferre@atmel.com; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > harini.katakam@xilinx.com; andrei.pistirica@microchip.com > Subject: Re: [PATCH v2 4/4] net: macb: Add hardware PTP support > > On Fri, Jun 02, 2017 at 03:28:10PM +0100, Rafal Ozieblo wrote: > > > +int gem_get_hwtst(struct net_device *dev, struct ifreq *rq) > > +{ > > + struct macb *bp = netdev_priv(dev); > > + struct hwtstamp_config *tstamp_config = &bp->tstamp_config; > > + > > + if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0) > > + return -EFAULT; > > Segmentation fault? Really? > > ... > > > +int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd) > > +{ > > + struct macb *bp = netdev_priv(dev); > > + struct hwtstamp_config *tstamp_config = &bp->tstamp_config; > > + enum macb_bd_control tx_bd_control = TSTAMP_DISABLED; > > + enum macb_bd_control rx_bd_control = TSTAMP_DISABLED; > > + u32 regval; > > + > > + if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0) > > + return -EFAULT; > > here again Would "ENOTSUP" be sufficient ? > > Thanks, > Richard Regards, Rafal
On Fri, Jun 02, 2017 at 03:28:10PM +0100, Rafal Ozieblo wrote: > +static s32 gem_get_ptp_max_adj(void) > +{ > + return 64E6; > +} This is a floating point constant. Please use integer instead. > + > +static int gem_get_ts_info(struct net_device *dev, > + struct ethtool_ts_info *info) > +{ > + struct macb *bp = netdev_priv(dev); > + > + ethtool_op_get_ts_info(dev, info); This default is misguided. > + if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0) > + return 0; Try this: if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0) { ethtool_op_get_ts_info(dev, info); return 0; } > + 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; > + info->tx_types = > + (1 << HWTSTAMP_TX_ONESTEP_SYNC) | > + (1 << HWTSTAMP_TX_OFF) | > + (1 << HWTSTAMP_TX_ON); > + info->rx_filters = > + (1 << HWTSTAMP_FILTER_NONE) | > + (1 << HWTSTAMP_FILTER_ALL); > + info->phc_index = -1; > + > + if (bp->ptp_clock) > + info->phc_index = ptp_clock_index(bp->ptp_clock); Like this please: info->phc_index = bp->ptp_clock ? ptp_clock_index(bp->ptp_clock) : -1; > + > + return 0; > +} > + > +static struct macb_ptp_info gem_ptp_info = { > + .ptp_init = gem_ptp_init, > + .ptp_remove = gem_ptp_remove, > + .get_ptp_max_adj = gem_get_ptp_max_adj, > + .get_tsu_rate = gem_get_tsu_rate, > + .get_ts_info = gem_get_ts_info, > + .get_hwtst = gem_get_hwtst, > + .set_hwtst = gem_set_hwtst, > +}; > +#endif > + > static int macb_get_ts_info(struct net_device *netdev, > struct ethtool_ts_info *info) > { > @@ -2636,12 +2707,16 @@ static void macb_configure_caps(struct macb *bp, > dcfg = gem_readl(bp, DCFG2); > if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0) > bp->caps |= MACB_CAPS_FIFO_MODE; > - if (IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) && gem_has_ptp(bp)) { > +#ifdef CONFIG_MACB_USE_HWSTAMP > + if (gem_has_ptp(bp)) { > if (!GEM_BFEXT(TSU, gem_readl(bp, DCFG5))) > pr_err("GEM doesn't support hardware ptp.\n"); > - else > + else { > bp->hw_dma_cap |= HW_DMA_CAP_PTP; > + bp->ptp_info = &gem_ptp_info; > + } > } > +#endif > } > > dev_dbg(&bp->pdev->dev, "Cadence caps 0x%08x\n", bp->caps); > @@ -3247,7 +3322,9 @@ static const struct macb_config np4_config = { > }; > > static const struct macb_config zynqmp_config = { > - .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO, > + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | > + MACB_CAPS_JUMBO | > + MACB_CAPS_GEM_HAS_PTP, > .dma_burst_length = 16, > .clk_init = macb_clk_init, > .init = macb_init, > @@ -3281,7 +3358,9 @@ MODULE_DEVICE_TABLE(of, macb_dt_ids); > #endif /* CONFIG_OF */ > > static const struct macb_config default_gem_config = { > - .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO, > + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | > + MACB_CAPS_JUMBO | > + MACB_CAPS_GEM_HAS_PTP, > .dma_burst_length = 16, > .clk_init = macb_clk_init, > .init = macb_init, > diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c > new file mode 100755 > index 0000000..d536970 > --- /dev/null > +++ b/drivers/net/ethernet/cadence/macb_ptp.c > @@ -0,0 +1,512 @@ > +/** > + * 1588 PTP support for Cadence GEM device. > + * > + * Copyright (C) 2017 Cadence Design Systems - http://www.cadence.com > + * > + * Authors: Rafal Ozieblo <rafalo@cadence.com> > + * Bartosz Folta <bfolta@cadence.com> > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 of > + * the License as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > +#include <linux/kernel.h> > +#include <linux/types.h> > +#include <linux/clk.h> > +#include <linux/device.h> > +#include <linux/etherdevice.h> > +#include <linux/platform_device.h> > +#include <linux/time64.h> > +#include <linux/ptp_classify.h> > +#include <linux/if_ether.h> > +#include <linux/if_vlan.h> > +#include <linux/net_tstamp.h> > +#include <linux/circ_buf.h> > +#include <linux/spinlock.h> > + > +#include "macb.h" > + > +#define GEM_PTP_TIMER_NAME "gem-ptp-timer" > + > +static struct macb_dma_desc_ptp *macb_ptp_desc(struct macb *bp, > + struct macb_dma_desc *desc) > +{ > + if (bp->hw_dma_cap == HW_DMA_CAP_PTP) > + return (struct macb_dma_desc_ptp *) > + ((u8 *)desc + sizeof(struct macb_dma_desc)); > + if (bp->hw_dma_cap == HW_DMA_CAP_64B_PTP) > + return (struct macb_dma_desc_ptp *) > + ((u8 *)desc + sizeof(struct macb_dma_desc) > + + sizeof(struct macb_dma_desc_64)); > + return NULL; > +} > + > +static int gem_tsu_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts) > +{ > + long first, second; > + u32 secl, sech; > + unsigned long flags; > + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info); Please list locals in "upside down Christmas tree" style: struct macb *bp = container_of(ptp, struct macb, ptp_clock_info); unsigned long flags; long first, second; u32 secl, sech; > + spin_lock_irqsave(&bp->tsu_clk_lock, flags); > + first = gem_readl(bp, TN); > + secl = gem_readl(bp, TSL); > + sech = gem_readl(bp, TSH); > + second = gem_readl(bp, TN); > + > + /* test for nsec rollover */ > + if (first > second) { > + /* if so, use later read & re-read seconds > + * (assume all done within 1s) > + */ > + ts->tv_nsec = gem_readl(bp, TN); > + secl = gem_readl(bp, TSL); > + sech = gem_readl(bp, TSH); > + } else { > + ts->tv_nsec = first; > + } > + > + spin_unlock_irqrestore(&bp->tsu_clk_lock, flags); > + ts->tv_sec = (((u64)sech << GEM_TSL_SIZE) | secl) > + & TSU_SEC_MAX_VAL; > + return 0; > +} > + > +static int gem_tsu_set_time(struct ptp_clock_info *ptp, > + const struct timespec64 *ts) > +{ > + u32 ns, sech, secl; > + unsigned long flags; > + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info); here too ... > + secl = (u32)ts->tv_sec; > + sech = (ts->tv_sec >> GEM_TSL_SIZE) & ((1 << GEM_TSH_SIZE) - 1); > + ns = ts->tv_nsec; > + > + spin_lock_irqsave(&bp->tsu_clk_lock, flags); > + > + /* TSH doesn't latch the time and no atomicity! */ > + gem_writel(bp, TN, 0); /* clear to avoid overflow */ > + gem_writel(bp, TSH, sech); > + /* write lower bits 2nd, for synchronized secs update */ > + gem_writel(bp, TSL, secl); > + gem_writel(bp, TN, ns); > + > + spin_unlock_irqrestore(&bp->tsu_clk_lock, flags); > + > + return 0; > +} > + > +static int gem_tsu_incr_set(struct macb *bp, struct tsu_incr *incr_spec) > +{ > + unsigned long flags; > + > + /* tsu_timer_incr register must be written after > + * the tsu_timer_incr_sub_ns register and the write operation > + * will cause the value written to the tsu_timer_incr_sub_ns register > + * to take effect. > + */ > + spin_lock_irqsave(&bp->tsu_clk_lock, flags); > + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, incr_spec->sub_ns)); > + gem_writel(bp, TI, GEM_BF(NSINCR, incr_spec->ns)); > + spin_unlock_irqrestore(&bp->tsu_clk_lock, flags); > + > + return 0; > +} > + > +static int gem_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) > +{ > + struct tsu_incr incr_spec; > + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info); > + u64 adj; > + u32 word; > + bool neg_adj = false; and here ... > + > + if (scaled_ppm < 0) { > + neg_adj = true; > + scaled_ppm = -scaled_ppm; > + } > + > + /* Adjustment is relative to base frequency */ > + incr_spec.sub_ns = bp->tsu_incr.sub_ns; > + incr_spec.ns = bp->tsu_incr.ns; > + > + /* scaling: unused(8bit) | ns(8bit) | fractions(16bit) */ > + word = ((u64)incr_spec.ns << GEM_SUBNSINCR_SIZE) + incr_spec.sub_ns; > + adj = (u64)scaled_ppm * word; > + /* Divide with rounding, equivalent to floating dividing: > + * (temp / USEC_PER_SEC) + 0.5 > + */ > + adj += (USEC_PER_SEC >> 1); > + adj >>= GEM_SUBNSINCR_SIZE; /* remove fractions */ > + adj = div_u64(adj, USEC_PER_SEC); > + adj = neg_adj ? (word - adj) : (word + adj); > + > + incr_spec.ns = (adj >> GEM_SUBNSINCR_SIZE) > + & ((1 << GEM_NSINCR_SIZE) - 1); > + incr_spec.sub_ns = adj & ((1 << GEM_SUBNSINCR_SIZE) - 1); > + gem_tsu_incr_set(bp, &incr_spec); > + return 0; > +} > + > +static int gem_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta) > +{ > + struct timespec64 now, then = ns_to_timespec64(delta); > + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info); > + u32 adj, sign = 0; and here too. Please check the other functions... > + if (delta < 0) { > + sign = 1; > + delta = -delta; > + } > + > + if (delta > TSU_NSEC_MAX_VAL) { > + gem_tsu_get_time(&bp->ptp_clock_info, &now); > + if (sign) > + now = timespec64_sub(now, then); > + else > + now = timespec64_add(now, then); > + > + gem_tsu_set_time(&bp->ptp_clock_info, > + (const struct timespec64 *)&now); > + } else { > + adj = (sign << GEM_ADDSUB_OFFSET) | delta; > + > + gem_writel(bp, TA, adj); > + } > + > + return 0; > +} > + > +static int gem_ptp_enable(struct ptp_clock_info *ptp, > + struct ptp_clock_request *rq, int on) > +{ > + return -EOPNOTSUPP; > +} > + > +static struct ptp_clock_info gem_ptp_caps_template = { > + .owner = THIS_MODULE, > + .name = GEM_PTP_TIMER_NAME, > + .max_adj = 0, > + .n_alarm = 0, > + .n_ext_ts = 0, > + .n_per_out = 0, > + .n_pins = 0, > + .pps = 1, > + .adjfine = gem_ptp_adjfine, > + .adjtime = gem_ptp_adjtime, > + .gettime64 = gem_tsu_get_time, > + .settime64 = gem_tsu_set_time, > + .enable = gem_ptp_enable, > +}; > + > +static void gem_ptp_init_timer(struct macb *bp) > +{ > + u32 rem = 0; > + > + bp->tsu_incr.ns = div_u64_rem(NSEC_PER_SEC, bp->tsu_rate, &rem); > + if (rem) { > + u64 adj = rem; This variable belongs above, not in the if-block. > + adj <<= GEM_SUBNSINCR_SIZE; > + bp->tsu_incr.sub_ns = div_u64(adj, bp->tsu_rate); > + } else { > + bp->tsu_incr.sub_ns = 0; > + } > +} > + > +static void gem_ptp_init_tsu(struct macb *bp) > +{ > + struct timespec64 ts; > + > + /* 1. get current system time */ > + ts = ns_to_timespec64(ktime_to_ns(ktime_get_real())); > + > + /* 2. set ptp timer */ > + gem_tsu_set_time(&bp->ptp_clock_info, &ts); > + > + /* 3. set PTP timer increment value to BASE_INCREMENT */ > + gem_tsu_incr_set(bp, &bp->tsu_incr); > + > + gem_writel(bp, TA, 0); > +} > + > +static void gem_ptp_clear_timer(struct macb *bp) > +{ > + bp->tsu_incr.ns = 0; > + bp->tsu_incr.sub_ns = 0; What is the point of this function? > + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, 0)); > + gem_writel(bp, TI, GEM_BF(NSINCR, 0)); > + gem_writel(bp, TA, 0); > +} > + > +static int gem_hw_timestamp(struct macb *bp, u32 dma_desc_ts_1, > + u32 dma_desc_ts_2, struct timespec64 *ts) > +{ > + struct timespec64 tsu; > + > + ts->tv_sec = (GEM_BFEXT(DMA_SECH, dma_desc_ts_2) << GEM_DMA_SECL_SIZE) | > + GEM_BFEXT(DMA_SECL, dma_desc_ts_1); > + ts->tv_nsec = GEM_BFEXT(DMA_NSEC, dma_desc_ts_1); > + > + /* TSU overlapping workaround > + * The timestamp only contains lower few bits of seconds, > + * so add value from 1588 timer > + */ > + gem_tsu_get_time(&bp->ptp_clock_info, &tsu); > + > + /* If the top bit is set in the timestamp, > + * but not in 1588 timer, it has rolled over, > + * so subtract max size > + */ > + if ((ts->tv_sec & (GEM_DMA_SEC_TOP >> 1)) && > + !(tsu.tv_sec & (GEM_DMA_SEC_TOP >> 1))) > + ts->tv_sec -= GEM_DMA_SEC_TOP; > + > + ts->tv_sec += ((~GEM_DMA_SEC_MASK) & tsu.tv_sec); > + > + return 0; > +} > + > +void gem_ptp_rxstamp(struct macb *bp, struct sk_buff *skb, > + struct macb_dma_desc *desc) > +{ > + struct timespec64 ts; > + struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); > + struct macb_dma_desc_ptp *desc_ptp; > + > + if (GEM_BFEXT(DMA_RXVALID, desc->addr)) { > + desc_ptp = macb_ptp_desc(bp, desc); > + gem_hw_timestamp(bp, desc_ptp->ts_1, desc_ptp->ts_2, &ts); > + memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps)); > + shhwtstamps->hwtstamp = ktime_set(ts.tv_sec, ts.tv_nsec); > + } > +} > + > +static void gem_tstamp_tx(struct macb *bp, struct sk_buff *skb, > + struct macb_dma_desc_ptp *desc_ptp) > +{ > + struct skb_shared_hwtstamps shhwtstamps; > + struct timespec64 ts; > + > + gem_hw_timestamp(bp, desc_ptp->ts_1, desc_ptp->ts_2, &ts); > + memset(&shhwtstamps, 0, sizeof(shhwtstamps)); > + shhwtstamps.hwtstamp = ktime_set(ts.tv_sec, ts.tv_nsec); > + skb_tstamp_tx(skb, &shhwtstamps); > +} > + > +int gem_ptp_txstamp(struct macb_queue *queue, struct sk_buff *skb, > + struct macb_dma_desc *desc) > +{ > + struct gem_tx_ts *tx_timestamp; > + struct macb_dma_desc_ptp *desc_ptp; > + unsigned long head = queue->tx_ts_head; > + unsigned long tail = READ_ONCE(queue->tx_ts_tail); > + > + if (!GEM_BFEXT(DMA_TXVALID, desc->ctrl)) > + return -EINVAL; > + > + if (CIRC_SPACE(head, tail, PTP_TS_BUFFER_SIZE) == 0) > + return -ENOMEM; > + > + skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; > + desc_ptp = macb_ptp_desc(queue->bp, desc); > + tx_timestamp = &queue->tx_timestamps[head]; > + tx_timestamp->skb = skb; > + tx_timestamp->desc_ptp.ts_1 = desc_ptp->ts_1; > + tx_timestamp->desc_ptp.ts_2 = desc_ptp->ts_2; > + /* move head */ > + smp_store_release(&queue->tx_ts_head, > + (head + 1) & (PTP_TS_BUFFER_SIZE - 1)); > + > + schedule_work(&queue->tx_ts_task); Since the time stamp is in the buffer descriptor, why delay the delivery via the work item? > + return 0; > +} > + > +static void gem_tx_timestamp_flush(struct work_struct *work) > +{ > + struct macb_queue *queue = > + container_of(work, struct macb_queue, tx_ts_task); > + struct gem_tx_ts *tx_ts; > + unsigned long head, tail; > + > + /* take current head */ > + head = smp_load_acquire(&queue->tx_ts_head); > + tail = queue->tx_ts_tail; > + > + while (CIRC_CNT(head, tail, PTP_TS_BUFFER_SIZE)) { > + tx_ts = &queue->tx_timestamps[tail]; > + gem_tstamp_tx(queue->bp, tx_ts->skb, &tx_ts->desc_ptp); > + /* cleanup */ > + dev_kfree_skb_any(tx_ts->skb); > + /* remove old tail */ > + smp_store_release(&queue->tx_ts_tail, > + (tail + 1) & (PTP_TS_BUFFER_SIZE - 1)); > + tail = queue->tx_ts_tail; > + } > +} > + > +void gem_ptp_init(struct net_device *dev) > +{ > + struct macb *bp = netdev_priv(dev); > + unsigned int q; > + struct macb_queue *queue; > + > + bp->ptp_clock_info = gem_ptp_caps_template; > + > + /* nominal frequency and maximum adjustment in ppb */ > + bp->tsu_rate = bp->ptp_info->get_tsu_rate(bp); > + bp->ptp_clock_info.max_adj = bp->ptp_info->get_ptp_max_adj(); > + gem_ptp_init_timer(bp); > + bp->ptp_clock = ptp_clock_register(&bp->ptp_clock_info, &dev->dev); > + if (IS_ERR(&bp->ptp_clock)) { > + bp->ptp_clock = NULL; > + pr_err("ptp clock register failed\n"); > + return; > + } ptp_clock_register() can also return NULL, and so you can avoid the following code in that case, right? > + > + spin_lock_init(&bp->tsu_clk_lock); > + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) { > + queue->tx_ts_head = 0; > + queue->tx_ts_tail = 0; > + INIT_WORK(&queue->tx_ts_task, gem_tx_timestamp_flush); > + } > + > + gem_ptp_init_tsu(bp); > + > + dev_info(&bp->pdev->dev, "%s ptp clock registered.\n", > + GEM_PTP_TIMER_NAME); > +} > + > +void gem_ptp_remove(struct net_device *ndev) > +{ > + struct macb *bp = netdev_priv(ndev); > + > + if (bp->ptp_clock) > + ptp_clock_unregister(bp->ptp_clock); > + > + gem_ptp_clear_timer(bp); Why is this 'clear' needed? > + dev_info(&bp->pdev->dev, "%s ptp clock unregistered.\n", > + GEM_PTP_TIMER_NAME); > +} Thanks, Richard
On Tue, Jun 06, 2017 at 08:54:50AM +0000, Rafal Ozieblo wrote:
> Would "ENOTSUP" be sufficient ?
You mean EOPNOTSUPP? Yes, sounds ok to me. EFAULT is surely wrong.
Thanks,
Richard
> From: Richard Cochran [mailto:richardcochran@gmail.com] > Sent: 6 czerwca 2017 20:34 > To: Rafal Ozieblo <rafalo@cadence.com> > Cc: David Miller <davem@davemloft.net>; nicolas.ferre@atmel.com; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > harini.katakam@xilinx.com; andrei.pistirica@microchip.com > Subject: Re: [PATCH v2 4/4] net: macb: Add hardware PTP support > > On Fri, Jun 02, 2017 at 03:28:10PM +0100, Rafal Ozieblo wrote: > > +static void gem_ptp_clear_timer(struct macb *bp) > > +{ > > + bp->tsu_incr.ns = 0; > > + bp->tsu_incr.sub_ns = 0; > > What is the point of this function? Cleaning all bellow registers will stop hardware PTP clock. > > > + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, 0)); > > + gem_writel(bp, TI, GEM_BF(NSINCR, 0)); > > + gem_writel(bp, TA, 0); > > +} (...) > > +int gem_ptp_txstamp(struct macb_queue *queue, struct sk_buff *skb, > > + struct macb_dma_desc *desc) > > +{ > > + struct gem_tx_ts *tx_timestamp; > > + struct macb_dma_desc_ptp *desc_ptp; > > + unsigned long head = queue->tx_ts_head; > > + unsigned long tail = READ_ONCE(queue->tx_ts_tail); > > + > > + if (!GEM_BFEXT(DMA_TXVALID, desc->ctrl)) > > + return -EINVAL; > > + > > + if (CIRC_SPACE(head, tail, PTP_TS_BUFFER_SIZE) == 0) > > + return -ENOMEM; > > + > > + skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; > > + desc_ptp = macb_ptp_desc(queue->bp, desc); > > + tx_timestamp = &queue->tx_timestamps[head]; > > + tx_timestamp->skb = skb; > > + tx_timestamp->desc_ptp.ts_1 = desc_ptp->ts_1; > > + tx_timestamp->desc_ptp.ts_2 = desc_ptp->ts_2; > > + /* move head */ > > + smp_store_release(&queue->tx_ts_head, > > + (head + 1) & (PTP_TS_BUFFER_SIZE - 1)); > > + > > + schedule_work(&queue->tx_ts_task); > > Since the time stamp is in the buffer descriptor, why delay the > delivery via the work item? I put comment about that a few months ago: https://patchwork.ozlabs.org/patch/705629/ Let me quote part about not doing it via worker: " I think, you can not do it in that way. It will hold two locks. If you enable appropriate option in kernel (as far as I remember CONFIG_DEBUG_SPINLOCK) you will get a warning here. Please look at following call-stack: 1. macb_interrupt() // spin_lock(&bp->lock) is taken 2. macb_tx_interrupt() 3. macb_handle_txtstamp() 4. skb_tstamp_tx() 5. __skb_tstamp_tx() 6. skb_may_tx_timestamp() 7. read_lock_bh() // second lock is taken I know that those are different locks and different types. But this could lead to deadlocks. This is the reason of warning I could see. And this is the reason why I get timestamp in interrupt routine but pass it to skb outside interrupt (using circular buffer). Please, refer to this: https://lkml.org/lkml/2016/11/18/168 1. macb_tx_interrupt() 2. macb_tx_timestamp_add() and schedule_work(&queue->tx_timestamp_task) Then, outside interrupt (without holding a lock) : 1. macb_tx_timestamp_flush() 2. macb_tstamp_tx() 3. skb_tstamp_tx()" > > > + return 0; > > +} (...) > > +void gem_ptp_remove(struct net_device *ndev) > > +{ > > + struct macb *bp = netdev_priv(ndev); > > + > > + if (bp->ptp_clock) > > + ptp_clock_unregister(bp->ptp_clock); > > + > > + gem_ptp_clear_timer(bp); > > Why is this 'clear' needed? To stop hardware PTP clock. > > > + dev_info(&bp->pdev->dev, "%s ptp clock unregistered.\n", > > + GEM_PTP_TIMER_NAME); > > +} > > Thanks, > Richard I'll correct all other issues. Regards, Rafal
On Wed, Jun 07, 2017 at 11:13:36AM +0000, Rafal Ozieblo wrote: > Please look at following call-stack: > > 1. macb_interrupt() // spin_lock(&bp->lock) is taken > 2. macb_tx_interrupt() > 3. macb_handle_txtstamp() > 4. skb_tstamp_tx() > 5. __skb_tstamp_tx() > 6. skb_may_tx_timestamp() > 7. read_lock_bh() // second lock is taken Well, you can always drop the lock, or postpone the call to skb_tstamp_tx() until after the spin lock is released... > I know that those are different locks and different types. But this could lead > to deadlocks. This is the reason of warning I could see. Can you please post the lockdep splat? Thanks, Richard
> From: Richard Cochran [mailto:richardcochran@gmail.com] > Sent: 7 czerwca 2017 15:28 > To: Rafal Ozieblo <rafalo@cadence.com> > Cc: David Miller <davem@davemloft.net>; nicolas.ferre@atmel.com; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > harini.katakam@xilinx.com; andrei.pistirica@microchip.com > Subject: Re: [PATCH v2 4/4] net: macb: Add hardware PTP support > > ********** EXTERNAL MAIL. PLEASE USE CAUTION WHEN CLICKING LINKS > OR OPENING ATTACHMENTS ********** > > On Wed, Jun 07, 2017 at 11:13:36AM +0000, Rafal Ozieblo wrote: > > Please look at following call-stack: > > > > 1. macb_interrupt() // spin_lock(&bp->lock) is taken > > 2. macb_tx_interrupt() > > 3. macb_handle_txtstamp() > > 4. skb_tstamp_tx() > > 5. __skb_tstamp_tx() > > 6. skb_may_tx_timestamp() > > 7. read_lock_bh() // second lock is taken > > Well, you can always drop the lock, or postpone the call to > skb_tstamp_tx() until after the spin lock is released... Postponing is done using worker. Another pros is that skb_tstamp_tx() is called outside interrupt routine. > > > I know that those are different locks and different types. But this could > lead > > to deadlocks. This is the reason of warning I could see. > > Can you please post the lockdep splat? > [ 1264.352242] ================================= [ 1264.356719] [ INFO: inconsistent lock state ] [ 1264.361242] 4.4.0-42139-g418b7a0-dirty #16 Not tainted [ 1264.366502] --------------------------------- [ 1264.371004] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage. [ 1264.377204] ptp4l/105 [HC1[1]:SC0[4]:HE0:SE0] takes: [ 1264.382286] (&(&list->lock)->rlock){?.-...}, at: [<d03250f8>] skb_queue_tail+0x18/0x34 [ 1264.390622] {HARDIRQ-ON-W} state was registered at: [ 1264.395618] [<d0039d50>] lock_acquire+0x124/0x158 [ 1264.400730] [<d03e81f8>] _raw_spin_lock_bh+0x50/0x5c [ 1264.406095] [<d0381745>] first_packet_length+0x35/0x1b4 [ 1264.411710] [<d0381f80>] udp_poll+0x30/0x3c [ 1264.416267] [<d031743d>] sock_poll+0x141/0x148 [ 1264.421092] [<d00ea20d>] do_sys_poll+0x171/0x300 [ 1264.426111] [<d00ea41d>] SyS_poll+0x45/0x80 [ 1264.430676] [<d000481c>] system_call+0x40/0x50 [ 1264.435492] [<d00043d1>] common_exception_return+0x0/0x83 [ 1264.441274] irq event stamp: 23325 [ 1264.444802] hardirqs last enabled at (23324): [<d03e84da>] _raw_spin_unlock_irqrestore+0x4e/0x78 [ 1264.453929] hardirqs last disabled at (23325): [<d00065af>] do_interrupt+0x13/0x90 [ 1264.461732] softirqs last enabled at (23292): [<d0362cf8>] ip_finish_output2+0x80/0x43c [ 1264.470073] softirqs last disabled at (23294): [<d0362da6>] ip_finish_output2+0x12e/0x43c [ 1264.478480] [ 1264.478480] other info that might help us debug this: [ 1264.485211] Possible unsafe locking scenario: [ 1264.485211] [ 1264.491323] CPU0 [ 1264.493883] ---- [ 1264.496439] lock(&(&list->lock)->rlock); [ 1264.500665] <Interrupt> [ 1264.503392] lock(&(&list->lock)->rlock); [ 1264.507793] [ 1264.507793] *** DEADLOCK *** [ 1264.507793] [ 1264.514016] 4 locks held by ptp4l/105: [ 1264.517886] #0: (rcu_read_lock_bh){......}, at: [<d0362da6>] ip_finish_output2+0x12e/0x43c [ 1264.526675] #1: (rcu_read_lock_bh){......}, at: [<d03330f4>] __dev_queue_xmit+0x38/0x5f8 [ 1264.535287] #2: (_xmit_ETHER#2){+.-...}, at: [<d0348381>] sch_direct_xmit+0x51/0x138 [ 1264.543593] #3: (&(&bp->lock)->rlock){-.-...}, at: [<d02caeb2>] macb_interrupt+0x36/0x564 [ 1264.552316] [ 1264.552316] stack backtrace: [ 1264.556962] CPU: 0 PID: 105 Comm: ptp4l Not tainted 4.4.0-42139-g418b7a0-dirty #16 [ 1264.564654] [ 1264.564654] Stack: d1786130 d75815e3 00001ffc d75815e0 9003811c d7581620 d7558c00 d1c0bd60 [ 1264.564654] 00000004 00000001 00000001 d7581600 900389fc d7581650 d7558c00 d7559160 [ 1264.564654] 00000004 00000000 00000000 d7559160 d04ecc34 d1bf2de0 00000179 d7581620 [ 1264.588339] Call Trace: [ 1264.590894] [<d009a8af>] print_usage_bug.part.33+0x1c7/0x1cc [ 1264.596883] [<d003811c>] mark_lock+0x2f4/0x44c [ 1264.601639] [<d00389fc>] __lock_acquire+0x460/0x1228 [ 1264.606907] [<d0039d50>] lock_acquire+0x124/0x158 [ 1264.611907] [<d03e8335>] _raw_spin_lock_irqsave+0x45/0x54 [ 1264.617612] [<d03250f8>] skb_queue_tail+0x18/0x34 [ 1264.622594] [<d03251f0>] sock_queue_err_skb+0xdc/0x108 [ 1264.628017] [<d0326705>] __skb_complete_tx_timestamp+0x55/0x60 [ 1264.634146] [<d0326810>] __skb_tstamp_tx+0x94/0x98 [ 1264.639215] [<d032682c>] skb_tstamp_tx+0x18/0x1c [ 1264.644110] [<d02cb065>] macb_interrupt+0x1e9/0x564 [ 1264.649302] [<d0041af5>] handle_irq_event_percpu+0xe9/0x2a4 [ 1264.655179] [<d0041ce6>] handle_irq_event+0x36/0x50 [ 1264.660343] [<d0043965>] handle_level_irq+0x99/0xb8 [ 1264.665525] [<d00415c5>] generic_handle_irq+0x19/0x24 [ 1264.670863] [<d000491d>] do_IRQ+0x35/0x38 [ 1264.675141] [<d000660c>] do_interrupt+0x70/0x90 [ 1264.679959] [<d00043d1>] common_exception_return+0x0/0x83 [ 1264.685635] [<d02c8eb4>] macb_start_xmit+0x414/0x418 [ 1264.690905] [<d0332eb1>] dev_hard_start_xmit+0x2ed/0x450 [ 1264.696521] [<d034839e>] sch_direct_xmit+0x6e/0x138 [ 1264.701720] [<d0333449>] __dev_queue_xmit+0x38d/0x5f8 [ 1264.707058] [<d03336c8>] dev_queue_xmit+0x14/0x18 [ 1264.712049] [<d033ab64>] neigh_resolve_output+0x180/0x190 [ 1264.717734] [<d0362ff2>] ip_finish_output2+0x37a/0x43c [ 1264.723186] [<d0364168>] ip_finish_output+0x1b8/0x200 [ 1264.728532] [<d0364969>] ip_mc_output+0x179/0x1cc [ 1264.733532] [<d03642e5>] ip_local_out+0x45/0x5c [ 1264.738359] [<d03651ae>] ip_send_skb+0x16/0x64 [ 1264.743097] [<d03815f4>] udp_send_skb+0x160/0x1c8 [ 1264.748098] [<d0382741>] udp_sendmsg+0x45d/0x5a8 [ 1264.753009] [<d038a7f9>] inet_sendmsg+0x21/0x3c [ 1264.757801] [<d0317de5>] sock_sendmsg+0x1d/0x44 [ 1264.762629] [<d03188e1>] SyS_sendto+0x89/0xa4 [ 1264.767288] [<d000481c>] system_call+0x40/0x50 [ 1264.772010] [<d00043d1>] common_exception_return+0x0/0x83 > Thanks, > Richard Regards, Rafal
diff --git a/drivers/net/ethernet/cadence/Makefile b/drivers/net/ethernet/cadence/Makefile index 31ea6e3..1d66ddb 100644 --- a/drivers/net/ethernet/cadence/Makefile +++ b/drivers/net/ethernet/cadence/Makefile @@ -3,5 +3,9 @@ # macb-y := macb_main.o +ifeq ($(CONFIG_MACB_USE_HWSTAMP),y) +macb-y += macb_ptp.o +endif + obj-$(CONFIG_MACB) += macb.o obj-$(CONFIG_MACB_PCI) += macb_pci.o diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h index 4359b08..4285daa6 100644 --- a/drivers/net/ethernet/cadence/macb.h +++ b/drivers/net/ethernet/cadence/macb.h @@ -11,6 +11,8 @@ #define _MACB_H #include <linux/phy.h> +#include <linux/ptp_clock_kernel.h> +#include <linux/net_tstamp.h> #if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) || defined(CONFIG_MACB_USE_HWSTAMP) #define MACB_EXT_DESC @@ -90,6 +92,10 @@ #define GEM_SA3T 0x009C /* Specific3 Top */ #define GEM_SA4B 0x00A0 /* Specific4 Bottom */ #define GEM_SA4T 0x00A4 /* Specific4 Top */ +#define GEM_EFTSH 0x00e8 /* PTP Event Frame Transmitted Seconds Register 47:32 */ +#define GEM_EFRSH 0x00ec /* PTP Event Frame Received Seconds Register 47:32 */ +#define GEM_PEFTSH 0x00f0 /* PTP Peer Event Frame Transmitted Seconds Register 47:32 */ +#define GEM_PEFRSH 0x00f4 /* PTP Peer Event Frame Received Seconds Register 47:32 */ #define GEM_OTX 0x0100 /* Octets transmitted */ #define GEM_OCTTXL 0x0100 /* Octets transmitted [31:0] */ #define GEM_OCTTXH 0x0104 /* Octets transmitted [47:32] */ @@ -159,6 +165,9 @@ #define GEM_DCFG6 0x0294 /* Design Config 6 */ #define GEM_DCFG7 0x0298 /* Design Config 7 */ +#define GEM_TXBDCTRL 0x04cc /* TX Buffer Descriptor control register */ +#define GEM_RXBDCTRL 0x04d0 /* RX Buffer Descriptor control register */ + #define GEM_ISR(hw_q) (0x0400 + ((hw_q) << 2)) #define GEM_TBQP(hw_q) (0x0440 + ((hw_q) << 2)) #define GEM_TBQPH(hw_q) (0x04C8) @@ -195,6 +204,8 @@ #define MACB_TZQ_OFFSET 12 /* Transmit zero quantum pause frame */ #define MACB_TZQ_SIZE 1 #define MACB_SRTSM_OFFSET 15 +#define MACB_OSSMODE_OFFSET 24 /* Enable One Step Synchro Mode */ +#define MACB_OSSMODE_SIZE 1 /* Bitfields in NCFGR */ #define MACB_SPD_OFFSET 0 /* Speed */ @@ -452,6 +463,52 @@ #define GEM_NSINCR_OFFSET 0 #define GEM_NSINCR_SIZE 8 +/* Bitfields in TSH */ +#define GEM_TSH_OFFSET 0 /* TSU timer value (s). MSB [47:32] of seconds timer count */ +#define GEM_TSH_SIZE 16 + +/* Bitfields in TSL */ +#define GEM_TSL_OFFSET 0 /* TSU timer value (s). LSB [31:0] of seconds timer count */ +#define GEM_TSL_SIZE 32 + +/* Bitfields in TN */ +#define GEM_TN_OFFSET 0 /* TSU timer value (ns) */ +#define GEM_TN_SIZE 30 + +/* Bitfields in TXBDCTRL */ +#define GEM_TXTSMODE_OFFSET 4 /* TX Descriptor Timestamp Insertion mode */ +#define GEM_TXTSMODE_SIZE 2 + +/* Bitfields in RXBDCTRL */ +#define GEM_RXTSMODE_OFFSET 4 /* RX Descriptor Timestamp Insertion mode */ +#define GEM_RXTSMODE_SIZE 2 + +/* Transmit DMA buffer descriptor Word 1 */ +#define GEM_DMA_TXVALID_OFFSET 23 /* timestamp has been captured in the Buffer Descriptor */ +#define GEM_DMA_TXVALID_SIZE 1 + +/* Receive DMA buffer descriptor Word 0 */ +#define GEM_DMA_RXVALID_OFFSET 2 /* indicates a valid timestamp in the Buffer Descriptor */ +#define GEM_DMA_RXVALID_SIZE 1 + +/* DMA buffer descriptor Word 2 (32 bit addressing) or Word 4 (64 bit addressing) */ +#define GEM_DMA_SECL_OFFSET 30 /* Timestamp seconds[1:0] */ +#define GEM_DMA_SECL_SIZE 2 +#define GEM_DMA_NSEC_OFFSET 0 /* Timestamp nanosecs [29:0] */ +#define GEM_DMA_NSEC_SIZE 30 + +/* DMA buffer descriptor Word 3 (32 bit addressing) or Word 5 (64 bit addressing) */ + +/* New hardware supports 12 bit precision of timestamp in DMA buffer descriptor. + * Old hardware supports only 6 bit precision but it is enough for PTP. + * Less accuracy is used always instead of checking hardware version. + */ +#define GEM_DMA_SECH_OFFSET 0 /* Timestamp seconds[5:2] */ +#define GEM_DMA_SECH_SIZE 4 +#define GEM_DMA_SEC_WIDTH (GEM_DMA_SECH_SIZE + GEM_DMA_SECL_SIZE) +#define GEM_DMA_SEC_TOP (1 << GEM_DMA_SEC_WIDTH) +#define GEM_DMA_SEC_MASK (GEM_DMA_SEC_TOP - 1) + /* Bitfields in ADJ */ #define GEM_ADDSUB_OFFSET 31 #define GEM_ADDSUB_SIZE 1 @@ -527,6 +584,8 @@ #define queue_readl(queue, reg) (queue)->bp->macb_reg_readl((queue)->bp, (queue)->reg) #define queue_writel(queue, reg, value) (queue)->bp->macb_reg_writel((queue)->bp, (queue)->reg, (value)) +#define PTP_TS_BUFFER_SIZE 128 /* must be power of 2 */ + /* Conditional GEM/MACB macros. These perform the operation to the correct * register dependent on whether the device is a GEM or a MACB. For registers * and bitfields that are common across both devices, use macb_{read,write}l @@ -574,6 +633,11 @@ struct macb_dma_desc_ptp { u32 ts_1; u32 ts_2; }; + +struct gem_tx_ts { + struct sk_buff *skb; + struct macb_dma_desc_ptp desc_ptp; +}; #endif /* DMA descriptor bitfields */ @@ -889,6 +953,11 @@ struct macb_config { int jumbo_max_len; }; +struct tsu_incr { + u32 sub_ns; + u32 ns; +}; + struct macb_queue { struct macb *bp; int irq; @@ -905,6 +974,12 @@ struct macb_queue { struct macb_tx_skb *tx_skb; dma_addr_t tx_ring_dma; struct work_struct tx_error_task; + +#ifdef CONFIG_MACB_USE_HWSTAMP + struct work_struct tx_ts_task; + unsigned int tx_ts_head, tx_ts_tail; + struct gem_tx_ts tx_timestamps[PTP_TS_BUFFER_SIZE]; +#endif }; struct macb { @@ -975,8 +1050,59 @@ struct macb { #ifdef MACB_EXT_DESC uint8_t hw_dma_cap; #endif + spinlock_t tsu_clk_lock; /* gem tsu clock locking */ + unsigned int tsu_rate; + struct ptp_clock *ptp_clock; + struct ptp_clock_info ptp_clock_info; + struct tsu_incr tsu_incr; + struct hwtstamp_config tstamp_config; }; +#ifdef CONFIG_MACB_USE_HWSTAMP +#define GEM_TSEC_SIZE (GEM_TSH_SIZE + GEM_TSL_SIZE) +#define TSU_SEC_MAX_VAL (((u64)1 << GEM_TSEC_SIZE) - 1) +#define TSU_NSEC_MAX_VAL ((1 << GEM_TN_SIZE) - 1) + +enum macb_bd_control { + TSTAMP_DISABLED, + TSTAMP_FRAME_PTP_EVENT_ONLY, + TSTAMP_ALL_PTP_FRAMES, + TSTAMP_ALL_FRAMES, +}; + +void gem_ptp_init(struct net_device *ndev); +void gem_ptp_remove(struct net_device *ndev); +int gem_ptp_txstamp(struct macb_queue *queue, struct sk_buff *skb, struct macb_dma_desc *des); +void gem_ptp_rxstamp(struct macb *bp, struct sk_buff *skb, struct macb_dma_desc *desc); +static inline int gem_ptp_do_txstamp(struct macb_queue *queue, struct sk_buff *skb, struct macb_dma_desc *desc) +{ + if (queue->bp->tstamp_config.tx_type == TSTAMP_DISABLED) + return -ENOTSUPP; + + return gem_ptp_txstamp(queue, skb, desc); +} + +static inline void gem_ptp_do_rxstamp(struct macb *bp, struct sk_buff *skb, struct macb_dma_desc *desc) +{ + if (bp->tstamp_config.rx_filter == TSTAMP_DISABLED) + return; + + gem_ptp_rxstamp(bp, skb, desc); +} +int gem_get_hwtst(struct net_device *dev, struct ifreq *rq); +int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd); +#else +static inline void gem_ptp_init(struct net_device *ndev) { } +static inline void gem_ptp_remove(struct net_device *ndev) { } + +static inline int gem_ptp_do_txstamp(struct macb_queue *queue, struct sk_buff *skb, struct macb_dma_desc *desc) +{ + return -1; +} + +static inline void gem_ptp_do_rxstamp(struct macb *bp, struct sk_buff *skb, struct macb_dma_desc *desc) { } +#endif + static inline bool macb_is_gem(struct macb *bp) { return !!(bp->caps & MACB_CAPS_MACB_IS_GEM); diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 3151429..87a6580 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -828,6 +828,12 @@ static void macb_tx_interrupt(struct macb_queue *queue) /* First, update TX stats if needed */ if (skb) { + if (gem_ptp_do_txstamp(queue, skb, desc) == 0) { + /* skb now belongs to timestamp buffer + * and will be removed later + */ + tx_skb->skb = NULL; + } netdev_vdbg(bp->dev, "skb %u (data %p) TX complete\n", macb_tx_ring_wrap(bp, tail), skb->data); @@ -994,6 +1000,8 @@ static int gem_rx(struct macb *bp, int budget) bp->dev->stats.rx_packets++; bp->dev->stats.rx_bytes += skb->len; + gem_ptp_do_rxstamp(bp, skb, desc); + #if defined(DEBUG) && defined(VERBOSE_DEBUG) netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n", skb->len, skb->csum); @@ -1315,7 +1323,6 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id) if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE) queue_writel(queue, ISR, MACB_BIT(HRESP)); } - status = queue_readl(queue, ISR); } @@ -1645,7 +1652,6 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev) /* Make newly initialized descriptor visible to hardware */ wmb(); - skb_tx_timestamp(skb); macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART)); @@ -2503,6 +2509,71 @@ static int macb_set_ringparam(struct net_device *netdev, return 0; } +#ifdef CONFIG_MACB_USE_HWSTAMP +static unsigned int gem_get_tsu_rate(struct macb *bp) +{ + struct clk *tsu_clk; + unsigned int tsu_rate; + + tsu_clk = devm_clk_get(&bp->pdev->dev, "tsu_clk"); + if (!IS_ERR(tsu_clk)) + tsu_rate = clk_get_rate(tsu_clk); + /* try pclk instead */ + else if (!IS_ERR(bp->pclk)) { + tsu_clk = bp->pclk; + tsu_rate = clk_get_rate(tsu_clk); + } else + return -ENOTSUPP; + return tsu_rate; +} + +static s32 gem_get_ptp_max_adj(void) +{ + return 64E6; +} + +static int gem_get_ts_info(struct net_device *dev, + struct ethtool_ts_info *info) +{ + struct macb *bp = netdev_priv(dev); + + ethtool_op_get_ts_info(dev, info); + if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0) + return 0; + + 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; + info->tx_types = + (1 << HWTSTAMP_TX_ONESTEP_SYNC) | + (1 << HWTSTAMP_TX_OFF) | + (1 << HWTSTAMP_TX_ON); + info->rx_filters = + (1 << HWTSTAMP_FILTER_NONE) | + (1 << HWTSTAMP_FILTER_ALL); + info->phc_index = -1; + + if (bp->ptp_clock) + info->phc_index = ptp_clock_index(bp->ptp_clock); + + return 0; +} + +static struct macb_ptp_info gem_ptp_info = { + .ptp_init = gem_ptp_init, + .ptp_remove = gem_ptp_remove, + .get_ptp_max_adj = gem_get_ptp_max_adj, + .get_tsu_rate = gem_get_tsu_rate, + .get_ts_info = gem_get_ts_info, + .get_hwtst = gem_get_hwtst, + .set_hwtst = gem_set_hwtst, +}; +#endif + static int macb_get_ts_info(struct net_device *netdev, struct ethtool_ts_info *info) { @@ -2636,12 +2707,16 @@ static void macb_configure_caps(struct macb *bp, dcfg = gem_readl(bp, DCFG2); if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0) bp->caps |= MACB_CAPS_FIFO_MODE; - if (IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) && gem_has_ptp(bp)) { +#ifdef CONFIG_MACB_USE_HWSTAMP + if (gem_has_ptp(bp)) { if (!GEM_BFEXT(TSU, gem_readl(bp, DCFG5))) pr_err("GEM doesn't support hardware ptp.\n"); - else + else { bp->hw_dma_cap |= HW_DMA_CAP_PTP; + bp->ptp_info = &gem_ptp_info; + } } +#endif } dev_dbg(&bp->pdev->dev, "Cadence caps 0x%08x\n", bp->caps); @@ -3247,7 +3322,9 @@ static const struct macb_config np4_config = { }; static const struct macb_config zynqmp_config = { - .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO, + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | + MACB_CAPS_JUMBO | + MACB_CAPS_GEM_HAS_PTP, .dma_burst_length = 16, .clk_init = macb_clk_init, .init = macb_init, @@ -3281,7 +3358,9 @@ MODULE_DEVICE_TABLE(of, macb_dt_ids); #endif /* CONFIG_OF */ static const struct macb_config default_gem_config = { - .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO, + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | + MACB_CAPS_JUMBO | + MACB_CAPS_GEM_HAS_PTP, .dma_burst_length = 16, .clk_init = macb_clk_init, .init = macb_init, diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c new file mode 100755 index 0000000..d536970 --- /dev/null +++ b/drivers/net/ethernet/cadence/macb_ptp.c @@ -0,0 +1,512 @@ +/** + * 1588 PTP support for Cadence GEM device. + * + * Copyright (C) 2017 Cadence Design Systems - http://www.cadence.com + * + * Authors: Rafal Ozieblo <rafalo@cadence.com> + * Bartosz Folta <bfolta@cadence.com> + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 of + * the License as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include <linux/kernel.h> +#include <linux/types.h> +#include <linux/clk.h> +#include <linux/device.h> +#include <linux/etherdevice.h> +#include <linux/platform_device.h> +#include <linux/time64.h> +#include <linux/ptp_classify.h> +#include <linux/if_ether.h> +#include <linux/if_vlan.h> +#include <linux/net_tstamp.h> +#include <linux/circ_buf.h> +#include <linux/spinlock.h> + +#include "macb.h" + +#define GEM_PTP_TIMER_NAME "gem-ptp-timer" + +static struct macb_dma_desc_ptp *macb_ptp_desc(struct macb *bp, + struct macb_dma_desc *desc) +{ + if (bp->hw_dma_cap == HW_DMA_CAP_PTP) + return (struct macb_dma_desc_ptp *) + ((u8 *)desc + sizeof(struct macb_dma_desc)); + if (bp->hw_dma_cap == HW_DMA_CAP_64B_PTP) + return (struct macb_dma_desc_ptp *) + ((u8 *)desc + sizeof(struct macb_dma_desc) + + sizeof(struct macb_dma_desc_64)); + return NULL; +} + +static int gem_tsu_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts) +{ + long first, second; + u32 secl, sech; + unsigned long flags; + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info); + + spin_lock_irqsave(&bp->tsu_clk_lock, flags); + first = gem_readl(bp, TN); + secl = gem_readl(bp, TSL); + sech = gem_readl(bp, TSH); + second = gem_readl(bp, TN); + + /* test for nsec rollover */ + if (first > second) { + /* if so, use later read & re-read seconds + * (assume all done within 1s) + */ + ts->tv_nsec = gem_readl(bp, TN); + secl = gem_readl(bp, TSL); + sech = gem_readl(bp, TSH); + } else { + ts->tv_nsec = first; + } + + spin_unlock_irqrestore(&bp->tsu_clk_lock, flags); + ts->tv_sec = (((u64)sech << GEM_TSL_SIZE) | secl) + & TSU_SEC_MAX_VAL; + return 0; +} + +static int gem_tsu_set_time(struct ptp_clock_info *ptp, + const struct timespec64 *ts) +{ + u32 ns, sech, secl; + unsigned long flags; + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info); + + secl = (u32)ts->tv_sec; + sech = (ts->tv_sec >> GEM_TSL_SIZE) & ((1 << GEM_TSH_SIZE) - 1); + ns = ts->tv_nsec; + + spin_lock_irqsave(&bp->tsu_clk_lock, flags); + + /* TSH doesn't latch the time and no atomicity! */ + gem_writel(bp, TN, 0); /* clear to avoid overflow */ + gem_writel(bp, TSH, sech); + /* write lower bits 2nd, for synchronized secs update */ + gem_writel(bp, TSL, secl); + gem_writel(bp, TN, ns); + + spin_unlock_irqrestore(&bp->tsu_clk_lock, flags); + + return 0; +} + +static int gem_tsu_incr_set(struct macb *bp, struct tsu_incr *incr_spec) +{ + unsigned long flags; + + /* tsu_timer_incr register must be written after + * the tsu_timer_incr_sub_ns register and the write operation + * will cause the value written to the tsu_timer_incr_sub_ns register + * to take effect. + */ + spin_lock_irqsave(&bp->tsu_clk_lock, flags); + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, incr_spec->sub_ns)); + gem_writel(bp, TI, GEM_BF(NSINCR, incr_spec->ns)); + spin_unlock_irqrestore(&bp->tsu_clk_lock, flags); + + return 0; +} + +static int gem_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) +{ + struct tsu_incr incr_spec; + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info); + u64 adj; + u32 word; + bool neg_adj = false; + + if (scaled_ppm < 0) { + neg_adj = true; + scaled_ppm = -scaled_ppm; + } + + /* Adjustment is relative to base frequency */ + incr_spec.sub_ns = bp->tsu_incr.sub_ns; + incr_spec.ns = bp->tsu_incr.ns; + + /* scaling: unused(8bit) | ns(8bit) | fractions(16bit) */ + word = ((u64)incr_spec.ns << GEM_SUBNSINCR_SIZE) + incr_spec.sub_ns; + adj = (u64)scaled_ppm * word; + /* Divide with rounding, equivalent to floating dividing: + * (temp / USEC_PER_SEC) + 0.5 + */ + adj += (USEC_PER_SEC >> 1); + adj >>= GEM_SUBNSINCR_SIZE; /* remove fractions */ + adj = div_u64(adj, USEC_PER_SEC); + adj = neg_adj ? (word - adj) : (word + adj); + + incr_spec.ns = (adj >> GEM_SUBNSINCR_SIZE) + & ((1 << GEM_NSINCR_SIZE) - 1); + incr_spec.sub_ns = adj & ((1 << GEM_SUBNSINCR_SIZE) - 1); + gem_tsu_incr_set(bp, &incr_spec); + return 0; +} + +static int gem_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta) +{ + struct timespec64 now, then = ns_to_timespec64(delta); + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info); + u32 adj, sign = 0; + + if (delta < 0) { + sign = 1; + delta = -delta; + } + + if (delta > TSU_NSEC_MAX_VAL) { + gem_tsu_get_time(&bp->ptp_clock_info, &now); + if (sign) + now = timespec64_sub(now, then); + else + now = timespec64_add(now, then); + + gem_tsu_set_time(&bp->ptp_clock_info, + (const struct timespec64 *)&now); + } else { + adj = (sign << GEM_ADDSUB_OFFSET) | delta; + + gem_writel(bp, TA, adj); + } + + return 0; +} + +static int gem_ptp_enable(struct ptp_clock_info *ptp, + struct ptp_clock_request *rq, int on) +{ + return -EOPNOTSUPP; +} + +static struct ptp_clock_info gem_ptp_caps_template = { + .owner = THIS_MODULE, + .name = GEM_PTP_TIMER_NAME, + .max_adj = 0, + .n_alarm = 0, + .n_ext_ts = 0, + .n_per_out = 0, + .n_pins = 0, + .pps = 1, + .adjfine = gem_ptp_adjfine, + .adjtime = gem_ptp_adjtime, + .gettime64 = gem_tsu_get_time, + .settime64 = gem_tsu_set_time, + .enable = gem_ptp_enable, +}; + +static void gem_ptp_init_timer(struct macb *bp) +{ + u32 rem = 0; + + bp->tsu_incr.ns = div_u64_rem(NSEC_PER_SEC, bp->tsu_rate, &rem); + if (rem) { + u64 adj = rem; + + adj <<= GEM_SUBNSINCR_SIZE; + bp->tsu_incr.sub_ns = div_u64(adj, bp->tsu_rate); + } else { + bp->tsu_incr.sub_ns = 0; + } +} + +static void gem_ptp_init_tsu(struct macb *bp) +{ + struct timespec64 ts; + + /* 1. get current system time */ + ts = ns_to_timespec64(ktime_to_ns(ktime_get_real())); + + /* 2. set ptp timer */ + gem_tsu_set_time(&bp->ptp_clock_info, &ts); + + /* 3. set PTP timer increment value to BASE_INCREMENT */ + gem_tsu_incr_set(bp, &bp->tsu_incr); + + gem_writel(bp, TA, 0); +} + +static void gem_ptp_clear_timer(struct macb *bp) +{ + bp->tsu_incr.ns = 0; + bp->tsu_incr.sub_ns = 0; + + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, 0)); + gem_writel(bp, TI, GEM_BF(NSINCR, 0)); + gem_writel(bp, TA, 0); +} + +static int gem_hw_timestamp(struct macb *bp, u32 dma_desc_ts_1, + u32 dma_desc_ts_2, struct timespec64 *ts) +{ + struct timespec64 tsu; + + ts->tv_sec = (GEM_BFEXT(DMA_SECH, dma_desc_ts_2) << GEM_DMA_SECL_SIZE) | + GEM_BFEXT(DMA_SECL, dma_desc_ts_1); + ts->tv_nsec = GEM_BFEXT(DMA_NSEC, dma_desc_ts_1); + + /* TSU overlapping workaround + * The timestamp only contains lower few bits of seconds, + * so add value from 1588 timer + */ + gem_tsu_get_time(&bp->ptp_clock_info, &tsu); + + /* If the top bit is set in the timestamp, + * but not in 1588 timer, it has rolled over, + * so subtract max size + */ + if ((ts->tv_sec & (GEM_DMA_SEC_TOP >> 1)) && + !(tsu.tv_sec & (GEM_DMA_SEC_TOP >> 1))) + ts->tv_sec -= GEM_DMA_SEC_TOP; + + ts->tv_sec += ((~GEM_DMA_SEC_MASK) & tsu.tv_sec); + + return 0; +} + +void gem_ptp_rxstamp(struct macb *bp, struct sk_buff *skb, + struct macb_dma_desc *desc) +{ + struct timespec64 ts; + struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); + struct macb_dma_desc_ptp *desc_ptp; + + if (GEM_BFEXT(DMA_RXVALID, desc->addr)) { + desc_ptp = macb_ptp_desc(bp, desc); + gem_hw_timestamp(bp, desc_ptp->ts_1, desc_ptp->ts_2, &ts); + memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps)); + shhwtstamps->hwtstamp = ktime_set(ts.tv_sec, ts.tv_nsec); + } +} + +static void gem_tstamp_tx(struct macb *bp, struct sk_buff *skb, + struct macb_dma_desc_ptp *desc_ptp) +{ + struct skb_shared_hwtstamps shhwtstamps; + struct timespec64 ts; + + gem_hw_timestamp(bp, desc_ptp->ts_1, desc_ptp->ts_2, &ts); + memset(&shhwtstamps, 0, sizeof(shhwtstamps)); + shhwtstamps.hwtstamp = ktime_set(ts.tv_sec, ts.tv_nsec); + skb_tstamp_tx(skb, &shhwtstamps); +} + +int gem_ptp_txstamp(struct macb_queue *queue, struct sk_buff *skb, + struct macb_dma_desc *desc) +{ + struct gem_tx_ts *tx_timestamp; + struct macb_dma_desc_ptp *desc_ptp; + unsigned long head = queue->tx_ts_head; + unsigned long tail = READ_ONCE(queue->tx_ts_tail); + + if (!GEM_BFEXT(DMA_TXVALID, desc->ctrl)) + return -EINVAL; + + if (CIRC_SPACE(head, tail, PTP_TS_BUFFER_SIZE) == 0) + return -ENOMEM; + + skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; + desc_ptp = macb_ptp_desc(queue->bp, desc); + tx_timestamp = &queue->tx_timestamps[head]; + tx_timestamp->skb = skb; + tx_timestamp->desc_ptp.ts_1 = desc_ptp->ts_1; + tx_timestamp->desc_ptp.ts_2 = desc_ptp->ts_2; + /* move head */ + smp_store_release(&queue->tx_ts_head, + (head + 1) & (PTP_TS_BUFFER_SIZE - 1)); + + schedule_work(&queue->tx_ts_task); + return 0; +} + +static void gem_tx_timestamp_flush(struct work_struct *work) +{ + struct macb_queue *queue = + container_of(work, struct macb_queue, tx_ts_task); + struct gem_tx_ts *tx_ts; + unsigned long head, tail; + + /* take current head */ + head = smp_load_acquire(&queue->tx_ts_head); + tail = queue->tx_ts_tail; + + while (CIRC_CNT(head, tail, PTP_TS_BUFFER_SIZE)) { + tx_ts = &queue->tx_timestamps[tail]; + gem_tstamp_tx(queue->bp, tx_ts->skb, &tx_ts->desc_ptp); + /* cleanup */ + dev_kfree_skb_any(tx_ts->skb); + /* remove old tail */ + smp_store_release(&queue->tx_ts_tail, + (tail + 1) & (PTP_TS_BUFFER_SIZE - 1)); + tail = queue->tx_ts_tail; + } +} + +void gem_ptp_init(struct net_device *dev) +{ + struct macb *bp = netdev_priv(dev); + unsigned int q; + struct macb_queue *queue; + + bp->ptp_clock_info = gem_ptp_caps_template; + + /* nominal frequency and maximum adjustment in ppb */ + bp->tsu_rate = bp->ptp_info->get_tsu_rate(bp); + bp->ptp_clock_info.max_adj = bp->ptp_info->get_ptp_max_adj(); + gem_ptp_init_timer(bp); + bp->ptp_clock = ptp_clock_register(&bp->ptp_clock_info, &dev->dev); + if (IS_ERR(&bp->ptp_clock)) { + bp->ptp_clock = NULL; + pr_err("ptp clock register failed\n"); + return; + } + + spin_lock_init(&bp->tsu_clk_lock); + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) { + queue->tx_ts_head = 0; + queue->tx_ts_tail = 0; + INIT_WORK(&queue->tx_ts_task, gem_tx_timestamp_flush); + } + + gem_ptp_init_tsu(bp); + + dev_info(&bp->pdev->dev, "%s ptp clock registered.\n", + GEM_PTP_TIMER_NAME); +} + +void gem_ptp_remove(struct net_device *ndev) +{ + struct macb *bp = netdev_priv(ndev); + + if (bp->ptp_clock) + ptp_clock_unregister(bp->ptp_clock); + + gem_ptp_clear_timer(bp); + + dev_info(&bp->pdev->dev, "%s ptp clock unregistered.\n", + GEM_PTP_TIMER_NAME); +} + +static int gem_ptp_set_ts_mode(struct macb *bp, + enum macb_bd_control tx_bd_control, + enum macb_bd_control rx_bd_control) +{ + gem_writel(bp, TXBDCTRL, GEM_BF(TXTSMODE, tx_bd_control)); + gem_writel(bp, RXBDCTRL, GEM_BF(RXTSMODE, rx_bd_control)); + + return 0; +} + +int gem_get_hwtst(struct net_device *dev, struct ifreq *rq) +{ + struct macb *bp = netdev_priv(dev); + struct hwtstamp_config *tstamp_config = &bp->tstamp_config; + + if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0) + return -EFAULT; + + if (copy_to_user(rq->ifr_data, tstamp_config, sizeof(*tstamp_config))) + return -EFAULT; + else + return 0; +} + +static int gem_ptp_set_one_step_sync(struct macb *bp, u8 enable) +{ + u32 reg_val; + + reg_val = macb_readl(bp, NCR); + + if (enable) + macb_writel(bp, NCR, reg_val | MACB_BIT(OSSMODE)); + else + macb_writel(bp, NCR, reg_val & ~MACB_BIT(OSSMODE)); + + return 0; +} + +int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd) +{ + struct macb *bp = netdev_priv(dev); + struct hwtstamp_config *tstamp_config = &bp->tstamp_config; + enum macb_bd_control tx_bd_control = TSTAMP_DISABLED; + enum macb_bd_control rx_bd_control = TSTAMP_DISABLED; + u32 regval; + + if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0) + return -EFAULT; + + if (copy_from_user(tstamp_config, ifr->ifr_data, + sizeof(*tstamp_config))) + return -EFAULT; + + /* reserved for future extensions */ + if (tstamp_config->flags) + return -EINVAL; + + switch (tstamp_config->tx_type) { + case HWTSTAMP_TX_OFF: + break; + case HWTSTAMP_TX_ONESTEP_SYNC: + if (gem_ptp_set_one_step_sync(bp, 1) != 0) + return -ERANGE; + case HWTSTAMP_TX_ON: + tx_bd_control = TSTAMP_ALL_FRAMES; + break; + default: + return -ERANGE; + } + + switch (tstamp_config->rx_filter) { + case HWTSTAMP_FILTER_NONE: + break; + case HWTSTAMP_FILTER_PTP_V1_L4_SYNC: + break; + case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ: + break; + case HWTSTAMP_FILTER_PTP_V2_EVENT: + case HWTSTAMP_FILTER_PTP_V2_L2_EVENT: + case HWTSTAMP_FILTER_PTP_V2_L4_EVENT: + case HWTSTAMP_FILTER_PTP_V2_SYNC: + case HWTSTAMP_FILTER_PTP_V2_L2_SYNC: + case HWTSTAMP_FILTER_PTP_V2_L4_SYNC: + case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ: + case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ: + case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ: + rx_bd_control = TSTAMP_ALL_PTP_FRAMES; + tstamp_config->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT; + regval = macb_readl(bp, NCR); + macb_writel(bp, NCR, (regval | MACB_BIT(SRTSM))); + break; + case HWTSTAMP_FILTER_PTP_V1_L4_EVENT: + case HWTSTAMP_FILTER_ALL: + rx_bd_control = TSTAMP_ALL_FRAMES; + tstamp_config->rx_filter = HWTSTAMP_FILTER_ALL; + break; + default: + tstamp_config->rx_filter = HWTSTAMP_FILTER_NONE; + return -ERANGE; + } + + if (gem_ptp_set_ts_mode(bp, tx_bd_control, rx_bd_control) != 0) + return -ERANGE; + + if (copy_to_user(ifr->ifr_data, tstamp_config, sizeof(*tstamp_config))) + return -EFAULT; + else + return 0; +} +
This patch is based on original Harini's patch and Andrei's patch, implemented in a separate file to ease the review/maintanance and integration with other platforms. This driver supports GEM-GXL: - Register ptp clock framework - Initialize PTP related registers - HW time stamp on the PTP Ethernet packets are received using the SO_TIMESTAMPING API. Time stamps are obtained from the dma buffer descriptors - add macb_ptp to compilation chain Signed-off-by: Rafal Ozieblo <rafalo@cadence.com> --- drivers/net/ethernet/cadence/Makefile | 4 + drivers/net/ethernet/cadence/macb.h | 126 ++++++++ drivers/net/ethernet/cadence/macb_main.c | 91 +++++- drivers/net/ethernet/cadence/macb_ptp.c | 512 +++++++++++++++++++++++++++++++ 4 files changed, 727 insertions(+), 6 deletions(-) create mode 100755 drivers/net/ethernet/cadence/macb_ptp.c