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

[Vladimir Oltean <olteanv@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>