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

[Kory Maincent <kory.maincent@bootlin.com>]

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@kernel.org 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@est.tech/

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