[PATCH v2 0/4] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching

[PATCH v2 0/4] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching

[Michael Dege]

Hello!

The current R-Car S4 rswitch driver only supports port based fowarding.
This patch set adds HW offloading for L2 switching/bridgeing. The driver
hooks into switchdev.

1. Rename the base driver file to keep the driver name (rswitch.ko)

2. Add setting of default MAC ageing time in hardware.

3. Add the L2 driver extension in a separate file. The HW offloading
is automatically configured when a port is added to the bridge device.

Usage example:
ip link add name br0 type bridge
ip link set dev tsn0 master br0
ip link set dev tsn1 master br0
ip link set dev br0 up
ip link set dev tsn0 up
ip link set dev tsn1 up

Layer 2 traffic is now fowarded by HW from port TSN0 to port TSN1.

4. Provides the functionality to set the MAC table ageing time in the
Rswitch.

Usage example:
ip link change dev br0 type bridge ageing 100

Changes in v2:
- Pulled default ageing setting into separate patch.
- Changed logging priority from info to dbg.
- Updated usage examples.
- Fixed passing of ageing parameter. Parameter is already in seconds
  no need to convert. Parameter checking improved.
- Updated commit message of [3/4] to point out that the switch hardware
  only supports the offloading of one bridge device. 
- Link to v1: https://lore.kernel.org/r/20250704-add_l2_switching-v1-0-ff882aacb258@renesas.com

Thanks,

Michael

Signed-off-by: Michael Dege <michael.dege@renesas.com>
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
To: Niklas Söderlund <niklas.soderlund@ragnatech.se>
To: Paul Barker <paul@pbarker.dev>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: Andrew Lunn <andrew+netdev@lunn.ch>
To: David S. Miller <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Michael Dege (4):
      net: renesas: rswitch: rename rswitch.c to rswitch_main.c
      net: renesas: rswitch: configure default ageing time
      net: renesas: rswitch: add offloading for L2 switching
      net: renesas: rswitch: add modifiable ageing time

 drivers/net/ethernet/renesas/Makefile              |   1 +
 drivers/net/ethernet/renesas/rswitch.h             |  42 ++-
 drivers/net/ethernet/renesas/rswitch_l2.c          | 339 +++++++++++++++++++++
 drivers/net/ethernet/renesas/rswitch_l2.h          |  15 +
 .../ethernet/renesas/{rswitch.c => rswitch_main.c} |  88 +++++-
 5 files changed, 479 insertions(+), 6 deletions(-)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20250616-add_l2_switching-345117105d0d

Best regards,
-- 
Michael Dege <michael.dege@renesas.com>

[PATCH v2 1/4] net: renesas: rswitch: rename rswitch.c to rswitch_main.c

[Michael Dege]

Adding new functionality to the driver. Therefore splitting into multiple
c files to keep them manageable. New functionality will be added to
separate files.

Signed-off-by: Michael Dege <michael.dege@renesas.com>
---
 drivers/net/ethernet/renesas/Makefile                      | 1 +
 drivers/net/ethernet/renesas/{rswitch.c => rswitch_main.c} | 0
 2 files changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/renesas/Makefile b/drivers/net/ethernet/renesas/Makefile
index f65fc76f8b4df8dd9f24af836b6dc0772965366f..6222298bb5582b7091cf8de76acb83ac7dd39c11 100644
--- a/drivers/net/ethernet/renesas/Makefile
+++ b/drivers/net/ethernet/renesas/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_SH_ETH) += sh_eth.o
 ravb-objs := ravb_main.o ravb_ptp.o
 obj-$(CONFIG_RAVB) += ravb.o
 
+rswitch-objs := rswitch_main.o
 obj-$(CONFIG_RENESAS_ETHER_SWITCH) += rswitch.o
 
 obj-$(CONFIG_RENESAS_GEN4_PTP) += rcar_gen4_ptp.o
diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch_main.c
similarity index 100%
rename from drivers/net/ethernet/renesas/rswitch.c
rename to drivers/net/ethernet/renesas/rswitch_main.c

-- 
2.49.0

Re: [PATCH v2 1/4] net: renesas: rswitch: rename rswitch.c to rswitch_main.c

[Andrew Lunn]

On Tue, Jul 08, 2025 at 11:27:37AM +0200, Michael Dege wrote:
> Adding new functionality to the driver. Therefore splitting into multiple
> c files to keep them manageable. New functionality will be added to
> separate files.
> 
> Signed-off-by: Michael Dege <michael.dege@renesas.com>

I gave a Reviewed-by: for this patch. You are expected to include this
in new versions of the patch. b4 prep can do a lot of this monkey work
for you if you use it.

    Andrew

---
pw-bot: cr

RE: [PATCH v2 1/4] net: renesas: rswitch: rename rswitch.c to rswitch_main.c

[Michael Dege]

Hello Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Tuesday, July 8, 2025 11:55 PM
> To: Michael Dege <michael.dege@renesas.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>; Niklas Söderlund
> <niklas.soderlund@ragnatech.se>; Paul Barker <paul@pbarker.dev>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; linux-kernel@vger.kernel.org; Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Subject: Re: [PATCH v2 1/4] net: renesas: rswitch: rename rswitch.c to rswitch_main.c
>
> On Tue, Jul 08, 2025 at 11:27:37AM +0200, Michael Dege wrote:
> > Adding new functionality to the driver. Therefore splitting into
> > multiple c files to keep them manageable. New functionality will be
> > added to separate files.
> >
> > Signed-off-by: Michael Dege <michael.dege@renesas.com>
>
> I gave a Reviewed-by: for this patch. You are expected to include this in new versions of the patch.
> b4 prep can do a lot of this monkey work for you if you use it.

I am sorry, I didn't check this before sending v2. I am using b4. I don't know why it wasn't added
Automatically.

Michael

>
>     Andrew
>
> ---
> pw-bot: cr
________________________________

Renesas Electronics Europe GmbH
Registered Office: Arcadiastrasse 10
DE-40472 Duesseldorf
Commercial Registry: Duesseldorf, HRB 3708
Managing Director: Carsten Jauch
VAT-No.: DE 14978647
Tax-ID-No: 105/5839/1793

Legal Disclaimer: This e-mail communication (and any attachment/s) is confidential and contains proprietary information, some or all of which may be legally privileged. It is intended solely for the use of the individual or entity to which it is addressed. Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.

[PATCH v2 2/4] net: renesas: rswitch: configure default ageing time

[Michael Dege]

Enable MAC ageing by setting up the timer and setting the ageging
time to the default of 300s.

Signed-off-by: Michael Dege <michael.dege@renesas.com>
---
 drivers/net/ethernet/renesas/rswitch.h      | 13 ++++++++++++-
 drivers/net/ethernet/renesas/rswitch_main.c | 11 ++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/rswitch.h b/drivers/net/ethernet/renesas/rswitch.h
index 532192cbca4b520e06a7e35653929d8364f1ccb2..feb62b99bceb50e1d68345ff9eba581f7c38edbe 100644
--- a/drivers/net/ethernet/renesas/rswitch.h
+++ b/drivers/net/ethernet/renesas/rswitch.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /* Renesas Ethernet Switch device driver
  *
- * Copyright (C) 2022 Renesas Electronics Corporation
+ * Copyright (C) 2022-2025 Renesas Electronics Corporation
  */
 
 #ifndef __RSWITCH_H__
@@ -826,6 +826,17 @@ enum rswitch_gwca_mode {
 
 #define FWPBFCSDC(j, i)         (FWPBFCSDC00 + (i) * 0x10 + (j) * 0x04)
 
+#define FWMACAGUSPC_MACAGUSP	GENMASK(9, 0)
+#define FWMACAGC_MACAGT		GENMASK(15, 0)
+#define FWMACAGC_MACAGE		BIT(16)
+#define FWMACAGC_MACAGSL	BIT(17)
+#define FWMACAGC_MACAGPM	BIT(18)
+#define FWMACAGC_MACDES		BIT(24)
+#define FWMACAGC_MACAGOG	BIT(28)
+#define FWMACAGC_MACDESOG	BIT(29)
+
+#define RSW_AGEING_TIME		300
+
 /* TOP */
 #define TPEMIMC7(queue)		(TPEMIMC70 + (queue) * 4)
 
diff --git a/drivers/net/ethernet/renesas/rswitch_main.c b/drivers/net/ethernet/renesas/rswitch_main.c
index aba772e14555d30891dc74a5a123121dad77e92b..2474271cac7f057fffb472131f5a7a72a0fa9a87 100644
--- a/drivers/net/ethernet/renesas/rswitch_main.c
+++ b/drivers/net/ethernet/renesas/rswitch_main.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Renesas Ethernet Switch device driver
  *
- * Copyright (C) 2022 Renesas Electronics Corporation
+ * Copyright (C) 2022-2025 Renesas Electronics Corporation
  */
 
 #include <linux/clk.h>
@@ -113,6 +113,7 @@ static void rswitch_fwd_init(struct rswitch_private *priv)
 {
 	u32 all_ports_mask = GENMASK(RSWITCH_NUM_AGENTS - 1, 0);
 	unsigned int i;
+	u32 reg_val;
 
 	/* Start with empty configuration */
 	for (i = 0; i < RSWITCH_NUM_AGENTS; i++) {
@@ -128,6 +129,14 @@ static void rswitch_fwd_init(struct rswitch_private *priv)
 		iowrite32(0, priv->addr + FWPBFC(i));
 	}
 
+	/* Configure MAC table aging */
+	rswitch_modify(priv->addr, FWMACAGUSPC, FWMACAGUSPC_MACAGUSP,
+		       FIELD_PREP(FWMACAGUSPC_MACAGUSP, 0x140));
+
+	reg_val = FIELD_PREP(FWMACAGC_MACAGT, RSW_AGEING_TIME);
+	reg_val |= FWMACAGC_MACAGE | FWMACAGC_MACAGSL;
+	iowrite32(reg_val, priv->addr + FWMACAGC);
+
 	/* For enabled ETHA ports, setup port based forwarding */
 	rswitch_for_each_enabled_port(priv, i) {
 		/* Port based forwarding from port i to GWCA port */

-- 
2.49.0

Re: [PATCH v2 2/4] net: renesas: rswitch: configure default ageing time

[Andrew Lunn]

> +	/* Configure MAC table aging */
> +	rswitch_modify(priv->addr, FWMACAGUSPC, FWMACAGUSPC_MACAGUSP,
> +		       FIELD_PREP(FWMACAGUSPC_MACAGUSP, 0x140));

Could you replace this 0x140 magic number with a #define explaining
what it means?

	Andrew

[PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching

[Michael Dege]

This commit adds hardware offloading for L2 switching on R-Car S4.

On S4 brdev is limited to one per-device (not per port). Reasoning
is that hw L2 forwarding support lacks any sort of source port based
filtering, which makes it unusable to offload more than one bridge
device. Either you allow hardware to forward destination MAC to a
port, or you have to send it to CPU. You can't make it forward only
if src and dst ports are in the same brdev.

Signed-off-by: Michael Dege <michael.dege@renesas.com>
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
 drivers/net/ethernet/renesas/Makefile       |   2 +-
 drivers/net/ethernet/renesas/rswitch.h      |  29 ++-
 drivers/net/ethernet/renesas/rswitch_l2.c   | 318 ++++++++++++++++++++++++++++
 drivers/net/ethernet/renesas/rswitch_l2.h   |  15 ++
 drivers/net/ethernet/renesas/rswitch_main.c |  77 ++++++-
 5 files changed, 436 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/renesas/Makefile b/drivers/net/ethernet/renesas/Makefile
index 6222298bb5582b7091cf8de76acb83ac7dd39c11..d63e0c61bb68a9993d388967aea9e1d50f6a95be 100644
--- a/drivers/net/ethernet/renesas/Makefile
+++ b/drivers/net/ethernet/renesas/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_SH_ETH) += sh_eth.o
 ravb-objs := ravb_main.o ravb_ptp.o
 obj-$(CONFIG_RAVB) += ravb.o
 
-rswitch-objs := rswitch_main.o
+rswitch-objs := rswitch_main.o rswitch_l2.o
 obj-$(CONFIG_RENESAS_ETHER_SWITCH) += rswitch.o
 
 obj-$(CONFIG_RENESAS_GEN4_PTP) += rcar_gen4_ptp.o
diff --git a/drivers/net/ethernet/renesas/rswitch.h b/drivers/net/ethernet/renesas/rswitch.h
index feb62b99bceb50e1d68345ff9eba581f7c38edbe..faf6c53062fb339c2de6b1c77f39ba1a884878c1 100644
--- a/drivers/net/ethernet/renesas/rswitch.h
+++ b/drivers/net/ethernet/renesas/rswitch.h
@@ -8,12 +8,18 @@
 #define __RSWITCH_H__
 
 #include <linux/platform_device.h>
+#include <linux/phy.h>
+
 #include "rcar_gen4_ptp.h"
 
 #define RSWITCH_MAX_NUM_QUEUES	128
 
 #define RSWITCH_NUM_AGENTS	5
 #define RSWITCH_NUM_PORTS	3
+
+#define rswitch_for_all_ports(_priv, _rdev)			\
+	list_for_each_entry(_rdev, &_priv->port_list, list)
+
 #define rswitch_for_each_enabled_port(priv, i)		\
 	for (i = 0; i < RSWITCH_NUM_PORTS; i++)		\
 		if (priv->rdev[i]->disabled)		\
@@ -809,7 +815,8 @@ enum rswitch_gwca_mode {
 #define FWPC0_IP4EA		BIT(10)
 #define FWPC0_IPDSA		BIT(12)
 #define FWPC0_IPHLA		BIT(18)
-#define FWPC0_MACSDA		BIT(20)
+#define FWPC0_MACDSA		BIT(20)
+#define FWPC0_MACSSA		BIT(23)
 #define FWPC0_MACHLA		BIT(26)
 #define FWPC0_MACHMA		BIT(27)
 #define FWPC0_VLANSA		BIT(28)
@@ -820,12 +827,18 @@ enum rswitch_gwca_mode {
 
 #define FWPC2(i)		(FWPC20 + (i) * 0x10)
 #define FWCP2_LTWFW		GENMASK(16 + (RSWITCH_NUM_AGENTS - 1), 16)
+#define FWCP2_LTWFW_MASK	GENMASK(16 + (RSWITCH_NUM_AGENTS - 1), 16)
 
 #define FWPBFC(i)		(FWPBFC0 + (i) * 0x10)
 #define FWPBFC_PBDV		GENMASK(RSWITCH_NUM_AGENTS - 1, 0)
 
 #define FWPBFCSDC(j, i)         (FWPBFCSDC00 + (i) * 0x10 + (j) * 0x04)
 
+#define FWMACHEC_MACHMUE_MASK	GENMASK(26, 16)
+
+#define FWMACTIM_MACTIOG	BIT(0)
+#define FWMACTIM_MACTR		BIT(1)
+
 #define FWMACAGUSPC_MACAGUSP	GENMASK(9, 0)
 #define FWMACAGC_MACAGT		GENMASK(15, 0)
 #define FWMACAGC_MACAGE		BIT(16)
@@ -1005,10 +1018,18 @@ struct rswitch_device {
 	DECLARE_BITMAP(ts_skb_used, TS_TAGS_PER_PORT);
 	bool disabled;
 
+	struct list_head list;
+
 	int port;
 	struct rswitch_etha *etha;
 	struct device_node *np_port;
 	struct phy *serdes;
+
+	struct net_device *brdev;	/* master bridge device */
+	unsigned int learning_requested : 1;
+	unsigned int learning_offloaded : 1;
+	unsigned int forwarding_requested : 1;
+	unsigned int forwarding_offloaded : 1;
 };
 
 struct rswitch_mfwd_mac_table_entry {
@@ -1033,11 +1054,17 @@ struct rswitch_private {
 	struct rswitch_etha etha[RSWITCH_NUM_PORTS];
 	struct rswitch_mfwd mfwd;
 
+	struct list_head port_list;
+
 	spinlock_t lock;	/* lock interrupt registers' control */
 	struct clk *clk;
 
 	bool etha_no_runtime_change;
 	bool gwca_halt;
+	struct net_device *offload_brdev;
 };
 
+bool is_rdev(const struct net_device *ndev);
+void rswitch_modify(void __iomem *addr, enum rswitch_reg reg, u32 clear, u32 set);
+
 #endif	/* #ifndef __RSWITCH_H__ */
diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c b/drivers/net/ethernet/renesas/rswitch_l2.c
new file mode 100644
index 0000000000000000000000000000000000000000..4177601000016b61def64735710052ba25222ff2
--- /dev/null
+++ b/drivers/net/ethernet/renesas/rswitch_l2.c
@@ -0,0 +1,318 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Renesas Ethernet Switch device driver
+ *
+ * Copyright (C) 2025 Renesas Electronics Corporation
+ */
+
+#include <linux/err.h>
+#include <linux/etherdevice.h>
+#include <linux/if_bridge.h>
+#include <linux/kernel.h>
+#include <net/switchdev.h>
+
+#include "rswitch.h"
+#include "rswitch_l2.h"
+
+static bool rdev_for_l2_offload(struct rswitch_device *rdev)
+{
+	return rdev->priv->offload_brdev &&
+	       rdev->brdev == rdev->priv->offload_brdev &&
+	       (test_bit(rdev->port, rdev->priv->opened_ports));
+}
+
+void rswitch_update_l2_offload(struct rswitch_private *priv)
+{
+	bool learning_needed, forwarding_needed;
+	unsigned int all_ports_mask, fwd_mask;
+	struct rswitch_device *rdev;
+
+	/* calculate fwd_mask with zeroes in bits corresponding to ports that
+	 * shall participate in hardware forwarding
+	 */
+	all_ports_mask = GENMASK(RSWITCH_NUM_AGENTS - 1, 0);
+	fwd_mask = all_ports_mask;
+
+	rswitch_for_all_ports(priv, rdev) {
+		if (rdev_for_l2_offload(rdev) && rdev->forwarding_requested)
+			fwd_mask &= ~BIT(rdev->port);
+	}
+
+	rswitch_for_all_ports(priv, rdev) {
+		if (rdev_for_l2_offload(rdev)) {
+			learning_needed = rdev->learning_requested;
+			forwarding_needed = rdev->forwarding_requested;
+		} else {
+			learning_needed = false;
+			forwarding_needed = false;
+		}
+
+		if (!rdev->learning_offloaded && learning_needed) {
+			rswitch_modify(priv->addr, FWPC0(rdev->port),
+				       0,
+				       FWPC0_MACSSA | FWPC0_MACHLA | FWPC0_MACHMA);
+
+			rdev->learning_offloaded = true;
+			netdev_info(rdev->ndev, "starting hw learning\n");
+		}
+
+		if (rdev->learning_offloaded && !learning_needed) {
+			rswitch_modify(priv->addr, FWPC0(rdev->port),
+				       FWPC0_MACSSA | FWPC0_MACHLA | FWPC0_MACHMA,
+				       0);
+
+			rdev->learning_offloaded = false;
+			netdev_info(rdev->ndev, "stopping hw learning\n");
+		}
+
+		if (forwarding_needed) {
+			/* Update allowed offload destinations even for ports
+			 * with L2 offload enabled earlier.
+			 *
+			 * Do not allow L2 forwarding to self for hw port.
+			 */
+			iowrite32(FIELD_PREP(FWCP2_LTWFW_MASK, fwd_mask | BIT(rdev->port)),
+				  priv->addr + FWPC2(rdev->port));
+
+			if (!rdev->forwarding_offloaded) {
+				rswitch_modify(priv->addr, FWPC0(rdev->port),
+					       0,
+					       FWPC0_MACDSA);
+
+				rdev->forwarding_offloaded = true;
+				netdev_info(rdev->ndev,
+					    "starting hw forwarding\n");
+			}
+		} else if (rdev->forwarding_offloaded) {
+			iowrite32(FIELD_PREP(FWCP2_LTWFW_MASK, fwd_mask | BIT(rdev->port)),
+				  priv->addr + FWPC2(rdev->port));
+
+			rswitch_modify(priv->addr, FWPC0(rdev->port),
+				       FWPC0_MACDSA,
+				       0);
+
+			rdev->forwarding_offloaded = false;
+			netdev_info(rdev->ndev, "stopping hw forwarding\n");
+		}
+	}
+}
+
+static void rswitch_update_offload_brdev(struct rswitch_private *priv,
+					 bool force_update_l2_offload)
+{
+	struct net_device *offload_brdev = NULL;
+	struct rswitch_device *rdev, *rdev2;
+
+	rswitch_for_all_ports(priv, rdev) {
+		if (!rdev->brdev)
+			continue;
+		rswitch_for_all_ports(priv, rdev2) {
+			if (rdev2 == rdev)
+				break;
+			if (rdev2->brdev == rdev->brdev) {
+				offload_brdev = rdev->brdev;
+				break;
+			}
+		}
+		if (offload_brdev)
+			break;
+	}
+
+	if (offload_brdev == priv->offload_brdev) {
+		if (offload_brdev && force_update_l2_offload)
+			rswitch_update_l2_offload(priv);
+		return;
+	}
+
+	if (offload_brdev && !priv->offload_brdev)
+		dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
+			netdev_name(offload_brdev));
+	else if (!offload_brdev && priv->offload_brdev)
+		dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
+			netdev_name(priv->offload_brdev));
+	else
+		dev_dbg(&priv->pdev->dev,
+			"changing l2 offload from %s to %s\n",
+			netdev_name(priv->offload_brdev),
+			netdev_name(offload_brdev));
+
+	priv->offload_brdev = offload_brdev;
+
+	rswitch_update_l2_offload(priv);
+}
+
+static bool rswitch_port_check(const struct net_device *ndev)
+{
+	return is_rdev(ndev);
+}
+
+static void rswitch_port_update_brdev(struct net_device *ndev,
+				      struct net_device *brdev)
+{
+	struct rswitch_device *rdev;
+
+	if (!is_rdev(ndev))
+		return;
+
+	rdev = netdev_priv(ndev);
+	rdev->brdev = brdev;
+	rswitch_update_offload_brdev(rdev->priv, false);
+}
+
+static int rswitch_port_update_stp_state(struct net_device *ndev, u8 stp_state)
+{
+	struct rswitch_device *rdev;
+
+	if (!is_rdev(ndev))
+		return -ENODEV;
+
+	rdev = netdev_priv(ndev);
+	rdev->learning_requested = (stp_state == BR_STATE_LEARNING ||
+				    stp_state == BR_STATE_FORWARDING);
+	rdev->forwarding_requested = (stp_state == BR_STATE_FORWARDING);
+	rswitch_update_l2_offload(rdev->priv);
+
+	return 0;
+}
+
+static int rswitch_netdevice_event(struct notifier_block *nb,
+				   unsigned long event, void *ptr)
+{
+	struct net_device *ndev = netdev_notifier_info_to_dev(ptr);
+	struct netdev_notifier_changeupper_info *info;
+	struct net_device *brdev;
+
+	if (!rswitch_port_check(ndev))
+		return NOTIFY_DONE;
+	if (event != NETDEV_CHANGEUPPER)
+		return NOTIFY_DONE;
+
+	info = ptr;
+
+	if (netif_is_bridge_master(info->upper_dev)) {
+		brdev = info->linking ? info->upper_dev : NULL;
+		rswitch_port_update_brdev(ndev, brdev);
+	}
+
+	return NOTIFY_OK;
+}
+
+static int rswitch_port_attr_set(struct net_device *ndev, const void *ctx,
+				 const struct switchdev_attr *attr,
+				 struct netlink_ext_ack *extack)
+{
+	switch (attr->id) {
+	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
+		return rswitch_port_update_stp_state(ndev, attr->u.stp_state);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int rswitch_port_obj_add(struct net_device *ndev, const void *ctx,
+				const struct switchdev_obj *obj,
+				struct netlink_ext_ack *extack)
+{
+	return -EOPNOTSUPP;
+}
+
+static int rswitch_port_obj_del(struct net_device *ndev, const void *ctx,
+				const struct switchdev_obj *obj)
+{
+	return -EOPNOTSUPP;
+}
+
+static int rswitch_switchdev_event(struct notifier_block *nb,
+				   unsigned long event, void *ptr)
+{
+	struct net_device *ndev = switchdev_notifier_info_to_dev(ptr);
+	int ret;
+
+	if (event == SWITCHDEV_PORT_ATTR_SET) {
+		ret = switchdev_handle_port_attr_set(ndev, ptr,
+						     rswitch_port_check,
+						     rswitch_port_attr_set);
+		return notifier_from_errno(ret);
+	}
+
+	if (!rswitch_port_check(ndev))
+		return NOTIFY_DONE;
+
+	return notifier_from_errno(-EOPNOTSUPP);
+}
+
+static int rswitch_switchdev_blocking_event(struct notifier_block *nb,
+					    unsigned long event, void *ptr)
+{
+	struct net_device *ndev = switchdev_notifier_info_to_dev(ptr);
+	int ret;
+
+	switch (event) {
+	case SWITCHDEV_PORT_OBJ_ADD:
+		ret = switchdev_handle_port_obj_add(ndev, ptr,
+						    rswitch_port_check,
+						    rswitch_port_obj_add);
+		break;
+	case SWITCHDEV_PORT_OBJ_DEL:
+		ret = switchdev_handle_port_obj_del(ndev, ptr,
+						    rswitch_port_check,
+						    rswitch_port_obj_del);
+		break;
+	case SWITCHDEV_PORT_ATTR_SET:
+		ret = switchdev_handle_port_attr_set(ndev, ptr,
+						     rswitch_port_check,
+						     rswitch_port_attr_set);
+		break;
+	default:
+		if (!rswitch_port_check(ndev))
+			return NOTIFY_DONE;
+		ret = -EOPNOTSUPP;
+	}
+
+	return notifier_from_errno(ret);
+}
+
+static struct notifier_block rswitch_netdevice_nb = {
+	.notifier_call = rswitch_netdevice_event,
+};
+
+static struct notifier_block rswitch_switchdev_nb = {
+	.notifier_call = rswitch_switchdev_event,
+};
+
+static struct notifier_block rswitch_switchdev_blocking_nb = {
+	.notifier_call = rswitch_switchdev_blocking_event,
+};
+
+int rswitch_register_notifiers(void)
+{
+	int ret;
+
+	ret = register_netdevice_notifier(&rswitch_netdevice_nb);
+	if (ret)
+		goto register_netdevice_notifier_failed;
+
+	ret = register_switchdev_notifier(&rswitch_switchdev_nb);
+	if (ret)
+		goto register_switchdev_notifier_failed;
+
+	ret = register_switchdev_blocking_notifier(&rswitch_switchdev_blocking_nb);
+	if (ret)
+		goto register_switchdev_blocking_notifier_failed;
+
+	return 0;
+
+register_switchdev_blocking_notifier_failed:
+	unregister_switchdev_notifier(&rswitch_switchdev_nb);
+register_switchdev_notifier_failed:
+	unregister_netdevice_notifier(&rswitch_netdevice_nb);
+register_netdevice_notifier_failed:
+
+	return ret;
+}
+
+void rswitch_unregister_notifiers(void)
+{
+	unregister_switchdev_blocking_notifier(&rswitch_switchdev_blocking_nb);
+	unregister_switchdev_notifier(&rswitch_switchdev_nb);
+	unregister_netdevice_notifier(&rswitch_netdevice_nb);
+}
diff --git a/drivers/net/ethernet/renesas/rswitch_l2.h b/drivers/net/ethernet/renesas/rswitch_l2.h
new file mode 100644
index 0000000000000000000000000000000000000000..57050ede8f31848cde5a497811a6ee1b60dedc65
--- /dev/null
+++ b/drivers/net/ethernet/renesas/rswitch_l2.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Renesas Ethernet Switch device driver
+ *
+ * Copyright (C) 2025 Renesas Electronics Corporation
+ */
+
+#ifndef __RSWITCH_L2_H__
+#define __RSWITCH_L2_H__
+
+void rswitch_update_l2_offload(struct rswitch_private *priv);
+
+int rswitch_register_notifiers(void);
+void rswitch_unregister_notifiers(void);
+
+#endif	/* #ifndef __RSWITCH_L2_H__ */
diff --git a/drivers/net/ethernet/renesas/rswitch_main.c b/drivers/net/ethernet/renesas/rswitch_main.c
index 2474271cac7f057fffb472131f5a7a72a0fa9a87..7814bd08ee16ca32ba935a19ea2a1cd1fd41a236 100644
--- a/drivers/net/ethernet/renesas/rswitch_main.c
+++ b/drivers/net/ethernet/renesas/rswitch_main.c
@@ -8,8 +8,11 @@
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/etherdevice.h>
+#include <linux/ethtool.h>
+#include <linux/ip.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
+#include <linux/list.h>
 #include <linux/module.h>
 #include <linux/net_tstamp.h>
 #include <linux/of.h>
@@ -25,6 +28,7 @@
 #include <linux/sys_soc.h>
 
 #include "rswitch.h"
+#include "rswitch_l2.h"
 
 static int rswitch_reg_wait(void __iomem *addr, u32 offs, u32 mask, u32 expected)
 {
@@ -34,7 +38,7 @@ static int rswitch_reg_wait(void __iomem *addr, u32 offs, u32 mask, u32 expected
 					 1, RSWITCH_TIMEOUT_US);
 }
 
-static void rswitch_modify(void __iomem *addr, enum rswitch_reg reg, u32 clear, u32 set)
+void rswitch_modify(void __iomem *addr, enum rswitch_reg reg, u32 clear, u32 set)
 {
 	iowrite32((ioread32(addr + reg) & ~clear) | set, addr + reg);
 }
@@ -109,11 +113,12 @@ static void rswitch_top_init(struct rswitch_private *priv)
 }
 
 /* Forwarding engine block (MFWD) */
-static void rswitch_fwd_init(struct rswitch_private *priv)
+static int rswitch_fwd_init(struct rswitch_private *priv)
 {
 	u32 all_ports_mask = GENMASK(RSWITCH_NUM_AGENTS - 1, 0);
 	unsigned int i;
 	u32 reg_val;
+	int ret;
 
 	/* Start with empty configuration */
 	for (i = 0; i < RSWITCH_NUM_AGENTS; i++) {
@@ -149,6 +154,17 @@ static void rswitch_fwd_init(struct rswitch_private *priv)
 
 	/* For GWCA port, allow direct descriptor forwarding */
 	rswitch_modify(priv->addr, FWPC1(priv->gwca.index), FWPC1_DDE, FWPC1_DDE);
+
+	/* Initialize hardware L2 forwarding table */
+
+	/* Allow entire table to be used for "unsecure" entries */
+	rswitch_modify(priv->addr, FWMACHEC, 0, FWMACHEC_MACHMUE_MASK);
+
+	/* Initialize MAC hash table */
+	iowrite32(FWMACTIM_MACTIOG, priv->addr + FWMACTIM);
+	ret = rswitch_reg_wait(priv->addr, FWMACTIM, FWMACTIM_MACTIOG, 0);
+
+	return ret;
 }
 
 /* Gateway CPU agent block (GWCA) */
@@ -1611,6 +1627,9 @@ static int rswitch_open(struct net_device *ndev)
 
 	netif_start_queue(ndev);
 
+	if (rdev->brdev)
+		rswitch_update_l2_offload(rdev->priv);
+
 	return 0;
 };
 
@@ -1633,6 +1652,9 @@ static int rswitch_stop(struct net_device *ndev)
 
 	napi_disable(&rdev->napi);
 
+	if (rdev->brdev)
+		rswitch_update_l2_offload(rdev->priv);
+
 	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS))
 		iowrite32(GWCA_TS_IRQ_BIT, rdev->priv->addr + GWTSDID);
 
@@ -1859,16 +1881,46 @@ static int rswitch_eth_ioctl(struct net_device *ndev, struct ifreq *req, int cmd
 	}
 }
 
+static int rswitch_get_port_parent_id(struct net_device *ndev,
+				      struct netdev_phys_item_id *ppid)
+{
+	struct rswitch_device *rdev = netdev_priv(ndev);
+	const char *name;
+
+	name = dev_name(&rdev->priv->pdev->dev);
+	ppid->id_len = min_t(size_t, strlen(name), sizeof(ppid->id_len));
+	memcpy(ppid->id, name, ppid->id_len);
+
+	return 0;
+}
+
+static int rswitch_get_phys_port_name(struct net_device *ndev,
+				      char *name, size_t len)
+{
+	struct rswitch_device *rdev = netdev_priv(ndev);
+
+	snprintf(name, len, "tsn%d", rdev->port);
+
+	return 0;
+}
+
 static const struct net_device_ops rswitch_netdev_ops = {
 	.ndo_open = rswitch_open,
 	.ndo_stop = rswitch_stop,
 	.ndo_start_xmit = rswitch_start_xmit,
 	.ndo_get_stats = rswitch_get_stats,
 	.ndo_eth_ioctl = rswitch_eth_ioctl,
+	.ndo_get_port_parent_id = rswitch_get_port_parent_id,
+	.ndo_get_phys_port_name = rswitch_get_phys_port_name,
 	.ndo_validate_addr = eth_validate_addr,
 	.ndo_set_mac_address = eth_mac_addr,
 };
 
+bool is_rdev(const struct net_device *ndev)
+{
+	return (ndev->netdev_ops == &rswitch_netdev_ops);
+}
+
 static int rswitch_get_ts_info(struct net_device *ndev, struct kernel_ethtool_ts_info *info)
 {
 	struct rswitch_device *rdev = netdev_priv(ndev);
@@ -1968,6 +2020,8 @@ static int rswitch_device_alloc(struct rswitch_private *priv, unsigned int index
 	if (err < 0)
 		goto out_txdmac;
 
+	list_add_tail(&rdev->list, &priv->port_list);
+
 	return 0;
 
 out_txdmac:
@@ -1987,6 +2041,7 @@ static void rswitch_device_free(struct rswitch_private *priv, unsigned int index
 	struct rswitch_device *rdev = priv->rdev[index];
 	struct net_device *ndev = rdev->ndev;
 
+	list_del(&rdev->list);
 	rswitch_txdmac_free(ndev);
 	rswitch_rxdmac_free(ndev);
 	of_node_put(rdev->np_port);
@@ -2033,7 +2088,9 @@ static int rswitch_init(struct rswitch_private *priv)
 		}
 	}
 
-	rswitch_fwd_init(priv);
+	err = rswitch_fwd_init(priv);
+	if (err < 0)
+		goto err_fwd_init;
 
 	err = rcar_gen4_ptp_register(priv->ptp_priv, RCAR_GEN4_PTP_REG_LAYOUT,
 				     clk_get_rate(priv->clk));
@@ -2082,6 +2139,7 @@ static int rswitch_init(struct rswitch_private *priv)
 err_gwca_request_irq:
 	rcar_gen4_ptp_unregister(priv->ptp_priv);
 
+err_fwd_init:
 err_ptp_register:
 	for (i = 0; i < RSWITCH_NUM_PORTS; i++)
 		rswitch_device_free(priv, i);
@@ -2116,6 +2174,7 @@ static int renesas_eth_sw_probe(struct platform_device *pdev)
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
+
 	spin_lock_init(&priv->lock);
 
 	priv->clk = devm_clk_get(&pdev->dev, NULL);
@@ -2153,6 +2212,8 @@ static int renesas_eth_sw_probe(struct platform_device *pdev)
 	if (!priv->gwca.queues)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&priv->port_list);
+
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
@@ -2163,6 +2224,15 @@ static int renesas_eth_sw_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	if (list_empty(&priv->port_list))
+		dev_warn(&pdev->dev, "could not initialize any ports\n");
+
+	ret = rswitch_register_notifiers();
+	if (ret) {
+		dev_err(&pdev->dev, "could not register notifiers\n");
+		return ret;
+	}
+
 	device_set_wakeup_capable(&pdev->dev, 1);
 
 	return ret;
@@ -2196,6 +2266,7 @@ static void renesas_eth_sw_remove(struct platform_device *pdev)
 {
 	struct rswitch_private *priv = platform_get_drvdata(pdev);
 
+	rswitch_unregister_notifiers();
 	rswitch_deinit(priv);
 
 	pm_runtime_put(&pdev->dev);

-- 
2.49.0

Re: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching

[Simon Horman]

On Tue, Jul 08, 2025 at 11:27:39AM +0200, Michael Dege wrote:
> This commit adds hardware offloading for L2 switching on R-Car S4.
> 
> On S4 brdev is limited to one per-device (not per port). Reasoning
> is that hw L2 forwarding support lacks any sort of source port based
> filtering, which makes it unusable to offload more than one bridge
> device. Either you allow hardware to forward destination MAC to a
> port, or you have to send it to CPU. You can't make it forward only
> if src and dst ports are in the same brdev.
> 
> Signed-off-by: Michael Dege <michael.dege@renesas.com>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>

...

> diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c b/drivers/net/ethernet/renesas/rswitch_l2.c

...

> +static void rswitch_update_offload_brdev(struct rswitch_private *priv,
> +					 bool force_update_l2_offload)
> +{
> +	struct net_device *offload_brdev = NULL;
> +	struct rswitch_device *rdev, *rdev2;
> +
> +	rswitch_for_all_ports(priv, rdev) {
> +		if (!rdev->brdev)
> +			continue;
> +		rswitch_for_all_ports(priv, rdev2) {
> +			if (rdev2 == rdev)
> +				break;
> +			if (rdev2->brdev == rdev->brdev) {
> +				offload_brdev = rdev->brdev;
> +				break;
> +			}
> +		}
> +		if (offload_brdev)
> +			break;
> +	}
> +
> +	if (offload_brdev == priv->offload_brdev) {
> +		if (offload_brdev && force_update_l2_offload)
> +			rswitch_update_l2_offload(priv);
> +		return;
> +	}
> +
> +	if (offload_brdev && !priv->offload_brdev)
> +		dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
> +			netdev_name(offload_brdev));
> +	else if (!offload_brdev && priv->offload_brdev)
> +		dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
> +			netdev_name(priv->offload_brdev));
> +	else
> +		dev_dbg(&priv->pdev->dev,
> +			"changing l2 offload from %s to %s\n",
> +			netdev_name(priv->offload_brdev),
> +			netdev_name(offload_brdev));

Smatch flags a false-positive about possible NULL references by the
netdev_name() calls on the line above.

Due to the previous if statement it seems to me that cannot occur.
But it did take me a few moments to convince myself of that.

So while I don't think we should write our code to static analysis tooling.
I did play around a bit to see if I could come up with something that is
both easier on the eyes and keeps Smatch happy.

Perhaps it isn't easier on the eyes, but rather I'm just more familiar with
the code now. But in any case, I'm sharing what I came up with in case it
is useful. (Compile tested only!).


        if (!offload_brdev && !priv->offload_brdev)
                return;

        if (!priv->offload_brdev)
                dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
                        netdev_name(offload_brdev));
        else if (!offload_brdev)
                dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
                        netdev_name(priv->offload_brdev));
        else if (offload_brdev != priv->offload_brdev)
                dev_dbg(&priv->pdev->dev,
                        "changing l2 offload from %s to %s\n",
                        netdev_name(priv->offload_brdev),
                        netdev_name(offload_brdev));
        else if (!force_update_l2_offload)
                return;

> +
> +	priv->offload_brdev = offload_brdev;
> +
> +	rswitch_update_l2_offload(priv);
> +}

...

RE: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching

[Michael Dege]

Hello Simon,

Thank you for your comments.

> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Tuesday, July 8, 2025 12:48 PM
> To: Michael Dege <michael.dege@renesas.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>; Niklas Söderlund
> <niklas.soderlund@ragnatech.se>; Paul Barker <paul@pbarker.dev>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; linux-kernel@vger.kernel.org; Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Subject: Re: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching
>
> On Tue, Jul 08, 2025 at 11:27:39AM +0200, Michael Dege wrote:
> > This commit adds hardware offloading for L2 switching on R-Car S4.
> >
> > On S4 brdev is limited to one per-device (not per port). Reasoning is
> > that hw L2 forwarding support lacks any sort of source port based
> > filtering, which makes it unusable to offload more than one bridge
> > device. Either you allow hardware to forward destination MAC to a
> > port, or you have to send it to CPU. You can't make it forward only if
> > src and dst ports are in the same brdev.
> >
> > Signed-off-by: Michael Dege <michael.dege@renesas.com>
> > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
>
> ...
>
> > diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c
> > b/drivers/net/ethernet/renesas/rswitch_l2.c
>
> ...
>
> > +static void rswitch_update_offload_brdev(struct rswitch_private *priv,
> > +                                    bool force_update_l2_offload)
> > +{
> > +   struct net_device *offload_brdev = NULL;
> > +   struct rswitch_device *rdev, *rdev2;
> > +
> > +   rswitch_for_all_ports(priv, rdev) {
> > +           if (!rdev->brdev)
> > +                   continue;
> > +           rswitch_for_all_ports(priv, rdev2) {
> > +                   if (rdev2 == rdev)
> > +                           break;
> > +                   if (rdev2->brdev == rdev->brdev) {
> > +                           offload_brdev = rdev->brdev;
> > +                           break;
> > +                   }
> > +           }
> > +           if (offload_brdev)
> > +                   break;
> > +   }
> > +
> > +   if (offload_brdev == priv->offload_brdev) {
> > +           if (offload_brdev && force_update_l2_offload)
> > +                   rswitch_update_l2_offload(priv);
> > +           return;
> > +   }
> > +
> > +   if (offload_brdev && !priv->offload_brdev)
> > +           dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
> > +                   netdev_name(offload_brdev));
> > +   else if (!offload_brdev && priv->offload_brdev)
> > +           dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
> > +                   netdev_name(priv->offload_brdev));
> > +   else
> > +           dev_dbg(&priv->pdev->dev,
> > +                   "changing l2 offload from %s to %s\n",
> > +                   netdev_name(priv->offload_brdev),
> > +                   netdev_name(offload_brdev));
>
> Smatch flags a false-positive about possible NULL references by the
> netdev_name() calls on the line above.
>
> Due to the previous if statement it seems to me that cannot occur.
> But it did take me a few moments to convince myself of that.
>
> So while I don't think we should write our code to static analysis tooling.
> I did play around a bit to see if I could come up with something that is both easier on the eyes and
> keeps Smatch happy.
>
> Perhaps it isn't easier on the eyes, but rather I'm just more familiar with the code now. But in any
> case, I'm sharing what I came up with in case it is useful. (Compile tested only!).
>
>
>         if (!offload_brdev && !priv->offload_brdev)
>                 return;
>
>         if (!priv->offload_brdev)
>                 dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
>                         netdev_name(offload_brdev));
>         else if (!offload_brdev)
>                 dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
>                         netdev_name(priv->offload_brdev));
>         else if (offload_brdev != priv->offload_brdev)
>                 dev_dbg(&priv->pdev->dev,
>                         "changing l2 offload from %s to %s\n",
>                         netdev_name(priv->offload_brdev),
>                         netdev_name(offload_brdev));
>         else if (!force_update_l2_offload)
>                 return;
>

I will integrate this into the code and test it.

Best regards,

Michael

> > +
> > +   priv->offload_brdev = offload_brdev;
> > +
> > +   rswitch_update_l2_offload(priv);
> > +}
>
> ...
________________________________

Renesas Electronics Europe GmbH
Registered Office: Arcadiastrasse 10
DE-40472 Duesseldorf
Commercial Registry: Duesseldorf, HRB 3708
Managing Director: Carsten Jauch
VAT-No.: DE 14978647
Tax-ID-No: 105/5839/1793

Legal Disclaimer: This e-mail communication (and any attachment/s) is confidential and contains proprietary information, some or all of which may be legally privileged. It is intended solely for the use of the individual or entity to which it is addressed. Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.

RE: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching

[Michael Dege]

Hello Simon,

> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Tuesday, July 8, 2025 12:48 PM
> To: Michael Dege <michael.dege@renesas.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>; Niklas Söderlund
> <niklas.soderlund@ragnatech.se>; Paul Barker <paul@pbarker.dev>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; linux-kernel@vger.kernel.org; Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Subject: Re: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching
>
> On Tue, Jul 08, 2025 at 11:27:39AM +0200, Michael Dege wrote:
> > This commit adds hardware offloading for L2 switching on R-Car S4.
> >
> > On S4 brdev is limited to one per-device (not per port). Reasoning is
> > that hw L2 forwarding support lacks any sort of source port based
> > filtering, which makes it unusable to offload more than one bridge
> > device. Either you allow hardware to forward destination MAC to a
> > port, or you have to send it to CPU. You can't make it forward only if
> > src and dst ports are in the same brdev.
> >
> > Signed-off-by: Michael Dege <michael.dege@renesas.com>
> > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
>
> ...
>
> > diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c
> > b/drivers/net/ethernet/renesas/rswitch_l2.c
>
> ...
>
> > +static void rswitch_update_offload_brdev(struct rswitch_private *priv,
> > +                                    bool force_update_l2_offload)
> > +{
> > +   struct net_device *offload_brdev = NULL;
> > +   struct rswitch_device *rdev, *rdev2;
> > +
> > +   rswitch_for_all_ports(priv, rdev) {
> > +           if (!rdev->brdev)
> > +                   continue;
> > +           rswitch_for_all_ports(priv, rdev2) {
> > +                   if (rdev2 == rdev)
> > +                           break;
> > +                   if (rdev2->brdev == rdev->brdev) {
> > +                           offload_brdev = rdev->brdev;
> > +                           break;
> > +                   }
> > +           }
> > +           if (offload_brdev)
> > +                   break;
> > +   }
> > +
> > +   if (offload_brdev == priv->offload_brdev) {
> > +           if (offload_brdev && force_update_l2_offload)
> > +                   rswitch_update_l2_offload(priv);
> > +           return;
> > +   }
> > +
> > +   if (offload_brdev && !priv->offload_brdev)
> > +           dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
> > +                   netdev_name(offload_brdev));
> > +   else if (!offload_brdev && priv->offload_brdev)
> > +           dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
> > +                   netdev_name(priv->offload_brdev));
> > +   else
> > +           dev_dbg(&priv->pdev->dev,
> > +                   "changing l2 offload from %s to %s\n",
> > +                   netdev_name(priv->offload_brdev),
> > +                   netdev_name(offload_brdev));
>
> Smatch flags a false-positive about possible NULL references by the
> netdev_name() calls on the line above.
>
> Due to the previous if statement it seems to me that cannot occur.
> But it did take me a few moments to convince myself of that.
>
> So while I don't think we should write our code to static analysis tooling.
> I did play around a bit to see if I could come up with something that is both easier on the eyes and
> keeps Smatch happy.
>
> Perhaps it isn't easier on the eyes, but rather I'm just more familiar with the code now. But in any
> case, I'm sharing what I came up with in case it is useful. (Compile tested only!).
>
>
>         if (!offload_brdev && !priv->offload_brdev)
>                 return;
>
>         if (!priv->offload_brdev)
>                 dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
>                         netdev_name(offload_brdev));
>         else if (!offload_brdev)
>                 dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
>                         netdev_name(priv->offload_brdev));
>         else if (offload_brdev != priv->offload_brdev)
>                 dev_dbg(&priv->pdev->dev,
>                         "changing l2 offload from %s to %s\n",
>                         netdev_name(priv->offload_brdev),
>                         netdev_name(offload_brdev));
>         else if (!force_update_l2_offload)
>                 return;
>

I updated the code, I hope it is OK, because I had to do it differently from your suggestion, because
not all cases worked as expected.

The reworked code is tested.

Best regards,

Michael

> > +
> > +   priv->offload_brdev = offload_brdev;
> > +
> > +   rswitch_update_l2_offload(priv);
> > +}
>
> ...
________________________________

Renesas Electronics Europe GmbH
Registered Office: Arcadiastrasse 10
DE-40472 Duesseldorf
Commercial Registry: Duesseldorf, HRB 3708
Managing Director: Carsten Jauch
VAT-No.: DE 14978647
Tax-ID-No: 105/5839/1793

Legal Disclaimer: This e-mail communication (and any attachment/s) is confidential and contains proprietary information, some or all of which may be legally privileged. It is intended solely for the use of the individual or entity to which it is addressed. Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.

Re: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching

[Simon Horman]

On Thu, Jul 10, 2025 at 12:36:10PM +0000, Michael Dege wrote:
> Hello Simon,
> 
> > -----Original Message-----
> > From: Simon Horman <horms@kernel.org>
> > Sent: Tuesday, July 8, 2025 12:48 PM
> > To: Michael Dege <michael.dege@renesas.com>
> > Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>; Niklas Söderlund
> > <niklas.soderlund@ragnatech.se>; Paul Barker <paul@pbarker.dev>; Andrew Lunn <andrew+netdev@lunn.ch>;
> > David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> > <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; linux-renesas-
> > soc@vger.kernel.org; linux-kernel@vger.kernel.org; Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> > Subject: Re: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching
> >
> > On Tue, Jul 08, 2025 at 11:27:39AM +0200, Michael Dege wrote:
> > > This commit adds hardware offloading for L2 switching on R-Car S4.
> > >
> > > On S4 brdev is limited to one per-device (not per port). Reasoning is
> > > that hw L2 forwarding support lacks any sort of source port based
> > > filtering, which makes it unusable to offload more than one bridge
> > > device. Either you allow hardware to forward destination MAC to a
> > > port, or you have to send it to CPU. You can't make it forward only if
> > > src and dst ports are in the same brdev.
> > >
> > > Signed-off-by: Michael Dege <michael.dege@renesas.com>
> > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> >
> > ...
> >
> > > diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c
> > > b/drivers/net/ethernet/renesas/rswitch_l2.c
> >
> > ...
> >
> > > +static void rswitch_update_offload_brdev(struct rswitch_private *priv,
> > > +                                    bool force_update_l2_offload)
> > > +{
> > > +   struct net_device *offload_brdev = NULL;
> > > +   struct rswitch_device *rdev, *rdev2;
> > > +
> > > +   rswitch_for_all_ports(priv, rdev) {
> > > +           if (!rdev->brdev)
> > > +                   continue;
> > > +           rswitch_for_all_ports(priv, rdev2) {
> > > +                   if (rdev2 == rdev)
> > > +                           break;
> > > +                   if (rdev2->brdev == rdev->brdev) {
> > > +                           offload_brdev = rdev->brdev;
> > > +                           break;
> > > +                   }
> > > +           }
> > > +           if (offload_brdev)
> > > +                   break;
> > > +   }
> > > +
> > > +   if (offload_brdev == priv->offload_brdev) {
> > > +           if (offload_brdev && force_update_l2_offload)
> > > +                   rswitch_update_l2_offload(priv);
> > > +           return;
> > > +   }
> > > +
> > > +   if (offload_brdev && !priv->offload_brdev)
> > > +           dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
> > > +                   netdev_name(offload_brdev));
> > > +   else if (!offload_brdev && priv->offload_brdev)
> > > +           dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
> > > +                   netdev_name(priv->offload_brdev));
> > > +   else
> > > +           dev_dbg(&priv->pdev->dev,
> > > +                   "changing l2 offload from %s to %s\n",
> > > +                   netdev_name(priv->offload_brdev),
> > > +                   netdev_name(offload_brdev));
> >
> > Smatch flags a false-positive about possible NULL references by the
> > netdev_name() calls on the line above.
> >
> > Due to the previous if statement it seems to me that cannot occur.
> > But it did take me a few moments to convince myself of that.
> >
> > So while I don't think we should write our code to static analysis tooling.
> > I did play around a bit to see if I could come up with something that is both easier on the eyes and
> > keeps Smatch happy.
> >
> > Perhaps it isn't easier on the eyes, but rather I'm just more familiar with the code now. But in any
> > case, I'm sharing what I came up with in case it is useful. (Compile tested only!).
> >
> >
> >         if (!offload_brdev && !priv->offload_brdev)
> >                 return;
> >
> >         if (!priv->offload_brdev)
> >                 dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
> >                         netdev_name(offload_brdev));
> >         else if (!offload_brdev)
> >                 dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
> >                         netdev_name(priv->offload_brdev));
> >         else if (offload_brdev != priv->offload_brdev)
> >                 dev_dbg(&priv->pdev->dev,
> >                         "changing l2 offload from %s to %s\n",
> >                         netdev_name(priv->offload_brdev),
> >                         netdev_name(offload_brdev));
> >         else if (!force_update_l2_offload)
> >                 return;
> >
> 
> I updated the code, I hope it is OK, because I had to do it differently from your suggestion, because
> not all cases worked as expected.
> 
> The reworked code is tested.

Thanks. FWIIW, Smatch still complains.

But if your code is correct and tested, then I think we should not
update it a 2nd time to make the tooling happy.

Re: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching

[Andrew Lunn]

On Tue, Jul 08, 2025 at 11:27:39AM +0200, Michael Dege wrote:
> This commit adds hardware offloading for L2 switching on R-Car S4.
> 
> On S4 brdev is limited to one per-device (not per port). Reasoning
> is that hw L2 forwarding support lacks any sort of source port based
> filtering, which makes it unusable to offload more than one bridge
> device. Either you allow hardware to forward destination MAC to a
> port, or you have to send it to CPU. You can't make it forward only
> if src and dst ports are in the same brdev.
> 
> Signed-off-by: Michael Dege <michael.dege@renesas.com>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>

Since you are submitting this, your Signed-off-by: should be last.

	Andrew

RE: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching

[Michael Dege]

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Tuesday, July 8, 2025 11:53 PM
> To: Michael Dege <michael.dege@renesas.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>; Niklas Söderlund
> <niklas.soderlund@ragnatech.se>; Paul Barker <paul@pbarker.dev>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; linux-kernel@vger.kernel.org; Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Subject: Re: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching
>
> On Tue, Jul 08, 2025 at 11:27:39AM +0200, Michael Dege wrote:
> > This commit adds hardware offloading for L2 switching on R-Car S4.
> >
> > On S4 brdev is limited to one per-device (not per port). Reasoning is
> > that hw L2 forwarding support lacks any sort of source port based
> > filtering, which makes it unusable to offload more than one bridge
> > device. Either you allow hardware to forward destination MAC to a
> > port, or you have to send it to CPU. You can't make it forward only if
> > src and dst ports are in the same brdev.
> >
> > Signed-off-by: Michael Dege <michael.dege@renesas.com>
> > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
>
> Since you are submitting this, your Signed-off-by: should be last.
>
>       Andrew

This is fixed in v3.

Best regards,

Michael
________________________________

Renesas Electronics Europe GmbH
Registered Office: Arcadiastrasse 10
DE-40472 Duesseldorf
Commercial Registry: Duesseldorf, HRB 3708
Managing Director: Carsten Jauch
VAT-No.: DE 14978647
Tax-ID-No: 105/5839/1793

Legal Disclaimer: This e-mail communication (and any attachment/s) is confidential and contains proprietary information, some or all of which may be legally privileged. It is intended solely for the use of the individual or entity to which it is addressed. Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.

Re: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching

[Andrew Lunn]

> +void rswitch_update_l2_offload(struct rswitch_private *priv)
> +{
> +	bool learning_needed, forwarding_needed;
> +	unsigned int all_ports_mask, fwd_mask;
> +	struct rswitch_device *rdev;
> +
> +	/* calculate fwd_mask with zeroes in bits corresponding to ports that
> +	 * shall participate in hardware forwarding
> +	 */
> +	all_ports_mask = GENMASK(RSWITCH_NUM_AGENTS - 1, 0);
> +	fwd_mask = all_ports_mask;
> +
> +	rswitch_for_all_ports(priv, rdev) {
> +		if (rdev_for_l2_offload(rdev) && rdev->forwarding_requested)
> +			fwd_mask &= ~BIT(rdev->port);
> +	}
> +
> +	rswitch_for_all_ports(priv, rdev) {
> +		if (rdev_for_l2_offload(rdev)) {
> +			learning_needed = rdev->learning_requested;
> +			forwarding_needed = rdev->forwarding_requested;
> +		} else {
> +			learning_needed = false;
> +			forwarding_needed = false;
> +		}
> +
> +		if (!rdev->learning_offloaded && learning_needed) {
> +			rswitch_modify(priv->addr, FWPC0(rdev->port),
> +				       0,
> +				       FWPC0_MACSSA | FWPC0_MACHLA | FWPC0_MACHMA);
> +
> +			rdev->learning_offloaded = true;
> +			netdev_info(rdev->ndev, "starting hw learning\n");
> +		}
> +
> +		if (rdev->learning_offloaded && !learning_needed) {
> +			rswitch_modify(priv->addr, FWPC0(rdev->port),
> +				       FWPC0_MACSSA | FWPC0_MACHLA | FWPC0_MACHMA,
> +				       0);
> +
> +			rdev->learning_offloaded = false;
> +			netdev_info(rdev->ndev, "stopping hw learning\n");
> +		}
> +
> +		if (forwarding_needed) {
> +			/* Update allowed offload destinations even for ports
> +			 * with L2 offload enabled earlier.
> +			 *
> +			 * Do not allow L2 forwarding to self for hw port.
> +			 */
> +			iowrite32(FIELD_PREP(FWCP2_LTWFW_MASK, fwd_mask | BIT(rdev->port)),
> +				  priv->addr + FWPC2(rdev->port));
> +
> +			if (!rdev->forwarding_offloaded) {
> +				rswitch_modify(priv->addr, FWPC0(rdev->port),
> +					       0,
> +					       FWPC0_MACDSA);
> +
> +				rdev->forwarding_offloaded = true;
> +				netdev_info(rdev->ndev,
> +					    "starting hw forwarding\n");
> +			}
> +		} else if (rdev->forwarding_offloaded) {
> +			iowrite32(FIELD_PREP(FWCP2_LTWFW_MASK, fwd_mask | BIT(rdev->port)),
> +				  priv->addr + FWPC2(rdev->port));
> +
> +			rswitch_modify(priv->addr, FWPC0(rdev->port),
> +				       FWPC0_MACDSA,
> +				       0);
> +
> +			rdev->forwarding_offloaded = false;
> +			netdev_info(rdev->ndev, "stopping hw forwarding\n");
> +		}
> +	}
> +}

This function seems overly complicated.

Normally you can change a ports STP state without needing to consider
other ports. Can you separate STP from joining ports to a bridge? That
might help simplify this function, break it up into two functions.

> +static int rswitch_port_obj_add(struct net_device *ndev, const void *ctx,
> +				const struct switchdev_obj *obj,
> +				struct netlink_ext_ack *extack)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int rswitch_port_obj_del(struct net_device *ndev, const void *ctx,
> +				const struct switchdev_obj *obj)
> +{
> +	return -EOPNOTSUPP;
> +}

> +static int rswitch_switchdev_blocking_event(struct notifier_block *nb,
> +					    unsigned long event, void *ptr)
> +{
> +	struct net_device *ndev = switchdev_notifier_info_to_dev(ptr);
> +	int ret;
> +
> +	switch (event) {
> +	case SWITCHDEV_PORT_OBJ_ADD:
> +		ret = switchdev_handle_port_obj_add(ndev, ptr,
> +						    rswitch_port_check,
> +						    rswitch_port_obj_add);
> +		break;
> +	case SWITCHDEV_PORT_OBJ_DEL:
> +		ret = switchdev_handle_port_obj_del(ndev, ptr,
> +						    rswitch_port_check,
> +						    rswitch_port_obj_del);
> +		break;

Since these two are hard coded to return EOPNOTSUPP, it seems like you
could just return that here?

>  /* Forwarding engine block (MFWD) */
> -static void rswitch_fwd_init(struct rswitch_private *priv)
> +static int rswitch_fwd_init(struct rswitch_private *priv)
>  {
> +	/* Initialize MAC hash table */
> +	iowrite32(FWMACTIM_MACTIOG, priv->addr + FWMACTIM);
> +	ret = rswitch_reg_wait(priv->addr, FWMACTIM, FWMACTIM_MACTIOG, 0);
> +
> +	return ret;

Just

	return rswitch_reg_wait(priv->addr, FWMACTIM, FWMACTIM_MACTIOG, 0);

	Andrew

[PATCH v2 4/4] net: renesas: rswitch: add modifiable ageing time

[Michael Dege]

This commit allows the setting of the MAC table aging in the R-Car S4
Rswitch using the SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME attribute.

Signed-off-by: Michael Dege <michael.dege@renesas.com>
---
 drivers/net/ethernet/renesas/rswitch_l2.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c b/drivers/net/ethernet/renesas/rswitch_l2.c
index 4177601000016b61def64735710052ba25222ff2..3d47ac668b4f5dfca87a192f2fc020597bef115d 100644
--- a/drivers/net/ethernet/renesas/rswitch_l2.c
+++ b/drivers/net/ethernet/renesas/rswitch_l2.c
@@ -196,6 +196,25 @@ static int rswitch_netdevice_event(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
+static int rswitch_update_ageing_time(struct net_device *ndev, clock_t time)
+{
+	struct rswitch_device *rdev = netdev_priv(ndev);
+	u32 reg_val;
+
+	if (!is_rdev(ndev))
+		return -ENODEV;
+
+	if (!FIELD_FIT(FWMACAGC_MACAGT, time))
+		return -EINVAL;
+
+	rdev = netdev_priv(ndev);
+	reg_val = FIELD_PREP(FWMACAGC_MACAGT, time);
+	reg_val |= FWMACAGC_MACAGE | FWMACAGC_MACAGSL;
+	iowrite32(reg_val, rdev->priv->addr + FWMACAGC);
+
+	return 0;
+}
+
 static int rswitch_port_attr_set(struct net_device *ndev, const void *ctx,
 				 const struct switchdev_attr *attr,
 				 struct netlink_ext_ack *extack)
@@ -203,6 +222,8 @@ static int rswitch_port_attr_set(struct net_device *ndev, const void *ctx,
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
 		return rswitch_port_update_stp_state(ndev, attr->u.stp_state);
+	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
+		return rswitch_update_ageing_time(ndev, attr->u.ageing_time);
 	default:
 		return -EOPNOTSUPP;
 	}

-- 
2.49.0