Re: [PATCH ethtool v2 2/3] ethtool: pse-pd: Add PSE priority support

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

Le Wed, 13 Aug 2025 14:25:22 +0200,
Oleksij Rempel <o.rempel@pengutronix.de> a écrit :

> Hi Kory,
> 
> Thank you for your work! Here are some review comments...

Hello Oleksij,

> 
> On Wed, Aug 13, 2025 at 10:57:51AM +0200, Kory Maincent wrote:
> > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> > 
> > Add support for PSE (Power Sourcing Equipment) priority management:
> > - Add priority configuration parameter (prio) for port priority management
> > - Display power domain index, maximum priority, and current priority
> > 
> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> > ---
> >  ethtool.8.in     | 13 +++++++++++++
> >  ethtool.c        |  1 +
> >  netlink/pse-pd.c | 29 +++++++++++++++++++++++++++++
> >  3 files changed, 43 insertions(+)
> > 
> > diff --git a/ethtool.8.in b/ethtool.8.in
> > index 29b8a8c..163b2b0 100644
> > --- a/ethtool.8.in
> > +++ b/ethtool.8.in
> > @@ -561,6 +561,7 @@ ethtool \- query or control network driver and hardware
> > settings .RB [ c33\-pse\-admin\-control
> >  .BR enable | disable ]
> >  .BN c33\-pse\-avail\-pw\-limit N
> > +.BN prio N
> >  .HP
> >  .B ethtool \-\-flash\-module\-firmware
> >  .I devname
> > @@ -1911,6 +1912,15 @@ This attribute specifies the allowed power limit
> > ranges in mW for configuring the c33-pse-avail-pw-limit parameter. It
> > defines the valid power levels that can be assigned to the c33 PSE in
> > compliance with the c33 standard.
> > +.TP
> > +.B power-domain-index
> > +This attribute defines the index of the PSE Power Domain.  
> 
> May be:
> 
> Reports the index of the PSE power domain the port belongs to. Every
> port belongs to exactly one power domain. Port priorities are defined
> within that power domain.
> 
> Each power domain may have its own maximum budget (e.g., 100 W per
> domain) in addition to a system-wide budget (e.g., 200 W overall).
> Domain limits are enforced first: if a single domain reaches its budget,
> only ports in that domain are affected. The system-wide budget is
> enforced across all domains; only when it is exceeded do cross-domain
> priorities apply.

Thanks for the doc review!
Maybe we should not talking about cross-domain priority yet, we don't know how
PSE are supposed to behave on that specific case. 

...

> > +	if (tb[ETHTOOL_A_PSE_PRIO]) {
> > +		u32 val;
> > +
> > +		val = mnl_attr_get_u32(tb[ETHTOOL_A_PSE_PRIO]);
> > +		print_uint(PRINT_ANY, "priority", "Priority %u\n", val);  
> 
> missing colon
> 		print_uint(PRINT_ANY, "priority", "Priority: %u\n", val);

Well spotted, thanks!

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