[PATCH net-next 0/4] net: pse-pd: core fixes ahead of the controller notifier

[PATCH net-next 0/4] net: pse-pd: core fixes ahead of the controller notifier

[Carlo Szelinsky]

These four PSE core fixes were part of the "decouple controller lookup
from MDIO probe" series [1]; Oleksij asked to split them out. They do
not need phylib, and each one fixes a problem on its own:

 1. Unregister frees the PI data while the controller can still be
    found. Take it off the list and stop the IRQ and the worker first.
 2. A failed registration leaks memory, and can leave freed power
    domains in the global map. Clean up on the error paths.
 3. si3474: use dev_err_probe(), so the deferral from 4 is not logged
    as an error.
 4. Defer the PSE driver's probe until every PI's vpwr supply is there.

Patch 1 conflicts with the pending net series [2]; keep the order from
patch 1. Patch 2 conflicts with two of the net fixes posted alongside
[3]; the note below its --- explains how to merge them.

Tested in QEMU on arm64 with KASAN, lockdep and kmemleak: controllers
whose supply never appears defer, and an injected pse_register_pw_ds()
failure is cleaned up and retried. The only KASAN reports are the known
stale-handle bug at MDIO bus unbind, the same without this series; the
decouple series fixes it. This series, the decouple series and the net
fixes together also run on a Hasivo S600WP (hs104), with 200 controller
unbind/bind cycles and no warnings or leaks. Every patch builds with
W=1, and PHYLIB=m, PHYLIB=n and PSE_CONTROLLER=n link.

[1] https://lore.kernel.org/netdev/20261004164219.1161294-1-github@szelinsky.de/
[2] https://lore.kernel.org/netdev/20260813200653.980170-1-github@szelinsky.de/
[3] "net: pse-pd: assorted lifetime and error-path fixes", for net

Carlo Szelinsky (4):
  net: pse-pd: unlink the controller before freeing its PI data
  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

 drivers/net/pse-pd/pse_core.c | 175 ++++++++++++++++++++++++++++++----
 drivers/net/pse-pd/si3474.c   |   7 +-
 2 files changed, 158 insertions(+), 24 deletions(-)


base-commit: 98c2ad4f5d805dbc1ea436a334ee59311806a48c
-- 
2.43.0

[PATCH net-next 1/4] net: pse-pd: unlink the controller before freeing its PI data

[Carlo Szelinsky]

pse_controller_unregister() frees the PI array before it takes the
controller off pse_controller_list. In that window of_pse_control_get()
can still find the controller and read the freed array, and so can
pse_isr() and the notification worker.

Disable the IRQ, take the controller off the list and stop the worker
first. Only then free the PI data.

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

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index a5e6d7b26b9f..364acc025097 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -1114,15 +1114,19 @@ EXPORT_SYMBOL_GPL(pse_controller_register);
  */
 void pse_controller_unregister(struct pse_controller_dev *pcdev)
 {
-	pse_flush_pw_ds(pcdev);
-	pse_release_pis(pcdev);
+	/* Stop the IRQ, new lookups and the worker before freeing pcdev->pi. */
 	if (pcdev->irq)
 		disable_irq(pcdev->irq);
-	cancel_work_sync(&pcdev->ntf_work);
-	kfifo_free(&pcdev->ntf_fifo);
+
 	mutex_lock(&pse_list_mutex);
 	list_del(&pcdev->list);
 	mutex_unlock(&pse_list_mutex);
+
+	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

Re: [PATCH net-next 1/4] net: pse-pd: unlink the controller before freeing its PI data

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Incomplete fix: the PI regulators still use pcdev->pi after it
  is freed.
- [High] Incomplete fix: pse_control handles that consumers already hold
  keep using pcdev->pi, and pcdev, after unregister.
- [High] Race: ntf_work can be re-queued after cancel_work_sync(), so
  'stop the worker first' does not hold.
- [Low] Missing Fixes: tag.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but
  devm_pse_irq_helper() and pse_controller_unregister() disagree about…

--

net: pse-pd: unlink the controller before freeing its PI data

This commit reorders pse_controller_unregister(). The IRQ is now disabled,
the controller is taken off pse_controller_list and the notification
worker is cancelled before the PI array, the power domains and the kfifo
are freed. The goal is to stop of_pse_control_get(), pse_isr() and the
worker from reading the freed PI array.

> Signed-off-by: Carlo Szelinsky <github@szelinsky.de>

[Severity: Low]
Should this have a Fixes: tag? Before this patch,
of_pse_control_get() -> of_pse_match_pi() can read the freed pcdev->pi
while the controller is still on pse_controller_list.

pse_release_pis() was placed before list_del() by 9be9567a7c59 ("net:
pse-pd: Add support for PSE PIs"). The IRQ and worker teardown that runs
after the free came from ffef61d6d273. pse_flush_pw_ds() came from
50f8b341d268.

> diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
> index a5e6d7b26b9fd..364acc0250971 100644
> --- a/drivers/net/pse-pd/pse_core.c
> +++ b/drivers/net/pse-pd/pse_core.c
> @@ -1114,15 +1114,19 @@ EXPORT_SYMBOL_GPL(pse_controller_register);
>   */
>  void pse_controller_unregister(struct pse_controller_dev *pcdev)
>  {
> -	pse_flush_pw_ds(pcdev);
> -	pse_release_pis(pcdev);
> +	/* Stop the IRQ, new lookups and the worker before freeing pcdev->pi. */
>  	if (pcdev->irq)
>  		disable_irq(pcdev->irq);

[Severity: Medium]
This isn't a bug introduced by this patch, but is the IRQ passed to
disable_irq() here always one the controller owns?

devm_pse_irq_helper() records the IRQ even when the request fails:

drivers/net/pse-pd/pse_core.c:devm_pse_irq_helper() {
    ...
	ret = devm_request_threaded_irq(dev, irq, NULL, pse_isr,
					IRQF_ONESHOT | irq_flags,
					irq_name, h);
	if (ret)
		dev_err(pcdev->dev, "Failed to request IRQ %d\n", irq);

	pcdev->irq = irq;
	return ret;
}

Say the request in tps23881_setup_irq() fails with -EBUSY because another
device already owns the line. tps23881 probe then fails and devres runs:

devm_pse_controller_release()
  pse_controller_unregister()
    disable_irq(pcdev->irq)

Could this leave the other device's interrupt masked, with no matching
enable_irq()?

In the normal devm flow, the IRQ is requested after the controller is
registered. Devres therefore frees it before pse_controller_unregister()
runs, and disable_irq() acts on a line that is already freed. That is
harmless for a non-shared IRQ.

Would it leave the line disabled for the other sharers if a driver passed
IRQF_SHARED?

> -	cancel_work_sync(&pcdev->ntf_work);
> -	kfifo_free(&pcdev->ntf_fifo);
> +
>  	mutex_lock(&pse_list_mutex);
>  	list_del(&pcdev->list);
>  	mutex_unlock(&pse_list_mutex);

[Severity: High]
Unlinking from pse_controller_list blocks new of_pse_control_get()
lookups. What about the pse_control handles that consumers already hold?

A PHY that got phydev->psec earlier keeps psec->pcdev and psec->id.

pse_controller_unregister() does not walk or invalidate
pcdev->pse_control_head. It also does not take pcdev->lock to keep out
operations already in progress.

pse_control_get_internal() only pins the module with try_module_get(),
and that does not prevent a sysfs unbind.

After an unbind, can an ETHTOOL_MSG_PSE_GET request still reach the freed
array through the stale handle?

pse_get_pse_attributes()
  pse_ethtool_get_status(phydev->psec, ...)
    pi = &pcdev->pi[psec->id];
    ...
    if (pi->pw_d) {
        status->pw_d_id = pi->pw_d->id;

pse_get_sw_admin_state() reads the same array.

pse_ethtool_c33_set_config(), pse_ethtool_podl_set_config(),
pse_ethtool_set_pw_limit() and pse_ethtool_set_prio() read and write
pcdev->pi[psec->id]. They also call regulator_enable() and
regulator_disable().

The final put from the PHY looks affected as well:

phy_device_remove()
  pse_control_put(phydev->psec)
    __pse_control_release()
      if (psec->pcdev->pi[psec->id].admin_state_enabled)
          regulator_disable(psec->ps);
      ...
      module_put(psec->pcdev->owner);

Once the remaining devres entries run, pcdev itself is gone too, since it
is embedded in the driver's devm-allocated private data.

This lifetime problem also exists without this patch. However, the commit
message says "Only then free the PI data", which suggests the reorder
makes the free safe.

Would a complete fix need to revoke and drain the existing controls, or
keep the PI data alive until they are released? Nothing later in the
series seems to change this.

> +
> +	cancel_work_sync(&pcdev->ntf_work);

[Severity: High]
Can ntf_work be queued again after this cancel_work_sync()?

pse_isr() is not the only producer. pse_disable_pi_pol() runs in process
context under pcdev->lock and does:

drivers/net/pse-pd/pse_core.c:pse_disable_pi_pol() {
    ...
	kfifo_in_spinlocked(&pcdev->ntf_fifo, &ntf, 1, &pcdev->ntf_fifo_lock);
	schedule_work(&pcdev->ntf_work);
    ...
}

disable_irq() does not stop this. Take a controller that uses
PSE_BUDGET_EVAL_STRAT_STATIC with software power control, such as
tps23881. There, the regulator op, which is still registered, can reach
it:

pse_pi_enable()
  _pse_pi_delivery_power_sw_pw_ctrl()
    pse_pi_allocate_pw_budget()
      pse_pi_allocate_pw_budget_static_prio()
        pse_disable_pi_prio()
          pse_disable_pi_pol()

An existing psec can also reach it, through either of these:

pse_ethtool_set_prio() -> pse_pw_d_retry_power_delivery()
  -> _pse_pi_delivery_power_sw_pw_ctrl()

_pse_pi_disable() -> pse_pw_d_retry_power_delivery()

pse_controller_unregister() never takes pcdev->lock and never waits for
these callers. Is this interleaving possible?

CPU1 (unbind)                       CPU0 (pse_pi_enable() or set-prio)
pse_controller_unregister()
  disable_irq()
  list_del()
  cancel_work_sync()
                                    mutex_lock(&pcdev->lock)
                                    ...
                                    pse_disable_pi_pol()
                                      kfifo_in_spinlocked()
                                      schedule_work(&pcdev->ntf_work)
  pse_release_pis()
  kfifo_free()

The work would then be pending on a pcdev whose pi array is freed. The
devm private data that contains pcdev is freed shortly after.

If the worker's pse_control_put() drops the last reference,
__pse_control_release() reads psec->pcdev->pi[] from freed memory.

This looks like the worker case the commit message says is fixed. Nothing
later in the series adds a dead-state check or locking here.

> +
> +	pse_flush_pw_ds(pcdev);
> +	pse_release_pis(pcdev);
> +	kfifo_free(&pcdev->ntf_fifo);

[Severity: High]
Can the PI regulators still use pcdev->pi after pse_release_pis() frees
it here?

devm_pse_pi_regulator_register() registers each PI regulator with
devm_regulator_register(pcdev->dev, ...) from inside
pse_controller_register(). devm_pse_controller_register() adds its own
devres entry only after that call returns:

drivers/net/pse-pd/pse_core.c:devm_pse_controller_register() {
    ...
	ret = pse_controller_register(pcdev);
	...
	*pcdevp = pcdev;
	devres_add(dev, pcdevp);
    ...
}

Devres releases entries in LIFO order. So on unbind or rmmod,
devm_pse_controller_release() -> pse_controller_unregister() frees
pcdev->pi first, and the regulators are unregistered after that.

In that window, any user can reach pse_pi_is_enabled() by reading the
regulator's world-readable sysfs state attribute
(state_show() -> ops->is_enabled). It reads:

	if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[id].pw_d)) {
		ret = pcdev->pi[id].admin_state_enabled;

pse_pi_enable() and pse_pi_disable() also write
pcdev->pi[id].admin_state_enabled. That happens when a consumer holding
psec->ps calls regulator_enable() or regulator_disable().

The next patch in the series, "net: pse-pd: unwind allocations when
controller registration fails", says the regulator ops use the PI array
until devres unregisters them. Yet pse_controller_unregister() still
frees the array before that point, and nothing later in the series
changes this.

Should the free move to after the regulators are unregistered, or should
the regulators be torn down first?

>  }
>  EXPORT_SYMBOL_GPL(pse_controller_unregister);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018081853.47484-1-github%40szelinsky.de

[PATCH net-next 2/4] net: pse-pd: unwind allocations when controller registration fails

[Carlo Szelinsky]

When pse_controller_register() fails, it leaks the notification kfifo,
the PI array and the OF node references. Worse, if pse_register_pw_ds()
fails part way, the power domains it created stay in the global map
while devres frees them, so the next controller that registers reads
freed memory. A later patch makes -EPROBE_DEFER a normal result here,
so this will happen more often.

Free what was allocated on each error path. The PI array is still
leaked once the PI regulators are registered, because their ops use it
until devres unregisters them.

pse_register_pw_ds() now undoes its own power domains while it still
holds pse_pw_d_mutex, so no other controller can pick one up just
before it is freed. pi[].pw_d is cleared under pcdev->lock before its
reference is dropped, and pse_ethtool_set_prio() and
pse_pi_update_pw_budget() now check it under that lock.

A power domain shared by two controllers is still devm memory of the
controller that created it; that is fixed separately on net.

Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---

Notes:
    Merging with the net fixes:
    - "net: pse-pd: do not tie a shared power domain to its creator's
      devres": move its kfree(pw_d) to the end of
      pse_pw_d_release_locked(), and name that function in its
      pse_alloc_pw_d() comment. In pse_flush_pw_ds(), keep this patch's
      pse_pi_take_pw_d() call.
    - "net: pse-pd: check the PI state under the lock when setting a power
      limit": in pse_pi_update_pw_budget(), keep the net fix's check.

 drivers/net/pse-pd/pse_core.c | 82 ++++++++++++++++++++++++++++-------
 1 file changed, 66 insertions(+), 16 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 364acc025097..05b02d736931 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -878,7 +878,8 @@ devm_pse_pi_regulator_register(struct pse_controller_dev *pcdev,
 	return 0;
 }
 
-static void __pse_pw_d_release(struct kref *kref)
+/* Release for a put made with pse_pw_d_mutex already held. */
+static void pse_pw_d_release_locked(struct kref *kref)
 {
 	struct pse_power_domain *pw_d = container_of(kref,
 						     struct pse_power_domain,
@@ -886,9 +887,30 @@ static void __pse_pw_d_release(struct kref *kref)
 
 	regulator_put(pw_d->supply);
 	xa_erase(&pse_pw_d_map, pw_d->id);
+}
+
+static void __pse_pw_d_release(struct kref *kref)
+{
+	pse_pw_d_release_locked(kref);
 	mutex_unlock(&pse_pw_d_mutex);
 }
 
+/* Clear pi[id].pw_d before its reference is dropped. The PI regulator
+ * ops hold pcdev->lock while they use it.
+ */
+static struct pse_power_domain *
+pse_pi_take_pw_d(struct pse_controller_dev *pcdev, int id)
+{
+	struct pse_power_domain *pw_d;
+
+	mutex_lock(&pcdev->lock);
+	pw_d = pcdev->pi[id].pw_d;
+	pcdev->pi[id].pw_d = NULL;
+	mutex_unlock(&pcdev->lock);
+
+	return pw_d;
+}
+
 /**
  * pse_flush_pw_ds - flush all PSE power domains of a PSE
  * @pcdev: a pointer to the initialized PSE controller device
@@ -899,10 +921,11 @@ static void pse_flush_pw_ds(struct pse_controller_dev *pcdev)
 	int i;
 
 	for (i = 0; i < pcdev->nr_lines; i++) {
-		if (!pcdev->pi[i].pw_d)
+		pw_d = pse_pi_take_pw_d(pcdev, i);
+		if (!pw_d)
 			continue;
 
-		pw_d = xa_load(&pse_pw_d_map, pcdev->pi[i].pw_d->id);
+		pw_d = xa_load(&pse_pw_d_map, pw_d->id);
 		if (!pw_d)
 			continue;
 
@@ -998,6 +1021,13 @@ static int pse_register_pw_ds(struct pse_controller_dev *pcdev)
 	}
 
 out:
+	/* Undo this call's domains before another controller can find them. */
+	while (ret && i--) {
+		struct pse_power_domain *pw_d = pse_pi_take_pw_d(pcdev, i);
+
+		if (pw_d)
+			kref_put(&pw_d->refcnt, pse_pw_d_release_locked);
+	}
 	mutex_unlock(&pse_pw_d_mutex);
 	return ret;
 }
@@ -1058,17 +1088,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 +
@@ -1076,7 +1107,9 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 	 */
 	reg_name_len = strlen(dev_name(pcdev->dev)) + 18;
 
-	/* Register PI regulators */
+	/* Register PI regulators. Their ops index pcdev->pi[] until devres
+	 * unregisters them, so failures from here on must not free it.
+	 */
 	for (i = 0; i < pcdev->nr_lines; i++) {
 		char *reg_name;
 
@@ -1085,26 +1118,38 @@ 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;
+		goto free_kfifo;
 
 	mutex_lock(&pse_list_mutex);
 	list_add(&pcdev->list, &pse_controller_list);
 	mutex_unlock(&pse_list_mutex);
 
 	return 0;
+
+release_pis:
+	pse_release_pis(pcdev);
+	pcdev->pi = NULL;
+
+free_kfifo:
+	cancel_work_sync(&pcdev->ntf_work);
+	kfifo_free(&pcdev->ntf_fifo);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(pse_controller_register);
 
@@ -1820,6 +1865,10 @@ static int pse_pi_update_pw_budget(struct pse_controller_dev *pcdev, int id,
 	 * power budget update
 	 */
 	mutex_lock(&pcdev->lock);
+	/* pi->pw_d may have been cleared since the caller looked. */
+	if (!pi->pw_d)
+		goto out;
+
 	previous_pw_allocated = pi->pw_allocated_mW;
 	pw_diff = pw_req - previous_pw_allocated;
 	if (!pw_diff) {
@@ -1921,15 +1970,16 @@ int pse_ethtool_set_prio(struct pse_control *psec,
 	const struct pse_controller_ops *ops;
 	int ret = 0;
 
-	if (!pcdev->pi[psec->id].pw_d) {
-		NL_SET_ERR_MSG(extack, "no power domain attached");
-		return -EOPNOTSUPP;
-	}
-
 	/* We don't want priority change in the middle of an
 	 * enable/disable call or a priority mode change
 	 */
 	mutex_lock(&pcdev->lock);
+	if (!pcdev->pi[psec->id].pw_d) {
+		NL_SET_ERR_MSG(extack, "no power domain attached");
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
 	switch (pcdev->pi[psec->id].pw_d->budget_eval_strategy) {
 	case PSE_BUDGET_EVAL_STRAT_STATIC:
 		if (prio >= pcdev->nr_lines) {
-- 
2.43.0

Re: [PATCH net-next 2/4] net: pse-pd: unwind allocations when controller registration fails

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In pse_controller_register(), every failure after the PI
  regulator loop starts goes to free_kfifo, which skips…
- [Medium] The patch makes pcdev->lock the rule for pi[].pw_d:
  pse_pi_take_pw_d() clears it under the lock before the reference is…
- [Low] The commit fixes a real use-after-free that exists before this
  patch, but it carries no Fixes: tag.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the normal
  unregister path breaks the lifetime rule the patch states.
- [High] This problem wasn't introduced by this patch, but a PSE power
  domain shared by several controllers is still allocated with…
- [Medium] This problem wasn't introduced by this patch, but the driver's
  setup_pi_matrix() side effects are never undone when…

--

net: pse-pd: unwind allocations when controller registration fails

This adds error unwinding to pse_controller_register(), so the
notification kfifo, the PI array and the OF node references are released
when registration fails. pse_register_pw_ds() now undoes its own power
domains under pse_pw_d_mutex when it fails part way. pi[].pw_d is now
cleared under pcdev->lock, and the pw_d checks in pse_ethtool_set_prio()
and pse_pi_update_pw_budget() are done under that lock.

> diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
> index 364acc0250971..05b02d736931c 100644
> --- a/drivers/net/pse-pd/pse_core.c
> +++ b/drivers/net/pse-pd/pse_core.c

[ ... ]

> @@ -899,10 +921,11 @@ static void pse_flush_pw_ds(struct pse_controller_dev *pcdev)
>  	int i;
>  
>  	for (i = 0; i < pcdev->nr_lines; i++) {
> -		if (!pcdev->pi[i].pw_d)
> +		pw_d = pse_pi_take_pw_d(pcdev, i);
> +		if (!pw_d)
>  			continue;
>  
> -		pw_d = xa_load(&pse_pw_d_map, pcdev->pi[i].pw_d->id);
> +		pw_d = xa_load(&pse_pw_d_map, pw_d->id);
>  		if (!pw_d)
>  			continue;
>  

[Severity: High]
This is a pre-existing issue, but it sits next to this teardown code.
devm_pse_alloc_pw_d() still allocates the power domain against the
controller that creates it:

	pw_d = devm_kzalloc(dev, sizeof(*pw_d), GFP_KERNEL);

Other controllers then borrow it in pse_register_pw_ds() through
regulator_is_equal() and kref_get().

Say controller A creates the domain and controller B borrows it. When A
unbinds, pse_flush_pw_ds() drops only A's reference (2 -> 1), so
pse_pw_d_release_locked() and xa_erase() don't run. A's devres then frees
the pw_d memory while it is still in pse_pw_d_map and still referenced
from B's pcdev->pi[].pw_d.

Can B then read freed memory from pse_ethtool_get_status(),
pse_ethtool_set_prio(), pse_ethtool_set_pw_limit(), its PI regulator ops
or pse_isr()? Two other paths look affected too:

- A later controller's pse_register_pw_ds() calls
  regulator_is_equal(pw_d->supply, ...) on it.
- B's own pse_flush_pw_ds() or unwind kref_put() writes to it.

The commit message says this is fixed separately on net. That fix is not
in this tree or in this series.

[ ... ]

> @@ -998,6 +1021,13 @@ static int pse_register_pw_ds(struct pse_controller_dev *pcdev)
>  	}
>  
>  out:
> +	/* Undo this call's domains before another controller can find them. */
> +	while (ret && i--) {
> +		struct pse_power_domain *pw_d = pse_pi_take_pw_d(pcdev, i);
> +
> +		if (pw_d)
> +			kref_put(&pw_d->refcnt, pse_pw_d_release_locked);
> +	}
>  	mutex_unlock(&pse_pw_d_mutex);
>  	return ret;
>  }

[Severity: Low]
This fixes a use-after-free that exists before this patch. Before, a
failure of devm_pse_alloc_pw_d() or regulator_get() at PI i jumped to out
and only dropped pse_pw_d_mutex.

The domains created for PIs 0..i-1 stayed in pse_pw_d_map. Their
regulator_get() and kref_get() references were never dropped, and devres
then freed them. The next pse_register_pw_ds() call then walks freed
memory in xa_for_each() and regulator_is_equal().

The kfifo leak on the early error returns, and the PI and OF node leak
after a setup_pi_matrix() failure, are older too.

Should this patch carry a Fixes: tag, at least for the commit that added
PSE power domains and pse_register_pw_ds()? Without one, stable tooling
won't pick up the use-after-free fix. The commit message also says the
related shared-domain fix goes to net, while this one does not.

> @@ -1058,17 +1088,18 @@ int pse_controller_register(struct pse_controller_dev *pcdev)

[ ... ]

>  	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 +
> @@ -1076,7 +1107,9 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
>  	 */
>  	reg_name_len = strlen(dev_name(pcdev->dev)) + 18;
>  
> -	/* Register PI regulators */
> +	/* Register PI regulators. Their ops index pcdev->pi[] until devres
> +	 * unregisters them, so failures from here on must not free it.
> +	 */

[Severity: High]
This is a pre-existing issue, but does the normal unregister path follow
the rule in this comment?

In devm_pse_controller_register(), the PI regulators' devres entries are
added inside pse_controller_register(). The controller's own entry is
added after that:

	ret = pse_controller_register(pcdev);
	...
	*pcdevp = pcdev;
	devres_add(dev, pcdevp);

Devres releases in LIFO order. On unbind, devm_pse_controller_release()
-> pse_controller_unregister() therefore runs before devm_rdev_release()
for the PI regulators:

	pse_flush_pw_ds(pcdev);
	pse_release_pis(pcdev);
	kfifo_free(&pcdev->ntf_fifo);

pse_release_pis() calls kfree(pcdev->pi), but the PI regulators are
still registered with driver_data = pcdev.

Can a read of the regulator sysfs state attribute in that window reach
pse_pi_is_enabled() and index the freed pcdev->pi[id]? Two other paths
look like they could do the same:

- flush_work(&rdev->disable_work) in regulator_unregister().
- Ethtool callers that still hold a psec.

The new pse_pi_take_pw_d() call in pse_flush_pw_ds() expects PI
regulator ops to run during teardown. The array those ops index is freed
right after it.

>  	for (i = 0; i < pcdev->nr_lines; i++) {
>  		char *reg_name;
>  
> @@ -1085,26 +1118,38 @@ 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;
> +		goto free_kfifo;

[Severity: Medium]
Do these goto free_kfifo paths leak the OF node references held in
pcdev->pi[]?

Three failures skip pse_release_pis(): the reg_name allocation, the
devm_pse_pi_regulator_register() call and the pse_register_pw_ds() call.
pse_release_pis() is the only place that drops pi[i].np (taken with
of_node_get() in of_load_pse_pis()) and pi[i].pairset[0/1].np (taken with
of_parse_phandle()):

	of_node_put(pcdev->pi[i].pairset[0].np);
	of_node_put(pcdev->pi[i].pairset[1].np);
	of_node_put(pcdev->pi[i].np);

On failure, devm_pse_controller_register() calls devres_free(pcdevp), so
nothing drops these references later.

The commit message says the OF node reference leak is fixed, and names
only the PI array as still leaked. The reason given for keeping pi[]
doesn't seem to cover these references:

- regulator_register() takes its own reference on the PI node.
- The pairset nodes are only used by setup_pi_matrix(), which has
  already run.
- A failed controller is never added to pse_controller_list.

Could the node references be put and the pointers cleared without
freeing the array? Also, if the first reg_name allocation fails, no PI
regulator exists yet. Could the array itself be freed in that case?

>  
>  	mutex_lock(&pse_list_mutex);
>  	list_add(&pcdev->list, &pse_controller_list);
>  	mutex_unlock(&pse_list_mutex);
>  
>  	return 0;
> +
> +release_pis:
> +	pse_release_pis(pcdev);
> +	pcdev->pi = NULL;
> +
> +free_kfifo:
> +	cancel_work_sync(&pcdev->ntf_work);
> +	kfifo_free(&pcdev->ntf_fifo);
> +
> +	return ret;
>  }

[Severity: Medium]
This is a pre-existing issue, but none of the error paths after a
successful setup_pi_matrix() undo what the driver did in that callback.

For pd692x0, pd692x0_setup_pi_matrix() reserves budget on each manager's
upstream supply in pd692x0_req_managers_pw_budget():

	ret = regulator_request_power_budget(supply, pw_budget);

If a later step in pse_controller_register() fails, pd692x0_i2c_probe()
just returns the error from devm_pse_controller_register().
pd692x0_managers_free_pw_budget() is only called from the error labels in
setup_pi_matrix() and from pd692x0_i2c_remove(). Remove does not run
after a failed probe.

Does this leave the reservation on the supply's regulator_dev, so the
next probe attempt sees less budget, possibly none? pse_controller_ops
has no callback to undo setup_pi_matrix(), so the core can't roll this
back by itself.

>  EXPORT_SYMBOL_GPL(pse_controller_register);
>  
> @@ -1820,6 +1865,10 @@ static int pse_pi_update_pw_budget(struct pse_controller_dev *pcdev, int id,
>  	 * power budget update
>  	 */
>  	mutex_lock(&pcdev->lock);
> +	/* pi->pw_d may have been cleared since the caller looked. */
> +	if (!pi->pw_d)
> +		goto out;
> +

[Severity: Medium]
This recheck covers pse_pi_update_pw_budget() itself. Doesn't
pse_ethtool_set_pw_limit() still read pw_d and the PI state without
pcdev->lock before it calls this function?

	if (pse_pw_d_is_sw_pw_control(psec->pcdev,
				      psec->pcdev->pi[psec->id].pw_d) &&
	    psec->pcdev->pi[psec->id].admin_state_enabled &&
	    psec->pcdev->pi[psec->id].isr_pd_detected) {
		ret = pse_pi_update_pw_budget(psec->pcdev, psec->id,
					      pw_limit, extack);

pse_pw_d_is_sw_pw_control() dereferences pw_d->budget_eval_strategy. A
concurrent pse_controller_unregister() -> pse_flush_pw_ds() can clear
pw_d through pse_pi_take_pw_d() and drop the last reference between that
unlocked load and the dereference.

There is a second race that needs no unbind. pse_isr() ->
pse_set_config_isr() can release the PI's budget and clear
isr_pd_detected under pcdev->lock after the unlocked checks above.
pse_pi_update_pw_budget() then finds pw_d non-NULL and requests budget
again for a PI that is disconnected.

Should admin_state_enabled and isr_pd_detected be rechecked here as
well, or should the whole check in pse_ethtool_set_pw_limit() move under
the lock?

The merge notes mention a net fix, "check the PI state under the lock
when setting a power limit", which may cover this. That fix is not in
this tree or in this series.

>  	previous_pw_allocated = pi->pw_allocated_mW;
>  	pw_diff = pw_req - previous_pw_allocated;
>  	if (!pw_diff) {

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018081853.47484-1-github%40szelinsky.de

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

[Carlo Szelinsky]

The next patch makes devm_pse_controller_register() return
-EPROBE_DEFER until a PI's vpwr supply is there. Use dev_err_probe(), as
pd692x0 and tps23881 already do, so a deferral is not logged as an error
on every retry.

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 4/4] net: pse-pd: check the PI vpwr supply before registering the controller

[Carlo Szelinsky]

A regulator registers even when its supply is not there yet. So a PSE
controller registers while its vpwr supply is missing, and every PI
lookup returns -EPROBE_DEFER until the supply appears. Today the MDIO
bus owner keeps deferring until then, but a consumer that only looks a
PI up when a controller registers, as phylib will with the PSE
notifier, would never get it.

Check every PI's vpwr supply before the PI regulators are registered,
and defer the PSE driver's probe if one is missing. The check runs
after setup_pi_matrix(), because pd692x0 registers the supply
regulators there.

The check is simpler than the regulator core's: a supply that is
registered, but whose device is not bound yet, still passes, and that
PI then gets no power domain. On the other hand, a PI whose supply
appears late now gets a power domain; before, pse_register_pw_ds()
skipped it.

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

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 05b02d736931..44986749600e 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -11,6 +11,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>
@@ -826,6 +827,65 @@ static const struct regulator_ops pse_pi_ops = {
 	.set_current_limit = pse_pi_set_current_limit,
 };
 
+/* Look up PI @id's vpwr supply on its own node, as the regulator core
+ * will. Only if the PI node has a vpwr-supply phandle: otherwise the
+ * lookup falls back to of_get_child_regulator() on the controller node,
+ * which can return a sibling PI's supply.
+ *
+ * Return: 0 if this PI needs nothing more, 1 if the controller node has
+ *	   to answer for it, or a negative error such as -EPROBE_DEFER.
+ */
+static int pse_pi_check_supply(struct pse_controller_dev *pcdev, int id)
+{
+	struct device_node *np;
+	struct regulator *supply;
+	int ret;
+
+	/* The PIs the registration loop skips. */
+	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);
+	if (ret == -ENODEV)
+		return 1;
+
+	return dev_err_probe(pcdev->dev, ret,
+			     "PI %d: failed to get vpwr supply\n", id);
+}
+
+/* The core's fallback: a vpwr-supply on the controller node. */
+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)
+		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)
@@ -1068,6 +1128,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;
 
@@ -1102,6 +1163,26 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 			goto release_pis;
 	}
 
+	/* Check vpwr supplies while pcdev->pi can still be freed. Not before
+	 * setup_pi_matrix(): pd692x0 registers the regulators its PIs name
+	 * there.
+	 */
+	for (i = 0; i < pcdev->nr_lines; i++) {
+		ret = pse_pi_check_supply(pcdev, i);
+		if (ret < 0)
+			goto release_pis;
+
+		/* Same answer for every PI: ask once. */
+		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

Re: [PATCH net-next 4/4] net: pse-pd: check the PI vpwr supply before registering the controller

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new check in pse_pi_check_supply() and
  pse_controller_check_supply() is weaker than what the regulator core…
- [Medium] The new check loop in pse_controller_register() can fail
  (typically with -EPROBE_DEFER) after pcdev->ops->setup_pi_matrix() has…
- [Low] The comment above pse_controller_check_supply() says the core's
  fallback is 'a vpwr-supply on the controller node'.

--

net: pse-pd: check the PI vpwr supply before registering the controller

This commit adds a check to pse_controller_register(). After
setup_pi_matrix() and before the PI regulators are registered, it looks
up each PI's vpwr supply, or the controller's fallback vpwr supply. If a
supply is missing, the PSE driver's probe is deferred, so that a consumer
that looks a PI up when the controller registers does not get
-EPROBE_DEFER.

> diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
> index 05b02d736931c..44986749600e3 100644
> --- a/drivers/net/pse-pd/pse_core.c
> +++ b/drivers/net/pse-pd/pse_core.c

[ ... ]

> @@ -826,6 +827,65 @@ static const struct regulator_ops pse_pi_ops = {
>  	.set_current_limit = pse_pi_set_current_limit,
>  };
>  

[ ... ]

> +static int pse_pi_check_supply(struct pse_controller_dev *pcdev, int id)
> +{

[ ... ]

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

[Severity: Medium]

Can this check pass even though resolving the PI's supply later still
defers?

of_regulator_get_optional() and regulator_get_optional() both go through
_regulator_get_common(). That calls regulator_resolve_supply() on the
supply rdev itself, which only resolves the supply's own upstream supply.
So the check never asks whether the supply's parent device is bound.
pse_controller_check_supply() accepts the supply under the same
condition.

When devm_pse_pi_regulator_register() registers the PI regulator,
regulator_resolve_supply() on the PI rdev does run that bound check:

drivers/regulator/core.c:regulator_resolve_supply() {
    ...
	if (r->dev.parent && r->dev.parent != rdev->dev.parent) {
		if (!device_is_bound(r->dev.parent)) {
			put_device(&r->dev);
			ret = -EPROBE_DEFER;
			goto out;
		}
	}
    ...
}

regulator_register() ignores that error, which leaves the PI rdev with
rdev->supply == NULL. pse_register_pw_ds() then skips the PI, and the
controller is still added to pse_controller_list.

A PI lookup in that window still returns -EPROBE_DEFER:

pse_control_get_internal()
  regulator_get_exclusive()
    _regulator_get_common()
      regulator_resolve_supply()

The window opens when the supplier driver has called regulator_register()
but its probe has not returned yet. Examples are async probing,
fw_devlink=permissive or off, or a supplier probe that registers the
regulator and then defers. regulator_put() runs right after the check, so
nothing holds the supplier in place between the check and PI
registration.

The commit message says:

  a supply that is registered, but whose device is not bound yet, still
  passes, and that PI then gets no power domain.

In this case, wouldn't PI lookups also keep returning -EPROBE_DEFER after
the controller has registered? That looks like the failure the commit
message gives as its motivation, for a consumer such as the planned
phylib PSE notifier that only looks a PI up when a controller registers.

Also, pse_register_pw_ds() only runs at registration. Would that PI then
never get a power domain?

> +
> +	ret = PTR_ERR(supply);
> +	if (ret == -ENODEV)
> +		return 1;
> +
> +	return dev_err_probe(pcdev->dev, ret,
> +			     "PI %d: failed to get vpwr supply\n", id);
> +}
> +
> +/* The core's fallback: a vpwr-supply on the controller node. */

[Severity: Low]

This isn't a bug, but is this comment accurate? The lookup goes
regulator_get_optional(pcdev->dev, "vpwr")->regulator_dev_lookup()->
regulator_dt_lookup()->of_get_regulator(). If the controller node has no
vpwr-supply property, of_get_regulator() does this:

	regnode = of_get_child_regulator(dev->of_node, prop_name);

That walks every descendant node, including sibling pse-pi nodes, before
it falls back to the supply map and the lookup by name.

This matches what the regulator core does. However, the comment on
pse_pi_check_supply() names the sibling PI fallback as the hazard it
avoids.

Could this comment say that the fallback also walks the controller's
child nodes?

> +static int pse_controller_check_supply(struct pse_controller_dev *pcdev)
> +{

[ ... ]

> @@ -1102,6 +1163,26 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
>  			goto release_pis;
>  	}
>  
> +	/* Check vpwr supplies while pcdev->pi can still be freed. Not before
> +	 * setup_pi_matrix(): pd692x0 registers the regulators its PIs name
> +	 * there.
> +	 */
> +	for (i = 0; i < pcdev->nr_lines; i++) {
> +		ret = pse_pi_check_supply(pcdev, i);
> +		if (ret < 0)
> +			goto release_pis;

[Severity: Medium]

Does this new exit, and the pse_controller_check_supply() exit just
below it, leak the vmain power budget that pd692x0 reserved in
setup_pi_matrix()?

By this point pd692x0_setup_pi_matrix() has already returned success,
after this call chain:

pd692x0_setup_pi_matrix()
  pd692x0_req_managers_pw_budget()
    regulator_request_power_budget(supply, pw_budget);

That raises rdev->pw_requested_mW on vmain. The reservation is released
only by pd692x0_managers_free_pw_budget(), and that is called only from:

  - the error labels inside pd692x0_setup_pi_matrix()
  - pd692x0_i2c_remove(), which does not run when probe fails

release_pis only calls pse_release_pis(), and no driver hook undoes
setup_pi_matrix(). When devm unregisters the manager regulators, it puts
the vmain consumer handle, but that does not lower pw_requested_mW
either.

On the next probe attempt, regulator_get_unclaimed_power_budget() returns
the reduced value, possibly 0, so manager_pw_budget[i] comes out smaller.
cfg_saved is already true, so the hardware power bank is not
reprogrammed. The first reservation can never be freed, because remove
only frees manager_pw_budget[i].

This can happen when a pd692x0 PI's vpwr-supply points at a regulator
that is late or missing, so pse_pi_check_supply() returns -EPROBE_DEFER.

Would it make sense to give drivers a way to undo setup_pi_matrix() on
this error path?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018081853.47484-1-github%40szelinsky.de

Re: [PATCH net-next 0/4] net: pse-pd: core fixes ahead of the controller notifier

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next 0/4] net: pse-pd: core fixes ahead of the controller notifier

[Kory Maincent]

On 10/18/26 10:18, Carlo Szelinsky wrote:
> These four PSE core fixes were part of the "decouple controller lookup
> from MDIO probe" series [1]; Oleksij asked to split them out. They do
> not need phylib, and each one fixes a problem on its own:
> 
>   1. Unregister frees the PI data while the controller can still be
>      found. Take it off the list and stop the IRQ and the worker first.
>   2. A failed registration leaks memory, and can leave freed power
>      domains in the global map. Clean up on the error paths.
>   3. si3474: use dev_err_probe(), so the deferral from 4 is not logged
>      as an error.
>   4. Defer the PSE driver's probe until every PI's vpwr supply is there.
> 
> Patch 1 conflicts with the pending net series [2]; keep the order from
> patch 1. Patch 2 conflicts with two of the net fixes posted alongside
> [3]; the note below its --- explains how to merge them.
> 
> Tested in QEMU on arm64 with KASAN, lockdep and kmemleak: controllers
> whose supply never appears defer, and an injected pse_register_pw_ds()
> failure is cleaned up and retried. The only KASAN reports are the known
> stale-handle bug at MDIO bus unbind, the same without this series; the
> decouple series fixes it. This series, the decouple series and the net
> fixes together also run on a Hasivo S600WP (hs104), with 200 controller
> unbind/bind cycles and no warnings or leaks. Every patch builds with
> W=1, and PHYLIB=m, PHYLIB=n and PSE_CONTROLLER=n link.

These are fixes, please send them to net instead.

-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com