[PATCH net-next 0/8] net: mdio: realtek-rtl9300: Refactor initialization and port lookup

[PATCH net-next 0/8] net: mdio: realtek-rtl9300: Refactor initialization and port lookup

[Markus Stockhausen]

The Realtek Otto switch platform consists of four different series

- RTL838x aka maple   : 28 port 1G Switches
- RTL839x aka cypress : 52 port 1G Switches
- RTL930x aka longan  : 28 port 1G/2.5G/10G Switches
- RTL931x aka mango   : 56 port 1G/2.5G/10G Switches

A lot has been done to enhance the ethernet MDIO driver for simple 
integration of more SoCs. Now it is time to solve inconveniences
that were discovered during daily operation. That includes

- Tightening error handling and improving overall robustness.
- Adding support for PHY packages.
- Fixing setup order issues. These currently hinder the driver from 
  properly enabling the hardware on devices where U-Boot skips the 
  setup and leaves the controller registers untouched.
- Converting the port lookup from O(n) to O(1) to save cycles on 
  the slow embedded CPUs.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---


Markus Stockhausen (8):
  net: mdio: realtek-rtl9300: Convert to fwnode API
  net: mdio: realtek-rtl9300: Correctly handle ethernet-phy-package
  net: mdio: realtek-rtl9300: harden otto_emdio_map_ports()
  net: mdio: realtek-rtl9300: harden otto_emdio_probe_one()
  net: mdio: realtek-rtl9300: adapt spaces for defines
  net: mdio: realtek-rtl9300: relocate topology setup
  net: mdio: realtek-rtl9300: reorder controller setup
  net: mdio: realtek-rtl9300: Convert port lookup from O(n) to O(1)

 drivers/net/mdio/mdio-realtek-rtl9300.c | 235 ++++++++++++++----------
 1 file changed, 143 insertions(+), 92 deletions(-)

-- 
2.54.0

[PATCH net-next 1/8] net: mdio: realtek-rtl9300: Convert to fwnode API

[Markus Stockhausen]

The driver still uses legacy "of_" functions to determine the port mapping
during probing. Convert this to the modern fwnode API. With that also fix
a subtle lookup bug in the original code.

  mdio_dn = phy_dn->parent;
  if (mdio_dn->parent != dev->of_node)
    continue;

In the case of a very buggy device tree, phy_dn might be a root node. When
trying to look up its grandparent this leads to a NULL pointer access.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index bafcd6afe619..1ccc0ab0daef 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -438,20 +438,25 @@ static int otto_emdio_map_ports(struct device *dev)
 				     dev_fwnode(parent));
 
 	fwnode_for_each_child_node_scoped(ports, port) {
-		struct device_node *mdio_dn;
 		u32 addr;
 		u32 bus;
 		u32 pn;
 
-		struct device_node *phy_dn __free(device_node) =
-			of_parse_phandle(to_of_node(port), "phy-handle", 0);
+		struct fwnode_handle *phy_fwnode __free(fwnode_handle) =
+			fwnode_find_reference(port, "phy-handle", 0);
 		/* skip ports without phys */
-		if (!phy_dn)
+		if (IS_ERR(phy_fwnode))
 			continue;
 
-		mdio_dn = phy_dn->parent;
+		struct fwnode_handle *bus_fwnode __free(fwnode_handle) =
+			fwnode_get_parent(phy_fwnode);
+		if (!bus_fwnode)
+			continue;
+
+		struct fwnode_handle *ctrl_fwnode __free(fwnode_handle) =
+			fwnode_get_parent(bus_fwnode);
 		/* only map ports that are connected to this mdio-controller */
-		if (mdio_dn->parent != dev->of_node)
+		if (ctrl_fwnode != dev_fwnode(dev))
 			continue;
 
 		err = fwnode_property_read_u32(port, "reg", &pn);
@@ -464,14 +469,14 @@ static int otto_emdio_map_ports(struct device *dev)
 		if (test_bit(pn, priv->valid_ports))
 			return dev_err_probe(dev, -EINVAL, "duplicated port number %d\n", pn);
 
-		err = of_property_read_u32(mdio_dn, "reg", &bus);
+		err = fwnode_property_read_u32(bus_fwnode, "reg", &bus);
 		if (err)
 			return err;
 
 		if (bus >= priv->info->num_buses)
 			return dev_err_probe(dev, -EINVAL, "illegal smi bus number %d\n", bus);
 
-		err = of_property_read_u32(phy_dn, "reg", &addr);
+		err = fwnode_property_read_u32(phy_fwnode, "reg", &addr);
 		if (err)
 			return err;
 
-- 
2.54.0

[PATCH net-next 2/8] net: mdio: realtek-rtl9300: Correctly handle ethernet-phy-package

[Markus Stockhausen]

Realtek Otto switches usually make use of multiport PHYs (e.g. 8 port
1G RTL8218D or 4 port 2.5G RTL8224). The device tree can describe this
fact via an "ethernet-phy-package" node that resides between the bus
and the PHY node.

When looking up the device tree bus node via the chain port->phy->parent
the driver totally ignores the existence of a PHY package. Enhance the
lookup to take care of this feature.

Signed-off-by: Manuel Stocker <mensi@mensi.ch>
Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 1ccc0ab0daef..381c45e0597b 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -421,6 +421,21 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
 	return 0;
 }
 
+static struct fwnode_handle *otto_emdio_get_bus_node(struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *parent = fwnode_get_parent(fwnode);
+	struct fwnode_handle *grandparent;
+
+	if (parent && fwnode_name_eq(parent, "ethernet-phy-package")) {
+		grandparent = fwnode_get_parent(parent);
+		fwnode_handle_put(parent);
+
+		return grandparent;
+	}
+
+	return parent;
+}
+
 /* The mdio-controller is part of a switch block so we parse the sibling
  * ethernet-ports node and build a mapping of the switch port to MDIO bus/addr
  * based on the phy-handle.
@@ -449,7 +464,7 @@ static int otto_emdio_map_ports(struct device *dev)
 			continue;
 
 		struct fwnode_handle *bus_fwnode __free(fwnode_handle) =
-			fwnode_get_parent(phy_fwnode);
+			otto_emdio_get_bus_node(phy_fwnode);
 		if (!bus_fwnode)
 			continue;
 
-- 
2.54.0

[PATCH net-next 3/8] net: mdio: realtek-rtl9300: harden otto_emdio_map_ports()

[Markus Stockhausen]

Due to its design the MDIO driver needs to set up a port to bus/address
mapping during probing. The "ethernet-ports" subnodes are scanned and
from the "phy-handle" property the MDIO nodes are looked up. In case of
a malformed device tree the driver might produce out-of-bounds accesses.
The PHY address is not checked against the maximum supported address.

Add a sanity check. Do not use the existing (unused) MAX_SMI_ADDR define
but drop it in favour of PHY_MAX_ADDR.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 381c45e0597b..517b01115f34 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -78,7 +78,6 @@
 
 #define MAX_PORTS       28
 #define MAX_SMI_BUSSES  4
-#define MAX_SMI_ADDR	0x1f
 #define RAW_PAGE(priv)	((priv)->info->num_pages - 1)
 
 
@@ -495,6 +494,9 @@ static int otto_emdio_map_ports(struct device *dev)
 		if (err)
 			return err;
 
+		if (addr >= PHY_MAX_ADDR)
+			return dev_err_probe(dev, -EINVAL, "illegal smi address %d\n", addr);
+
 		__set_bit(pn, priv->valid_ports);
 		priv->smi_bus[pn] = bus;
 		priv->smi_addr[pn] = addr;
-- 
2.54.0

[PATCH net-next 4/8] net: mdio: realtek-rtl9300: harden otto_emdio_probe_one()

[Markus Stockhausen]

The bus probing of the MDIO driver uses a two stage approach.

1. The device tree "ethernet-ports" node is scanned to build a mapping
   between ports and PHYs.

2. The children of the device tree "controller" are scanned to create
   the individual MDIO buses.

The first step already checks the consistency of the PHY and bus nodes
that are linked via the ports. But it might miss a dangling bus child
node that is not linked. Step two simply iterates over all bus child
nodes and might read malformed data from nodes not checked in step one.

Harden this and return a meaningful error message.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 517b01115f34..cec2afed5b6f 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -380,7 +380,11 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
 
 	err = fwnode_property_read_u32(node, "reg", &mdio_bus);
 	if (err)
-		return err;
+		return dev_err_probe(dev, err, "undefined smi bus number\n");
+
+	if (mdio_bus >= priv->info->num_buses)
+		return dev_err_probe(dev, -EINVAL,
+				     "illegal (dangling) smi bus number %d\n", mdio_bus);
 
 	/* The MDIO accesses from the kernel work with the PHY polling unit in
 	 * the switch. We need to tell the PPU to operate either in GPHY (i.e.
-- 
2.54.0

[PATCH net-next 5/8] net: mdio: realtek-rtl9300: adapt spaces for defines

[Markus Stockhausen]

Register and fields will get longer prefixes in the future.
Keep define blocks clean and readable and add another tab.
Additionally clean up some multi-space separators with tabs.

Just cosmetics, no functional change.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 56 ++++++++++++-------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index cec2afed5b6f..74ea2550e652 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -51,34 +51,34 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 
-#define RTL9300_NUM_BUSES		4
-#define RTL9300_NUM_PAGES		4096
-#define RTL9300_NUM_PORTS		28
-#define SMI_GLB_CTRL			0xca00
-#define   GLB_CTRL_INTF_SEL(intf)	BIT(16 + (intf))
-#define SMI_PORT0_15_POLLING_SEL	0xca08
-#define RTL9300_SMI_ACCESS_PHY_CTRL_0	0xcb70
-#define RTL9300_SMI_ACCESS_PHY_CTRL_1	0xcb74
-#define   PHY_CTRL_REG_ADDR		GENMASK(24, 20)
-#define   PHY_CTRL_PARK_PAGE		GENMASK(19, 15)
-#define   PHY_CTRL_MAIN_PAGE		GENMASK(14, 3)
-#define   PHY_CTRL_WRITE		BIT(2)
-#define   PHY_CTRL_READ			0
-#define   PHY_CTRL_TYPE_C45		BIT(1)
-#define   PHY_CTRL_TYPE_C22		0
-#define   PHY_CTRL_CMD			BIT(0)
-#define   PHY_CTRL_FAIL			BIT(25)
-#define RTL9300_SMI_ACCESS_PHY_CTRL_2	0xcb78
-#define   PHY_CTRL_INDATA		GENMASK(31, 16)
-#define   PHY_CTRL_DATA			GENMASK(15, 0)
-#define RTL9300_SMI_ACCESS_PHY_CTRL_3	0xcb7c
-#define   PHY_CTRL_MMD_DEVAD		GENMASK(20, 16)
-#define   PHY_CTRL_MMD_REG		GENMASK(15, 0)
-#define SMI_PORT0_5_ADDR_CTRL		0xcb80
-
-#define MAX_PORTS       28
-#define MAX_SMI_BUSSES  4
-#define RAW_PAGE(priv)	((priv)->info->num_pages - 1)
+#define RTL9300_NUM_BUSES			4
+#define RTL9300_NUM_PAGES			4096
+#define RTL9300_NUM_PORTS			28
+#define SMI_GLB_CTRL				0xca00
+#define   GLB_CTRL_INTF_SEL(intf)		BIT(16 + (intf))
+#define SMI_PORT0_15_POLLING_SEL		0xca08
+#define RTL9300_SMI_ACCESS_PHY_CTRL_0		0xcb70
+#define RTL9300_SMI_ACCESS_PHY_CTRL_1		0xcb74
+#define   PHY_CTRL_REG_ADDR			GENMASK(24, 20)
+#define   PHY_CTRL_PARK_PAGE			GENMASK(19, 15)
+#define   PHY_CTRL_MAIN_PAGE			GENMASK(14, 3)
+#define   PHY_CTRL_WRITE			BIT(2)
+#define   PHY_CTRL_READ				0
+#define   PHY_CTRL_TYPE_C45			BIT(1)
+#define   PHY_CTRL_TYPE_C22			0
+#define   PHY_CTRL_CMD				BIT(0)
+#define   PHY_CTRL_FAIL				BIT(25)
+#define RTL9300_SMI_ACCESS_PHY_CTRL_2		0xcb78
+#define   PHY_CTRL_INDATA			GENMASK(31, 16)
+#define   PHY_CTRL_DATA				GENMASK(15, 0)
+#define RTL9300_SMI_ACCESS_PHY_CTRL_3		0xcb7c
+#define   PHY_CTRL_MMD_DEVAD			GENMASK(20, 16)
+#define   PHY_CTRL_MMD_REG			GENMASK(15, 0)
+#define SMI_PORT0_5_ADDR_CTRL			0xcb80
+
+#define MAX_PORTS				28
+#define MAX_SMI_BUSSES				4
+#define RAW_PAGE(priv)				((priv)->info->num_pages - 1)
 
 
 struct otto_emdio_cmd_regs {
-- 
2.54.0

[PATCH net-next 6/8] net: mdio: realtek-rtl9300: relocate topology setup

[Markus Stockhausen]

Until now the driver sets up the port to bus/address topology of the
controller after all buses are set up via otto_emdio_probe_one(). This
does not work for devices where U-Boot skips this setup. It is not
only needed for the hardware internal background PHY polling engine
but essential for access to the PHYs during probing.

Depending on the SoC type there exist two different register arrays

- Bus mapping registers (RTL930x, RTL931x) define to which bus the port
  is attached. E.g. [1]
- Address mapping registers (RTL838x, RTL930x, RTL931x) define to which
  address of the bus the port is attached. E.g. [2]

Relocate the topology setup and make it generic. For this

- Define device-specific bus_base/addr_base attributes that give the
  register base address where the mapping lives. In case one or both are
  not given the SoC does not support this specific type of mapping.
- Create a helper otto_emdio_setup_topology() that writes the detected
  topology to the registers.
- Call this helper prior to otto_emdio_probe_one().
- Remove unneeded code from otto_emdio_9300_mdiobus_init().

Subtle change: The old coding used regmap_bulk_write and silently wrote
bus=0/address=0 to mapping registers for ports that are out of scope.
The new coding leaves those untouched.

[1] https://svanheule.net/realtek/longan/register/smi_port0_15_polling_sel
[2] https://svanheule.net/realtek/longan/register/smi_port0_5_addr_ctrl

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 70 ++++++++++++++++---------
 1 file changed, 45 insertions(+), 25 deletions(-)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 74ea2550e652..9f67d8dd4185 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -56,7 +56,7 @@
 #define RTL9300_NUM_PORTS			28
 #define SMI_GLB_CTRL				0xca00
 #define   GLB_CTRL_INTF_SEL(intf)		BIT(16 + (intf))
-#define SMI_PORT0_15_POLLING_SEL		0xca08
+#define RTL9300_SMI_PORT0_15_POLLING_SEL	0xca08
 #define RTL9300_SMI_ACCESS_PHY_CTRL_0		0xcb70
 #define RTL9300_SMI_ACCESS_PHY_CTRL_1		0xcb74
 #define   PHY_CTRL_REG_ADDR			GENMASK(24, 20)
@@ -74,7 +74,7 @@
 #define RTL9300_SMI_ACCESS_PHY_CTRL_3		0xcb7c
 #define   PHY_CTRL_MMD_DEVAD			GENMASK(20, 16)
 #define   PHY_CTRL_MMD_REG			GENMASK(15, 0)
-#define SMI_PORT0_5_ADDR_CTRL			0xcb80
+#define RTL9300_SMI_PORT0_5_ADDR_CTRL		0xcb80
 
 #define MAX_PORTS				28
 #define MAX_SMI_BUSSES				4
@@ -89,6 +89,8 @@ struct otto_emdio_cmd_regs {
 };
 
 struct otto_emdio_info {
+	u32 addr_map_base;
+	u32 bus_map_base;
 	u32 cmd_fail;
 	u32 cmd_read;
 	u32 cmd_write;
@@ -327,24 +329,46 @@ static int otto_emdio_write_c45(struct mii_bus *bus, int phy_id,
 	return ret;
 }
 
-static int otto_emdio_9300_mdiobus_init(struct otto_emdio_priv *priv)
+static int otto_emdio_write_mapping(struct otto_emdio_priv *priv, u32 base, u32 port,
+				    u32 ports_per_reg, u32 bits_per_val, u32 value)
 {
-	u32 glb_ctrl_mask = 0, glb_ctrl_val = 0;
-	struct regmap *regmap = priv->regmap;
-	u32 port_addr[5] = { 0 };
-	u32 poll_sel[2] = { 0 };
-	int i, err;
+	u32 shift = (port % ports_per_reg) * bits_per_val;
+	u32 reg = base + (port / ports_per_reg) * 4;
+	u32 mask = GENMASK(bits_per_val - 1, 0);
 
-	/* Associate the port with the SMI interface and PHY */
-	for_each_set_bit(i, priv->valid_ports, priv->info->num_ports) {
-		int pos;
+	return regmap_update_bits(priv->regmap, reg, mask << shift, value << shift);
+}
 
-		pos = (i % 6) * 5;
-		port_addr[i / 6] |= (priv->smi_addr[i] & 0x1f) << pos;
+static int otto_emdio_setup_topology(struct otto_emdio_priv *priv)
+{
+	const struct otto_emdio_info *info = priv->info;
+	u32 port;
+	int ret;
 
-		pos = (i % 16) * 2;
-		poll_sel[i / 16] |= (priv->smi_bus[i] & 0x3) << pos;
+	for_each_set_bit(port, priv->valid_ports, info->num_ports) {
+		if (info->bus_map_base) {
+			/* 16 ports per register, 2 bits per port (bus index 0-3) */
+			ret = otto_emdio_write_mapping(priv, info->bus_map_base, port,
+						       16, 2, priv->smi_bus[port]);
+			if (ret)
+				return ret;
+		}
+		if (info->addr_map_base) {
+			/* 6 ports per register, 5 bits per port (PHY address 0-31) */
+			ret = otto_emdio_write_mapping(priv, info->addr_map_base, port,
+						       6, 5, priv->smi_addr[port]);
+			if (ret)
+				return ret;
+		}
 	}
+	return 0;
+}
+
+static int otto_emdio_9300_mdiobus_init(struct otto_emdio_priv *priv)
+{
+	u32 glb_ctrl_mask = 0, glb_ctrl_val = 0;
+	struct regmap *regmap = priv->regmap;
+	int i, err;
 
 	/* Put the interfaces into C45 mode if required */
 	glb_ctrl_mask = GENMASK(19, 16);
@@ -352,16 +376,6 @@ static int otto_emdio_9300_mdiobus_init(struct otto_emdio_priv *priv)
 		if (priv->smi_bus_is_c45[i])
 			glb_ctrl_val |= GLB_CTRL_INTF_SEL(i);
 
-	err = regmap_bulk_write(regmap, SMI_PORT0_5_ADDR_CTRL,
-				port_addr, 5);
-	if (err)
-		return err;
-
-	err = regmap_bulk_write(regmap, SMI_PORT0_15_POLLING_SEL,
-				poll_sel, 2);
-	if (err)
-		return err;
-
 	err = regmap_update_bits(regmap, SMI_GLB_CTRL,
 				 glb_ctrl_mask, glb_ctrl_val);
 	if (err)
@@ -534,6 +548,10 @@ static int otto_emdio_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	err = otto_emdio_setup_topology(priv);
+	if (err)
+		return err;
+
 	device_for_each_child_node_scoped(dev, child) {
 		err = otto_emdio_probe_one(dev, priv, child);
 		if (err)
@@ -548,6 +566,8 @@ static int otto_emdio_probe(struct platform_device *pdev)
 }
 
 static const struct otto_emdio_info otto_emdio_9300_info = {
+	.addr_map_base = RTL9300_SMI_PORT0_5_ADDR_CTRL,
+	.bus_map_base = RTL9300_SMI_PORT0_15_POLLING_SEL,
 	.cmd_fail = PHY_CTRL_FAIL,
 	.cmd_read = PHY_CTRL_READ,
 	.cmd_write = PHY_CTRL_WRITE,
-- 
2.54.0

[PATCH net-next 7/8] net: mdio: realtek-rtl9300: reorder controller setup

[Markus Stockhausen]

After the former refactoring the existing otto_emdio_9300_mdiobus_init()
contains only the c22/c45 bus mode setup. Like the topology setup this
must run before bus registration. Otherwise the bus does not "speak" the
right protocol for PHY setup.

This setup is device-specific and other SoCs will need to set up other
register bits in the controller in the future. Therefore

- Add a new device-specific setup_controller() into the info structure.
- Relocate otto_emdio_priv to satisfy the new info structure dependency.
- Rename otto_emdio_9300_mdiobus_init accordingly and add it to the
  RTL9300 info structure. At the same time, adapt register naming
  for the function to make it clear that it only applies to this SoC.
- Call setup_controller() prior to bus registration.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 44 ++++++++++++++-----------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 9f67d8dd4185..50fc8582571d 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -54,8 +54,8 @@
 #define RTL9300_NUM_BUSES			4
 #define RTL9300_NUM_PAGES			4096
 #define RTL9300_NUM_PORTS			28
-#define SMI_GLB_CTRL				0xca00
-#define   GLB_CTRL_INTF_SEL(intf)		BIT(16 + (intf))
+#define RTL9300_SMI_GLB_CTRL			0xca00
+#define   RTL9300_GLB_CTRL_INTF_SEL(intf)	BIT(16 + (intf))
 #define RTL9300_SMI_PORT0_15_POLLING_SEL	0xca08
 #define RTL9300_SMI_ACCESS_PHY_CTRL_0		0xcb70
 #define RTL9300_SMI_ACCESS_PHY_CTRL_1		0xcb74
@@ -88,6 +88,17 @@ struct otto_emdio_cmd_regs {
 	u32 port_mask_low;
 };
 
+struct otto_emdio_priv {
+	const struct otto_emdio_info *info;
+	struct regmap *regmap;
+	struct mutex lock; /* protect HW access */
+	DECLARE_BITMAP(valid_ports, MAX_PORTS);
+	u8 smi_bus[MAX_PORTS];
+	u8 smi_addr[MAX_PORTS];
+	bool smi_bus_is_c45[MAX_SMI_BUSSES];
+	struct mii_bus *bus[MAX_SMI_BUSSES];
+};
+
 struct otto_emdio_info {
 	u32 addr_map_base;
 	u32 bus_map_base;
@@ -98,23 +109,13 @@ struct otto_emdio_info {
 	u8 num_buses;
 	u8 num_ports;
 	u16 num_pages;
+	int (*setup_controller)(struct otto_emdio_priv *priv);
 	int (*read_c22)(struct mii_bus *bus, int port, int regnum, u32 *value);
 	int (*read_c45)(struct mii_bus *bus, int port, int dev_addr, int regnum, u32 *value);
 	int (*write_c22)(struct mii_bus *bus, int port, int regnum, u16 value);
 	int (*write_c45)(struct mii_bus *bus, int port, int dev_addr, int regnum, u16 value);
 };
 
-struct otto_emdio_priv {
-	const struct otto_emdio_info *info;
-	struct regmap *regmap;
-	struct mutex lock; /* protect HW access */
-	DECLARE_BITMAP(valid_ports, MAX_PORTS);
-	u8 smi_bus[MAX_PORTS];
-	u8 smi_addr[MAX_PORTS];
-	bool smi_bus_is_c45[MAX_SMI_BUSSES];
-	struct mii_bus *bus[MAX_SMI_BUSSES];
-};
-
 struct otto_emdio_chan {
 	struct otto_emdio_priv *priv;
 	u8 mdio_bus;
@@ -364,7 +365,7 @@ static int otto_emdio_setup_topology(struct otto_emdio_priv *priv)
 	return 0;
 }
 
-static int otto_emdio_9300_mdiobus_init(struct otto_emdio_priv *priv)
+static int otto_emdio_9300_setup_controller(struct otto_emdio_priv *priv)
 {
 	u32 glb_ctrl_mask = 0, glb_ctrl_val = 0;
 	struct regmap *regmap = priv->regmap;
@@ -374,9 +375,9 @@ static int otto_emdio_9300_mdiobus_init(struct otto_emdio_priv *priv)
 	glb_ctrl_mask = GENMASK(19, 16);
 	for (i = 0; i < priv->info->num_buses; i++)
 		if (priv->smi_bus_is_c45[i])
-			glb_ctrl_val |= GLB_CTRL_INTF_SEL(i);
+			glb_ctrl_val |= RTL9300_GLB_CTRL_INTF_SEL(i);
 
-	err = regmap_update_bits(regmap, SMI_GLB_CTRL,
+	err = regmap_update_bits(regmap, RTL9300_SMI_GLB_CTRL,
 				 glb_ctrl_mask, glb_ctrl_val);
 	if (err)
 		return err;
@@ -552,16 +553,18 @@ static int otto_emdio_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	if (priv->info->setup_controller) {
+		err = priv->info->setup_controller(priv);
+		if (err)
+			return dev_err_probe(dev, err, "failed to setup MDIO bus controller\n");
+	}
+
 	device_for_each_child_node_scoped(dev, child) {
 		err = otto_emdio_probe_one(dev, priv, child);
 		if (err)
 			return err;
 	}
 
-	err = otto_emdio_9300_mdiobus_init(priv);
-	if (err)
-		return dev_err_probe(dev, err, "failed to initialise MDIO bus controller\n");
-
 	return 0;
 }
 
@@ -580,6 +583,7 @@ static const struct otto_emdio_info otto_emdio_9300_info = {
 	.num_buses = RTL9300_NUM_BUSES,
 	.num_ports = RTL9300_NUM_PORTS,
 	.num_pages = RTL9300_NUM_PAGES,
+	.setup_controller = otto_emdio_9300_setup_controller,
 	.read_c22 = otto_emdio_9300_read_c22,
 	.read_c45 = otto_emdio_9300_read_c45,
 	.write_c22 = otto_emdio_9300_write_c22,
-- 
2.54.0

[PATCH net-next 8/8] net: mdio: realtek-rtl9300: Convert port lookup from O(n) to O(1)

[Markus Stockhausen]

When addressing a PHY on a bus/address the driver must translate that into
a port number. This is currently done with an effort of O(n). The driver
loops over all known ports and checks each if bus and address match. This
is very inefficient especially for the following reasons:

- The lookup must run for each single read/write operation
- The SoCs have very low specs (e.g. RTL838x one core, 16K cache, 500 MHz)
- High-port-count devices RTL839x and RTL931x provide up to 52 PHY ports

Create a reverse lookup table per channel during setup. This stores the
(overall) port number per PHY (on that channel). Take special care about
absent ports that are missing from device tree because of hardware design.
Save memory by using s8 type for the table (-1 = absent, 0..56 = port).
Convert the lookup in otto_emdio_phy_to_port() from loop-based to
table-based.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 29 +++++++++++++------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 50fc8582571d..a36cb711b8ad 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -78,6 +78,7 @@
 
 #define MAX_PORTS				28
 #define MAX_SMI_BUSSES				4
+#define PORT_ABSENT				-1
 #define RAW_PAGE(priv)				((priv)->info->num_pages - 1)
 
 
@@ -118,23 +119,17 @@ struct otto_emdio_info {
 
 struct otto_emdio_chan {
 	struct otto_emdio_priv *priv;
-	u8 mdio_bus;
+	s8 port[PHY_MAX_ADDR];
 };
 
-static int otto_emdio_phy_to_port(struct mii_bus *bus, int phy_id)
+static int otto_emdio_phy_to_port(struct mii_bus *bus, unsigned int phy_id)
 {
 	struct otto_emdio_chan *chan = bus->priv;
-	struct otto_emdio_priv *priv;
-	int i;
-
-	priv = chan->priv;
 
-	for_each_set_bit(i, priv->valid_ports, priv->info->num_ports)
-		if (priv->smi_bus[i] == chan->mdio_bus &&
-		    priv->smi_addr[i] == phy_id)
-			return i;
+	if (phy_id >= PHY_MAX_ADDR || chan->port[phy_id] == PORT_ABSENT)
+		return -ENOENT;
 
-	return -ENOENT;
+	return chan->port[phy_id];
 }
 
 static struct otto_emdio_priv *otto_emdio_bus_to_priv(struct mii_bus *bus)
@@ -390,8 +385,8 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
 {
 	struct otto_emdio_chan *chan;
 	struct mii_bus *bus;
-	u32 mdio_bus;
-	int err;
+	u32 mdio_bus, port;
+	int i, err;
 
 	err = fwnode_property_read_u32(node, "reg", &mdio_bus);
 	if (err)
@@ -427,11 +422,17 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
 	}
 	bus->parent = dev;
 	chan = bus->priv;
-	chan->mdio_bus = mdio_bus;
 	chan->priv = priv;
 
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-%d", dev_name(dev), mdio_bus);
 
+	/* setup reverse lookup bus/address -> port */
+	for (i = 0; i < PHY_MAX_ADDR; i++)
+		chan->port[i] = PORT_ABSENT;
+	for_each_set_bit(port, priv->valid_ports, priv->info->num_ports)
+		if (priv->smi_bus[port] == mdio_bus)
+			chan->port[priv->smi_addr[port]] = port;
+
 	err = devm_of_mdiobus_register(dev, bus, to_of_node(node));
 	if (err)
 		return dev_err_probe(dev, err, "cannot register MDIO bus\n");
-- 
2.54.0

RE: [PATCH net-next 2/8] net: mdio: realtek-rtl9300: Correctly handle ethernet-phy-package

[Jagielski, Jedrzej]

From: Markus Stockhausen <markus.stockhausen@gmx.de> 
Sent: Tuesday, June 2, 2026 7:27 AM

>Realtek Otto switches usually make use of multiport PHYs (e.g. 8 port
>1G RTL8218D or 4 port 2.5G RTL8224). The device tree can describe this
>fact via an "ethernet-phy-package" node that resides between the bus
>and the PHY node.
>
>When looking up the device tree bus node via the chain port->phy->parent
>the driver totally ignores the existence of a PHY package. Enhance the
>lookup to take care of this feature.
>
>Signed-off-by: Manuel Stocker <mensi@mensi.ch>
>Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

Hi Markus
quite unclear (at least for me) what's Manuel's role on that patch.
If he's a co-author please add proper tag, if you are sending Manuel's
patch on his behalf please change authorship. 

Code itself looks fine

RE: [PATCH net-next 3/8] net: mdio: realtek-rtl9300: harden otto_emdio_map_ports()

[Jagielski, Jedrzej]

From: Markus Stockhausen <markus.stockhausen@gmx.de> 
Sent: Tuesday, June 2, 2026 7:27 AM

>Due to its design the MDIO driver needs to set up a port to bus/address
>mapping during probing. The "ethernet-ports" subnodes are scanned and
>from the "phy-handle" property the MDIO nodes are looked up. In case of
>a malformed device tree the driver might produce out-of-bounds accesses.
>The PHY address is not checked against the maximum supported address.
>
>Add a sanity check. Do not use the existing (unused) MAX_SMI_ADDR define
>but drop it in favour of PHY_MAX_ADDR.
>
>Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
>---
> drivers/net/mdio/mdio-realtek-rtl9300.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
>index 381c45e0597b..517b01115f34 100644
>--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
>+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
>@@ -78,7 +78,6 @@
> 
> #define MAX_PORTS       28
> #define MAX_SMI_BUSSES  4
>-#define MAX_SMI_ADDR	0x1f
> #define RAW_PAGE(priv)	((priv)->info->num_pages - 1)
> 
> 
>@@ -495,6 +494,9 @@ static int otto_emdio_map_ports(struct device *dev)
> 		if (err)
> 			return err;
> 
>+		if (addr >= PHY_MAX_ADDR)
>+			return dev_err_probe(dev, -EINVAL, "illegal smi address %d\n", addr);

Could we have more meaningful error code than EINVAL; EFAULT perhaps if that's bad addr case?
what do you think?
Also - is %d convenient for printing address value?

>+
> 		__set_bit(pn, priv->valid_ports);
> 		priv->smi_bus[pn] = bus;
> 		priv->smi_addr[pn] = addr;
>-- 
>2.54.0

RE: [PATCH net-next 5/8] net: mdio: realtek-rtl9300: adapt spaces for defines

[Jagielski, Jedrzej]

From: Markus Stockhausen <markus.stockhausen@gmx.de> 
Sent: Tuesday, June 2, 2026 7:27 AM

>Register and fields will get longer prefixes in the future.

I don't get the reason for touching it again in the future;
cannot it be renamed with adding prefix right now?

>Keep define blocks clean and readable and add another tab.
>Additionally clean up some multi-space separators with tabs.
>
>Just cosmetics, no functional change.
>
>Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

RE: [PATCH net-next 6/8] net: mdio: realtek-rtl9300: relocate topology setup

[Jagielski, Jedrzej]

From: Markus Stockhausen <markus.stockhausen@gmx.de> 
Sent: Tuesday, June 2, 2026 7:27 AM

>Until now the driver sets up the port to bus/address topology of the
>controller after all buses are set up via otto_emdio_probe_one(). This
>does not work for devices where U-Boot skips this setup. It is not
>only needed for the hardware internal background PHY polling engine
>but essential for access to the PHYs during probing.
>
>Depending on the SoC type there exist two different register arrays
>
>- Bus mapping registers (RTL930x, RTL931x) define to which bus the port
>  is attached. E.g. [1]
>- Address mapping registers (RTL838x, RTL930x, RTL931x) define to which
>  address of the bus the port is attached. E.g. [2]
>
>Relocate the topology setup and make it generic. For this
>
>- Define device-specific bus_base/addr_base attributes that give the
>  register base address where the mapping lives. In case one or both are
>  not given the SoC does not support this specific type of mapping.
>- Create a helper otto_emdio_setup_topology() that writes the detected
>  topology to the registers.
>- Call this helper prior to otto_emdio_probe_one().
>- Remove unneeded code from otto_emdio_9300_mdiobus_init().
>
>Subtle change: The old coding used regmap_bulk_write and silently wrote
>bus=0/address=0 to mapping registers for ports that are out of scope.
>The new coding leaves those untouched.
>
>[1] https://svanheule.net/realtek/longan/register/smi_port0_15_polling_sel
>[2] https://svanheule.net/realtek/longan/register/smi_port0_5_addr_ctrl
>
>Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
>---
> drivers/net/mdio/mdio-realtek-rtl9300.c | 70 ++++++++++++++++---------
> 1 file changed, 45 insertions(+), 25 deletions(-)
>
>diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
>index 74ea2550e652..9f67d8dd4185 100644
>--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
>+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
>@@ -56,7 +56,7 @@
> #define RTL9300_NUM_PORTS			28
> #define SMI_GLB_CTRL				0xca00
> #define   GLB_CTRL_INTF_SEL(intf)		BIT(16 + (intf))
>-#define SMI_PORT0_15_POLLING_SEL		0xca08
>+#define RTL9300_SMI_PORT0_15_POLLING_SEL	0xca08
> #define RTL9300_SMI_ACCESS_PHY_CTRL_0		0xcb70
> #define RTL9300_SMI_ACCESS_PHY_CTRL_1		0xcb74
> #define   PHY_CTRL_REG_ADDR			GENMASK(24, 20)
>@@ -74,7 +74,7 @@
> #define RTL9300_SMI_ACCESS_PHY_CTRL_3		0xcb7c
> #define   PHY_CTRL_MMD_DEVAD			GENMASK(20, 16)
> #define   PHY_CTRL_MMD_REG			GENMASK(15, 0)
>-#define SMI_PORT0_5_ADDR_CTRL			0xcb80
>+#define RTL9300_SMI_PORT0_5_ADDR_CTRL		0xcb80
> 
> #define MAX_PORTS				28
> #define MAX_SMI_BUSSES				4
>@@ -89,6 +89,8 @@ struct otto_emdio_cmd_regs {
> };
> 
> struct otto_emdio_info {
>+	u32 addr_map_base;
>+	u32 bus_map_base;
> 	u32 cmd_fail;
> 	u32 cmd_read;
> 	u32 cmd_write;
>@@ -327,24 +329,46 @@ static int otto_emdio_write_c45(struct mii_bus *bus, int phy_id,
> 	return ret;
> }
> 
>-static int otto_emdio_9300_mdiobus_init(struct otto_emdio_priv *priv)
>+static int otto_emdio_write_mapping(struct otto_emdio_priv *priv, u32 base, u32 port,
>+				    u32 ports_per_reg, u32 bits_per_val, u32 value)
> {
>-	u32 glb_ctrl_mask = 0, glb_ctrl_val = 0;
>-	struct regmap *regmap = priv->regmap;
>-	u32 port_addr[5] = { 0 };
>-	u32 poll_sel[2] = { 0 };
>-	int i, err;
>+	u32 shift = (port % ports_per_reg) * bits_per_val;

are we sure @ports_per_reg cannot be equal to 0 in any scenario?

>+	u32 reg = base + (port / ports_per_reg) * 4;
>+	u32 mask = GENMASK(bits_per_val - 1, 0);
> 
>-	/* Associate the port with the SMI interface and PHY */
>-	for_each_set_bit(i, priv->valid_ports, priv->info->num_ports) {
>-		int pos;
>+	return regmap_update_bits(priv->regmap, reg, mask << shift, value << shift);
>+}
> 
>-		pos = (i % 6) * 5;
>-		port_addr[i / 6] |= (priv->smi_addr[i] & 0x1f) << pos;
>+static int otto_emdio_setup_topology(struct otto_emdio_priv *priv)
>+{
>+	const struct otto_emdio_info *info = priv->info;
>+	u32 port;
>+	int ret;
> 
>-		pos = (i % 16) * 2;
>-		poll_sel[i / 16] |= (priv->smi_bus[i] & 0x3) << pos;
>+	for_each_set_bit(port, priv->valid_ports, info->num_ports) {
>+		if (info->bus_map_base) {
>+			/* 16 ports per register, 2 bits per port (bus index 0-3) */
>+			ret = otto_emdio_write_mapping(priv, info->bus_map_base, port,
>+						       16, 2, priv->smi_bus[port]);
>+			if (ret)
>+				return ret;
>+		}
>+		if (info->addr_map_base) {
>+			/* 6 ports per register, 5 bits per port (PHY address 0-31) */
>+			ret = otto_emdio_write_mapping(priv, info->addr_map_base, port,
>+						       6, 5, priv->smi_addr[port]);
>+			if (ret)
>+				return ret;
>+		}
> 	}
>+	return 0;

Magic numbers appearing here and there, what is discouraged in netdev i believe
Any chance to replace them with corresponding defines?

>+}
>+
>+static int otto_emdio_9300_mdiobus_init(struct otto_emdio_priv *priv)
>+{
>+	u32 glb_ctrl_mask = 0, glb_ctrl_val = 0;
>+	struct regmap *regmap = priv->regmap;
>+	int i, err;
> 
> 	/* Put the interfaces into C45 mode if required */
> 	glb_ctrl_mask = GENMASK(19, 16);
>@@ -352,16 +376,6 @@ static int otto_emdio_9300_mdiobus_init(struct otto_emdio_priv *priv)
> 		if (priv->smi_bus_is_c45[i])
> 			glb_ctrl_val |= GLB_CTRL_INTF_SEL(i);
> 
>-	err = regmap_bulk_write(regmap, SMI_PORT0_5_ADDR_CTRL,
>-				port_addr, 5);
>-	if (err)
>-		return err;
>-
>-	err = regmap_bulk_write(regmap, SMI_PORT0_15_POLLING_SEL,
>-				poll_sel, 2);
>-	if (err)
>-		return err;
>-
> 	err = regmap_update_bits(regmap, SMI_GLB_CTRL,
> 				 glb_ctrl_mask, glb_ctrl_val);
> 	if (err)
>@@ -534,6 +548,10 @@ static int otto_emdio_probe(struct platform_device *pdev)
> 	if (err)
> 		return err;
> 
>+	err = otto_emdio_setup_topology(priv);
>+	if (err)
>+		return err;
>+
> 	device_for_each_child_node_scoped(dev, child) {
> 		err = otto_emdio_probe_one(dev, priv, child);
> 		if (err)
>@@ -548,6 +566,8 @@ static int otto_emdio_probe(struct platform_device *pdev)
> }
> 
> static const struct otto_emdio_info otto_emdio_9300_info = {
>+	.addr_map_base = RTL9300_SMI_PORT0_5_ADDR_CTRL,
>+	.bus_map_base = RTL9300_SMI_PORT0_15_POLLING_SEL,
> 	.cmd_fail = PHY_CTRL_FAIL,
> 	.cmd_read = PHY_CTRL_READ,
> 	.cmd_write = PHY_CTRL_WRITE,
>-- 
>2.54.0

AW: [PATCH net-next 5/8] net: mdio: realtek-rtl9300: adapt spaces for defines

[Markus Stockhausen]

> Von: Jagielski, Jedrzej <jedrzej.jagielski@intel.com> 
> Gesendet: Dienstag, 2. Juni 2026 11:43
> Betreff: RE: [PATCH net-next 5/8] net: mdio: realtek-rtl9300: adapt spaces
for defines
> 
> From: Markus Stockhausen <markus.stockhausen@gmx.de> 
> Sent: Tuesday, June 2, 2026 7:27 AM
> 
> >Register and fields will get longer prefixes in the future.
>
> I don't get the reason for touching it again in the future;
> cannot it be renamed with adding prefix right now?

All remaining field defines will get an RTL9300 prefix. As there
are some inconsistencies regarding the spacing (tabs on the top,
spaces on the bottom) I gave it a separate commit. If it is
accepted I will make ths a combined "Change defines prefixes 
and spacing" commit.

Markus

AW: [PATCH net-next 2/8] net: mdio: realtek-rtl9300: Correctly handle ethernet-phy-package

[Markus Stockhausen]

> Von: Jagielski, Jedrzej <jedrzej.jagielski@intel.com> 
> Gesendet: Dienstag, 2. Juni 2026 11:32
> Betreff: RE: [PATCH net-next 2/8] net: mdio: realtek-rtl9300: Correctly
handle ethernet-phy-package
> ...
> >Signed-off-by: Manuel Stocker <mensi@mensi.ch>
> >Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
> 
> Hi Markus
> quite unclear (at least for me) what's Manuel's role on that patch.
> If he's a co-author please add proper tag, if you are sending Manuel's
patch on his behalf please change authorship. 

Unsure about this but situation as follows:

- Manuel identified the issue downstream and sent a patch [1]
- I advised to make that a separate function.
- Manuel sent a new version that I merged
- Now I'm sending the same upstream

Please advise how I need to reflect this here.

Markus

[1] https://github.com/openwrt/openwrt/pull/23591

AW: [PATCH net-next 3/8] net: mdio: realtek-rtl9300: harden otto_emdio_map_ports()

[Markus Stockhausen]

> Von: Jagielski, Jedrzej <jedrzej.jagielski@intel.com> 
> Gesendet: Dienstag, 2. Juni 2026 11:38
> Betreff: RE: [PATCH net-next 3/8] net: mdio: realtek-rtl9300: harden
otto_emdio_map_ports()
> ...
> >+		if (addr >= PHY_MAX_ADDR)
> >+			return dev_err_probe(dev, -EINVAL, "illegal smi
address %d\n", 
> >+addr);
>
> Could we have more meaningful error code than EINVAL; EFAULT perhaps if
that's bad addr case?
> what do you think?

EFAULT seems to be the most unusal error code here.
But looking at other implementations there is no clear
guidance.

- phy package uses -EINVAL [1]
- ax88172a uses -ENODEV [2]
- altera uses -ENODEV [3]

> Also - is %d convenient for printing address value?

Will take that as it matches the other MDIO outputs. 

Markus

[1]
https://elixir.bootlin.com/linux/v6.18.1/source/drivers/net/phy/phy_package.
c#L215 
[2]
https://elixir.bootlin.com/linux/v6.18.1/source/drivers/net/usb/ax88172a.c#L
213
[3]
https://elixir.bootlin.com/linux/v6.18.1/source/drivers/net/ethernet/altera/
altera_tse_main.c#L625 

AW: [PATCH net-next 6/8] net: mdio: realtek-rtl9300: relocate topology setup

[Markus Stockhausen]

> Von: Jagielski, Jedrzej <jedrzej.jagielski@intel.com> 
> Gesendet: Dienstag, 2. Juni 2026 11:51
> Betreff: RE: [PATCH net-next 6/8] net: mdio: realtek-rtl9300: relocate
topology setup
> ... 
> >-static int otto_emdio_9300_mdiobus_init(struct otto_emdio_priv *priv)
> >+static int otto_emdio_write_mapping(struct otto_emdio_priv *priv, u32
base, u32 port,
> >+				    u32 ports_per_reg, u32 bits_per_val, u32
value)
> > {
> >-	u32 glb_ctrl_mask = 0, glb_ctrl_val = 0;
> >-	struct regmap *regmap = priv->regmap;
> >-	u32 port_addr[5] = { 0 };
> >-	u32 poll_sel[2] = { 0 };
> >-	int i, err;
> >+	u32 shift = (port % ports_per_reg) * bits_per_val;
>
> are we sure @ports_per_reg cannot be equal to 0 in any scenario?

This helper is only implemented to make the topology 
setup code easier to read. So this is never gets a zero input. 
I can rename the function to clearer indicate the tight coupling.

> ... 
> >-		pos = (i % 16) * 2;
> >-		poll_sel[i / 16] |= (priv->smi_bus[i] & 0x3) << pos;
> >+	for_each_set_bit(port, priv->valid_ports, info->num_ports) {
> >+		if (info->bus_map_base) {
> >+			/* 16 ports per register, 2 bits per port (bus index
0-3) */
> >+			ret = otto_emdio_write_mapping(priv,
info->bus_map_base, port,
> >+						       16, 2,
priv->smi_bus[port]);
> >+			if (ret)
> >+				return ret;
> >+		}
> >+		if (info->addr_map_base) {
> >+			/* 6 ports per register, 5 bits per port (PHY
address 0-31) */
> >+			ret = otto_emdio_write_mapping(priv,
info->addr_map_base, port,
> >+						       6, 5,
priv->smi_addr[port]);
> >+			if (ret)
> >+				return ret;
> >+		}
> > 	}
> >+	return 0;
>
> Magic numbers appearing here and there, what is discouraged in netdev i
believe
> Any chance to replace them with corresponding defines?

No problem, will ad that to v2.

Markus

RE: [PATCH net-next 5/8] net: mdio: realtek-rtl9300: adapt spaces for defines

[Jagielski, Jedrzej]

From: Markus Stockhausen <markus.stockhausen@gmx.de> 
Sent: Tuesday, June 2, 2026 12:19 PM

>> Von: Jagielski, Jedrzej <jedrzej.jagielski@intel.com> 
>> Gesendet: Dienstag, 2. Juni 2026 11:43
>> Betreff: RE: [PATCH net-next 5/8] net: mdio: realtek-rtl9300: adapt spaces
>for defines
>> 
>> From: Markus Stockhausen <markus.stockhausen@gmx.de> 
>> Sent: Tuesday, June 2, 2026 7:27 AM
>> 
>> >Register and fields will get longer prefixes in the future.
>>
>> I don't get the reason for touching it again in the future;
>> cannot it be renamed with adding prefix right now?
>
>All remaining field defines will get an RTL9300 prefix. As there
>are some inconsistencies regarding the spacing (tabs on the top,
>spaces on the bottom) I gave it a separate commit. If it is
>accepted I will make ths a combined "Change defines prefixes 
>and spacing" commit.
>
>Markus

So for me both are resonable
either add prefixes within the commit fixing spaces/tabs
either fix spaces/tabs when touching given macro within each
commit

touching it again and again seems to be redundant ihmo

RE: [PATCH net-next 2/8] net: mdio: realtek-rtl9300: Correctly handle ethernet-phy-package

[Jagielski, Jedrzej]

From: Markus Stockhausen <markus.stockhausen@gmx.de> 
Sent: Tuesday, June 2, 2026 12:23 PM

>> Von: Jagielski, Jedrzej <jedrzej.jagielski@intel.com> 
>> Gesendet: Dienstag, 2. Juni 2026 11:32
>> Betreff: RE: [PATCH net-next 2/8] net: mdio: realtek-rtl9300: Correctly
>handle ethernet-phy-package
>> ...
>> >Signed-off-by: Manuel Stocker <mensi@mensi.ch>
>> >Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
>> 
>> Hi Markus
>> quite unclear (at least for me) what's Manuel's role on that patch.
>> If he's a co-author please add proper tag, if you are sending Manuel's
>patch on his behalf please change authorship. 
>
>Unsure about this but situation as follows:
>
>- Manuel identified the issue downstream and sent a patch [1]
>- I advised to make that a separate function.
>- Manuel sent a new version that I merged
>- Now I'm sending the same upstream
>
>Please advise how I need to reflect this here.
>
>Markus
>
>[1] https://github.com/openwrt/openwrt/pull/23591

So it looks like you are upstreaming Manuel's patch right?
In that case using --author option of git commit would be the
most appropriate way i believe.

There is also Suggested-by tag which can be used in situation
when you advised to Manuel.

RE: [PATCH net-next 3/8] net: mdio: realtek-rtl9300: harden otto_emdio_map_ports()

[Jagielski, Jedrzej]

From: Markus Stockhausen <markus.stockhausen@gmx.de> 
Sent: Tuesday, June 2, 2026 12:43 PM

>> Von: Jagielski, Jedrzej <jedrzej.jagielski@intel.com>
>> Gesendet: Dienstag, 2. Juni 2026 11:38
>> Betreff: RE: [PATCH net-next 3/8] net: mdio: realtek-rtl9300: harden
>otto_emdio_map_ports()
>> ...
>> >+		if (addr >= PHY_MAX_ADDR)
>> >+			return dev_err_probe(dev, -EINVAL, "illegal smi
>address %d\n", 
>> >+addr);
>>
>> Could we have more meaningful error code than EINVAL; EFAULT perhaps 
>> if
>that's bad addr case?
>> what do you think?
>
>EFAULT seems to be the most unusal error code here.

#define	EFAULT		14	/* Bad address */

But yeah, EINVAL is still fine

>But looking at other implementations there is no clear guidance.
>
>- phy package uses -EINVAL [1]
>- ax88172a uses -ENODEV [2]
>- altera uses -ENODEV [3]
>
>> Also - is %d convenient for printing address value?
>
>Will take that as it matches the other MDIO outputs. 

Sure, not insisting but [2] actually shows that hex is used
		netdev_err(dev->net, "Invalid PHY address %#x\n", ret);

>
>Markus
>
>[1]
>https://elixir.bootlin.com/linux/v6.18.1/source/drivers/net/phy/phy_package.
>c#L215
>[2]
>https://elixir.bootlin.com/linux/v6.18.1/source/drivers/net/usb/ax88172a.c#L
>213
>[3]
>https://elixir.bootlin.com/linux/v6.18.1/source/drivers/net/ethernet/altera/
>altera_tse_main.c#L625 

Re: [PATCH net-next 1/8] net: mdio: realtek-rtl9300: Convert to fwnode API

[Andrew Lunn]

On Tue, Jun 02, 2026 at 07:26:33AM +0200, Markus Stockhausen wrote:
> The driver still uses legacy "of_" functions to determine the port mapping
> during probing. Convert this to the modern fwnode API. With that also fix
> a subtle lookup bug in the original code.

fwnode does not exist because it is modern. It allows you to have
other bindings, like ACPI binding, using the same code. Do you need an
ACPI binding? Have you documented this new ACPI binding? Has ACPI been
tested? Are there any OF properties which as listed as deprecated
which have been converted to fwnode? A new binding should not start
out with deprecated properties.

	Andrew

Re: [PATCH net-next 3/8] net: mdio: realtek-rtl9300: harden otto_emdio_map_ports()

[Andrew Lunn]

On Tue, Jun 02, 2026 at 07:26:35AM +0200, Markus Stockhausen wrote:
> Due to its design the MDIO driver needs to set up a port to bus/address
> mapping during probing. The "ethernet-ports" subnodes are scanned and
> from the "phy-handle" property the MDIO nodes are looked up. In case of
> a malformed device tree the driver might produce out-of-bounds accesses.
> The PHY address is not checked against the maximum supported address.
> 
> Add a sanity check. Do not use the existing (unused) MAX_SMI_ADDR define
> but drop it in favour of PHY_MAX_ADDR.
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
> ---
>  drivers/net/mdio/mdio-realtek-rtl9300.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 381c45e0597b..517b01115f34 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -78,7 +78,6 @@
>  
>  #define MAX_PORTS       28
>  #define MAX_SMI_BUSSES  4
> -#define MAX_SMI_ADDR	0x1f
>  #define RAW_PAGE(priv)	((priv)->info->num_pages - 1)
>  
>  
> @@ -495,6 +494,9 @@ static int otto_emdio_map_ports(struct device *dev)
>  		if (err)
>  			return err;
>  
> +		if (addr >= PHY_MAX_ADDR)
> +			return dev_err_probe(dev, -EINVAL, "illegal smi address %d\n", addr);

Maybe use the existing helper: of_mdio_parse_addr(). It already has
error handling.

	Andrew

Re: [PATCH net-next 5/8] net: mdio: realtek-rtl9300: adapt spaces for defines

[Andrew Lunn]

On Tue, Jun 02, 2026 at 07:26:37AM +0200, Markus Stockhausen wrote:
> Register and fields will get longer prefixes in the future.
> Keep define blocks clean and readable and add another tab.
> Additionally clean up some multi-space separators with tabs.

As pointed out in another reply, there are logically two different
changes going on here.

Doing checkpatch cleanup is fine, when part of a larger patch
series. So if checkpatch is complaining about spaces vs tabs, feel
free to clean them up. But please say in the commit message that is
what you are doing.

Changing the indentation level i would do when you add the prefix
which needs the new indentation level.

      Andrew

Re: [PATCH net-next 6/8] net: mdio: realtek-rtl9300: relocate topology setup

[Andrew Lunn]

On Tue, Jun 02, 2026 at 07:26:38AM +0200, Markus Stockhausen wrote:
> Until now the driver sets up the port to bus/address topology of the
> controller after all buses are set up via otto_emdio_probe_one(). This
> does not work for devices where U-Boot skips this setup. It is not
> only needed for the hardware internal background PHY polling engine
> but essential for access to the PHYs during probing.

Talking about polling, when do the locking issues around polling get
solved? How many more patchsets before that appears?

	Andrew

AW: [PATCH net-next 1/8] net: mdio: realtek-rtl9300: Convert to fwnode API

[Markus Stockhausen]

> Von: Andrew Lunn <andrew@lunn.ch> 
> Gesendet: Dienstag, 2. Juni 2026 14:06
> An: Markus Stockhausen <markus.stockhausen@gmx.de>
> Betreff: Re: [PATCH net-next 1/8] net: mdio: realtek-rtl9300: Convert to
fwnode API
> 
> On Tue, Jun 02, 2026 at 07:26:33AM +0200, Markus Stockhausen wrote:
> > The driver still uses legacy "of_" functions to determine the port
mapping
> > during probing. Convert this to the modern fwnode API. With that also
fix
> > a subtle lookup bug in the original code.
>
> fwnode does not exist because it is modern. It allows you to have
> other bindings, like ACPI binding, using the same code. Do you need an
> ACPI binding? Have you documented this new ACPI binding? Has ACPI been
> tested? Are there any OF properties which as listed as deprecated
> which have been converted to fwnode? A new binding should not start
> out with deprecated properties.

As I'm no firmware expert, thanks for the note. 
I came across this mix in the old code:

	err = fwnode_property_read_u32(port, "reg", &pn);
	...
	err = of_property_read_u32(mdio_dn, "reg", &bus);

So I decided to get consistent and opt for the 
fwnode version. To be precise: We are using 
device tree only here and we do not need to 
change any structure or properties. 

Leave all as is then? If yes, do I use "of_" or "fwnode_" 
for new functions? E.g. for patch 2/8.

Markus

Re: [PATCH net-next 8/8] net: mdio: realtek-rtl9300: Convert port lookup from O(n) to O(1)

[Andrew Lunn]

On Tue, Jun 02, 2026 at 07:26:40AM +0200, Markus Stockhausen wrote:
> When addressing a PHY on a bus/address the driver must translate that into
> a port number. This is currently done with an effort of O(n). The driver
> loops over all known ports and checks each if bus and address match. This
> is very inefficient especially for the following reasons:
> 
> - The lookup must run for each single read/write operation
> - The SoCs have very low specs (e.g. RTL838x one core, 16K cache, 500 MHz)
> - High-port-count devices RTL839x and RTL931x provide up to 52 PHY ports
> 
> Create a reverse lookup table per channel during setup. This stores the
> (overall) port number per PHY (on that channel). Take special care about
> absent ports that are missing from device tree because of hardware design.
> Save memory by using s8 type for the table (-1 = absent, 0..56 = port).
> Convert the lookup in otto_emdio_phy_to_port() from loop-based to
> table-based.

Generally, for performance patches, you would include some benchmark
numbers.

What percentage of the time of an MDIO transaction is doing the
lookup, vs shifting bits out the hardware and polling for completion?
A 2.5MHz bus is not very fast, compared to a 500MHz CPU.

	Andrew

Re: [PATCH net-next 1/8] net: mdio: realtek-rtl9300: Convert to fwnode API

[Andrew Lunn]

On Tue, Jun 02, 2026 at 02:29:23PM +0200, Markus Stockhausen wrote:
> > Von: Andrew Lunn <andrew@lunn.ch> 
> > Gesendet: Dienstag, 2. Juni 2026 14:06
> > An: Markus Stockhausen <markus.stockhausen@gmx.de>
> > Betreff: Re: [PATCH net-next 1/8] net: mdio: realtek-rtl9300: Convert to
> fwnode API
> > 
> > On Tue, Jun 02, 2026 at 07:26:33AM +0200, Markus Stockhausen wrote:
> > > The driver still uses legacy "of_" functions to determine the port
> mapping
> > > during probing. Convert this to the modern fwnode API. With that also
> fix
> > > a subtle lookup bug in the original code.
> >
> > fwnode does not exist because it is modern. It allows you to have
> > other bindings, like ACPI binding, using the same code. Do you need an
> > ACPI binding? Have you documented this new ACPI binding? Has ACPI been
> > tested? Are there any OF properties which as listed as deprecated
> > which have been converted to fwnode? A new binding should not start
> > out with deprecated properties.
> 
> As I'm no firmware expert, thanks for the note. 
> I came across this mix in the old code:
> 
> 	err = fwnode_property_read_u32(port, "reg", &pn);
> 	...
> 	err = of_property_read_u32(mdio_dn, "reg", &bus);
> 
> So I decided to get consistent and opt for the 
> fwnode version. To be precise: We are using 
> device tree only here and we do not need to 
> change any structure or properties. 
> 
> Leave all as is then? If yes, do I use "of_" or "fwnode_" 
> for new functions? E.g. for patch 2/8.

Please use of_

And in this case, i think there is a helper you can use. In general,
take a look around and try to reuse as much existing code as
possible. That might mean changing core code, making a static function
global, etc.

	Andrew

AW: [PATCH net-next 6/8] net: mdio: realtek-rtl9300: relocate topology setup

[Markus Stockhausen]

> Von: Andrew Lunn <andrew@lunn.ch> 
> Gesendet: Dienstag, 2. Juni 2026 14:24
> An: Markus Stockhausen <markus.stockhausen@gmx.de>
> Betreff: Re: [PATCH net-next 6/8] net: mdio: realtek-rtl9300: relocate
topology setup
> 
> On Tue, Jun 02, 2026 at 07:26:38AM +0200, Markus Stockhausen wrote:
> > Until now the driver sets up the port to bus/address topology of the
> > controller after all buses are set up via otto_emdio_probe_one(). This
> > does not work for devices where U-Boot skips this setup. It is not
> > only needed for the hardware internal background PHY polling engine
> > but essential for access to the PHYs during probing.
>
> Talking about polling, when do the locking issues around polling get
> solved? How many more patchsets before that appears?

This series relocates all hardware setup before bus registration.
- It avoids wrong PHY access before topology setup. 
- Side effect: setup does not interfere with concurrent bus reads/writes

With the next series I want to deactivate/activate hardware polling 
during the whole probing process (besides other things). This avoids
that hardware polling fills wrong MAC register states during
bus/PHY setup.

Are you thinking about this? If yes I can add that to this series.

Markus

Re: [PATCH net-next 6/8] net: mdio: realtek-rtl9300: relocate topology setup

[Andrew Lunn]

> With the next series I want to deactivate/activate hardware polling 
> during the whole probing process (besides other things). This avoids
> that hardware polling fills wrong MAC register states during
> bus/PHY setup.
> 
> Are you thinking about this? If yes I can add that to this series.

No, i mean during normal operation. When the core takes the mdio bus
lock, you need to disable polling until the lock is released. That is
going to need some additional API calls in struct mii_bus.

      Andrew

Re: [PATCH net-next 1/8] net: mdio: realtek-rtl9300: Convert to fwnode API

[Jakub Kicinski]

On Tue,  2 Jun 2026 07:26:33 +0200 Markus Stockhausen wrote:
> -		struct device_node *phy_dn __free(device_node) =
> -			of_parse_phandle(to_of_node(port), "phy-handle", 0);
> +		struct fwnode_handle *phy_fwnode __free(fwnode_handle) =
> +			fwnode_find_reference(port, "phy-handle", 0);
>  		/* skip ports without phys */
> -		if (!phy_dn)
> +		if (IS_ERR(phy_fwnode))
>  			continue;
>  
> -		mdio_dn = phy_dn->parent;
> +		struct fwnode_handle *bus_fwnode __free(fwnode_handle) =
> +			fwnode_get_parent(phy_fwnode);
> +		if (!bus_fwnode)
> +			continue;

Variable declarations mid functions are a no-go for netdev.

If you could please move away from using the __free() crap I'd
appreciate. Clearly missed it yesterday when applying these patches

Quoting documentation:

  Using device-managed and cleanup.h constructs
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  
  Low level cleanup constructs (such as ``__free()``) can be used when building
  APIs and helpers, especially scoped iterators. However, direct use of
  ``__free()`` within networking core and drivers is discouraged.
  Similar guidance applies to declaring variables mid-function.
  
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#using-device-managed-and-cleanup-h-constructs

AW: [PATCH net-next 6/8] net: mdio: realtek-rtl9300: relocate topology setup

[Markus Stockhausen]

> Von: Andrew Lunn <andrew@lunn.ch> 
> Gesendet: Dienstag, 2. Juni 2026 17:18
> An: Markus Stockhausen <markus.stockhausen@gmx.de>
> Betreff: Re: [PATCH net-next 6/8] net: mdio: realtek-rtl9300: relocate
topology setup
> 
> > With the next series I want to deactivate/activate hardware polling 
> > during the whole probing process (besides other things). This avoids
> > that hardware polling fills wrong MAC register states during
> > bus/PHY setup.
> > 
> > Are you thinking about this? If yes I can add that to this series.
>
> No, i mean during normal operation. When the core takes the mdio bus
> lock, you need to disable polling until the lock is released. That is
> going to need some additional API calls in struct mii_bus.

I have always tried to avoid kernel hacks downstream and never
thought about enhancing the mii_bus. But as this seems to be 
some kind of invitation I will shortly explain the situation here.

The hardware polling in these devices is very important for the DSA
driver. Especially as a port can be either PHY or SerDes driven. Some
MAC operations simply do not work if a wrong state is reported.

Hardware polling and PHY access coexist without any issues. The
MDIO controller coordinates the different requests and the kernel 
does not need to take care about that. 

However we have rare cases where polling should be disabled.
There is no evidence about any issues in the OpenWrt ecosystem 
but vendor SDK disables polling for certain sequence of operations. 
These include

- PHY Firmware patching
- Switching the port on a dual Fibre/Copper PHY
- Enabling/disabling EEE on a port

So we can benefit from some kind of bus enhancement that 
allows to "isolate" a PHY on a bus from any hardware interfering. 
Best case would be to allow the PHY to access these functions.

As said no idea at the moment because never thought about it.
So any guidance is appreciated.

Markus

AW: [PATCH net-next 1/8] net: mdio: realtek-rtl9300: Convert to fwnode API

[Markus Stockhausen]

> Von: Jakub Kicinski <kuba@kernel.org> 
> Gesendet: Dienstag, 2. Juni 2026 20:06
> An: Markus Stockhausen <markus.stockhausen@gmx.de>
> Betreff: Re: [PATCH net-next 1/8] net: mdio: realtek-rtl9300: Convert to
fwnode API
> 
> On Tue,  2 Jun 2026 07:26:33 +0200 Markus Stockhausen wrote:
> > -		struct device_node *phy_dn __free(device_node) =
> > -			of_parse_phandle(to_of_node(port), "phy-handle", 0);
> > +		struct fwnode_handle *phy_fwnode __free(fwnode_handle) =
> > +			fwnode_find_reference(port, "phy-handle", 0);
> >  		/* skip ports without phys */
> > -		if (!phy_dn)
> > +		if (IS_ERR(phy_fwnode))
> >  			continue;
> >  
> > -		mdio_dn = phy_dn->parent;
> > +		struct fwnode_handle *bus_fwnode __free(fwnode_handle) =
> > +			fwnode_get_parent(phy_fwnode);
> > +		if (!bus_fwnode)
> > +			continue;
>
> Variable declarations mid functions are a no-go for netdev.
>
> If you could please move away from using the __free() crap I'd
> appreciate. Clearly missed it yesterday when applying these patches

Thanks for the reminder.

Yesterdays patches should contain neither of those. For the
new series I stupidly assumed that the existing __free usage
is accepted. Will take care about this for v2 and future patches.

Markus

AW: [PATCH net-next 8/8] net: mdio: realtek-rtl9300: Convert port lookup from O(n) to O(1)

[Markus Stockhausen]

> Von: Andrew Lunn <andrew@lunn.ch> 
> Gesendet: Dienstag, 2. Juni 2026 14:30
> An: Markus Stockhausen <markus.stockhausen@gmx.de>
> Betreff: Re: [PATCH net-next 8/8] net: mdio: realtek-rtl9300: Convert port
lookup from O(n) to O(1)
>...
> > Create a reverse lookup table per channel during setup. This stores 
> > the
> > (overall) port number per PHY (on that channel). Take special care 
> > about absent ports that are missing from device tree because of hardware
design.
> > Save memory by using s8 type for the table (-1 = absent, 0..56 = port).
> > Convert the lookup in otto_emdio_phy_to_port() from loop-based to 
> > table-based.
>
> Generally, for performance patches, you would include some benchmark
numbers.
>
> What percentage of the time of an MDIO transaction is doing the lookup, vs
shifting bits out the hardware and polling for completion?
> A 2.5MHz bus is not very fast, compared to a 500MHz CPU.

IIRC I saved 100us per polling loop over all 52 ports of a RTL839x switch.
But this was too long ago and I will need to verify this once again.  It is
not critical for now so I will drop the patch for v2.

Markus