Re: [PATCH net 05/10] net: pse-pd: check the PI state under the lock when setting a power limit
From: Kory Maincent
Date: Wed Oct 21 2026 - 11:44:26 EST
On 10/21/26 17:40, Kyle Swenson wrote:
On Tue, Oct 20, 2026 at 12:19:28PM +0200, Kory Maincent wrote:
On 10/20/26 07:40, Oleksij Rempel wrote:
CCing regulator related folks.
On Mon, Oct 19, 2026 at 02:30:24PM +0200, Kory Maincent wrote:
On 10/19/26 10:51, netdev-bot+sashiko@xxxxxxxxxx wrote:
previous_pw_allocated = pi->pw_allocated_mW;
pw_diff = pw_req - previous_pw_allocated;
if (!pw_diff) {
[...]
regulator_set_current_limit()
pse_pi_set_current_limit()
ops->pi_set_pw_limit() <- new, higher limit
@Oleksij, Here is another example of an issue related to the PSE PI being a
regulator provider.
@Carlo, Kyle is working on a series that remove the support for a PSE PI
being a regulator provider. We will see where it goes.
Removing the regulator provider support is a bad idea:
- We will end up implementing a poor man's regulator framework inside the
network subsystem. Networking folks will have to deal with physical constraints
already solved by the power management community, requiring a different mindset
and domain experience.
That's what we thought at the beginning of the implementation of PoE but in
fact that's not the case. I don't see currently any cases of using already
solved regulator constraint in the PoE core.
The drawback are:
- locks issues: solved one by using worker but there are still others
appearing like this one
- code complexity due to all the regulator callback wrapper
- converting mW to mA for regulator framework then to mW again for the
PSE driver callbacks. There are even devices that are using current
limit therefore there will be 3 conversions.
The more painful issues are the lock issues.
- We lose built-in regulator infrastructure: DT safety constraints
(regulator-max-microamp), over-current/under-voltage notifications, and
sysfs/debugfs (regulator_summary) visibility.
I disagree here.
regulator-max-microamp: has not interest from the PI side but this is
related to its power supply. The PI should still stay as a regulator
consumer so the power supply could describe this binding.
over-current/under-voltage notifications: we are already reporting these PSE
PIs notification through ethtool. We will loose
regulator_notifier_call_chain() call but well there is no consumer to the PI
regulator provider so it's mainly useless.
Yes, we will loose sysfs/debugfs visibility, but is someone really using it
as all PSE communication goes through ethtool?
- The race above is just a lock scope issue across pcdev->lock and the
regulator call, not a flaw in the abstraction.
That's not something easy to solve as PSE and regulator provider are nested
together.
Either we put all of the lock issues inside workers, it will not be
synchronous and will smell hackish but well...
Or we try to combine the PSE lock with the regulator nested lock in one way
or another.
Do you have a better idea?
- Writing a custom power state machine in net/ will introduce more bugs than it
fixes - especially if we overestimate our ability to upstream complex custom
logic under LLM review supervision.
Removing regulator provider will simplify a lot the implementation so I
think it will rather remove bugs than introduce new one.
I think we should let Kyle works on it with a RFC and we will see where it
goes.
For what it's worth, and for others that might be interested but I missed
on cc, the RFC is here:
https://lore.kernel.org/netdev/20261020-pse_regulator_fixes-v1-1-fd6084a22cba@xxxxxxxx/
Thank's for that.
I will take a look at it when I have time, but it seems promising to me to solve lots of issue as you mention.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com