[PATCH net-next v15 00/13] net: mdio: realtek-rtl9300: Add RTL83xx support

[PATCH net-next v15 00/13] net: mdio: realtek-rtl9300: Add RTL83xx support

[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

While the MDIO hardware polling unit and its necessity for the MAC 
layer was always well known, no detailed documentation was available.
For this series the MDIO bus was inspected with a logic analyzer for 
a better understanding how polling and kernel access interact on the 
bus. All this will be explained now in the driver comments.

This patch series adds support for the RTL83xx devices. For this

- Enhance device tree binding.
- Add special handling for limitations enforced by hardware polling.
  These already have minor side effects on RTL93xx devices but are even
  more critical for the RTL83xx hardware.
- Add RTL83xx coding.

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

v14 -> v15:
  - Copy kerneldoc for phy_detach_internal(). (Andrew)
  - Keep phy_detach() comment in phy_attach() error path. (Andrew)
  - Use more SHA digits for fixes tag. (Sashiko)
  - Replace -ENOTSUPP with -EOPNOTSUPP. (Sashiko)
  - Replace dev_WARN_ONCE() with dev_warn_once(). This reduces log
    spam especially for high port count devices. (Sashiko)
v14: https://lore.kernel.org/netdev/20260817162632.1208891-1-markus.stockhausen@gmx.de/
v14 sashiko-gemini: https://sashiko.dev/#/patchset/20260817162632.1208891-1-markus.stockhausen%40gmx.de

v13 -> v14:
  - Add a patch to allow C22/C45 access in parallel. Add C45 read/write 
    functions for RTL838x and RTL839x. Drop the corresponding pre-patch 
    that checked for empty C45 functions. (Andrew)
  - Do NOT add Reviewed-by to "Add support for RTL838x" patch as it
    was changed due to above adaption. (Markus)
  - Add separate patch with C45 over C22 error check. (Andrew)
  - Relocate addition of phy_detach_internal() into a separate commit
    for easier reviews. (Andrew)
  - Add Reviewed-by to "Add polling documentation" patch (Andrew)
  - Add Reviewed-by to "reword Kconfig & module" patch (Andrew)
  - Add "Fixes" to bind/unbind attribute patch (Sashiko)
v13: https://lore.kernel.org/netdev/20260807171058.522833-1-markus.stockhausen@gmx.de/
v13 sashiko-nipa: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260807171058.522833-1-markus.stockhausen%40gmx.de
v13 sashiko-gemini: https://sashiko.dev/#/patchset/20260807171058.522833-1-markus.stockhausen%40gmx.de

v12 -> v13:
  - Unmapped PHYs do not exist on these devices. Add error message and
    clarify commit message of patch 5. (Sashiko)
  - Rename not only Kconfig but also module description to reflect
    the list of supported devices. (Sashiko)
  - Reword sysfs bind/unbind patch to clarify that for now this kind
    of mitigation is sufficient. (Sashiko)
  - Explain in commit message that relying on regmap lock for 
    enabling/disabling of polling is sufficient. (Sashiko)
  - Use dev_err_probe() for C45 check in otto_emdio_map_ports().
    (Sashiko) 
  - Make clear that the bit registers (polling & port_mask) are 2x32 
    bit and have their lower bits 0..31 in <reg> and higher bits 
    31..51 are in <reg+4>. (Sashiko) 
v12: https://lore.kernel.org/netdev/20260803171853.507506-1-markus.stockhausen@gmx.de/
v12 sashiko-nipa: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260803171853.507506-1-markus.stockhausen%40gmx.de
v12 sashiko-gemini: https://sashiko.dev/#/patchset/20260803171853.507506-1-markus.stockhausen%40gmx.de

v11 -> v12:
  - v11 was sent with the wrong starting commit and with leftovers
    from v10. Sorry for that and thanks to Jeremy.
  - Allow to support devices that as of now only need C22 read/write
    helpers. This avoids to carry around unneeded C45 code for RTL83xx.
    The feature is implemented with an additional patch. With this 
    drop RTL83xx C45 helpers. (Sashiko)
v11: https://lore.kernel.org/netdev/20260802083528.490216-1-markus.stockhausen@gmx.de/
v11 Sashiko review faild

v10 -> v11:
  - Fix typo in bindings patch. Cypress has only 52 ports. (Sashiko)
  - Drop "extend priv lifetime" patch this was too complex and produced
    even more Sashiko findings. Instead use suppress_bind_attrs to
    disable sysfs unbinding. (Sashiko)
  - Rename detach helper phy_detach_internal() to avoid confusion with
    unlocked helpers that start with two underscores. (Sashiko)
  - Clarify the reason for the unconditional notify_phy_detach()
    that Sashiko complained about. (Markus)  
  - Explain that FIELD_PREP(RTL8390_PHY_CTRL_EXT_PAGE, 0x1ff) is taken
    over from SDK and no meaningful name can be given for 0x1ff. Just
    like RTL9300_PHY_CTRL_PARK_PAGE. (Sashiko).
v10: https://lore.kernel.org/netdev/20260731061400.78301-1-markus.stockhausen@gmx.de/
v10 sashiko-nipa: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260731061400.78301-1-markus.stockhausen%40gmx.de
v10 sashiko-gemini: https://sashiko.dev/#/patchset/20260731061400.78301-1-markus.stockhausen%40gmx.de

v9 -> v10:
  - Run devm_add_action_or_reset() after devm_of_mdiobus_register().
    Critical last minute flaw during v9 refactoring. (Sashiko)
  - Enclose page access into guard() during detach notification to
    avoid concurrency issues during teardown. (Sashiko) 
  - Clarify that system suspend/resume ist not supported. Hardware
    polling will not need to take care about system power management.
    (Sashiko)
  - Add patch to reword Kconfig description. (Sashiko)
  - Remark: v9 was mislabeled as v8
v9: https://lore.kernel.org/netdev/20260729160227.155613-1-markus.stockhausen@gmx.de/
v9 Sashiko review: https://sashiko.dev/#/patchset/20260729160227.155613-1-markus.stockhausen%40gmx.de

v8 -> v9:
  - Align polling documentation with sample. Instead of "read" it
    must be "write". (Sashiko)
  - If otto_emdio_init_polling() fails do not leave otto_emdio_probe()
    but jump to error label for cleanup. (Sashiko)
  - Bind cleanup action to the bus and not the controller. This was
    done wrong in v7. (Sashiko)
  - Clear Realtek C22 PHY tracking page during detach. This ensures
    that next PHY attachment does not work on wrong initial page.
    (Sashiko)
  - Explain that polling registers have different names for the
    devices to avoid AI confusion. (Sashiko)
  - Improve notify_phy_attach() and notify_phy_detach() documentation
    to better reflect under what conditions these callbacks run.
    (Sashiko)
  - The v8 lifetime increase rework changed an error message without
    explanation. Undo that change. (Sashiko)
  - Improve page tracking commit message. Explain that bus scan runs
    before Realtek PHY check and thus a delayed error handling is
    accepted for now. (Sashiko)
  - Explain that the driver implements c45 access for RTL838x and
    RTL839x for completeness. (Sashiko)
  - Replace kzalloc() by kzalloc_obj due to checkpatch. (Markus)
  - Use define for page register 31. (Sashiko)
  - Avoid duplicate pointer walk in otto_emdio_notify_phy_attach().
    (Sashiko)
  - Drop unneded parenthesis for phy vendor check inside
    otto_emdio_notify_phy_attach(). (Sashiko)
v8: https://lore.kernel.org/netdev/20260727191559.19617-1-markus.stockhausen@gmx.de/
v8 Sashiko review: https://sashiko.dev/#/patchset/20260727191559.19617-1-markus.stockhausen%40gmx.de 

v7 -> v8:
  - Add patch 4 to increase lifetime of controller->priv structure so
    it is available until the last bus is unregistered. This avoids
    a use-after-free if controller is unbound via sysfs. (Sashiko)
v7: https://lore.kernel.org/netdev/20260726071751.1359156-1-markus.stockhausen@gmx.de/
v7 Sashiko review: https://sashiko.dev/#/message/20260726071751.1359156-1-markus.stockhausen%40gmx.de

v6 -> v7:
  - Drop phy_poll bitmap. It was used to detect asymmetrical attach()
    and detach() callbacks. With v6 callback refactoring this is
    now obsolete. (Sashiko)
  - Relocate notify_detach() callback directly behind phy_suspend()
    for better symmetry. To avoid further AI review complaints about
    missing LIFO compliance explain that phy_detach() even without
    the patch is not compliant. (Sashiko)
v6: https://lore.kernel.org/netdev/20260724055611.1008577-1-markus.stockhausen@gmx.de/
v6 Sashiko review: https://sashiko.dev/#/patchset/20260724055611.1008577-1-markus.stockhausen%40gmx.de  

v5 -> v6:
  - Adapt polling documentation with new findings after questions
    about consistency of 100MBit downspeed detection. (Andrew)
  - Add a clearer explanation to the page tracking commit and why
    it was chosen over a polling enabling/disabling solution. (Andrew)
  - Make bus notification symmetrical and respect LIFO order. For this
    add a __phy_detach() helper that avoids code duplication in the
    phy_attach_direct() error paths. Drop the mdio_bus_notified 
    status bit. (Andrew, Sashiko)
  - Adapt notify_phy_attach()/notify_phy_detach() documentation to
    reflect above changes.
  - Drop lockdep_assert_held check from otto_emdio_set_port_polling()
    and remove lock handling during polling initialization. At that
    point in time the driver has exclusive access. (Andrew)
  - Drop polling check in otto_emdio_notify_phy_attach(). Notification
    is symmetrical now so there is no chance of double invocation.
    (Andrew)
  - Add Reviewed-by to "Increase MDIO timeout" patch (Andrew)
v5: https://lore.kernel.org/netdev/20260722062356.12291-1-markus.stockhausen@gmx.de/
v5 Sashiko review: https://sashiko.dev/#/patchset/20260722062356.12291-1-markus.stockhausen%40gmx.de

v4 -> v5:
  - Design bus notification symmetric so that notify_phy_detach
    is only invoked when notify_phy_attach succeeded. (Jakub)
  - Improve documentation of notification callbacks. (Jakub)
v4: https://lore.kernel.org/netdev/20260709064157.2865063-1-markus.stockhausen@gmx.de/
v4 Sashiko review: https://sashiko.dev/#/patchset/20260709064157.2865063-1-markus.stockhausen%40gmx.de

v3 -> v4:
  - Add Acked-by for dt-bindings commit (Conor) 
  - Test phy_poll bitfield under lock to avoid race. (Sashiko) 
  - Fix whitespace in commit "configure hardware polling" (Markus)
v3: https://lore.kernel.org/netdev/20260705163532.2853959-1-markus.stockhausen@gmx.de/
v3 Sashiko review: https://sashiko.dev/#/patchset/20260705163532.2853959-1-markus.stockhausen%40gmx.de

v2 -> v3:
  - Enhance documentation and make clear that the driver and not the
    kernel must handle the bus mess. (Andrew)
  - Block non-Realtek PHYs on C22 buses to avoid issues with driver
    internal register 31 handling. (Andrew, Chris)
  - Drop C45 over C22 patch. This would need a bus lock/unlock design.
    For all known hardware designs it is not needed. (Andrew)
  - Drop tune_polling() and init_polling() from private structure.
    It is not used in this series and only produces review bot
    questions. (Sashiko)
  - Sort patches for better logical consistency. (Markus)
  - Add device tree patch that was missed in v2. (Markus)
v2: https://lore.kernel.org/netdev/20260629152336.2239826-1-markus.stockhausen@gmx.de/
v2 Sashiko review: https://sashiko.dev/#/patchset/20260629152336.2239826-1-markus.stockhausen%40gmx.de

v1 -> v2:
  - The polling activation logic was refactored. V1 simply activated
    polling after bus probing. Now a dedicated phydev/bus callback
    takes care of this and also handles deferred PHY probing. (Sashiko)
  - Run MMD prefix helper before register 31 (aka Realtek page register) 
    handling. (Jakub's bot)
  - Always run MMD postfix - even if the c22 register access fails. This 
    ensures that the MMD state machine stays consistent. Adapt the error 
    handling inside the postfix function to not overwrite the real MDIO 
    return code (Sashiko, Jakub's bot)
  - Drop unused RTL8390_PHY_CTRL_PARK_PAGE define. Like on RTL931x this 
    field must not be set and thus can be ignored. (Sashiko)
  - Change title in device tree documentation. Because of this do NOT 
    add the Reviewed-by of Krzysztof. (Jakub's bot)
  - Fix wrong use of RTL839x in commit message of patch "c45 over c22 
    mitigation". RTL930x was wrongly named RTL839x in the list of good 
    devices. (Markus)
  - Fix typos (e.g. c22 over c45) in polling documentation (Jakub's bot)
v1: https://lore.kernel.org/netdev/20260613112946.1071411-1-markus.stockhausen@gmx.de/
v1 Sashiko review: https://sashiko.dev/#/patchset/20260613112946.1071411-1-markus.stockhausen@gmx.de


Daniel Golle (1):
  net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

Markus Stockhausen (12):
  dt-bindings: net: realtek,rtl9301-mdio: Add RTL83xx series
  net: mdio: realtek-rtl9300: Add polling documentation
  net: mdio: realtek-rtl9300: deny C45 over C22 access
  net: phy: add phy_detach_internal() helper
  net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes
  net: mdio: realtek-rtl9300: Configure hardware polling during probing
  net: mdio: realtek-rtl9300: Add page tracking
  net: mdio: realtek-rtl9300: Increase MDIO timeout
  net: mdio: realtek-rtl9300: Open up C22 and C45 space in parallel
  net: mdio: realtek-rtl9300: Add support for RTL838x
  net: mdio: realtek-rtl9300: Add support for RTL839x
  net: mdio: realtek-rtl9300: reword Kconfig and module description

 .../bindings/net/realtek,rtl9301-mdio.yaml    |  14 +-
 drivers/net/mdio/Kconfig                      |   4 +-
 drivers/net/mdio/mdio-realtek-rtl9300.c       | 426 +++++++++++++++++-
 drivers/net/phy/phy_device.c                  | 188 ++++----
 include/linux/phy.h                           |  18 +
 5 files changed, 547 insertions(+), 103 deletions(-)

-- 
2.55.0

[PATCH net-next v15 01/13] dt-bindings: net: realtek,rtl9301-mdio: Add RTL83xx series

[Markus Stockhausen]

The lower end Realtek Otto switches provide 1G only and are divided into
two series:

- Maple  : RTL838x up to 28 ports
- Cypress: RTL839x up to 52 ports

The Maple based devices have 3 different SoCs: RTL8380, RTL8381 and
RTL8382. The Cypress series consists of the RTL8391, RTL8392 and
RTL8393 SoCs. The MDIO controller of these switches works like the
existing RTL93xx logic but has different characteristics and different
registers. Add new compatibles in the device tree.

With the extended compatibility list change the title to better reflect
its scope. Especially add the "Ethernet" tag as these devices have
multiple MDIO controllers.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/net/realtek,rtl9301-mdio.yaml         | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
index 271e05bae9c5..67e0b23a8470 100644
--- a/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
+++ b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/net/realtek,rtl9301-mdio.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Realtek RTL9300 MDIO Controller
+title: Realtek Otto Switches Ethernet MDIO Controller
 
 maintainers:
   - Chris Packham <chris.packham@alliedtelesis.co.nz>
@@ -12,6 +12,16 @@ maintainers:
 properties:
   compatible:
     oneOf:
+      - items:
+          - enum:
+              - realtek,rtl8381-mdio
+              - realtek,rtl8382-mdio
+          - const: realtek,rtl8380-mdio
+      - items:
+          - enum:
+              - realtek,rtl8392-mdio
+              - realtek,rtl8393-mdio
+          - const: realtek,rtl8391-mdio
       - items:
           - enum:
               - realtek,rtl9302b-mdio
@@ -24,6 +34,8 @@ properties:
               - realtek,rtl9313-mdio
           - const: realtek,rtl9311-mdio
       - enum:
+          - realtek,rtl8380-mdio
+          - realtek,rtl8391-mdio
           - realtek,rtl9301-mdio
           - realtek,rtl9311-mdio
 
-- 
2.55.0

[PATCH net-next v15 02/13] net: mdio: realtek-rtl9300: Add polling documentation

[Markus Stockhausen]

Add a detailed explanation how the hardware polling unit in the
Realtek Otto switches works. This simplifies developing future
patches and reviewing them.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 75 +++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index afd52a1cd7f8..73ac5fdcd267 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -35,6 +35,81 @@
  *
  * The driver works out the mapping based on the MDIO bus described in device tree and phandles on
  * the ethernet-ports property.
+ *
+ * The devices have a hardware polling unit that runs in the background without any CPU load. It
+ * constantly scans the MDIO bus and the attached PHYs and updates the MAC status registers.
+ *
+ * How does the polling work?
+ *
+ * Each device has a SMI_POLL_CTRL register. A per-port bitmask decides if the hardware polling of
+ * the associated bus/address is active or not. The hardware runs a tight loop over this and for
+ * each set polling bit it issues a status check for the PHY. Attaching a logic analyzer to the
+ * MDIO bus of an RTL8380 and RTL8393 gives the following commands (in kernel notation):
+ *
+ *	RTL8380				RTL8393
+ *	---------------------------	---------------------------
+ *	phy_write(phy, 31, 0x0);	phy_read(phy, 0);
+ *	phy_write(phy, 13, 0x7);	phy_read(phy, 1);
+ *	phy_write(phy, 14, 0x3c);	phy_read(phy, 4);
+ *	phy_write(phy, 13, 0x8007);	phy_read(phy, 5);
+ *	phy_read(phy, 14);		phy_read(phy, 6);
+ *	phy_write(phy, 13, 0x7);	phy_read(phy, 9);
+ *	phy_write(phy, 14, 0x3d);	phy_read(phy, 10);
+ *	phy_write(phy, 13, 0x8007);	phy_read(phy, 15);
+ *	phy_read(phy, 14);		phy_write(phy, 13, 0x7);
+ *	phy_read(phy, 9);		phy_write(phy, 14, 0x3c);
+ *	phy_read(phy, 10);		phy_write(phy, 13, 0x4007);
+ *	phy_read(phy, 15);		phy_read(phy, 14);
+ *	phy_read(phy, 0);		phy_write(phy, 13, 0x7);
+ *	phy_read(phy, 1);		phy_write(phy, 14, 0x3d);
+ *	phy_read(phy, 4);		phy_write(phy, 13, 0x4007);
+ *	phy_read(phy, 5);		phy_read(phy, 14);
+ *	phy_read(phy, 6);
+ *
+ * After one PHY status is read, the polling engine goes over to the next PHY. If one bus is fully
+ * scanned it switches over to the next bus. Basically the polling system is always busy and the
+ * MAC state is updated in real-time.
+ *
+ * This is a glimpse look at the complexity of the polling and leaves out the C45 case and the MAC
+ * register update logic afterwards. Important to note:
+ *
+ * - 100 MBit downshifts (broken cables) are identified and propagated to the MAC correctly
+ * - Access to MDIO_AN_EEE_ADV and MDIO_AN_EEE_LPABLE works via C45 over C22.
+ * - It is unclear if these sequences change for different PHYs.
+ * - Access to Realtek reserved register 26 (link speed) has not yet been seen.
+ *
+ * How does MDIO access from kernel work?
+ *
+ * When issuing MDIO accesses via an MMIO based interface the final write to the command register
+ * sets a "run command now" bit. Between two polling sequences for different PHYs the hardware
+ * checks if a user command needs to run and sends it onto the bus. Afterwards it simply continues
+ * its polling work. Inspecting the command sequence for a paged write on the logic analyzer gives:
+ *
+ *	RTL8380				RTL8393
+ *	---------------------------	---------------------------
+ *	phy_write(phy, 31, page);	phy_write(phy, 31, page);
+ *	phy_write(phy, reg, value);	phy_write(phy, reg, value);
+ *					phy_write(phy, 31, 0);
+ *
+ * What does this mean?
+ *
+ * There are slight differences in polling and PHY access between the models but the challenge
+ * stays the same. On the one hand that greatly simplifies the MAC layer, on the other hand it
+ * has some implications for the kernel PHY subsystem.
+ *
+ * - Without the polling and a proper MAC status, some of the link handling features do not work.
+ *   Especially an unpopulated MAC_LINK_STS register cancels operations to other MAC registers.
+ * - The Realtek page register 31 is magically modified in the background so that polling will
+ *   read the right data. On the RTL838x polling simply resets it to zero. Other devices seem
+ *   to track the page access "magically" in the background.
+ * - A C45 over C22 kernel access sequence is most likely to fail because chances are high that
+ *   the polling engine overwrites registers 13/14 in between.
+ * - PHY firmware loading can have issues. Especially if a PHY is designed to expect a clean
+ *   sequence of registers and values without deviation.
+ * - An access to one PHY will need to wait for the next free slot of the polling engine.
+ *
+ * Conclusion: The Realtek MDIO bus driver PHY access must know and handle any interference that
+ * arises from the above described hardware polling.
  */
 
 #include <linux/bitfield.h>
-- 
2.55.0

[PATCH net-next v15 03/13] net: mdio: realtek-rtl9300: deny C45 over C22 access

[Markus Stockhausen]

Hardware polling breaks C45 over C22 PHY access. Currently this
is accepted by the driver but it silently fails with undefined
results. Make the situation clear to the caller by adding checks,
returning an error and giving a one-time warning message.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 73ac5fdcd267..69565a87f99d 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -115,6 +115,7 @@
 #include <linux/bitfield.h>
 #include <linux/bitmap.h>
 #include <linux/bits.h>
+#include <linux/device.h>
 #include <linux/find.h>
 #include <linux/mdio.h>
 #include <linux/mfd/syscon.h>
@@ -453,6 +454,12 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
 	int ret, port;
 	u32 value;
 
+	if (regnum == MII_MMD_CTRL || regnum == MII_MMD_DATA) {
+		dev_warn_once(&bus->dev,
+			      "C45 over C22 read access broken due to polling\n");
+		return -EOPNOTSUPP;
+	}
+
 	port = otto_emdio_phy_to_port(bus, phy_id);
 	if (port < 0)
 		return port;
@@ -463,11 +470,18 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
 	return ret ? ret : value;
 }
 
-static int otto_emdio_write_c22(struct mii_bus *bus, int phy_id, int regnum, u16 value)
+static int otto_emdio_write_c22(struct mii_bus *bus, int phy_id, int regnum,
+				u16 value)
 {
 	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
 	int ret, port;
 
+	if (regnum == MII_MMD_CTRL || regnum == MII_MMD_DATA) {
+		dev_warn_once(&bus->dev,
+			      "C45 over C22 write access broken due to polling\n");
+		return -EOPNOTSUPP;
+	}
+
 	port = otto_emdio_phy_to_port(bus, phy_id);
 	if (port < 0)
 		return port;
-- 
2.55.0

[PATCH net-next v15 04/13] net: phy: add phy_detach_internal() helper

[Markus Stockhausen]

For the upcoming MDIO bus notification enhancements the PHY detach path
needs to inform the bus about these actions. Until now phy_detach() is
used for the regular detach path as well as from the phy_attach_direct()
error paths So in the future there must be a clear indication if a
notification is needed (normal path) or not (cleanup path).

Carve out the phy_detach() code into a new helper phy_detach_internal()
and make phy_detach() a short stub that calls the new helper.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/phy/phy_device.c | 172 +++++++++++++++++++----------------
 1 file changed, 92 insertions(+), 80 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 94b2e85e00a3..2827d2226d6a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1734,6 +1734,98 @@ static bool phy_drv_supports_irq(const struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
+/**
+ * phy_detach_internal - detach a PHY device from its network device
+ * @phydev: target phy_device struct
+ *
+ * This detaches the phy device from its network device and the phy
+ * driver, and drops the reference count taken in phy_attach_direct().
+ */
+static void phy_detach_internal(struct phy_device *phydev)
+{
+	struct net_device *dev = phydev->attached_dev;
+	struct module *ndev_owner = NULL;
+	struct mii_bus *bus;
+
+	if (phydev->devlink) {
+		device_link_del(phydev->devlink);
+		phydev->devlink = NULL;
+	}
+
+	if (phydev->sysfs_links) {
+		if (dev)
+			sysfs_remove_link(&dev->dev.kobj, "phydev");
+		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
+	}
+
+	if (!phydev->attached_dev)
+		sysfs_remove_file(&phydev->mdio.dev.kobj,
+				  &dev_attr_phy_standalone.attr);
+
+	phy_suspend(phydev);
+	if (dev) {
+		struct hwtstamp_provider *hwprov;
+
+		/* hwprov may technically be protected by ops lock but
+		 * not for devices with a phydev, see phy_link_topo_add_phy()
+		 */
+		hwprov = rtnl_dereference(dev->hwprov);
+		/* Disable timestamp if it is the one selected */
+		if (hwprov && hwprov->phydev == phydev) {
+			rcu_assign_pointer(dev->hwprov, NULL);
+			kfree_rcu(hwprov, rcu_head);
+		}
+
+		phydev->attached_dev->phydev = NULL;
+		phydev->attached_dev = NULL;
+		phy_link_topo_del_phy(dev, phydev);
+	}
+
+	phydev->phy_link_change = NULL;
+	phydev->phylink = NULL;
+
+	if (phydev->mdio.dev.driver)
+		module_put(phydev->mdio.dev.driver->owner);
+
+	/* If the device had no specific driver before (i.e. - it
+	 * was using the generic driver), we unbind the device
+	 * from the generic driver so that there's a chance a
+	 * real driver could be loaded
+	 */
+	if (phydev->is_genphy_driven) {
+		device_release_driver(&phydev->mdio.dev);
+		phydev->is_genphy_driven = 0;
+	}
+
+	/* Assert the reset signal */
+	phy_device_reset(phydev, 1);
+
+	/*
+	 * The phydev might go away on the put_device() below, so avoid
+	 * a use-after-free bug by reading the underlying bus first.
+	 */
+	bus = phydev->mdio.bus;
+
+	put_device(&phydev->mdio.dev);
+	if (dev)
+		ndev_owner = dev->dev.parent->driver->owner;
+	if (ndev_owner != bus->owner)
+		module_put(bus->owner);
+}
+
+/**
+ * phy_detach - detach a PHY device from its network device
+ * @phydev: target phy_device struct
+ *
+ * This detaches the phy device from its network device and the phy
+ * driver, and drops the reference count taken in phy_attach_direct().
+ */
+void phy_detach(struct phy_device *phydev)
+{
+	phy_detach_internal(phydev);
+}
+EXPORT_SYMBOL(phy_detach);
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -1906,86 +1998,6 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 }
 EXPORT_SYMBOL(phy_attach_direct);
 
-/**
- * phy_detach - detach a PHY device from its network device
- * @phydev: target phy_device struct
- *
- * This detaches the phy device from its network device and the phy
- * driver, and drops the reference count taken in phy_attach_direct().
- */
-void phy_detach(struct phy_device *phydev)
-{
-	struct net_device *dev = phydev->attached_dev;
-	struct module *ndev_owner = NULL;
-	struct mii_bus *bus;
-
-	if (phydev->devlink) {
-		device_link_del(phydev->devlink);
-		phydev->devlink = NULL;
-	}
-
-	if (phydev->sysfs_links) {
-		if (dev)
-			sysfs_remove_link(&dev->dev.kobj, "phydev");
-		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
-	}
-
-	if (!phydev->attached_dev)
-		sysfs_remove_file(&phydev->mdio.dev.kobj,
-				  &dev_attr_phy_standalone.attr);
-
-	phy_suspend(phydev);
-	if (dev) {
-		struct hwtstamp_provider *hwprov;
-
-		/* hwprov may technically be protected by ops lock but
-		 * not for devices with a phydev, see phy_link_topo_add_phy()
-		 */
-		hwprov = rtnl_dereference(dev->hwprov);
-		/* Disable timestamp if it is the one selected */
-		if (hwprov && hwprov->phydev == phydev) {
-			rcu_assign_pointer(dev->hwprov, NULL);
-			kfree_rcu(hwprov, rcu_head);
-		}
-
-		phydev->attached_dev->phydev = NULL;
-		phydev->attached_dev = NULL;
-		phy_link_topo_del_phy(dev, phydev);
-	}
-
-	phydev->phy_link_change = NULL;
-	phydev->phylink = NULL;
-
-	if (phydev->mdio.dev.driver)
-		module_put(phydev->mdio.dev.driver->owner);
-
-	/* If the device had no specific driver before (i.e. - it
-	 * was using the generic driver), we unbind the device
-	 * from the generic driver so that there's a chance a
-	 * real driver could be loaded
-	 */
-	if (phydev->is_genphy_driven) {
-		device_release_driver(&phydev->mdio.dev);
-		phydev->is_genphy_driven = 0;
-	}
-
-	/* Assert the reset signal */
-	phy_device_reset(phydev, 1);
-
-	/*
-	 * The phydev might go away on the put_device() below, so avoid
-	 * a use-after-free bug by reading the underlying bus first.
-	 */
-	bus = phydev->mdio.bus;
-
-	put_device(&phydev->mdio.dev);
-	if (dev)
-		ndev_owner = dev->dev.parent->driver->owner;
-	if (ndev_owner != bus->owner)
-		module_put(bus->owner);
-}
-EXPORT_SYMBOL(phy_detach);
-
 int phy_suspend(struct phy_device *phydev)
 {
 	struct net_device *netdev = phydev->attached_dev;
-- 
2.55.0

[PATCH net-next v15 05/13] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

[Markus Stockhausen]

From: Daniel Golle <daniel@makrotopia.org>

Some MDIO buses require programming PHY polling registers depending
on the PHY type. RealTek switch SoCs are the most prominent example
of a DSA switch which doesn't allow to program MAC speed, duplex and
flow-control settings without using PHY polling to do so [1].

Avoid a half-baked solution in the MDIO bus driver because

- it must reinvent the bus scanning to determine the PHYs and
- it must anticipate the right point in time (e.g. deferred PHYs).

Hence there is a need to inform the MDIO bus driver that a PHY is
being attached or detached. Provide two hooks in struct mii_bus

- notify_phy_attach(): called in phy_attach_direct() after PHY
  hardware has been initialized and just before PHY is resumed.
- notify_phy_detach(): called in phy_detach() right after PHY
  has been suspended.

Worth to notice: As of now phy_detach() is not 100% LIFO symmetric
to phy_attach_direct(). E.g. sysfs links are torn down before
suspend while being created before resume. Without reordering of the
detach function the above mentioned notifier placement is the best
possible symmetric implementation. An unconditional call of
notify_phy_detach() was favoured [3].

Remark! A slightly different version of this patch was part of a
former series [2]. The discussion already showed that an initialization
hook should be placed somewhere late during the whole setup. This
commit implants it right after phy_init_hw() as suggested. On top of
this it adds the detach hook.

[1] https://github.com/openwrt/openwrt/pull/21515#discussion_r2714069716
[2] https://lore.kernel.org/netdev/cover.1769053496.git.daniel@makrotopia.org/
[3] https://lore.kernel.org/netdev/9e40f50b-357a-4a93-9f59-94847850835d@lunn.ch/#t

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/phy/phy_device.c | 20 ++++++++++++++++----
 include/linux/phy.h          | 18 ++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2827d2226d6a..de59d864b060 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1737,11 +1737,12 @@ static bool phy_drv_supports_irq(const struct phy_driver *phydrv)
 /**
  * phy_detach_internal - detach a PHY device from its network device
  * @phydev: target phy_device struct
+ * @notify_bus: whether to notify the MDIO bus about the PHY detach
  *
  * This detaches the phy device from its network device and the phy
  * driver, and drops the reference count taken in phy_attach_direct().
  */
-static void phy_detach_internal(struct phy_device *phydev)
+static void phy_detach_internal(struct phy_device *phydev, bool notify_bus)
 {
 	struct net_device *dev = phydev->attached_dev;
 	struct module *ndev_owner = NULL;
@@ -1763,6 +1764,10 @@ static void phy_detach_internal(struct phy_device *phydev)
 				  &dev_attr_phy_standalone.attr);
 
 	phy_suspend(phydev);
+
+	if (notify_bus && phydev->mdio.bus->notify_phy_detach)
+		phydev->mdio.bus->notify_phy_detach(phydev);
+
 	if (dev) {
 		struct hwtstamp_provider *hwprov;
 
@@ -1822,7 +1827,8 @@ static void phy_detach_internal(struct phy_device *phydev)
  */
 void phy_detach(struct phy_device *phydev)
 {
-	phy_detach_internal(phydev);
+	/* cleanup including bus notification */
+	phy_detach_internal(phydev, true);
 }
 EXPORT_SYMBOL(phy_detach);
 
@@ -1968,6 +1974,12 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	if (err)
 		goto error;
 
+	if (phydev->mdio.bus->notify_phy_attach) {
+		err = phydev->mdio.bus->notify_phy_attach(phydev);
+		if (err)
+			goto error;
+	}
+
 	phy_resume(phydev);
 
 	/**
@@ -1982,8 +1994,8 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	return err;
 
 error:
-	/* phy_detach() does all of the cleanup below */
-	phy_detach(phydev);
+	/* phy_detach_internal() does all of the cleanup below */
+	phy_detach_internal(phydev, false);
 	return err;
 
 error_module_put:
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5f8d65868e0f..3d8afe6b7f1c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -376,6 +376,24 @@ struct mii_bus {
 			 int regnum, u16 val);
 	/** @reset: Perform a reset of the bus */
 	int (*reset)(struct mii_bus *bus);
+	/**
+	 * @notify_phy_attach: Perform post-attach handling for MDIO bus
+	 * drivers. Optional and independent of @notify_phy_detach. Called
+	 * in phy_attach_direct() right before phy_resume(). Runs in process
+	 * context, may sleep and may be called with RTNL held. Must not
+	 * acquire or rely on RTNL. Returns 0 on success or negative errno
+	 * on failure. Must unwind its own state on error as attachment is
+	 * aborted.
+	 */
+	int (*notify_phy_attach)(struct phy_device *phydev);
+	/**
+	 * @notify_phy_detach: Perform pre-detach handling for MDIO bus
+	 * drivers. Optional and independent of @notify_phy_attach. Called
+	 * in phy_detach() right after phy_suspend(). Runs in process context,
+	 * may sleep and may be called with RTNL held. Must not acquire or
+	 * rely on RTNL.
+	 */
+	void (*notify_phy_detach)(struct phy_device *phydev);
 
 	/** @stats: Statistic counters per device on the bus */
 	struct mdio_bus_stats stats[PHY_MAX_ADDR];
-- 
2.55.0

[PATCH net-next v15 06/13] net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes

[Markus Stockhausen]

The Realtek MDIO driver relies on devm managed resources for lifetime
management and allocates its internal central controller structure
otto_emdio_priv via devm_kzalloc(). This can lead to a use-after-free
in the following sitauation:

- If the driver is removed or unbound via sysfs the controller devm
  cleanup is triggered immediately. A corrupt devicetree might produce
  a load failure and a cleanup too.
- However the MDIO buses are allocated with devm_mdiobus_alloc_size()
  and are reference counted. So if a MAC driver still holds a reference
  to a PHY device on the bus, the mii_bus will remain alive.
- Any action that is run for the buses afterwards still relies on the
  availability of the central priv structure. This will access already
  freed memory.

There are multiple mitigations to choose from

- Clone all controller->priv data into the bus->priv structure.
- disable sysfs bind/unbind attributes
- Check for "bus->state != MDIOBUS_REGISTERED" in all callbacks

This SoC-integrated MDIO controller cannot be physically hot-plugged
and manual unbinding gives no benefit. The driver is usually
integrated into the kernel and not loaded as a module. Avoid a
complex driver rewrite (data duplication) for now and set
suppress_bind_attrs to true to prevent unbinding via sysfs. This
mitigates issues for the known use cases.

Fixes: 24e31e4747698 ("net: mdio: Add RTL9300 MDIO driver")
Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 69565a87f99d..7de8adec243b 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -844,6 +844,7 @@ static struct platform_driver otto_emdio_driver = {
 	.driver = {
 		.name = "mdio-rtl9300",
 		.of_match_table = otto_emdio_ids,
+		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.55.0

[PATCH net-next v15 07/13] net: mdio: realtek-rtl9300: Configure hardware polling during probing

[Markus Stockhausen]

During PHY probing and configuration complex configuration sequences
might be issued and firmware might be loaded. Hardware polling can
interfere badly with that. E.g. a hardware polling MMD c45 over c22
request might break an ongoing firmware loading sequence.

To avoid such issues the polling of the Realtek Otto switches can be
(de)activated with one or two 32 bit mask registers. Each bit enables
(=1) or disables (=0) the polling of the corresponding port. Make use
of this as follows:

- Disable polling for all ports when the MDIO driver starts.
- Reenable polling just after the PHY has been attached.
- Disable polling just before the PHY is being detached.

This synchronizes the kernel and hardware polling to some extent. It
gracefully handles deferred probing of PHYs in case the driver is
loaded asynchronously during boot. Additionally it brings the hardware
polling into a consistent operation mode for devices where U-Boot does
not take care.

Important notes about the implementation:

Realtek is very inconsistent about its register naming. RTL930x uses
SMI_POLL_CTRL for polling control while it is SMI_PORT_POLLING_CTRL
on RTL931x. Keep these vendor names.

These devices do not support power management for the whole system.
So mdio_bus_phy_resume() is not used and it is not required to
disable/enable hardware polling for that usecase.

These devices have no CPU MAC to PHY design. Thus abort phy_attach()
in case PHYs are not mapped to valid switch ports. All MDIO bus
commands will fail afterwards, leaving it in an unusable state.

Enabling/disabling hardware polling is a simple regmap bit access.
For this it is enough to rely on the regmap internal lock.

This polling enabling/disabling does not solve the current inability
of the driver to handle C45-over-C22 access during normal operation.
For all existing designs this is not needed.

[1] https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/mdio/mdio-realtek-otto.c#L818
[2] https://lore.kernel.org/netdev/680696024a8648535ce6dee771fe4de67802e0e8.1769053496.git.daniel@makrotopia.org/

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

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 7de8adec243b..a925640290cb 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -146,6 +146,7 @@
 #define   RTL9300_PHY_CTRL_INDATA		GENMASK(31, 16)
 #define   RTL9300_PHY_CTRL_DATA			GENMASK(15, 0)
 #define RTL9300_SMI_ACCESS_PHY_CTRL_3		0xcb7c
+#define RTL9300_SMI_POLL_CTRL			0xca90
 #define RTL9300_SMI_PORT0_5_ADDR_CTRL		0xcb80
 
 #define RTL9310_NUM_BUSES			4
@@ -171,6 +172,7 @@
 #define   RTL9310_PHY_CTRL_INDATA		GENMASK(15, 0)
 #define RTL9310_SMI_INDRT_ACCESS_MMD_CTRL	0x0c18
 #define RTL9310_SMI_PORT_ADDR_CTRL		0x0c74
+#define RTL9310_SMI_PORT_POLLING_CTRL		0x0ccc
 #define RTL9310_SMI_PORT_POLLING_SEL		0x0c9c
 
 #define PHY_CTRL_CMD				BIT(0)
@@ -218,6 +220,7 @@ struct otto_emdio_info {
 	u8 num_buses;
 	u8 num_ports;
 	u16 num_pages;
+	u32 poll_ctrl;
 	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);
@@ -253,6 +256,12 @@ static struct otto_emdio_priv *otto_emdio_bus_to_priv(struct mii_bus *bus)
 	return chan->priv;
 }
 
+static int otto_emdio_set_port_polling(struct otto_emdio_priv *priv, int port, bool active)
+{
+	return regmap_assign_bits(priv->regmap, priv->info->poll_ctrl + (port / 32) * 4,
+				  BIT(port % 32), active);
+}
+
 static int otto_emdio_run_cmd(struct mii_bus *bus, u32 cmd,
 			      struct otto_emdio_cmd_regs *cmd_data)
 {
@@ -596,6 +605,36 @@ static int otto_emdio_9310_setup_controller(struct otto_emdio_priv *priv)
 	return 0;
 }
 
+static int otto_emdio_notify_phy_attach(struct phy_device *phydev)
+{
+	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(phydev->mdio.bus);
+	int port = otto_emdio_phy_to_port(phydev->mdio.bus, phydev->mdio.addr);
+
+	if (port < 0) {
+		/* All subsequent bus operations will fail */
+		phydev_err(phydev, "PHY is not mapped to a valid switch port\n");
+		return port;
+	}
+
+	return otto_emdio_set_port_polling(priv, port, true);
+}
+
+static void otto_emdio_notify_phy_detach(struct phy_device *phydev)
+{
+	struct mii_bus *bus = phydev->mdio.bus;
+	struct otto_emdio_priv *priv;
+	int port;
+
+	priv = otto_emdio_bus_to_priv(phydev->mdio.bus);
+	port = otto_emdio_phy_to_port(phydev->mdio.bus, phydev->mdio.addr);
+
+	if (port < 0)
+		return;
+
+	if (otto_emdio_set_port_polling(priv, port, false))
+		dev_err(bus->parent, "failed to disable polling for port %d\n", port);
+}
+
 static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv,
 				 struct fwnode_handle *node)
 {
@@ -625,6 +664,9 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
 		bus->write = otto_emdio_write_c22;
 	}
 	bus->parent = dev;
+	bus->notify_phy_attach = otto_emdio_notify_phy_attach;
+	bus->notify_phy_detach = otto_emdio_notify_phy_detach;
+
 	chan = bus->priv;
 	chan->mdio_bus = mdio_bus;
 	chan->priv = priv;
@@ -741,6 +783,19 @@ static int otto_emdio_map_ports(struct device *dev)
 	return err;
 }
 
+static int otto_emdio_init_polling(struct otto_emdio_priv *priv)
+{
+	int err;
+
+	for (int port = 0; port < priv->info->num_ports; port++) {
+		err = otto_emdio_set_port_polling(priv, port, false);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 static int otto_emdio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -760,6 +815,10 @@ static int otto_emdio_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
+	err = otto_emdio_init_polling(priv);
+	if (err)
+		return err;
+
 	platform_set_drvdata(pdev, priv);
 
 	err = otto_emdio_map_ports(dev);
@@ -800,6 +859,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,
+	.poll_ctrl = RTL9300_SMI_POLL_CTRL,
 	.setup_controller = otto_emdio_9300_setup_controller,
 	.read_c22 = otto_emdio_9300_read_c22,
 	.read_c45 = otto_emdio_9300_read_c45,
@@ -825,6 +885,7 @@ static const struct otto_emdio_info otto_emdio_9310_info = {
 	.num_buses = RTL9310_NUM_BUSES,
 	.num_pages = RTL9310_NUM_PAGES,
 	.num_ports = RTL9310_NUM_PORTS,
+	.poll_ctrl = RTL9310_SMI_PORT_POLLING_CTRL,
 	.setup_controller = otto_emdio_9310_setup_controller,
 	.read_c22 = otto_emdio_9310_read_c22,
 	.read_c45 = otto_emdio_9310_read_c45,
-- 
2.55.0

[PATCH net-next v15 08/13] net: mdio: realtek-rtl9300: Add page tracking

[Markus Stockhausen]

The hardware polling unit of the Realtek switches has a very special
handling for c22 PHY register 31 (aka Realtek page register) in place.

- On the RTL838x it is permanently reset to zero.
- On other devices there is some magic saving/restoring (aka parking)
  in the background in place.

This makes access to PHYs a gamble.

It is vital to keep the polling alive so the MAC layer can rely on
consistent data. Intercept access to c22 register 31 and handle it
internally. Store the desired value for each port in the driver. When
issuing hardware access to other registers add the page to the command
towards the controller. Given this, the hardware will run two c22
commands that are not interrupted by polling.

  ... hardware poll ...
  phy_write(phy, 31, page)
  phy_write(phy, reg, value)
  ... hardware poll ...

Looking at this implementation one might argue that disabling/enabling
polling might be a cleaner solution. But one must remember that

- This driver differentiates clearly between C22 and C45 buses. During
  probing it enables only one of the protocols for a bus.
- All known devices run RTL8218 (B/D/E) or RTL8214FC on 1G
- RTL839x gives link flapping when deactivating polling for a port

So a solution for a Realtek-only ecosystem is required. This commit
copies the downstream-proven driver-only page handling patch without
any new MDIO callbacks and is the lowest common denominator. If a
non-Realtek PHY is identified on a c22 bus the attachment aborts. It
should be noted that bus scan runs with the page handling already in
place before the check in notify_phy_attach(). This is accepted for
now.

Remark: To keep this simple, writes to register 31 are only accepted
if they are lower than the device specific raw page - 0..4094/8190.
Otherwise -EINVAL is returned. Under the above assumption (Only 1G
Realtek PHYs on a c22 bus) this is no limitation.

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

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index a925640290cb..d6027062e452 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -179,6 +179,9 @@
 #define PHY_CTRL_MMD_DEVAD			GENMASK(20, 16)
 #define PHY_CTRL_MMD_REG			GENMASK(15, 0)
 
+#define RTL_VENDOR_ID				0x001cc800
+#define RTL_PAGE_SELECT				31
+
 #define MAP_ADDRS_PER_REG			6
 #define MAP_BITS_PER_ADDR			5
 #define MAP_BITS_PER_BUS			2
@@ -204,6 +207,7 @@ struct otto_emdio_priv {
 	struct regmap *regmap;
 	struct mutex lock; /* protect HW access */
 	DECLARE_BITMAP(valid_ports, MAX_PORTS);
+	u16 page[MAX_PORTS];
 	u8 smi_bus[MAX_PORTS];
 	u8 smi_addr[MAX_PORTS];
 	bool smi_bus_is_c45[MAX_SMI_BUSSES];
@@ -355,7 +359,7 @@ static int otto_emdio_9300_read_c22(struct mii_bus *bus, int port, int regnum, u
 	struct otto_emdio_cmd_regs cmd_data = {
 		.c22_data	= FIELD_PREP(RTL9300_PHY_CTRL_REG_ADDR, regnum) |
 				  FIELD_PREP(RTL9300_PHY_CTRL_PARK_PAGE, 0x1f) |
-				  FIELD_PREP(RTL9300_PHY_CTRL_MAIN_PAGE, RAW_PAGE(priv)),
+				  FIELD_PREP(RTL9300_PHY_CTRL_MAIN_PAGE, priv->page[port]),
 		.io_data	= FIELD_PREP(RTL9300_PHY_CTRL_INDATA, port),
 	};
 
@@ -369,7 +373,7 @@ static int otto_emdio_9300_write_c22(struct mii_bus *bus, int port, int regnum,
 	struct otto_emdio_cmd_regs cmd_data = {
 		.c22_data	= FIELD_PREP(RTL9300_PHY_CTRL_REG_ADDR, regnum) |
 				  FIELD_PREP(RTL9300_PHY_CTRL_PARK_PAGE, 0x1f) |
-				  FIELD_PREP(RTL9300_PHY_CTRL_MAIN_PAGE, RAW_PAGE(priv)),
+				  FIELD_PREP(RTL9300_PHY_CTRL_MAIN_PAGE, priv->page[port]),
 		.io_data	= FIELD_PREP(RTL9300_PHY_CTRL_INDATA, value),
 		.port_mask_low	= BIT(port),
 	};
@@ -409,7 +413,7 @@ static int otto_emdio_9310_read_c22(struct mii_bus *bus, int port, int regnum, u
 	struct otto_emdio_cmd_regs cmd_data = {
 		.broadcast	= FIELD_PREP(RTL9310_BC_PORT_ID, port),
 		.c22_data	= FIELD_PREP(RTL9310_PHY_CTRL_REG_ADDR, regnum) |
-				  FIELD_PREP(RTL9310_PHY_CTRL_MAIN_PAGE, RAW_PAGE(priv)),
+				  FIELD_PREP(RTL9310_PHY_CTRL_MAIN_PAGE, priv->page[port]),
 	};
 
 	return otto_emdio_read_cmd(bus, RTL9310_PHY_CTRL_TYPE_C22, &cmd_data,
@@ -421,7 +425,7 @@ static int otto_emdio_9310_write_c22(struct mii_bus *bus, int port, int regnum,
 	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
 	struct otto_emdio_cmd_regs cmd_data = {
 		.c22_data	= FIELD_PREP(RTL9310_PHY_CTRL_REG_ADDR, regnum) |
-				  FIELD_PREP(RTL9310_PHY_CTRL_MAIN_PAGE, RAW_PAGE(priv)),
+				  FIELD_PREP(RTL9310_PHY_CTRL_MAIN_PAGE, priv->page[port]),
 		.io_data	= FIELD_PREP(RTL9310_PHY_CTRL_INDATA, value),
 		.port_mask_high	= (u32)(BIT_ULL(port) >> 32),
 		.port_mask_low	= (u32)(BIT_ULL(port)),
@@ -473,8 +477,12 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
 	if (port < 0)
 		return port;
 
-	scoped_guard(mutex, &priv->lock)
+	scoped_guard(mutex, &priv->lock) {
+		if (regnum == RTL_PAGE_SELECT)
+			return priv->page[port];
+
 		ret = priv->info->read_c22(bus, port, regnum, &value);
+	}
 
 	return ret ? ret : value;
 }
@@ -495,8 +503,17 @@ static int otto_emdio_write_c22(struct mii_bus *bus, int phy_id, int regnum,
 	if (port < 0)
 		return port;
 
-	scoped_guard(mutex, &priv->lock)
+	scoped_guard(mutex, &priv->lock) {
+		if (regnum == RTL_PAGE_SELECT) {
+			if (value >= RAW_PAGE(priv))
+				return -EINVAL;
+
+			priv->page[port] = value;
+			return 0;
+		}
+
 		ret = priv->info->write_c22(bus, port, regnum, value);
+	}
 
 	return ret;
 }
@@ -607,8 +624,9 @@ static int otto_emdio_9310_setup_controller(struct otto_emdio_priv *priv)
 
 static int otto_emdio_notify_phy_attach(struct phy_device *phydev)
 {
-	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(phydev->mdio.bus);
 	int port = otto_emdio_phy_to_port(phydev->mdio.bus, phydev->mdio.addr);
+	struct otto_emdio_chan *chan = phydev->mdio.bus->priv;
+	struct otto_emdio_priv *priv = chan->priv;
 
 	if (port < 0) {
 		/* All subsequent bus operations will fail */
@@ -616,6 +634,16 @@ static int otto_emdio_notify_phy_attach(struct phy_device *phydev)
 		return port;
 	}
 
+	/* "sync" page in case of previously failed attachment */
+	scoped_guard(mutex, &priv->lock)
+		priv->page[port] = 0;
+
+	if (!priv->smi_bus_is_c45[chan->mdio_bus] &&
+	    !phy_id_compare_vendor(phydev->phy_id, RTL_VENDOR_ID)) {
+		phydev_err(phydev, "Only Realtek PHYs allowed on C22 bus\n");
+		return -EOPNOTSUPP;
+	}
+
 	return otto_emdio_set_port_polling(priv, port, true);
 }
 
@@ -631,6 +659,10 @@ static void otto_emdio_notify_phy_detach(struct phy_device *phydev)
 	if (port < 0)
 		return;
 
+	/* "sync" page for next attachment */
+	scoped_guard(mutex, &priv->lock)
+		priv->page[port] = 0;
+
 	if (otto_emdio_set_port_polling(priv, port, false))
 		dev_err(bus->parent, "failed to disable polling for port %d\n", port);
 }
-- 
2.55.0

[PATCH net-next v15 09/13] net: mdio: realtek-rtl9300: Increase MDIO timeout

[Markus Stockhausen]

RTL838x devices with 28 ports produce PHY access timeout errors during
one of three boots while waiting for MDIO command completion. This is
currently set to 1ms.

Background: Access to the Realtek Otto ethernet MDIO bus must wait for
a free slot between two hardware polls. The polling sequence consists
of at least 17 commands on the RTL838x devices. This delay can be nicely
seen when disabling polling completely. The following times are measured
on a bus running on the default 2.5MHz. Time measured is from the last
register write that sets the command-start-bit until the hardware
responds with the command-finished-bit set.

- average c22 read with polling enabled on all ports: ~380us
- average c22 read with polling enabled on one port: ~380us
- average c22 read with polling completely disabled: ~180us

For this bus frequency the bare hardware runtime for a single command
(32 bit preamble + 32 bit data) is ~25us. So the hardware adds quite
some overhead. On top of this comes the fact that the RTL838x devices
are low on resources (500Mhz 4Kec core with 16K cache).

Increase the timeout to 10ms to be on the safe side.

Remark! In a future patch the bus clock frequency will be made
configurable with a minimum frequency of 1.25MHz. Setting this
(e.g. for debugging purposes) doubles the command run times but
will safely stay below 10ms.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/mdio/mdio-realtek-rtl9300.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index d6027062e452..ae7ba86e90b9 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -274,9 +274,9 @@ static int otto_emdio_run_cmd(struct mii_bus *bus, u32 cmd,
 	u32 cmdstate;
 	int ret;
 
-	/* Defensive pre check just in case something goes horrible wrong */
+	/* Defensive pre check just in case something goes horribly wrong */
 	ret = regmap_read_poll_timeout(priv->regmap, info->cmd_regs.c22_data,
-				       cmdstate, !(cmdstate & PHY_CTRL_CMD), 10, 1000);
+				       cmdstate, !(cmdstate & PHY_CTRL_CMD), 10, 10000);
 	if (ret)
 		return ret;
 
@@ -316,7 +316,7 @@ static int otto_emdio_run_cmd(struct mii_bus *bus, u32 cmd,
 		return ret;
 
 	ret = regmap_read_poll_timeout(priv->regmap, info->cmd_regs.c22_data,
-				       cmdstate, !(cmdstate & PHY_CTRL_CMD), 10, 1000);
+				       cmdstate, !(cmdstate & PHY_CTRL_CMD), 10, 10000);
 	if (ret)
 		return ret;
 
-- 
2.55.0

[PATCH net-next v15 10/13] net: mdio: realtek-rtl9300: Open up C22 and C45 space in parallel

[Markus Stockhausen]

Until now the MDIO driver supports either access to the C22 or to
the C45 address space of the PHYs. This is due to the fact that
the polling configuration favours one of the address spaces and
limits access to the other space. E.g. when polling is set to
C22 most of the C45 space can not be accessed.

There are however some exceptions from that. EEE register access
is allowed independently from the polling mode [1]. As the
downstream driver already allows parallel C22/C45 access [2] do
this in upstream too.

[1] https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/phy/realtek/realtek_multiport.c#L349
[2] https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/mdio/mdio-realtek-otto.c#L1072

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

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index ae7ba86e90b9..b90972c1dc6a 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -688,13 +688,10 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
 		return -ENOMEM;
 
 	bus->name = "Realtek Switch MDIO Bus";
-	if (priv->smi_bus_is_c45[mdio_bus]) {
-		bus->read_c45 = otto_emdio_read_c45;
-		bus->write_c45 = otto_emdio_write_c45;
-	} else {
-		bus->read = otto_emdio_read_c22;
-		bus->write = otto_emdio_write_c22;
-	}
+	bus->read_c45 = otto_emdio_read_c45;
+	bus->write_c45 = otto_emdio_write_c45;
+	bus->read = otto_emdio_read_c22;
+	bus->write = otto_emdio_write_c22;
 	bus->parent = dev;
 	bus->notify_phy_attach = otto_emdio_notify_phy_attach;
 	bus->notify_phy_detach = otto_emdio_notify_phy_detach;
-- 
2.55.0

[PATCH net-next v15 11/13] net: mdio: realtek-rtl9300: Add support for RTL838x

[Markus Stockhausen]

The MDIO driver has been prepared for multiple device support. Add all
required bits for the RTL838x (aka maple) series. This is straightforward
but some things are worth mentioning.

- The device has a lot in common with the RTL930x series. It has 28 ports,
  4096 (Realtek) pages and 4 MMIO registers. With this a lot of the
  existing RTL9300 defines could be reused. But to avoid confusion and
  for better readability duplicate the defines with a proper prefix.
- The MDIO engine has no fail bit. Thus the mask is set to zero.
- There is only one SMI bus for 1G PHYs. No bus_map_base register exists.
- The setup_controller() function needs no C45 configuration as polling
  only works in C22 mode
- The PHY access must be activated by a special register flag.

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

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index b90972c1dc6a..57ab89afb9e5 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -126,6 +126,28 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 
+#define RTL8380_NUM_BUSES			1
+#define RTL8380_NUM_PAGES			4096
+#define RTL8380_NUM_PORTS			28
+#define RTL8380_SMI_GLB_CTRL			0xa100
+#define   RTL8380_SMI_PHY_PATCH_DONE		BIT(15)
+#define RTL8380_SMI_ACCESS_PHY_CTRL_0		0xa1b8
+#define RTL8380_SMI_ACCESS_PHY_CTRL_1		0xa1bc
+#define   RTL8380_PHY_CTRL_REG_ADDR		GENMASK(24, 20)
+#define   RTL8380_PHY_CTRL_PARK_PAGE		GENMASK(19, 15)
+#define   RTL8380_PHY_CTRL_MAIN_PAGE		GENMASK(14, 3)
+#define   RTL8380_PHY_CTRL_WRITE		BIT(2)
+#define   RTL8380_PHY_CTRL_READ			0
+#define   RTL8380_PHY_CTRL_TYPE_C45		BIT(1)
+#define   RTL8380_PHY_CTRL_TYPE_C22		0
+#define   RTL8380_PHY_CTRL_FAIL			0 /* no fail indicator */
+#define RTL8380_SMI_ACCESS_PHY_CTRL_2		0xa1c0
+#define   RTL8380_PHY_CTRL_INDATA		GENMASK(31, 16)
+#define   RTL8380_PHY_CTRL_DATA			GENMASK(15, 0)
+#define RTL8380_SMI_ACCESS_PHY_CTRL_3		0xa1c4
+#define RTL8380_SMI_POLL_CTRL			0xa17c
+#define RTL8380_SMI_PORT0_5_ADDR_CTRL		0xa1c8
+
 #define RTL9300_NUM_BUSES			4
 #define RTL9300_NUM_PAGES			4096
 #define RTL9300_NUM_PORTS			28
@@ -353,6 +375,60 @@ static int otto_emdio_write_cmd(struct mii_bus *bus, u32 cmd,
 	return otto_emdio_run_cmd(bus, cmd | priv->info->cmd_write, cmd_data);
 }
 
+static int otto_emdio_8380_read_c22(struct mii_bus *bus, int port, int regnum, u32 *value)
+{
+	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
+	struct otto_emdio_cmd_regs cmd_data = {
+		.c22_data	= FIELD_PREP(RTL8380_PHY_CTRL_REG_ADDR, regnum) |
+				  FIELD_PREP(RTL8380_PHY_CTRL_PARK_PAGE, 0x1f) |
+				  FIELD_PREP(RTL8380_PHY_CTRL_MAIN_PAGE, priv->page[port]),
+		.io_data	= FIELD_PREP(RTL8380_PHY_CTRL_INDATA, port),
+	};
+
+	return otto_emdio_read_cmd(bus, RTL8380_PHY_CTRL_TYPE_C22, &cmd_data,
+				   RTL8380_PHY_CTRL_DATA, value);
+}
+
+static int otto_emdio_8380_write_c22(struct mii_bus *bus, int port, int regnum, u16 value)
+{
+	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
+	struct otto_emdio_cmd_regs cmd_data = {
+		.c22_data	= FIELD_PREP(RTL8380_PHY_CTRL_REG_ADDR, regnum) |
+				  FIELD_PREP(RTL8380_PHY_CTRL_PARK_PAGE, 0x1f) |
+				  FIELD_PREP(RTL8380_PHY_CTRL_MAIN_PAGE, priv->page[port]),
+		.io_data	= FIELD_PREP(RTL8380_PHY_CTRL_INDATA, value),
+		.port_mask_low	= BIT(port),
+	};
+
+	return otto_emdio_write_cmd(bus, RTL8380_PHY_CTRL_TYPE_C22, &cmd_data);
+}
+
+static int otto_emdio_8380_read_c45(struct mii_bus *bus, int port,
+				    int dev_addr, int regnum, u32 *value)
+{
+	struct otto_emdio_cmd_regs cmd_data = {
+		.c45_data	= FIELD_PREP(PHY_CTRL_MMD_DEVAD, dev_addr) |
+				  FIELD_PREP(PHY_CTRL_MMD_REG, regnum),
+		.io_data	= FIELD_PREP(RTL8380_PHY_CTRL_INDATA, port),
+	};
+
+	return otto_emdio_read_cmd(bus, RTL8380_PHY_CTRL_TYPE_C45, &cmd_data,
+				   RTL8380_PHY_CTRL_DATA, value);
+}
+
+static int otto_emdio_8380_write_c45(struct mii_bus *bus, int port,
+				     int dev_addr, int regnum, u16 value)
+{
+	struct otto_emdio_cmd_regs cmd_data = {
+		.c45_data	= FIELD_PREP(PHY_CTRL_MMD_DEVAD, dev_addr) |
+				  FIELD_PREP(PHY_CTRL_MMD_REG, regnum),
+		.io_data	= FIELD_PREP(RTL8380_PHY_CTRL_INDATA, value),
+		.port_mask_low	= BIT(port),
+	};
+
+	return otto_emdio_write_cmd(bus, RTL8380_PHY_CTRL_TYPE_C45, &cmd_data);
+}
+
 static int otto_emdio_9300_read_c22(struct mii_bus *bus, int port, int regnum, u32 *value)
 {
 	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
@@ -586,6 +662,15 @@ static int otto_emdio_setup_topology(struct otto_emdio_priv *priv)
 	return 0;
 }
 
+static int otto_emdio_8380_setup_controller(struct otto_emdio_priv *priv)
+{
+	/*
+	 * PHY_PATCH_DONE enables PHY control via SoC. This is required for PHY access, including
+	 * patching and must be set before the PHYs are probed.
+	 */
+	return regmap_set_bits(priv->regmap, RTL8380_SMI_GLB_CTRL, RTL8380_SMI_PHY_PATCH_DONE);
+}
+
 static int otto_emdio_9300_setup_controller(struct otto_emdio_priv *priv)
 {
 	u32 glb_ctrl_mask = 0, glb_ctrl_val = 0;
@@ -873,6 +958,28 @@ static int otto_emdio_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct otto_emdio_info otto_emdio_8380_info = {
+	.addr_map_base = RTL8380_SMI_PORT0_5_ADDR_CTRL,
+	.cmd_fail = RTL8380_PHY_CTRL_FAIL,
+	.cmd_read = RTL8380_PHY_CTRL_READ,
+	.cmd_write = RTL8380_PHY_CTRL_WRITE,
+	.cmd_regs = {
+		.c22_data = RTL8380_SMI_ACCESS_PHY_CTRL_1,
+		.c45_data = RTL8380_SMI_ACCESS_PHY_CTRL_3,
+		.io_data = RTL8380_SMI_ACCESS_PHY_CTRL_2,
+		.port_mask_low = RTL8380_SMI_ACCESS_PHY_CTRL_0,
+	},
+	.num_buses = RTL8380_NUM_BUSES,
+	.num_pages = RTL8380_NUM_PAGES,
+	.num_ports = RTL8380_NUM_PORTS,
+	.poll_ctrl = RTL8380_SMI_POLL_CTRL,
+	.setup_controller = otto_emdio_8380_setup_controller,
+	.read_c22 = otto_emdio_8380_read_c22,
+	.read_c45 = otto_emdio_8380_read_c45,
+	.write_c22 = otto_emdio_8380_write_c22,
+	.write_c45 = otto_emdio_8380_write_c45,
+};
+
 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,
@@ -923,6 +1030,7 @@ static const struct otto_emdio_info otto_emdio_9310_info = {
 };
 
 static const struct of_device_id otto_emdio_ids[] = {
+	{ .compatible = "realtek,rtl8380-mdio", .data = &otto_emdio_8380_info },
 	{ .compatible = "realtek,rtl9301-mdio", .data = &otto_emdio_9300_info },
 	{ .compatible = "realtek,rtl9311-mdio", .data = &otto_emdio_9310_info },
 	{}
-- 
2.55.0

[PATCH net-next v15 12/13] net: mdio: realtek-rtl9300: Add support for RTL839x

[Markus Stockhausen]

The MDIO driver has been prepared for multiple device support. Add all
required bits for the RTL839x (aka cypress) series. This is straightforward
but some things are worth mentioning.

- The device has a lot in common with the RTL931x series. It has 8192
  (Realtek) pages and 7 MMIO registers.
- There are two SMI buses for 1G PHYs. Neither the bus nor address map
  registers exist.
- The hardware has not much to configure. So the setup_controller()
  function is not needed.
- The bit specific registers (polling & port_mask) are 2x32 bit wide. The
  ports 0..31 got to <reg> and the ports 32..51 go to <reg+4>.
- C22 read/write functions must be called with PARK_PAGE = 0. Keep code
  clean and avoid setting it to zero, matching the behavior of the RTL9310
  logic.
- As per SDK the broadcast register allows to write to multiple ports
  at the same time. Unlike RTL9310 where this is filled with a bit mask
  for the current port RTL8390 does not use it for normal reads/writes.
  It is simply set to 0 like the SDK does.
- The SDK fills the EXT_PAGE register with 0x1ff for C22 access and with
  0x0 for C45 access. The reason for this is currently unknown and a
  meaningful name can not be given. Align the driver coding with the
  RTL9300_PHY_CTRL_PARK_PAGE settings and simply fill the hardcoded value.

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

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 57ab89afb9e5..86b2040fa333 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -148,6 +148,28 @@
 #define RTL8380_SMI_POLL_CTRL			0xa17c
 #define RTL8380_SMI_PORT0_5_ADDR_CTRL		0xa1c8
 
+#define RTL8390_NUM_BUSES			2
+#define RTL8390_NUM_PAGES			8192
+#define RTL8390_NUM_PORTS			52
+#define RTL8390_BCAST_PHYID_CTRL		0x03ec
+#define RTL8390_PHYREG_ACCESS_CTRL		0x03dc
+#define   RTL8390_PHY_CTRL_REG_ADDR		GENMASK(9, 5)
+#define   RTL8390_PHY_CTRL_MAIN_PAGE		GENMASK(22, 10)
+#define   RTL8390_PHY_CTRL_FAIL			BIT(1)
+#define   RTL8390_PHY_CTRL_WRITE		BIT(3)
+#define   RTL8390_PHY_CTRL_READ			0
+#define   RTL8390_PHY_CTRL_TYPE_C45		BIT(2)
+#define   RTL8390_PHY_CTRL_TYPE_C22		0
+#define RTL8390_PHYREG_CTRL			0x03e0
+#define   RTL8390_PHY_CTRL_EXT_PAGE		GENMASK(8, 0)
+#define RTL8390_PHYREG_DATA_CTRL		0x03f0
+#define   RTL8390_PHY_CTRL_INDATA		GENMASK(31, 16)
+#define   RTL8390_PHY_CTRL_DATA			GENMASK(15, 0)
+#define RTL8390_PHYREG_MMD_CTRL			0x03f4
+#define RTL8390_PHYREG_PORT_CTRL_LOW		0x03e4
+#define RTL8390_PHYREG_PORT_CTRL_HIGH		0x03e8
+#define RTL8390_SMI_PORT_POLLING_CTRL		0x03fc
+
 #define RTL9300_NUM_BUSES			4
 #define RTL9300_NUM_PAGES			4096
 #define RTL9300_NUM_PORTS			28
@@ -429,6 +451,62 @@ static int otto_emdio_8380_write_c45(struct mii_bus *bus, int port,
 	return otto_emdio_write_cmd(bus, RTL8380_PHY_CTRL_TYPE_C45, &cmd_data);
 }
 
+static int otto_emdio_8390_read_c22(struct mii_bus *bus, int port, int regnum, u32 *value)
+{
+	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
+	struct otto_emdio_cmd_regs cmd_data = {
+		.c22_data	= FIELD_PREP(RTL8390_PHY_CTRL_REG_ADDR, regnum) |
+				  FIELD_PREP(RTL8390_PHY_CTRL_MAIN_PAGE, priv->page[port]),
+		.ext_page	= FIELD_PREP(RTL8390_PHY_CTRL_EXT_PAGE, 0x1ff),
+		.io_data	= FIELD_PREP(RTL8390_PHY_CTRL_INDATA, port),
+	};
+
+	return otto_emdio_read_cmd(bus, RTL8390_PHY_CTRL_TYPE_C22, &cmd_data,
+				   RTL8390_PHY_CTRL_DATA, value);
+}
+
+static int otto_emdio_8390_write_c22(struct mii_bus *bus, int port, int regnum, u16 value)
+{
+	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
+	struct otto_emdio_cmd_regs cmd_data = {
+		.c22_data	= FIELD_PREP(RTL8390_PHY_CTRL_REG_ADDR, regnum) |
+				  FIELD_PREP(RTL8390_PHY_CTRL_MAIN_PAGE, priv->page[port]),
+		.ext_page	= FIELD_PREP(RTL8390_PHY_CTRL_EXT_PAGE, 0x1ff),
+		.io_data	= FIELD_PREP(RTL8390_PHY_CTRL_INDATA, value),
+		.port_mask_high	= (u32)(BIT_ULL(port) >> 32),
+		.port_mask_low	= (u32)(BIT_ULL(port)),
+	};
+
+	return otto_emdio_write_cmd(bus, RTL8390_PHY_CTRL_TYPE_C22, &cmd_data);
+}
+
+static int otto_emdio_8390_read_c45(struct mii_bus *bus, int port,
+				    int dev_addr, int regnum, u32 *value)
+{
+	struct otto_emdio_cmd_regs cmd_data = {
+		.c45_data	= FIELD_PREP(PHY_CTRL_MMD_DEVAD, dev_addr) |
+				  FIELD_PREP(PHY_CTRL_MMD_REG, regnum),
+		.io_data	= FIELD_PREP(RTL8390_PHY_CTRL_INDATA, port),
+	};
+
+	return otto_emdio_read_cmd(bus, RTL8390_PHY_CTRL_TYPE_C45, &cmd_data,
+				   RTL8390_PHY_CTRL_DATA, value);
+}
+
+static int otto_emdio_8390_write_c45(struct mii_bus *bus, int port,
+				     int dev_addr, int regnum, u16 value)
+{
+	struct otto_emdio_cmd_regs cmd_data = {
+		.c45_data	= FIELD_PREP(PHY_CTRL_MMD_DEVAD, dev_addr) |
+				  FIELD_PREP(PHY_CTRL_MMD_REG, regnum),
+		.io_data	= FIELD_PREP(RTL8390_PHY_CTRL_INDATA, value),
+		.port_mask_high	= (u32)(BIT_ULL(port) >> 32),
+		.port_mask_low	= (u32)(BIT_ULL(port)),
+	};
+
+	return otto_emdio_write_cmd(bus, RTL8390_PHY_CTRL_TYPE_C45, &cmd_data);
+}
+
 static int otto_emdio_9300_read_c22(struct mii_bus *bus, int port, int regnum, u32 *value)
 {
 	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
@@ -980,6 +1058,29 @@ static const struct otto_emdio_info otto_emdio_8380_info = {
 	.write_c45 = otto_emdio_8380_write_c45,
 };
 
+static const struct otto_emdio_info otto_emdio_8390_info = {
+	.cmd_fail = RTL8390_PHY_CTRL_FAIL,
+	.cmd_read = RTL8390_PHY_CTRL_READ,
+	.cmd_write = RTL8390_PHY_CTRL_WRITE,
+	.cmd_regs = {
+		.broadcast = RTL8390_BCAST_PHYID_CTRL,
+		.c22_data = RTL8390_PHYREG_ACCESS_CTRL,
+		.c45_data = RTL8390_PHYREG_MMD_CTRL,
+		.ext_page = RTL8390_PHYREG_CTRL,
+		.io_data = RTL8390_PHYREG_DATA_CTRL,
+		.port_mask_low = RTL8390_PHYREG_PORT_CTRL_LOW,
+		.port_mask_high = RTL8390_PHYREG_PORT_CTRL_HIGH,
+	},
+	.num_buses = RTL8390_NUM_BUSES,
+	.num_pages = RTL8390_NUM_PAGES,
+	.num_ports = RTL8390_NUM_PORTS,
+	.poll_ctrl = RTL8390_SMI_PORT_POLLING_CTRL,
+	.read_c22 = otto_emdio_8390_read_c22,
+	.read_c45 = otto_emdio_8390_read_c45,
+	.write_c22 = otto_emdio_8390_write_c22,
+	.write_c45 = otto_emdio_8390_write_c45,
+};
+
 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,
@@ -1031,6 +1132,7 @@ static const struct otto_emdio_info otto_emdio_9310_info = {
 
 static const struct of_device_id otto_emdio_ids[] = {
 	{ .compatible = "realtek,rtl8380-mdio", .data = &otto_emdio_8380_info },
+	{ .compatible = "realtek,rtl8391-mdio", .data = &otto_emdio_8390_info },
 	{ .compatible = "realtek,rtl9301-mdio", .data = &otto_emdio_9300_info },
 	{ .compatible = "realtek,rtl9311-mdio", .data = &otto_emdio_9310_info },
 	{}
-- 
2.55.0

[PATCH net-next v15 13/13] net: mdio: realtek-rtl9300: reword Kconfig and module description

[Markus Stockhausen]

The Realtek MDIO driver has been enhanced to support multiple
devices from the RTL83xx/RTL93xx series. Reword Kconfig and
module description accordingly.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/mdio/Kconfig                | 4 ++--
 drivers/net/mdio/mdio-realtek-rtl9300.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index a05229838cb4..d44278f26fab 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -172,11 +172,11 @@ config MDIO_IPQ8064
 	  interface units of the IPQ8064 SoC
 
 config MDIO_REALTEK_RTL9300
-	tristate "Realtek RTL9300 MDIO interface support"
+	tristate "Realtek RTL83xx/RTL93xx MDIO interface support"
 	depends on MACH_REALTEK_RTL || COMPILE_TEST
 	help
 	  This driver supports the MDIO interface found in the Realtek
-	  RTL9300 family of Ethernet switches with integrated SoC.
+	  RTL83xx/RTL93xx family of Ethernet switches with integrated SoC.
 
 config MDIO_REGMAP
 	tristate
diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 86b2040fa333..11d078ef941c 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -1150,5 +1150,5 @@ static struct platform_driver otto_emdio_driver = {
 
 module_platform_driver(otto_emdio_driver);
 
-MODULE_DESCRIPTION("RTL9300 MDIO driver");
+MODULE_DESCRIPTION("RTL83xx/RTL93xx MDIO driver");
 MODULE_LICENSE("GPL");
-- 
2.55.0

Re: [PATCH net-next v15 04/13] net: phy: add phy_detach_internal() helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] phy_attach_direct() error path destructively tears down pre-existing PHY attachments and corrupts module refcounts.
--

commit 9c0668542a6d1a738f40f7d8a8ebdeecb5d1257f
Author: Markus Stockhausen <markus.stockhausen@gmx.de>
Subject: net: phy: add phy_detach_internal() helper

This patch carves out the phy_detach() code into a new helper
phy_detach_internal() and makes phy_detach() a short stub that calls the
new helper. This prepares for upcoming MDIO bus notification enhancements
that need to distinguish between normal detach and cleanup paths.

> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 8cb0d60fcbba9..69b04a92b7766 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1763,6 +1763,98 @@ static bool phy_drv_supports_irq(const struct phy_driver *phydrv)
>  	return phydrv->config_intr && phydrv->handle_interrupt;
>  }
>  
> +/**
> + * phy_detach_internal - detach a PHY device from its network device
> + * @phydev: target phy_device struct
> + *
> + * This detaches the phy device from its network device and the phy
> + * driver, and drops the reference count taken in phy_attach_direct().
> + */
> +static void phy_detach_internal(struct phy_device *phydev)
> +{
> +	struct net_device *dev = phydev->attached_dev;
> +	struct module *ndev_owner = NULL;
> +	struct mii_bus *bus;

[ ... ]

> +	phy_suspend(phydev);
> +	if (dev) {
> +		struct hwtstamp_provider *hwprov;
> +
> +		/* hwprov may technically be protected by ops lock but
> +		 * not for devices with a phydev, see phy_link_topo_add_phy()
> +		 */
> +		hwprov = rtnl_dereference(dev->hwprov);
> +		/* Disable timestamp if it is the one selected */
> +		if (hwprov && hwprov->phydev == phydev) {
> +			rcu_assign_pointer(dev->hwprov, NULL);
> +			kfree_rcu(hwprov, rcu_head);
> +		}
> +
> +		phydev->attached_dev->phydev = NULL;
> +		phydev->attached_dev = NULL;
> +		phy_link_topo_del_phy(dev, phydev);
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but this logic can tear down
an existing attachment when an error occurs during phy_attach_direct().

If phy_attach_direct() is called on a PHY that is already attached to
another device, it hits this check:

drivers/net/phy/phy_device.c:phy_attach_direct() {
	if (phydev->attached_dev) {
		dev_err(&dev->dev, "PHY already attached\n");
		err = -EBUSY;
		goto error;
	}
...
error:
	/* phy_detach() does all of the cleanup below */
	phy_detach(phydev);
	return err;
}

This calls phy_detach_internal() on the old attached_dev, completely
destroying the pre-existing attachment (clearing its phydev pointer,
suspending the PHY, etc.) without notifying it. Can we abort harmlessly
instead of destructively modifying the pre-existing state?

> +
> +	phydev->phy_link_change = NULL;
> +	phydev->phylink = NULL;
> +
> +	if (phydev->mdio.dev.driver)
> +		module_put(phydev->mdio.dev.driver->owner);
> +
> +	/* If the device had no specific driver before (i.e. - it
> +	 * was using the generic driver), we unbind the device
> +	 * from the generic driver so that there's a chance a
> +	 * real driver could be loaded
> +	 */
> +	if (phydev->is_genphy_driven) {
> +		device_release_driver(&phydev->mdio.dev);
> +		phydev->is_genphy_driven = 0;
> +	}
> +
> +	/* Assert the reset signal */
> +	phy_device_reset(phydev, 1);
> +
> +	/*
> +	 * The phydev might go away on the put_device() below, so avoid
> +	 * a use-after-free bug by reading the underlying bus first.
> +	 */
> +	bus = phydev->mdio.bus;
> +
> +	put_device(&phydev->mdio.dev);
> +	if (dev)
> +		ndev_owner = dev->dev.parent->driver->owner;
> +	if (ndev_owner != bus->owner)
> +		module_put(bus->owner);
> +}

[Severity: High]
This is a pre-existing issue, but this balances the bus owner refcount
based on the old device's driver owner.

In the case where phy_attach_direct() fails due to the PHY already being
attached, this module_put() executes with the old caller's module owner.
Could this lead to module refcount underflows or leaks because it balances
the new caller's try_module_get() using the old caller's module?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831143439.2404484-1-markus.stockhausen@gmx.de?part=4

Re: [PATCH net-next v15 06/13] net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete Fix / Use-After-Free due to devm lifetime mismatch
--

commit d4a21a88756b9fbf7a5e82fcf73b2397de6c2abb
Author: Markus Stockhausen <markus.stockhausen@gmx.de>
Subject: net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes

This commit mitigates a use-after-free issue in the Realtek MDIO driver by
setting suppress_bind_attrs to prevent unbinding via sysfs, which avoids
triggering the devm cleanup while bus references are still held.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 69565a87f99db..7de8adec243ba 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -844,6 +844,7 @@ static struct platform_driver otto_emdio_driver = {
>  	.driver = {
>  		.name = "mdio-rtl9300",
>  		.of_match_table = otto_emdio_ids,
> +		.suppress_bind_attrs = true,

[Severity: High]
This is a pre-existing issue, but this mitigation appears incomplete. It
prevents unbinding via sysfs, but does it cover the probe failure vector?

In otto_emdio_probe(), the priv structure is allocated via devm_kzalloc(). 
Then, it iterates over child nodes:

    device_for_each_child_node_scoped(dev, child) {
        err = otto_emdio_probe_one(dev, priv, child);
        if (err)
            return err;
    }

If probing fails on a subsequent child node, the parent probe returns an error.
This triggers devres cleanup which frees the priv structure.

However, if an earlier bus was successfully registered, a MAC driver might have
already attached to a PHY on that bus, holding a reference to the mii_bus. 

When that MAC driver subsequently performs a read, it calls 
otto_emdio_read_c22():

    scoped_guard(mutex, &priv->lock)
        ret = priv->info->read_c22(bus, port, regnum, &value);

Because priv was already freed by devres, does this result in a 
use-after-free? 

Could a malformed device tree triggering a failure on a secondary MDIO bus 
still cause memory corruption due to this lifetime mismatch, despite the
sysfs unbind mitigation?

>  	},
>  };
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831143439.2404484-1-markus.stockhausen@gmx.de?part=6

Re: [PATCH net-next v15 07/13] net: mdio: realtek-rtl9300: Configure hardware polling during probing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing bug in `phy_attach_direct`: the error path for an already-attached PHY inadvertently tears down the existing attachment.
--

commit b9cbeb0822e562618f953089bee5ff136bf2e472
Author: Markus Stockhausen <markus.stockhausen@gmx.de>

net: mdio: realtek-rtl9300: Configure hardware polling during probing

This commit synchronizes hardware polling with the kernel's MDIO driver by
disabling polling at driver start and just before PHY detach, and re-enabling
it after PHY attach. It introduces regmap bit access to handle these polling
state transitions and gracefully handles deferred probing of PHYs.

Link: https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/mdio/mdio-realtek-otto.c#L818
Link: https://lore.kernel.org/netdev/680696024a8648535ce6dee771fe4de67802e0e8.1769053496.git.daniel@makrotopia.org/

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 7de8adec243ba..a925640290cb0 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c

[ ... ]

> @@ -625,6 +664,9 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
>  		bus->write = otto_emdio_write_c22;
>  	}
>  	bus->parent = dev;
> +	bus->notify_phy_attach = otto_emdio_notify_phy_attach;
> +	bus->notify_phy_detach = otto_emdio_notify_phy_detach;
> +

[Severity: High]
This is a pre-existing issue, but while looking at how phy_attach_direct()
calls notify_phy_attach(), I noticed a bug in the error handling for
already-attached PHYs.

Does this error path inadvertently tear down an existing valid attachment?

If a second MAC driver attempts to attach to a PHY that is already attached to
a different interface, phy_attach_direct() catches the error and jumps to the
error label:

drivers/net/phy/phy_device.c:phy_attach_direct() {
    ...
	if (phydev->attached_dev) {
		dev_err(&dev->dev, "PHY already attached\n");
		err = -EBUSY;
		goto error;
	}
    ...
error:
	/* phy_detach_internal() does all of the cleanup below */
	phy_detach_internal(phydev, false);
	return err;
}

This unconditional cleanup calls phy_detach_internal(), which nullifies the
existing pointers:

drivers/net/phy/phy_device.c:phy_detach_internal() {
    ...
		phydev->attached_dev->phydev = NULL;
		phydev->attached_dev = NULL;
		phy_link_topo_del_phy(dev, phydev);
    ...
}

Will this cause a NULL pointer dereference or use-after-free in the original
MAC driver when it next tries to access the PHY, as its phydev pointer has been
unexpectedly cleared?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831143439.2404484-1-markus.stockhausen@gmx.de?part=7

Re: [PATCH net-next v15 10/13] net: mdio: realtek-rtl9300: Open up C22 and C45 space in parallel

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] MDIO bus read and write operations incorrectly return non-standard error codes (-ENOENT for unmapped ports and -ENXIO on NACK), which causes the MDIO core to fatally abort bus scanning and registration.
--

commit be85adee2936e783b98939418cafee8c47a996b5
Author: Markus Stockhausen <markus.stockhausen@gmx.de>

net: mdio: realtek-rtl9300: Open up C22 and C45 space in parallel
    
Until now the MDIO driver supports either access to the C22 or to
the C45 address space of the PHYs. This is due to the fact that
the polling configuration favours one of the address spaces and
limits access to the other space. E.g. when polling is set to
C22 most of the C45 space can not be accessed.
    
There are however some exceptions from that. EEE register access
is allowed independently from the polling mode [1]. As the
downstream driver already allows parallel C22/C45 access [2] do
this in upstream too.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index ae7ba86e90b9a..b90972c1dc6a2 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -688,13 +688,10 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
>  		return -ENOMEM;
>  
>  	bus->name = "Realtek Switch MDIO Bus";
> -	if (priv->smi_bus_is_c45[mdio_bus]) {
> -		bus->read_c45 = otto_emdio_read_c45;
> -		bus->write_c45 = otto_emdio_write_c45;
> -	} else {
> -		bus->read = otto_emdio_read_c22;
> -		bus->write = otto_emdio_write_c22;
> -	}
> +	bus->read_c45 = otto_emdio_read_c45;
> +	bus->write_c45 = otto_emdio_write_c45;
> +	bus->read = otto_emdio_read_c22;
> +	bus->write = otto_emdio_write_c22;

[Severity: High]
This is a pre-existing issue, but I noticed that MDIO bus read and write
operations incorrectly return non-standard error codes (-ENOENT for unmapped
ports and -ENXIO on NACK), which causes the MDIO core to fatally abort bus
scanning and registration.

For example, when a PHY address is unmapped, -ENOENT is propagated to the MDIO
core during a C45 or C22 bus read:

drivers/net/mdio/mdio-realtek-rtl9300.c:otto_emdio_phy_to_port() {
    ...
    return -ENOENT;
}

drivers/net/mdio/mdio-realtek-rtl9300.c:otto_emdio_read_c45() {
    ...
    port = otto_emdio_phy_to_port(bus, phy_id);
    if (port < 0)
        return port;
    ...
}

And when the PHY NACKs the transaction, it returns -ENXIO instead of
successfully completing a read with 0xffff:

drivers/net/mdio/mdio-realtek-rtl9300.c:otto_emdio_run_cmd() {
    ...
    return cmdstate & info->cmd_fail ? -ENXIO : 0;
}

The get_phy_c22_id() and get_phy_c45_ids() core functions translate any unknown
error (like -ENOENT or -ENXIO) to -EIO. This -EIO error propagates to
__of_mdiobus_parse_phys() and mdiobus_scan_bus_c22()/mdiobus_scan_bus_c45(),
which treat it as a fatal bus error, aborting the scanning and registration
process. Consequently, no PHYs on that bus will be functional.

To comply with the MDIO subsystem and allow fallback scanning to continue, read
functions must return 0xffff (simulating line pull-ups) or explicitly return
-ENODEV (or -EIO, which is safely translated to -ENODEV by the core) when a
device is missing or unmapped.

Should these functions be updated to return compliant error codes to prevent the
MDIO core from fatally aborting bus scanning?

>  	bus->parent = dev;
>  	bus->notify_phy_attach = otto_emdio_notify_phy_attach;
>  	bus->notify_phy_detach = otto_emdio_notify_phy_detach;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831143439.2404484-1-markus.stockhausen@gmx.de?part=10

Re: [PATCH net-next v15 03/13] net: mdio: realtek-rtl9300: deny C45 over C22 access

[Andrew Lunn]

On Mon, Aug 31, 2026 at 04:34:29PM +0200, Markus Stockhausen wrote:
> Hardware polling breaks C45 over C22 PHY access. Currently this
> is accepted by the driver but it silently fails with undefined
> results. Make the situation clear to the caller by adding checks,
> returning an error and giving a one-time warning message.
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

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

    Andrew

Re: [PATCH net-next v15 04/13] net: phy: add phy_detach_internal() helper

[Andrew Lunn]

On Mon, Aug 31, 2026 at 04:34:30PM +0200, Markus Stockhausen wrote:
> For the upcoming MDIO bus notification enhancements the PHY detach path
> needs to inform the bus about these actions. Until now phy_detach() is
> used for the regular detach path as well as from the phy_attach_direct()
> error paths So in the future there must be a clear indication if a
> notification is needed (normal path) or not (cleanup path).
> 
> Carve out the phy_detach() code into a new helper phy_detach_internal()
> and make phy_detach() a short stub that calls the new helper.
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

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

    Andrew

Re: [PATCH net-next v15 05/13] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

[Andrew Lunn]

On Mon, Aug 31, 2026 at 04:34:31PM +0200, Markus Stockhausen wrote:
> From: Daniel Golle <daniel@makrotopia.org>
> 
> Some MDIO buses require programming PHY polling registers depending
> on the PHY type. RealTek switch SoCs are the most prominent example
> of a DSA switch which doesn't allow to program MAC speed, duplex and
> flow-control settings without using PHY polling to do so [1].
> 
> Avoid a half-baked solution in the MDIO bus driver because
> 
> - it must reinvent the bus scanning to determine the PHYs and
> - it must anticipate the right point in time (e.g. deferred PHYs).
> 
> Hence there is a need to inform the MDIO bus driver that a PHY is
> being attached or detached. Provide two hooks in struct mii_bus
> 
> - notify_phy_attach(): called in phy_attach_direct() after PHY
>   hardware has been initialized and just before PHY is resumed.
> - notify_phy_detach(): called in phy_detach() right after PHY
>   has been suspended.
> 
> Worth to notice: As of now phy_detach() is not 100% LIFO symmetric
> to phy_attach_direct(). E.g. sysfs links are torn down before
> suspend while being created before resume. Without reordering of the
> detach function the above mentioned notifier placement is the best
> possible symmetric implementation. An unconditional call of
> notify_phy_detach() was favoured [3].
> 
> Remark! A slightly different version of this patch was part of a
> former series [2]. The discussion already showed that an initialization
> hook should be placed somewhere late during the whole setup. This
> commit implants it right after phy_init_hw() as suggested. On top of
> this it adds the detach hook.
> 
> [1] https://github.com/openwrt/openwrt/pull/21515#discussion_r2714069716
> [2] https://lore.kernel.org/netdev/cover.1769053496.git.daniel@makrotopia.org/
> [3] https://lore.kernel.org/netdev/9e40f50b-357a-4a93-9f59-94847850835d@lunn.ch/#t
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

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

    Andrew

Re: [PATCH net-next v15 06/13] net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes

[Andrew Lunn]

On Mon, Aug 31, 2026 at 04:34:32PM +0200, Markus Stockhausen wrote:
> The Realtek MDIO driver relies on devm managed resources for lifetime
> management and allocates its internal central controller structure
> otto_emdio_priv via devm_kzalloc(). This can lead to a use-after-free
> in the following sitauation:
> 
> - If the driver is removed or unbound via sysfs the controller devm
>   cleanup is triggered immediately. A corrupt devicetree might produce
>   a load failure and a cleanup too.
> - However the MDIO buses are allocated with devm_mdiobus_alloc_size()
>   and are reference counted. So if a MAC driver still holds a reference
>   to a PHY device on the bus, the mii_bus will remain alive.
> - Any action that is run for the buses afterwards still relies on the
>   availability of the central priv structure. This will access already
>   freed memory.
> 
> There are multiple mitigations to choose from
> 
> - Clone all controller->priv data into the bus->priv structure.
> - disable sysfs bind/unbind attributes
> - Check for "bus->state != MDIOBUS_REGISTERED" in all callbacks
> 
> This SoC-integrated MDIO controller cannot be physically hot-plugged
> and manual unbinding gives no benefit. The driver is usually
> integrated into the kernel and not loaded as a module. Avoid a
> complex driver rewrite (data duplication) for now and set
> suppress_bind_attrs to true to prevent unbinding via sysfs. This
> mitigates issues for the known use cases.
> 
> Fixes: 24e31e4747698 ("net: mdio: Add RTL9300 MDIO driver")
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

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

    Andrew

Re: [PATCH net-next v15 07/13] net: mdio: realtek-rtl9300: Configure hardware polling during probing

[Andrew Lunn]

On Mon, Aug 31, 2026 at 04:34:33PM +0200, Markus Stockhausen wrote:
> During PHY probing and configuration complex configuration sequences
> might be issued and firmware might be loaded. Hardware polling can
> interfere badly with that. E.g. a hardware polling MMD c45 over c22
> request might break an ongoing firmware loading sequence.
> 
> To avoid such issues the polling of the Realtek Otto switches can be
> (de)activated with one or two 32 bit mask registers. Each bit enables
> (=1) or disables (=0) the polling of the corresponding port. Make use
> of this as follows:
> 
> - Disable polling for all ports when the MDIO driver starts.
> - Reenable polling just after the PHY has been attached.
> - Disable polling just before the PHY is being detached.
> 
> This synchronizes the kernel and hardware polling to some extent. It
> gracefully handles deferred probing of PHYs in case the driver is
> loaded asynchronously during boot. Additionally it brings the hardware
> polling into a consistent operation mode for devices where U-Boot does
> not take care.
> 
> Important notes about the implementation:
> 
> Realtek is very inconsistent about its register naming. RTL930x uses
> SMI_POLL_CTRL for polling control while it is SMI_PORT_POLLING_CTRL
> on RTL931x. Keep these vendor names.
> 
> These devices do not support power management for the whole system.
> So mdio_bus_phy_resume() is not used and it is not required to
> disable/enable hardware polling for that usecase.
> 
> These devices have no CPU MAC to PHY design. Thus abort phy_attach()
> in case PHYs are not mapped to valid switch ports. All MDIO bus
> commands will fail afterwards, leaving it in an unusable state.
> 
> Enabling/disabling hardware polling is a simple regmap bit access.
> For this it is enough to rely on the regmap internal lock.
> 
> This polling enabling/disabling does not solve the current inability
> of the driver to handle C45-over-C22 access during normal operation.
> For all existing designs this is not needed.
> 
> [1] https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/mdio/mdio-realtek-otto.c#L818
> [2] https://lore.kernel.org/netdev/680696024a8648535ce6dee771fe4de67802e0e8.1769053496.git.daniel@makrotopia.org/
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

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

    Andrew

Re: [PATCH net-next v15 08/13] net: mdio: realtek-rtl9300: Add page tracking

[Andrew Lunn]

On Mon, Aug 31, 2026 at 04:34:34PM +0200, Markus Stockhausen wrote:
61;8001;1c> The hardware polling unit of the Realtek switches has a very special
> handling for c22 PHY register 31 (aka Realtek page register) in place.
> 
> - On the RTL838x it is permanently reset to zero.
> - On other devices there is some magic saving/restoring (aka parking)
>   in the background in place.
> 
> This makes access to PHYs a gamble.
> 
> It is vital to keep the polling alive so the MAC layer can rely on
> consistent data. Intercept access to c22 register 31 and handle it
> internally. Store the desired value for each port in the driver. When
> issuing hardware access to other registers add the page to the command
> towards the controller. Given this, the hardware will run two c22
> commands that are not interrupted by polling.
> 
>   ... hardware poll ...
>   phy_write(phy, 31, page)
>   phy_write(phy, reg, value)
>   ... hardware poll ...
> 
> Looking at this implementation one might argue that disabling/enabling
> polling might be a cleaner solution. But one must remember that
> 
> - This driver differentiates clearly between C22 and C45 buses. During
>   probing it enables only one of the protocols for a bus.
> - All known devices run RTL8218 (B/D/E) or RTL8214FC on 1G
> - RTL839x gives link flapping when deactivating polling for a port
> 
> So a solution for a Realtek-only ecosystem is required. This commit
> copies the downstream-proven driver-only page handling patch without
> any new MDIO callbacks and is the lowest common denominator. If a
> non-Realtek PHY is identified on a c22 bus the attachment aborts. It
> should be noted that bus scan runs with the page handling already in
> place before the check in notify_phy_attach(). This is accepted for
> now.
> 
> Remark: To keep this simple, writes to register 31 are only accepted
> if they are lower than the device specific raw page - 0..4094/8190.
> Otherwise -EINVAL is returned. Under the above assumption (Only 1G
> Realtek PHYs on a c22 bus) this is no limitation.
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

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

    Andrew

Re: [PATCH net-next v15 10/13] net: mdio: realtek-rtl9300: Open up C22 and C45 space in parallel

[Andrew Lunn]

On Mon, Aug 31, 2026 at 04:34:36PM +0200, Markus Stockhausen wrote:
> Until now the MDIO driver supports either access to the C22 or to
> the C45 address space of the PHYs. This is due to the fact that
> the polling configuration favours one of the address spaces and
> limits access to the other space. E.g. when polling is set to
> C22 most of the C45 space can not be accessed.
> 
> There are however some exceptions from that. EEE register access
> is allowed independently from the polling mode [1]. As the
> downstream driver already allows parallel C22/C45 access [2] do
> this in upstream too.
> 
> [1] https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/phy/realtek/realtek_multiport.c#L349
> [2] https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/mdio/mdio-realtek-otto.c#L1072
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

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

    Andrew

Re: [PATCH net-next v15 11/13] net: mdio: realtek-rtl9300: Add support for RTL838x

[Andrew Lunn]

On Mon, Aug 31, 2026 at 04:34:37PM +0200, Markus Stockhausen wrote:
> The MDIO driver has been prepared for multiple device support. Add all
> required bits for the RTL838x (aka maple) series. This is straightforward
> but some things are worth mentioning.
> 
> - The device has a lot in common with the RTL930x series. It has 28 ports,
>   4096 (Realtek) pages and 4 MMIO registers. With this a lot of the
>   existing RTL9300 defines could be reused. But to avoid confusion and
>   for better readability duplicate the defines with a proper prefix.
> - The MDIO engine has no fail bit. Thus the mask is set to zero.
> - There is only one SMI bus for 1G PHYs. No bus_map_base register exists.
> - The setup_controller() function needs no C45 configuration as polling
>   only works in C22 mode
> - The PHY access must be activated by a special register flag.
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

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

    Andrew

Re: [PATCH net-next v15 12/13] net: mdio: realtek-rtl9300: Add support for RTL839x

[Andrew Lunn]

On Mon, Aug 31, 2026 at 04:34:38PM +0200, Markus Stockhausen wrote:
61;8001;1c> The MDIO driver has been prepared for multiple device support. Add all
> required bits for the RTL839x (aka cypress) series. This is straightforward
> but some things are worth mentioning.
> 
> - The device has a lot in common with the RTL931x series. It has 8192
>   (Realtek) pages and 7 MMIO registers.
> - There are two SMI buses for 1G PHYs. Neither the bus nor address map
>   registers exist.
> - The hardware has not much to configure. So the setup_controller()
>   function is not needed.
> - The bit specific registers (polling & port_mask) are 2x32 bit wide. The
>   ports 0..31 got to <reg> and the ports 32..51 go to <reg+4>.
> - C22 read/write functions must be called with PARK_PAGE = 0. Keep code
>   clean and avoid setting it to zero, matching the behavior of the RTL9310
>   logic.
> - As per SDK the broadcast register allows to write to multiple ports
>   at the same time. Unlike RTL9310 where this is filled with a bit mask
>   for the current port RTL8390 does not use it for normal reads/writes.
>   It is simply set to 0 like the SDK does.
> - The SDK fills the EXT_PAGE register with 0x1ff for C22 access and with
>   0x0 for C45 access. The reason for this is currently unknown and a
>   meaningful name can not be given. Align the driver coding with the
>   RTL9300_PHY_CTRL_PARK_PAGE settings and simply fill the hardcoded value.
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

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

    Andrew

AW: [PATCH net-next v15 12/13] net: mdio: realtek-rtl9300: Add support for RTL839x

[Markus Stockhausen]

Hi Andrew,

> Von: Andrew Lunn <andrew@lunn.ch> 
> Gesendet: Mittwoch, 2. September 2026 02:18
> An: Markus Stockhausen <markus.stockhausen@gmx.de>
> Betreff: Re: [PATCH net-next v15 12/13] net: mdio: realtek-rtl9300: Add
support for RTL839x
> ... 
> > Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Thousand thanks for your support.

Markus

Re: [net-next,v15,03/13] net: mdio: realtek-rtl9300: deny C45 over C22 access

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 1

- [Medium] The new guard is keyed purely on the register number, so it
  rejects every C22 access to registers 13/14, not only the C45-over-C22…
- [Low] The patch fixes a defect that exists in released kernels — the
  commit message itself states that C45-over-C22 access "is accepted by…

--

net: mdio: realtek-rtl9300: deny C45 over C22 access

This adds a register number check to the C22 read and write accessors of
the RTL9300 MDIO driver so that accesses to MII_MMD_CTRL and MII_MMD_DATA
return -EOPNOTSUPP and emit a one-time warning. The stated reason is that
hardware polling breaks C45 over C22 PHY access.

No Link: tags are present in the commit header.

> net: mdio: realtek-rtl9300: deny C45 over C22 access
>
>     Hardware polling breaks C45 over C22 PHY access. Currently this
>     is accepted by the driver but it silently fails with undefined
>     results.

[Severity: Low]
The changelog describes behaviour that "is accepted by the driver but it
silently fails with undefined results", which reads like a defect present in
released kernels, but there is no Fixes: tag and no note about stable
routing or about this being deliberately net-next only.

The broken accessors are the pre-existing otto_emdio_read_c22() and
otto_emdio_write_c22() installed as bus->read/bus->write in
otto_emdio_probe_one(), which came in with commit 24e31e4747698 ("net:
mdio: Add RTL9300 MDIO driver"). Another patch of this same series ("net:
mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes") already
carries exactly that tag.

Should this one carry a Fixes: tag as well, so backporters can tell which
kernels are affected and whether the new -EOPNOTSUPP behaviour is intended
for stable?

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 73ac5fdcd2674..69565a87f99db 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c

[ ... ]

> @@ -453,6 +454,12 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
>  	int ret, port;
>  	u32 value;
>  
> +	if (regnum == MII_MMD_CTRL || regnum == MII_MMD_DATA) {
> +		dev_warn_once(&bus->dev,
> +			      "C45 over C22 read access broken due to polling\n");
> +		return -EOPNOTSUPP;
> +	}
> +
>  	port = otto_emdio_phy_to_port(bus, phy_id);
>  	if (port < 0)
>  		return port;

[ ... ]

> @@ -463,11 +470,18 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
>  	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
>  	int ret, port;
>  
> +	if (regnum == MII_MMD_CTRL || regnum == MII_MMD_DATA) {
> +		dev_warn_once(&bus->dev,
> +			      "C45 over C22 write access broken due to polling\n");
> +		return -EOPNOTSUPP;
> +	}
> +

[Severity: Medium]
Does this filter reject more than the C45 over C22 indirection?

The mii_bus ->read/->write ABI in include/linux/phy.h only passes
(addr, regnum), so otto_emdio_read_c22() and otto_emdio_write_c22() have no
way to tell an MMD indirection sequence from a plain access to C22 register
13/14. The check keys purely on the register number, so every access to
0x0d/0x0e is denied.

Two Clause 22 Realtek PHYs use register 14 as an ordinary vendor register,
and both entries in realtek_drvs explicitly declare no MMD support:

drivers/net/phy/realtek/realtek_main.c:
	static int rtl8211b_suspend(struct phy_device *phydev)
	{
		phy_write(phydev, MII_MMD_DATA, BIT(9));

		return genphy_suspend(phydev);
	}

	static int rtl8211b_resume(struct phy_device *phydev)
	{
		phy_write(phydev, MII_MMD_DATA, 0);

		return genphy_resume(phydev);
	}

	PHY_ID_MATCH_EXACT(0x001cc910),
	.name		= "RTL8211 Gigabit Ethernet",
	.config_aneg	= rtl8211_config_aneg,
	.read_mmd	= &genphy_read_mmd_unsupported,
	.write_mmd	= &genphy_write_mmd_unsupported,

and rtl8211_config_aneg() does the same for its forced-100Mbit quirk:

		phy_write(phydev, 0x17, 0x2138);
		phy_write(phydev, 0x0e, 0x0260);

Both callers discard the phy_write() return value. On a bus that is not
classified as native C45, otto_emdio_probe_one() installs these accessors:

	} else {
		bus->read = otto_emdio_read_c22;
		bus->write = otto_emdio_write_c22;
	}

so rtl8211b_suspend() -> phy_write() -> mdiobus_write() ->
otto_emdio_write_c22() now returns -EOPNOTSUPP before touching hardware,
the RTL8211B power-down bit is never programmed, suspend still reports
success through genphy_suspend(), and the bus prints "C45 over C22 write
access broken due to polling" even though no MMD access took place.

The C22 bus filter added later in the series by "net: mdio:
realtek-rtl9300: Add page tracking" only checks
phy_id_compare_vendor(phydev->phy_id, RTL_VENDOR_ID), which RTL8211
(0x001cc910) and RTL8211B (0x001cc912) both match, so these PHYs are not
excluded.

Does this also break direct register access to 0x0d/0x0e from userspace via
SIOCGMIIREG/SIOCSMIIREG, which reaches phy_mii_ioctl() ->
mdiobus_read()/mdiobus_write() with no MMD emulation involved?

Would it work to scope the rejection to the actual C22 MMD emulation path,
or to express it as a bus level "no MMD over C22" capability that phylib can
consult, instead of filtering two register numbers in the bus accessors?

>  	port = otto_emdio_phy_to_port(bus, phy_id);
>  	if (port < 0)
>  		return port;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831143439.2404484-1-markus.stockhausen%40gmx.de

Re: [net-next,v15,05/13] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] New struct mii_bus callbacks (notify_phy_attach /
  notify_phy_detach) and their core call sites in phy_attach_direct() /…

--

net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

This adds two optional callbacks to struct mii_bus so an MDIO bus driver
can be told when a PHY is attached or detached. notify_phy_attach() is
invoked in phy_attach_direct() after phy_init_hw() and before
phy_resume(), and notify_phy_detach() is invoked from phy_detach() right
after phy_suspend(). phy_detach_internal() gains a notify_bus argument so
the attach error path does not emit a detach notification.

References from the commit message:
[1] https://github.com/openwrt/openwrt/pull/21515#discussion_r2714069716
[2] https://lore.kernel.org/netdev/cover.1769053496.git.daniel@makrotopia.org/
[3] https://lore.kernel.org/netdev/9e40f50b-357a-4a93-9f59-94847850835d@lunn.ch/#t

> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 69b04a92b7766..41d1b20cde2c0 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c

[ ... ]

> @@ -1997,6 +2003,12 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
>  	if (err)
>  		goto error;
>  
> +	if (phydev->mdio.bus->notify_phy_attach) {
> +		err = phydev->mdio.bus->notify_phy_attach(phydev);
> +		if (err)
> +			goto error;
> +	}
> +
>  	phy_resume(phydev);

[ ... ]

> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 5f8d65868e0ff..3d8afe6b7f1c5 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -376,6 +376,24 @@ struct mii_bus {
>  			 int regnum, u16 val);
>  	/** @reset: Perform a reset of the bus */
>  	int (*reset)(struct mii_bus *bus);
> +	/**
> +	 * @notify_phy_attach: Perform post-attach handling for MDIO bus
> +	 * drivers. Optional and independent of @notify_phy_detach. Called
> +	 * in phy_attach_direct() right before phy_resume(). Runs in process
> +	 * context, may sleep and may be called with RTNL held. Must not
> +	 * acquire or rely on RTNL. Returns 0 on success or negative errno
> +	 * on failure. Must unwind its own state on error as attachment is
> +	 * aborted.
> +	 */
> +	int (*notify_phy_attach)(struct phy_device *phydev);
> +	/**
> +	 * @notify_phy_detach: Perform pre-detach handling for MDIO bus
> +	 * drivers. Optional and independent of @notify_phy_attach. Called
> +	 * in phy_detach() right after phy_suspend(). Runs in process context,
> +	 * may sleep and may be called with RTNL held. Must not acquire or
> +	 * rely on RTNL.
> +	 */
> +	void (*notify_phy_detach)(struct phy_device *phydev);

[Severity: Low]
This isn't a bug, but at this commit nothing in the tree assigns
mii_bus::notify_phy_attach or mii_bus::notify_phy_detach, so both new
members and the two call sites in phy_attach_direct() and
phy_detach_internal() are inert here. Both call sites are NULL-checked, so
there is no misbehaviour.

Checking the rest of the series, the first user does arrive later:
drivers/net/mdio/mdio-realtek-rtl9300.c adds
otto_emdio_notify_phy_attach()/otto_emdio_notify_phy_detach() and wires
them up in otto_emdio_probe_one():

	bus->notify_phy_attach = otto_emdio_notify_phy_attach;
	bus->notify_phy_detach = otto_emdio_notify_phy_detach;

Would it be worth mentioning in the changelog that the first consumer of
these hooks is added by the following patch in the same series ("net: mdio:
realtek-rtl9300: Configure hardware polling during probing"), so readers of
this commit alone know the API is not left without a user?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831143439.2404484-1-markus.stockhausen%40gmx.de

Re: [net-next,v15,06/13] net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete fix: `.suppress_bind_attrs = true` in
  `otto_emdio_driver` (drivers/net/mdio/mdio-realtek-rtl9300.c:847)…

--

net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes

This sets suppress_bind_attrs in otto_emdio_driver so the driver cannot be
unbound through sysfs. The stated goal is to mitigate a use-after-free of
the devm allocated otto_emdio_priv structure, which the reference counted
mii_bus objects keep pointing at after the controller devres is released.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 69565a87f99db..7de8adec243ba 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -844,6 +844,7 @@ static struct platform_driver otto_emdio_driver = {
>  	.driver = {
>  		.name = "mdio-rtl9300",
>  		.of_match_table = otto_emdio_ids,
> +		.suppress_bind_attrs = true,

[Severity: High]

Does this flag actually cover the paths named in the commit message?  In
drivers/base/bus.c the only consumers are bus_add_driver() and
bus_remove_driver():

	if (!drv->suppress_bind_attrs) {
		error = add_bind_files(drv);

plus the CONFIG_DEBUG_TEST_DRIVER_REMOVE check in drivers/base/dd.c.  So
the sysfs bind/unbind files disappear, but device_release_driver() and the
devres unwind can still be reached by other means.

The commit message names the probe failure case as a trigger:

    "A corrupt devicetree might produce a load failure and a cleanup too."

That path is still open.  otto_emdio_probe() registers buses one child at a
time and returns on the first failure:

	device_for_each_child_node_scoped(dev, child) {
		err = otto_emdio_probe_one(dev, priv, child);
		if (err)
			return err;
	}

If the second child hits "undefined smi bus number" or
"illegal (dangling) smi bus number", the first bus was already published by
devm_of_mdiobus_register(), and the driver core then releases priv (and
destroys priv->lock) while the mii_bus can survive on a phy_device
reference held by a MAC/DSA consumer.

Can the bus callbacks then run against the freed priv?  otto_emdio_probe_one()
stores a raw back pointer into the refcounted bus:

	chan = bus->priv;
	chan->mdio_bus = mdio_bus;
	chan->priv = priv;

and otto_emdio_bus_to_priv() hands it back with no lifetime or bus->state
check, so otto_emdio_read_c22() and friends do:

	scoped_guard(mutex, &priv->lock)
		ret = priv->info->read_c22(bus, port, regnum, &value);

which is a mutex_lock plus an indirect call through freed memory.  Note that
phy_detach_internal() in drivers/net/phy/phy_device.c calls the new hook
unconditionally:

	if (notify_bus && phydev->mdio.bus->notify_phy_detach)
		phydev->mdio.bus->notify_phy_detach(phydev);

and __mdiobus_read()/__mdiobus_write() never test bus->state either.

The commit message also states:

    "The driver is usually integrated into the kernel and not loaded as a
    module."

Is that enforced anywhere?  drivers/net/mdio/Kconfig has:

	config MDIO_REALTEK_RTL9300
		tristate "Realtek RTL9300 MDIO interface support"

and the file still ends with module_platform_driver(otto_emdio_driver) and
MODULE_DEVICE_TABLE(), so rmmod reaches driver_unregister() ->
driver_detach() and the same devres teardown.  Removal of the parent node
via of_platform_depopulate() has the same effect.

Given that, should the Fixes: tag and "This mitigates issues for the known
use cases" be softened, or would one of the other two options listed in the
commit message (making bus->priv self contained, or gating the callbacks and
the notify hooks on bus->state) be the change to make instead?  Those would
replace this line rather than build on it.

The end of the series (c965284c5acebf7c11c9214f77cbc28873491728) still has
devm_kzalloc() for priv, chan->priv = priv and module_platform_driver(), so
no later patch changes the ownership model.

>  	},
>  };

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831143439.2404484-1-markus.stockhausen%40gmx.de

Re: [net-next,v15,07/13] net: mdio: realtek-rtl9300: Configure hardware polling during probing

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 2

- [Medium] Asymmetric polling control: otto_emdio_init_polling()
  (drivers/net/mdio/mdio-realtek-rtl9300.c:786) clears the chip-wide…
- [Low] No rollback of the chip-wide polling state on probe failure and
  no teardown counterpart.
- [Low] Concern that otto_emdio_set_port_polling() writes the SMI
  polling-control register without holding priv->lock, allowing polling…

--

net: mdio: realtek-rtl9300: Configure hardware polling during probing

This adds a poll_ctrl register offset per chip variant and a helper to
toggle the per-port hardware polling bit. Polling is disabled for all
ports at probe time, re-enabled from a new notify_phy_attach callback and
disabled again from notify_phy_detach.

References given in the commit message:
[1] https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/mdio/mdio-realtek-otto.c#L818
[2] https://lore.kernel.org/netdev/680696024a8648535ce6dee771fe4de67802e0e8.1769053496.git.daniel@makrotopia.org/

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 7de8adec243ba..a925640290cb0 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c

[ ... ]

> @@ -253,6 +256,12 @@ static struct otto_emdio_priv *otto_emdio_bus_to_priv(struct mii_bus *bus)
>  	return chan->priv;
>  }
>  
> +static int otto_emdio_set_port_polling(struct otto_emdio_priv *priv, int port, bool active)
> +{
> +	return regmap_assign_bits(priv->regmap, priv->info->poll_ctrl + (port / 32) * 4,
> +				  BIT(port % 32), active);
> +}

[Severity: Low]
A question about the claim in the commit message that "Enabling/disabling
hardware polling is a simple regmap bit access. For this it is enough to
rely on the regmap internal lock." otto_emdio_set_port_polling() runs
without priv->lock, so a polling toggle for one port can land while an
indirect command sequence for another port is in flight.

Reading the file again this looks fine, since priv->lock only covers the
indirect command register sequence:

    otto_emdio_read_cmd()/otto_emdio_write_cmd() {
        lockdep_assert_held(&priv->lock);

and poll_ctrl (0xca90 / 0x0ccc) is not one of the command registers
(c22_data/c45_data/io_data/port_mask_low/port_mask_high/broadcast/ext_page),
while regmap_assign_bits() does the read-modify-write under the regmap
lock. Also, in steady state the polling bits of all attached ports are
already set while commands run, so taking priv->lock around the toggle
would not change the interference behaviour.

Is that reasoning correct, i.e. is the intended invariant "priv->lock
covers the command sequence registers only" rather than "all hardware
access"?

[ ... ]

> @@ -596,6 +605,36 @@ static int otto_emdio_9310_setup_controller(struct otto_emdio_priv *priv)
>  	return 0;
>  }
>  
> +static int otto_emdio_notify_phy_attach(struct phy_device *phydev)
> +{
> +	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(phydev->mdio.bus);
> +	int port = otto_emdio_phy_to_port(phydev->mdio.bus, phydev->mdio.addr);
> +
> +	if (port < 0) {
> +		/* All subsequent bus operations will fail */
> +		phydev_err(phydev, "PHY is not mapped to a valid switch port\n");
> +		return port;
> +	}
> +
> +	return otto_emdio_set_port_polling(priv, port, true);
> +}

[ ... ]

> @@ -741,6 +783,19 @@ static int otto_emdio_map_ports(struct device *dev)
>  	return err;
>  }
>  
> +static int otto_emdio_init_polling(struct otto_emdio_priv *priv)
> +{
> +	int err;
> +
> +	for (int port = 0; port < priv->info->num_ports; port++) {
> +		err = otto_emdio_set_port_polling(priv, port, false);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}

[Severity: Medium]
Is the enable/disable pair symmetric here?

otto_emdio_init_polling() clears the polling bit for every port from 0 to
info->num_ports - 1, but the only place a bit is ever set again is
otto_emdio_notify_phy_attach(), which is reached only from
phy_attach_direct() and returns early when otto_emdio_phy_to_port() cannot
map the address:

    if (port < 0) {
        /* All subsequent bus operations will fail */
        phydev_err(phydev, "PHY is not mapped to a valid switch port\n");
        return port;
    }

priv->valid_ports is filled only from ethernet-ports children that carry a
phy-handle:

    otto_emdio_map_ports() {
        ...
        phy_dn = of_parse_phandle(port_dn, "phy-handle", 0);
        /* skip ports without phys */
        if (!phy_dn)
            continue;

So can a port that never gets a phydev attached ever have its polling bit
restored? cameo-rtl9302c-2x-rtl8224-2xge.dts has port@24 and port@25 with
phy-mode = "10gbase-r" and no phy-handle, which look like concrete
instances of that.

The file header states the constraint this affects:

 * - Without the polling and a proper MAC status, some of the link handling features do not work.
 *   Especially an unpopulated MAC_LINK_STS register cancels operations to other MAC registers.

The commit message says "Disable polling for all ports when the MDIO driver
starts" and "it brings the hardware polling into a consistent operation mode
for devices where U-Boot does not take care", but does not mention that
ports without a kernel-managed PHY are never re-enabled and that the
bootloader mask is overwritten without any save/restore. Could that be
spelled out, or the loop restricted to priv->valid_ports?

Note there is no in-tree MAC or DSA driver for these SoCs today, so on a
pure mainline kernel notify_phy_attach() never runs and the net effect of
loading this driver is that polling stays off for all ports. That makes the
functional impact only demonstrable with an out-of-tree or future MAC
driver, but the asymmetry in the code is visible as is.

>  static int otto_emdio_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -760,6 +815,10 @@ static int otto_emdio_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->regmap))
>  		return PTR_ERR(priv->regmap);
>  
> +	err = otto_emdio_init_polling(priv);
> +	if (err)
> +		return err;
> +
>  	platform_set_drvdata(pdev, priv);
>  
>  	err = otto_emdio_map_ports(dev);

[Severity: Low]
Should the polling mask be restored when probe fails?

otto_emdio_init_polling() runs right after the syscon regmap is obtained,
before platform_set_drvdata() and before otto_emdio_map_ports(),
otto_emdio_setup_topology(), info->setup_controller() and
otto_emdio_probe_one()/devm_of_mdiobus_register(), each of which does a
plain "return err".

The bits live in the shared switch syscon regmap rather than in
driver-private memory, so devres unwinding does not undo them, and the
driver has neither a .remove callback nor a devm_add_action() for this.
Does a non-deferred probe failure (missing ethernet-ports node, illegal or
duplicate port number, devm_mdiobus_alloc_size() returning -ENOMEM,
devm_of_mdiobus_register() failing) leave the switch with polling disabled
for all ports with nothing to restore it?

The same applies mid-loop in otto_emdio_init_polling():

    err = otto_emdio_set_port_polling(priv, port, false);
    if (err)
        return err;

which returns with a partially modified mask and no saved copy of the
previous register value.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831143439.2404484-1-markus.stockhausen%40gmx.de

Re: [net-next,v15,08/13] net: mdio: realtek-rtl9300: Add page tracking

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 3

- [Low] The commit fixes a pre-existing, user-visible malfunction (before
  this patch all C22 commands used RAW_PAGE, i.e. the page write and the…
- [Low] The new C22 attachment gate treats the single encoded vendor
  value 0x001cc800 as covering every Realtek PHY:…
- [Low] The new register-31 interception and its page-range validation
  are keyed on an exact equality test (`regnum == RTL_PAGE_SELECT`)…

--

net: mdio: realtek-rtl9300: Add page tracking

This intercepts Clause 22 accesses to PHY register 31 in the rtl9300 MDIO
driver, caches the selected page per port in priv->page[], and folds that
page into the MAIN_PAGE field of every hardware C22 command. It also resets
the cached page on attach/detach and rejects non-Realtek PHYs on a C22 bus.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index a925640290cb0..d6027062e452f 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c

Before the diff, one question about the changelog:

>     This makes access to PHYs a gamble.

[Severity: Low]
Should this carry a Fixes: tag?

Prior to this patch all four C22 command builders used
FIELD_PREP(..._MAIN_PAGE, RAW_PAGE(priv)), so a page write and the
following register access were separate unpaged transactions that the
hardware poller can interleave. That is the user-visible malfunction the
changelog describes, and it is present in released code.

The core of the fix, the register 31 interception in otto_emdio_read_c22()
and otto_emdio_write_c22() plus passing priv->page[port] into the command,
does not depend on the notify_phy_attach()/notify_phy_detach()
infrastructure added earlier in this series; only the page reset and the
vendor gate do. Another patch in the same series ("net: mdio:
realtek-rtl9300: suppress sysfs bind/unbind attributes") does carry
Fixes: 24e31e4747698, so could the changelog state whether a stable
backport is intended here, or why it is not possible?

> @@ -179,6 +179,9 @@
>  #define PHY_CTRL_MMD_DEVAD			GENMASK(20, 16)
>  #define PHY_CTRL_MMD_REG			GENMASK(15, 0)
>  
> +#define RTL_VENDOR_ID				0x001cc800
> +#define RTL_PAGE_SELECT				31
> +
>  #define MAP_ADDRS_PER_REG			6
>  #define MAP_BITS_PER_ADDR			5
>  #define MAP_BITS_PER_BUS			2
> @@ -204,6 +207,7 @@ struct otto_emdio_priv {
>  	struct regmap *regmap;
>  	struct mutex lock; /* protect HW access */
>  	DECLARE_BITMAP(valid_ports, MAX_PORTS);
> +	u16 page[MAX_PORTS];
>  	u8 smi_bus[MAX_PORTS];
>  	u8 smi_addr[MAX_PORTS];
>  	bool smi_bus_is_c45[MAX_SMI_BUSSES];

[ ... ]

> @@ -369,7 +373,7 @@ static int otto_emdio_9300_write_c22(struct mii_bus *bus, int port, int regnum,
>  	struct otto_emdio_cmd_regs cmd_data = {
>  		.c22_data	= FIELD_PREP(RTL9300_PHY_CTRL_REG_ADDR, regnum) |
>  				  FIELD_PREP(RTL9300_PHY_CTRL_PARK_PAGE, 0x1f) |
> -				  FIELD_PREP(RTL9300_PHY_CTRL_MAIN_PAGE, RAW_PAGE(priv)),
> +				  FIELD_PREP(RTL9300_PHY_CTRL_MAIN_PAGE, priv->page[port]),
>  		.io_data	= FIELD_PREP(RTL9300_PHY_CTRL_INDATA, value),
>  		.port_mask_low	= BIT(port),
>  	};

[ ... ]

> @@ -473,8 +477,12 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
>  	if (port < 0)
>  		return port;
>  
> -	scoped_guard(mutex, &priv->lock)
> +	scoped_guard(mutex, &priv->lock) {
> +		if (regnum == RTL_PAGE_SELECT)
> +			return priv->page[port];
> +
>  		ret = priv->info->read_c22(bus, port, regnum, &value);
> +	}
>  
>  	return ret ? ret : value;
>  }
> @@ -495,8 +503,17 @@ static int otto_emdio_write_c22(struct mii_bus *bus, int phy_id, int regnum,
>  	if (port < 0)
>  		return port;
>  
> -	scoped_guard(mutex, &priv->lock)
> +	scoped_guard(mutex, &priv->lock) {
> +		if (regnum == RTL_PAGE_SELECT) {
> +			if (value >= RAW_PAGE(priv))
> +				return -EINVAL;
> +
> +			priv->page[port] = value;
> +			return 0;
> +		}
> +
>  		ret = priv->info->write_c22(bus, port, regnum, value);
> +	}
>  
>  	return ret;
>  }

[Severity: Low]
Can a regnum whose low five bits are 31 slip past this interception and
reach the real hardware register 31?

The tests in otto_emdio_read_c22() and otto_emdio_write_c22() are exact
equality checks, and neither function bounds-checks regnum against the
5-bit C22 register space. The command builders then truncate it, for
example in otto_emdio_9300_write_c22():

	.c22_data = FIELD_PREP(RTL9300_PHY_CTRL_REG_ADDR /* GENMASK(24,20) */, regnum) | ...

__FIELD_PREP() in include/linux/bitfield.h masks the shifted value at
runtime; the BUILD_BUG_ON range check only fires for compile-time
constants. So regnum 63, 95, ... 0xffff become an access to register 31
while skipping both the interception and the value >= RAW_PAGE(priv)
guard.

Register numbers arrive unfiltered from userspace:

  phy_mii_ioctl()            /* SIOCSMIIREG, raw __u16 mii_data->reg_num */
    mdiobus_write()
      __mdiobus_write()      /* only checks addr >= PHY_MAX_ADDR */
        otto_emdio_write_c22()
          otto_emdio_9300_write_c22()

With regnum = 63 the -EINVAL guard that deliberately forbids selecting the
raw page (4095/8191) no longer applies, and the PHY can be parked on the
raw page that the changelog describes as making access "a gamble". The same
equality-only pattern applies to the MII_MMD_CTRL/MII_MMD_DATA rejection
added by "net: mdio: realtek-rtl9300: deny C45 over C22 access", where
regnum 45/46 alias to 13/14. Would rejecting regnum > 0x1f before
dispatching be preferable?

> @@ -607,8 +624,9 @@ static int otto_emdio_9310_setup_controller(struct otto_emdio_priv *priv)
>  
>  static int otto_emdio_notify_phy_attach(struct phy_device *phydev)
>  {
> -	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(phydev->mdio.bus);
>  	int port = otto_emdio_phy_to_port(phydev->mdio.bus, phydev->mdio.addr);
> +	struct otto_emdio_chan *chan = phydev->mdio.bus->priv;
> +	struct otto_emdio_priv *priv = chan->priv;
>  
>  	if (port < 0) {
>  		/* All subsequent bus operations will fail */
> @@ -616,6 +634,16 @@ static int otto_emdio_notify_phy_attach(struct phy_device *phydev)
>  		return port;
>  	}
>  
> +	/* "sync" page in case of previously failed attachment */
> +	scoped_guard(mutex, &priv->lock)
> +		priv->page[port] = 0;
> +
> +	if (!priv->smi_bus_is_c45[chan->mdio_bus] &&
> +	    !phy_id_compare_vendor(phydev->phy_id, RTL_VENDOR_ID)) {
> +		phydev_err(phydev, "Only Realtek PHYs allowed on C22 bus\n");
> +		return -EOPNOTSUPP;
> +	}
> +
>  	return otto_emdio_set_port_polling(priv, port, true);
>  }

[Severity: Low]
Does this gate reject some Realtek PHYs?

phy_id_compare_vendor() in include/linux/phy.h compares bits 31:10:

	#define PHY_ID_MATCH_VENDOR_MASK GENMASK(31, 10)
	...
	static inline bool phy_id_compare_vendor(u32 id, u32 vendor_mask)
	{
		return phy_id_compare(id, vendor_mask, PHY_ID_MATCH_VENDOR_MASK);
	}

Realtek parts using the legacy, non-OUI ID encoding do not match
0x001cc800. realtek_drvs[] in drivers/net/phy/realtek/realtek_main.c has:

		PHY_ID_MATCH_EXACT(0x00008201),
		.name		= "RTL8201CP Ethernet",
		.read_page	= rtl821x_read_page,
		.write_page	= rtl821x_write_page,

0x00008201 masked with GENMASK(31, 10) is 0x00008000, so the comparison
fails and attachment aborts with -EOPNOTSUPP, leaving the port unusable,
even though that PHY uses register 31 as page select and therefore fits the
emulation added here.

No in-tree DT currently puts such a PHY on one of these SMI buses, so this
looks unlikely to be hit in practice, but should the check also accept the
legacy Realtek ID range so it matches the intent in its own error message?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831143439.2404484-1-markus.stockhausen%40gmx.de

Re: [net-next,v15,10/13] net: mdio: realtek-rtl9300: Open up C22 and C45 space in parallel

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 4 · Low: 0

- [Medium] otto_emdio_probe_one() now installs both accessor pairs
  (bus->read/write and bus->read_c45/write_c45) on every SMI bus, but…
- [Medium] priv->smi_bus_is_c45[] is set per physical SMI bus whenever
  any DT child of that bus declares compatible = "ethernet-phy-…
- [Medium] Installing bus->read_c45/bus->write_c45 on every SMI bus
  (including the C22-polling ones) makes PHY-driver code paths live that…
- [Medium] The driver's C22 path is Realtek-specific:
  otto_emdio_9300_read_c22()/otto_emdio_9300_write_c22() always encode…

--

net: mdio: realtek-rtl9300: Open up C22 and C45 space in parallel

The patch stops selecting the mii_bus accessors based on the per-bus
smi_bus_is_c45[] flag and instead registers both the C22 and the C45
read/write callbacks on every SMI bus, matching what the downstream
driver does.

Link: https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/phy/realtek/realtek_multiport.c#L349
Link: https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/mdio/mdio-realtek-otto.c#L1072

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index ae7ba86e90b9a..b90972c1dc6a2 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -688,13 +688,10 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
>  		return -ENOMEM;
>  
>  	bus->name = "Realtek Switch MDIO Bus";
> -	if (priv->smi_bus_is_c45[mdio_bus]) {
> -		bus->read_c45 = otto_emdio_read_c45;
> -		bus->write_c45 = otto_emdio_write_c45;
> -	} else {
> -		bus->read = otto_emdio_read_c22;
> -		bus->write = otto_emdio_write_c22;
> -	}
> +	bus->read_c45 = otto_emdio_read_c45;
> +	bus->write_c45 = otto_emdio_write_c45;
> +	bus->read = otto_emdio_read_c22;
> +	bus->write = otto_emdio_write_c22;

[Severity: Medium]
The commit message says that "when polling is set to C22 most of the C45
space can not be accessed" and that only "EEE register access is allowed
independently from the polling mode".  Should the opening up be limited to
that documented exception rather than to the whole opposite address space?

The hardware frame format and the PPU scan mode are still derived per bus
from priv->smi_bus_is_c45[] in otto_emdio_9300_setup_controller():

	if (priv->smi_bus_is_c45[i])
		glb_ctrl_val |= RTL9300_GLB_CTRL_INTF_SEL(i);

and in otto_emdio_9310_setup_controller():

	err = regmap_assign_bits(priv->regmap, RTL9310_SMI_GLB_CTRL1,
				 RTL9310_SMI_GLB_FMT_SEL_C45(i),
				 priv->smi_bus_is_c45[i]);

so the restriction described in the changelog is unchanged.  Before this
patch an access into the non-configured space was refused by mdio_bus.c
because the callback was NULL:

drivers/net/phy/mdio_bus.c:__mdiobus_c45_read() {
	if (bus->read_c45)
		retval = bus->read_c45(bus, addr, devad, regnum);
	else
		retval = -EOPNOTSUPP;
	...
}

Now the command is issued to the controller instead.  otto_emdio_run_cmd()
only reports the controller FAIL bit:

	return cmdstate & info->cmd_fail ? -ENXIO : 0;

Can a command that the controller accepts but cannot perform correctly in
the current mode return success here, so that otto_emdio_read_c45() hands
the data register contents to phylib as a valid PHY register value?  And can
a wrongly executed write leave the PHY in a state the driver has no record
of?

The port is also not excluded from the polling engine around a transfer -
otto_emdio_set_port_polling() is only toggled from the attach/detach
notifiers, and priv->lock only serializes the CPU side command writes.

Note the driver keeps the opposite policy elsewhere.  The C22 handlers still
refuse MMD access with:

	if (regnum == MII_MMD_CTRL || regnum == MII_MMD_DATA) {
		dev_warn_once(&bus->dev,
			      "C45 over C22 read access broken due to polling\n");
		return -EOPNOTSUPP;
	}

and the file header comment states:

 * - Access to MDIO_AN_EEE_ADV and MDIO_AN_EEE_LPABLE works via C45 over C22.
 * - It is unclear if these sequences change for different PHYs.

Should the header comment and the MMD guard be updated together with this
change, or should the new access be narrowed to the EEE registers?

[Severity: Medium]
Does this change PHY detection and binding on device trees that mix C22 and
C45 PHYs on one SMI bus?

otto_emdio_map_ports() sets the flag for the whole bus as soon as a single
child declares Clause 45:

	if (of_device_is_compatible(phy_dn, "ethernet-phy-ieee802.3-c45"))
		priv->smi_bus_is_c45[bus] = true;

and setup_controller() then programs the frame format for the entire
interface from that flag.  Previously a PHY of the opposite protocol on such
a bus was unreachable, so get_phy_c22_id()/get_phy_c45_ids() returned
-EOPNOTSUPP and of_mdiobus_register() failed with a clear error.  With both
callback pairs installed the ID read is now issued on an interface programmed
for the other framing, and since otto_emdio_run_cmd() only checks the FAIL
bit, whatever is in the data register becomes the PHY ID used for driver
matching.

The C45 fallback in get_phy_device() also becomes reachable on C22 polling
buses:

drivers/net/phy/phy_device.c:get_phy_device() {
	if (!is_c45 && phy_id == 0 && bus->read_c45) {
		r = get_phy_c45_ids(bus, addr, &c45_ids);
		if (!r)
			return phy_device_create(bus, addr, phy_id,
						 true, &c45_ids);
	}
	...
}

Can this create a bogus C45 phydev where registration previously returned
-ENODEV?

Once such a mismatched PHY is attached, otto_emdio_notify_phy_attach()
enables hardware polling for the port without comparing the individual PHY
protocol against the bus polling mode:

	if (!priv->smi_bus_is_c45[chan->mdio_bus] &&
	    !phy_id_compare_vendor(phydev->phy_id, RTL_VENDOR_ID)) {
		phydev_err(phydev, "Only Realtek PHYs allowed on C22 bus\n");
		return -EOPNOTSUPP;
	}

	return otto_emdio_set_port_polling(priv, port, true);

Would the PPU then send wrong-format polling transactions for that port,
leaving MAC_LINK_STS unpopulated as described in the file header comment?

[Severity: Medium]
Can enabling read_c45/write_c45 on C22 polling buses route MMD accesses to
the wrong PHY on the same bus?

The Realtek PHY driver reaches the bus C45 accessors through helpers that
pass the literal address 0 rather than phydev->mdio.addr:

drivers/net/phy/realtek/realtek_main.c:
static int rtlgen_read_vend2(struct phy_device *phydev, int regnum)
{
	return __mdiobus_c45_read(phydev->mdio.bus, 0, MDIO_MMD_VEND2, regnum);
}

static int rtlgen_write_vend2(struct phy_device *phydev, int regnum, u16 val)
{
	return __mdiobus_c45_write(phydev->mdio.bus, 0, MDIO_MMD_VEND2, regnum,
				   val);
}

Those helpers were written for the r8169 internal PHY bus (commit
da681ed73fb98 "net: phy: realtek: improve mmd register access for internal
PHY's").  rtlgen_read_mmd()/rtlgen_write_mmd() use them precisely for the
EEE registers this patch wants to open up:

	if (devnum == MDIO_MMD_VEND2)
		ret = rtlgen_read_vend2(phydev, regnum);
	else if (devnum == MDIO_MMD_PCS && regnum == MDIO_PCS_EEE_ABLE)
		ret = rtlgen_read_vend2(phydev, RTL_MDIO_PCS_EEE_ABLE);
	else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_ADV)
		ret = rtlgen_read_vend2(phydev, RTL_MDIO_AN_EEE_ADV);

and they are installed as .read_mmd/.write_mmd on the "Generic FE-GE Realtek
PHY" entry matched by rtlgen_match_phy_device() for RTL_GENERIC_PHYID, a
Realtek vendor ID this MDIO driver explicitly admits on C22 buses.

Before this patch __mdiobus_c45_read() returned -EOPNOTSUPP on such a bus.
Now the literal 0 is resolved against the shared switch bus:

	port = otto_emdio_phy_to_port(bus, phy_id);
	if (port < 0)
		return port;

So phy_ethtool_set_eee() -> genphy_c45_write_eee_adv() -> phy_write_mmd()
for port N would rewrite the EEE advertisement of whichever port is mapped
to SMI address 0, and genphy_c45_read_eee_abilities() would populate port
N's supported_eee from that port.  If no port on the bus uses address 0,
otto_emdio_phy_to_port() returns -ENOENT instead of -EOPNOTSUPP, which
callers that treat -EOPNOTSUPP as "feature absent" do not expect:

drivers/net/phy/realtek/realtek_main.c:rtl822x_read_mmd() {
	...
	if (ret != -EOPNOTSUPP)
		return ret;
	...
}

The hardcoded address 0 predates this series, but is this patch what makes
it reachable on C22 buses?

[Severity: Medium]
Registering the C22 accessors on C45 buses also makes them reachable for
non-Realtek PHYs.  Should the vendor gating follow the PHY rather than the
bus?

The C22 path is Realtek specific.  otto_emdio_9300_read_c22() and
otto_emdio_9300_write_c22() always encode the page fields:

	.c22_data	= FIELD_PREP(RTL9300_PHY_CTRL_REG_ADDR, regnum) |
			  FIELD_PREP(RTL9300_PHY_CTRL_PARK_PAGE, 0x1f) |
			  FIELD_PREP(RTL9300_PHY_CTRL_MAIN_PAGE, priv->page[port]),

so per the file header comment the controller emits a page select write to
PHY register 31 before every C22 transaction, while
otto_emdio_read_c22()/otto_emdio_write_c22() emulate register 31 in
software.

The only guard against doing that to a PHY without a Realtek page register
is bus scoped, in otto_emdio_notify_phy_attach():

	if (!priv->smi_bus_is_c45[chan->mdio_bus] &&
	    !phy_id_compare_vendor(phydev->phy_id, RTL_VENDOR_ID)) {
		phydev_err(phydev, "Only Realtek PHYs allowed on C22 bus\n");
		return -EOPNOTSUPP;
	}

On a C45 bus that check is skipped, and a non-Realtek 10G PHY is legitimately
allowed there.  Previously bus->read/bus->write were NULL on such a bus, so
no C22 access could reach that PHY.  Now a PHY driver's phy_read()/
phy_write(), or userspace via ioctl(SIOCGMIIREG)/ioctl(SIOCSMIIREG), takes
the non-C45 branch:

drivers/net/phy/phy.c:phy_mii_ioctl() {
	case SIOCGMIIREG:
		if (mdio_phy_id_is_c45(mii_data->phy_id)) {
			...
			ret = mdiobus_c45_read(phydev->mdio.bus, prtad, devad,
	...
}

Can this write the vendor specific register 31 of a foreign PHY, and can a
read of register 31 return driver bookkeeping instead of the device's actual
register content, leaving software and hardware state diverged?

Extending the vendor check to the C22 accessors on every bus, or omitting the
page fields for non-Realtek devices, would keep the gating with the PHY.

>  	bus->parent = dev;
>  	bus->notify_phy_attach = otto_emdio_notify_phy_attach;
>  	bus->notify_phy_detach = otto_emdio_notify_phy_detach;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831143439.2404484-1-markus.stockhausen%40gmx.de

Re: [net-next,v15,11/13] net: mdio: realtek-rtl9300: Add support for RTL838x

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 3

- [Medium] On the newly added RTL838x variant, a device tree that
  declares a Clause 45 PHY (compatible = "ethernet-phy-ieee802.3-c45")…
- [Low] DISCARDED (fixed later in the same series): Kconfig prompt/help
  text and MODULE_DESCRIPTION still advertise only the RTL9300 family…
- [Low] The driver now matches "realtek,rtl8380-mdio", but no in-tree
  binding documents a parent node that satisfies what the driver…
- [Low] The RTL838x variant introduced here declares RTL8380_NUM_BUSES =
  1, but Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml…

--

net: mdio: realtek-rtl9300: Add support for RTL838x

This patch adds the RTL838x (maple) variant to the Realtek MDIO driver by
duplicating the RTL9300-style register defines with an RTL8380 prefix,
adding C22/C45 accessors, a setup_controller() callback that sets the PHY
patch-done bit, and the new "realtek,rtl8380-mdio" match entry.

A few questions below on the new variant description and the device tree
side.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index b90972c1dc6a2..57ab89afb9e5e 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -126,6 +126,28 @@
>  #include <linux/property.h>
>  #include <linux/regmap.h>
>  
> +#define RTL8380_NUM_BUSES			1
      ^^^^

[Severity: Low]
Should the binding also express the one-bus limit for this compatible?
Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml applies the
same subnode pattern to every compatible and has no per-compatible if/then
block:

  patternProperties:
    '^mdio-bus@[0-3]$':
      $ref: mdio.yaml#

So a device tree for realtek,rtl8380-mdio with mdio-bus@1 through
mdio-bus@3 validates cleanly, but the driver then refuses it in
otto_emdio_probe_one():

  if (mdio_bus >= priv->info->num_buses)
          return dev_err_probe(dev, -EINVAL,
                               "illegal (dangling) smi bus number %d\n", mdio_bus);

and otto_emdio_map_ports() likewise returns -EINVAL for
"illegal smi bus number". Since otto_emdio_probe() propagates that error,
the whole controller fails to probe and even the valid bus 0 is never
registered. The later 2-bus RTL839x compatible in this series has the same
gap.

> +#define RTL8380_NUM_PAGES			4096
> +#define RTL8380_NUM_PORTS			28
> +#define RTL8380_SMI_GLB_CTRL			0xa100
> +#define   RTL8380_SMI_PHY_PATCH_DONE		BIT(15)
> +#define RTL8380_SMI_ACCESS_PHY_CTRL_0		0xa1b8
> +#define RTL8380_SMI_ACCESS_PHY_CTRL_1		0xa1bc
> +#define   RTL8380_PHY_CTRL_REG_ADDR		GENMASK(24, 20)
> +#define   RTL8380_PHY_CTRL_PARK_PAGE		GENMASK(19, 15)
> +#define   RTL8380_PHY_CTRL_MAIN_PAGE		GENMASK(14, 3)
> +#define   RTL8380_PHY_CTRL_WRITE		BIT(2)
> +#define   RTL8380_PHY_CTRL_READ			0
> +#define   RTL8380_PHY_CTRL_TYPE_C45		BIT(1)
> +#define   RTL8380_PHY_CTRL_TYPE_C22		0
> +#define   RTL8380_PHY_CTRL_FAIL			0 /* no fail indicator */
> +#define RTL8380_SMI_ACCESS_PHY_CTRL_2		0xa1c0
> +#define   RTL8380_PHY_CTRL_INDATA		GENMASK(31, 16)
> +#define   RTL8380_PHY_CTRL_DATA			GENMASK(15, 0)
> +#define RTL8380_SMI_ACCESS_PHY_CTRL_3		0xa1c4
> +#define RTL8380_SMI_POLL_CTRL			0xa17c
> +#define RTL8380_SMI_PORT0_5_ADDR_CTRL		0xa1c8
> +
>  #define RTL9300_NUM_BUSES			4
>  #define RTL9300_NUM_PAGES			4096
>  #define RTL9300_NUM_PORTS			28

[ ... ]

> @@ -586,6 +662,15 @@ static int otto_emdio_setup_topology(struct otto_emdio_priv *priv)
>  	return 0;
>  }
>  
> +static int otto_emdio_8380_setup_controller(struct otto_emdio_priv *priv)
> +{
> +	/*
> +	 * PHY_PATCH_DONE enables PHY control via SoC. This is required for PHY access, including
> +	 * patching and must be set before the PHYs are probed.
> +	 */
> +	return regmap_set_bits(priv->regmap, RTL8380_SMI_GLB_CTRL, RTL8380_SMI_PHY_PATCH_DONE);
> +}

[Severity: Medium]
Can priv->smi_bus_is_c45[] end up set on RTL838x with nothing in the
hardware matching it?

otto_emdio_map_ports() sets the flag for any variant:

  if (of_device_is_compatible(phy_dn, "ethernet-phy-ieee802.3-c45"))
          priv->smi_bus_is_c45[bus] = true;

For RTL9300/RTL9310 that flag is consumed by their setup_controller()
callbacks, which program RTL9300_GLB_CTRL_INTF_SEL and
RTL9310_SMI_GLB_FMT_SEL_C45. The new RTL838x callback above never reads it,
which matches the commit message: "The setup_controller() function needs no
C45 configuration as polling only works in C22 mode".

The flag still has a software consumer in otto_emdio_notify_phy_attach():

  if (!priv->smi_bus_is_c45[chan->mdio_bus] &&
      !phy_id_compare_vendor(phydev->phy_id, RTL_VENDOR_ID)) {
          phydev_err(phydev, "Only Realtek PHYs allowed on C22 bus\n");
          return -EOPNOTSUPP;
  }

  return otto_emdio_set_port_polling(priv, port, true);

With RTL8380_NUM_BUSES == 1, a single PHY node declaring
compatible = "ethernet-phy-ieee802.3-c45" disables that check for every PHY
on the device, and polling is then armed anyway for the port. The PHY keeps
being accessed with the Realtek paged C22 encoding (priv->page[port] into
RTL8380_PHY_CTRL_MAIN_PAGE) by a polling engine that only does C22.

Because RTL8380_PHY_CTRL_FAIL is 0, otto_emdio_run_cmd() can never report a
hardware failure here:

  return cmdstate & info->cmd_fail ? -ENXIO : 0;

Would it make sense for the RTL838x path to reject (or at least warn about)
a bus flagged as C45, rather than letting the driver bookkeeping and the
hardware mode diverge silently?

[ ... ]

> @@ -923,6 +1030,7 @@ static const struct otto_emdio_info otto_emdio_9310_info = {
>  };
>  
>  static const struct of_device_id otto_emdio_ids[] = {
> +	{ .compatible = "realtek,rtl8380-mdio", .data = &otto_emdio_8380_info },

[Severity: Low]
Is there a binding for the parent node this compatible needs?

otto_emdio_probe() takes its regmap from the parent:

  priv->regmap = syscon_node_to_regmap(dev->parent->of_node);

and otto_emdio_map_ports() requires an "ethernet-ports" child of that same
parent, otherwise probe fails with -EINVAL "missing ethernet-ports":

  ports_dn = of_get_child_by_name(parent->of_node, "ethernet-ports");

The only schema describing such a container (syscon + simple-mfd with
ethernet-ports and an mdio-controller child) is
Documentation/devicetree/bindings/net/realtek,rtl9301-switch.yaml, and its
compatible enum lists only realtek,rtl9301/9302b/9302c/9303-switch, also at
the end of this series. A grep of Documentation/devicetree/bindings finds no
rtl838x/rtl8380 switch compatible.

That leaves a DT author either inventing an undocumented parent compatible
(caught by dtbs_check) or claiming realtek,rtl9301-switch on RTL838x
hardware, whose register window this patch treats as different (0xa1xx
versus 0xcaxx). Should a parent binding for the RTL838x switch/syscon node
be added alongside this?

[Severity: Low]
This isn't a bug, but with RTL838x now matched, the Kconfig prompt and help
text in drivers/net/mdio/Kconfig still say:

  tristate "Realtek RTL9300 MDIO interface support"
  ...
    This driver supports the MDIO interface found in the Realtek
    RTL9300 family of Ethernet switches with integrated SoC.

and MODULE_DESCRIPTION() in this file still says "RTL9300 MDIO driver". The
later patch in this series, "net: mdio: realtek-rtl9300: reword Kconfig and
module description", changes both to RTL83xx/RTL93xx, so this is only a
transient state within the series. Would folding the wording change in
earlier keep each commit self-consistent?

>  	{ .compatible = "realtek,rtl9301-mdio", .data = &otto_emdio_9300_info },
>  	{ .compatible = "realtek,rtl9311-mdio", .data = &otto_emdio_9310_info },
>  	{}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831143439.2404484-1-markus.stockhausen%40gmx.de

Re: [net-next,v15,12/13] net: mdio: realtek-rtl9300: Add support for RTL839x

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] The binding schema
  Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml…

--

net: mdio: realtek-rtl9300: Add support for RTL839x

This adds the register definitions, C22/C45 accessors and the
otto_emdio_info instance for the RTL839x (cypress) family, plus the
realtek,rtl8391-mdio compatible to otto_emdio_ids[]. The new device
declares 2 SMI buses, 8192 pages and 52 ports.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 57ab89afb9e5e..86b2040fa3335 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -148,6 +148,28 @@
>  #define RTL8380_SMI_POLL_CTRL			0xa17c
>  #define RTL8380_SMI_PORT0_5_ADDR_CTRL		0xa1c8
>  
> +#define RTL8390_NUM_BUSES			2

[ ... ]

> @@ -980,6 +1058,29 @@ static const struct otto_emdio_info otto_emdio_8380_info = {
>  	.write_c45 = otto_emdio_8380_write_c45,
>  };
>  
> +static const struct otto_emdio_info otto_emdio_8390_info = {
> +	.cmd_fail = RTL8390_PHY_CTRL_FAIL,
> +	.cmd_read = RTL8390_PHY_CTRL_READ,
> +	.cmd_write = RTL8390_PHY_CTRL_WRITE,
> +	.cmd_regs = {
> +		.broadcast = RTL8390_BCAST_PHYID_CTRL,
> +		.c22_data = RTL8390_PHYREG_ACCESS_CTRL,
> +		.c45_data = RTL8390_PHYREG_MMD_CTRL,
> +		.ext_page = RTL8390_PHYREG_CTRL,
> +		.io_data = RTL8390_PHYREG_DATA_CTRL,
> +		.port_mask_low = RTL8390_PHYREG_PORT_CTRL_LOW,
> +		.port_mask_high = RTL8390_PHYREG_PORT_CTRL_HIGH,
> +	},
> +	.num_buses = RTL8390_NUM_BUSES,

[Severity: Low]
Should the binding schema be narrowed for this compatible too?

Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml has a
single unconditional child-node pattern that applies to every compatible
in the file:

    patternProperties:
      '^mdio-bus@[0-3]$':
        $ref: mdio.yaml#

There is no allOf/if-then block that restricts the index per compatible,
so a realtek,rtl8391-mdio node containing mdio-bus@2 or mdio-bus@3 passes
dt_binding_check, while the driver data added here says the part only has
two buses.

The driver then rejects such a tree in otto_emdio_probe_one():

	if (mdio_bus >= priv->info->num_buses)
		return dev_err_probe(dev, -EINVAL,
				     "illegal (dangling) smi bus number %d\n", mdio_bus);

and again in otto_emdio_map_ports() when a phy-handle resolves to a bus
node with reg = <2>:

	if (bus >= priv->info->num_buses) {
		err = dev_err_probe(dev, -EINVAL, "illegal smi bus number %d\n", bus);

Since the whole probe fails in that case, no MDIO bus is registered at all
for the switch. Would it make sense to add if-then blocks keying on
realtek,rtl8391-mdio (2 buses) and realtek,rtl8380-mdio (1 bus) so the
schema enforces the same limit the driver does?

> +	.num_pages = RTL8390_NUM_PAGES,
> +	.num_ports = RTL8390_NUM_PORTS,
> +	.poll_ctrl = RTL8390_SMI_PORT_POLLING_CTRL,
> +	.read_c22 = otto_emdio_8390_read_c22,
> +	.read_c45 = otto_emdio_8390_read_c45,
> +	.write_c22 = otto_emdio_8390_write_c22,
> +	.write_c45 = otto_emdio_8390_write_c45,
> +};
> +
>  static const struct otto_emdio_info otto_emdio_9300_info = {

[ ... ]

> @@ -1031,6 +1132,7 @@ static const struct otto_emdio_info otto_emdio_9310_info = {
>  
>  static const struct of_device_id otto_emdio_ids[] = {
>  	{ .compatible = "realtek,rtl8380-mdio", .data = &otto_emdio_8380_info },
> +	{ .compatible = "realtek,rtl8391-mdio", .data = &otto_emdio_8390_info },
>  	{ .compatible = "realtek,rtl9301-mdio", .data = &otto_emdio_9300_info },
>  	{ .compatible = "realtek,rtl9311-mdio", .data = &otto_emdio_9310_info },
>  	{}

For reference, the lax [0-3] pattern predates this series, but until now it
only covered realtek,rtl9301-mdio and realtek,rtl9311-mdio, which do have
four buses (RTL9300_NUM_BUSES and RTL9310_NUM_BUSES are both 4), so the
schema matched the driver. The mismatch appears with the 2-bus RTL839x
compatible claimed here.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831143439.2404484-1-markus.stockhausen%40gmx.de