[PATCH net-next v8 3/9] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

[PATCH net-next v8 3/9] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

[Markus Stockhausen]

From: Daniel Golle <daniel@makrotopia.org>

Some MDIO buses require programiming 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 teared 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. For this

- Relocate code from phy_detach() into a new helper __phy_detach()
- The helper takes an additional parameter notify_bus that decides
  if the bus notification should be sent or not.
- Call the helper with notification from slimmed down version of
  phy_detach() and without notification from phy_attach_direct()
  error path.

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/

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

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 94b2e85e00a3..233622e9ba61 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1734,6 +1734,96 @@ static bool phy_drv_supports_irq(const struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
+static void __phy_detach(struct phy_device *phydev, bool notify_bus)
+{
+	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 (notify_bus && phydev->mdio.bus->notify_phy_detach)
+		phydev->mdio.bus->notify_phy_detach(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)
+{
+	/* cleanup including bus notification */
+	__phy_detach(phydev, true);
+}
+EXPORT_SYMBOL(phy_detach);
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -1876,6 +1966,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);
 
 	/**
@@ -1890,8 +1986,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);
+	/* cleanup without bus notification */
+	__phy_detach(phydev, false);
 	return err;
 
 error_module_put:
@@ -1906,86 +2002,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;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 11092c3175b3..71405283bc6e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -376,6 +376,20 @@ 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. Called in phy_attach_direct() right before phy_resume().
+	 *   Runs in process context and may sleep. Return 0 on success or a
+	 *   negative errno on failure (which aborts the attach sequence).
+	 */
+	int (*notify_phy_attach)(struct phy_device *phydev);
+	/**
+	 * @notify_phy_detach: Perform pre-detach handling for MDIO bus
+	 *   drivers. Called in phy_detach() right after phy_suspend().
+	 *   Runs in process context and may sleep. Only invoked if
+	 *   @notify_phy_attach was previously called and succeeded.
+	 */
+	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

Re: [PATCH net-next v8 3/9] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

[sashiko-bot]

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

Pre-existing issues:
- [High] Calling `phy_attach_direct` on an already attached PHY destructively tears down the existing legitimate attachment and causes module refcount imbalances.
--

--- Patch [3]: [PATCH net-next v8 3/9] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus ---
commit 93555b251005e09563b58618551521a23eeca40d
Author: Daniel Golle <daniel@makrotopia.org>

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

This commit introduces notify_phy_attach() and notify_phy_detach() hooks
to struct mii_bus for MDIO bus drivers. It also relocates code from
phy_detach() into a new __phy_detach() helper to ensure symmetrical bus
notification during PHY attach and detach phases.

Link: https://github.com/openwrt/openwrt/pull/21515#discussion_r2714069716
Link: https://lore.kernel.org/netdev/cover.1769053496.git.daniel@makrotopia.org/

> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 94b2e85e00a37..233622e9ba61f 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c

[ ... ]

> @@ -1890,8 +1986,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);
> +	/* cleanup without bus notification */
> +	__phy_detach(phydev, false);

[Severity: High]
This is a pre-existing issue, but since the error path is being modified here,
I wanted to ask about how we handle failures when a PHY is already attached.

If phy_attach_direct() is called on a PHY that is already attached to another
device, it returns -EBUSY and jumps to the error label:

	if (phydev->attached_dev) {
		dev_err(&dev->dev, "PHY already attached\n");
		err = -EBUSY;
		goto error;
	}

Because this error path unconditionally calls __phy_detach(), and 
__phy_detach() uses the current phydev->attached_dev as its target, 
does this perform a full teardown on the existing active connection?

static void __phy_detach(struct phy_device *phydev, bool notify_bus)
{
	struct net_device *dev = phydev->attached_dev;
...
		phydev->attached_dev->phydev = NULL;
		phydev->attached_dev = NULL;


Could this break the live attachment and cause module refcount imbalances, 
since the original MAC driver is completely unaware its PHY was detached and 
its dev->phydev pointer was set to NULL?

Should the error path cleanly roll back only the references taken for the 
new failed attachment instead of destroying the pre-existing state?

>  	return err;
>  
>  error_module_put:

[ ... ]

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

[PATCH net-next v8 0/9] 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>
---

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 (8):
  dt-bindings: net: realtek,rtl9301-mdio: Add RTL83xx series
  net: mdio: realtek-rtl9300: Add polling documentation
  net: mdio: realtek-rtl9300: extend controller structure lifetime
  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: Add support for RTL838x
  net: mdio: realtek-rtl9300: Add support for RTL839x

 .../bindings/net/realtek,rtl9301-mdio.yaml    |  14 +-
 drivers/net/mdio/mdio-realtek-rtl9300.c       | 445 +++++++++++++++++-
 drivers/net/phy/phy_device.c                  | 180 +++----
 include/linux/phy.h                           |  18 +
 4 files changed, 553 insertions(+), 104 deletions(-)

-- 
2.55.0

[PATCH net-next v8 1/9] 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 56 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 v8 2/9] 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>
---
 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 v8 3/9] 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. For this

- Relocate code from phy_detach() into a new helper __phy_detach()
- The helper takes an additional parameter notify_bus that decides
  if the bus notification should be sent or not.
- Call the helper with notification from slimmed down version of
  phy_detach() and without notification from phy_attach_direct()
  error path.

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/

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

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 94b2e85e00a3..233622e9ba61 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1734,6 +1734,96 @@ static bool phy_drv_supports_irq(const struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
+static void __phy_detach(struct phy_device *phydev, bool notify_bus)
+{
+	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 (notify_bus && phydev->mdio.bus->notify_phy_detach)
+		phydev->mdio.bus->notify_phy_detach(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)
+{
+	/* cleanup including bus notification */
+	__phy_detach(phydev, true);
+}
+EXPORT_SYMBOL(phy_detach);
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -1876,6 +1966,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);
 
 	/**
@@ -1890,8 +1986,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);
+	/* cleanup without bus notification */
+	__phy_detach(phydev, false);
 	return err;
 
 error_module_put:
@@ -1906,86 +2002,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;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 11092c3175b3..fef0c6a3e27f 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 v8 4/9] net: mdio: realtek-rtl9300: extend controller structure lifetime

[Markus Stockhausen]

Currently, the driver allocates the otto_emdio_priv structure using
devm_kzalloc(), tightly coupling its lifetime to the platform device.
Additionally the driver registers up to four MDIO buses. During unbind
or removal the devm teardown process might free the controller private
data before the MDIO buses are fully unregistered. This can lead to a
use-after-free if the MDIO subsystem accesses bus-specific operations
(which rely on controller->priv) during the teardown phase.

Fix this by decoupling the lifetime of the private data from the
platform device and directly tying it to the registered MDIO buses:

- Switch from devm() to normal functions in controller probing and
  introduce a kref counter.
- For each successfully initialized bus, increment the kref counter and
  register a devm action to decrement it on cleanup.
- Free the controller->priv data after the final reference has been
  removed.

For the new logic to work properly the controller from now on relies
on at least one attached bus. If no bus is given probing fails as
the controller has no real justification for existence.

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

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 73ac5fdcd267..3b99978a2511 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -116,6 +116,7 @@
 #include <linux/bitmap.h>
 #include <linux/bits.h>
 #include <linux/find.h>
+#include <linux/kref.h>
 #include <linux/mdio.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mutex.h>
@@ -198,6 +199,7 @@ struct otto_emdio_cmd_regs {
 
 struct otto_emdio_priv {
 	const struct otto_emdio_info *info;
+	struct kref kref;
 	struct regmap *regmap;
 	struct mutex lock; /* protect HW access */
 	DECLARE_BITMAP(valid_ports, MAX_PORTS);
@@ -582,6 +584,21 @@ static int otto_emdio_9310_setup_controller(struct otto_emdio_priv *priv)
 	return 0;
 }
 
+static void otto_emdio_cleanup_controller(struct kref *kref)
+{
+	struct otto_emdio_priv *priv = container_of(kref, struct otto_emdio_priv, kref);
+
+	mutex_destroy(&priv->lock);
+	kfree(priv);
+}
+
+static void otto_emdio_cleanup_bus(void *data)
+{
+	struct otto_emdio_priv *priv = data;
+
+	kref_put(&priv->kref, otto_emdio_cleanup_controller);
+}
+
 static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv,
 				 struct fwnode_handle *node)
 {
@@ -617,6 +634,11 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
 
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-%d", dev_name(dev), mdio_bus);
 
+	kref_get(&priv->kref);
+	err = devm_add_action_or_reset(&bus->dev, otto_emdio_cleanup_bus, priv);
+	if (err)
+		return dev_err_probe(dev, err, "cannot register cleanup action\n");
+
 	err = devm_of_mdiobus_register(dev, bus, to_of_node(node));
 	if (err)
 		return dev_err_probe(dev, err, "cannot register MDIO bus\n");
@@ -731,44 +753,54 @@ static int otto_emdio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct otto_emdio_priv *priv;
+	int buses_registered = 0;
 	int err;
 
-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	priv = kzalloc_obj(*priv, GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
-	err = devm_mutex_init(dev, &priv->lock);
-	if (err)
-		return err;
+	kref_init(&priv->kref);
+	mutex_init(&priv->lock);
 
 	priv->info = device_get_match_data(dev);
 	priv->regmap = syscon_node_to_regmap(dev->parent->of_node);
-	if (IS_ERR(priv->regmap))
-		return PTR_ERR(priv->regmap);
+	if (IS_ERR(priv->regmap)) {
+		err = PTR_ERR(priv->regmap);
+		goto err_out;
+	}
 
 	platform_set_drvdata(pdev, priv);
 
 	err = otto_emdio_map_ports(dev);
 	if (err)
-		return err;
+		goto err_out;
 
 	err = otto_emdio_setup_topology(priv);
 	if (err)
-		return err;
+		goto err_out;
 
 	if (priv->info->setup_controller) {
 		err = priv->info->setup_controller(priv);
-		if (err)
-			return dev_err_probe(dev, err, "failed to setup MDIO bus controller\n");
+		if (err) {
+			dev_err_probe(dev, err, "failed to setup MDIO bus controller\n");
+			goto err_out;
+		}
 	}
 
 	device_for_each_child_node_scoped(dev, child) {
 		err = otto_emdio_probe_one(dev, priv, child);
 		if (err)
-			return err;
+			goto err_out;
+		buses_registered++;
 	}
 
-	return 0;
+	if (!buses_registered)
+		err = dev_err_probe(dev, -ENODEV, "no MDIO buses registered\n");
+err_out:
+	kref_put(&priv->kref, otto_emdio_cleanup_controller);
+
+	return err;
 }
 
 static const struct otto_emdio_info otto_emdio_9300_info = {
-- 
2.55.0

[PATCH net-next v8 5/9] 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.

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.

[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 | 57 +++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index 3b99978a2511..aca0a9448bb0 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)
@@ -219,6 +221,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);
@@ -254,6 +257,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)
 {
@@ -584,6 +593,32 @@ 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)
+		return port;
+
+	return otto_emdio_set_port_polling(priv, port, true);
+}
+
+static void otto_emdio_notify_phy_detach(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 mii_bus *bus = phydev->mdio.bus;
+	int ret;
+
+	if (port < 0)
+		return;
+
+	ret = otto_emdio_set_port_polling(priv, port, false);
+	if (ret)
+		dev_err(bus->parent, "failed to disable polling for port %d\n", port);
+}
+
 static void otto_emdio_cleanup_controller(struct kref *kref)
 {
 	struct otto_emdio_priv *priv = container_of(kref, struct otto_emdio_priv, kref);
@@ -628,6 +663,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;
@@ -749,6 +787,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;
@@ -770,6 +821,10 @@ static int otto_emdio_probe(struct platform_device *pdev)
 		goto err_out;
 	}
 
+	err = otto_emdio_init_polling(priv);
+	if (err)
+		goto err_out;
+
 	platform_set_drvdata(pdev, priv);
 
 	err = otto_emdio_map_ports(dev);
@@ -818,6 +873,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,
@@ -843,6 +899,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 v8 6/9] 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 delay is accepted
for not-yet-known hardware designs with non-Realtek c22 PHYs.

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 c22 bus) this is no limitation.

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

diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
index aca0a9448bb0..dc3661b2cd94 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
@@ -205,6 +208,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];
@@ -356,7 +360,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),
 	};
 
@@ -370,7 +374,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),
 	};
@@ -410,7 +414,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,
@@ -422,7 +426,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)),
@@ -468,8 +472,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;
 }
@@ -483,8 +491,17 @@ static int otto_emdio_write_c22(struct mii_bus *bus, int phy_id, int regnum, u16
 	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;
 }
@@ -595,12 +612,19 @@ 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)
 		return port;
 
+	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);
 }
 
@@ -608,12 +632,17 @@ static void otto_emdio_notify_phy_detach(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 mii_bus *bus = phydev->mdio.bus;
 	int ret;
 
 	if (port < 0)
 		return;
 
+	/* avoid wrong cached page for next attachment */
+	if (!priv->smi_bus_is_c45[chan->mdio_bus])
+		priv->page[port] = 0;
+
 	ret = otto_emdio_set_port_polling(priv, port, false);
 	if (ret)
 		dev_err(bus->parent, "failed to disable polling for port %d\n", port);
-- 
2.55.0

[PATCH net-next v8 7/9] 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 dc3661b2cd94..543f16f84133 100644
--- a/drivers/net/mdio/mdio-realtek-rtl9300.c
+++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
@@ -275,9 +275,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;
 
@@ -317,7 +317,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 v8 8/9] 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 setup but must activate
  the PHY access.

While all existing RTL838x designs only use c22 bus access, this patch
implements the c45 code paths too. The hardware supports it and this
simplifies future expandability. Leave it to the devicetree if someone
wants to use c45 on these devices. This avoids adding extra checks to
the driver.

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 543f16f84133..26beba9625a8 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
@@ -354,6 +376,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);
@@ -574,6 +650,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;
@@ -887,6 +972,28 @@ static int otto_emdio_probe(struct platform_device *pdev)
 	return err;
 }
 
+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,
@@ -937,6 +1044,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 v8 9/9] 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.
- 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.

While all existing RTL839x designs only use c22 bus access, this patch
implements the c45 code paths too. The hardware supports it and this
simplifies future expandability. Leave it to the devicetree if someone
wants to use c45 on these devices. This avoids adding extra checks to
the driver.

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 26beba9625a8..d9d4b704afb3 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
@@ -430,6 +452,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);
@@ -994,6 +1072,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,
@@ -1045,6 +1146,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

Re: [PATCH net-next v8 3/9] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

[sashiko-bot]

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

Pre-existing issues:
- [High] Calling phy_attach_direct() on an already-attached PHY destructively tears down the existing valid attachment and leaves the bus driver state dangling.
--

--- Patch [3]: [PATCH net-next v8 3/9] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus ---
commit f45546de5e393510abd217da3bdb7bad6d759d93
Author: Daniel Golle <daniel@makrotopia.org>

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

This commit introduces notify_phy_attach and notify_phy_detach hooks to
the mii_bus structure. This allows MDIO bus drivers to handle PHY specific
configurations, such as polling registers, symmetrically during attach and
detach phases.

Link: https://github.com/openwrt/openwrt/pull/21515#discussion_r2714069716
Link: https://lore.kernel.org/netdev/cover.1769053496.git.daniel@makrotopia.org/

> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c

[ ... ]

> @@ -1890,8 +1986,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);
> +	/* cleanup without bus notification */
> +	__phy_detach(phydev, false);
>  	return err;

[Severity: High]
This is a pre-existing issue, but does this error path incorrectly tear down
a valid pre-existing attachment when phy_attach_direct() is called on an
already-attached PHY?

When phy_attach_direct() detects the PHY is already attached:

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;
        }
        ...
}

It jumps to the shared error path. Since the error path calls
__phy_detach(phydev, false) and phydev->attached_dev still points to the
old, valid network device, it destroys the existing attachment behind the
scenes.

Additionally, because the patch explicitly suppresses the bus detach
notification by passing false to __phy_detach(), doesn't this leave the
MDIO bus driver's state (like hardware polling configurations) permanently
out of sync?

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

Re: [PATCH net-next v8 4/9] net: mdio: realtek-rtl9300: extend controller structure lifetime

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Calling devm_add_action_or_reset() on the uninitialized &bus->dev causes a guaranteed NULL pointer dereference, and on error paths leaks resources.
--

--- Patch [4]: [PATCH net-next v8 4/9] net: mdio: realtek-rtl9300: extend controller structure lifetime ---
commit 34bdc9b9e9ece2a1428b242a3ed3e140500fd37f
Author: Markus Stockhausen <markus.stockhausen@gmx.de>

net: mdio: realtek-rtl9300: extend controller structure lifetime

This patch changes the allocation of the otto_emdio_priv structure from
devm_kzalloc() to kzalloc() and introduces a kref counter. It attempts to
decouple the controller's lifetime from the platform device by tying it
to the registered MDIO buses via devm_add_action_or_reset().

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 73ac5fdcd2674..3b99978a2511e 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
[ ... ]
> @@ -617,6 +634,11 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
>  
>  	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-%d", dev_name(dev), mdio_bus);
>  
> +	kref_get(&priv->kref);
> +	err = devm_add_action_or_reset(&bus->dev, otto_emdio_cleanup_bus, priv);

[Severity: Critical]
Will this cause a list corruption crash unconditionally on every probe?

The bus structure is allocated via devm_mdiobus_alloc_size(), which returns
zero-initialized memory, meaning bus->dev.devres_head is uninitialized.
Calling devm_add_action_or_reset() here will attempt to append to this
zero-initialized list.

Additionally, if the crash is bypassed, devm_of_mdiobus_register() later
calls device_initialize(&bus->dev). This resets devres_head to empty,
destroying the action and leaking the kref.

Finally, if mdiobus_register() is never reached on an error path,
mdiobus_free() handles the MDIOBUS_ALLOCATED state by directly calling
kfree(bus). This bypasses device_release() and devres_release_all(),
permanently leaking the devres structure and the priv kref.

> +	if (err)
> +		return dev_err_probe(dev, err, "cannot register cleanup action\n");
> +
>  	err = devm_of_mdiobus_register(dev, bus, to_of_node(node));

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

Re: [PATCH net-next v8 5/9] 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:
- [Medium] Hardware polling is not disabled during system suspend/resume, allowing it to race with and break PHY firmware loading during `phy_init_hw()` on resume.
--

--- Patch [5]: [PATCH net-next v8 5/9] net: mdio: realtek-rtl9300: Configure hardware polling during probing ---
commit c01be295c733b6b22769f54cba9a0edc13200b00
Author: Markus Stockhausen <markus.stockhausen@gmx.de>

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

This commit configures hardware polling on Realtek Otto switches to avoid
interfering with PHY configuration. It disables polling for all ports on
startup, enables it upon PHY attachment, and disables it before detachment.

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 3b99978a2511e..aca0a9448bb0c 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
[ ... ]
> @@ -584,6 +593,32 @@ 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)
> +		return port;
> +
> +	return otto_emdio_set_port_polling(priv, port, true);
> +}

[Severity: Medium]
Does this leave hardware polling active during system suspend and resume?

When the system resumes, the phy_device PM operations invoke
mdio_bus_phy_resume(), which calls phy_init_hw(). Since the platform
driver lacks PM hooks to pause and resume the polling unit, could the
active hardware polling race with and break PHY firmware loading during
this resume sequence?

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

Re: [PATCH net-next v8 6/9] net: mdio: realtek-rtl9300: Add page tracking

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Data race on `priv->page[port]` due to missing `priv->lock` in `otto_emdio_notify_phy_detach`.
--

--- Patch [6]: [PATCH net-next v8 6/9] net: mdio: realtek-rtl9300: Add page tracking ---
commit 05ca21acb226d57fd7de64847d2a32c8443b9dcd
Author: Markus Stockhausen <markus.stockhausen@gmx.de>
net: mdio: realtek-rtl9300: Add page tracking

This commit intercepts accesses to the Realtek page register (C22 reg 31)
to prevent interference with the hardware polling unit. It stores the target
page per-port and injects it into hardware commands. It also restricts C22
bus attachment to Realtek PHYs and resets the cached page upon detach.

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

[ ... ]

> @@ -608,12 +632,17 @@ static void otto_emdio_notify_phy_detach(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 mii_bus *bus = phydev->mdio.bus;
>  	int ret;
>  
>  	if (port < 0)
>  		return;
>  
> +	/* avoid wrong cached page for next attachment */
> +	if (!priv->smi_bus_is_c45[chan->mdio_bus])
> +		priv->page[port] = 0;

[Severity: Medium]
Does this write to priv->page[port] need to be protected by priv->lock?

Looking at otto_emdio_read_c22() and otto_emdio_write_c22() introduced
earlier in this patch, accesses to priv->page[port] are protected by taking
priv->lock. Here in otto_emdio_notify_phy_detach(), the assignment happens
without acquiring priv->lock.

Could this cause a data race if a concurrent MDIO read/write ioctl occurs
while the PHY is being detached?

> +
>  	ret = otto_emdio_set_port_polling(priv, port, false);
>  	if (ret)
>  		dev_err(bus->parent, "failed to disable polling for port %d\n", port);

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