Re: [PATCH net 05/10] net: pse-pd: check the PI state under the lock when setting a power limit

[Kyle Swenson]

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/

Thanks,
Kyle