Re: [PATCH net-next v5 06/12] net: pse-pd: Add support for budget evaluation strategies

[Kory Maincent]

On Sat, 1 Mar 2025 14:00:43 +0100
Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote:

> On Thu, Feb 27, 2025 at 07:26:40PM +0100, Kory Maincent wrote:
> > On Thu, 27 Feb 2025 17:40:42 +0100
> > Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote:

> > > I would prefer to have it in the for of devlink or use regulator netlink
> > > interface. But, we do not need to do this discussion right now.  
> > 
> > If we want to report the method we should discuss it now. We shouldn't add
> > BUDGET_EVAL_STRAT uAPI to ethtool if we use another way to get and set the
> > method later.  
> 
> Ok, I assume we are talking about different things. I mean - not port
> specific configurations and diagnostic, will have different interface.
> 
> BUDGET_EVAL_STRAT is port specific. HP and Cisco implement it as port
> specific. PD692x0 Protocol manual describe it as port specific too:
> 3.3.6 Set BT Port Parameters
>  Bits [3..0]—BT port PM mode
>   0x0: The port power that is used for power management purposes is
>        dynamic (Iport x Vmain).
>   0x1: The port power that is used for power management purposes is port
>        TPPL_BT.
>   0x2: The port power that is used for power management purposes is
>        dynamic for non LLDP/CDP/Autoclass ports and TPPL_BT for
> LLDP/CDP/Autoclass ports. 0xF: Do not change settings.

I don't really understand how that can be port specific when the power budget is
per PD69208 manager. Maybe I am missing information here.

> > We could also not report the method for now and assume the user knows it for
> > the two controllers currently supported.  
> 
> On one side: it is not just status, but also active configuration. By
> implementing the interface we may break default configuration and user
> expectations.

Yes we should not implement the budget method get/set interface in this series.
 
> On other side: PD692x0 seems to need more then just setting prios to
> manage them correctly. For example power bank limits should be set,
> otherwise internal firmware won't be able to perform budget calculations.

Patch 8 is already configuring the power PD692x0 bank limit according to PSE
power domain budget.

> So, I assume, critical components are missing anyway.

As we are not supporting the budget method configured by the user in this
series, I agreed we should not add any uAPI related to it that could be broken
or confusing later.

I will remove it and send v6.

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