| Message ID | 20240402114405.219100-2-c-vankar@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Enable RX HW timestamp for PTP packets using CPTS FIFO | expand |
On Tue, 2024-04-02 at 17:14 +0530, Chintan Vankar wrote: > Add a new function "am65_cpts_rx_timestamp()" which checks for PTP > packets from header and timestamps them. > > Add another function "am65_cpts_find_rx_ts()" which finds CPTS FIFO > Event to get the timestamp of received PTP packet. > > Signed-off-by: Chintan Vankar <c-vankar@ti.com> > --- > > Link to v4: > https://lore.kernel.org/r/20240327054234.1906957-1-c-vankar@ti.com/ > > Changes from v4 to v5: > - Updated commit message. > - Replaced "list_del_entry()" and "list_add()" functions with equivalent > "list_move()" function. > > drivers/net/ethernet/ti/am65-cpts.c | 64 +++++++++++++++++++++++++++++ > drivers/net/ethernet/ti/am65-cpts.h | 6 +++ > 2 files changed, 70 insertions(+) > > diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c > index c66618d91c28..bc0bfda1db12 100644 > --- a/drivers/net/ethernet/ti/am65-cpts.c > +++ b/drivers/net/ethernet/ti/am65-cpts.c > @@ -906,6 +906,70 @@ 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, 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); am65_cpts_fifo_read() acquires and releases this same lock. If moving to a lockless schema is too complex, you should at least try to acquire the lock only once. e.g. factor out a lockless __am65_cpts_fifo_read variant and explicitly acquire the lock before invoke it. > + 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); Jakub suggested to use list_move() in v4, you should apply that here, too. Cheers, Paolo
On 04/04/24 16:10, Paolo Abeni wrote: > On Tue, 2024-04-02 at 17:14 +0530, Chintan Vankar wrote: >> Add a new function "am65_cpts_rx_timestamp()" which checks for PTP >> packets from header and timestamps them. >> >> Add another function "am65_cpts_find_rx_ts()" which finds CPTS FIFO >> Event to get the timestamp of received PTP packet. >> >> Signed-off-by: Chintan Vankar <c-vankar@ti.com> >> --- >> >> Link to v4: >> https://lore.kernel.org/r/20240327054234.1906957-1-c-vankar@ti.com/ >> >> Changes from v4 to v5: >> - Updated commit message. >> - Replaced "list_del_entry()" and "list_add()" functions with equivalent >> "list_move()" function. >> >> drivers/net/ethernet/ti/am65-cpts.c | 64 +++++++++++++++++++++++++++++ >> drivers/net/ethernet/ti/am65-cpts.h | 6 +++ >> 2 files changed, 70 insertions(+) >> >> diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c >> index c66618d91c28..bc0bfda1db12 100644 >> --- a/drivers/net/ethernet/ti/am65-cpts.c >> +++ b/drivers/net/ethernet/ti/am65-cpts.c >> @@ -906,6 +906,70 @@ 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, 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); > > am65_cpts_fifo_read() acquires and releases this same lock. If moving > to a lockless schema is too complex, you should at least try to acquire > the lock only once. e.g. factor out a lockless __am65_cpts_fifo_read > variant and explicitly acquire the lock before invoke it. > Moving to a lockless schema is complex for now, but I will make the changes suggested by you in next version. >> + 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); > > Jakub suggested to use list_move() in v4, you should apply that here, > too. > Okay. I will update that too. > Cheers, > > Paolo > >
diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c index c66618d91c28..bc0bfda1db12 100644 --- a/drivers/net/ethernet/ti/am65-cpts.c +++ b/drivers/net/ethernet/ti/am65-cpts.c @@ -906,6 +906,70 @@ 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, 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_move(&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; + + /* am65_cpts_rx_timestamp() is called before eth_type_trans(), so + * skb MAC Hdr properties are not configured yet. Hence need to + * reset skb MAC header here + */ + skb_reset_mac_header(skb); + 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_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..90296968a75c 100644 --- a/drivers/net/ethernet/ti/am65-cpts.h +++ b/drivers/net/ethernet/ti/am65-cpts.h @@ -22,6 +22,7 @@ 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); @@ -48,6 +49,11 @@ static inline int am65_cpts_phc_index(struct am65_cpts *cpts) return -1; } +static inline void am65_cpts_rx_timestamp(struct am65_cpts *cpts, + struct sk_buff *skb) +{ +} + static inline void am65_cpts_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb) {
Add a new function "am65_cpts_rx_timestamp()" which checks for PTP packets from header and timestamps them. Add another function "am65_cpts_find_rx_ts()" which finds CPTS FIFO Event to get the timestamp of received PTP packet. Signed-off-by: Chintan Vankar <c-vankar@ti.com> --- Link to v4: https://lore.kernel.org/r/20240327054234.1906957-1-c-vankar@ti.com/ Changes from v4 to v5: - Updated commit message. - Replaced "list_del_entry()" and "list_add()" functions with equivalent "list_move()" function. drivers/net/ethernet/ti/am65-cpts.c | 64 +++++++++++++++++++++++++++++ drivers/net/ethernet/ti/am65-cpts.h | 6 +++ 2 files changed, 70 insertions(+)