Re: [PATCH net-next] cadence: Add hardware PTP support.

[Richard Cochran]

On Fri, Nov 18, 2016 at 11:55:37AM +, Rafal Ozieblo wrote:
> >I'm not sure of your application and why this is necessary.
> >Can you please check Andrei's patches and mine and Richard Cochran's 
> >comments?
> >I use linuxptp to test.

And please, PLEASE, put me onto CC for all PTP related patches.  I am
the maintainer, you know.

Thanks,
Richard

RE: [PATCH net-next] cadence: Add hardware PTP support.

[Rafal Ozieblo]

>Hi Rafal   
> 
>   
>  
>I'm still comparing the full solution but just a couple of things first:   
> 
> 
> 
>> @@ -876,6 +964,17 @@ static int gem_rx(struct macb *bp, int budget)  
>>  
>> bp->stats.rx_packets++;  
>>  
>> bp->stats.rx_bytes += skb->len;  
>>  
>>  
>>  
>> +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)   
>>  
>> +   if (bp->ptp_hw_support) {
>>  
>> +   struct timespec64 ts;
>>  
>> +
>>  
>> +   if (MACB_BFEXT(DMA_RX_TS_VALID, desc->addr)) {   
>>  
>> +   macb_hw_timestamp(bp, desc->dma_desc_ts_1, 
>> desc->dma_desc_ts_2, ); 
>> +   skb_hwtstamps(skb)->hwtstamp = 
>> ktime_set(ts.tv_sec, ts.tv_nsec);  
>> +   }
>>  
>> +   }
>>  
>> +#endif  
>>  
>> +
>>  
>   
>  
>I've had to read PTP event registers for upper part of seconds timestamp in 
>addition to the descriptor (since descriptor only has 5 bits of seconds TS).   
>  
>I don't know which version of the IP you use - it could be different.  
> 
>Please let me know so that I can check the spec.   
> 
>Same with tx timestamp of course.

I use GEM_GXL 1p9 and 1p10 but I should be comatible with old version as well.

From documentation:

Receive Buffer Descriptor Entry
Word 0:
Bit 2:
Address [2] of beginning of buffer.
Or
In Extended Buffer Descriptor Mode (DMA configuration register[28] = 1), 
indicates a valid timestamp in the BD entry

Transmit Buffer Descriptor Entry
Word 1:
Bit 23:
For Extended Buffer Descriptor Mode this bit Indicates a timestamp has been 
captured in the BD. Otherwise Reserved.

>   
>  
> 
> 
>> @@ -1195,6 +1297,87 @@ static irqreturn_t macb_interrupt(int irq, void 
>> *dev_id)   
>> queue_writel(queue, ISR, MACB_BIT(HRESP));   
>>  
>> }
>>  
>>  
>>  
>> +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)   
>>  
>> +   if (status & MACB_BIT(PTP_DELAY_REQ_FRAME_RECEIVED)) {   
>>  
>> +   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE) 
>>  
>> +   queue_writel(queue, ISR, 
>> MACB_BIT(PTP_DELAY_REQ_FRAME_RECEIVED)); 
>> +   if (macb_ptp_time_frame_rx_get(bp, ) != 0) {  
>>  
>> +   ts.tv_sec = 0;   
>>  
>> +   ts.tv_nsec = 0;  
>>  
>> +   }
>>  
>> +   macb_ptp_event(bp, ); 
>>  
>> +   }
>>  
>> +
>>  
>> +   

Re: [PATCH net-next] cadence: Add hardware PTP support.

[Harini Katakam]

Hi Rafal

I'm still comparing the full solution but just a couple of things first:

> @@ -876,6 +964,17 @@ static int gem_rx(struct macb *bp, int budget)
> bp->stats.rx_packets++;
> bp->stats.rx_bytes += skb->len;
>
> +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
> +   if (bp->ptp_hw_support) {
> +   struct timespec64 ts;
> +
> +   if (MACB_BFEXT(DMA_RX_TS_VALID, desc->addr)) {
> +   macb_hw_timestamp(bp, desc->dma_desc_ts_1, 
> desc->dma_desc_ts_2, );
> +   skb_hwtstamps(skb)->hwtstamp = 
> ktime_set(ts.tv_sec, ts.tv_nsec);
> +   }
> +   }
> +#endif
> +

I've had to read PTP event registers for upper part of seconds timestamp
in addition to the descriptor (since descriptor only has 5 bits of seconds TS).
I don't know which version of the IP you use - it could be different.
Please let me know so that I can check the spec.
Same with tx timestamp of course.


> @@ -1195,6 +1297,87 @@ static irqreturn_t macb_interrupt(int irq, void 
> *dev_id)
> queue_writel(queue, ISR, MACB_BIT(HRESP));
> }
>
> +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
> +   if (status & MACB_BIT(PTP_DELAY_REQ_FRAME_RECEIVED)) {
> +   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +   queue_writel(queue, ISR, 
> MACB_BIT(PTP_DELAY_REQ_FRAME_RECEIVED));
> +   if (macb_ptp_time_frame_rx_get(bp, ) != 0) {
> +   ts.tv_sec = 0;
> +   ts.tv_nsec = 0;
> +   }
> +   macb_ptp_event(bp, );
> +   }
> +
> +   if (status & MACB_BIT(PTP_SYNC_FRAME_RECEIVED)) {
> +   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +   queue_writel(queue, ISR, 
> MACB_BIT(PTP_SYNC_FRAME_RECEIVED));
> +   if (macb_ptp_time_frame_rx_get(bp, ) != 0) {
> +   ts.tv_sec = 0;
> +   ts.tv_nsec = 0;
> +   }
> +   macb_ptp_event(bp, );
> +   }
> +
> +   if (status & MACB_BIT(PTP_DELAY_REQ_FRAME_TRANSMITTED)) {
> +   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +   queue_writel(queue, ISR, 
> MACB_BIT(PTP_DELAY_REQ_FRAME_TRANSMITTED));
> +   if (macb_ptp_time_frame_tx_get(bp, ) != 0) {
> +   ts.tv_sec = 0;
> +   ts.tv_nsec = 0;
> +   }
> +   macb_ptp_event(bp, );
> +   }
> +
> +   if (status & MACB_BIT(PTP_SYNC_FRAME_TRANSMITTED)) {
> +   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +   queue_writel(queue, ISR, 
> MACB_BIT(PTP_SYNC_FRAME_TRANSMITTED));
> +   if (macb_ptp_time_frame_tx_get(bp, ) != 0) {
> +   ts.tv_sec = 0;
> +   ts.tv_nsec = 0;
> +   }
> +   macb_ptp_event(bp, );
> +   }
> +
> +   if (status & MACB_BIT(PTP_PDELAY_REQ_FRAME_RECEIVED)) {
> +   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +   queue_writel(queue, ISR, 
> MACB_BIT(PTP_PDELAY_REQ_FRAME_RECEIVED));
> +   if (macb_ptp_time_peer_frame_rx_get(bp, ) != 0) {
> +   ts.tv_sec = 0;
> +   ts.tv_nsec = 0;
> +   }
> +   macb_ptp_event(bp, );
> +   }
> +
> +   if (status & MACB_BIT(PTP_PDELAY_RESP_FRAME_RECEIVED)) {
> +   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +   queue_writel(queue, ISR, 
> MACB_BIT(PTP_PDELAY_RESP_FRAME_RECEIVED));
> +   if (macb_ptp_time_peer_frame_rx_get(bp, ) != 0) {
> +   ts.tv_sec = 0;
> +   ts.tv_nsec = 0;
> +   }
> +   macb_ptp_event(bp, );
> +   }
> +
> +   if (status & MACB_BIT(PTP_PDELAY_REQ_FRAME_TRANSMITTED)) {
> +   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +   queue_writel(queue, ISR, 
> MACB_BIT(PTP_PDELAY_REQ_FRAME_TRANSMITTED));
> +   if (macb_ptp_time_peer_frame_tx_get(bp, ) != 0) {
> +   ts.tv_sec = 0;
> +   ts.tv_nsec = 0;
> +   }
> +   macb_ptp_event(bp, );
> +   }
> +
> +   if (status & MACB_BIT(PTP_PDELAY_RESP_FRAME_TRANSMITTED)) {
> +  

Re: [PATCH net-next] cadence: Add hardware PTP support.

[Nicolas Ferre]

Le 18/11/2016 à 11:47, Rafal Ozieblo a écrit :
> Signed-off-by: Rafal Ozieblo 

Note to David: This is more a RFC: we are discussing the addition of
this feature and how to cover all macb revisions and optional features
implemented in actual products.

regards,

> ---
>  Documentation/devicetree/bindings/net/macb.txt |   1 +
>  drivers/net/ethernet/cadence/macb.c| 742 
> -
>  drivers/net/ethernet/cadence/macb.h| 217 +++-
>  3 files changed, 950 insertions(+), 10 deletions(-)

[..]

-- 
Nicolas Ferre

[PATCH net-next] cadence: Add hardware PTP support.

[Rafal Ozieblo]

Signed-off-by: Rafal Ozieblo 
---
 Documentation/devicetree/bindings/net/macb.txt |   1 +
 drivers/net/ethernet/cadence/macb.c| 742 -
 drivers/net/ethernet/cadence/macb.h| 217 +++-
 3 files changed, 950 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/macb.txt 
b/Documentation/devicetree/bindings/net/macb.txt
index 1506e94..27966ae 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -22,6 +22,7 @@ Required properties:
Required elements: 'pclk', 'hclk'
Optional elements: 'tx_clk'
Optional elements: 'rx_clk' applies to cdns,zynqmp-gem
+   Optional elements: 'tsu_clk'
 - clocks: Phandles to input clocks.
 
 Optional properties for PHY child node:
diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index e1847ce..8481e4a 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -32,7 +32,10 @@
 #include 
 #include 
 #include 
-
+#include 
+#include 
+#include 
+#include 
 #include "macb.h"
 
 #define MACB_RX_BUFFER_SIZE128
@@ -665,6 +668,81 @@ static void macb_tx_error_task(struct work_struct *work)
spin_unlock_irqrestore(>lock, flags);
 }
 
+#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
+static int macb_hw_timestamp(struct macb *bp, u32 dma_desc_ts_1, u32 
dma_desc_ts_2, struct timespec64 *ts)
+{
+   struct timespec64 tsu;
+
+   ts->tv_sec = (MACB_BFEXT(DMA_TS_MSB_SEC, dma_desc_ts_2) << 
MACB_DMA_TS_LSB_SEC_SIZE) |
+   MACB_BFEXT(DMA_TS_LSB_SEC, dma_desc_ts_1);
+   ts->tv_nsec = MACB_BFEXT(DMA_TS_NSEC, dma_desc_ts_1);
+
+   /* TSU overlaping workaround
+* The timestamp only contains lower few bits of seconds,
+* so add value from 1588 timer
+*/
+   macb_ptp_time_get(bp, );
+
+   /* 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 & (MACB_DMA_TS_SEC_TOP >> 1)) && 
+   !(tsu.tv_sec & (MACB_DMA_TS_SEC_TOP >> 1)))
+   ts->tv_sec -= MACB_DMA_TS_SEC_TOP;
+
+   ts->tv_sec += ((~MACB_DMA_TS_SEC_MASK) & (tsu.tv_sec));
+
+   return 0;
+}
+
+static void macb_tstamp_tx(struct macb *bp, struct sk_buff *skb, struct 
macb_dma_desc *desc)
+{
+   struct skb_shared_hwtstamps shhwtstamps;
+   struct timespec64 ts;
+
+   if(MACB_BFEXT(DMA_TX_TS_VALID, desc->ctrl)) {
+   macb_hw_timestamp(bp, desc->dma_desc_ts_1, desc->dma_desc_ts_2, 
);
+   memset(, 0, sizeof(shhwtstamps));
+   shhwtstamps.hwtstamp = ktime_set(ts.tv_sec, ts.tv_nsec);
+   skb_tstamp_tx(skb, );
+   }
+}
+
+static void macb_tx_timestamp_flush(struct work_struct *work)
+{
+   struct macb_queue *queue = container_of(work, struct macb_queue, 
tx_timestamp_task);
+   struct macb_tx_timestamp *tx_timestamp;
+   unsigned long head = smp_load_acquire(>tx_tstamp_head);
+   unsigned long tail = queue->tx_tstamp_tail;
+
+   while (CIRC_CNT(head, tail, PTP_TS_BUFFER_SIZE)) {
+   tx_timestamp = >tx_timestamps[tail];
+   macb_tstamp_tx(queue->bp, tx_timestamp->skb, 
_timestamp->desc);
+   /* cleanup */
+   dev_kfree_skb_any(tx_timestamp->skb);
+   smp_store_release(>tx_tstamp_tail, (tail + 1) & 
(PTP_TS_BUFFER_SIZE - 1));
+   tail = queue->tx_tstamp_tail;
+   }
+}
+
+static int macb_tx_timestamp_add(struct macb_queue *queue, struct sk_buff 
*skb, struct macb_dma_desc *desc)
+{
+   struct macb_tx_timestamp *tx_timestamp;
+   unsigned long head = queue->tx_tstamp_head;
+   unsigned long tail = ACCESS_ONCE(queue->tx_tstamp_tail);
+
+   if (CIRC_SPACE(head, tail, PTP_TS_BUFFER_SIZE) == 0)
+   return -ENOMEM;
+
+   tx_timestamp = >tx_timestamps[head];
+   tx_timestamp->skb = skb;
+   memcpy(_timestamp->desc, desc, sizeof(tx_timestamp->desc));
+   smp_store_release(>tx_tstamp_head, (head + 1) & 
(PTP_TS_BUFFER_SIZE - 1));
+   return 0;
+}
+#endif
+
 static void macb_tx_interrupt(struct macb_queue *queue)
 {
unsigned int tail;
@@ -712,6 +790,16 @@ static void macb_tx_interrupt(struct macb_queue *queue)
netdev_vdbg(bp->dev, "skb %u (data %p) TX 
complete\n",
macb_tx_ring_wrap(bp, tail),
skb->data);
+#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
+   if (bp->ptp_hw_support)
+   if (macb_tx_timestamp_add(queue, skb, 
desc) == 0) {
+/* skb now belongs to 
timestamp buffer
+* and will be removed later
+