[PATCH net-next v8 0/7] net: pse-pd: decouple controller lookup from MDIO probe

[PATCH net-next v8 0/7] net: pse-pd: decouple controller lookup from MDIO probe

[Carlo Szelinsky]

This is v8 of the series Corey started as an RFC [3]; I took it over
from v2 [1]. It takes the PSE controller lookup out of the MDIO probe
path, so a modular PSE controller driver no longer makes the PHY (and any
DSA switch behind it) spin on -EPROBE_DEFER until the PSE module loads.

v7 [10] drew an LLM review [11]. Four of its findings are fixed here,
one I disagree with (patch 5 below), and the rest I answered in that
thread. v6 [7] drew one too [8]; v7 folded the patches it was mostly
about into the phy patch, so no commit carries the rtnl detour or the
deferred release. Per Documentation/process/maintainer-netdev.rst I also
ran LLM review locally on v7 and v8 before posting.

Patch 2 reorders pse_controller_unregister() around the new event,
because a subscriber runs arbitrary teardown inside it. The controller
is unlinked from pse_controller_list before the event and before
anything is freed, so a lookup racing the teardown resolves nothing
rather than a controller whose pcdev->pi[] pse_release_pis() is about to
free. v6 disclosed that as pre-existing and reachable only from phy
registration; it is reachable from more than that here, because after
the last patch a second controller registering runs a PSE_REGISTERED
walk that calls of_pse_control_get() for every phy on mdio_bus_type that
has a DT node and no handle yet, which happens on any board with two PSE
controllers. disable_irq() moves up for the same reason: pse_isr()
queues notifications and reaches pcdev->pi. On tps23881, the only
in-tree user of devm_pse_irq_helper(), devres has already run free_irq()
by then, so the move is what makes the ordering hold for a driver that
requests its irq earlier rather than leaving it to devres.

[9] makes a related reordering for net, independently of any subscriber,
so pse_controller_unregister() conflicts if [9] is applied. The order is
not identical: [9] leaves the unlink below cancel_work_sync() and
pse_flush_pw_ds(), which it can, having no event to place. Here the
event has to sit after the unlink and before the frees, and
cancel_work_sync() after the event, so the unlink moves up to just after
disable_irq(). The merged function wants this order, which already
includes [9]'s reordering:

    if (pcdev->irq)
            disable_irq(pcdev->irq);
    mutex_lock(&pse_list_mutex);
    list_del(&pcdev->list);
    mutex_unlock(&pse_list_mutex);
    blocking_notifier_call_chain(&pse_controller_notifier,
                                 PSE_UNREGISTERED, pcdev);
    cancel_work_sync(&pcdev->ntf_work);
    WARN_ON(!list_empty(&pcdev->pse_control_head));
    pse_flush_pw_ds(pcdev);
    pse_release_pis(pcdev);
    kfifo_free(&pcdev->ntf_fifo);

I am happy to send that as a follow-up on top of the merge if that is
easier than carrying it in the conflict.

Kory, a specific ask on patch 2. cancel_work_sync() sits below the event,
not above it, because __pse_control_release() can re-enter your budget
code. regulator_disable() on a PI that is still on runs
_pse_pi_disable(). With the static strategy that retries a pending port
on the same power domain, and if the domain is still over budget it sheds
a lower priority one through pse_disable_pi_pol() - which queues a
notification and calls schedule_work() from inside the walk. Draining
before the walk would leave that work racing kfifo_free(). Draining after
it also keeps the worker's own transient reference, taken by
pse_control_find_by_id(), from becoming the last one once
pse_release_pis() has freed the array.

I staged that case in QEMU (below) and confirmed it with a dump_stack()
from pse_disable_pi_pol(), so it is a path I have hit rather than only
reasoned about. The ordering rests on your design though, so please take
a look at it.

Patch 5: each PSE PI regulator is registered with a "vpwr" supply. The
regulator core deliberately treats an unresolved supply at registration as
non-fatal, so pse_controller_register() completes and PSE_REGISTERED fires
for a controller whose PIs cannot be handed out yet: regulator_get_exclusive()
in pse_control_get_internal() resolves the supply itself and keeps
returning -EPROBE_DEFER until the vpwr provider appears. Before this series
the MDIO layer propagated that and deferred probe retried it. After it,
phylib has no event left to retry on, and the port would silently lose PSE
for good.

Patch 5 checks every PI's supply before registering any PI regulator, so
the PSE driver's own probe defers and deferred probe handles the
ordering. It checks exactly the PIs the registration loop creates a
regulator for, including a controller with no pse-pis node, and it
follows both stages the core uses - the PI node, then the controller
device - because a vpwr-supply written once on the controller node is
invisible from the PI node but resolves at stage two.

The review of v7 found two things wrong with that. The PI-node lookup has
to be taken only once the PI's own phandle resolves, or
of_get_child_regulator() walks the whole controller subtree and answers
one PI from a sibling's supply before the controller node is ever
consulted; that is fixed. The core reaches the same sibling walk, but
only after the controller's own vpwr-supply has had its turn.

It also asked for the loop to move ahead of setup_pi_matrix(), because
pd692x0 claims a power budget there that nothing gives back when
registration fails later. I tried that and it is wrong: pd692x0's pse-pi
nodes name the manager regulators setup_pi_matrix() itself registers, so
checking earlier defers on a supply only that driver can bring up and it
never probes. I added a controller of that shape to the test setup and
confirmed it both ways. The loop stays where it is and the changelog says
what the placement does not clean up.

-EPERM is now treated as resolved, since it only means the provider is
held exclusively. The check still stops short of the core's
device_is_bound() gate, so a probe interleaving with the provider's own
can slip through, and nothing retries that PI until the next
PSE_REGISTERED. fw_devlink only prevents that when the vpwr-supply sits
on the controller node: one in a pse-pi node gives the controller just a
SYNC_STATE_ONLY proxy link, which does not hold its probe back.

Patch 3 exists because patch 5 makes -EPROBE_DEFER an ordinary return
from pse_controller_register(). That function has no unwind past
kfifo_alloc(): the kfifo leaks on every failure, and past
of_load_pse_pis(), which cleans up after itself, so do the PI array and
its OF references - once today and on each retry after patch 5 - and a
partial pse_register_pw_ds() leaves devm-allocated power domains in the
global xarray for the next registration to trip over.

Patch 3 adds the unwind, at two depths: pse_pi_ops index pcdev->pi[], and
the PI regulators are devm-registered, so once one exists the array
cannot be freed here at all and stays leaked as it is today. It is
released on the failures that happen while it exists and before the first
PI regulator does - setup_pi_matrix() and the supply check - which is
where the ordinary deferral now lands. It also clears pi[].pw_d, and says
what it does not cover: a power domain shared with another controller can
still be freed under it, which is pre-existing and wants the domain out
of devm.

Patch 4 is a small si3474 change for the same reason. si3474 logged
every controller registration failure with dev_err(), and its binding
puts vpwr-supply in the pse-pi nodes, which fw_devlink does not wait
for, so with patch 5 it would log an error on every deferred probe
retry until the supply appears. It now uses dev_err_probe(), as
pd692x0 and tps23881 already do.

The v6 review [8] caught two things in the phy patch, both fixed in v7.
The error paths of phy_device_register() could leak a handle:
device_add() puts the phy on the klist before its own later failure
points, so a PSE_REGISTERED walk can attach one that nothing releases. A
put at the out: label does not work, because by then device_add() has
unwound the phy off the bus and the PSE_UNREGISTERED walk would miss it
too. phydev->psec_detached now covers registration as well as removal,
so no handle is attached in that window. The write before device_add()
is the one place it is set without pse_phy_lock(), which is safe because
the phy is not on the klist yet and device_add()'s own locking orders
it.

That flag is a plain bool rather than another bit in the flags word,
since it is written under pse_phy_lock() while its neighbours are written
under phydev->lock and rtnl.

Patch 6 is a one-entry change in drivers/of/. fw_devlink treats "pses"
as a supplier binding, so a phy that references a PI has a device link to
the PSE controller and its driver probe waits for it. That was invisible
while the PSE lookup deferred the phy anyway, since fwnode_mdio removed
it again on every retry. Once patch 7 stops
deferring, the phy is registered while its own driver is still blocked,
and a MAC attaching in that window takes the generic driver through
phy_attach_direct() - device_bind_driver() then forces the bind past the
pending link, and nothing re-probes it later.

Marking the link FWLINK_FLAG_IGNORE drops it entirely: no device link, no
ordering, no cycle detection, and the fwnode link goes at the consumer's
device_add(). That is what post-init-providers already does.

Two things do ride on that link today, because it is managed - the fwnode
link sits on the phy's own node, so fw_devlink_create_devlink() asks for
fw_devlink_flags, which defaults to FW_DEVLINK_FLAGS_RPM and carries
neither DL_FLAG_STATELESS nor DL_FLAG_SYNC_STATE_ONLY, so
device_link_add() promotes it to DL_FLAG_MANAGED. So unbinding the PSE
controller releases the phy's driver, and DL_FLAG_AUTOPROBE_CONSUMER
probes the phy when the controller binds. Both are given up on purpose:
the notifier does the attach and detach now, without tearing the port's
phy driver down, and there is no deferral left for an autoprobe to wait
on. DL_FLAG_PM_RUNTIME is the one that really had no effect, since no PSE
driver implements PM ops, sync_state or runtime PM.

That also means patch 6 is not a no-op on its own. The MDIO path
registers the phy before it looks the PI up, so the link exists and goes
active as soon as the controller binds, which puts the cascade's removal
at patch 6 rather than at patch 7. It also stops holding the phy's driver
back during those retries, so at patch 6 alone the driver probes and is
removed again on each one until the controller binds; patch 7 removes
the retries. Its changelog says both.

Rob, Saravana: this one is yours, and it is why the phy patch is safe to
take.

No Fixes: tag. 5e82147de1cb ("net: mdiobus: search for PSE nodes by
parsing PHY nodes.") is the commit to blame, but this is a refactor
across net, phylib and drivers/of plus six new exports, and tagging it
would invite a stable backport of all that to cure a probe-retry loop.

How it works: pse_core gets a notifier chain (REGISTERED /
UNREGISTERED). phylib subscribes, owns phydev->psec, and attaches the
handle when the controller shows up instead of during probe. fwnode_mdio
loses its PSE awareness, so no PSE-originated -EPROBE_DEFER leaves it.
Patch 6 removes the other half, the fw_devlink link that deferred the
phy's driver. With both gone there is no probe-retry loop left.

On the tags: Jonas tested the v4 shape and Aleksander tested the v6
locking, which is unchanged here. Neither tested the fixes above. On the
folded phy patch the code they exercised is intact, so I have kept their
tags there. Jonas's tag also rides on patch 2, and that one did change
in v7 - pse_controller_unregister() is reordered around the event - so
it is the weakest of the tags I kept. Patch 1 only gained a kernel-doc
correction. Happy to drop any of them if either would prefer.

Tested on a Realtek rtl9303 PoE switch with an HS104 PSE controller on
i2c, with a PD drawing power on one port. That board carries the notifier
mechanism from patches 1, 2 and 7; the failure paths of patches 3 and 5
are exercised in QEMU only, below, patch 6 only by booting with it
applied, and patch 4 is build-tested only.

 - clean boot, no probe-retry loop, the controller registers once
 - rmmod is refused while a phy holds a handle
 - i2c unbind: the notifier walk drops the handle and the port powers
   down, and ethtool reports no PSE attached
 - i2c bind: the handle comes back and the PD is powered again
 - six unbind/bind cycles, power domain index stable

Also exercised under QEMU, on arm64 under KASAN, PROVE_LOCKING and
kmemleak. The device tree has five PSE controllers:

 - a working one, plus an MDIO bus with two phys, only one of which
   references a PI. Unbinding the controller detaches that phy's handle
   and rebinding re-attaches it; the other phy is never touched;
   unbinding the MDIO bus releases a live handle.
 - one whose vpwr provider never appears - must defer.
 - one with its vpwr-supply on the controller node instead of the PI
   nodes, which the core resolves one stage later - must defer.
 - one whose controller node carries a vpwr-supply that never appears,
   with the first PI naming its own working supply and the next none, so
   a check that lets of_get_child_regulator() answer the second from the
   first misses the controller's supply - must defer.
 - one shaped like pd692x0, whose PI names a regulator the driver only
   registers from setup_pi_matrix() - must register, not defer.

I confirmed the last three both ways, with and without the fix each is
there for. The WARN_ON in patch 7 stays silent across six unbind cycles
and kmemleak reports nothing.

The same setup stages an over-budget static-priority domain for the
patch 2 case above: releasing a handle in the walk sheds a lower
priority port through pse_disable_pi_pol(). No lockdep splat on that
path, which re-enters the regulator core from a notifier callback under
the chain's rwsem and pse_phy_mutex.

Build matrix, all linking a real vmlinux: PHYLIB=y, PHYLIB=m (the config
that failed to link in v5), PHYLIB=n, PSE_CONTROLLER=n, and CONFIG_OF=n.

Changes in v8:
 - Rewrite the changelogs: drop references to other patches by position
   and to the pending net series (its conflict note is now below the
   --- of patch 2), reflow, and cut review history that belongs here.
 - Patch 2: add Co-developed-by, since the reordering is mine.
 - New patch 4: switch si3474 to dev_err_probe() for the controller
   registration error, so the supply check's -EPROBE_DEFER is not
   logged as an error on every retry.
 - Patch 7: say that any phy registered with a DT node now gets its PI,
   including ones found by mdiobus_scan(), and that a persistent lookup
   error is reported again on each controller registration.
 - Patch 3: clear pi[].pw_d in pse_flush_pw_ds(). The domain is devm
   memory of whoever created it, and pse_pi_is_enabled() still reaches
   that pointer from the regulator "state" attribute.
 - Patch 5: take the PI-node supply lookup only once that PI's own
   vpwr-supply phandle resolves. of_regulator_get_optional() falls back
   to of_get_child_regulator() on the device node, so a PI naming no
   supply of its own was answered from a sibling PI's before the
   controller node was consulted, masking an unresolved vpwr-supply
   there.
 - Patch 5: treat -EPERM from the supply get as resolved. It only means
   another consumer holds the provider exclusively, which the core's own
   resolution never asks about.
 - Patch 5: say why the checks cannot move ahead of setup_pi_matrix(),
   which the review asked for - a pd692x0 PI names a regulator that
   function registers itself.
 - New patch 6: stop "pses" gating probe in fw_devlink, so the phy is not
   left bound to the generic driver once patch 7 removes the deferral.
   Its changelog spells out that the link is managed, so dropping it also
   drops the supplier-unbind cascade and DL_FLAG_AUTOPROBE_CONSUMER, that
   this happens at patch 6 rather than at patch 7, and that on its own it
   lets the phy driver probe and unbind on every MDIO retry.
 - Patch 5: ask the controller-device stage of the supply lookup once for
   the whole controller instead of once per PI.
 - Patch 3: drop the flush_pw_ds label, which had to jump over
   release_pis, in favour of flushing inline at the one failure that
   needs it.
 - Document what is not fixed: the worker drain is only final once the
   phy patch lands, devres already quiesces the irq for tps23881, the OF
   references leak with the PI array on the three paths that keep it, a
   PSE probe failing after registration powers the PI down, and a shared
   power domain can still be freed under a second controller. The last
   is pre-existing and wants the domain out of devm, so not fixed here.
 - Fix changelog claims the review caught: pse_release_pis() does run
   from of_load_pse_pis() too, and nothing retries a PI that resolves
   after the pre-check (v7 said it could "resolve late").
 - Fix the phy_try_attach_pse() comment: other errors are not
   necessarily a broken binding, and -EPROBE_DEFER from a registered
   controller whose vpwr provider is not yet bound is not retried.
 - Patch 3: the shared power domain problem is not a refcount race, as
   my reply to the v7 review put it - pse_register_pw_ds() takes its
   reference under pse_pw_d_mutex and kref_put_mutex() takes that mutex
   for the final put. It is only that the domain is devm memory of the
   controller that created it. Changelog corrected.
 - Correct changelog and comment claims found in a final audit: patch 5's
   comments no longer say nothing retries the consumer at that commit
   (fwnode_mdio still does) or describe a CONFIG_OF=n path that cannot
   be reached; patch 7 no longer says a hard lookup error used to be
   retried by deferred probe - it failed the whole MDIO bus registration.

Changes in v7:
 - Patch 2: reorder pse_controller_unregister() around the event - unlink
   and disable_irq() before it, cancel_work_sync() after it, the frees
   last. The pse_control_head WARN_ON goes with the phy patch instead
   (patch 7 here), with the walk that empties the list - in patch 2 an
   unbind would trip it, since the fwnode_mdio hook still hands out
   handles nothing releases.
 - New patch 3: unwind the kfifo, the PI array and the power domains when
   controller registration fails.
 - New patch 4: check every PI vpwr supply before registering the
   controller, so a consumer never meets a registered controller that can
   only answer -EPROBE_DEFER.
 - Fold old patches 4 and 5 into the phy patch, so no intermediate commit
   carries the rtnl recursion or the deferred release.
 - Hold phydev->psec_detached across registration too, make it a bool
   rather than a bitfield, and drop the unsafe release from
   phy_device_register()'s error path.
 - Drop netsec from the deadlock list; fix the module-unload rationale;
   document that a transient attach error is no longer retried by deferred
   probe.
 - Include <linux/notifier.h> in phy_device.c.
 - Rebased on net-next.

Changes in v6:
 - Fix a v5 build regression: the mutex moved into pse_core, since
   net/ethtool is always in vmlinux while PHYLIB is tristate.
 - Fold phy_device_register_locked() back into phy_device_register().

Changes in v5 (since v4 [4]):
 - Replace rtnl with a dedicated mutex in the PSE attach path; with rtnl
   it deadlocked lantiq_etop [5].
 - Put phydev->psec back in phy_device_remove(), closing the
   use-after-free Paolo forwarded [6].

Changes in v4:
 - Add Tested-by from Jonas Jelonek. No code changes.

Changes in v3:
 - Drop patch 1 (regulator handle fix); it went to net separately [2].

v1 was an RFC by Corey [3].

[1] https://lore.kernel.org/netdev/20260620112440.1734404-1-github@szelinsky.de/
[2] https://lore.kernel.org/netdev/20260624204017.2752934-1-github@szelinsky.de/
[3] https://lore.kernel.org/netdev/20260423-pse-notifier-decouple-v1-0-86ed750a9d62@leavitt.info/
[4] https://lore.kernel.org/netdev/20260630091125.3162481-1-github@szelinsky.de/
[5] https://lore.kernel.org/netdev/bac5e6e9-7358-4ccb-87fc-9c40baa33682@wp.pl/
[6] https://lore.kernel.org/netdev/20260703071025.100797-1-pabeni@redhat.com/
[7] https://lore.kernel.org/netdev/20260906153102.959217-1-github@szelinsky.de/
[8] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260906153102.959217-1-github%40szelinsky.de
[9] https://lore.kernel.org/netdev/20260813200653.980170-1-github@szelinsky.de/
[10] https://lore.kernel.org/netdev/20260927191850.1370515-1-github@szelinsky.de/
[11] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927191850.1370515-1-github%40szelinsky.de

Carlo Szelinsky (4):
  net: pse-pd: unwind allocations when controller registration fails
  net: pse-pd: si3474: use dev_err_probe() for controller registration
  net: pse-pd: check the PI vpwr supply before registering the
    controller
  of: property: do not let "pses" block a consumer's probe

Corey Leavitt (3):
  net: pse-pd: add notifier chain for controller lifecycle events
  net: pse-pd: fire lifecycle events on controller register/unregister
  net: phy: own phydev->psec via PSE notifier and remove fwnode_mdio
    hook

 drivers/net/mdio/fwnode_mdio.c |  34 ----
 drivers/net/phy/phy_device.c   | 144 ++++++++++++++-
 drivers/net/pse-pd/pse_core.c  | 321 +++++++++++++++++++++++++++++++--
 drivers/net/pse-pd/si3474.c    |   7 +-
 drivers/of/property.c          |   5 +-
 include/linux/phy.h            |   7 +
 include/linux/pse-pd/pse.h     |  65 +++++++
 net/ethtool/pse-pd.c           |  16 +-
 8 files changed, 540 insertions(+), 59 deletions(-)


base-commit: e3bfd25626b44b6fa61a13c17178922171d519ce
-- 
2.43.0

[PATCH net-next v8 1/7] net: pse-pd: add notifier chain for controller lifecycle events

[Carlo Szelinsky]

From: Corey Leavitt <corey@leavitt.info>

Introduce a blocking notifier chain that allows other subsystems to be
informed when a PSE controller is registered or unregistered, and
provide pse_register_notifier() / pse_unregister_notifier() as the
subscriber interface.

A follow-up change uses this to let the phy subsystem own the
phydev->psec lifecycle directly, decoupling PSE lookup from
fwnode_mdiobus_register_phy() and removing the probe-time
-EPROBE_DEFER coupling that currently exists between mdio, phy and
pse-pd when the PSE controller driver is modular.

A blocking chain (rather than atomic) is used because callbacks sleep.
The phy subscriber takes a mutex, and of_pse_control_get() queries the
controller hardware, over i2c or a UART on most drivers. The contract
also lets a subscriber take rtnl, which is why a controller driver must
not hold it across pse_controller_register() or
pse_controller_unregister().

The enum pse_controller_event is placed outside the
IS_ENABLED(CONFIG_PSE_CONTROLLER) guard so that subscribers compiled
into a kernel without PSE support can still reference the event values
in dead-code paths without breaking the build.

This is pure infrastructure: nothing fires events yet, and nothing
subscribes. No observable behavior change.

Signed-off-by: Corey Leavitt <corey@leavitt.info>
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
---
 drivers/net/pse-pd/pse_core.c | 34 ++++++++++++++++++++++++++++++++++
 include/linux/pse-pd/pse.h    | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index a5e6d7b26b9f..84c734ed4553 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -8,6 +8,7 @@
 #include <linux/device.h>
 #include <linux/ethtool.h>
 #include <linux/ethtool_netlink.h>
+#include <linux/notifier.h>
 #include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/pse-pd/pse.h>
@@ -23,6 +24,39 @@ static LIST_HEAD(pse_controller_list);
 static DEFINE_XARRAY_ALLOC(pse_pw_d_map);
 static DEFINE_MUTEX(pse_pw_d_mutex);
 
+static BLOCKING_NOTIFIER_HEAD(pse_controller_notifier);
+
+/**
+ * pse_register_notifier - register a callback for PSE controller events
+ * @nb: notifier block to register
+ *
+ * See enum pse_controller_event for events fired and their subscriber
+ * contract. Callbacks run in process context; they may sleep, take
+ * rtnl, and call of_pse_control_get(). The chain fires synchronously,
+ * so a PSE controller driver's probe/unbind path must not hold any
+ * such lock when calling pse_controller_register() or
+ * pse_controller_unregister().
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+int pse_register_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&pse_controller_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(pse_register_notifier);
+
+/**
+ * pse_unregister_notifier - unregister a previously registered callback
+ * @nb: notifier block previously passed to pse_register_notifier()
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+int pse_unregister_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&pse_controller_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(pse_unregister_notifier);
+
 /**
  * struct pse_control - a PSE control
  * @pcdev: a pointer to the PSE controller device
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 4e5696cfade7..bc5d36bcd993 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -21,6 +21,7 @@ struct net_device;
 struct phy_device;
 struct pse_controller_dev;
 struct netlink_ext_ack;
+struct notifier_block;
 
 /* C33 PSE extended state and substate. */
 struct ethtool_c33_pse_ext_state_info {
@@ -337,6 +338,25 @@ enum pse_budget_eval_strategies {
 	PSE_BUDGET_EVAL_STRAT_DYNAMIC	= 1 << 2,
 };
 
+/**
+ * enum pse_controller_event - PSE controller lifecycle events
+ *
+ * Event data in callbacks is always a pointer to the struct
+ * pse_controller_dev firing the event.
+ *
+ * @PSE_REGISTERED: controller added to pse_controller_list and
+ *	resolvable by of_pse_control_get().
+ * @PSE_UNREGISTERED: controller already taken off pse_controller_list, so
+ *	no longer resolvable, but still valid to dereference for the
+ *	duration of the callback. Subscribers holding pse_control
+ *	references targeting it must drop them before returning and must
+ *	not acquire new references for it.
+ */
+enum pse_controller_event {
+	PSE_REGISTERED,
+	PSE_UNREGISTERED,
+};
+
 #if IS_ENABLED(CONFIG_PSE_CONTROLLER)
 int pse_controller_register(struct pse_controller_dev *pcdev);
 void pse_controller_unregister(struct pse_controller_dev *pcdev);
@@ -366,6 +386,9 @@ int pse_ethtool_set_prio(struct pse_control *psec,
 bool pse_has_podl(struct pse_control *psec);
 bool pse_has_c33(struct pse_control *psec);
 
+int pse_register_notifier(struct notifier_block *nb);
+int pse_unregister_notifier(struct notifier_block *nb);
+
 #else
 
 static inline struct pse_control *of_pse_control_get(struct device_node *node,
@@ -416,6 +439,16 @@ static inline bool pse_has_c33(struct pse_control *psec)
 	return false;
 }
 
+static inline int pse_register_notifier(struct notifier_block *nb)
+{
+	return 0;
+}
+
+static inline int pse_unregister_notifier(struct notifier_block *nb)
+{
+	return 0;
+}
+
 #endif
 
 #endif
-- 
2.43.0

[PATCH net-next v8 2/7] net: pse-pd: fire lifecycle events on controller register/unregister

[Carlo Szelinsky]

From: Corey Leavitt <corey@leavitt.info>

Hook the pse_controller_notifier chain so that pse_controller_register()
fires PSE_REGISTERED after the controller has been added to
pse_controller_list (i.e. is now resolvable by of_pse_control_get()),
and pse_controller_unregister() fires PSE_UNREGISTERED once it has been
taken back off, while pcdev and everything a subscriber's pse_control
points at are still valid to dereference.

No subscriber exists yet, so the event itself does nothing. The
reordering of pse_controller_unregister() around it is not a no-op,
though: it closes a teardown race that is reachable today, with no
subscriber involved.

The frees move to the bottom. pse_flush_pw_ds() and pse_release_pis()
are currently the first two statements, ahead of disable_irq() and
cancel_work_sync(), so a concurrent of_pse_control_get() and the
notification worker can reach pcdev->pi[] after pse_release_pis() has
freed it, and so can pse_isr() for a driver that requests its irq
before registering. They also have to stay below the event, because the
release path a subscriber runs reads pcdev->pi[] and pi->pw_d->supply.

The controller is unlinked before the event and before anything is
freed. of_pse_control_get() walks pse_controller_list and dereferences
pcdev->pi[] through of_pse_match_pi(). Subscribers are handed pcdev as
the event data and do not need it on the list, so taking it off first
costs nothing and closes that race for every caller.

disable_irq() moves up for the same reason: pse_isr() queues
notifications and reaches pcdev->pi, and nothing after it re-enables
the interrupt. On tps23881, the only in-tree user of
devm_pse_irq_helper(), the irq is requested after
devm_pse_controller_register(), so devres has already run free_irq()
by the time this runs. The move is what makes the ordering hold for a
driver that requests its irq earlier.

cancel_work_sync() moves above the frees but stays below the event. A
subscriber dropping the last pse_control reference reaches
__pse_control_release(), which calls regulator_disable() if the PI is
still on. With the static budget strategy that retries any port on the
same power domain waiting for power, and if the domain is still over
budget it sheds a lower priority port through pse_disable_pi_pol(),
which queues a notification and calls schedule_work(). Draining the
worker before the event would leave work queued behind it, racing the
kfifo_free() below. Draining after it also keeps the worker's own
transient reference, taken by pse_control_find_by_id(), from becoming
the last one after pse_release_pis() has freed the array.

That retry can also call ops->pi_enable() through
_pse_pi_delivery_power_sw_pw_ctrl(), so a port on the controller being
torn down can be energised from inside the event. The driver is still
attached at that point - the event runs before pse_release_pis() and
before devres unwinds the PI regulators - so the call is legal.

The drain is not yet final on its own. A pse_control holder that does
not subscribe - every phy today, since fwnode_mdio hands out handles
that only phy_device_remove() releases - can still reach
pse_disable_pi_pol() from the ethtool path and queue work after it.
That is how the tree behaves today and this does not widen it; it is
closed once phylib releases its handles in the event under a common
lock.

Signed-off-by: Corey Leavitt <corey@leavitt.info>
Co-developed-by: Carlo Szelinsky <github@szelinsky.de>
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
---

Notes:
    This conflicts in pse_controller_unregister() with the net series at
    https://lore.kernel.org/netdev/20260813200653.980170-1-github@szelinsky.de/
    which reorders the same function without an event to place. The
    resolution should take the order here, which already includes that
    reordering; the cover letter has the merged function.

 drivers/net/pse-pd/pse_core.c | 37 +++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 84c734ed4553..dc261beb6170 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -1138,6 +1138,9 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 	list_add(&pcdev->list, &pse_controller_list);
 	mutex_unlock(&pse_list_mutex);
 
+	blocking_notifier_call_chain(&pse_controller_notifier,
+				     PSE_REGISTERED, pcdev);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pse_controller_register);
@@ -1148,15 +1151,41 @@ EXPORT_SYMBOL_GPL(pse_controller_register);
  */
 void pse_controller_unregister(struct pse_controller_dev *pcdev)
 {
-	pse_flush_pw_ds(pcdev);
-	pse_release_pis(pcdev);
+	/* Raise the interrupt's disable depth before anything is freed.
+	 * pse_isr() queues notifications and reaches pcdev->pi, and nothing
+	 * below re-enables it. For a driver that requests its irq after
+	 * devm_pse_controller_register(), devres has already run free_irq()
+	 * by the time we get here and this only bumps the depth - the
+	 * ordering does not rely on that, so a driver requesting the irq
+	 * earlier is covered too.
+	 */
 	if (pcdev->irq)
 		disable_irq(pcdev->irq);
-	cancel_work_sync(&pcdev->ntf_work);
-	kfifo_free(&pcdev->ntf_fifo);
+
+	/* Unlink before the event: of_pse_control_get() walks
+	 * pse_controller_list and dereferences pcdev->pi[] through
+	 * of_pse_match_pi(), so no lookup may still reach this controller
+	 * once its teardown starts. Subscribers are handed pcdev as the
+	 * event data, so the notifier does not need it on the list.
+	 */
 	mutex_lock(&pse_list_mutex);
 	list_del(&pcdev->list);
 	mutex_unlock(&pse_list_mutex);
+
+	blocking_notifier_call_chain(&pse_controller_notifier,
+				     PSE_UNREGISTERED, pcdev);
+
+	/* After the event, not before. A subscriber dropping the last
+	 * pse_control reference reaches __pse_control_release() ->
+	 * regulator_disable() -> _pse_pi_disable(), which can end up in
+	 * pse_disable_pi_pol() and queue a notification of its own, so a
+	 * cancel_work_sync() placed above the walk would not stay drained.
+	 */
+	cancel_work_sync(&pcdev->ntf_work);
+
+	pse_flush_pw_ds(pcdev);
+	pse_release_pis(pcdev);
+	kfifo_free(&pcdev->ntf_fifo);
 }
 EXPORT_SYMBOL_GPL(pse_controller_unregister);
 
-- 
2.43.0

[PATCH net-next v8 3/7] net: pse-pd: unwind allocations when controller registration fails

[Carlo Szelinsky]

pse_controller_register() allocates the notification kfifo and, through
of_load_pse_pis(), the PI array plus OF references on each described PI
node and its pairset nodes. Every failure after that point simply
returns and none of it is freed. of_load_pse_pis() cleans up after
itself, but past that point kfifo_free() and pse_release_pis() only run
from pse_controller_unregister(), which a failed registration never
reaches, and devm_pse_controller_register() drops only its own devres
cookie. pcdev->pi is a plain allocation, so nothing else will ever free
it.

A partial pse_register_pw_ds() is worse than a leak. The power domains
it already created are devm-allocated but live in the global
pse_pw_d_map, so the failed probe frees them while that xarray still
points at them, and the next controller to register walks into freed
memory in pse_register_pw_ds(), reading pw_d->supply for
regulator_is_equal().

Unwind at two depths, because the PI array cannot always be freed here.
pse_pi_ops.is_enabled(), .enable() and .disable() all index pcdev->pi[],
and the PI regulators are devm-registered on pcdev->dev, so from the
first successful devm_pse_pi_regulator_register() until devres unwinds
the failed probe there are live regulators whose ops would follow a
freed pointer; regulator_late_cleanup() and the "state" class attribute
both reach them. So:

 - failures before of_load_pse_pis() succeeds free only the kfifo;
 - failures after it and before the PI regulator loop (today
   setup_pi_matrix()) also release the PI array and its OF references;
 - failures from the loop onwards free the kfifo, and the
   pse_register_pw_ds() one also flushes the power domains, but leave
   the PI array and its OF references leaked exactly as today rather
   than hand live regulators a dangling pointer.

A driver's setup_pi_matrix() may have registered devm regulators of its
own by the time it fails - pd692x0 registers its managers there - but
those do not index pcdev->pi[]. Freeing the array safely past the loop
would need the regulator ops to tolerate a NULL pcdev->pi, which they do
not today.

pcdev->pi is cleared at the release_pis label and deliberately not
inside pse_release_pis(): pse_controller_unregister() calls that helper
with every PI regulator still registered, and pse_pi_is_enabled()
indexes pcdev->pi[] unguarded behind the "state" attribute, so clearing
it there would turn that pre-existing read of freed memory into a NULL
dereference.

pse_flush_pw_ds() now also clears pi[].pw_d. The domain is devm memory
of whichever controller created it, so it can go as soon as that probe
unwinds, and pse_pi_is_enabled() still reaches pi[].pw_d from the
"state" attribute until the PI regulators are unregistered.

This does not fix a shared power domain. The reference counting is
sound - pse_register_pw_ds() takes its kref_get() under pse_pw_d_mutex
and kref_put_mutex() takes that mutex for the final put - but if
another controller on the same supply holds a reference, the flush only
goes 2->1 and devres frees the creator's pw_d underneath it anyway.
Unregistering a controller whose domain is shared has the same problem
today. Fixing it means moving the domain out of devm memory so that
only the last put frees it.

Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 56 ++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index dc261beb6170..eeefbf25e671 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -937,11 +937,21 @@ static void pse_flush_pw_ds(struct pse_controller_dev *pcdev)
 			continue;
 
 		pw_d = xa_load(&pse_pw_d_map, pcdev->pi[i].pw_d->id);
-		if (!pw_d)
+		if (!pw_d) {
+			pcdev->pi[i].pw_d = NULL;
 			continue;
+		}
 
 		kref_put_mutex(&pw_d->refcnt, __pse_pw_d_release,
 			       &pse_pw_d_mutex);
+		/* The pw_d is devm memory of whichever controller created
+		 * it, so it can go away as soon as that probe unwinds.
+		 * Nothing may be left pointing at it: pse_pi_is_enabled()
+		 * reaches pi[].pw_d from the regulator "state" attribute,
+		 * which stays readable until the PI regulators are
+		 * unregistered after us.
+		 */
+		pcdev->pi[i].pw_d = NULL;
 	}
 }
 
@@ -1092,17 +1102,18 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 	    !pcdev->ops->pi_get_pw_status) {
 		dev_err(pcdev->dev,
 			"Mandatory status report callbacks are missing");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto free_kfifo;
 	}
 
 	ret = of_load_pse_pis(pcdev);
 	if (ret)
-		return ret;
+		goto free_kfifo;
 
 	if (pcdev->ops->setup_pi_matrix) {
 		ret = pcdev->ops->setup_pi_matrix(pcdev);
 		if (ret)
-			return ret;
+			goto release_pis;
 	}
 
 	/* Each regulator name len is pcdev dev name + 7 char +
@@ -1110,7 +1121,12 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 	 */
 	reg_name_len = strlen(dev_name(pcdev->dev)) + 18;
 
-	/* Register PI regulators */
+	/* Register PI regulators. Once one of these exists, pse_pi_ops index
+	 * pcdev->pi[] and nothing here can unregister it again, so the array
+	 * must outlive this function. Failures below therefore unwind to
+	 * free_kfifo and deliberately leak it, as they already do today,
+	 * rather than hand the live regulators a freed pointer.
+	 */
 	for (i = 0; i < pcdev->nr_lines; i++) {
 		char *reg_name;
 
@@ -1119,20 +1135,27 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 			continue;
 
 		reg_name = devm_kzalloc(pcdev->dev, reg_name_len, GFP_KERNEL);
-		if (!reg_name)
-			return -ENOMEM;
+		if (!reg_name) {
+			ret = -ENOMEM;
+			goto free_kfifo;
+		}
 
 		snprintf(reg_name, reg_name_len, "pse-%s_pi%d",
 			 dev_name(pcdev->dev), i);
 
 		ret = devm_pse_pi_regulator_register(pcdev, reg_name, i);
 		if (ret)
-			return ret;
+			goto free_kfifo;
 	}
 
 	ret = pse_register_pw_ds(pcdev);
-	if (ret)
-		return ret;
+	if (ret) {
+		/* Deliberately not release_pis: the PI regulators registered
+		 * above index pcdev->pi[] and outlive this function.
+		 */
+		pse_flush_pw_ds(pcdev);
+		goto free_kfifo;
+	}
 
 	mutex_lock(&pse_list_mutex);
 	list_add(&pcdev->list, &pse_controller_list);
@@ -1142,6 +1165,19 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 				     PSE_REGISTERED, pcdev);
 
 	return 0;
+
+	/* Only for failures above the PI regulator loop, where no regulator
+	 * indexes pcdev->pi[] yet. Anything below it has to go straight to
+	 * free_kfifo instead.
+	 */
+release_pis:
+	pse_release_pis(pcdev);
+	pcdev->pi = NULL;
+
+free_kfifo:
+	kfifo_free(&pcdev->ntf_fifo);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(pse_controller_register);
 
-- 
2.43.0

[PATCH net-next v8 4/7] net: pse-pd: si3474: use dev_err_probe() for controller registration

[Carlo Szelinsky]

si3474_i2c_probe() reports a failed devm_pse_controller_register() with
dev_err() and returns the error. That is fine as long as registration
only fails for real, but a follow-up change makes it return
-EPROBE_DEFER while a PI's vpwr supply provider has not registered yet.

skyworks,si3474.yaml puts vpwr-supply in the pse-pi nodes. A supply
named from a child node only gives the controller a SYNC_STATE_ONLY
proxy link in fw_devlink, which does not hold its probe back, so si3474
can probe before that regulator and would then log an error on every
deferred probe retry until it appears.

Use dev_err_probe(), which stays quiet for -EPROBE_DEFER and records the
reason for devices_deferred instead, and prints the error symbolically
rather than as a raw hex value.

Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/si3474.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/pse-pd/si3474.c b/drivers/net/pse-pd/si3474.c
index 1845b9c51cf7..91800f39971f 100644
--- a/drivers/net/pse-pd/si3474.c
+++ b/drivers/net/pse-pd/si3474.c
@@ -541,10 +541,9 @@ static int si3474_i2c_probe(struct i2c_client *client)
 	priv->pcdev.nr_lines = SI3474_MAX_CHANS;
 
 	ret = devm_pse_controller_register(dev, &priv->pcdev);
-	if (ret) {
-		dev_err(dev, "Failed to register PSE controller: 0x%x\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to register PSE controller\n");
 
 	return 0;
 }
-- 
2.43.0

[PATCH net-next v8 5/7] net: pse-pd: check the PI vpwr supply before registering the controller

[Carlo Szelinsky]

Each PSE PI regulator is registered with a "vpwr" supply, which the
bindings expect the board to provide via vpwr-supply in the PI node.
The regulator core deliberately treats an unresolved supply at
registration time as non-fatal and lets registration succeed.

That leaves pse_controller_register() completing for a controller whose
PIs cannot be handed out: regulator_get_exclusive() in
pse_control_get_internal() resolves the supply itself and returns
-EPROBE_DEFER until the provider appears, so of_pse_control_get() keeps
failing for this PI even though the controller is registered. A
consumer that only retries when a controller registers - which is what
phylib becomes once it attaches from the PSE notifier - then never gets
its PI.

Check every PI's supply first, before any regulator of ours is
registered, so the error surfaces in the PSE driver's own probe and
deferred probe retries it there. The check skips exactly the PIs the
registration loop skips, so a controller with no pse-pis node - every
PI has a NULL np but still gets a regulator - is covered too, through
the controller device.

The check follows both stages regulator_resolve_supply() uses: the PI's
own node, then the controller device. A vpwr-supply written once on the
controller node is invisible from the PI node, while the core finds it
at stage two. pse_pi_check_supply() answers from the PI node and
returns 1 when it cannot; pse_controller_check_supply() is then asked
at the first PI that needs it and not again, since its answer is the
same for all of them. Asking it inside the loop keeps the order the
core resolves in, so the first PI to fail is the one reported, and a
later PI's hard error cannot replace an earlier PI's -EPROBE_DEFER.

The PI node is consulted only once its own vpwr-supply phandle
resolves. of_regulator_get_optional() falls back to
of_get_child_regulator() on the device node, which walks every
descendant of the controller, so a PI naming no supply of its own would
otherwise be answered from a sibling PI's before the controller node
was consulted. The core reaches that sibling walk only at its second
stage, after the controller's own vpwr-supply.

-EPERM is treated as resolved: it only means another consumer holds the
provider exclusively, which regulator_resolve_supply() never asks about.

It is not a full replica. The core also defers while the supply
provider's parent device is not yet bound, which regulator_get_optional()
does not check, so a PSE probe interleaving with the provider's own
probe can still pass, and nothing retries that PI until the next
PSE_REGISTERED or a phy re-registration. fw_devlink only prevents this
when vpwr-supply is on the controller node; one in a pse-pi node only
gives the controller a SYNC_STATE_ONLY proxy link, which does not hold
its probe back. Closing that would mean asking the core about
rdev->supply after registration, when the PI array can no longer be
released.

The checks run after setup_pi_matrix() and before the registration
loop. They cannot run earlier: a PI's vpwr-supply may name a regulator
the controller registers in setup_pi_matrix() - microchip,pd692x0.yaml
points its pse-pi nodes at the manager regulators
pd692x0_register_managers_regulator() creates - so an earlier check
would defer forever on a supply only this driver can provide. They
should not run inside the loop either: ti,tps23881.yaml puts pse-pi@0
on vpwr1 and pse-pi@1 on vpwr2, so "one PI resolves, the next defers"
is the ordinary case, and deferring after the first PI regulator exists
would leak the PI array on every retry.

What this placement does not recover is anything setup_pi_matrix() has
already claimed. pd692x0 requests a power budget on its manager
supplies there and frees it only from that function's own error labels
or from remove(). That is true of every existing failure after
setup_pi_matrix() too, and the new deferral does not hit it with the
documented binding, since a pd692x0 PI's supplies are the managers it
has just registered. A board pointing a pd692x0 PI at an external
provider would; that cleanup belongs in the driver.

This widens what a missing provider costs: before, only the PI on that
supply failed, and only when a consumer asked for it; now the whole
controller defers until every provider has appeared. That is the normal
contract for a regulator consumer, and the alternative - a controller
that advertises PIs it cannot hand out - is the bug being fixed.

Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 126 ++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index eeefbf25e671..e560833ad034 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -12,6 +12,7 @@
 #include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/pse-pd/pse.h>
+#include <linux/regulator/consumer.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 #include <linux/rtnetlink.h>
@@ -860,6 +861,98 @@ static const struct regulator_ops pse_pi_ops = {
 	.set_current_limit = pse_pi_set_current_limit,
 };
 
+/* The regulator core treats an unresolved "vpwr" supply as non-fatal and
+ * retries it on its own later, which would leave this controller able to
+ * register while of_pse_control_get() still fails with -EPROBE_DEFER for
+ * this PI. A consumer that is not itself retried by deferred probe -
+ * phylib, once it attaches from the PSE notifier - would never get it.
+ * Check the supply up front so the PSE driver's own probe defers instead.
+ *
+ * This is the first of the two stages the core uses, the PI's own node.
+ * pse_controller_check_supply() is the second, the controller device.
+ * Neither get falls back to the dummy regulator, so a PI that names no supply
+ * anywhere still reports -ENODEV and keeps its existing behaviour - the
+ * core resolves it to the dummy when the PI regulator is registered.
+ *
+ * The PI node is consulted only once its own phandle is known to resolve.
+ * of_regulator_get_optional() falls back to of_get_child_regulator() on
+ * the *device* node, which walks every descendant of the controller, so a
+ * PI that does not name a supply itself would otherwise be answered from
+ * a sibling PI's and pass before the controller node is ever consulted.
+ * The core reaches the same sibling walk, but only at its second stage
+ * and only after the controller's own vpwr-supply has had its turn - so
+ * the gate is what keeps a controller-level supply from being masked.
+ *
+ * Return: 0 if this PI needs nothing more, 1 if the controller device has
+ *	   to answer for it, or a negative error.
+ */
+static int pse_pi_check_supply(struct pse_controller_dev *pcdev, int id)
+{
+	struct device_node *np;
+	struct regulator *supply;
+	int ret;
+
+	/* Skip exactly the PIs the registration loop below skips, so every
+	 * regulator that will be created is covered.
+	 */
+	if (!pcdev->no_of_pse_pi && !pcdev->pi[id].np)
+		return 0;
+
+	np = pcdev->pi[id].np ?
+	     of_parse_phandle(pcdev->pi[id].np, "vpwr-supply", 0) : NULL;
+	if (!np)
+		return 1;
+
+	of_node_put(np);
+	supply = of_regulator_get_optional(pcdev->dev, pcdev->pi[id].np,
+					   "vpwr");
+	if (!IS_ERR(supply)) {
+		regulator_put(supply);
+		return 0;
+	}
+
+	ret = PTR_ERR(supply);
+	/* -EPERM only says someone holds this provider exclusively, which
+	 * the core's own resolution never asks about. The provider is
+	 * there, so this PI is fine.
+	 */
+	if (ret == -EPERM)
+		return 0;
+	/* Only -ENODEV moves on to the second stage; a provider that has
+	 * not registered yet gives -EPROBE_DEFER, which is the case worth
+	 * catching here.
+	 */
+	if (ret == -ENODEV)
+		return 1;
+
+	return dev_err_probe(pcdev->dev, ret,
+			     "PI %d: failed to get vpwr supply\n", id);
+}
+
+/* Second stage of pse_pi_check_supply(). A vpwr-supply on the controller
+ * node covers every PI, and is where a PI without a node of its own
+ * resolves too, so the answer is the same for all of them - ask it once,
+ * and only if some PI needs it.
+ */
+static int pse_controller_check_supply(struct pse_controller_dev *pcdev)
+{
+	struct regulator *supply;
+	int ret;
+
+	supply = regulator_get_optional(pcdev->dev, "vpwr");
+	if (!IS_ERR(supply)) {
+		regulator_put(supply);
+		return 0;
+	}
+
+	ret = PTR_ERR(supply);
+	if (ret == -ENODEV || ret == -EPERM)
+		return 0;
+
+	return dev_err_probe(pcdev->dev, ret,
+			     "failed to get vpwr supply\n");
+}
+
 static int
 devm_pse_pi_regulator_register(struct pse_controller_dev *pcdev,
 			       char *name, int id)
@@ -1082,6 +1175,7 @@ static void pse_send_ntf_worker(struct work_struct *work)
  */
 int pse_controller_register(struct pse_controller_dev *pcdev)
 {
+	bool ctrl_supply_ok = false;
 	size_t reg_name_len;
 	int ret, i;
 
@@ -1116,6 +1210,38 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 			goto release_pis;
 	}
 
+	/* Check every PI supply before any regulator of ours is registered:
+	 * a provider that has not probed yet is the ordinary -EPROBE_DEFER
+	 * case, and unwinding it must still be able to release the PI array,
+	 * which it cannot once a PI regulator exists.
+	 *
+	 * This has to stay after setup_pi_matrix(). A PI's vpwr-supply can
+	 * name a regulator the controller itself creates there - pd692x0's
+	 * pse-pi nodes point at its manager regulators - so checking any
+	 * earlier would defer on a supply that only this driver can bring
+	 * up, and it would never probe.
+	 */
+	for (i = 0; i < pcdev->nr_lines; i++) {
+		ret = pse_pi_check_supply(pcdev, i);
+		if (ret < 0)
+			goto release_pis;
+
+		/* The controller device answers for every PI that its own
+		 * node cannot, so it is asked at the first such PI and not
+		 * again. Asking it here rather than after the loop keeps
+		 * the order the core resolves in, so the first PI to fail
+		 * is still the one reported.
+		 */
+		if (!ret || ctrl_supply_ok)
+			continue;
+
+		ret = pse_controller_check_supply(pcdev);
+		if (ret)
+			goto release_pis;
+
+		ctrl_supply_ok = true;
+	}
+
 	/* Each regulator name len is pcdev dev name + 7 char +
 	 * int max digit number (10) + 1
 	 */
-- 
2.43.0

[PATCH net-next v8 6/7] of: property: do not let "pses" block a consumer's probe

[Carlo Szelinsky]

fw_devlink treats "pses" as a supplier binding, so a PHY that
references a PSE PI has its driver probe held in
device_links_check_suppliers() until the PSE controller binds: by a
fwnode link to the PI node until then, and by a device link to the
controller once it has bound.

That is harmless while the PSE lookup itself defers the PHY:
fwnode_mdio registers it, fails the lookup and removes it again on
every retry, so it never stays registered long enough to be bound
early. Once phylib stops deferring on PSE and picks the handle up from
a notifier instead, the PHY is registered while its own driver is
still blocked, and a MAC or DSA switch attaching in that window binds
the generic driver:

  phy_attach_direct()
    if (!d->driver)
            d->driver = &genphy_driver.mdiodrv.driver;
    device_bind_driver()
      device_links_force_bind()

device_bind_driver() does not wait for suppliers:
device_links_force_bind() drops any managed supplier link that is not
available yet, and driver_bound() purges the PHY's remaining fwnode
supplier links. Nothing re-probes the PHY once the PSE controller shows
up, so the port keeps running on genphy until a rebind or a reboot.

The link is not needed for correctness. PSE is not a resource the
consumer must have before it probes: phylib looks a PI up when its
controller becomes available and releases it when the controller goes
away, and of_pse_control_get() is the only reader of the property.

Mark it FWLINK_FLAG_IGNORE, as post-init-providers already is. That
drops the dependency entirely: fw_devlink_create_devlink() returns
early, so no device link is made, the fwnode link is deleted at the
consumer's device_add() like any other that has been handled, and
fw_devlink neither gates probe on it nor uses it for cycle detection.

Two things ride on the link today, and both go with it, because it is
managed. of_link_property() passes no get_con_dev for parse_pses, so
the fwnode link sits on the PHY's own node, which is the PHY device's
fwnode by the time it is registered, and fw_devlink_create_devlink()
takes its first branch:

	if (con->fwnode == link->consumer)
		flags = fw_devlink_get_flags(link->flags);
	else
		flags = FW_DEVLINK_FLAGS_PERMISSIVE;

With link->flags clear that returns fw_devlink_flags, by default
FW_DEVLINK_FLAGS_RPM, which carries neither DL_FLAG_STATELESS nor
DL_FLAG_SYNC_STATE_ONLY, so device_link_add() makes it DL_FLAG_MANAGED.
Unbinding the PSE controller therefore releases the PHY's driver today,
and DL_FLAG_AUTOPROBE_CONSUMER probes the PHY once the controller
binds. Both are given up on purpose: phylib's notifier takes the PI
away on unbind and hands it back on bind without tearing the PHY driver
down, and with no deferral left there is nothing for an autoprobe to
wait for. DL_FLAG_PM_RUNTIME and the dpm_list reordering go without
consequence, since no PSE driver implements PM ops, .shutdown,
sync_state or runtime PM.

This change is not a no-op while fwnode_mdio still does the lookup.
fwnode_mdiobus_register_phy() registers the PHY before it looks the PI
up, so the device link is created regardless and becomes active as
soon as the controller is bound; the unbind cascade above goes away
from here on. The link also held the PHY's driver back on each of those
retries while the controller was unbound. Without it the driver probes
inside device_add() - phy drivers are PROBE_FORCE_SYNCHRONOUS - and is
removed again when the lookup defers, once per retry of the MDIO bus
owner, until the PSE controller binds or fwnode_mdio stops doing the
lookup.

Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/of/property.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 72cf12907de0..5ec0f05b87ac 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1564,7 +1564,10 @@ static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_backlight, },
 	{ .parse_prop = parse_panel, },
 	{ .parse_prop = parse_msi_parent, },
-	{ .parse_prop = parse_pses, },
+	{
+		.parse_prop = parse_pses,
+		.fwlink_flags = FWLINK_FLAG_IGNORE,
+	},
 	{ .parse_prop = parse_power_supplies, },
 	{ .parse_prop = parse_mmc_pwrseq, },
 	{ .parse_prop = parse_gpio_compat, },
-- 
2.43.0

[PATCH net-next v8 7/7] net: phy: own phydev->psec via PSE notifier and remove fwnode_mdio hook

[Carlo Szelinsky]

From: Corey Leavitt <corey@leavitt.info>

fwnode_mdiobus_register_phy() resolved the `pses` phandle during PHY
registration and stashed the result in phydev->psec. With a modular PSE
controller driver that lookup returns -EPROBE_DEFER until the module is
loaded, so the MDIO bus owner - a MAC or a DSA switch - keeps bouncing
off deferred probe, registering and removing the PHY each time. On some
boards that shows up as a boot-time probe-retry storm and PHYs that
never stay registered.

Transfer ownership of phydev->psec to phylib and drive it from the PSE
controller lifecycle notifier instead:

  - On PSE_REGISTERED: walk mdio_bus_type and attach phydev->psec for
    every registered phy whose psec is still NULL. This is the "phy was
    enumerated before the PSE controller loaded" case, the root cause
    of the retry storm.

  - On PSE_UNREGISTERED: walk it again and release every phydev->psec
    that targets the departing controller, before pse_release_pis()
    frees pcdev->pi. Without this a phy still holding a reference would
    cause a use-after-free in __pse_control_release()'s
    pcdev->pi[psec->id] access.

  - phy_device_register() attaches once for a controller that is
    already registered. The phydev->psec check in phy_try_attach_pse()
    makes the two paths idempotent.

fwnode_find_pse_control() and its call site go away, and fwnode_mdio
drops the PSE header. No PSE-originated -EPROBE_DEFER leaves the MDIO
layer any more, so the retry storm is gone.

The attach now covers every phy registered with a DT node, not only
those registered through fwnode_mdiobus_register_phy(). A phy found by
mdiobus_scan() on a bus registered with plain mdiobus_register() gets
its node from of_mdiobus_link_mdiodev(), and a `pses` property there is
now honoured too.

The attach happens after device_add() has made the phy visible on
mdio_bus_type. Neither device_add() nor the attach that follows it may
take rtnl, for two reasons:

  - Binding a phy that itself provides an SFP cage reaches
    sfp_bus_add_upstream() via phy_probe() -> phy_setup_ports() ->
    phy_sfp_probe(), and that takes rtnl_lock(). Reported on RTL8214FC.

  - Some drivers register their MDIO bus from ndo_init - lantiq_etop
    via ltq_etop_init(), sni_ave via ave_init() - which
    register_netdevice() already calls under rtnl, and the bus
    registration reaches phy_device_register() from there
    (mdiobus_scan() for lantiq_etop, of_mdiobus_register_phy() for
    sni_ave).

phydev->psec is therefore serialised by a dedicated mutex, taken
through pse_phy_lock(). It lives in pse_core rather than phylib because
net/ethtool is always built into vmlinux while PHYLIB is tristate, so a
phylib export would be unresolved with CONFIG_PHYLIB=m or =n;
PSE_CONTROLLER is bool, so pse_core is always reachable. The notifier
walks change phydev->psec without rtnl, so the ethtool PSE paths take
the same mutex to keep a released handle from being dereferenced. Lock
order is rtnl -> pse_phy_mutex -> pse_list_mutex -> pcdev->lock.

phy_device_remove() releases phydev->psec synchronously, while the phy
is still on the bus. A phy that has been device_del()'d but is still
pinned - an attached netdev, or an SFP module phy waiting for
phy_device_free() - is off the mdio_bus_type klist and so invisible to
the PSE_UNREGISTERED walk, and a deferred release would then touch a
pcdev->pi[] the controller has already freed. Releasing before
device_del() means the walk and the release contend for the same mutex,
so whichever runs second sees NULL.

phydev->psec_detached closes two windows around that by telling
phy_try_attach_pse() to skip the phy. bus_for_each_dev() still reaches
a phy until bus_remove_device() takes it off the klist, well into
device_del(), so a PSE_REGISTERED walk could otherwise attach a fresh
handle just after the release, with nothing left to free it. The same
applies during registration: device_add() puts the phy on the bus
before its own later failure points, and its unwind takes it back off,
so a handle attached in that window would be missed by the
PSE_UNREGISTERED walk as well. The flag is therefore held until
registration has succeeded, and set again from phy_device_remove(). It
is a plain bool rather than another bit in the flags word: it is
written under pse_phy_lock(), while neighbours such as suspended and
sysfs_links are written under phydev->lock and rtnl, and a shared
storage unit would make those read-modify-write updates race.

A lookup error no longer fails anything. of_pse_control_get() reaches
the hardware - pse_pi_is_hw_enabled() calls pi_get_admin_state(), a bus
transaction on tps23881, si3474, pd692x0 and realtek-pse-mcu - and that
error used to propagate out of fwnode_mdiobus_register_phy() and fail
the whole MDIO bus registration. Now it leaves the port without PSE
until a controller registers again or the phy is re-registered, and is
reported with phydev_warn(). Because every PSE_REGISTERED walk retries
each phy that has no handle yet, a persistent error such as a
`#pse-cells` mismatch - which also trips the WARN_ON() in
of_pse_control_get() - is now reported again on each controller
registration rather than once.

-EPROBE_DEFER stays silent because the notifier retries it at
PSE_REGISTERED time, and the controller's vpwr supply check removes the
case where a registered controller keeps returning it for a provider
that has not appeared. It does not remove every source: the regulator
core still defers a supply whose provider is registered but not yet
bound. Such a PI is not retried until the next PSE_REGISTERED or a phy
re-registration.

The REGISTERED walk runs inside pse_controller_register(), so it
attaches handles part-way through the PSE driver's own probe. For a PI
the hardware already has powered, pse_control_get_internal() records
admin_state_enabled and takes the exclusive regulator. If that probe
then fails after registration - tps23881 requests its irq after
devm_pse_controller_register() - devres unwinds into the UNREGISTERED
walk, the last reference goes, and __pse_control_release() disables
the PI. A port that was up before the driver loaded is then powered
down when that driver fails to finish probing, the same as on a clean
unbind.

The UNREGISTERED walk does not help rmmod: pse_control_get_internal()
takes try_module_get(pcdev->owner) per handle, so while a phy holds one
the unload is refused before the module exit path runs. What the walk
covers is driver unbind and device removal, where
pse_controller_unregister() runs with the module loaded.

The walk relies on the teardown order pse_controller_unregister()
already has: the controller is off pse_controller_list and the
interrupt is off, so nothing new can reach the PI array; pcdev->pi and
the power domains are still live, which the release path needs because
dropping the last reference can disable a PI that is still energised;
and the notification worker is drained after the walk, both because
that release path can queue work and because the worker's transient
pse_control reference must not become the last one once the array has
been freed.

Now that the walk empties pcdev->pse_control_head on unregister, warn
if anything is still on it afterwards. Nothing can add one at that
point: the controller is off the list, the irq is off and the worker is
drained. It is quiet because of_pse_control_get() has a single caller
and the walk covers it, and it fires if a consumer that does not
subscribe is ever added. It could not be added earlier, while the
fwnode_mdio hook gave each matching phy a handle that nothing released
on controller unbind.

Reported-by: Jonas Jelonek <jelonek.jonas@gmail.com>
Closes: https://lore.kernel.org/netdev/e00048dd-1ed3-40c3-9912-59bccf015ad5@gmail.com/
Reported-by: Aleksander Jan Bajkowski <olek2@wp.pl>
Closes: https://lore.kernel.org/netdev/bac5e6e9-7358-4ccb-87fc-9c40baa33682@wp.pl/
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Link: https://lore.kernel.org/netdev/20260703071025.100797-1-pabeni@redhat.com/
Signed-off-by: Corey Leavitt <corey@leavitt.info>
Co-developed-by: Carlo Szelinsky <github@szelinsky.de>
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
Tested-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
 drivers/net/mdio/fwnode_mdio.c |  34 --------
 drivers/net/phy/phy_device.c   | 144 ++++++++++++++++++++++++++++++++-
 drivers/net/pse-pd/pse_core.c  |  68 ++++++++++++++++
 include/linux/phy.h            |   7 ++
 include/linux/pse-pd/pse.h     |  32 ++++++++
 net/ethtool/pse-pd.c           |  16 ++--
 6 files changed, 261 insertions(+), 40 deletions(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index ba7091518265..7bd979b59f49 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -11,33 +11,11 @@
 #include <linux/fwnode_mdio.h>
 #include <linux/of.h>
 #include <linux/phy.h>
-#include <linux/pse-pd/pse.h>
 
 MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("FWNODE MDIO bus (Ethernet PHY) accessors");
 
-static struct pse_control *
-fwnode_find_pse_control(struct fwnode_handle *fwnode,
-			struct phy_device *phydev)
-{
-	struct pse_control *psec;
-	struct device_node *np;
-
-	if (!IS_ENABLED(CONFIG_PSE_CONTROLLER))
-		return NULL;
-
-	np = to_of_node(fwnode);
-	if (!np)
-		return NULL;
-
-	psec = of_pse_control_get(np, phydev);
-	if (PTR_ERR(psec) == -ENOENT)
-		return NULL;
-
-	return psec;
-}
-
 static struct mii_timestamper *
 fwnode_find_mii_timestamper(struct fwnode_handle *fwnode)
 {
@@ -118,7 +96,6 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 				struct fwnode_handle *child, u32 addr)
 {
 	struct mii_timestamper *mii_ts = NULL;
-	struct pse_control *psec = NULL;
 	struct phy_device *phy;
 	bool is_c45;
 	u32 phy_id;
@@ -159,14 +136,6 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 			goto clean_phy;
 	}
 
-	psec = fwnode_find_pse_control(child, phy);
-	if (IS_ERR(psec)) {
-		rc = PTR_ERR(psec);
-		goto unregister_phy;
-	}
-
-	phy->psec = psec;
-
 	/* phy->mii_ts may already be defined by the PHY driver. A
 	 * mii_timestamper probed via the device tree will still have
 	 * precedence.
@@ -176,9 +145,6 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 
 	return 0;
 
-unregister_phy:
-	if (is_acpi_node(child) || is_of_node(child))
-		phy_device_remove(phy);
 clean_phy:
 	phy_device_free(phy);
 clean_mii_ts:
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 5b13a74e2fa9..497cf179398b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -26,6 +26,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/netdevice.h>
+#include <linux/notifier.h>
 #include <linux/phy.h>
 #include <linux/phylib_stubs.h>
 #include <linux/phy_led_triggers.h>
@@ -1012,9 +1013,116 @@ struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
 }
 EXPORT_SYMBOL(get_phy_device);
 
+/* Best-effort attach of phydev->psec from a DT `pses = <&...>` phandle.
+ * Caller must hold pse_phy_lock(). A missing phandle (-ENOENT) is silent,
+ * and so is -EPROBE_DEFER. That mostly means the controller is not
+ * registered yet, which the notifier retries at PSE_REGISTERED time, but
+ * a registered controller returns it too while its vpwr provider is
+ * registered and not yet bound, and that is not retried until the next
+ * PSE_REGISTERED. Anything else is warned about and left non-fatal, so
+ * the phy still registers - it is not necessarily a broken binding, it
+ * can equally be -ENOMEM or the PI already being held, and nothing
+ * retries it until the next PSE_REGISTERED.
+ *
+ * A phy with psec_detached set is skipped: it is either not registered yet
+ * or on its way out, and nothing would release a handle attached now.
+ */
+static void phy_try_attach_pse(struct phy_device *phydev)
+{
+	struct pse_control *psec;
+	struct device_node *np;
+
+	pse_phy_lock_assert_held();
+
+	np = phydev->mdio.dev.of_node;
+	if (!np)
+		return;
+
+	if (phydev->psec || phydev->psec_detached)
+		return;
+
+	psec = of_pse_control_get(np, phydev);
+	if (IS_ERR(psec)) {
+		if (PTR_ERR(psec) != -EPROBE_DEFER && PTR_ERR(psec) != -ENOENT)
+			phydev_warn(phydev, "failed to get PSE control: %pe\n",
+				    psec);
+		return;
+	}
+
+	phydev->psec = psec;
+}
+
+static int phy_pse_attach_one(struct device *dev, void *data)
+{
+	pse_phy_lock_assert_held();
+
+	if (dev->type != &mdio_bus_phy_type)
+		return 0;
+
+	phy_try_attach_pse(to_phy_device(dev));
+	return 0;
+}
+
+static int phy_pse_detach_one(struct device *dev, void *data)
+{
+	struct pse_controller_dev *pcdev = data;
+	struct phy_device *phydev;
+	struct pse_control *psec;
+
+	pse_phy_lock_assert_held();
+
+	if (dev->type != &mdio_bus_phy_type)
+		return 0;
+
+	phydev = to_phy_device(dev);
+	psec = phydev->psec;
+	if (!psec || !pse_control_matches_pcdev(psec, pcdev))
+		return 0;
+
+	phydev->psec = NULL;
+	pse_control_put(psec);
+	return 0;
+}
+
+static int phy_pse_notifier_event(struct notifier_block *nb,
+				  unsigned long event, void *data)
+{
+	switch (event) {
+	case PSE_REGISTERED:
+		pse_phy_lock();
+		bus_for_each_dev(&mdio_bus_type, NULL, NULL,
+				 phy_pse_attach_one);
+		pse_phy_unlock();
+		return NOTIFY_OK;
+	case PSE_UNREGISTERED:
+		pse_phy_lock();
+		bus_for_each_dev(&mdio_bus_type, NULL, data,
+				 phy_pse_detach_one);
+		pse_phy_unlock();
+		return NOTIFY_OK;
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
+static struct notifier_block phy_pse_notifier __read_mostly = {
+	.notifier_call = phy_pse_notifier_event,
+};
+
 /**
  * phy_device_register - Register the phy device on the MDIO bus
  * @phydev: phy_device structure to be added to the MDIO bus
+ *
+ * phydev->psec is attached after device_add() has made the phy visible on
+ * mdio_bus_type, so that a concurrent PSE notifier walk and the attach can
+ * never leave the phy unattached. Neither step takes rtnl: keeping
+ * device_add() out of rtnl avoids deadlocking when binding a phy that itself
+ * provides an SFP cage (phy_probe() -> phy_sfp_probe() ->
+ * sfp_bus_add_upstream() takes rtnl), and pse_phy_lock() rather than rtnl
+ * guards the attach so a bus registered from ndo_init (which already holds
+ * rtnl) does not recurse on it.
+ *
+ * Return: 0 on success, negative error code on failure.
  */
 int phy_device_register(struct phy_device *phydev)
 {
@@ -1034,12 +1142,25 @@ int phy_device_register(struct phy_device *phydev)
 		goto out;
 	}
 
+	/* Keep the PSE_REGISTERED walk off this phy until registration has
+	 * actually succeeded. device_add() puts the phy on the bus before its
+	 * own later failure points, and its unwind takes it back off, so a
+	 * handle attached in that window would be missed by the
+	 * PSE_UNREGISTERED walk as well and left with no owner.
+	 */
+	phydev->psec_detached = true;
+
 	err = device_add(&phydev->mdio.dev);
 	if (err) {
 		phydev_err(phydev, "failed to add\n");
 		goto out;
 	}
 
+	pse_phy_lock();
+	phydev->psec_detached = false;
+	phy_try_attach_pse(phydev);
+	pse_phy_unlock();
+
 	return 0;
 
  out:
@@ -1061,8 +1182,22 @@ EXPORT_SYMBOL(phy_device_register);
  */
 void phy_device_remove(struct phy_device *phydev)
 {
+	struct pse_control *psec;
+
 	unregister_mii_timestamper(phydev->mii_ts);
-	pse_control_put(phydev->psec);
+
+	/* Detach synchronously, before the phy leaves the bus, so the put cannot
+	 * outlive the PSE controller: an off-bus but still-pinned phy is missed
+	 * by the PSE_UNREGISTERED walk. pse_phy_lock() serialises against that
+	 * walk, and psec_detached keeps the PSE_REGISTERED walk from attaching a
+	 * new handle in the window before device_del() takes the phy off the bus.
+	 */
+	pse_phy_lock();
+	psec = phydev->psec;
+	phydev->psec = NULL;
+	phydev->psec_detached = true;
+	pse_control_put(psec);
+	pse_phy_unlock();
 
 	device_del(&phydev->mdio.dev);
 
@@ -3976,8 +4111,14 @@ static int __init phy_init(void)
 	if (rc)
 		goto err_c45;
 
+	rc = pse_register_notifier(&phy_pse_notifier);
+	if (rc)
+		goto err_genphy;
+
 	return 0;
 
+err_genphy:
+	phy_driver_unregister(&genphy_driver);
 err_c45:
 	phy_driver_unregister(&genphy_c45_driver);
 err_ethtool_phy_ops:
@@ -3994,6 +4135,7 @@ static int __init phy_init(void)
 
 static void __exit phy_exit(void)
 {
+	pse_unregister_notifier(&phy_pse_notifier);
 	phy_driver_unregister(&genphy_c45_driver);
 	phy_driver_unregister(&genphy_driver);
 	rtnl_lock();
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index e560833ad034..bfa025e2194b 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -25,8 +25,55 @@ static LIST_HEAD(pse_controller_list);
 static DEFINE_XARRAY_ALLOC(pse_pw_d_map);
 static DEFINE_MUTEX(pse_pw_d_mutex);
 
+/* Serialises phydev->psec against the PSE controller lifecycle notifier and
+ * the ethtool PSE paths, in place of rtnl. The attach must not take rtnl: an
+ * MDIO bus registered from ndo_init (e.g. lantiq_etop) calls
+ * phy_device_register() with rtnl already held, so taking rtnl for the attach
+ * would deadlock. It lives here rather than in phylib because PSE_CONTROLLER
+ * is bool, so pse_core is always built into vmlinux and net/ethtool can call
+ * these directly; phylib is tristate and must not be linked against from
+ * built-in code. Lock order: rtnl -> pse_phy_mutex -> pse_list_mutex ->
+ * pcdev->lock.
+ */
+static DEFINE_MUTEX(pse_phy_mutex);
+
 static BLOCKING_NOTIFIER_HEAD(pse_controller_notifier);
 
+/**
+ * pse_phy_lock - serialise access to phydev->psec
+ *
+ * Held by the PSE controller lifecycle notifier, by the phy attach and detach
+ * paths and by the ethtool PSE paths. The PSE_UNREGISTERED walk clears
+ * phydev->psec and drops the phy's reference under this lock, so anything that
+ * attaches, detaches or dereferences phydev->psec must hold it across the
+ * whole access.
+ */
+void pse_phy_lock(void)
+{
+	mutex_lock(&pse_phy_mutex);
+}
+EXPORT_SYMBOL_GPL(pse_phy_lock);
+
+/**
+ * pse_phy_unlock - release the lock taken by pse_phy_lock()
+ */
+void pse_phy_unlock(void)
+{
+	mutex_unlock(&pse_phy_mutex);
+}
+EXPORT_SYMBOL_GPL(pse_phy_unlock);
+
+#ifdef CONFIG_LOCKDEP
+/**
+ * pse_phy_lock_assert_held - assert that pse_phy_lock() is held
+ */
+void pse_phy_lock_assert_held(void)
+{
+	lockdep_assert_held(&pse_phy_mutex);
+}
+EXPORT_SYMBOL_GPL(pse_phy_lock_assert_held);
+#endif
+
 /**
  * pse_register_notifier - register a callback for PSE controller events
  * @nb: notifier block to register
@@ -1345,6 +1392,13 @@ void pse_controller_unregister(struct pse_controller_dev *pcdev)
 	 */
 	cancel_work_sync(&pcdev->ntf_work);
 
+	/* Every handle should be gone here: subscribers drop theirs in the
+	 * event above, and the worker's transient one goes with the drain.
+	 * The controller is off the list and the irq is off, so nothing can
+	 * add one. Anything left is a holder nobody accounted for.
+	 */
+	WARN_ON(!list_empty(&pcdev->pse_control_head));
+
 	pse_flush_pw_ds(pcdev);
 	pse_release_pis(pcdev);
 	kfifo_free(&pcdev->ntf_fifo);
@@ -2206,3 +2260,17 @@ bool pse_has_c33(struct pse_control *psec)
 	return psec->pcdev->types & ETHTOOL_PSE_C33;
 }
 EXPORT_SYMBOL_GPL(pse_has_c33);
+
+/**
+ * pse_control_matches_pcdev - Test whether a pse_control targets a controller
+ * @psec: pse_control obtained from of_pse_control_get()
+ * @pcdev: PSE controller to compare against
+ *
+ * Return: %true if @psec was obtained from @pcdev, %false otherwise.
+ */
+bool pse_control_matches_pcdev(struct pse_control *psec,
+			       struct pse_controller_dev *pcdev)
+{
+	return psec->pcdev == pcdev;
+}
+EXPORT_SYMBOL_GPL(pse_control_matches_pcdev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7c5098a0dd6c..ef5b5e6f4e0f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -666,6 +666,12 @@ struct phy_oatc14_sqi_capability {
  * @master_slave_state: Current master/slave configuration
  * @mii_ts: Pointer to time stamper callbacks
  * @psec: Pointer to Power Sourcing Equipment control struct
+ * @psec_detached: Set while phylib will not accept a PSE handle for this
+ *	phy, either because registration has not completed or because it has
+ *	already released @psec, so the PSE_REGISTERED notifier walk skips it.
+ *	Not a bitfield: once the phy is on the bus it is written under
+ *	pse_phy_lock() while other flags are written under phydev->lock or
+ *	rtnl, and they must not share a storage unit
  * @ports: List of PHY ports structures
  * @n_ports: Number of ports currently attached to the PHY
  * @max_n_ports: Max number of ports this PHY can expose
@@ -807,6 +813,7 @@ struct phy_device {
 	struct net_device *attached_dev;
 	struct mii_timestamper *mii_ts;
 	struct pse_control *psec;
+	bool psec_detached;
 
 	struct list_head ports;
 	int n_ports;
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index bc5d36bcd993..16181f8a2c97 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -386,9 +386,23 @@ int pse_ethtool_set_prio(struct pse_control *psec,
 bool pse_has_podl(struct pse_control *psec);
 bool pse_has_c33(struct pse_control *psec);
 
+bool pse_control_matches_pcdev(struct pse_control *psec,
+			       struct pse_controller_dev *pcdev);
+
 int pse_register_notifier(struct notifier_block *nb);
 int pse_unregister_notifier(struct notifier_block *nb);
 
+void pse_phy_lock(void);
+void pse_phy_unlock(void);
+
+#ifdef CONFIG_LOCKDEP
+void pse_phy_lock_assert_held(void);
+#else
+static inline void pse_phy_lock_assert_held(void)
+{
+}
+#endif
+
 #else
 
 static inline struct pse_control *of_pse_control_get(struct device_node *node,
@@ -439,6 +453,12 @@ static inline bool pse_has_c33(struct pse_control *psec)
 	return false;
 }
 
+static inline bool pse_control_matches_pcdev(struct pse_control *psec,
+					     struct pse_controller_dev *pcdev)
+{
+	return false;
+}
+
 static inline int pse_register_notifier(struct notifier_block *nb)
 {
 	return 0;
@@ -449,6 +469,18 @@ static inline int pse_unregister_notifier(struct notifier_block *nb)
 	return 0;
 }
 
+static inline void pse_phy_lock(void)
+{
+}
+
+static inline void pse_phy_unlock(void)
+{
+}
+
+static inline void pse_phy_lock_assert_held(void)
+{
+}
+
 #endif
 
 #endif
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index 757c9e0cc856..654325946aaa 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -71,7 +71,9 @@ static int pse_prepare_data(const struct ethnl_req_info *req_base,
 	if (ret < 0)
 		return ret;
 
+	pse_phy_lock();
 	ret = pse_get_pse_attributes(phydev, info->extack, data);
+	pse_phy_unlock();
 
 	ethnl_ops_complete(dev);
 
@@ -281,9 +283,12 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
 
 	phydev = ethnl_req_get_phydev(req_info, tb, ETHTOOL_A_PSE_HEADER,
 				      info->extack);
+
+	pse_phy_lock();
+
 	ret = ethnl_set_pse_validate(phydev, info);
 	if (ret)
-		return ret;
+		goto out;
 
 	if (tb[ETHTOOL_A_PSE_PRIO]) {
 		unsigned int prio;
@@ -291,7 +296,7 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
 		prio = nla_get_u32(tb[ETHTOOL_A_PSE_PRIO]);
 		ret = pse_ethtool_set_prio(phydev->psec, info->extack, prio);
 		if (ret)
-			return ret;
+			goto out;
 	}
 
 	if (tb[ETHTOOL_A_C33_PSE_AVAIL_PW_LIMIT]) {
@@ -301,7 +306,7 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
 		ret = pse_ethtool_set_pw_limit(phydev->psec, info->extack,
 					       pw_limit);
 		if (ret)
-			return ret;
+			goto out;
 	}
 
 	/* These values are already validated by the ethnl_pse_set_policy */
@@ -319,10 +324,11 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
 		 */
 		ret = pse_ethtool_set_config(phydev->psec, info->extack,
 					     &config);
-		if (ret)
-			return ret;
 	}
 
+out:
+	pse_phy_unlock();
+
 	/* Return errno or zero - PSE has no notification */
 	return ret;
 }
-- 
2.43.0