diff mbox series

[net-next,v2,4/4] net: dsa: xrs700x: add HSR offloading support

Message ID 20210204215926.64377-5-george.mccollister@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series add HSR offloading support for DSA switches | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 1 maintainers not CCed: davem@davemloft.net
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 153 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

George McCollister Feb. 4, 2021, 9:59 p.m. UTC
Add offloading for HSR/PRP (IEC 62439-3) tag insertion, tag removal
forwarding and duplication supported by the xrs7000 series switches.

Only HSR v1 and PRP v1 are supported by the xrs7000 series switches (HSR
v0 is not).

Signed-off-by: George McCollister <george.mccollister@gmail.com>
---
 drivers/net/dsa/xrs700x/xrs700x.c     | 106 ++++++++++++++++++++++++++++++++++
 drivers/net/dsa/xrs700x/xrs700x_reg.h |   5 ++
 net/dsa/tag_xrs700x.c                 |   7 ++-
 3 files changed, 117 insertions(+), 1 deletion(-)

Comments

Vladimir Oltean Feb. 6, 2021, 11:53 p.m. UTC | #1
On Thu, Feb 04, 2021 at 03:59:26PM -0600, George McCollister wrote:
> +static int xrs700x_hsr_join(struct dsa_switch *ds, int port,
> +			    struct net_device *hsr)
> +{
> +	unsigned int val = XRS_HSR_CFG_HSR_PRP;
> +	struct dsa_port *partner = NULL, *dp;
> +	struct xrs700x *priv = ds->priv;
> +	struct net_device *slave;
> +	enum hsr_version ver;
> +	int ret;
> +
> +	ret = hsr_get_version(hsr, &ver);
> +	if (ret)
> +		return ret;
> +
> +	if (ver == HSR_V1)
> +		val |= XRS_HSR_CFG_HSR;
> +	else if (ver == PRP_V1)
> +		val |= XRS_HSR_CFG_PRP;
> +	else
> +		return -EOPNOTSUPP;
> +
> +	dsa_hsr_foreach_port(dp, ds, hsr) {
> +		partner = dp;
> +	}
> +
> +	/* We can't enable redundancy on the switch until both
> +	 * redundant ports have signed up.
> +	 */
> +	if (!partner)
> +		return 0;
> +
> +	regmap_fields_write(priv->ps_forward, partner->index,
> +			    XRS_PORT_DISABLED);
> +	regmap_fields_write(priv->ps_forward, port, XRS_PORT_DISABLED);
> +
> +	regmap_write(priv->regmap, XRS_HSR_CFG(partner->index),
> +		     val | XRS_HSR_CFG_LANID_A);
> +	regmap_write(priv->regmap, XRS_HSR_CFG(port),
> +		     val | XRS_HSR_CFG_LANID_B);
> +
> +	/* Clear bits for both redundant ports (HSR only) and the CPU port to
> +	 * enable forwarding.
> +	 */
> +	val = GENMASK(ds->num_ports - 1, 0);
> +	if (ver == HSR_V1) {
> +		val &= ~BIT(partner->index);
> +		val &= ~BIT(port);
> +	}
> +	val &= ~BIT(dsa_upstream_port(ds, port));
> +	regmap_write(priv->regmap, XRS_PORT_FWD_MASK(partner->index), val);
> +	regmap_write(priv->regmap, XRS_PORT_FWD_MASK(port), val);
> +
> +	regmap_fields_write(priv->ps_forward, partner->index,
> +			    XRS_PORT_FORWARDING);
> +	regmap_fields_write(priv->ps_forward, port, XRS_PORT_FORWARDING);
> +
> +	slave = dsa_to_port(ds, port)->slave;
> +
> +	slave->features |= NETIF_F_HW_HSR_TAG_INS | NETIF_F_HW_HSR_TAG_RM |
> +			   NETIF_F_HW_HSR_FWD | NETIF_F_HW_HSR_DUP;
> +
> +	return 0;
> +}

Is it deliberate that only one slave HSR/PRP port will have the offload
ethtool features set? If yes, then I find that a bit odd from a user
point of view.
George McCollister Feb. 8, 2021, 2:46 p.m. UTC | #2
On Sat, Feb 6, 2021 at 5:53 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Thu, Feb 04, 2021 at 03:59:26PM -0600, George McCollister wrote:
> > +static int xrs700x_hsr_join(struct dsa_switch *ds, int port,
> > +                         struct net_device *hsr)
> > +{
> > +     unsigned int val = XRS_HSR_CFG_HSR_PRP;
> > +     struct dsa_port *partner = NULL, *dp;
> > +     struct xrs700x *priv = ds->priv;
> > +     struct net_device *slave;
> > +     enum hsr_version ver;
> > +     int ret;
> > +
> > +     ret = hsr_get_version(hsr, &ver);
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (ver == HSR_V1)
> > +             val |= XRS_HSR_CFG_HSR;
> > +     else if (ver == PRP_V1)
> > +             val |= XRS_HSR_CFG_PRP;
> > +     else
> > +             return -EOPNOTSUPP;
> > +
> > +     dsa_hsr_foreach_port(dp, ds, hsr) {
> > +             partner = dp;
> > +     }
> > +
> > +     /* We can't enable redundancy on the switch until both
> > +      * redundant ports have signed up.
> > +      */
> > +     if (!partner)
> > +             return 0;
> > +
> > +     regmap_fields_write(priv->ps_forward, partner->index,
> > +                         XRS_PORT_DISABLED);
> > +     regmap_fields_write(priv->ps_forward, port, XRS_PORT_DISABLED);
> > +
> > +     regmap_write(priv->regmap, XRS_HSR_CFG(partner->index),
> > +                  val | XRS_HSR_CFG_LANID_A);
> > +     regmap_write(priv->regmap, XRS_HSR_CFG(port),
> > +                  val | XRS_HSR_CFG_LANID_B);
> > +
> > +     /* Clear bits for both redundant ports (HSR only) and the CPU port to
> > +      * enable forwarding.
> > +      */
> > +     val = GENMASK(ds->num_ports - 1, 0);
> > +     if (ver == HSR_V1) {
> > +             val &= ~BIT(partner->index);
> > +             val &= ~BIT(port);
> > +     }
> > +     val &= ~BIT(dsa_upstream_port(ds, port));
> > +     regmap_write(priv->regmap, XRS_PORT_FWD_MASK(partner->index), val);
> > +     regmap_write(priv->regmap, XRS_PORT_FWD_MASK(port), val);
> > +
> > +     regmap_fields_write(priv->ps_forward, partner->index,
> > +                         XRS_PORT_FORWARDING);
> > +     regmap_fields_write(priv->ps_forward, port, XRS_PORT_FORWARDING);
> > +
> > +     slave = dsa_to_port(ds, port)->slave;
> > +
> > +     slave->features |= NETIF_F_HW_HSR_TAG_INS | NETIF_F_HW_HSR_TAG_RM |
> > +                        NETIF_F_HW_HSR_FWD | NETIF_F_HW_HSR_DUP;
> > +
> > +     return 0;
> > +}
>
> Is it deliberate that only one slave HSR/PRP port will have the offload
> ethtool features set? If yes, then I find that a bit odd from a user
> point of view.

No. Good catch. This is a mistake I introduced when I added the code
for finding the partner. Originally for testing I had hacks that hard
coded the ports used and reconfigured HSR for each join.
diff mbox series

Patch

diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index 259f5e657c46..566ce9330903 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -7,6 +7,8 @@ 
 #include <net/dsa.h>
 #include <linux/if_bridge.h>
 #include <linux/of_device.h>
+#include <linux/netdev_features.h>
+#include <linux/if_hsr.h>
 #include "xrs700x.h"
 #include "xrs700x_reg.h"
 
@@ -496,6 +498,108 @@  static void xrs700x_bridge_leave(struct dsa_switch *ds, int port,
 	xrs700x_bridge_common(ds, port, bridge, false);
 }
 
+static int xrs700x_hsr_join(struct dsa_switch *ds, int port,
+			    struct net_device *hsr)
+{
+	unsigned int val = XRS_HSR_CFG_HSR_PRP;
+	struct dsa_port *partner = NULL, *dp;
+	struct xrs700x *priv = ds->priv;
+	struct net_device *slave;
+	enum hsr_version ver;
+	int ret;
+
+	ret = hsr_get_version(hsr, &ver);
+	if (ret)
+		return ret;
+
+	if (ver == HSR_V1)
+		val |= XRS_HSR_CFG_HSR;
+	else if (ver == PRP_V1)
+		val |= XRS_HSR_CFG_PRP;
+	else
+		return -EOPNOTSUPP;
+
+	dsa_hsr_foreach_port(dp, ds, hsr) {
+		partner = dp;
+	}
+
+	/* We can't enable redundancy on the switch until both
+	 * redundant ports have signed up.
+	 */
+	if (!partner)
+		return 0;
+
+	regmap_fields_write(priv->ps_forward, partner->index,
+			    XRS_PORT_DISABLED);
+	regmap_fields_write(priv->ps_forward, port, XRS_PORT_DISABLED);
+
+	regmap_write(priv->regmap, XRS_HSR_CFG(partner->index),
+		     val | XRS_HSR_CFG_LANID_A);
+	regmap_write(priv->regmap, XRS_HSR_CFG(port),
+		     val | XRS_HSR_CFG_LANID_B);
+
+	/* Clear bits for both redundant ports (HSR only) and the CPU port to
+	 * enable forwarding.
+	 */
+	val = GENMASK(ds->num_ports - 1, 0);
+	if (ver == HSR_V1) {
+		val &= ~BIT(partner->index);
+		val &= ~BIT(port);
+	}
+	val &= ~BIT(dsa_upstream_port(ds, port));
+	regmap_write(priv->regmap, XRS_PORT_FWD_MASK(partner->index), val);
+	regmap_write(priv->regmap, XRS_PORT_FWD_MASK(port), val);
+
+	regmap_fields_write(priv->ps_forward, partner->index,
+			    XRS_PORT_FORWARDING);
+	regmap_fields_write(priv->ps_forward, port, XRS_PORT_FORWARDING);
+
+	slave = dsa_to_port(ds, port)->slave;
+
+	slave->features |= NETIF_F_HW_HSR_TAG_INS | NETIF_F_HW_HSR_TAG_RM |
+			   NETIF_F_HW_HSR_FWD | NETIF_F_HW_HSR_DUP;
+
+	return 0;
+}
+
+static void xrs700x_hsr_leave(struct dsa_switch *ds, int port,
+			      struct net_device *hsr)
+{
+	struct dsa_port *partner = NULL, *dp;
+	struct xrs700x *priv = ds->priv;
+	struct net_device *slave;
+	unsigned int val;
+
+	dsa_hsr_foreach_port(dp, ds, hsr) {
+		partner = dp;
+	}
+
+	if (!partner)
+		return;
+
+	regmap_fields_write(priv->ps_forward, partner->index,
+			    XRS_PORT_DISABLED);
+	regmap_fields_write(priv->ps_forward, port, XRS_PORT_DISABLED);
+
+	regmap_write(priv->regmap, XRS_HSR_CFG(partner->index), 0);
+	regmap_write(priv->regmap, XRS_HSR_CFG(port), 0);
+
+	/* Clear bit for the CPU port to enable forwarding. */
+	val = GENMASK(ds->num_ports - 1, 0);
+	val &= ~BIT(dsa_upstream_port(ds, port));
+	regmap_write(priv->regmap, XRS_PORT_FWD_MASK(partner->index), val);
+	regmap_write(priv->regmap, XRS_PORT_FWD_MASK(port), val);
+
+	regmap_fields_write(priv->ps_forward, partner->index,
+			    XRS_PORT_FORWARDING);
+	regmap_fields_write(priv->ps_forward, port, XRS_PORT_FORWARDING);
+
+	slave = dsa_to_port(ds, port)->slave;
+
+	slave->features &= ~(NETIF_F_HW_HSR_TAG_INS | NETIF_F_HW_HSR_TAG_RM |
+			   NETIF_F_HW_HSR_FWD | NETIF_F_HW_HSR_DUP);
+}
+
 static const struct dsa_switch_ops xrs700x_ops = {
 	.get_tag_protocol	= xrs700x_get_tag_protocol,
 	.setup			= xrs700x_setup,
@@ -509,6 +613,8 @@  static const struct dsa_switch_ops xrs700x_ops = {
 	.get_stats64		= xrs700x_get_stats64,
 	.port_bridge_join	= xrs700x_bridge_join,
 	.port_bridge_leave	= xrs700x_bridge_leave,
+	.port_hsr_join		= xrs700x_hsr_join,
+	.port_hsr_leave		= xrs700x_hsr_leave,
 };
 
 static int xrs700x_detect(struct xrs700x *priv)
diff --git a/drivers/net/dsa/xrs700x/xrs700x_reg.h b/drivers/net/dsa/xrs700x/xrs700x_reg.h
index a135d4d92b6d..470d00e07f15 100644
--- a/drivers/net/dsa/xrs700x/xrs700x_reg.h
+++ b/drivers/net/dsa/xrs700x/xrs700x_reg.h
@@ -49,6 +49,11 @@ 
 
 /* Port Configuration Registers - HSR/PRP */
 #define XRS_HSR_CFG(x)			(XRS_PORT_HSR_BASE(x) + 0x0)
+#define XRS_HSR_CFG_HSR_PRP		BIT(0)
+#define XRS_HSR_CFG_HSR			0
+#define XRS_HSR_CFG_PRP			BIT(8)
+#define XRS_HSR_CFG_LANID_A		0
+#define XRS_HSR_CFG_LANID_B		BIT(10)
 
 /* Port Configuration Registers - PTP */
 #define XRS_PTP_RX_SYNC_DELAY_NS_LO(x)	(XRS_PORT_PTP_BASE(x) + 0x2)
diff --git a/net/dsa/tag_xrs700x.c b/net/dsa/tag_xrs700x.c
index db0ed1a5fcb7..858cdf9d2913 100644
--- a/net/dsa/tag_xrs700x.c
+++ b/net/dsa/tag_xrs700x.c
@@ -11,12 +11,17 @@ 
 
 static struct sk_buff *xrs700x_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_port *partner, *dp = dsa_slave_to_port(dev);
 	u8 *trailer;
 
 	trailer = skb_put(skb, 1);
 	trailer[0] = BIT(dp->index);
 
+	if (dp->hsr_dev)
+		dsa_hsr_foreach_port(partner, dp->ds, dp->hsr_dev)
+			if (partner != dp)
+				trailer[0] |= BIT(partner->index);
+
 	return skb;
 }