Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 2 Aug 2021 16:43:36 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     DENG Qingfang <dqfext@gmail.com>
Cc:     Sean Wang <sean.wang@mediatek.com>,
        Landen Chao <Landen.Chao@mediatek.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
        Eric Woudstra <ericwouds@gmail.com>,
        =?utf-8?B?UmVuw6k=?= van Dorst <opensource@vdorst.com>,
        Frank Wunderlich <frank-w@public-files.de>
Subject: Re: [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on
 filter ID 1
Message-ID: <20210802134336.gv66le6u2z52kfkh@skbuf>
References: <20210731191023.1329446-1-dqfext@gmail.com>
 <20210731191023.1329446-4-dqfext@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210731191023.1329446-4-dqfext@gmail.com>
Precedence: bulk

On Sun, Aug 01, 2021 at 03:10:21AM +0800, DENG Qingfang wrote:
> As filter ID 1 is used, set STP state also on it.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
>  drivers/net/dsa/mt7530.c | 3 ++-
>  drivers/net/dsa/mt7530.h | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 3876e265f844..38d6ce37d692 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1147,7 +1147,8 @@ mt7530_stp_state_set(struct dsa_switch *ds, int port, u8 state)
>  		break;
>  	}
>  
> -	mt7530_rmw(priv, MT7530_SSP_P(port), FID_PST_MASK, stp_state);
> +	mt7530_rmw(priv, MT7530_SSP_P(port), FID_PST_MASK,
> +		   FID_PST(stp_state));
>  }
>  
>  static int
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index a308886fdebc..294ff1cbd9e0 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -181,7 +181,7 @@ enum mt7530_vlan_egress_attr {
>  
>  /* Register for port STP state control */
>  #define MT7530_SSP_P(x)			(0x2000 + ((x) * 0x100))
> -#define  FID_PST(x)			((x) & 0x3)

Shouldn't these macros have _two_ arguments, the FID and the port state?

> +#define  FID_PST(x)			(((x) & 0x3) * 0x5)

"* 5": explanation?

>  #define  FID_PST_MASK			FID_PST(0x3)
>  
>  enum mt7530_stp_state {
> -- 
> 2.25.1
> 

I don't exactly understand how this patch works, sorry.
Are you altering port state only on bridged ports, or also on standalone
ports after this patch? Are standalone ports in the proper STP state
(FORWARDING)?