linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
@ 2025-07-04  5:51 Michael Dege
  2025-07-04  5:51 ` [PATCH 1/3] net: renesas: rswitch: rename rswitch.c to rswitch_main.c Michael Dege
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Michael Dege @ 2025-07-04  5:51 UTC (permalink / raw)
  To: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: netdev, linux-renesas-soc, linux-kernel, Michael Dege,
	Nikita Yushchenko

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 the L2 driver extension in a separate file. The HW offloading
is automatically configured when a port is added to the bridge device.

Ussage example:
ip link add name br type bridge
ip link set dev tsn0 master br
ip link set dev tsn1 master br
ip link set dev br 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.

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

Usage example:
brctl setageing br 300

Thanks,

Michael

Signed-off-by: Michael Dege <michael.dege@renesas.com>
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
Michael Dege (3):
      net: renesas: rswitch: rename rswitch.c to rswitch_main.c
      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             |  43 ++-
 drivers/net/ethernet/renesas/rswitch_l2.c          | 344 +++++++++++++++++++++
 drivers/net/ethernet/renesas/rswitch_l2.h          |  15 +
 .../ethernet/renesas/{rswitch.c => rswitch_main.c} |  88 +++++-
 5 files changed, 485 insertions(+), 6 deletions(-)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20250616-add_l2_switching-345117105d0d

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/3] net: renesas: rswitch: rename rswitch.c to rswitch_main.c
  2025-07-04  5:51 [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching Michael Dege
@ 2025-07-04  5:51 ` Michael Dege
  2025-07-04  8:34   ` Andrew Lunn
  2025-07-04  9:11   ` Niklas Söderlund
  2025-07-04  5:51 ` [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching Michael Dege
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 22+ messages in thread
From: Michael Dege @ 2025-07-04  5:51 UTC (permalink / raw)
  To: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: netdev, linux-renesas-soc, linux-kernel, Michael Dege,
	Nikita Yushchenko

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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching
  2025-07-04  5:51 [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching Michael Dege
  2025-07-04  5:51 ` [PATCH 1/3] net: renesas: rswitch: rename rswitch.c to rswitch_main.c Michael Dege
@ 2025-07-04  5:51 ` Michael Dege
  2025-07-04  8:43   ` Andrew Lunn
  2025-07-04  5:51 ` [PATCH 3/3] net: renesas: rswitch: add modifiable ageing time Michael Dege
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Michael Dege @ 2025-07-04  5:51 UTC (permalink / raw)
  To: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: netdev, linux-renesas-soc, linux-kernel, Michael Dege,
	Nikita Yushchenko

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

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      |  42 +++-
 drivers/net/ethernet/renesas/rswitch_l2.c   | 318 ++++++++++++++++++++++++++++
 drivers/net/ethernet/renesas/rswitch_l2.h   |  15 ++
 drivers/net/ethernet/renesas/rswitch_main.c |  88 +++++++-
 5 files changed, 458 insertions(+), 7 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 532192cbca4b520e06a7e35653929d8364f1ccb2..45a9d02c5b5d3e7b62554bd56c6c0cb9231f684c 100644
--- a/drivers/net/ethernet/renesas/rswitch.h
+++ b/drivers/net/ethernet/renesas/rswitch.h
@@ -1,19 +1,25 @@
 /* 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__
 #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)
@@ -821,11 +828,28 @@ 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)
+#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)
 
@@ -994,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 {
@@ -1022,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..242beb1f15c089585f5fe5019f626df8824b971a
--- /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_info(&priv->pdev->dev, "starting l2 offload for %s\n",
+			 netdev_name(offload_brdev));
+	else if (!offload_brdev && priv->offload_brdev)
+		dev_info(&priv->pdev->dev, "stopping l2 offload for %s\n",
+			 netdev_name(priv->offload_brdev));
+	else
+		dev_info(&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 aba772e14555d30891dc74a5a123121dad77e92b..12689530e09385b7912b25fa1b982bfd637948d4 100644
--- a/drivers/net/ethernet/renesas/rswitch_main.c
+++ b/drivers/net/ethernet/renesas/rswitch_main.c
@@ -1,15 +1,18 @@
 // 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>
 #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,10 +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;
+	u32 ret;
 
 	/* Start with empty configuration */
 	for (i = 0; i < RSWITCH_NUM_AGENTS; i++) {
@@ -128,6 +134,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 */
@@ -140,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) */
@@ -1602,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;
 };
 
@@ -1624,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);
 
@@ -1850,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);
@@ -1959,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:
@@ -1978,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);
@@ -2024,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));
@@ -2073,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);
@@ -2107,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);
@@ -2144,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);
 
@@ -2154,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;
@@ -2187,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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/3] net: renesas: rswitch: add modifiable ageing time
  2025-07-04  5:51 [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching Michael Dege
  2025-07-04  5:51 ` [PATCH 1/3] net: renesas: rswitch: rename rswitch.c to rswitch_main.c Michael Dege
  2025-07-04  5:51 ` [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching Michael Dege
@ 2025-07-04  5:51 ` Michael Dege
  2025-07-07 12:50   ` Geert Uytterhoeven
  2025-07-04  8:33 ` [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching Andrew Lunn
  2025-07-04  8:46 ` Andrew Lunn
  4 siblings, 1 reply; 22+ messages in thread
From: Michael Dege @ 2025-07-04  5:51 UTC (permalink / raw)
  To: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: netdev, linux-renesas-soc, linux-kernel, Michael Dege,
	Nikita Yushchenko

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.h    |  1 +
 drivers/net/ethernet/renesas/rswitch_l2.c | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/net/ethernet/renesas/rswitch.h b/drivers/net/ethernet/renesas/rswitch.h
index 45a9d02c5b5d3e7b62554bd56c6c0cb9231f684c..11ff1102668b081e395664ea73e19e0ecef74e24 100644
--- a/drivers/net/ethernet/renesas/rswitch.h
+++ b/drivers/net/ethernet/renesas/rswitch.h
@@ -849,6 +849,7 @@ enum rswitch_gwca_mode {
 #define FWMACAGC_MACDESOG	BIT(29)
 
 #define RSW_AGEING_TIME		300
+#define RSW_MAX_AGEING_TIME	65535
 
 /* TOP */
 #define TPEMIMC7(queue)		(TPEMIMC70 + (queue) * 4)
diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c b/drivers/net/ethernet/renesas/rswitch_l2.c
index 242beb1f15c089585f5fe5019f626df8824b971a..c8a8a60a20e70f7ce421280ed35c0c4afe1ed039 100644
--- a/drivers/net/ethernet/renesas/rswitch_l2.c
+++ b/drivers/net/ethernet/renesas/rswitch_l2.c
@@ -196,6 +196,30 @@ 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, time_val;
+
+	if (!is_rdev(ndev))
+		return -ENODEV;
+
+	/* Although brctl accepts the ageing time parameter in seconds, the value
+	 * passed to the driver is multiplied by 100. We need it in seconds.
+	 */
+	time_val = (u32)time / 100;
+
+	if (time_val > RSW_MAX_AGEING_TIME)
+		return -EINVAL;
+
+	rdev = netdev_priv(ndev);
+	reg_val = FIELD_PREP(FWMACAGC_MACAGT, time_val);
+	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 +227,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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
  2025-07-04  5:51 [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching Michael Dege
                   ` (2 preceding siblings ...)
  2025-07-04  5:51 ` [PATCH 3/3] net: renesas: rswitch: add modifiable ageing time Michael Dege
@ 2025-07-04  8:33 ` Andrew Lunn
  2025-07-07 10:41   ` Michael Dege
  2025-07-04  8:46 ` Andrew Lunn
  4 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2025-07-04  8:33 UTC (permalink / raw)
  To: Michael Dege
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-renesas-soc, linux-kernel,
	Nikita Yushchenko

On Fri, Jul 04, 2025 at 07:51:14AM +0200, Michael Dege wrote:
> 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 the L2 driver extension in a separate file. The HW offloading
> is automatically configured when a port is added to the bridge device.
> 
> Ussage example:
> ip link add name br type bridge
> ip link set dev tsn0 master br
> ip link set dev tsn1 master br
> ip link set dev br up
> ip link set dev tsn0 up
> ip link set dev tsn1 up

It is not wrong, but it is normal for an interface to have a number of
some sort. So br0.

> 
> Layer 2 traffic is now fowarded by HW from port TSN0 to port TSN1.
> 
> 3. Provides the functionality to set the MAC table ageing time in the
> Rswitch.
> 
> Usage example:
> brctl setageing br 300

brctl is deprecated. iproute2 has a method to do this, which would be
a better reference to give.

  Andrew

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/3] net: renesas: rswitch: rename rswitch.c to rswitch_main.c
  2025-07-04  5:51 ` [PATCH 1/3] net: renesas: rswitch: rename rswitch.c to rswitch_main.c Michael Dege
@ 2025-07-04  8:34   ` Andrew Lunn
  2025-07-04  9:11   ` Niklas Söderlund
  1 sibling, 0 replies; 22+ messages in thread
From: Andrew Lunn @ 2025-07-04  8:34 UTC (permalink / raw)
  To: Michael Dege
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-renesas-soc, linux-kernel,
	Nikita Yushchenko

On Fri, Jul 04, 2025 at 07:51:15AM +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>

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

    Andrew

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching
  2025-07-04  5:51 ` [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching Michael Dege
@ 2025-07-04  8:43   ` Andrew Lunn
  2025-07-04  9:02     ` Nikita Yushchenko
  2025-07-07  8:13     ` Michael Dege
  0 siblings, 2 replies; 22+ messages in thread
From: Andrew Lunn @ 2025-07-04  8:43 UTC (permalink / raw)
  To: Michael Dege
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-renesas-soc, linux-kernel,
	Nikita Yushchenko

>  #include <linux/platform_device.h>
> +#include <linux/phy.h>
> +

It seems odd that a patch adding L2 support needs to touch PHYs?

> @@ -994,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 */

How many ports does this device have? If it is just two, this might
work. But for a multi-port device, you need to keep this in the port
structure.

> +bool is_rdev(const struct net_device *ndev);
> +void rswitch_modify(void __iomem *addr, enum rswitch_reg reg, u32 clear, u32 set);

Are these actually needed? It seems like they could be local
functions.

> +	if (offload_brdev && !priv->offload_brdev)
> +		dev_info(&priv->pdev->dev, "starting l2 offload for %s\n",
> +			 netdev_name(offload_brdev));
> +	else if (!offload_brdev && priv->offload_brdev)
> +		dev_info(&priv->pdev->dev, "stopping l2 offload for %s\n",
> +			 netdev_name(priv->offload_brdev));

Please don't spam the log like this dev_dbg() maybe.

> @@ -128,6 +134,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);
> +

Please pull ageing out into a patch of its own.

    Andrew

---
pw-bot: cr

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
  2025-07-04  5:51 [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching Michael Dege
                   ` (3 preceding siblings ...)
  2025-07-04  8:33 ` [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching Andrew Lunn
@ 2025-07-04  8:46 ` Andrew Lunn
  2025-07-04  9:05   ` Nikita Yushchenko
  4 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2025-07-04  8:46 UTC (permalink / raw)
  To: Michael Dege
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-renesas-soc, linux-kernel,
	Nikita Yushchenko

On Fri, Jul 04, 2025 at 07:51:14AM +0200, Michael Dege wrote:
> 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 the L2 driver extension in a separate file. The HW offloading
> is automatically configured when a port is added to the bridge device.
> 
> Ussage example:
> ip link add name br type bridge
> ip link set dev tsn0 master br
> ip link set dev tsn1 master br
> ip link set dev br up
> ip link set dev tsn0 up
> ip link set dev tsn1 up

Looking at the code, it is not clear to me what would happen with:

ip link add name br0 type bridge
ip link set dev tsn0 master br0
ip link set dev br0 up
ip link set dev tsn0 up
ip link add name br1 type bridge
ip link set dev tsn1 master br1
ip link set dev br1 up
ip link set dev tsn1 up

   Andrew

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching
  2025-07-04  8:43   ` Andrew Lunn
@ 2025-07-04  9:02     ` Nikita Yushchenko
  2025-07-04 17:49       ` Andrew Lunn
  2025-07-07  8:13     ` Michael Dege
  1 sibling, 1 reply; 22+ messages in thread
From: Nikita Yushchenko @ 2025-07-04  9:02 UTC (permalink / raw)
  To: Andrew Lunn, Michael Dege
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-renesas-soc, linux-kernel

>>   	struct phy *serdes;
>> +
>> +	struct net_device *brdev;	/* master bridge device */
> 
> How many ports does this device have? If it is just two, this might
> work. But for a multi-port device, you need to keep this in the port
> structure.

Having per-device (not per port) brdev was designed by me.  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.

There are 3 ports in the S4 SoC, but the rswitch IP is parametrized so any number of ports could be 
possible. And, we have been implementing virtual ports (not yet in the patchset by Michael) which opened 
possibility to have netdevs of the same rswitch to be in multiple brdevs even on S4. But still had to 
limit to one brdev due to that hw limitation.

There could be a theoretical possibility to use hw L3 forwarding features for better L2 forwarding, but 
that is tricky and creating such a design did not succeed so far.

Nikita

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
  2025-07-04  8:46 ` Andrew Lunn
@ 2025-07-04  9:05   ` Nikita Yushchenko
  2025-07-04 17:53     ` Andrew Lunn
  0 siblings, 1 reply; 22+ messages in thread
From: Nikita Yushchenko @ 2025-07-04  9:05 UTC (permalink / raw)
  To: Andrew Lunn, Michael Dege
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-renesas-soc, linux-kernel

> Looking at the code, it is not clear to me what would happen with:
> 
> ip link add name br0 type bridge
> ip link set dev tsn0 master br0
> ip link set dev br0 up
> ip link set dev tsn0 up
> ip link add name br1 type bridge
> ip link set dev tsn1 master br1
> ip link set dev br1 up
> ip link set dev tsn1 up

Per design, it shall enable hardware forwarding when two ports are in the same brdev.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/3] net: renesas: rswitch: rename rswitch.c to rswitch_main.c
  2025-07-04  5:51 ` [PATCH 1/3] net: renesas: rswitch: rename rswitch.c to rswitch_main.c Michael Dege
  2025-07-04  8:34   ` Andrew Lunn
@ 2025-07-04  9:11   ` Niklas Söderlund
  1 sibling, 0 replies; 22+ messages in thread
From: Niklas Söderlund @ 2025-07-04  9:11 UTC (permalink / raw)
  To: Michael Dege
  Cc: Yoshihiro Shimoda, Paul Barker, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev,
	linux-renesas-soc, linux-kernel, Nikita Yushchenko

Hi Michael,

Thanks for your work.

On 2025-07-04 07:51:15 +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>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  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
> 

-- 
Kind Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching
  2025-07-04  9:02     ` Nikita Yushchenko
@ 2025-07-04 17:49       ` Andrew Lunn
  0 siblings, 0 replies; 22+ messages in thread
From: Andrew Lunn @ 2025-07-04 17:49 UTC (permalink / raw)
  To: Nikita Yushchenko
  Cc: Michael Dege, Yoshihiro Shimoda, Niklas Söderlund,
	Paul Barker, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, netdev, linux-renesas-soc,
	linux-kernel

On Fri, Jul 04, 2025 at 11:02:55AM +0200, Nikita Yushchenko wrote:
> > >   	struct phy *serdes;
> > > +
> > > +	struct net_device *brdev;	/* master bridge device */
> > 
> > How many ports does this device have? If it is just two, this might
> > work. But for a multi-port device, you need to keep this in the port
> > structure.
> 
> Having per-device (not per port) brdev was designed by me.  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.

This needs to be part of the commit message, since it is not
obvious. I think some comments in the code would also be good, where
it decides there is a bridge in use, and so it needs to reject the
offload for another bridge. This limitation is very unusual, pretty
much a design error, so it needs calling out.

	Andrew

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
  2025-07-04  9:05   ` Nikita Yushchenko
@ 2025-07-04 17:53     ` Andrew Lunn
  2025-07-04 17:56       ` Nikita Yushchenko
  0 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2025-07-04 17:53 UTC (permalink / raw)
  To: Nikita Yushchenko
  Cc: Michael Dege, Yoshihiro Shimoda, Niklas Söderlund,
	Paul Barker, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, netdev, linux-renesas-soc,
	linux-kernel

On Fri, Jul 04, 2025 at 11:05:14AM +0200, Nikita Yushchenko wrote:
> > Looking at the code, it is not clear to me what would happen with:
> > 
> > ip link add name br0 type bridge
> > ip link set dev tsn0 master br0
> > ip link set dev br0 up
> > ip link set dev tsn0 up
> > ip link add name br1 type bridge
> > ip link set dev tsn1 master br1
> > ip link set dev br1 up
> > ip link set dev tsn1 up
> 
> Per design, it shall enable hardware forwarding when two ports are in the same brdev.

So in this case, the hardware offload has been reserved by br0, but is
in fact never used, since there is only one port in the bridge. If i
was to then do

ip link set dev tsn2 master br1
ip link set dev tsn2 up

br1 would not be offloaded, but done in software.

	Andrew


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
  2025-07-04 17:53     ` Andrew Lunn
@ 2025-07-04 17:56       ` Nikita Yushchenko
  0 siblings, 0 replies; 22+ messages in thread
From: Nikita Yushchenko @ 2025-07-04 17:56 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Michael Dege, Yoshihiro Shimoda, Niklas Söderlund,
	Paul Barker, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, netdev, linux-renesas-soc,
	linux-kernel



04.07.2025 19:53, Andrew Lunn wrote:
> On Fri, Jul 04, 2025 at 11:05:14AM +0200, Nikita Yushchenko wrote:
>>> Looking at the code, it is not clear to me what would happen with:
>>>
>>> ip link add name br0 type bridge
>>> ip link set dev tsn0 master br0
>>> ip link set dev br0 up
>>> ip link set dev tsn0 up
>>> ip link add name br1 type bridge
>>> ip link set dev tsn1 master br1
>>> ip link set dev br1 up
>>> ip link set dev tsn1 up
>>
>> Per design, it shall enable hardware forwarding when two ports are in the same brdev.
> 
> So in this case, the hardware offload has been reserved by br0, but is
> in fact never used, since there is only one port in the bridge. If i
> was to then do
> 
> ip link set dev tsn2 master br1
> ip link set dev tsn2 up
> 
> br1 would not be offloaded, but done in software.

rswitch_update_offload_brdev() calculates which brdev to offload, and it only considers brdev having at 
least two rswitch ports. So in this case br1 shall get offloaded.

Nikita

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching
  2025-07-04  8:43   ` Andrew Lunn
  2025-07-04  9:02     ` Nikita Yushchenko
@ 2025-07-07  8:13     ` Michael Dege
  2025-07-07  8:42       ` Michael Dege
  2025-07-07 11:30       ` Andrew Lunn
  1 sibling, 2 replies; 22+ messages in thread
From: Michael Dege @ 2025-07-07  8:13 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nikita Yushchenko

Hello Andrew,

Thank you very much for your comments. I am currently figuring out how to take them into account.

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, July 4, 2025 10:44 AM
> 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 2/3] net: renesas: rswitch: add offloading for L2 switching
>
> >  #include <linux/platform_device.h>
> > +#include <linux/phy.h>
> > +
>
> It seems odd that a patch adding L2 support needs to touch PHYs?

I will figure out where it was needed. Maybe I can get rid of it, or if needed I will move it to the
File that needs it.

>
> > @@ -994,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 */
>
> How many ports does this device have? If it is just two, this might work. But for a multi-port device,
> you need to keep this in the port structure.
>
> > +bool is_rdev(const struct net_device *ndev); void rswitch_modify(void
> > +__iomem *addr, enum rswitch_reg reg, u32 clear, u32 set);
>
> Are these actually needed? It seems like they could be local functions.

Currently is_rdev() is only used in rswitch_l2.c. I moved it to that file and made it static. In the
future it will also be used in the L3 routing. The function rswitch_modify() is used in rswitch_main.c
and rswitch_l2.c I believe in this case it does make sense to have a single implementation. Or should
I use two local copies?

>
> > +   if (offload_brdev && !priv->offload_brdev)
> > +           dev_info(&priv->pdev->dev, "starting l2 offload for %s\n",
> > +                    netdev_name(offload_brdev));
> > +   else if (!offload_brdev && priv->offload_brdev)
> > +           dev_info(&priv->pdev->dev, "stopping l2 offload for %s\n",
> > +                    netdev_name(priv->offload_brdev));
>
> Please don't spam the log like this dev_dbg() maybe.

I'll change that.

>
> > @@ -128,6 +134,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);
> > +
>
> Please pull ageing out into a patch of its own.

OK, will do that.

Best regards,

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching
  2025-07-07  8:13     ` Michael Dege
@ 2025-07-07  8:42       ` Michael Dege
  2025-07-07 11:30       ` Andrew Lunn
  1 sibling, 0 replies; 22+ messages in thread
From: Michael Dege @ 2025-07-07  8:42 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nikita Yushchenko

Hello Andrew,

Short update on a few points.

> -----Original Message-----
> From: Michael Dege
> Sent: Monday, July 7, 2025 10:14 AM
> To: 'Andrew Lunn' <andrew@lunn.ch>
> 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 2/3] net: renesas: rswitch: add offloading for L2 switching
>
> Hello Andrew,
>
> Thank you very much for your comments. I am currently figuring out how to take them into account.
>
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Friday, July 4, 2025 10:44 AM
> > 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 2/3] net: renesas: rswitch: add offloading for L2
> > switching
> >
> > >  #include <linux/platform_device.h>
> > > +#include <linux/phy.h>
> > > +
> >
> > It seems odd that a patch adding L2 support needs to touch PHYs?
>
> I will figure out where it was needed. Maybe I can get rid of it, or if needed I will move it to the
> File that needs it.

This include is needed because struct rswitch_etha has a member of type phy_interface. And rswitch_etha
is a member of struct rswitch_device. And rswitch_device is used also in rswich_l2.c.

>
> >
> > > @@ -994,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 */
> >
> > How many ports does this device have? If it is just two, this might
> > work. But for a multi-port device, you need to keep this in the port structure.
> >
> > > +bool is_rdev(const struct net_device *ndev); void
> > > +rswitch_modify(void __iomem *addr, enum rswitch_reg reg, u32 clear,
> > > +u32 set);
> >
> > Are these actually needed? It seems like they could be local functions.
>
> Currently is_rdev() is only used in rswitch_l2.c. I moved it to that file and made it static. In the
> future it will also be used in the L3 routing. The function rswitch_modify() is used in rswitch_main.c
> and rswitch_l2.c I believe in this case it does make sense to have a single implementation. Or should
> I use two local copies?

The function is_rdev() is an accessor to rswitch_netdev_ops which is local
to rswitch_main.c. I believe it is better to provide this function globally
to the driver instead of the whole rswitch_netdev_ops structure.

Best regards,

Michael

>
> >
> > > + if (offload_brdev && !priv->offload_brdev)
> > > +         dev_info(&priv->pdev->dev, "starting l2 offload for %s\n",
> > > +                  netdev_name(offload_brdev));
> > > + else if (!offload_brdev && priv->offload_brdev)
> > > +         dev_info(&priv->pdev->dev, "stopping l2 offload for %s\n",
> > > +                  netdev_name(priv->offload_brdev));
> >
> > Please don't spam the log like this dev_dbg() maybe.
>
> I'll change that.
>
> >
> > > @@ -128,6 +134,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);
> > > +
> >
> > Please pull ageing out into a patch of its own.
>
> OK, will do that.
>
> Best regards,
>
> 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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
  2025-07-04  8:33 ` [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching Andrew Lunn
@ 2025-07-07 10:41   ` Michael Dege
  2025-07-07 11:53     ` Andrew Lunn
  0 siblings, 1 reply; 22+ messages in thread
From: Michael Dege @ 2025-07-07 10:41 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nikita Yushchenko

Hello Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, July 4, 2025 10:34 AM
> 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 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
>
> On Fri, Jul 04, 2025 at 07:51:14AM +0200, Michael Dege wrote:
> > 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 the L2 driver extension in a separate file. The HW offloading
> > is automatically configured when a port is added to the bridge device.
> >
> > Ussage example:
> > ip link add name br type bridge
> > ip link set dev tsn0 master br
> > ip link set dev tsn1 master br
> > ip link set dev br up
> > ip link set dev tsn0 up
> > ip link set dev tsn1 up
>
> It is not wrong, but it is normal for an interface to have a number of some sort. So br0.
>
> >
> > Layer 2 traffic is now fowarded by HW from port TSN0 to port TSN1.
> >
> > 3. Provides the functionality to set the MAC table ageing time in the
> > Rswitch.
> >
> > Usage example:
> > brctl setageing br 300
>
> brctl is deprecated. iproute2 has a method to do this, which would be a better reference to give.

There is a discrepancy between the value being passed by iproute2 and brctl. Iproute2 passes the
value unaltered in seconds and brctl passes the value in seconds multiplied by 100. So far this
is OK because brctl is deprecated. But the default value being passed when neither brctl is called
nor the ageing parameter is added when the bridge is created is also multiplied by 100 resulting in
30000s being set as default ageing time instead of 300s. Is this a known issue?

Best regards,

Michael

>
>   Andrew
________________________________

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching
  2025-07-07  8:13     ` Michael Dege
  2025-07-07  8:42       ` Michael Dege
@ 2025-07-07 11:30       ` Andrew Lunn
  1 sibling, 0 replies; 22+ messages in thread
From: Andrew Lunn @ 2025-07-07 11:30 UTC (permalink / raw)
  To: Michael Dege
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nikita Yushchenko

> > Are these actually needed? It seems like they could be local functions.
> 
> Currently is_rdev() is only used in rswitch_l2.c. I moved it to that file and made it static. In the
> future it will also be used in the L3 routing. The function rswitch_modify() is used in rswitch_main.c
> and rswitch_l2.c I believe in this case it does make sense to have a single implementation. Or should
> I use two local copies?

Have long do you think it will be before L3 is added? If you have the
code ready and waiting, then please keep it driver global. If it is
going to be a while before L3 is added, i would add it to
rswitch_l2.c, and move it to a common file when you add rswitch_l3.c

	Andrew

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
  2025-07-07 10:41   ` Michael Dege
@ 2025-07-07 11:53     ` Andrew Lunn
  2025-07-07 12:25       ` Michael Dege
  0 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2025-07-07 11:53 UTC (permalink / raw)
  To: Michael Dege
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nikita Yushchenko

> There is a discrepancy between the value being passed by iproute2 and brctl. Iproute2 passes the
> value unaltered in seconds and brctl passes the value in seconds multiplied by 100. So far this
> is OK because brctl is deprecated. But the default value being passed when neither brctl is called
> nor the ageing parameter is added when the bridge is created is also multiplied by 100 resulting in
> 30000s being set as default ageing time instead of 300s. Is this a known issue?

You need to be careful of 100. It is probably the value of HZ you are
using. This means it is probably in jiffies.

It could be whoever added support to DSA was using HZ = 1000, and so
assumed it has milliseconds, not jiffies, and the API is broken.

If you have time, could you dig into this. Trace there where jiffies
are used, seconds, and what conversions are preformed, for the default
values, netlink API and sysfs API. Once we know where it goes wrong,
we can figure out how to fix it.

	Andrew

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
  2025-07-07 11:53     ` Andrew Lunn
@ 2025-07-07 12:25       ` Michael Dege
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Dege @ 2025-07-07 12:25 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nikita Yushchenko

Hello Andrew,


> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Monday, July 7, 2025 1:54 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 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching
>
> > There is a discrepancy between the value being passed by iproute2 and
> > brctl. Iproute2 passes the value unaltered in seconds and brctl passes
> > the value in seconds multiplied by 100. So far this is OK because
> > brctl is deprecated. But the default value being passed when neither
> > brctl is called nor the ageing parameter is added when the bridge is created is also multiplied by
> 100 resulting in 30000s being set as default ageing time instead of 300s. Is this a known issue?
>
> You need to be careful of 100. It is probably the value of HZ you are using. This means it is probably
> in jiffies.
>
> It could be whoever added support to DSA was using HZ = 1000, and so assumed it has milliseconds, not
> jiffies, and the API is broken.
>
> If you have time, could you dig into this. Trace there where jiffies are used, seconds, and what
> conversions are preformed, for the default values, netlink API and sysfs API. Once we know where it
> goes wrong, we can figure out how to fix it.
>
>       Andrew

I poked around and it looks to me that the issue is in the default value:

Include/linux/if_bridge.h#65; #define BR_DEFAULT_AGEING_TIME (300 * HZ)

In our case HZ is 100. And the issue is only seen when no other value has been set. (fresh boot)

Both sysfs API and netlink API produce the same values. That is the value received in
switchdev_attr->u.stp_state is presented unaltered.

ip link change dev br type bridge ageing 10

and

echo 10 > /sys/class/net/br/bridge/ageing_time

Both result in a "10" being passed in switchdev_attr->u.stp_state.

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/3] net: renesas: rswitch: add modifiable ageing time
  2025-07-04  5:51 ` [PATCH 3/3] net: renesas: rswitch: add modifiable ageing time Michael Dege
@ 2025-07-07 12:50   ` Geert Uytterhoeven
  2025-07-07 12:59     ` Michael Dege
  0 siblings, 1 reply; 22+ messages in thread
From: Geert Uytterhoeven @ 2025-07-07 12:50 UTC (permalink / raw)
  To: Michael Dege
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-renesas-soc, linux-kernel,
	Nikita Yushchenko

Hi Michael,

On Fri, 4 Jul 2025 at 07:52, Michael Dege <michael.dege@renesas.com> wrote:
> 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>

Thanks for your patch!

> --- a/drivers/net/ethernet/renesas/rswitch.h
> +++ b/drivers/net/ethernet/renesas/rswitch.h
> @@ -849,6 +849,7 @@ enum rswitch_gwca_mode {
>  #define FWMACAGC_MACDESOG      BIT(29)
>
>  #define RSW_AGEING_TIME                300
> +#define RSW_MAX_AGEING_TIME    65535

This is not needed (see below).

>
>  /* TOP */
>  #define TPEMIMC7(queue)                (TPEMIMC70 + (queue) * 4)
> diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c b/drivers/net/ethernet/renesas/rswitch_l2.c
> index 242beb1f15c089585f5fe5019f626df8824b971a..c8a8a60a20e70f7ce421280ed35c0c4afe1ed039 100644
> --- a/drivers/net/ethernet/renesas/rswitch_l2.c
> +++ b/drivers/net/ethernet/renesas/rswitch_l2.c
> @@ -196,6 +196,30 @@ 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, time_val;
> +
> +       if (!is_rdev(ndev))
> +               return -ENODEV;
> +
> +       /* Although brctl accepts the ageing time parameter in seconds, the value
> +        * passed to the driver is multiplied by 100. We need it in seconds.
> +        */
> +       time_val = (u32)time / 100;

switchdev_attr.u.ageing_time is clock_t, which is long, so no cast is
needed before doing the division.  Actually the cast may truncate very
large values on 64-bit.
However, dropping the cast means time_val should be changed to clock_t.

> +
> +       if (time_val > RSW_MAX_AGEING_TIME)

if (!FIELD_FIT(FWMACAGC_MACAGT, time_val))

> +               return -EINVAL;
> +
> +       rdev = netdev_priv(ndev);
> +       reg_val = FIELD_PREP(FWMACAGC_MACAGT, time_val);
> +       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)

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 3/3] net: renesas: rswitch: add modifiable ageing time
  2025-07-07 12:50   ` Geert Uytterhoeven
@ 2025-07-07 12:59     ` Michael Dege
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Dege @ 2025-07-07 12:59 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Yoshihiro Shimoda, Niklas Söderlund, Paul Barker,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nikita Yushchenko

Hello Geert,

Thank you very much for your comments.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Monday, July 7, 2025 2:50 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 3/3] net: renesas: rswitch: add modifiable ageing time
>
> Hi Michael,
>
> On Fri, 4 Jul 2025 at 07:52, Michael Dege <michael.dege@renesas.com> wrote:
> > 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>
>
> Thanks for your patch!
>
> > --- a/drivers/net/ethernet/renesas/rswitch.h
> > +++ b/drivers/net/ethernet/renesas/rswitch.h
> > @@ -849,6 +849,7 @@ enum rswitch_gwca_mode {
> >  #define FWMACAGC_MACDESOG      BIT(29)
> >
> >  #define RSW_AGEING_TIME                300
> > +#define RSW_MAX_AGEING_TIME    65535
>
> This is not needed (see below).
>
> >
> >  /* TOP */
> >  #define TPEMIMC7(queue)                (TPEMIMC70 + (queue) * 4)
> > diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c
> > b/drivers/net/ethernet/renesas/rswitch_l2.c
> > index
> > 242beb1f15c089585f5fe5019f626df8824b971a..c8a8a60a20e70f7ce421280ed35c
> > 0c4afe1ed039 100644
> > --- a/drivers/net/ethernet/renesas/rswitch_l2.c
> > +++ b/drivers/net/ethernet/renesas/rswitch_l2.c
> > @@ -196,6 +196,30 @@ 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, time_val;
> > +
> > +       if (!is_rdev(ndev))
> > +               return -ENODEV;
> > +
> > +       /* Although brctl accepts the ageing time parameter in seconds, the value
> > +        * passed to the driver is multiplied by 100. We need it in seconds.
> > +        */
> > +       time_val = (u32)time / 100;
>
> switchdev_attr.u.ageing_time is clock_t, which is long, so no cast is needed before doing the
> division.  Actually the cast may truncate very large values on 64-bit.
> However, dropping the cast means time_val should be changed to clock_t.

This division has been dropped completely. As it is not needed. Andrew pointed out, that brctl
Is deprecated and I learned that the ageing time parameter in iproute2 is already in seconds.
I need the value in seconds for our hardware, therefore no conversion needed. Update will come
With V2 of this patch set.

>
> > +
> > +       if (time_val > RSW_MAX_AGEING_TIME)
>
> if (!FIELD_FIT(FWMACAGC_MACAGT, time_val))
>
Thank you this is helpful, I will change it in the code.

Best regards,

Michael

> > +               return -EINVAL;
> > +
> > +       rdev = netdev_priv(ndev);
> > +       reg_val = FIELD_PREP(FWMACAGC_MACAGT, time_val);
> > +       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)
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But when I'm talking to
> journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
________________________________

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2025-07-07 12:59 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-04  5:51 [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching Michael Dege
2025-07-04  5:51 ` [PATCH 1/3] net: renesas: rswitch: rename rswitch.c to rswitch_main.c Michael Dege
2025-07-04  8:34   ` Andrew Lunn
2025-07-04  9:11   ` Niklas Söderlund
2025-07-04  5:51 ` [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching Michael Dege
2025-07-04  8:43   ` Andrew Lunn
2025-07-04  9:02     ` Nikita Yushchenko
2025-07-04 17:49       ` Andrew Lunn
2025-07-07  8:13     ` Michael Dege
2025-07-07  8:42       ` Michael Dege
2025-07-07 11:30       ` Andrew Lunn
2025-07-04  5:51 ` [PATCH 3/3] net: renesas: rswitch: add modifiable ageing time Michael Dege
2025-07-07 12:50   ` Geert Uytterhoeven
2025-07-07 12:59     ` Michael Dege
2025-07-04  8:33 ` [PATCH 0/3] net: renesas: rswitch: R-Car S4 add HW offloading for layer 2 switching Andrew Lunn
2025-07-07 10:41   ` Michael Dege
2025-07-07 11:53     ` Andrew Lunn
2025-07-07 12:25       ` Michael Dege
2025-07-04  8:46 ` Andrew Lunn
2025-07-04  9:05   ` Nikita Yushchenko
2025-07-04 17:53     ` Andrew Lunn
2025-07-04 17:56       ` Nikita Yushchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).