On Sat, Feb 6, 2021 at 5:53 PM Vladimir Oltean <olte...@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.