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:                                
><snip>                                                                                                  
>> @@ -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, &ts); 
>> +                               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.

>                                                                                                        
><snip>                                                                                                  
>> @@ -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, &ts) != 0) {                               
>> +                               ts.tv_sec = 0;                                                        
>> +                               ts.tv_nsec = 0;                                                       
>> +                       }                                                                             
>> +                       macb_ptp_event(bp, &ts);                                                      
>> +               }                                                                                     
>> +                                                                                                     
>> +               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, &ts) != 0) {                               
>> +                               ts.tv_sec = 0;                                                        
>> +                               ts.tv_nsec = 0;                                                       
>> +                       }                                                                             
>> +                       macb_ptp_event(bp, &ts);                                                      
>> +               }                                                                                     
>> +                                                                                                     
>> +               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, &ts) != 0) {                               
>> +                               ts.tv_sec = 0;                                                        
>> +                               ts.tv_nsec = 0;                                                       
>> +                       }
>> +                       macb_ptp_event(bp, &ts);
>> +               }
>> +
>> +               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, &ts) != 0) {
>> +                               ts.tv_sec = 0;
>> +                               ts.tv_nsec = 0;
>> +                       }
>> +                       macb_ptp_event(bp, &ts);
>> +               }
>> +
>> +               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, &ts) != 0) {
>> +                               ts.tv_sec = 0;
>> +                               ts.tv_nsec = 0;
>> +                       }
>> +                       macb_ptp_event(bp, &ts);
>> +               }
>> +
>> +               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, &ts) != 0) {
>> +                               ts.tv_sec = 0;
>> +                               ts.tv_nsec = 0;
>> +                       }
>> +                       macb_ptp_event(bp, &ts);
>> +               }
>> +
>> +               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, &ts) != 0) {
>> +                               ts.tv_sec = 0;
>> +                               ts.tv_nsec = 0;
>> +                       }
>> +                       macb_ptp_event(bp, &ts);
>> +               }
>> +
>> +               if (status & MACB_BIT(PTP_PDELAY_RESP_FRAME_TRANSMITTED)) {
>> +                       if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
>> +                               queue_writel(queue, ISR, MACB_BIT(PTP_PDELAY_RESP_FRAME_TRANSMITTED));
>> +                       if (macb_ptp_time_peer_frame_tx_get(bp, &ts) != 0) {
>> +                               ts.tv_sec = 0;
>> +                               ts.tv_nsec = 0;
>> +                       }
>> +                       macb_ptp_event(bp, &ts);
>> +               }
>> +#endif
>
>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.
>

This could be an overhead. I use linuxptp as well and it works.

>Regards,
>Harini
>


ÂBest regards,

Rafal OziebloÂÂÂ|ÂÂÂFirmware System Engineer, 
phone nbr.: +48 32Â5085469 

www.cadence.com