Received-SPF: pass (google.com: domain of linux-kernel+bounces-67474-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 15 Feb 2024 20:09:52 +0200
From: Vladimir Oltean <olteanv@gmail.com>
To: Pawel Dembicki <paweldembicki@gmail.com>
Cc: netdev@vger.kernel.org, linus.walleij@linaro.org,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	UNGLinuxDriver@microchip.com, Russell King <linux@armlinux.org.uk>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v4 13/15] net: dsa: vsc73xx: Implement vsc73xx
 8021q tagger
Message-ID: <20240215180952.b55l3qxno2mexqqs@skbuf>
References: <20240213220331.239031-1-paweldembicki@gmail.com>
 <20240213220331.239031-1-paweldembicki@gmail.com>
 <20240213220331.239031-14-paweldembicki@gmail.com>
 <20240213220331.239031-14-paweldembicki@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240213220331.239031-14-paweldembicki@gmail.com>
 <20240213220331.239031-14-paweldembicki@gmail.com>

In the context of DSA, tagger is synonymous with "tagging protocol
driver", aka what the previous patch implemented in
net/dsa/tag_vsc73xx_8021q.c. It would be better to rename the commit
title to "Implement the tag_8021q VLAN operations".

On Tue, Feb 13, 2024 at 11:03:26PM +0100, Pawel Dembicki wrote:
> This patch is a simple implementation of 802.1q tagging in the vsc73xx
> driver. Currently, devices with DSA_TAG_PROTO_NONE are not functional.
> The VSC73XX family doesn't provide any tag support for external Ethernet
> ports.
> 
> The only option available is VLAN-based tagging, which requires constant
> hardware VLAN filtering. While the VSC73XX family supports provider
> bridging, it only supports QinQ without full implementation of 802.1AD.
> This means it only allows the doubled 0x8100 TPID.
> 
> In the simple port mode, QinQ is enabled to preserve forwarding of
> VLAN-tagged frames.
> 
> The tag driver introduces the most basic functionality required for
> proper tagging support.

I guess this sentence is in place from before the tagging protocol
driver split in v3. Please remove it, it is confusing now.

> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---
> v4:
>   - adjust tag8021q implementation for changed untagged vlan storage
>   - minor fixes
> v3:
>   - Split tagger and tag implementation into separate commits
> 
>  drivers/net/dsa/Kconfig                |  2 +-
>  drivers/net/dsa/vitesse-vsc73xx-core.c | 38 ++++++++++++++++++++++++--
>  2 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> index 3092b391031a..22a04636d09e 100644
> --- a/drivers/net/dsa/Kconfig
> +++ b/drivers/net/dsa/Kconfig
> @@ -126,7 +126,7 @@ config NET_DSA_SMSC_LAN9303_MDIO
>  
>  config NET_DSA_VITESSE_VSC73XX
>  	tristate
> -	select NET_DSA_TAG_NONE
> +	select NET_DSA_TAG_VSC73XX_8021Q
>  	select FIXED_PHY
>  	select VITESSE_PHY
>  	select GPIOLIB
> diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
> index 6c7bd1c200b4..9f94ae8c763a 100644
> --- a/drivers/net/dsa/vitesse-vsc73xx-core.c
> +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
> @@ -17,6 +17,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/device.h>
> +#include <linux/iopoll.h>

I believe iopoll.h is misplaced in this patch. Probably the first one to
have used read_poll_timeout() should have included it.

>  #include <linux/of.h>
>  #include <linux/of_mdio.h>
>  #include <linux/bitops.h>