Message-ID: <20241018143706.33d49872@kmaincent-XPS-13-7390>
Date: Fri, 18 Oct 2024 14:37:06 +0200
From: Kory Maincent <kory.maincent@...tlin.com>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: Kyle Swenson <kyle.swenson@....tech>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
 <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Jonathan Corbet
 <corbet@....net>, Donald Hunter <donald.hunter@...il.com>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org"
 <netdev@...r.kernel.org>, "linux-doc@...r.kernel.org"
 <linux-doc@...r.kernel.org>, Dent Project
 <dentproject@...uxfoundation.org>, "kernel@...gutronix.de"
 <kernel@...gutronix.de>
Subject: Re: [PATCH net-next 00/12] Add support for PSE port priority

On Fri, 18 Oct 2024 08:14:26 +0200
Oleksij Rempel <o.rempel@...gutronix.de> wrote:

> On Thu, Oct 17, 2024 at 12:35:57PM +0200, Kory Maincent wrote:
> > On Tue, 15 Oct 2024 11:43:52 +0200
> > Kory Maincent <kory.maincent@...tlin.com> wrote:
> >   
>  [...]  
> > > 
> > > Indeed we will have only static method for PSE controllers not supporting
> > > system power budget management like the TPS2388x or LTC426.
> > > Both method could be supported for "smart" PSE controller like PD692x0.
> > > 
> > > Let's begin with the static method implementation in the PSE framework for
> > > now. It will need the power domain notion you have talked about.  
> > 
> > While developing the software support for port priority in static method, I
> > faced an issue.
> > 
> > Supposing we are exceeding the power budget when we plug a new PD.
> > The port power should not be enabled directly or magic smoke will appear.
> > So we have to separate the detection part to know the needs of the PD from
> > the power enable part.
> > 
> > Currently the port power is enabled on the hardware automatically after the
> > detection process. There is no way to separate power port process and
> > detection process with the PD692x0 controller and it could be done on the
> > TPS23881 by configuring it to manual mode but: "The use of this mode is
> > intended for system diagnostic purposes only in the event that ports cannot
> > be powered in accordance with the IEEE 802.3bt standard from semiauto or
> > auto modes." Not sure we want that.
> > 
> > So in fact the workaround you talked about above will be needed for the two
> > PSE controllers.  
> 
> For the TPS23881, "9.1.1.2 Semiauto", seems to be exactly what we wont:
> "The port performs detection and classification (if valid detection
> occurs) continuously. Registers are updated each time a detection or
> classification occurs. The port power is not automatically turned on. A
> Power Enable command is required to turn on the port"

I tested reading the assigned class and not the requested class register so I
thought it was not working but indeed it detects the class even if the port
power is off. That's what I was looking for, nice!
Just figured out also that calling pwoff is reseting detection, classification,
power policy... So the port need to be setup again after a pwoff.
 
> For PD692x0 controller, i'm not 100% sure. There is "4.3.5 Set Enable/Disable
> Channels" command, "Sets individual port Enable (Delivering power
> enable) or Disable (Delivering power disable)." 
> 
> For my understanding, "Delivering power" is the state after
> classification. So, it is what we wont too.

On the PD692x0 there is also a requested class and power value but it stay "to
no class detected value" (0xc) if the port is not enabled.
It did not find a way to detect the class and keep port power off.
 
> If, it works in both cases, it would be a more elegant way to go. THe
> controller do auto- detection and classification, what we should do in
> the software is do decide if the PD can be enabled based on
> classification results, priority and available budget.
> 
> > > Both methods have their pros and cons. Since the dynamic method is not
> > > always desirable, and if there's no way to disable it in the PD692x0's
> > > firmware, one potential workaround could be handling the budget in
> > > software and dynamically setting per-port limits. For instance, with a
> > > total budget of 300W and unused ports, we could initially set 95W limits
> > > per port. As high-priority PDs (e.g., three 95W devices) are powered, we
> > > could dynamically reduce the power limit on the remaining ports to 15W,
> > > ensuring that no device exceeds that classification threshold.  
> > 
> > We would set port overcurrent limit for all unpowered ports when the power
> > budget available is less than max PI power 100W as you described.
> > If a new PD plugged exceed the overcurrent limit then it will raise an
> > interrupt and we could deal with the power budget to turn off low priority
> > ports at that time.   
> 
> > Mmh in fact I could not know if the overcurrent event interrupt comes from a
> > newly plugged PD or not.  
> 
> Hm..  in case of PD692x0, may be using event counters?

Counters? I don't see how.

> > An option: When we get new PD device plug interrupt event, we wait the end
> > of classification time (Tpon 400ms) and read the interrupt states again to
> > know if there is an overcurrent or not on the port.  
> 
> Let's try Semiauto mode for TPS23881 first, I assume it is designed
> exactly for this use case.

Yes,

> And then, test if PD692x0 supports a way to disable auto power delivery
> in the 4.3.5 command.

I don't have this 4.3.5 command. Are you refering to another document than the
communication protocol version 3.55 document?

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

Powered by blists - more mailing lists