Re: [PATCH net-next 08/12] net: pse-pd: pd692x0: Add support for PSE PI priority feature

[Oleksij Rempel]

Hi Andrew,

On Tue, Oct 08, 2024 at 06:50:25PM +0200, Andrew Lunn wrote:
> On Tue, Oct 08, 2024 at 03:57:22PM +0200, Oleksij Rempel wrote:
> > On Thu, Oct 03, 2024 at 01:41:02AM +0200, Andrew Lunn wrote:
> > > > +	msg = pd692x0_msg_template_list[PD692X0_MSG_SET_PORT_PARAM];
> > > > +	msg.sub[2] = id;
> > > > +	/* Controller priority from 1 to 3 */
> > > > +	msg.data[4] = prio + 1;
> > > 
> > > Does 0 have a meaning? It just seems an odd design if it does not.
> > 
> > 0 is not documented. But there are sub-priority which are not directly
> > configured by user, but affect the system behavior.
> > 
> > Priority#: Critical – 1; high – 2; low – 3
> >  For ports with the same priority, the PoE Controller sets the
> >  sub-priority according to the logic port number. (Lower number gets
> >  higher priority).
> 
> With less priorities than ports, there is always going to be something
> like this.
> 
> > 
> > Port priority affects:
> > 1. Power-up order: After a reset, the ports are powered up according to
> >  their priority, highest to lowest, highest priority will power up first.
> > 2. Shutdown order: When exceeding the power budget, lowest priority
> >  ports will turn off first.
> > 
> > Should we return sub priorities on the prio get request?
> 
> I should be optional, since we might not actually know what a
> particular device is doing. It could pick at random, it could pick a
> port which is consuming just enough to cover the shortfall if it was
> turned off, it could pick the highest consumer of the lowest priority
> etc. Some of these conditions are not going to be easy to describe
> even if we do know it.

After reviewing the manuals for LTC4266 and TPS2388x, I realized that these
controllers expose interfaces, but they don't implement prioritization concepts
themselves.

The LTC4266 and TPS2388x controllers provide only interfaces that allow the
kernel to manage shutdown and prioritization policies. For TPS2388x, fast
shutdown is implemented as a port bitmask with only two priorities, handled via
the OSS pin. Fast shutdown is triggered by the kernel on request by toggling
the corresponding pin, and the policy - when and why this pin is toggled - is
defined by the kernel or user space. Slow shutdown, on the other hand, is
managed via the I2C bus and allows for more refined control, enabling a wider
range of priorities and more granular policies.

I'll tend to hope we can reuse the proposed ETHTOOL_A_C33_PSE_PRIO interface
across different PSE controllers. However, it is already being mapped to
different shutdown concepts: PD692x0 firmware seems to rely on a slow shutdown
backed by internal policies, while TPS2388x maps it to fast shutdown with
driver specific policy. This inconsistency could force us to either break the
UAPI or introduce a new, inconsistent one once we realize TPS2388x fast
shutdown isn't what we actually need.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |