diff mbox series

[net-next,1/2] net: ethernet: ti: am65-cpts: Enable PTP RX HW timestamp using CPTS FIFO

Message ID 20240215110953.3225099-1-c-vankar@ti.com
State New
Headers
Series [net-next,1/2] net: ethernet: ti: am65-cpts: Enable PTP RX HW timestamp using CPTS FIFO |

Commit Message

Chintan Vankar Feb. 15, 2024, 11:09 a.m. UTC 
  CPTS module supports capturing timestamp for every packet it receives,
add a new function named "am65_cpts_rx_find_ts()" to get the timestamp
of received packets from CPTS FIFO.

Add another function named "am65_cpts_rx_timestamp()" which internally
calls "am65_cpts_rx_find_ts()" function and timestamps the received
PTP packets.

Signed-off-by: Chintan Vankar <c-vankar@ti.com>
---
 drivers/net/ethernet/ti/am65-cpts.c | 84 +++++++++++++++++++++--------
 drivers/net/ethernet/ti/am65-cpts.h | 11 ++--
 2 files changed, 67 insertions(+), 28 deletions(-)

Comments

Jacob Keller Feb. 16, 2024, 10:20 p.m. UTC | #1 
On 2/15/2024 3:09 AM, Chintan Vankar wrote:
> CPTS module supports capturing timestamp for every packet it receives,
> add a new function named "am65_cpts_rx_find_ts()" to get the timestamp
> of received packets from CPTS FIFO.
> 
> Add another function named "am65_cpts_rx_timestamp()" which internally
> calls "am65_cpts_rx_find_ts()" function and timestamps the received
> PTP packets.
> 
> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Roger Quadros Feb. 19, 2024, 10:25 a.m. UTC | #2 
On 15/02/2024 13:09, Chintan Vankar wrote:
> CPTS module supports capturing timestamp for every packet it receives,
> add a new function named "am65_cpts_rx_find_ts()" to get the timestamp
> of received packets from CPTS FIFO.
> 
> Add another function named "am65_cpts_rx_timestamp()" which internally
> calls "am65_cpts_rx_find_ts()" function and timestamps the received
> PTP packets.
> 
> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
> ---
>  drivers/net/ethernet/ti/am65-cpts.c | 84 +++++++++++++++++++++--------
>  drivers/net/ethernet/ti/am65-cpts.h | 11 ++--
>  2 files changed, 67 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c
> index c66618d91c28..92a3b40e60d6 100644
> --- a/drivers/net/ethernet/ti/am65-cpts.c
> +++ b/drivers/net/ethernet/ti/am65-cpts.c
> @@ -859,29 +859,6 @@ static long am65_cpts_ts_work(struct ptp_clock_info *ptp)
>  	return delay;
>  }
>  
> -/**
> - * am65_cpts_rx_enable - enable rx timestamping
> - * @cpts: cpts handle
> - * @en: enable
> - *
> - * This functions enables rx packets timestamping. The CPTS can timestamp all
> - * rx packets.
> - */
> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
> -{
> -	u32 val;
> -
> -	mutex_lock(&cpts->ptp_clk_lock);
> -	val = am65_cpts_read32(cpts, control);
> -	if (en)
> -		val |= AM65_CPTS_CONTROL_TSTAMP_EN;
> -	else
> -		val &= ~AM65_CPTS_CONTROL_TSTAMP_EN;
> -	am65_cpts_write32(cpts, val, control);
> -	mutex_unlock(&cpts->ptp_clk_lock);
> -}
> -EXPORT_SYMBOL_GPL(am65_cpts_rx_enable);
> -

This function is used in am65-cpsw-nuss.c so this patch will
break build.

This looks like preparation for the workaround in next patch
which affects only some platforms. So please restrict this limitation
only to those platforms that are affected.

>  static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>  {
>  	unsigned int ptp_class = ptp_classify_raw(skb);
> @@ -906,6 +883,67 @@ static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>  	return 1;
>  }
>  
> +static u64 am65_cpts_find_rx_ts(struct am65_cpts *cpts, struct sk_buff *skb,
> +				int ev_type, u32 skb_mtype_seqid)
> +{
> +	struct list_head *this, *next;
> +	struct am65_cpts_event *event;
> +	unsigned long flags;
> +	u32 mtype_seqid;
> +	u64 ns = 0;
> +
> +	am65_cpts_fifo_read(cpts);
> +	spin_lock_irqsave(&cpts->lock, flags);
> +	list_for_each_safe(this, next, &cpts->events) {
> +		event = list_entry(this, struct am65_cpts_event, list);
> +		if (time_after(jiffies, event->tmo)) {
> +			list_del_init(&event->list);
> +			list_add(&event->list, &cpts->pool);
> +			continue;
> +		}
> +
> +		mtype_seqid = event->event1 &
> +			      (AM65_CPTS_EVENT_1_MESSAGE_TYPE_MASK |
> +			       AM65_CPTS_EVENT_1_SEQUENCE_ID_MASK |
> +			       AM65_CPTS_EVENT_1_EVENT_TYPE_MASK);
> +
> +		if (mtype_seqid == skb_mtype_seqid) {
> +			ns = event->timestamp;
> +			list_del_init(&event->list);
> +			list_add(&event->list, &cpts->pool);
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&cpts->lock, flags);
> +
> +	return ns;
> +}
> +
> +void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb)
> +{
> +	struct am65_cpts_skb_cb_data *skb_cb = (struct am65_cpts_skb_cb_data *)skb->cb;
> +	struct skb_shared_hwtstamps *ssh;
> +	int ret;
> +	u64 ns;
> +
> +	ret = am65_skb_get_mtype_seqid(skb, &skb_cb->skb_mtype_seqid);
> +	if (!ret)
> +		return; /* if not PTP class packet */
> +
> +	skb_cb->skb_mtype_seqid |= (AM65_CPTS_EV_RX << AM65_CPTS_EVENT_1_EVENT_TYPE_SHIFT);
> +
> +	dev_dbg(cpts->dev, "%s mtype seqid %08x\n", __func__, skb_cb->skb_mtype_seqid);
> +
> +	ns = am65_cpts_find_rx_ts(cpts, skb, AM65_CPTS_EV_RX, skb_cb->skb_mtype_seqid);
> +	if (!ns)
> +		return;
> +
> +	ssh = skb_hwtstamps(skb);
> +	memset(ssh, 0, sizeof(*ssh));
> +	ssh->hwtstamp = ns_to_ktime(ns);
> +}
> +EXPORT_SYMBOL_GPL(am65_cpts_rx_timestamp);
> +
>  /**
>   * am65_cpts_tx_timestamp - save tx packet for timestamping
>   * @cpts: cpts handle
> diff --git a/drivers/net/ethernet/ti/am65-cpts.h b/drivers/net/ethernet/ti/am65-cpts.h
> index 6e14df0be113..6099d772799d 100644
> --- a/drivers/net/ethernet/ti/am65-cpts.h
> +++ b/drivers/net/ethernet/ti/am65-cpts.h
> @@ -22,9 +22,9 @@ void am65_cpts_release(struct am65_cpts *cpts);
>  struct am65_cpts *am65_cpts_create(struct device *dev, void __iomem *regs,
>  				   struct device_node *node);
>  int am65_cpts_phc_index(struct am65_cpts *cpts);
> +void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>  void am65_cpts_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>  void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en);
>  u64 am65_cpts_ns_gettime(struct am65_cpts *cpts);
>  int am65_cpts_estf_enable(struct am65_cpts *cpts, int idx,
>  			  struct am65_cpts_estf_cfg *cfg);
> @@ -48,17 +48,18 @@ static inline int am65_cpts_phc_index(struct am65_cpts *cpts)
>  	return -1;
>  }
>  
> -static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
> +static inline void am65_cpts_rx_timestamp(struct am65_cpts *cpts,
>  					  struct sk_buff *skb)
>  {
>  }
>  
> -static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
> -					       struct sk_buff *skb)
> +static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
> +					  struct sk_buff *skb)
>  {
>  }
>  
> -static inline void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
> +static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
> +					       struct sk_buff *skb)
>  {
>  }
>
Roger Quadros Feb. 19, 2024, 11:24 a.m. UTC | #3 
On 15/02/2024 13:09, Chintan Vankar wrote:
> CPTS module supports capturing timestamp for every packet it receives,
> add a new function named "am65_cpts_rx_find_ts()" to get the timestamp
> of received packets from CPTS FIFO.
> 
> Add another function named "am65_cpts_rx_timestamp()" which internally
> calls "am65_cpts_rx_find_ts()" function and timestamps the received
> PTP packets.
> 
> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
> ---
>  drivers/net/ethernet/ti/am65-cpts.c | 84 +++++++++++++++++++++--------
>  drivers/net/ethernet/ti/am65-cpts.h | 11 ++--
>  2 files changed, 67 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c
> index c66618d91c28..92a3b40e60d6 100644
> --- a/drivers/net/ethernet/ti/am65-cpts.c
> +++ b/drivers/net/ethernet/ti/am65-cpts.c
> @@ -859,29 +859,6 @@ static long am65_cpts_ts_work(struct ptp_clock_info *ptp)
>  	return delay;
>  }
>  
> -/**
> - * am65_cpts_rx_enable - enable rx timestamping
> - * @cpts: cpts handle
> - * @en: enable
> - *
> - * This functions enables rx packets timestamping. The CPTS can timestamp all
> - * rx packets.
> - */
> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
> -{
> -	u32 val;
> -
> -	mutex_lock(&cpts->ptp_clk_lock);
> -	val = am65_cpts_read32(cpts, control);
> -	if (en)
> -		val |= AM65_CPTS_CONTROL_TSTAMP_EN;
> -	else
> -		val &= ~AM65_CPTS_CONTROL_TSTAMP_EN;
> -	am65_cpts_write32(cpts, val, control);
> -	mutex_unlock(&cpts->ptp_clk_lock);
> -}
> -EXPORT_SYMBOL_GPL(am65_cpts_rx_enable);
> -
>  static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>  {
>  	unsigned int ptp_class = ptp_classify_raw(skb);
> @@ -906,6 +883,67 @@ static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>  	return 1;
>  }
>  
> +static u64 am65_cpts_find_rx_ts(struct am65_cpts *cpts, struct sk_buff *skb,
> +				int ev_type, u32 skb_mtype_seqid)
> +{
> +	struct list_head *this, *next;
> +	struct am65_cpts_event *event;
> +	unsigned long flags;
> +	u32 mtype_seqid;
> +	u64 ns = 0;
> +
> +	am65_cpts_fifo_read(cpts);

am65_cpts_fifo_read() is called from the CPTS interrupt handler and the
event is popped out of the FIFO and pushed into an event list.

Doesn't this race with that interrupt handler?
Can't you use that event list instead of reading cpts_fifo directly?

Something like am65_cpts_match_tx_ts()?

> +	spin_lock_irqsave(&cpts->lock, flags);
> +	list_for_each_safe(this, next, &cpts->events) {
> +		event = list_entry(this, struct am65_cpts_event, list);
> +		if (time_after(jiffies, event->tmo)) {
> +			list_del_init(&event->list);
> +			list_add(&event->list, &cpts->pool);
> +			continue;
> +		}
> +
> +		mtype_seqid = event->event1 &
> +			      (AM65_CPTS_EVENT_1_MESSAGE_TYPE_MASK |
> +			       AM65_CPTS_EVENT_1_SEQUENCE_ID_MASK |
> +			       AM65_CPTS_EVENT_1_EVENT_TYPE_MASK);
> +
> +		if (mtype_seqid == skb_mtype_seqid) {
> +			ns = event->timestamp;
> +			list_del_init(&event->list);
> +			list_add(&event->list, &cpts->pool);
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&cpts->lock, flags);
> +
> +	return ns;
> +}
> +
> +void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb)
> +{
> +	struct am65_cpts_skb_cb_data *skb_cb = (struct am65_cpts_skb_cb_data *)skb->cb;
> +	struct skb_shared_hwtstamps *ssh;
> +	int ret;
> +	u64 ns;
> +
> +	ret = am65_skb_get_mtype_seqid(skb, &skb_cb->skb_mtype_seqid);
> +	if (!ret)
> +		return; /* if not PTP class packet */
> +
> +	skb_cb->skb_mtype_seqid |= (AM65_CPTS_EV_RX << AM65_CPTS_EVENT_1_EVENT_TYPE_SHIFT);
> +
> +	dev_dbg(cpts->dev, "%s mtype seqid %08x\n", __func__, skb_cb->skb_mtype_seqid);
> +
> +	ns = am65_cpts_find_rx_ts(cpts, skb, AM65_CPTS_EV_RX, skb_cb->skb_mtype_seqid);
> +	if (!ns)
> +		return;
> +
> +	ssh = skb_hwtstamps(skb);
> +	memset(ssh, 0, sizeof(*ssh));
> +	ssh->hwtstamp = ns_to_ktime(ns);
> +}
> +EXPORT_SYMBOL_GPL(am65_cpts_rx_timestamp);
> +
>  /**
>   * am65_cpts_tx_timestamp - save tx packet for timestamping
>   * @cpts: cpts handle
> diff --git a/drivers/net/ethernet/ti/am65-cpts.h b/drivers/net/ethernet/ti/am65-cpts.h
> index 6e14df0be113..6099d772799d 100644
> --- a/drivers/net/ethernet/ti/am65-cpts.h
> +++ b/drivers/net/ethernet/ti/am65-cpts.h
> @@ -22,9 +22,9 @@ void am65_cpts_release(struct am65_cpts *cpts);
>  struct am65_cpts *am65_cpts_create(struct device *dev, void __iomem *regs,
>  				   struct device_node *node);
>  int am65_cpts_phc_index(struct am65_cpts *cpts);
> +void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>  void am65_cpts_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>  void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en);
>  u64 am65_cpts_ns_gettime(struct am65_cpts *cpts);
>  int am65_cpts_estf_enable(struct am65_cpts *cpts, int idx,
>  			  struct am65_cpts_estf_cfg *cfg);
> @@ -48,17 +48,18 @@ static inline int am65_cpts_phc_index(struct am65_cpts *cpts)
>  	return -1;
>  }
>  
> -static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
> +static inline void am65_cpts_rx_timestamp(struct am65_cpts *cpts,
>  					  struct sk_buff *skb)
>  {
>  }
>  
> -static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
> -					       struct sk_buff *skb)
> +static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
> +					  struct sk_buff *skb)
>  {
>  }
>  
> -static inline void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
> +static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
> +					       struct sk_buff *skb)
>  {
>  }
>
Chintan Vankar Feb. 26, 2024, 9:08 a.m. UTC | #4 
On 19/02/24 16:54, Roger Quadros wrote:
>
> On 15/02/2024 13:09, Chintan Vankar wrote:
>> CPTS module supports capturing timestamp for every packet it receives,
>> add a new function named "am65_cpts_rx_find_ts()" to get the timestamp
>> of received packets from CPTS FIFO.
>>
>> Add another function named "am65_cpts_rx_timestamp()" which internally
>> calls "am65_cpts_rx_find_ts()" function and timestamps the received
>> PTP packets.
>>
>> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
>> ---
>>   drivers/net/ethernet/ti/am65-cpts.c | 84 +++++++++++++++++++++--------
>>   drivers/net/ethernet/ti/am65-cpts.h | 11 ++--
>>   2 files changed, 67 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c
>> index c66618d91c28..92a3b40e60d6 100644
>> --- a/drivers/net/ethernet/ti/am65-cpts.c
>> +++ b/drivers/net/ethernet/ti/am65-cpts.c
>> @@ -859,29 +859,6 @@ static long am65_cpts_ts_work(struct ptp_clock_info *ptp)
>>   	return delay;
>>   }
>>   
>> -/**
>> - * am65_cpts_rx_enable - enable rx timestamping
>> - * @cpts: cpts handle
>> - * @en: enable
>> - *
>> - * This functions enables rx packets timestamping. The CPTS can timestamp all
>> - * rx packets.
>> - */
>> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
>> -{
>> -	u32 val;
>> -
>> -	mutex_lock(&cpts->ptp_clk_lock);
>> -	val = am65_cpts_read32(cpts, control);
>> -	if (en)
>> -		val |= AM65_CPTS_CONTROL_TSTAMP_EN;
>> -	else
>> -		val &= ~AM65_CPTS_CONTROL_TSTAMP_EN;
>> -	am65_cpts_write32(cpts, val, control);
>> -	mutex_unlock(&cpts->ptp_clk_lock);
>> -}
>> -EXPORT_SYMBOL_GPL(am65_cpts_rx_enable);
>> -
>>   static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>>   {
>>   	unsigned int ptp_class = ptp_classify_raw(skb);
>> @@ -906,6 +883,67 @@ static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>>   	return 1;
>>   }
>>   
>> +static u64 am65_cpts_find_rx_ts(struct am65_cpts *cpts, struct sk_buff *skb,
>> +				int ev_type, u32 skb_mtype_seqid)
>> +{
>> +	struct list_head *this, *next;
>> +	struct am65_cpts_event *event;
>> +	unsigned long flags;
>> +	u32 mtype_seqid;
>> +	u64 ns = 0;
>> +
>> +	am65_cpts_fifo_read(cpts);
> am65_cpts_fifo_read() is called from the CPTS interrupt handler and the
> event is popped out of the FIFO and pushed into an event list.
>
> Doesn't this race with that interrupt handler?
Could you clarify why there be a race condition ?
> Can't you use that event list instead of reading cpts_fifo directly?
>
> Something like am65_cpts_match_tx_ts()?
>
>> +	spin_lock_irqsave(&cpts->lock, flags);
>> +	list_for_each_safe(this, next, &cpts->events) {
>> +		event = list_entry(this, struct am65_cpts_event, list);
>> +		if (time_after(jiffies, event->tmo)) {
>> +			list_del_init(&event->list);
>> +			list_add(&event->list, &cpts->pool);
>> +			continue;
>> +		}
>> +
>> +		mtype_seqid = event->event1 &
>> +			      (AM65_CPTS_EVENT_1_MESSAGE_TYPE_MASK |
>> +			       AM65_CPTS_EVENT_1_SEQUENCE_ID_MASK |
>> +			       AM65_CPTS_EVENT_1_EVENT_TYPE_MASK);
>> +
>> +		if (mtype_seqid == skb_mtype_seqid) {
>> +			ns = event->timestamp;
>> +			list_del_init(&event->list);
>> +			list_add(&event->list, &cpts->pool);
>> +			break;
>> +		}
>> +	}
>> +	spin_unlock_irqrestore(&cpts->lock, flags);
>> +
>> +	return ns;
>> +}
>> +
>> +void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb)
>> +{
>> +	struct am65_cpts_skb_cb_data *skb_cb = (struct am65_cpts_skb_cb_data *)skb->cb;
>> +	struct skb_shared_hwtstamps *ssh;
>> +	int ret;
>> +	u64 ns;
>> +
>> +	ret = am65_skb_get_mtype_seqid(skb, &skb_cb->skb_mtype_seqid);
>> +	if (!ret)
>> +		return; /* if not PTP class packet */
>> +
>> +	skb_cb->skb_mtype_seqid |= (AM65_CPTS_EV_RX << AM65_CPTS_EVENT_1_EVENT_TYPE_SHIFT);
>> +
>> +	dev_dbg(cpts->dev, "%s mtype seqid %08x\n", __func__, skb_cb->skb_mtype_seqid);
>> +
>> +	ns = am65_cpts_find_rx_ts(cpts, skb, AM65_CPTS_EV_RX, skb_cb->skb_mtype_seqid);
>> +	if (!ns)
>> +		return;
>> +
>> +	ssh = skb_hwtstamps(skb);
>> +	memset(ssh, 0, sizeof(*ssh));
>> +	ssh->hwtstamp = ns_to_ktime(ns);
>> +}
>> +EXPORT_SYMBOL_GPL(am65_cpts_rx_timestamp);
>> +
>>   /**
>>    * am65_cpts_tx_timestamp - save tx packet for timestamping
>>    * @cpts: cpts handle
>> diff --git a/drivers/net/ethernet/ti/am65-cpts.h b/drivers/net/ethernet/ti/am65-cpts.h
>> index 6e14df0be113..6099d772799d 100644
>> --- a/drivers/net/ethernet/ti/am65-cpts.h
>> +++ b/drivers/net/ethernet/ti/am65-cpts.h
>> @@ -22,9 +22,9 @@ void am65_cpts_release(struct am65_cpts *cpts);
>>   struct am65_cpts *am65_cpts_create(struct device *dev, void __iomem *regs,
>>   				   struct device_node *node);
>>   int am65_cpts_phc_index(struct am65_cpts *cpts);
>> +void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>>   void am65_cpts_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>>   void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en);
>>   u64 am65_cpts_ns_gettime(struct am65_cpts *cpts);
>>   int am65_cpts_estf_enable(struct am65_cpts *cpts, int idx,
>>   			  struct am65_cpts_estf_cfg *cfg);
>> @@ -48,17 +48,18 @@ static inline int am65_cpts_phc_index(struct am65_cpts *cpts)
>>   	return -1;
>>   }
>>   
>> -static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
>> +static inline void am65_cpts_rx_timestamp(struct am65_cpts *cpts,
>>   					  struct sk_buff *skb)
>>   {
>>   }
>>   
>> -static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
>> -					       struct sk_buff *skb)
>> +static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
>> +					  struct sk_buff *skb)
>>   {
>>   }
>>   
>> -static inline void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
>> +static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
>> +					       struct sk_buff *skb)
>>   {
>>   }
>>
Roger Quadros Feb. 26, 2024, 12:49 p.m. UTC | #5 
On 26/02/2024 11:08, Chintan Vankar wrote:
> 
> On 19/02/24 16:54, Roger Quadros wrote:
>>
>> On 15/02/2024 13:09, Chintan Vankar wrote:
>>> CPTS module supports capturing timestamp for every packet it receives,
>>> add a new function named "am65_cpts_rx_find_ts()" to get the timestamp
>>> of received packets from CPTS FIFO.
>>>
>>> Add another function named "am65_cpts_rx_timestamp()" which internally
>>> calls "am65_cpts_rx_find_ts()" function and timestamps the received
>>> PTP packets.
>>>
>>> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
>>> ---
>>>   drivers/net/ethernet/ti/am65-cpts.c | 84 +++++++++++++++++++++--------
>>>   drivers/net/ethernet/ti/am65-cpts.h | 11 ++--
>>>   2 files changed, 67 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c
>>> index c66618d91c28..92a3b40e60d6 100644
>>> --- a/drivers/net/ethernet/ti/am65-cpts.c
>>> +++ b/drivers/net/ethernet/ti/am65-cpts.c
>>> @@ -859,29 +859,6 @@ static long am65_cpts_ts_work(struct ptp_clock_info *ptp)
>>>       return delay;
>>>   }
>>>   -/**
>>> - * am65_cpts_rx_enable - enable rx timestamping
>>> - * @cpts: cpts handle
>>> - * @en: enable
>>> - *
>>> - * This functions enables rx packets timestamping. The CPTS can timestamp all
>>> - * rx packets.
>>> - */
>>> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
>>> -{
>>> -    u32 val;
>>> -
>>> -    mutex_lock(&cpts->ptp_clk_lock);
>>> -    val = am65_cpts_read32(cpts, control);
>>> -    if (en)
>>> -        val |= AM65_CPTS_CONTROL_TSTAMP_EN;
>>> -    else
>>> -        val &= ~AM65_CPTS_CONTROL_TSTAMP_EN;
>>> -    am65_cpts_write32(cpts, val, control);
>>> -    mutex_unlock(&cpts->ptp_clk_lock);
>>> -}
>>> -EXPORT_SYMBOL_GPL(am65_cpts_rx_enable);
>>> -
>>>   static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>>>   {
>>>       unsigned int ptp_class = ptp_classify_raw(skb);
>>> @@ -906,6 +883,67 @@ static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>>>       return 1;
>>>   }
>>>   +static u64 am65_cpts_find_rx_ts(struct am65_cpts *cpts, struct sk_buff *skb,
>>> +                int ev_type, u32 skb_mtype_seqid)
>>> +{
>>> +    struct list_head *this, *next;
>>> +    struct am65_cpts_event *event;
>>> +    unsigned long flags;
>>> +    u32 mtype_seqid;
>>> +    u64 ns = 0;
>>> +
>>> +    am65_cpts_fifo_read(cpts);
>> am65_cpts_fifo_read() is called from the CPTS interrupt handler and the
>> event is popped out of the FIFO and pushed into an event list.
>>
>> Doesn't this race with that interrupt handler?
> Could you clarify why there be a race condition ?

I'm not sure so was asking. The question is, do you have to call
am65_cpts_fifo_read() here? If yes, could you please add a comment why.

>> Can't you use that event list instead of reading cpts_fifo directly?
>>
>> Something like am65_cpts_match_tx_ts()?
>>
>>> +    spin_lock_irqsave(&cpts->lock, flags);
>>> +    list_for_each_safe(this, next, &cpts->events) {
>>> +        event = list_entry(this, struct am65_cpts_event, list);
>>> +        if (time_after(jiffies, event->tmo)) {
>>> +            list_del_init(&event->list);
>>> +            list_add(&event->list, &cpts->pool);
>>> +            continue;
>>> +        }
>>> +
>>> +        mtype_seqid = event->event1 &
>>> +                  (AM65_CPTS_EVENT_1_MESSAGE_TYPE_MASK |
>>> +                   AM65_CPTS_EVENT_1_SEQUENCE_ID_MASK |
>>> +                   AM65_CPTS_EVENT_1_EVENT_TYPE_MASK);
>>> +
>>> +        if (mtype_seqid == skb_mtype_seqid) {
>>> +            ns = event->timestamp;
>>> +            list_del_init(&event->list);
>>> +            list_add(&event->list, &cpts->pool);
>>> +            break;
>>> +        }
>>> +    }
>>> +    spin_unlock_irqrestore(&cpts->lock, flags);
>>> +
>>> +    return ns;
>>> +}
>>> +
>>> +void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb)
>>> +{
>>> +    struct am65_cpts_skb_cb_data *skb_cb = (struct am65_cpts_skb_cb_data *)skb->cb;
>>> +    struct skb_shared_hwtstamps *ssh;
>>> +    int ret;
>>> +    u64 ns;
>>> +
>>> +    ret = am65_skb_get_mtype_seqid(skb, &skb_cb->skb_mtype_seqid);
>>> +    if (!ret)
>>> +        return; /* if not PTP class packet */
>>> +
>>> +    skb_cb->skb_mtype_seqid |= (AM65_CPTS_EV_RX << AM65_CPTS_EVENT_1_EVENT_TYPE_SHIFT);
>>> +
>>> +    dev_dbg(cpts->dev, "%s mtype seqid %08x\n", __func__, skb_cb->skb_mtype_seqid);
>>> +
>>> +    ns = am65_cpts_find_rx_ts(cpts, skb, AM65_CPTS_EV_RX, skb_cb->skb_mtype_seqid);
>>> +    if (!ns)
>>> +        return;
>>> +
>>> +    ssh = skb_hwtstamps(skb);
>>> +    memset(ssh, 0, sizeof(*ssh));
>>> +    ssh->hwtstamp = ns_to_ktime(ns);
>>> +}
>>> +EXPORT_SYMBOL_GPL(am65_cpts_rx_timestamp);
>>> +
>>>   /**
>>>    * am65_cpts_tx_timestamp - save tx packet for timestamping
>>>    * @cpts: cpts handle
>>> diff --git a/drivers/net/ethernet/ti/am65-cpts.h b/drivers/net/ethernet/ti/am65-cpts.h
>>> index 6e14df0be113..6099d772799d 100644
>>> --- a/drivers/net/ethernet/ti/am65-cpts.h
>>> +++ b/drivers/net/ethernet/ti/am65-cpts.h
>>> @@ -22,9 +22,9 @@ void am65_cpts_release(struct am65_cpts *cpts);
>>>   struct am65_cpts *am65_cpts_create(struct device *dev, void __iomem *regs,
>>>                      struct device_node *node);
>>>   int am65_cpts_phc_index(struct am65_cpts *cpts);
>>> +void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>>>   void am65_cpts_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>>>   void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>>> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en);
>>>   u64 am65_cpts_ns_gettime(struct am65_cpts *cpts);
>>>   int am65_cpts_estf_enable(struct am65_cpts *cpts, int idx,
>>>                 struct am65_cpts_estf_cfg *cfg);
>>> @@ -48,17 +48,18 @@ static inline int am65_cpts_phc_index(struct am65_cpts *cpts)
>>>       return -1;
>>>   }
>>>   -static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
>>> +static inline void am65_cpts_rx_timestamp(struct am65_cpts *cpts,
>>>                         struct sk_buff *skb)
>>>   {
>>>   }
>>>   -static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
>>> -                           struct sk_buff *skb)
>>> +static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
>>> +                      struct sk_buff *skb)
>>>   {
>>>   }
>>>   -static inline void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
>>> +static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
>>> +                           struct sk_buff *skb)
>>>   {
>>>   }
>>>
Chintan Vankar March 1, 2024, 11:06 a.m. UTC | #6 
On 26/02/24 18:19, Roger Quadros wrote:
> 
> 
> On 26/02/2024 11:08, Chintan Vankar wrote:
>>
>> On 19/02/24 16:54, Roger Quadros wrote:
>>>
>>> On 15/02/2024 13:09, Chintan Vankar wrote:
>>>> CPTS module supports capturing timestamp for every packet it receives,
>>>> add a new function named "am65_cpts_rx_find_ts()" to get the timestamp
>>>> of received packets from CPTS FIFO.
>>>>
>>>> Add another function named "am65_cpts_rx_timestamp()" which internally
>>>> calls "am65_cpts_rx_find_ts()" function and timestamps the received
>>>> PTP packets.
>>>>
>>>> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
>>>> ---
>>>>    drivers/net/ethernet/ti/am65-cpts.c | 84 +++++++++++++++++++++--------
>>>>    drivers/net/ethernet/ti/am65-cpts.h | 11 ++--
>>>>    2 files changed, 67 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c
>>>> index c66618d91c28..92a3b40e60d6 100644
>>>> --- a/drivers/net/ethernet/ti/am65-cpts.c
>>>> +++ b/drivers/net/ethernet/ti/am65-cpts.c
>>>> @@ -859,29 +859,6 @@ static long am65_cpts_ts_work(struct ptp_clock_info *ptp)
>>>>        return delay;
>>>>    }
>>>>    -/**
>>>> - * am65_cpts_rx_enable - enable rx timestamping
>>>> - * @cpts: cpts handle
>>>> - * @en: enable
>>>> - *
>>>> - * This functions enables rx packets timestamping. The CPTS can timestamp all
>>>> - * rx packets.
>>>> - */
>>>> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
>>>> -{
>>>> -    u32 val;
>>>> -
>>>> -    mutex_lock(&cpts->ptp_clk_lock);
>>>> -    val = am65_cpts_read32(cpts, control);
>>>> -    if (en)
>>>> -        val |= AM65_CPTS_CONTROL_TSTAMP_EN;
>>>> -    else
>>>> -        val &= ~AM65_CPTS_CONTROL_TSTAMP_EN;
>>>> -    am65_cpts_write32(cpts, val, control);
>>>> -    mutex_unlock(&cpts->ptp_clk_lock);
>>>> -}
>>>> -EXPORT_SYMBOL_GPL(am65_cpts_rx_enable);
>>>> -
>>>>    static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>>>>    {
>>>>        unsigned int ptp_class = ptp_classify_raw(skb);
>>>> @@ -906,6 +883,67 @@ static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>>>>        return 1;
>>>>    }
>>>>    +static u64 am65_cpts_find_rx_ts(struct am65_cpts *cpts, struct sk_buff *skb,
>>>> +                int ev_type, u32 skb_mtype_seqid)
>>>> +{
>>>> +    struct list_head *this, *next;
>>>> +    struct am65_cpts_event *event;
>>>> +    unsigned long flags;
>>>> +    u32 mtype_seqid;
>>>> +    u64 ns = 0;
>>>> +
>>>> +    am65_cpts_fifo_read(cpts);
>>> am65_cpts_fifo_read() is called from the CPTS interrupt handler and the
>>> event is popped out of the FIFO and pushed into an event list.
>>>
>>> Doesn't this race with that interrupt handler?
>> Could you clarify why there be a race condition ?
> 
> I'm not sure so was asking. The question is, do you have to call
> am65_cpts_fifo_read() here? If yes, could you please add a comment why.

Yes. We need to call "am65_cpts_fifo_read()" here. If we don't call it
here then there will be a race condition for the event to be added to
the list before "am65_cpts_get_rx_ts()" function is called.

If we have to implement it without invoking "am65_cpts_fifo_read()",
then there has to be a check to ensure that the interrupt associated
with the RX Event has successfully added that event to the list.
Otherwise, due to the nature of "spin_lock_irqsave", when the interrupt
has acquired the lock on one CPU, the "am65_cpts_get_rx_ts()" function
running on another CPU might acquire the lock from the interrupt and
search for the RX Event which the "am65_cpts_fifo_read()" invoked by
the interrupt was supposed to add in the event list. This is racy due
to which it is possible that the event is not yet added when
"am65_cpts_get_rx_ts()" is invoked. Therefore, to ensure that the
event is surely present, invoking "am65_cpts_fifo_read()" is
required.

> 
>>> Can't you use that event list instead of reading cpts_fifo directly?
>>>
>>> Something like am65_cpts_match_tx_ts()?
>>>
>>>> +    spin_lock_irqsave(&cpts->lock, flags);
>>>> +    list_for_each_safe(this, next, &cpts->events) {
>>>> +        event = list_entry(this, struct am65_cpts_event, list);
>>>> +        if (time_after(jiffies, event->tmo)) {
>>>> +            list_del_init(&event->list);
>>>> +            list_add(&event->list, &cpts->pool);
>>>> +            continue;
>>>> +        }
>>>> +
>>>> +        mtype_seqid = event->event1 &
>>>> +                  (AM65_CPTS_EVENT_1_MESSAGE_TYPE_MASK |
>>>> +                   AM65_CPTS_EVENT_1_SEQUENCE_ID_MASK |
>>>> +                   AM65_CPTS_EVENT_1_EVENT_TYPE_MASK);
>>>> +
>>>> +        if (mtype_seqid == skb_mtype_seqid) {
>>>> +            ns = event->timestamp;
>>>> +            list_del_init(&event->list);
>>>> +            list_add(&event->list, &cpts->pool);
>>>> +            break;
>>>> +        }
>>>> +    }
>>>> +    spin_unlock_irqrestore(&cpts->lock, flags);
>>>> +
>>>> +    return ns;
>>>> +}
>>>> +
>>>> +void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb)
>>>> +{
>>>> +    struct am65_cpts_skb_cb_data *skb_cb = (struct am65_cpts_skb_cb_data *)skb->cb;
>>>> +    struct skb_shared_hwtstamps *ssh;
>>>> +    int ret;
>>>> +    u64 ns;
>>>> +
>>>> +    ret = am65_skb_get_mtype_seqid(skb, &skb_cb->skb_mtype_seqid);
>>>> +    if (!ret)
>>>> +        return; /* if not PTP class packet */
>>>> +
>>>> +    skb_cb->skb_mtype_seqid |= (AM65_CPTS_EV_RX << AM65_CPTS_EVENT_1_EVENT_TYPE_SHIFT);
>>>> +
>>>> +    dev_dbg(cpts->dev, "%s mtype seqid %08x\n", __func__, skb_cb->skb_mtype_seqid);
>>>> +
>>>> +    ns = am65_cpts_find_rx_ts(cpts, skb, AM65_CPTS_EV_RX, skb_cb->skb_mtype_seqid);
>>>> +    if (!ns)
>>>> +        return;
>>>> +
>>>> +    ssh = skb_hwtstamps(skb);
>>>> +    memset(ssh, 0, sizeof(*ssh));
>>>> +    ssh->hwtstamp = ns_to_ktime(ns);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(am65_cpts_rx_timestamp);
>>>> +
>>>>    /**
>>>>     * am65_cpts_tx_timestamp - save tx packet for timestamping
>>>>     * @cpts: cpts handle
>>>> diff --git a/drivers/net/ethernet/ti/am65-cpts.h b/drivers/net/ethernet/ti/am65-cpts.h
>>>> index 6e14df0be113..6099d772799d 100644
>>>> --- a/drivers/net/ethernet/ti/am65-cpts.h
>>>> +++ b/drivers/net/ethernet/ti/am65-cpts.h
>>>> @@ -22,9 +22,9 @@ void am65_cpts_release(struct am65_cpts *cpts);
>>>>    struct am65_cpts *am65_cpts_create(struct device *dev, void __iomem *regs,
>>>>                       struct device_node *node);
>>>>    int am65_cpts_phc_index(struct am65_cpts *cpts);
>>>> +void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>>>>    void am65_cpts_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>>>>    void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>>>> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en);
>>>>    u64 am65_cpts_ns_gettime(struct am65_cpts *cpts);
>>>>    int am65_cpts_estf_enable(struct am65_cpts *cpts, int idx,
>>>>                  struct am65_cpts_estf_cfg *cfg);
>>>> @@ -48,17 +48,18 @@ static inline int am65_cpts_phc_index(struct am65_cpts *cpts)
>>>>        return -1;
>>>>    }
>>>>    -static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
>>>> +static inline void am65_cpts_rx_timestamp(struct am65_cpts *cpts,
>>>>                          struct sk_buff *skb)
>>>>    {
>>>>    }
>>>>    -static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
>>>> -                           struct sk_buff *skb)
>>>> +static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
>>>> +                      struct sk_buff *skb)
>>>>    {
>>>>    }
>>>>    -static inline void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
>>>> +static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
>>>> +                           struct sk_buff *skb)
>>>>    {
>>>>    }
>>>>    
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c
index c66618d91c28..92a3b40e60d6 100644
--- a/drivers/net/ethernet/ti/am65-cpts.c
+++ b/drivers/net/ethernet/ti/am65-cpts.c
@@ -859,29 +859,6 @@  static long am65_cpts_ts_work(struct ptp_clock_info *ptp)
 	return delay;
 }
 
-/**
- * am65_cpts_rx_enable - enable rx timestamping
- * @cpts: cpts handle
- * @en: enable
- *
- * This functions enables rx packets timestamping. The CPTS can timestamp all
- * rx packets.
- */
-void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
-{
-	u32 val;
-
-	mutex_lock(&cpts->ptp_clk_lock);
-	val = am65_cpts_read32(cpts, control);
-	if (en)
-		val |= AM65_CPTS_CONTROL_TSTAMP_EN;
-	else
-		val &= ~AM65_CPTS_CONTROL_TSTAMP_EN;
-	am65_cpts_write32(cpts, val, control);
-	mutex_unlock(&cpts->ptp_clk_lock);
-}
-EXPORT_SYMBOL_GPL(am65_cpts_rx_enable);
-
 static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
 {
 	unsigned int ptp_class = ptp_classify_raw(skb);
@@ -906,6 +883,67 @@  static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
 	return 1;
 }
 
+static u64 am65_cpts_find_rx_ts(struct am65_cpts *cpts, struct sk_buff *skb,
+				int ev_type, u32 skb_mtype_seqid)
+{
+	struct list_head *this, *next;
+	struct am65_cpts_event *event;
+	unsigned long flags;
+	u32 mtype_seqid;
+	u64 ns = 0;
+
+	am65_cpts_fifo_read(cpts);
+	spin_lock_irqsave(&cpts->lock, flags);
+	list_for_each_safe(this, next, &cpts->events) {
+		event = list_entry(this, struct am65_cpts_event, list);
+		if (time_after(jiffies, event->tmo)) {
+			list_del_init(&event->list);
+			list_add(&event->list, &cpts->pool);
+			continue;
+		}
+
+		mtype_seqid = event->event1 &
+			      (AM65_CPTS_EVENT_1_MESSAGE_TYPE_MASK |
+			       AM65_CPTS_EVENT_1_SEQUENCE_ID_MASK |
+			       AM65_CPTS_EVENT_1_EVENT_TYPE_MASK);
+
+		if (mtype_seqid == skb_mtype_seqid) {
+			ns = event->timestamp;
+			list_del_init(&event->list);
+			list_add(&event->list, &cpts->pool);
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&cpts->lock, flags);
+
+	return ns;
+}
+
+void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb)
+{
+	struct am65_cpts_skb_cb_data *skb_cb = (struct am65_cpts_skb_cb_data *)skb->cb;
+	struct skb_shared_hwtstamps *ssh;
+	int ret;
+	u64 ns;
+
+	ret = am65_skb_get_mtype_seqid(skb, &skb_cb->skb_mtype_seqid);
+	if (!ret)
+		return; /* if not PTP class packet */
+
+	skb_cb->skb_mtype_seqid |= (AM65_CPTS_EV_RX << AM65_CPTS_EVENT_1_EVENT_TYPE_SHIFT);
+
+	dev_dbg(cpts->dev, "%s mtype seqid %08x\n", __func__, skb_cb->skb_mtype_seqid);
+
+	ns = am65_cpts_find_rx_ts(cpts, skb, AM65_CPTS_EV_RX, skb_cb->skb_mtype_seqid);
+	if (!ns)
+		return;
+
+	ssh = skb_hwtstamps(skb);
+	memset(ssh, 0, sizeof(*ssh));
+	ssh->hwtstamp = ns_to_ktime(ns);
+}
+EXPORT_SYMBOL_GPL(am65_cpts_rx_timestamp);
+
 /**
  * am65_cpts_tx_timestamp - save tx packet for timestamping
  * @cpts: cpts handle
diff --git a/drivers/net/ethernet/ti/am65-cpts.h b/drivers/net/ethernet/ti/am65-cpts.h
index 6e14df0be113..6099d772799d 100644
--- a/drivers/net/ethernet/ti/am65-cpts.h
+++ b/drivers/net/ethernet/ti/am65-cpts.h
@@ -22,9 +22,9 @@  void am65_cpts_release(struct am65_cpts *cpts);
 struct am65_cpts *am65_cpts_create(struct device *dev, void __iomem *regs,
 				   struct device_node *node);
 int am65_cpts_phc_index(struct am65_cpts *cpts);
+void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
 void am65_cpts_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
 void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
-void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en);
 u64 am65_cpts_ns_gettime(struct am65_cpts *cpts);
 int am65_cpts_estf_enable(struct am65_cpts *cpts, int idx,
 			  struct am65_cpts_estf_cfg *cfg);
@@ -48,17 +48,18 @@  static inline int am65_cpts_phc_index(struct am65_cpts *cpts)
 	return -1;
 }
 
-static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
+static inline void am65_cpts_rx_timestamp(struct am65_cpts *cpts,
 					  struct sk_buff *skb)
 {
 }
 
-static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
-					       struct sk_buff *skb)
+static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts,
+					  struct sk_buff *skb)
 {
 }
 
-static inline void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
+static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
+					       struct sk_buff *skb)
 {
 }