Re: [PATCH 6/7] net: dsa: b53: Add logic for TX timestamping
From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2021-11-06 02:50:52
Also in:
lkml
On 11/4/2021 6:32 AM, Martin Kaistra wrote:
In order to get the switch to generate a timestamp for a transmitted packet, we need to set the TS bit in the BRCM tag. The switch will then create a status frame, which gets send back to the cpu. In b53_port_txtstamp() we put the skb into a waiting position. When a status frame is received, we extract the timestamp and put the time according to our timecounter into the waiting skb. When TX_TSTAMP_TIMEOUT is reached and we have no means to correctly get back a full timestamp, we cancel the process. As the status frame doesn't contain a reference to the original packet, only one packet with timestamp request can be sent at a time. Signed-off-by: Martin Kaistra <redacted> ---
[snip]
+static long b53_hwtstamp_work(struct ptp_clock_info *ptp)
+{
+ struct b53_device *dev =
+ container_of(ptp, struct b53_device, ptp_clock_info);
+ struct dsa_switch *ds = dev->ds;
+ int i;
+
+ for (i = 0; i < ds->num_ports; i++) {
+ struct b53_port_hwtstamp *ps;
+
+ if (!dsa_is_user_port(ds, i))
+ continue;Can you also check on !dsa_port_is_unused()? [snip]
quoted hunk ↗ jump to hunk
#endifdiff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c index 85dc47c22008..53cd0345df1b 100644 --- a/net/dsa/tag_brcm.c +++ b/net/dsa/tag_brcm.c@@ -8,6 +8,7 @@ #include <linux/dsa/brcm.h> #include <linux/etherdevice.h> #include <linux/list.h> +#include <linux/ptp_classify.h> #include <linux/slab.h> #include <linux/dsa/b53.h>@@ -85,9 +86,14 @@ static struct sk_buff *brcm_tag_xmit_ll(struct sk_buff *skb, unsigned int offset) { struct dsa_port *dp = dsa_slave_to_port(dev); + struct b53_device *b53_dev = dp->ds->priv; + unsigned int type = ptp_classify_raw(skb); u16 queue = skb_get_queue_mapping(skb); + struct b53_port_hwtstamp *ps; u8 *brcm_tag; + ps = &b53_dev->ports[dp->index].port_hwtstamp;
The dsa_port structure as a priv member which would be well suited to store &b53_dev->ports[dp->index].port_hwtstamp and avoid traversing multiple layers of objects here. You don't need to need b53_device at all, and even if you did, you could easily add a back pointer to it in port_hwstamp. This applies below as well in brcm_tag_rcv_ll [snip]
quoted hunk ↗ jump to hunk
+ /* Frames with this tag have one of these two layouts: * ----------------------------------- * | MAC DA | MAC SA | 4b tag | Type | DSA_TAG_PROTO_BRCM@@ -143,6 +181,9 @@ static struct sk_buff *brcm_tag_rcv_ll(struct sk_buff *skb, unsigned int offset, int *tag_len) { + struct b53_port_hwtstamp *ps; + struct b53_device *b53_dev; + struct dsa_port *dp; int source_port; u8 *brcm_tag; u32 tstamp;@@ -174,6 +215,16 @@ static struct sk_buff *brcm_tag_rcv_ll(struct sk_buff *skb, if (!skb->dev) return NULL; + /* Check whether this is a status frame */ + if (*tag_len == 8 && brcm_tag[3] & 0x20) {
Can we have an unlikely() here, because this is unlikely to happen except for switches that do support PTP, and we only have 53128 so far. Also a define for this 0x20 would be nice, it is the timestamp bit for the packet. -- Florian