Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Date:   Thu, 21 Sep 2023 22:32:24 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Lukasz Majewski <lukma@denx.de>
Cc:     Tristram.Ha@microchip.com, Eric Dumazet <edumazet@google.com>,
        Andrew Lunn <andrew@lunn.ch>, davem@davemloft.net,
        Woojung Huh <woojung.huh@microchip.com>,
        Oleksij Rempel <o.rempel@pengutronix.de>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, UNGLinuxDriver@microchip.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Vladimir Oltean <vladimir.oltean@nxp.com>
Subject: Re: [PATCH v5 net-next 5/5] net: dsa: microchip: Enable HSR
 offloading for KSZ9477
Message-ID: <20230921193224.l3ojpdcsb4bpfl7d@skbuf>
References: <20230920114343.1979843-1-lukma@denx.de>
 <20230920114343.1979843-1-lukma@denx.de>
 <20230920114343.1979843-6-lukma@denx.de>
 <20230920114343.1979843-6-lukma@denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230920114343.1979843-6-lukma@denx.de>
 <20230920114343.1979843-6-lukma@denx.de>
Precedence: bulk

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
>