Re: [PATCH v5 net-next 5/5] net: dsa: microchip: Enable HSR offloading for KSZ9477

[Vladimir Oltean]

On Wed, Sep 20, 2023 at 01:43:43PM +0200, Lukasz Majewski wrote:
> +void ksz9477_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr)
> +{
> +	struct ksz_device *dev = ds->priv;
> +	struct net_device *slave;
> +	u8 data;
> +
> +	/* Program which port(s) shall support HSR */
> +	ksz_rmw32(dev, REG_HSR_PORT_MAP__4, BIT(port), BIT(port));
> +
> +	/* Forward frames between HSR ports (i.e. bridge together HSR ports) */
> +	ksz9477_cfg_port_member(dev, port,
> +				BIT(dsa_upstream_port(ds, port)) | BIT(port));

Isn't this supposed to be

	ksz9477_cfg_port_member(dev, port,
				BIT(dsa_upstream_port(ds, port)) | BIT(pair));

where "pair" is not even passed as an argument to ksz9477_hsr_join(),
but represents the *other* port in the HSR ring, not this one?

> +
> +	if (!dev->hsr_ports) {
> +		/* Enable discarding of received HSR frames */
> +		ksz_read8(dev, REG_HSR_ALU_CTRL_0__1, &data);
> +		data |= HSR_DUPLICATE_DISCARD;
> +		data &= ~HSR_NODE_UNICAST;
> +		ksz_write8(dev, REG_HSR_ALU_CTRL_0__1, data);
> +	}
> +
> +	/* Enable per port self-address filtering.
> +	 * The global self-address filtering has already been enabled in the
> +	 * ksz9477_reset_switch() function.
> +	 */
> +	ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL, PORT_SRC_ADDR_FILTER, true);
> +
> +	/* Setup HW supported features for lan HSR ports */
> +	slave = dsa_to_port(ds, port)->slave;
> +	slave->features |= KSZ9477_SUPPORTED_HSR_FEATURES;
> +}
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index 1f3fb6c23f36..1f447a34f555 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -101,6 +101,11 @@ struct ksz_ptp_irq {
>  	int num;
>  };
>  
> +struct ksz_switch_macaddr {
> +	unsigned char addr[ETH_ALEN];
> +	refcount_t refcount;
> +};
> +
>  struct ksz_port {
>  	bool remove_tag;		/* Remove Tag flag set, for ksz8795 only */
>  	bool learning;
> @@ -169,6 +174,10 @@ struct ksz_device {
>  	struct mutex lock_irq;		/* IRQ Access */
>  	struct ksz_irq girq;
>  	struct ksz_ptp_data ptp_data;
> +
> +	struct ksz_switch_macaddr *switch_macaddr;
> +	struct net_device *hsr_dev;     /* HSR */

Please be consistent with the lines above, and use tabs to align the "/* HSR */"
comment.

> +	u8 hsr_ports;
>  };
>  
>  /* List of supported models */
> -- 
> 2.20.1
>