diff mbox series

[net-next,2/2,v5] net: dsa: rtl8366rb: Support the CPU DSA tag

Message ID 20200707211614.1217258-3-linus.walleij@linaro.org
State Superseded
Headers show
Series RTL8366RB tagging support | expand

Commit Message

Linus Walleij July 7, 2020, 9:16 p.m. UTC 
This activates the support to use the CPU tag to properly
direct ingress traffic to the right port.

Bit 15 in register RTL8368RB_CPU_CTRL_REG can be set to
1 to disable the insertion of the CPU tag which is what
the code currently does. The bit 15 define calls this
setting RTL8368RB_CPU_INSTAG which is confusing since the
inverse meaning is implied: programmers may think that
setting this bit to 1 will *enable* inserting the tag
rather than disabling it, so rename this setting in
bit 15 to RTL8368RB_CPU_NO_TAG which is more to the
point.

After this e.g. ping works out-of-the-box with the
RTL8366RB.

Cc: DENG Qingfang <dqfext@gmail.com>
Cc: Mauri Sandberg <sandberg@mailfence.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>

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

---
ChangeLog v4->v5:
- Split tagging support from the VLAN fix-ups.
ChangeLog v3->v4:
- Resend with the rest
ChangeLog v2->v3:
- Fix up the commit message.
- Collect Andrew's review tag.
ChangeLog v1->v2:
- Update the commit message to explain why we are renaming
  bit 15 in RTL8368RB_CPU_CTRL_REG.
---
 drivers/net/dsa/Kconfig     |  1 +
 drivers/net/dsa/rtl8366rb.c | 31 ++++++++-----------------------
 2 files changed, 9 insertions(+), 23 deletions(-)

-- 
2.26.2

Comments

Florian Fainelli July 7, 2020, 9:51 p.m. UTC | #1 
On 7/7/2020 2:16 PM, Linus Walleij wrote:
> This activates the support to use the CPU tag to properly

> direct ingress traffic to the right port.

> 

> Bit 15 in register RTL8368RB_CPU_CTRL_REG can be set to

> 1 to disable the insertion of the CPU tag which is what

> the code currently does. The bit 15 define calls this

> setting RTL8368RB_CPU_INSTAG which is confusing since the

> inverse meaning is implied: programmers may think that

> setting this bit to 1 will *enable* inserting the tag

> rather than disabling it, so rename this setting in

> bit 15 to RTL8368RB_CPU_NO_TAG which is more to the

> point.

> 

> After this e.g. ping works out-of-the-box with the

> RTL8366RB.

> 

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

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

> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

> 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/Kconfig b/drivers/net/dsa/Kconfig
index d0024cb30a7b..468b3c4273c5 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -70,6 +70,7 @@  config NET_DSA_QCA8K
 config NET_DSA_REALTEK_SMI
 	tristate "Realtek SMI Ethernet switch family support"
 	depends on NET_DSA
+	select NET_DSA_TAG_RTL4_A
 	select FIXED_PHY
 	select IRQ_DOMAIN
 	select REALTEK_PHY
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index fd1977590cb4..48f1ff746799 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -109,8 +109,8 @@ 
 /* CPU port control reg */
 #define RTL8368RB_CPU_CTRL_REG		0x0061
 #define RTL8368RB_CPU_PORTS_MSK		0x00FF
-/* Enables inserting custom tag length/type 0x8899 */
-#define RTL8368RB_CPU_INSTAG		BIT(15)
+/* Disables inserting custom tag length/type 0x8899 */
+#define RTL8368RB_CPU_NO_TAG		BIT(15)
 
 #define RTL8366RB_SMAR0			0x0070 /* bits 0..15 */
 #define RTL8366RB_SMAR1			0x0071 /* bits 16..31 */
@@ -844,16 +844,14 @@  static int rtl8366rb_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
-	/* Enable CPU port and enable inserting CPU tag
+	/* Enable CPU port with custom DSA tag 8899.
 	 *
-	 * Disabling RTL8368RB_CPU_INSTAG here will change the behaviour
-	 * of the switch totally and it will start talking Realtek RRCP
-	 * internally. It is probably possible to experiment with this,
-	 * but then the kernel needs to understand and handle RRCP first.
+	 * If you set RTL8368RB_CPU_NO_TAG (bit 15) in this registers
+	 * the custom tag is turned off.
 	 */
 	ret = regmap_update_bits(smi->map, RTL8368RB_CPU_CTRL_REG,
 				 0xFFFF,
-				 RTL8368RB_CPU_INSTAG | BIT(smi->cpu_port));
+				 BIT(smi->cpu_port));
 	if (ret)
 		return ret;
 
@@ -967,21 +965,8 @@  static enum dsa_tag_protocol rtl8366_get_tag_protocol(struct dsa_switch *ds,
 						      int port,
 						      enum dsa_tag_protocol mp)
 {
-	/* For now, the RTL switches are handled without any custom tags.
-	 *
-	 * It is possible to turn on "custom tags" by removing the
-	 * RTL8368RB_CPU_INSTAG flag when enabling the port but what it
-	 * does is unfamiliar to DSA: ethernet frames of type 8899, the Realtek
-	 * Remote Control Protocol (RRCP) start to appear on the CPU port of
-	 * the device. So this is not the ordinary few extra bytes in the
-	 * frame. Instead it appears that the switch starts to talk Realtek
-	 * RRCP internally which means a pretty complex RRCP implementation
-	 * decoding and responding the RRCP protocol is needed to exploit this.
-	 *
-	 * The OpenRRCP project (dormant since 2009) have reverse-egineered
-	 * parts of the protocol.
-	 */
-	return DSA_TAG_PROTO_NONE;
+	/* This switch uses the 4 byte protocol A Realtek DSA tag */
+	return DSA_TAG_PROTO_RTL4_A;
 }
 
 static void rtl8366rb_adjust_link(struct dsa_switch *ds, int port,