[RFC PATCH net-next] net: pse-pd: Don't be a regulator provider

[RFC PATCH net-next] net: pse-pd: Don't be a regulator provider

[Kyle Swenson]

Commit ffef61d6d273 ("net: pse-pd: Add support for budget evaluation
strategies") introduced an unusual regulator usecase- that is, a device
that's both a consumer and provider of the same regulator: every PI is
registered as a regulator whose "vpwr" supply is also requested by the
PSE core for the power domain.

This ended up causing a significant amount of complication for the PSE
core, and can result in an ABBA deadlock that lockdep doesn't see.  The
deadlock can happen when PSE ops are running concurrent with the
pse_isr, and I noticed it when a PoE port is enabled via ethtool and
that PoE port has a device that is ready to be powered.

  - As a regulator provider, PSE's ops (is_enabled, enable, disable, ...)
    take pcdev->lock while the core already holds the regulator lock
    (regulator lock -> pcdev->lock).

  - As a consumer of the power-budget API, pse_isr(),
    pse_ethtool_set_pw_limit() and pse_ethtool_set_prio() call
    regulator_request/free_power_budget() (which take the supply's regulator
    lock) while holding pcdev->lock, from contexts that are not regulator
    callbacks (pcdev->lock -> regulator lock).

And this can deadlock because the locking order between pcdev->lock and
the regulator lock is inverted in the two paths.

Lockdep doesn't flag this deadlock because regulator_lock_nested() takes
the lock via ww_mutex_trylock(), and apparently lockdep doesn't record a
dependency for a trylock.  However, if the trylock is adjusted to always
block, lockdep does report the ordering violation.

One way to fix this complication is to not register a regulator provider
per PI and reduce the pse_core's usage to only be a regulator consumer.
The regulator core used to provide some behaviour as a side effect of
the PI regulator, which the PSE core now does itself:

  - Enabling a PI enabled its "vpwr" supply, and a PI found enabled when
    its control was acquired took a reference on it. Each PI now tracks
    whether it holds an enable reference on the power domain supply.

  - The "vpwr" supply was resolved from the PI node, then from the
    controller node. It is now requested with of_regulator_get() from
    the same nodes, and power domains are matched on the provider node
    rather than with regulator_is_equal().  Consequently, PSE_CONTROLLER now
    depends on OF.

  - A supply whose provider has not registered yet still leaves the PI
    without a power domain, as before. pse_control_get() used to return
    -EPROBE_DEFER until that supply resolved, since getting the PI
    regulator resolved its supply; it now succeeds right away.

  - A failing hardware disable is still not reported, and the supply
    reference is still released, as the regulator disable callback did.

  - A failing regulator_disable() of the supply is not retried, as with
    the regulator devres helpers: the regulator core may already have
    dropped the consumer's enable reference when it reports the error.

User visible changes:

  - pse_ethtool_set_pw_limit() hands the limit in mW straight to
    pi_set_pw_limit(). A controller without pi_set_pw_limit() now
    returns -EOPNOTSUPP, where it used to return -EPERM once the PI voltage
    had been read.

  - The PI regulators no longer show up in the regulator sysfs and
    debugfs, and over-current and over-temperature events are no longer
    sent on a regulator notifier chain. The ethtool notifications are
    unchanged.

Fixes: ffef61d6d273 ("net: pse-pd: Add support for budget evaluation strategies")
Signed-off-by: Kyle Swenson <kyle.swenson@est.tech>
---
RFC: I'm posting this RFC to discuss how the ABBA deadlock in the PSE
core should be fixed.  The deadlock was pointed out by Sashiko in [1],
and I've hit it in testing [2]:

  > This isn't a bug introduced by this patch, but choosing the static
  > budget strategy exposes a lock ordering problem that already exists in
  > the PSE core.
  >
  > pse_isr() holds pcdev->lock and ends up taking the supply regulator
  > lock [...] An admin enable running at the same time takes the locks in
  > the opposite order [...] The ww_mutex scheme cannot break a cycle
  > that includes the plain pcdev->lock mutex.

With no PI regulator, nothing takes pcdev->lock from inside a regulator
callback, so only the pcdev->lock -> regulator lock order remains.

This RFC is one approach of probably several possible ways to fix the deadlock.
That said, after exploring this approach, I've found several other problems
others have hit in their drivers on the list get fixed as a result:

Setting a power limit on an unpowered port fails
------------------------------------------------

The core needs a port voltage to turn mW into uA for the regulator
API [3]:

  >     netlink error: Can't calculate the current, PSE voltage read is 0
  >     netlink error: Result not representable

Kory's suggestion was that pi_get_voltage should not return 0 [4]:

  > To have this working the pi_get_voltage should return some value
  > even with no PD plugged.

The Realtek PSE MCU driver already works around it this way [5]:

  > core: return constant voltage when MCU reports zero, in case the
  > port is not delivering. This avoids "Voltage null" errors in case a
  > power limit is set while port is not delivering (Sashiko)

The limit is now handed to pi_set_pw_limit() in mW, so no voltage is
needed and pi_get_voltage has no remaining user in the core.

Overflow and precision loss in the mW -> uA -> mW conversion
------------------------------------------------------------

The conversion can overflow on drivers that report a measured
voltage [6]:

  > For the largest allowed limit, 99900 mW, that overflows below about
  > 46.5 mV. Only 0 V is rejected, and si3474 reports the measured
  > voltage of each port. A negative result then hits the BUG_ON() in
  > regulator_check_current_limit().

The conversion is removed, along with pse_pi_get_current_limit(), so
the possibility of losing precision on the way back to mW is gone too.

Use-after-free in the PI regulator ops
--------------------------------------

The PI regulator ops can run after the PI array is freed [7]:

  > When regulator_unregister() flushes a pending disable, the
  > regulator core invokes pse_pi_disable(), which dereferences
  > pcdev->pi[id] [...] At that point the PI array is already freed, so
  > this is a use-after-free.

The PI regulator and its ops are removed.

Regulator enable count out of step after a failed disable
---------------------------------------------------------

pse_pi_disable() always returns 0, so the regulator's enable count
gets out of step with the PI state after a failed disable [8]:

  > a further disable attempt calls regulator_disable() with
  > use_count == 0, which the regulator core rejects with a WARN and
  > -EIO.

Each PI now tracks its own enable reference on the power domain supply.
A failed hardware disable is still not reported, as before.

PI lookups returning -EPROBE_DEFER after registration
-----------------------------------------------------

PI lookups can keep returning -EPROBE_DEFER after the controller has
registered, because pse_control_get() goes through the PI regulator
[9]:

  > A PI lookup in that window still returns -EPROBE_DEFER:
  >
  > pse_control_get_internal()
  >   regulator_get_exclusive()
  >     _regulator_get_common()
  >       regulator_resolve_supply()

pse_control_get() no longer gets a regulator. A PI whose supply has not
registered yet is still left without a power domain, as before.

Power domain matching
---------------------

Power domains are now matched on the provider's DT node instead of
regulator_is_equal() on the rdev->supply.

Drawbacks
---------

Maybe some drawbacks of the approach (but I'm biased, so hopefully
others can point out more severe problems):

    * PI regulators are dropped from sysfs/debugfs
    * There is now a hard dependency on OF.  I'm not sure if this
      matters, since there's not a PSE driver with ACPI match tables in
      the tree.  Perhaps if one is added, the supply lookup can be reworked at
      that point.

The broader trade-off of dropping the PI regulator was discussed in
[10], [11] and [12].

Finally, while this is fixing a deadlock I've hit pretty often in
testing, and has a "Fixes" tag, to me it's adjusting an intentional
design point of the PSE core, which implies it's a new change.  I don't
have a preference either way, and it doesn't matter for RFC.

[1] https://lore.kernel.org/netdev/179080386247.434549.15817042587071985098@kernel.org/
[2] https://lore.kernel.org/netdev/asM8rcKC1n1Tn4JD@est-xps15/
[3] https://lore.kernel.org/netdev/20260820142429.2285172-1-kyle.swenson@est.tech/
[4] https://lore.kernel.org/netdev/41dd51e5-ac20-48b5-9e7c-4d2d261f3402@bootlin.com/
[5] https://lore.kernel.org/netdev/20260728084307.2129515-1-jelonek.jonas@gmail.com/
[6] https://lore.kernel.org/netdev/20261018082314.48950-11-github@szelinsky.de/
[7] https://lore.kernel.org/netdev/20260813200653.980170-3-github@szelinsky.de/
[8] https://lore.kernel.org/netdev/20260818162606.3981504-1-kuba@kernel.org/
[9] https://lore.kernel.org/netdev/179239893933.3135.9046263155258328713@kernel.org/
[10] https://lore.kernel.org/netdev/bb8eaf92-2b5b-46a1-b39d-a29ae273a3b5@bootlin.com/
[11] https://lore.kernel.org/netdev/atb-3EIJynaRwuwe@pengutronix.de/
[12] https://lore.kernel.org/netdev/0378ccc3-4807-4d63-8e61-03b465952929@bootlin.com/
---
 drivers/net/pse-pd/Kconfig    |   1 +
 drivers/net/pse-pd/pse_core.c | 653 ++++++++++++++++++++----------------------
 include/linux/pse-pd/pse.h    |   5 +-
 3 files changed, 320 insertions(+), 339 deletions(-)

diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
index a0f2ae668c67..fc842554300b 100644
--- a/drivers/net/pse-pd/Kconfig
+++ b/drivers/net/pse-pd/Kconfig
@@ -4,10 +4,11 @@
 #
 
 menuconfig PSE_CONTROLLER
 	bool "Ethernet Power Sourcing Equipment Support"
 	depends on REGULATOR
+	depends on OF
 	help
 	  Generic Power Sourcing Equipment Controller support.
 
 	  If unsure, say no.
 
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index a5e6d7b26b9f..44518064b273 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -9,12 +9,11 @@
 #include <linux/ethtool.h>
 #include <linux/ethtool_netlink.h>
 #include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/pse-pd/pse.h>
-#include <linux/regulator/driver.h>
-#include <linux/regulator/machine.h>
+#include <linux/regulator/consumer.h>
 #include <linux/rtnetlink.h>
 #include <net/net_trackers.h>
 
 #define PSE_PW_D_LIMIT INT_MAX
 
@@ -25,36 +24,37 @@ static DEFINE_MUTEX(pse_pw_d_mutex);
 
 /**
  * struct pse_control - a PSE control
  * @pcdev: a pointer to the PSE controller device
  *         this PSE control belongs to
- * @ps: PSE PI supply of the PSE control
  * @list: list entry for the pcdev's PSE controller list
  * @id: ID of the PSE line in the PSE controller device
  * @refcnt: Number of gets of this pse_control
  * @attached_phydev: PHY device pointer attached by the PSE control
  */
 struct pse_control {
 	struct pse_controller_dev *pcdev;
-	struct regulator *ps;
 	struct list_head list;
 	unsigned int id;
 	struct kref refcnt;
 	struct phy_device *attached_phydev;
 };
 
 /**
  * struct pse_power_domain - a PSE power domain
  * @id: ID of the power domain
  * @supply: Power supply the Power Domain
+ * @supply_np: Provider node @supply was resolved to, NULL for the dummy
+ *	       regulator
  * @refcnt: Number of gets of this pse_power_domain
  * @budget_eval_strategy: Current power budget evaluation strategy of the
  *			  power domain
  */
 struct pse_power_domain {
 	int id;
 	struct regulator *supply;
+	struct device_node *supply_np;
 	struct kref refcnt;
 	u32 budget_eval_strategy;
 };
 
 static int of_load_single_pse_pi_pairset(struct device_node *node,
@@ -273,10 +273,25 @@ static struct net_device *pse_control_get_netdev(struct pse_control *psec)
 		return NULL;
 
 	return psec->attached_phydev->attached_dev;
 }
 
+/**
+ * pse_pi_is_described - Is the PI described by the PSE controller
+ * @pcdev: a pointer to the PSE controller device
+ * @id: Index of the PI
+ *
+ * A controller using the pse-pis devicetree node only describes a subset of
+ * its lines. Undescribed lines have no power domain and are never acted on.
+ *
+ * Return: true if the PI is described, false otherwise
+ */
+static bool pse_pi_is_described(struct pse_controller_dev *pcdev, int id)
+{
+	return pcdev->no_of_pse_pi || pcdev->pi[id].np;
+}
+
 /**
  * pse_pi_is_hw_enabled - Is PI enabled at the hardware level
  * @pcdev: a pointer to the PSE controller device
  * @id: Index of the PI
  *
@@ -407,33 +422,27 @@ static bool pse_pw_d_is_sw_pw_control(struct pse_controller_dev *pcdev,
 		return true;
 
 	return false;
 }
 
-static int pse_pi_is_enabled(struct regulator_dev *rdev)
+/**
+ * _pse_pi_is_enabled - Report the admin enabled state of a PI. Assumes the
+ *			PSE lock has been acquired.
+ * @pcdev: a pointer to the PSE
+ * @id: index of the PSE control
+ *
+ * Return: 1 if enabled, 0 if not, and a failure value on error
+ */
+static int _pse_pi_is_enabled(struct pse_controller_dev *pcdev, int id)
 {
-	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
-	const struct pse_controller_ops *ops;
-	int id, ret;
-
-	ops = pcdev->ops;
-	if (!ops->pi_get_admin_state)
+	if (!pcdev->ops->pi_get_admin_state)
 		return -EOPNOTSUPP;
 
-	id = rdev_get_id(rdev);
-	mutex_lock(&pcdev->lock);
-	if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[id].pw_d)) {
-		ret = pcdev->pi[id].admin_state_enabled;
-		goto out;
-	}
-
-	ret = pse_pi_is_hw_enabled(pcdev, id);
-
-out:
-	mutex_unlock(&pcdev->lock);
+	if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[id].pw_d))
+		return pcdev->pi[id].admin_state_enabled;
 
-	return ret;
+	return pse_pi_is_hw_enabled(pcdev, id);
 }
 
 /**
  * pse_pi_deallocate_pw_budget - Deallocate power budget of the PI
  * @pi: a pointer to the PSE PI
@@ -660,222 +669,136 @@ static int _pse_pi_delivery_power_sw_pw_ctrl(struct pse_controller_dev *pcdev,
 	}
 
 	return 0;
 }
 
-static int pse_pi_enable(struct regulator_dev *rdev)
+/**
+ * pse_pi_supply_enable - Take this PI's enable reference on the supply of its
+ *			  power domain. Assumes the PSE lock has been
+ *			  acquired.
+ * @pi: a pointer to the PSE PI
+ *
+ * pw_d->supply is a single consumer handle shared by every PI of the power
+ * domain, so its enable count is one counter for the whole domain. Tracking
+ * whether this PI contributes to it keeps the enable and disable calls
+ * balanced whatever order the admin state is reached in.
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int pse_pi_supply_enable(struct pse_pi *pi)
 {
-	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
-	const struct pse_controller_ops *ops;
-	int id, ret = 0;
-
-	ops = pcdev->ops;
-	if (!ops->pi_enable)
-		return -EOPNOTSUPP;
+	int ret;
 
-	id = rdev_get_id(rdev);
-	mutex_lock(&pcdev->lock);
-	if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[id].pw_d)) {
-		/* Manage enabled status by software.
-		 * Real enable process will happen if a port is connected.
-		 */
-		if (pcdev->pi[id].isr_pd_detected) {
-			struct netlink_ext_ack extack;
+	if (!pi->pw_d || pi->supply_enabled)
+		return 0;
 
-			ret = _pse_pi_delivery_power_sw_pw_ctrl(pcdev, id, &extack);
-		}
-		if (!ret || ret == -ERANGE) {
-			pcdev->pi[id].admin_state_enabled = 1;
-			ret = 0;
-		}
-		mutex_unlock(&pcdev->lock);
+	ret = regulator_enable(pi->pw_d->supply);
+	if (ret)
 		return ret;
-	}
 
-	ret = ops->pi_enable(pcdev, id);
-	if (!ret)
-		pcdev->pi[id].admin_state_enabled = 1;
-	mutex_unlock(&pcdev->lock);
-
-	return ret;
-}
-
-static int pse_pi_disable(struct regulator_dev *rdev)
-{
-	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
-	struct pse_pi *pi;
-	int id, ret;
+	pi->supply_enabled = true;
 
-	id = rdev_get_id(rdev);
-	pi = &pcdev->pi[id];
-	mutex_lock(&pcdev->lock);
-	ret = _pse_pi_disable(pcdev, id);
-	if (!ret)
-		pi->admin_state_enabled = 0;
-
-	mutex_unlock(&pcdev->lock);
 	return 0;
 }
 
-static int _pse_pi_get_voltage(struct regulator_dev *rdev)
-{
-	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
-	const struct pse_controller_ops *ops;
-	int id;
-
-	ops = pcdev->ops;
-	if (!ops->pi_get_voltage)
-		return -EOPNOTSUPP;
-
-	id = rdev_get_id(rdev);
-	return ops->pi_get_voltage(pcdev, id);
-}
-
-static int pse_pi_get_voltage(struct regulator_dev *rdev)
+/**
+ * pse_pi_supply_disable - Release this PI's enable reference on the supply of
+ *			   its power domain. Assumes the PSE lock has been
+ *			   acquired.
+ * @pi: a pointer to the PSE PI
+ *
+ * As with the regulator devres helpers, a failing disable is not retried:
+ * the regulator core may already have dropped the consumer's enable
+ * reference when it reports the error.
+ */
+static void pse_pi_supply_disable(struct pse_pi *pi)
 {
-	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
-	int ret;
-
-	mutex_lock(&pcdev->lock);
-	ret = _pse_pi_get_voltage(rdev);
-	mutex_unlock(&pcdev->lock);
+	if (!pi->supply_enabled)
+		return;
 
-	return ret;
+	regulator_disable(pi->pw_d->supply);
+	pi->supply_enabled = false;
 }
 
-static int pse_pi_get_current_limit(struct regulator_dev *rdev)
+/**
+ * _pse_pi_admin_enable - Mark a PI as administratively enabled and deliver
+ *			  power if possible. Assumes the PSE lock has been
+ *			  acquired.
+ * @pcdev: a pointer to the PSE
+ * @id: index of the PSE control
+ *
+ * The parent rail of the power domain is enabled for as long as at least one
+ * of its PIs is administratively enabled. This used to be done by the
+ * regulator core as a side effect of enabling the PI regulator.
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int _pse_pi_admin_enable(struct pse_controller_dev *pcdev, int id)
 {
-	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
-	const struct pse_controller_ops *ops;
-	int id, uV, mW, ret;
-	s64 tmp_64;
+	struct pse_pi *pi = &pcdev->pi[id];
+	bool supply_taken;
+	int ret = 0;
 
-	ops = pcdev->ops;
-	id = rdev_get_id(rdev);
-	if (!ops->pi_get_pw_limit || !ops->pi_get_voltage)
+	if (!pcdev->ops->pi_enable)
 		return -EOPNOTSUPP;
 
-	mutex_lock(&pcdev->lock);
-	ret = ops->pi_get_pw_limit(pcdev, id);
-	if (ret < 0)
-		goto out;
-	mW = ret;
-
-	ret = _pse_pi_get_voltage(rdev);
-	if (!ret) {
-		dev_err(pcdev->dev, "Voltage null\n");
-		ret = -ERANGE;
-		goto out;
-	}
-	if (ret < 0)
-		goto out;
-	uV = ret;
-
-	tmp_64 = mW;
-	tmp_64 *= 1000000000ull;
-	/* uA = mW * 1000000000 / uV */
-	ret = DIV_ROUND_CLOSEST_ULL(tmp_64, uV);
+	supply_taken = !pi->supply_enabled;
+	ret = pse_pi_supply_enable(pi);
+	if (ret)
+		return ret;
 
-out:
-	mutex_unlock(&pcdev->lock);
-	return ret;
-}
+	if (pse_pw_d_is_sw_pw_control(pcdev, pi->pw_d)) {
+		/* Manage enabled status by software.
+		 * Real enable process will happen if a port is connected.
+		 */
+		if (pi->isr_pd_detected) {
+			struct netlink_ext_ack extack;
 
-static int pse_pi_set_current_limit(struct regulator_dev *rdev, int min_uA,
-				    int max_uA)
-{
-	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
-	const struct pse_controller_ops *ops;
-	int id, mW, ret;
-	s64 tmp_64;
+			ret = _pse_pi_delivery_power_sw_pw_ctrl(pcdev, id, &extack);
+		}
+		if (!ret || ret == -ERANGE) {
+			pi->admin_state_enabled = 1;
+			return 0;
+		}
+		goto disable_supply;
+	}
 
-	ops = pcdev->ops;
-	if (!ops->pi_set_pw_limit || !ops->pi_get_voltage)
-		return -EOPNOTSUPP;
+	ret = pcdev->ops->pi_enable(pcdev, id);
+	if (ret)
+		goto disable_supply;
 
-	if (max_uA > MAX_PI_CURRENT)
-		return -ERANGE;
+	pi->admin_state_enabled = 1;
 
-	id = rdev_get_id(rdev);
-	mutex_lock(&pcdev->lock);
-	ret = _pse_pi_get_voltage(rdev);
-	if (!ret) {
-		dev_err(pcdev->dev, "Voltage null\n");
-		ret = -ERANGE;
-		goto out;
-	}
-	if (ret < 0)
-		goto out;
+	return 0;
 
-	tmp_64 = ret;
-	tmp_64 *= max_uA;
-	/* mW = uA * uV / 1000000000 */
-	mW = DIV_ROUND_CLOSEST_ULL(tmp_64, 1000000000);
-	ret = ops->pi_set_pw_limit(pcdev, id, mW);
-out:
-	mutex_unlock(&pcdev->lock);
+disable_supply:
+	if (supply_taken)
+		pse_pi_supply_disable(pi);
 
 	return ret;
 }
 
-static const struct regulator_ops pse_pi_ops = {
-	.is_enabled = pse_pi_is_enabled,
-	.enable = pse_pi_enable,
-	.disable = pse_pi_disable,
-	.get_voltage = pse_pi_get_voltage,
-	.get_current_limit = pse_pi_get_current_limit,
-	.set_current_limit = pse_pi_set_current_limit,
-};
-
-static int
-devm_pse_pi_regulator_register(struct pse_controller_dev *pcdev,
-			       char *name, int id)
+/**
+ * _pse_pi_admin_disable - Mark a PI as administratively disabled and stop
+ *			   delivering power. Assumes the PSE lock has been
+ *			   acquired.
+ * @pcdev: a pointer to the PSE
+ * @id: index of the PSE control
+ *
+ * As with the former regulator disable callback, a failure of the hardware
+ * disable is not reported and the supply reference is released regardless.
+ *
+ * Return: 0
+ */
+static int _pse_pi_admin_disable(struct pse_controller_dev *pcdev, int id)
 {
-	struct regulator_init_data *rinit_data;
-	struct regulator_config rconfig = {0};
-	struct regulator_desc *rdesc;
-	struct regulator_dev *rdev;
-
-	rinit_data = devm_kzalloc(pcdev->dev, sizeof(*rinit_data),
-				  GFP_KERNEL);
-	if (!rinit_data)
-		return -ENOMEM;
-
-	rdesc = devm_kzalloc(pcdev->dev, sizeof(*rdesc), GFP_KERNEL);
-	if (!rdesc)
-		return -ENOMEM;
+	struct pse_pi *pi = &pcdev->pi[id];
 
-	/* Regulator descriptor id have to be the same as its associated
-	 * PSE PI id for the well functioning of the PSE controls.
-	 */
-	rdesc->id = id;
-	rdesc->name = name;
-	rdesc->type = REGULATOR_VOLTAGE;
-	rdesc->ops = &pse_pi_ops;
-	rdesc->owner = pcdev->owner;
-
-	rinit_data->constraints.valid_ops_mask = REGULATOR_CHANGE_STATUS;
-
-	if (pcdev->ops->pi_set_pw_limit)
-		rinit_data->constraints.valid_ops_mask |=
-			REGULATOR_CHANGE_CURRENT;
-
-	rinit_data->supply_regulator = "vpwr";
-
-	rconfig.dev = pcdev->dev;
-	rconfig.driver_data = pcdev;
-	rconfig.init_data = rinit_data;
-	rconfig.of_node = pcdev->pi[id].np;
-
-	rdev = devm_regulator_register(pcdev->dev, rdesc, &rconfig);
-	if (IS_ERR(rdev)) {
-		dev_err_probe(pcdev->dev, PTR_ERR(rdev),
-			      "Failed to register regulator\n");
-		return PTR_ERR(rdev);
-	}
+	if (!_pse_pi_disable(pcdev, id))
+		pi->admin_state_enabled = 0;
 
-	pcdev->pi[id].rdev = rdev;
+	pse_pi_supply_disable(pi);
 
 	return 0;
 }
 
 static void __pse_pw_d_release(struct kref *kref)
@@ -885,10 +808,11 @@ static void __pse_pw_d_release(struct kref *kref)
 						     refcnt);
 
 	regulator_put(pw_d->supply);
 	xa_erase(&pse_pw_d_map, pw_d->id);
 	mutex_unlock(&pse_pw_d_mutex);
+	of_node_put(pw_d->supply_np);
 }
 
 /**
  * pse_flush_pw_ds - flush all PSE power domains of a PSE
  * @pcdev: a pointer to the initialized PSE controller device
@@ -934,10 +858,107 @@ static struct pse_power_domain *devm_pse_alloc_pw_d(struct device *dev)
 	kref_init(&pw_d->refcnt);
 	pw_d->id = index;
 	return pw_d;
 }
 
+/**
+ * pse_pi_supply_np - node the "vpwr" supply of a PI is looked up from
+ * @pcdev: a pointer to the PSE controller device
+ * @id: index of the PSE PI
+ *
+ * The PI regulator used to resolve its "vpwr" supply through
+ * regulator_resolve_supply(): first on the regulator device, whose node was
+ * the PI node, then on the parent device, the controller, whose own
+ * vpwr-supply is parsed before of_get_child_regulator() walks its children.
+ * of_regulator_get() walks the children of the consumer device, the
+ * controller, so the walk over the PI node's own children is not kept: a
+ * vpwr-supply in a child node of a PI is no longer found from that PI. The
+ * PI node is used only when it names a supply itself, because otherwise the
+ * child walk would answer from a sibling PI's supply before the controller
+ * node had its turn.
+ *
+ * Return: the device node to pass to of_regulator_get()
+ */
+static struct device_node *pse_pi_supply_np(struct pse_controller_dev *pcdev,
+					    int id)
+{
+	if (pcdev->pi[id].np &&
+	    of_property_present(pcdev->pi[id].np, "vpwr-supply"))
+		return pcdev->pi[id].np;
+
+	return pcdev->dev->of_node;
+}
+
+static struct device_node *pse_child_vpwr_supply(struct device_node *parent)
+{
+	struct device_node *regnode;
+
+	for_each_child_of_node_scoped(parent, child) {
+		regnode = of_parse_phandle(child, "vpwr-supply", 0);
+		if (regnode)
+			return regnode;
+
+		regnode = pse_child_vpwr_supply(child);
+		if (regnode)
+			return regnode;
+	}
+
+	return NULL;
+}
+
+/**
+ * pse_pi_supply_provider - provider node of_regulator_get() resolves the
+ *			    "vpwr" supply of a PI to
+ * @pcdev: a pointer to the PSE controller device
+ * @id: index of the PSE PI
+ *
+ * Mirrors of_get_regulator(): the phandle on the lookup node, then the first
+ * one found under the consumer device's children.
+ *
+ * Return: the provider node with a reference held, or NULL if none is
+ *	   described
+ */
+static struct device_node *
+pse_pi_supply_provider(struct pse_controller_dev *pcdev, int id)
+{
+	struct device_node *regnode;
+
+	regnode = of_parse_phandle(pse_pi_supply_np(pcdev, id), "vpwr-supply", 0);
+	if (regnode)
+		return regnode;
+
+	return pse_child_vpwr_supply(pcdev->dev->of_node);
+}
+
+/**
+ * pse_find_pw_d - Find the power domain of a supply provider
+ * @regnode: provider node, NULL for the dummy regulator
+ *
+ * of_regulator_get() resolves a provider node to the regulator registered
+ * with it, so two lookups returning the same node get the same regulator.
+ * Matching on the node before any handle is taken matters because every
+ * handle a controller requests is named "<dev_name>-vpwr": a second one on a
+ * rail it, or a previous bind of it, already holds collides with the first
+ * one's sysfs link, and releasing it removes that link. Assumes
+ * pse_pw_d_mutex is held.
+ *
+ * Return: the power domain, or NULL if none exists for @regnode
+ */
+static struct pse_power_domain *pse_find_pw_d(struct device_node *regnode)
+{
+	struct pse_power_domain *pw_d;
+	unsigned long index;
+
+	lockdep_assert_held(&pse_pw_d_mutex);
+
+	xa_for_each(&pse_pw_d_map, index, pw_d)
+		if (pw_d->supply_np == regnode)
+			return pw_d;
+
+	return NULL;
+}
+
 /**
  * pse_register_pw_ds - register the PSE power domains for a PSE
  * @pcdev: a pointer to the PSE controller device
  *
  * Return: 0 on success and failure value on error
@@ -946,51 +967,53 @@ static int pse_register_pw_ds(struct pse_controller_dev *pcdev)
 {
 	int i, ret = 0;
 
 	mutex_lock(&pse_pw_d_mutex);
 	for (i = 0; i < pcdev->nr_lines; i++) {
-		struct regulator_dev *rdev = pcdev->pi[i].rdev;
 		struct pse_power_domain *pw_d;
+		struct device_node *regnode;
+		struct device_node *np;
 		struct regulator *supply;
-		bool present = false;
-		unsigned long index;
 
-		/* No regulator or regulator parent supply registered.
-		 * We need a regulator parent to register a PSE power domain
-		 */
-		if (!rdev || !rdev->supply)
+		if (!pse_pi_is_described(pcdev, i))
 			continue;
 
-		xa_for_each(&pse_pw_d_map, index, pw_d) {
-			/* Power supply already registered as a PSE power
-			 * domain.
-			 */
-			if (regulator_is_equal(pw_d->supply, rdev->supply)) {
-				present = true;
-				pcdev->pi[i].pw_d = pw_d;
-				break;
-			}
-		}
-		if (present) {
+		regnode = pse_pi_supply_provider(pcdev, i);
+		pw_d = pse_find_pw_d(regnode);
+		if (pw_d) {
+			of_node_put(regnode);
 			kref_get(&pw_d->refcnt);
+			pcdev->pi[i].pw_d = pw_d;
 			continue;
 		}
 
-		pw_d = devm_pse_alloc_pw_d(pcdev->dev);
-		if (IS_ERR(pw_d)) {
-			ret = PTR_ERR(pw_d);
-			goto out;
-		}
+		np = pse_pi_supply_np(pcdev, i);
 
-		supply = regulator_get(&rdev->dev, rdev->supply_name);
+		/* A supply whose provider has not registered yet leaves the
+		 * PI without a power domain, as when the supply of the former
+		 * PI regulator was still unresolved at this point.
+		 */
+		supply = of_regulator_get(pcdev->dev, np, "vpwr");
 		if (IS_ERR(supply)) {
-			xa_erase(&pse_pw_d_map, pw_d->id);
+			of_node_put(regnode);
+			if (PTR_ERR(supply) == -EPROBE_DEFER)
+				continue;
+
 			ret = PTR_ERR(supply);
 			goto out;
 		}
 
+		pw_d = devm_pse_alloc_pw_d(pcdev->dev);
+		if (IS_ERR(pw_d)) {
+			regulator_put(supply);
+			of_node_put(regnode);
+			ret = PTR_ERR(pw_d);
+			goto out;
+		}
+
 		pw_d->supply = supply;
+		pw_d->supply_np = regnode;
 		if (pcdev->supp_budget_eval_strategies)
 			pw_d->budget_eval_strategy = pcdev->supp_budget_eval_strategies;
 		else
 			pw_d->budget_eval_strategy = PSE_BUDGET_EVAL_STRAT_DISABLED;
 		kref_init(&pw_d->refcnt);
@@ -1036,12 +1059,11 @@ static void pse_send_ntf_worker(struct work_struct *work)
  *
  * Return: 0 on success and failure value on error
  */
 int pse_controller_register(struct pse_controller_dev *pcdev)
 {
-	size_t reg_name_len;
-	int ret, i;
+	int ret;
 
 	mutex_init(&pcdev->lock);
 	INIT_LIST_HEAD(&pcdev->pse_control_head);
 	spin_lock_init(&pcdev->ntf_fifo_lock);
 	ret = kfifo_alloc(&pcdev->ntf_fifo, pcdev->nr_lines, GFP_KERNEL);
@@ -1069,35 +1091,10 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 		ret = pcdev->ops->setup_pi_matrix(pcdev);
 		if (ret)
 			return ret;
 	}
 
-	/* Each regulator name len is pcdev dev name + 7 char +
-	 * int max digit number (10) + 1
-	 */
-	reg_name_len = strlen(dev_name(pcdev->dev)) + 18;
-
-	/* Register PI regulators */
-	for (i = 0; i < pcdev->nr_lines; i++) {
-		char *reg_name;
-
-		/* Do not register regulator for PIs not described */
-		if (!pcdev->no_of_pse_pi && !pcdev->pi[i].np)
-			continue;
-
-		reg_name = devm_kzalloc(pcdev->dev, reg_name_len, GFP_KERNEL);
-		if (!reg_name)
-			return -ENOMEM;
-
-		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;
-	}
-
 	ret = pse_register_pw_ds(pcdev);
 	if (ret)
 		return ret;
 
 	mutex_lock(&pse_list_mutex);
@@ -1112,10 +1109,20 @@ EXPORT_SYMBOL_GPL(pse_controller_register);
  * pse_controller_unregister - unregister a PSE controller device
  * @pcdev: a pointer to the PSE controller device
  */
 void pse_controller_unregister(struct pse_controller_dev *pcdev)
 {
+	int i;
+
+	/* __pse_pw_d_release() calls regulator_put(), which warns on a
+	 * handle with a non-zero enable count.
+	 */
+	mutex_lock(&pcdev->lock);
+	for (i = 0; i < pcdev->nr_lines; i++)
+		pse_pi_supply_disable(&pcdev->pi[i]);
+	mutex_unlock(&pcdev->lock);
+
 	pse_flush_pw_ds(pcdev);
 	pse_release_pis(pcdev);
 	if (pcdev->irq)
 		disable_irq(pcdev->irq);
 	cancel_work_sync(&pcdev->ntf_work);
@@ -1171,29 +1178,10 @@ struct pse_irq {
 	struct pse_irq_desc desc;
 	unsigned long *notifs;
 	unsigned long *notifs_mask;
 };
 
-/**
- * pse_to_regulator_notifs - Convert PSE notifications to Regulator
- *			     notifications
- * @notifs: PSE notifications
- *
- * Return: Regulator notifications
- */
-static unsigned long pse_to_regulator_notifs(unsigned long notifs)
-{
-	unsigned long rnotifs = 0;
-
-	if (notifs & ETHTOOL_PSE_EVENT_OVER_CURRENT)
-		rnotifs |= REGULATOR_EVENT_OVER_CURRENT;
-	if (notifs & ETHTOOL_PSE_EVENT_OVER_TEMP)
-		rnotifs |= REGULATOR_EVENT_OVER_TEMP;
-
-	return rnotifs;
-}
-
 /**
  * pse_set_config_isr - Set PSE control config according to the PSE
  *			notifications
  * @pcdev: a pointer to the PSE
  * @id: index of the PSE control
@@ -1264,15 +1252,15 @@ static irqreturn_t pse_isr(int irq, void *data)
 		mutex_unlock(&pcdev->lock);
 		return IRQ_NONE;
 	}
 
 	for_each_set_bit(i, h->notifs_mask, pcdev->nr_lines) {
-		unsigned long notifs, rnotifs;
+		unsigned long notifs;
 		struct pse_ntf ntf = {};
 
 		/* Do nothing PI not described */
-		if (!pcdev->pi[i].rdev)
+		if (!pse_pi_is_described(pcdev, i))
 			continue;
 
 		notifs = h->notifs[i];
 		if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[i].pw_d)) {
 			ret = pse_set_config_isr(pcdev, i, notifs);
@@ -1286,14 +1274,10 @@ static irqreturn_t pse_isr(int irq, void *data)
 		ntf.notifs = notifs;
 		ntf.id = i;
 		kfifo_in_spinlocked(&pcdev->ntf_fifo, &ntf, 1,
 				    &pcdev->ntf_fifo_lock);
 		schedule_work(&pcdev->ntf_work);
-
-		rnotifs = pse_to_regulator_notifs(notifs);
-		regulator_notifier_call_chain(pcdev->pi[i].rdev, rnotifs,
-					      NULL);
 	}
 
 	mutex_unlock(&pcdev->lock);
 
 	return IRQ_HANDLED;
@@ -1360,18 +1344,20 @@ EXPORT_SYMBOL_GPL(devm_pse_irq_helper);
 
 static void __pse_control_release(struct kref *kref)
 {
 	struct pse_control *psec = container_of(kref, struct pse_control,
 						  refcnt);
+	struct pse_controller_dev *pcdev = psec->pcdev;
 
 	lockdep_assert_held(&pse_list_mutex);
 
-	if (psec->pcdev->pi[psec->id].admin_state_enabled)
-		regulator_disable(psec->ps);
-	regulator_put(psec->ps);
+	mutex_lock(&pcdev->lock);
+	if (pcdev->pi[psec->id].admin_state_enabled)
+		_pse_pi_admin_disable(pcdev, psec->id);
+	mutex_unlock(&pcdev->lock);
 
-	module_put(psec->pcdev->owner);
+	module_put(pcdev->owner);
 
 	list_del(&psec->list);
 	kfree(psec);
 }
 
@@ -1425,25 +1411,32 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
 	if (!pcdev->ops->pi_get_admin_state) {
 		ret = -EOPNOTSUPP;
 		goto free_psec;
 	}
 
-	/* Initialize admin_state_enabled before the regulator_get. This
-	 * aims to have the right value reported in the first is_enabled
-	 * call in case of control managed by software.
+	/* Initialize admin_state_enabled before the control becomes
+	 * reachable. This aims to have the right value reported in the first
+	 * status read in case of control managed by software.
+	 *
+	 * A PI left powered by the bootloader is adopted as administratively
+	 * enabled, so take the enable reference on the power domain supply
+	 * that goes with it. The regulator core used to do this when the
+	 * per-PI regulator was acquired.
 	 */
+	mutex_lock(&pcdev->lock);
 	ret = pse_pi_is_hw_enabled(pcdev, index);
-	if (ret < 0)
+	if (ret < 0) {
+		mutex_unlock(&pcdev->lock);
 		goto free_psec;
+	}
 
 	pcdev->pi[index].admin_state_enabled = ret;
-	psec->ps = regulator_get_exclusive(pcdev->dev,
-					   rdev_get_name(pcdev->pi[index].rdev));
-	if (IS_ERR(psec->ps)) {
-		ret = PTR_ERR(psec->ps);
+	if (ret)
+		ret = pse_pi_supply_enable(&pcdev->pi[index]);
+	mutex_unlock(&pcdev->lock);
+	if (ret)
 		goto put_module;
-	}
 
 	psec->pcdev = pcdev;
 	list_add(&psec->list, &pcdev->pse_control_head);
 	psec->id = index;
 	psec->attached_phydev = phydev;
@@ -1709,62 +1702,63 @@ int pse_ethtool_get_status(struct pse_control *psec,
 EXPORT_SYMBOL_GPL(pse_ethtool_get_status);
 
 static int pse_ethtool_c33_set_config(struct pse_control *psec,
 				      const struct pse_control_config *config)
 {
+	struct pse_controller_dev *pcdev = psec->pcdev;
+	struct pse_pi *pi = &pcdev->pi[psec->id];
 	int err = 0;
 
-	/* Look at admin_state_enabled status to not call regulator_enable
-	 * or regulator_disable twice creating a regulator counter mismatch
-	 */
+	mutex_lock(&pcdev->lock);
 	switch (config->c33_admin_control) {
 	case ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED:
-		/* We could have mismatch between admin_state_enabled and
-		 * state reported by regulator_is_enabled. This can occur when
-		 * the PI is forcibly turn off by the controller. Call
-		 * regulator_disable on that case to fix the counters state.
+		/* The PI may have been forcibly turned off by the controller
+		 * behind our back. Take it through a disable to release the
+		 * rail before enabling it again.
 		 */
-		if (psec->pcdev->pi[psec->id].admin_state_enabled &&
-		    !regulator_is_enabled(psec->ps)) {
-			err = regulator_disable(psec->ps);
+		if (pi->admin_state_enabled &&
+		    _pse_pi_is_enabled(pcdev, psec->id) == 0) {
+			err = _pse_pi_admin_disable(pcdev, psec->id);
 			if (err)
 				break;
 		}
-		if (!psec->pcdev->pi[psec->id].admin_state_enabled)
-			err = regulator_enable(psec->ps);
+		if (!pi->admin_state_enabled)
+			err = _pse_pi_admin_enable(pcdev, psec->id);
 		break;
 	case ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED:
-		if (psec->pcdev->pi[psec->id].admin_state_enabled)
-			err = regulator_disable(psec->ps);
+		if (pi->admin_state_enabled)
+			err = _pse_pi_admin_disable(pcdev, psec->id);
 		break;
 	default:
 		err = -EOPNOTSUPP;
 	}
+	mutex_unlock(&pcdev->lock);
 
 	return err;
 }
 
 static int pse_ethtool_podl_set_config(struct pse_control *psec,
 				       const struct pse_control_config *config)
 {
+	struct pse_controller_dev *pcdev = psec->pcdev;
+	struct pse_pi *pi = &pcdev->pi[psec->id];
 	int err = 0;
 
-	/* Look at admin_state_enabled status to not call regulator_enable
-	 * or regulator_disable twice creating a regulator counter mismatch
-	 */
+	mutex_lock(&pcdev->lock);
 	switch (config->podl_admin_control) {
 	case ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED:
-		if (!psec->pcdev->pi[psec->id].admin_state_enabled)
-			err = regulator_enable(psec->ps);
+		if (!pi->admin_state_enabled)
+			err = _pse_pi_admin_enable(pcdev, psec->id);
 		break;
 	case ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED:
-		if (psec->pcdev->pi[psec->id].admin_state_enabled)
-			err = regulator_disable(psec->ps);
+		if (pi->admin_state_enabled)
+			err = _pse_pi_admin_disable(pcdev, psec->id);
 		break;
 	default:
 		err = -EOPNOTSUPP;
 	}
+	mutex_unlock(&pcdev->lock);
 
 	return err;
 }
 
 /**
@@ -1850,33 +1844,18 @@ static int pse_pi_update_pw_budget(struct pse_controller_dev *pcdev, int id,
  */
 int pse_ethtool_set_pw_limit(struct pse_control *psec,
 			     struct netlink_ext_ack *extack,
 			     const unsigned int pw_limit)
 {
-	int uV, uA, ret, previous_pw_allocated = 0;
-	s64 tmp_64;
+	struct pse_controller_dev *pcdev = psec->pcdev;
+	int ret, previous_pw_allocated = 0;
 
 	if (pw_limit > MAX_PI_PW)
 		return -ERANGE;
 
-	ret = regulator_get_voltage(psec->ps);
-	if (!ret) {
-		NL_SET_ERR_MSG(extack,
-			       "Can't calculate the current, PSE voltage read is 0");
-		return -ERANGE;
-	}
-	if (ret < 0) {
-		NL_SET_ERR_MSG(extack,
-			       "Error reading PSE voltage");
-		return ret;
-	}
-	uV = ret;
-
-	tmp_64 = pw_limit;
-	tmp_64 *= 1000000000ull;
-	/* uA = mW * 1000000000 / uV */
-	uA = DIV_ROUND_CLOSEST_ULL(tmp_64, uV);
+	if (!pcdev->ops->pi_set_pw_limit)
+		return -EOPNOTSUPP;
 
 	/* Update power budget only in software power control case and
 	 * if a Power Device is powered.
 	 */
 	if (pse_pw_d_is_sw_pw_control(psec->pcdev,
@@ -1888,11 +1867,13 @@ int pse_ethtool_set_pw_limit(struct pse_control *psec,
 		if (ret < 0)
 			return ret;
 		previous_pw_allocated = ret;
 	}
 
-	ret = regulator_set_current_limit(psec->ps, 0, uA);
+	mutex_lock(&pcdev->lock);
+	ret = pcdev->ops->pi_set_pw_limit(pcdev, psec->id, pw_limit);
+	mutex_unlock(&pcdev->lock);
 	if (ret < 0 && previous_pw_allocated) {
 		pse_pi_update_pw_budget(psec->pcdev, psec->id,
 					previous_pw_allocated, extack);
 	}
 
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 4e5696cfade7..32a47cd8c81e 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -8,11 +8,10 @@
 #include <linux/list.h>
 #include <linux/netlink.h>
 #include <linux/kfifo.h>
 #include <uapi/linux/ethtool.h>
 #include <uapi/linux/ethtool_netlink_generated.h>
-#include <linux/regulator/driver.h>
 
 /* Maximum current in uA according to IEEE 802.3-2022 Table 145-1 */
 #define MAX_PI_CURRENT 1920000
 /* Maximum power in mW according to IEEE 802.3-2022 Table 145-16 */
 #define MAX_PI_PW 99900
@@ -242,12 +241,12 @@ struct pse_pi_pairset {
  * struct pse_pi - PSE PI (Power Interface) entity as described in
  *		   IEEE 802.3-2022 145.2.4
  *
  * @pairset: table of the PSE PI pinout alternative for the two pairset
  * @np: device node pointer of the PSE PI node
- * @rdev: regulator represented by the PSE PI
  * @admin_state_enabled: PI enabled state
+ * @supply_enabled: PI holds an enable reference on its power domain supply
  * @pw_d: Power domain of the PSE PI
  * @prio: Priority of the PSE PI. Used in static budget evaluation strategy
  * @isr_pd_detected: PSE PI detection status managed by the interruption
  *		     handler. This variable is relevant when the power enabled
  *		     management is managed in software like the static
@@ -256,12 +255,12 @@ struct pse_pi_pairset {
  *		     static budget evaluation strategy.
  */
 struct pse_pi {
 	struct pse_pi_pairset pairset[2];
 	struct device_node *np;
-	struct regulator_dev *rdev;
 	bool admin_state_enabled;
+	bool supply_enabled;
 	struct pse_power_domain *pw_d;
 	int prio;
 	bool isr_pd_detected;
 	int pw_allocated_mW;
 };

---
base-commit: 8cc24527d54d3ecaa09eec5a91bd107bb8b3d7d3
change-id: 20261020-pse_regulator_fixes-a16bfb0f90f6

Best regards,
-- 
Kyle Swenson <kyle.swenson@est.tech>