diff mbox series

[net-next,2/5,v2] net: dsa: rtl8366: Drop custom VLAN set-up

Message ID 20210830214859.403100-3-linus.walleij@linaro.org
State Superseded
Headers show
Series RTL8366RB improvements | expand

Commit Message

Linus Walleij Aug. 30, 2021, 9:48 p.m. UTC 
This hacky default VLAN setup was done in order to direct
packets to the right ports and provide port isolation, both
which we now support properly using custom tags and proper
bridge port isolation.

We can drop the custom VLAN code and leave all VLAN handling
alone, as users expect things to be. We can also drop
ds->configure_vlan_while_not_filtering = false; and let
the core deal with any VLANs it wants.

Cc: Vladimir Oltean <olteanv@gmail.com>
Cc: Alvin Šipraga <alsi@bang-olufsen.dk>
Cc: Mauri Sandberg <sandberg@mailfence.com>
Cc: DENG Qingfang <dqfext@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
ChangeLog v1->v2:
- No changes.
---
 drivers/net/dsa/realtek-smi-core.h |  1 -
 drivers/net/dsa/rtl8366.c          | 48 ------------------------------
 drivers/net/dsa/rtl8366rb.c        |  4 +--
 3 files changed, 1 insertion(+), 52 deletions(-)

-- 
2.31.1

Comments

Vladimir Oltean Aug. 30, 2021, 10:35 p.m. UTC | #1 
On Mon, Aug 30, 2021 at 11:48:56PM +0200, Linus Walleij wrote:
> This hacky default VLAN setup was done in order to direct

> packets to the right ports and provide port isolation, both

> which we now support properly using custom tags and proper

> bridge port isolation.

> 

> We can drop the custom VLAN code and leave all VLAN handling

> alone, as users expect things to be. We can also drop

> ds->configure_vlan_while_not_filtering = false; and let

> the core deal with any VLANs it wants.

> 

> Cc: Vladimir Oltean <olteanv@gmail.com>

> Cc: Alvin Šipraga <alsi@bang-olufsen.dk>

> Cc: Mauri Sandberg <sandberg@mailfence.com>

> Cc: DENG Qingfang <dqfext@gmail.com>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---


Reviewed-by: Vladimir Oltean <olteanv@gmail.com>


Please keep this review tag when posting v3.
Alvin Šipraga Aug. 30, 2021, 11:27 p.m. UTC | #2 
On 8/30/21 11:48 PM, Linus Walleij wrote:
> This hacky default VLAN setup was done in order to direct

> packets to the right ports and provide port isolation, both

> which we now support properly using custom tags and proper

> bridge port isolation.

> 

> We can drop the custom VLAN code and leave all VLAN handling

> alone, as users expect things to be. We can also drop

> ds->configure_vlan_while_not_filtering = false; and let

> the core deal with any VLANs it wants.

> 

> Cc: Vladimir Oltean <olteanv@gmail.com>

> Cc: Alvin Šipraga <alsi@bang-olufsen.dk>

> Cc: Mauri Sandberg <sandberg@mailfence.com>

> Cc: DENG Qingfang <dqfext@gmail.com>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---


Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>


> ChangeLog v1->v2:

> - No changes.

> ---

>   drivers/net/dsa/realtek-smi-core.h |  1 -

>   drivers/net/dsa/rtl8366.c          | 48 ------------------------------

>   drivers/net/dsa/rtl8366rb.c        |  4 +--

>   3 files changed, 1 insertion(+), 52 deletions(-)

> 

> diff --git a/drivers/net/dsa/realtek-smi-core.h b/drivers/net/dsa/realtek-smi-core.h

> index fcf465f7f922..c8fbd7b9fd0b 100644

> --- a/drivers/net/dsa/realtek-smi-core.h

> +++ b/drivers/net/dsa/realtek-smi-core.h

> @@ -129,7 +129,6 @@ int rtl8366_set_pvid(struct realtek_smi *smi, unsigned int port,

>   int rtl8366_enable_vlan4k(struct realtek_smi *smi, bool enable);

>   int rtl8366_enable_vlan(struct realtek_smi *smi, bool enable);

>   int rtl8366_reset_vlan(struct realtek_smi *smi);

> -int rtl8366_init_vlan(struct realtek_smi *smi);

>   int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,

>   			   struct netlink_ext_ack *extack);

>   int rtl8366_vlan_add(struct dsa_switch *ds, int port,

> diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c

> index 75897a369096..59c5bc4f7b71 100644

> --- a/drivers/net/dsa/rtl8366.c

> +++ b/drivers/net/dsa/rtl8366.c

> @@ -292,54 +292,6 @@ int rtl8366_reset_vlan(struct realtek_smi *smi)

>   }

>   EXPORT_SYMBOL_GPL(rtl8366_reset_vlan);

>   

> -int rtl8366_init_vlan(struct realtek_smi *smi)

> -{

> -	int port;

> -	int ret;

> -

> -	ret = rtl8366_reset_vlan(smi);

> -	if (ret)

> -		return ret;

> -

> -	/* Loop over the available ports, for each port, associate

> -	 * it with the VLAN (port+1)

> -	 */

> -	for (port = 0; port < smi->num_ports; port++) {

> -		u32 mask;

> -

> -		if (port == smi->cpu_port)

> -			/* For the CPU port, make all ports members of this

> -			 * VLAN.

> -			 */

> -			mask = GENMASK((int)smi->num_ports - 1, 0);

> -		else

> -			/* For all other ports, enable itself plus the

> -			 * CPU port.

> -			 */

> -			mask = BIT(port) | BIT(smi->cpu_port);

> -

> -		/* For each port, set the port as member of VLAN (port+1)

> -		 * and untagged, except for the CPU port: the CPU port (5) is

> -		 * member of VLAN 6 and so are ALL the other ports as well.

> -		 * Use filter 0 (no filter).

> -		 */

> -		dev_info(smi->dev, "VLAN%d port mask for port %d, %08x\n",

> -			 (port + 1), port, mask);

> -		ret = rtl8366_set_vlan(smi, (port + 1), mask, mask, 0);

> -		if (ret)

> -			return ret;

> -

> -		dev_info(smi->dev, "VLAN%d port %d, PVID set to %d\n",

> -			 (port + 1), port, (port + 1));

> -		ret = rtl8366_set_pvid(smi, port, (port + 1));

> -		if (ret)

> -			return ret;

> -	}

> -

> -	return rtl8366_enable_vlan(smi, true);

> -}

> -EXPORT_SYMBOL_GPL(rtl8366_init_vlan);

> -

>   int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,

>   			   struct netlink_ext_ack *extack)

>   {

> diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c

> index 50ee7cd62484..8b040440d2d4 100644

> --- a/drivers/net/dsa/rtl8366rb.c

> +++ b/drivers/net/dsa/rtl8366rb.c

> @@ -986,7 +986,7 @@ static int rtl8366rb_setup(struct dsa_switch *ds)

>   			return ret;

>   	}

>   

> -	ret = rtl8366_init_vlan(smi);

> +	ret = rtl8366_reset_vlan(smi);

>   	if (ret)

>   		return ret;

>   

> @@ -1000,8 +1000,6 @@ static int rtl8366rb_setup(struct dsa_switch *ds)

>   		return -ENODEV;

>   	}

>   

> -	ds->configure_vlan_while_not_filtering = false;

> -

>   	return 0;

>   }

>   

>
Florian Fainelli Aug. 31, 2021, 1:02 a.m. UTC | #3 
On 8/30/2021 2:48 PM, Linus Walleij wrote:
> This hacky default VLAN setup was done in order to direct

> packets to the right ports and provide port isolation, both

> which we now support properly using custom tags and proper

> bridge port isolation.

> 

> We can drop the custom VLAN code and leave all VLAN handling

> alone, as users expect things to be. We can also drop

> ds->configure_vlan_while_not_filtering = false; and let

> the core deal with any VLANs it wants.

> 

> Cc: Vladimir Oltean <olteanv@gmail.com>

> Cc: Alvin Šipraga <alsi@bang-olufsen.dk>

> Cc: Mauri Sandberg <sandberg@mailfence.com>

> Cc: DENG Qingfang <dqfext@gmail.com>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>


Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

-- 
Florian
diff mbox series

Patch

diff --git a/drivers/net/dsa/realtek-smi-core.h b/drivers/net/dsa/realtek-smi-core.h
index fcf465f7f922..c8fbd7b9fd0b 100644
--- a/drivers/net/dsa/realtek-smi-core.h
+++ b/drivers/net/dsa/realtek-smi-core.h
@@ -129,7 +129,6 @@  int rtl8366_set_pvid(struct realtek_smi *smi, unsigned int port,
 int rtl8366_enable_vlan4k(struct realtek_smi *smi, bool enable);
 int rtl8366_enable_vlan(struct realtek_smi *smi, bool enable);
 int rtl8366_reset_vlan(struct realtek_smi *smi);
-int rtl8366_init_vlan(struct realtek_smi *smi);
 int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
 			   struct netlink_ext_ack *extack);
 int rtl8366_vlan_add(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
index 75897a369096..59c5bc4f7b71 100644
--- a/drivers/net/dsa/rtl8366.c
+++ b/drivers/net/dsa/rtl8366.c
@@ -292,54 +292,6 @@  int rtl8366_reset_vlan(struct realtek_smi *smi)
 }
 EXPORT_SYMBOL_GPL(rtl8366_reset_vlan);
 
-int rtl8366_init_vlan(struct realtek_smi *smi)
-{
-	int port;
-	int ret;
-
-	ret = rtl8366_reset_vlan(smi);
-	if (ret)
-		return ret;
-
-	/* Loop over the available ports, for each port, associate
-	 * it with the VLAN (port+1)
-	 */
-	for (port = 0; port < smi->num_ports; port++) {
-		u32 mask;
-
-		if (port == smi->cpu_port)
-			/* For the CPU port, make all ports members of this
-			 * VLAN.
-			 */
-			mask = GENMASK((int)smi->num_ports - 1, 0);
-		else
-			/* For all other ports, enable itself plus the
-			 * CPU port.
-			 */
-			mask = BIT(port) | BIT(smi->cpu_port);
-
-		/* For each port, set the port as member of VLAN (port+1)
-		 * and untagged, except for the CPU port: the CPU port (5) is
-		 * member of VLAN 6 and so are ALL the other ports as well.
-		 * Use filter 0 (no filter).
-		 */
-		dev_info(smi->dev, "VLAN%d port mask for port %d, %08x\n",
-			 (port + 1), port, mask);
-		ret = rtl8366_set_vlan(smi, (port + 1), mask, mask, 0);
-		if (ret)
-			return ret;
-
-		dev_info(smi->dev, "VLAN%d port %d, PVID set to %d\n",
-			 (port + 1), port, (port + 1));
-		ret = rtl8366_set_pvid(smi, port, (port + 1));
-		if (ret)
-			return ret;
-	}
-
-	return rtl8366_enable_vlan(smi, true);
-}
-EXPORT_SYMBOL_GPL(rtl8366_init_vlan);
-
 int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
 			   struct netlink_ext_ack *extack)
 {
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index 50ee7cd62484..8b040440d2d4 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -986,7 +986,7 @@  static int rtl8366rb_setup(struct dsa_switch *ds)
 			return ret;
 	}
 
-	ret = rtl8366_init_vlan(smi);
+	ret = rtl8366_reset_vlan(smi);
 	if (ret)
 		return ret;
 
@@ -1000,8 +1000,6 @@  static int rtl8366rb_setup(struct dsa_switch *ds)
 		return -ENODEV;
 	}
 
-	ds->configure_vlan_while_not_filtering = false;
-
 	return 0;
 }