netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleksij Rempel <o.rempel@pengutronix.de>
To: Kory Maincent <kory.maincent@bootlin.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Donald Hunter <donald.hunter@gmail.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Dent Project <dentproject@linuxfoundation.org>,
	kernel@pengutronix.de
Subject: Re: [PATCH net-next v3 5/7] net: ethtool: Add new power limit get and set features
Date: Sun, 16 Jun 2024 08:07:20 +0200	[thread overview]
Message-ID: <Zm6BGJxu4bLVszFD@pengutronix.de> (raw)
In-Reply-To: <Zm3dTuXuVEF9MhDS@pengutronix.de>

On Sat, Jun 15, 2024 at 08:28:30PM +0200, Oleksij Rempel wrote:
> > > diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
> > > index 7dbf2ef3ac0e..a78b6aea84af 100644
> > > --- a/Documentation/networking/ethtool-netlink.rst
> > > +++ b/Documentation/networking/ethtool-netlink.rst
> > > @@ -1737,6 +1737,7 @@ Kernel response contents:
> > >                                                    PoE PSE.
> > >    ``ETHTOOL_A_C33_PSE_EXT_SUBSTATE``         u32  power extended substatus of
> > >                                                    the PoE PSE.
> > > +  ``ETHTOOL_A_C33_PSE_PW_LIMIT``             u32  power limit of the PoE PSE.
> > >    ======================================  ======  =============================
> > >  
> > >  When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` attribute identifies
> > > @@ -1799,6 +1800,9 @@ Possible values are:
> > >  		  ethtool_c33_pse_ext_substate_power_not_available
> > >  		  ethtool_c33_pse_ext_substate_short_detected
> > >  
> > > +When set, the optional ``ETHTOOL_A_C33_PSE_PW_LIMIT`` attribute identifies
> > > +the C33 PSE power limit in mW.

Except of current value, we need an interface to return list of supported
ranges. For example a controller with flexible configuration will have
one entry 

Proposed interface may look like this:

  ``ETHTOOL_A_C33_PSE_AVAIL_PWR_VAL_LIMIT``  u32  Get PoE PSE currently configured power value limit
  ``ETHTOOL_A_C33_PSE_PWR_LIMIT_RANGES``     nested  Supported power limit configuration ranges  
  ======================================  ======  =============================

 +------------------------------------------+--------+----------------------------+
 | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_RANGES``   | nested | array of power limit ranges|
 +-+----------------------------------------+--------+----------------------------+
 | | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_RANGE_ENTRY`` | nested | one power limit range  |
 +-+-+--------------------------------------+--------+----------------------------+
 | | | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_MIN``  | u32    | minimum power value (mW)   |
 +-+-+--------------------------------------+--------+----------------------------+
 | | | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_MAX``  | u32    | maximum power value (mW)   |
 +-+-+--------------------------------------+--------+----------------------------+

The min/max values should provide ranges actually configurable by PSE controller.
If controller works with fixed classes, the min and max values will be equal.

The ethtool output may look like this:

$ ethtool --get-pse eth0

Power Information for eth0:
=====================================
Current Power Limit: 15000 mW
Current Power Consumption: 12000 mW

Supported Power Limit Ranges:
  - Range 1: 0 - 7500 mW
  - Range 2: 7501 - 15000 mW
  - Range 3: 15001 - 30000 mW

Port Power Priority: 3
Supported Priority Range: 1 - 5

Pairs in Use: 4
Pair Configuration Type: Alternative A (MDI-X) and Alternative B(S)
PSE Type: Type 4
Detected PD Class: Class 5 (40000 mW max)

Low-Level Classification:
  - Classification Type: Autodetected
  - Configured PD Class: Class 5 (40000 mW max)

Maintain Power Signature (MPS) State: Present


> > > +
> > >  PSE_SET
> > >  =======
> > >  
> > > @@ -1810,6 +1814,7 @@ Request contents:
> > >    ``ETHTOOL_A_PSE_HEADER``                nested  request header
> > >    ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL``       u32  Control PoDL PSE Admin state
> > >    ``ETHTOOL_A_C33_PSE_ADMIN_CONTROL``        u32  Control PSE Admin state
> > > +  ``ETHTOOL_A_C33_PSE_PW_LIMIT``             u32  Control PoE PSE power limit
> > >    ======================================  ======  =============================
> > >  
> > >  When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL`` attribute is used
> > > @@ -1820,6 +1825,9 @@ to control PoDL PSE Admin functions. This option is implementing
> > >  The same goes for ``ETHTOOL_A_C33_PSE_ADMIN_CONTROL`` implementing
> > >  ``IEEE 802.3-2022`` 30.9.1.2.1 acPSEAdminControl.
> > >  
> > > +When set, the optional ``ETHTOOL_A_C33_PSE_PW_LIMIT`` attribute is used
> > > +to control C33 PSE power limit in mW.
> > 
> > 
> > The corresponding name int the IEEE 802.3-2022 seems to be pse_avail_pwr
> > in 145.2.5.4 Variables and pse_available_power in 33.2.4.4 Variables.
> > 
> > This variable is using classes instead of mW. pd692x0 seems to use
> > classes instead of mW too. May be it is better to use classes for UAPI
> > too? 
> 
> Huh... i took some more time to investigate it. Looks like there is no
> simple answer. Some devices seems to write power class on the box. Other
> client devices write power consumption in watts. IEEE 802.3-2022
> provides LLDP specification with PowerValue for watts and PowerClass for
> classes. Different product user interfaces provide class and/or watts.
> So, let's go with watts then. Please update the name to something like
> pse_available_power_value or pse_available_power_value_limit and
> document how it is related to State diagrams in the IEEE spec.

Here is proposal for documentation:

  ``ETHTOOL_A_C33_PSE_AVAIL_PWR_VAL_LIMIT``  u32  Control PoE PSE available power value limit

When set, the optional ``ETHTOOL_A_C33_PSE_AVAIL_PWR_VAL_LIMIT`` attribute is
used  to control the available power value limit for C33 PSE in milliwatts.
This attribute corresponds  to the `pse_available_power` variable described in
``IEEE 802.3-2022`` 33.2.4.4 Variables  and `pse_avail_pwr` in 145.2.5.4
Variables, which are described in power classes. 

It was decided to use milliwatts for this interface to unify it with other
power monitoring interfaces, which also use milliwatts, and to align with
various existing products that document power consumption in watts rather than
classes. If power limit configuration based on classes is needed, the
conversion can be done in user space, for example by ethtool.

-- 
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 |

  reply	other threads:[~2024-06-16  6:07 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-14 14:33 [PATCH net-next v3 0/7] net: pse-pd: Add new PSE c33 features Kory Maincent
2024-06-14 14:33 ` [PATCH net-next v3 1/7] net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state Kory Maincent
2024-06-15 11:22   ` Oleksij Rempel
2024-06-17 13:47     ` Kory Maincent
2024-06-17 19:55       ` Oleksij Rempel
2024-06-21 16:29         ` Kory Maincent
2024-06-22  5:06           ` Oleksij Rempel
2024-06-25  9:18         ` Kory Maincent
2024-06-25 10:33           ` Oleksij Rempel
2024-06-25 11:59             ` Kory Maincent
2024-06-14 14:33 ` [PATCH net-next v3 2/7] netlink: specs: Expand the PSE netlink command with C33 new features Kory Maincent
2024-06-17  8:01   ` Donald Hunter
2024-06-14 14:33 ` [PATCH net-next v3 3/7] net: pse-pd: pd692x0: Expand ethtool status message Kory Maincent
2024-06-14 14:33 ` [PATCH net-next v3 4/7] net: pse-pd: Add new power limit get and set c33 features Kory Maincent
2024-06-14 14:33 ` [PATCH net-next v3 5/7] net: ethtool: Add new power limit get and set features Kory Maincent
2024-06-15 15:59   ` Oleksij Rempel
2024-06-15 18:28     ` Oleksij Rempel
2024-06-16  6:07       ` Oleksij Rempel [this message]
2024-06-17 16:14         ` Kory Maincent
2024-06-17 19:57           ` Oleksij Rempel
2024-06-14 14:33 ` [PATCH net-next v3 6/7] netlink: specs: Expand the PSE netlink command with C33 pw-limit attributes Kory Maincent
2024-06-17  8:03   ` Donald Hunter
2024-06-17  9:53     ` Kory Maincent
2024-06-14 14:33 ` [PATCH net-next v3 7/7] net: pse-pd: pd692x0: Enhance with new current limit and voltage read callbacks Kory Maincent

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zm6BGJxu4bLVszFD@pengutronix.de \
    --to=o.rempel@pengutronix.de \
    --cc=davem@davemloft.net \
    --cc=dentproject@linuxfoundation.org \
    --cc=donald.hunter@gmail.com \
    --cc=edumazet@google.com \
    --cc=kernel@pengutronix.de \
    --cc=kory.maincent@bootlin.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).