Re: [PATCH net-next v4 13/15] net: dsa: vsc73xx: Implement vsc73xx 8021q tagger

Vladimir Oltean
2024-02-15T18:09:56Z

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>