diff mbox series

[net-next,2/9] ethtool: Expand Ethernet Power Equipment with PoE alongside PoDL

Message ID 20231116-feature_poe-v1-2-be48044bf249@bootlin.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: Add support for Power over Ethernet (PoE) | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3025 this patch: 3025
netdev/cc_maintainers warning 3 maintainers not CCed: piergiorgio.beruto@gmail.com vladimir.oltean@nxp.com mailhol.vincent@wanadoo.fr
netdev/build_clang success Errors and warnings before: 1289 this patch: 1289
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 3237 this patch: 3237
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 86 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Kory Maincent Nov. 16, 2023, 2:01 p.m. UTC
In the current PSE interface for Ethernet Power Equipment, support is
limited to PoDL. This patch extends the interface to accommodate the
objects specified in IEEE 802.3-2022 145.2 for Power sourcing
Equipment (PSE).

The following objects are now supported and considered mandatory:
- IEEE 802.3-2022 30.9.1.1.5 aPSEPowerDetectionStatus
- IEEE 802.3-2022 30.9.1.1.2 aPSEAdminState
- IEEE 802.3-2022 30.9.1.2.1 aPSEAdminControl

To avoid confusion between "PoDL PSE" and "PSE", which have similar names
but distinct values, we have followed the suggestion of Oleksij Rempel to
maintain separate naming schemes for each. You can find more details in the
discussion thread here:
https://lore.kernel.org/netdev/20230912110637.GI780075@pengutronix.de/

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 include/linux/pse-pd/pse.h           |  9 ++++++++
 include/uapi/linux/ethtool.h         | 43 ++++++++++++++++++++++++++++++++++++
 include/uapi/linux/ethtool_netlink.h |  3 +++
 3 files changed, 55 insertions(+)

Comments

Andrew Lunn Nov. 18, 2023, 5:38 p.m. UTC | #1
On Thu, Nov 16, 2023 at 03:01:34PM +0100, Kory Maincent wrote:
> In the current PSE interface for Ethernet Power Equipment, support is
> limited to PoDL. This patch extends the interface to accommodate the
> objects specified in IEEE 802.3-2022 145.2 for Power sourcing
> Equipment (PSE).

Sorry for taking a while getting to these patches. Plumbers and other
patches have been keeping me busy.

I'm trying to get my head around naming... Is there some sort of
hierarchy? Is PSE the generic concept for putting power down the
cable? Then you have the sub-type PoDL, and the sub-type PoE?

>  struct pse_control_config {
>  	enum ethtool_podl_pse_admin_state podl_admin_control;
> +	enum ethtool_pse_admin_state admin_control;

When i look at this, it seems to me admin_control should be generic
across all schemes which put power down the cable, and
podl_admin_control is specific to how PoDL puts power down the cable.

Since you appear to be adding support for a second way to put power
down the cable, i would expect something like poe_admin_control being
added here. But maybe that is in a later patch?

      Andrew
Kory Maincent Nov. 20, 2023, 10:09 a.m. UTC | #2
+Oleksij

Sorry forgot to CC you the series.
Maybe you should add yourself to the MAINTAINERS of pse-pd drivers subsystem?

On Sat, 18 Nov 2023 18:38:43 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Thu, Nov 16, 2023 at 03:01:34PM +0100, Kory Maincent wrote:
> > In the current PSE interface for Ethernet Power Equipment, support is
> > limited to PoDL. This patch extends the interface to accommodate the
> > objects specified in IEEE 802.3-2022 145.2 for Power sourcing
> > Equipment (PSE).  
> 
> Sorry for taking a while getting to these patches. Plumbers and other
> patches have been keeping me busy.

Don't worry you are doing a great job as a net maintainer and I won't raise any
remarks on delay considering how you are doing your job.
Thanks again for your review!!

> I'm trying to get my head around naming... Is there some sort of
> hierarchy? Is PSE the generic concept for putting power down the
> cable? Then you have the sub-type PoDL, and the sub-type PoE?

In fact as we discussed with Oleksij I decided to keep the naming as close as
possible to the IEEE 802.3 standard.
On the standard the PODL is naming like this aPoDLPSE* (ex: aPoDLPSEAdminState)
and the PSE is naming like this aPSE* (ex: aPSEAdminState) without any PoE
prefix. Maybe it is due to PoE being supported before PoDL and they didn't
expect the PoDL part.

> >  struct pse_control_config {
> >  	enum ethtool_podl_pse_admin_state podl_admin_control;
> > +	enum ethtool_pse_admin_state admin_control;  
> 
> When i look at this, it seems to me admin_control should be generic
> across all schemes which put power down the cable, and
> podl_admin_control is specific to how PoDL puts power down the cable.
>
> Since you appear to be adding support for a second way to put power
> down the cable, i would expect something like poe_admin_control being
> added here. But maybe that is in a later patch?

No as said above admin_control is for PoE and podl_admin_control is for PoDL.
Maybe you prefer to use poe_admin_control, and add poe prefix in the poe
variables. It will differ a bit from the IEEE standard naming but I agreed that
it would be more understandable in the development part.

I am open to the change.
Oleksij do you agree?

Regards,
Oleksij Rempel Nov. 20, 2023, 11:10 a.m. UTC | #3
Köry,

On Mon, Nov 20, 2023 at 11:09:44AM +0100, Köry Maincent wrote:
> +Oleksij
> 
> Sorry forgot to CC you the series.
> Maybe you should add yourself to the MAINTAINERS of pse-pd drivers subsystem?

ack, i'll take a look at this.

> On Sat, 18 Nov 2023 18:38:43 +0100
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > On Thu, Nov 16, 2023 at 03:01:34PM +0100, Kory Maincent wrote:
> > > In the current PSE interface for Ethernet Power Equipment, support is
> > > limited to PoDL. This patch extends the interface to accommodate the
> > > objects specified in IEEE 802.3-2022 145.2 for Power sourcing
> > > Equipment (PSE).  
> > 
> > Sorry for taking a while getting to these patches. Plumbers and other
> > patches have been keeping me busy.
> 
> Don't worry you are doing a great job as a net maintainer and I won't raise any
> remarks on delay considering how you are doing your job.
> Thanks again for your review!!
> 
> > I'm trying to get my head around naming... Is there some sort of
> > hierarchy? Is PSE the generic concept for putting power down the
> > cable? Then you have the sub-type PoDL, and the sub-type PoE?
> 
> In fact as we discussed with Oleksij I decided to keep the naming as close as
> possible to the IEEE 802.3 standard.
> On the standard the PODL is naming like this aPoDLPSE* (ex: aPoDLPSEAdminState)
> and the PSE is naming like this aPSE* (ex: aPSEAdminState) without any PoE
> prefix. Maybe it is due to PoE being supported before PoDL and they didn't
> expect the PoDL part.

"PoE" (initially Power via MDI?) and PoDL have kind of different technologies.
They use different negotiation and need different physical implementation.
IEEE 802.3 standard is trying to be backwards and kind of forwards compatible
for PoE. But not compatible between PoE and PoDL.

In general, it is not just about enabling or disabling power.
"admin_state" == enable is other way to say - "do the right thing".
And the "right thing" may include some kind of communication between PSE
(Power Source Equipment) and PD (Powered Device).

Since, some variants of  Single Pair Ethernet (SPE) are using same
auto negotiation protocol as not SPE variants. I can imagine, that some
day we will see a hybrid (SPE+nonSPE) devices. Wich will need to
support both: PoE and PoDL. I assume, in that case, we wont to be able
to control both variants separately.

This is why I prefer to have mapping of IEEE 802.3 specification to the
user space as close as possible.

> > >  struct pse_control_config {
> > >  	enum ethtool_podl_pse_admin_state podl_admin_control;
> > > +	enum ethtool_pse_admin_state admin_control;  
> > 
> > When i look at this, it seems to me admin_control should be generic
> > across all schemes which put power down the cable, and
> > podl_admin_control is specific to how PoDL puts power down the cable.
> >
> > Since you appear to be adding support for a second way to put power
> > down the cable, i would expect something like poe_admin_control being
> > added here. But maybe that is in a later patch?
> 
> No as said above admin_control is for PoE and podl_admin_control is for PoDL.
> Maybe you prefer to use poe_admin_control, and add poe prefix in the poe
> variables. It will differ a bit from the IEEE standard naming but I agreed that
> it would be more understandable in the development part.

Official name for "PoE" is "Power via Media Dependent Interface". PoE is
not used in the IEEE 802.3-2018. Using names not used in the specification,
make development even harder :)
Especially since there are even more marketing names (names not used in the
specification) for different PoE variants:
- 802.3af (802.3at Type 1), PoE
- 802.3at Type 2, PoE+
- 802.3bt Type 3, 4PPoE or PoE++
- 802.3bt Type 4, 4PPoE or PoE++

Regards,
Oleksij
Andrew Lunn Nov. 20, 2023, 6 p.m. UTC | #4
> > > >  struct pse_control_config {
> > > >  	enum ethtool_podl_pse_admin_state podl_admin_control;
> > > > +	enum ethtool_pse_admin_state admin_control;  
> > > 
> > > When i look at this, it seems to me admin_control should be generic
> > > across all schemes which put power down the cable, and
> > > podl_admin_control is specific to how PoDL puts power down the cable.
> > >
> > > Since you appear to be adding support for a second way to put power
> > > down the cable, i would expect something like poe_admin_control being
> > > added here. But maybe that is in a later patch?
> > 
> > No as said above admin_control is for PoE and podl_admin_control is for PoDL.
> > Maybe you prefer to use poe_admin_control, and add poe prefix in the poe
> > variables. It will differ a bit from the IEEE standard naming but I agreed that
> > it would be more understandable in the development part.
> 
> Official name for "PoE" is "Power via Media Dependent Interface". PoE is
> not used in the IEEE 802.3-2018. Using names not used in the specification,
> make development even harder :)
> Especially since there are even more marketing names (names not used in the
> specification) for different PoE variants:
> - 802.3af (802.3at Type 1), PoE
> - 802.3at Type 2, PoE+
> - 802.3bt Type 3, 4PPoE or PoE++
> - 802.3bt Type 4, 4PPoE or PoE++

From the 2018 standard:

  1.4.407 Power Sourcing Equipment (PSE): A DTE or midspan device that
  provides the power to a single link section. PSEs are defined for
  use with two different types of balanced twisted-pair PHYs. When
  used with 2 or 4 pair balanced twisted-pair (BASE-T) PHYs, (see IEEE
  Std 802.3, Clause 33), DTE powering is intended to provide a single
  10BASE-T, 100BASE-TX, or 1000BASE-T device with a unified interface
  for both the data it requires and the power to process these
  data. When used with single balanced twisted-pair (BASE-T1) PHYs
  (see IEEE Std 802.3, Clause 104), DTE powering is intended to
  provide a single 100BASE-T1 or 1000BASE-T1 device with a unified
  interface for both the data it requires and the power to process
  these data. A PSE used with balanced single twisted-pair PHYs is
  also referred to as a PoDL PSE.

So it seems like, anything not PoDL PSE does not have a name :-(

However, everything not PoDL PSE seems to be clause 33. So how about:

	enum ethtool_podl_pse_admin_state podl_admin_control;
	enum ethtool_c33_pse_admin_state c33_admin_control;  

At least inside the kernel we use c22, c45, c37 etc. I'm not sure they
are visible to userspace, but if we don't have a better name, maybe we
have to use c33 in userspace as well.

I do think naming like this makes it clear we are talking about two
parallel technologies, not a generic layer and then extensions for
podl.

What do you think?

	Andrew
Oleksij Rempel Nov. 20, 2023, 8:42 p.m. UTC | #5
On Mon, Nov 20, 2023 at 07:00:03PM +0100, Andrew Lunn wrote:
> > > > >  struct pse_control_config {
> > > > >  	enum ethtool_podl_pse_admin_state podl_admin_control;
> > > > > +	enum ethtool_pse_admin_state admin_control;  
> > > > 
> > > > When i look at this, it seems to me admin_control should be generic
> > > > across all schemes which put power down the cable, and
> > > > podl_admin_control is specific to how PoDL puts power down the cable.
> > > >
> > > > Since you appear to be adding support for a second way to put power
> > > > down the cable, i would expect something like poe_admin_control being
> > > > added here. But maybe that is in a later patch?
> > > 
> > > No as said above admin_control is for PoE and podl_admin_control is for PoDL.
> > > Maybe you prefer to use poe_admin_control, and add poe prefix in the poe
> > > variables. It will differ a bit from the IEEE standard naming but I agreed that
> > > it would be more understandable in the development part.
> > 
> > Official name for "PoE" is "Power via Media Dependent Interface". PoE is
> > not used in the IEEE 802.3-2018. Using names not used in the specification,
> > make development even harder :)
> > Especially since there are even more marketing names (names not used in the
> > specification) for different PoE variants:
> > - 802.3af (802.3at Type 1), PoE
> > - 802.3at Type 2, PoE+
> > - 802.3bt Type 3, 4PPoE or PoE++
> > - 802.3bt Type 4, 4PPoE or PoE++
> 
> From the 2018 standard:
> 
>   1.4.407 Power Sourcing Equipment (PSE): A DTE or midspan device that
>   provides the power to a single link section. PSEs are defined for
>   use with two different types of balanced twisted-pair PHYs. When
>   used with 2 or 4 pair balanced twisted-pair (BASE-T) PHYs, (see IEEE
>   Std 802.3, Clause 33), DTE powering is intended to provide a single
>   10BASE-T, 100BASE-TX, or 1000BASE-T device with a unified interface
>   for both the data it requires and the power to process these
>   data. When used with single balanced twisted-pair (BASE-T1) PHYs
>   (see IEEE Std 802.3, Clause 104), DTE powering is intended to
>   provide a single 100BASE-T1 or 1000BASE-T1 device with a unified
>   interface for both the data it requires and the power to process
>   these data. A PSE used with balanced single twisted-pair PHYs is
>   also referred to as a PoDL PSE.
> 
> So it seems like, anything not PoDL PSE does not have a name :-(
> 
> However, everything not PoDL PSE seems to be clause 33. So how about:
> 
> 	enum ethtool_podl_pse_admin_state podl_admin_control;
> 	enum ethtool_c33_pse_admin_state c33_admin_control;  
> 
> At least inside the kernel we use c22, c45, c37 etc. I'm not sure they
> are visible to userspace, but if we don't have a better name, maybe we
> have to use c33 in userspace as well.
> 
> I do think naming like this makes it clear we are talking about two
> parallel technologies, not a generic layer and then extensions for
> podl.
> 
> What do you think?

I'm OK with it.

Köry, can you please include some kernel documentation in your patches?
Something like this. I hope it will help to clarify things :) :

Power Sourcing Equipment (PSE) in IEEE 802.3 Standard
=====================================================

Overview
--------

Power Sourcing Equipment (PSE) is essential in networks for delivering power
along with data over Ethernet cables. It usually refers to devices like
switches and hubs that supply power to Powered Devices (PDs) such as IP
cameras, VoIP phones, and wireless access points.

PSE vs. PoDL PSE
----------------

PSE in the IEEE 802.3 standard generally refers to equipment that provides
power alongside data over Ethernet cables, typically associated with Power over
Ethernet (PoE). 

PoDL PSE, or Power over Data Lines PSE, specifically denotes PSEs operating
with single balanced twisted-pair PHYs, as per Clause 104 of IEEE 802.3. PoDL
is significant in contexts like automotive and industrial controls where power
and data delivery over a single pair is advantageous.

IEEE 802.3-2018 Addendums and Related Clauses
----------------------------------------------

Key addenda to the IEEE 802.3-2018 standard relevant to power delivery over
Ethernet are as follows:

- **802.3af (Approved in 2003-06-12)**: Known as PoE in the market, detailed in
  Clause 33, delivering up to 15.4W of power.
- **802.3at (Approved in 2009-09-11)**: Marketed as PoE+, enhancing PoE as
  covered in Clause 33, increasing power delivery to up to 30W.
- **802.3bt (Approved in 2018-09-27)**: Known as 4PPoE in the market, outlined
  in Clause 33. Type 3 delivers up to 60W, and Type 4 up to 100W.
- **802.3bu (Approved in 2016-12-07)**: Formerly referred to as PoDL, detailed
  in Clause 104. Introduces Classes 0 - 9. Class 9 PoDL PSE delivers up to ~65W

Kernel Naming Convention Recommendations
----------------------------------------

For clarity and consistency within the Linux kernel's networking subsystem, the
following naming conventions are recommended:

- For general PSE (PoE) code, use "c33_pse" key words. For example:
  ``enum ethtool_c33_pse_admin_state c33_admin_control;``.
  This aligns with Clause 33, encompassing various PoE forms.

- For PoDL PSE - specific code, use "podl_pse". For example:
  ``enum ethtool_podl_pse_admin_state podl_admin_control;`` to differentiate
  PoDL PSE settings according to Clause 104.

Summary of Clause 33: Data Terminal Equipment (DTE) Power via Media Dependent Interface (MDI)
-------------------------------------------------------------------------------------------

Clause 33 of the IEEE 802.3 standard defines the functional and electrical
characteristics of Powered Device (PD) and Power Sourcing Equipment (PSE).
These entities enable power delivery using the same generic cabling as for data
transmission, integrating power with data communication for devices such as
10BASE-T, 100BASE-TX, or 1000BASE-T.

Summary of Clause 104: Power over Data Lines (PoDL) of Single Balanced Twisted-Pair Ethernet
-------------------------------------------------------------------------------------------

Clause 104 of the IEEE 802.3 standard delineates the functional and electrical
characteristics of PoDL Powered Devices (PDs) and PoDL Power Sourcing Equipment
(PSEs). These are designed for use with single balanced twisted-pair Ethernet
Physical Layers. In this clause, 'PSE' refers specifically to PoDL PSE, and
'PD' to PoDL PD. The key intent is to provide devices with a unified interface
for both data and the power required to process this data over a single
balanced twisted-pair Ethernet connection.
Andrew Lunn Nov. 20, 2023, 10:14 p.m. UTC | #6
> > However, everything not PoDL PSE seems to be clause 33. So how about:
> > 
> > 	enum ethtool_podl_pse_admin_state podl_admin_control;
> > 	enum ethtool_c33_pse_admin_state c33_admin_control;  
> > 
> > At least inside the kernel we use c22, c45, c37 etc. I'm not sure they
> > are visible to userspace, but if we don't have a better name, maybe we
> > have to use c33 in userspace as well.
> > 
> > I do think naming like this makes it clear we are talking about two
> > parallel technologies, not a generic layer and then extensions for
> > podl.
> > 
> > What do you think?
> 
> I'm OK with it.

Great.

> 
> Köry, can you please include some kernel documentation in your patches?
> Something like this. I hope it will help to clarify things :) :

This is good. I'm just wondering where to put it. Ideally we want to
cross reference to it in both this header file, and in the netlink
UAPI.

	Andrew
Oleksij Rempel Nov. 21, 2023, 5:12 a.m. UTC | #7
On Mon, Nov 20, 2023 at 11:14:32PM +0100, Andrew Lunn wrote:
> > > However, everything not PoDL PSE seems to be clause 33. So how about:
> > > 
> > > 	enum ethtool_podl_pse_admin_state podl_admin_control;
> > > 	enum ethtool_c33_pse_admin_state c33_admin_control;  
> > > 
> > > At least inside the kernel we use c22, c45, c37 etc. I'm not sure they
> > > are visible to userspace, but if we don't have a better name, maybe we
> > > have to use c33 in userspace as well.
> > > 
> > > I do think naming like this makes it clear we are talking about two
> > > parallel technologies, not a generic layer and then extensions for
> > > podl.
> > > 
> > > What do you think?
> > 
> > I'm OK with it.
> 
> Great.
> 
> > 
> > Köry, can you please include some kernel documentation in your patches?
> > Something like this. I hope it will help to clarify things :) :
> 
> This is good. I'm just wondering where to put it. Ideally we want to
> cross reference to it in both this header file, and in the netlink
> UAPI.

Documentation/networking/pse-pd/introduction.rst ?
Kory Maincent Nov. 21, 2023, 10:02 a.m. UTC | #8
On Mon, 20 Nov 2023 19:00:03 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> > Official name for "PoE" is "Power via Media Dependent Interface". PoE is
> > not used in the IEEE 802.3-2018. Using names not used in the specification,
> > make development even harder :)
> > Especially since there are even more marketing names (names not used in the
> > specification) for different PoE variants:
> > - 802.3af (802.3at Type 1), PoE
> > - 802.3at Type 2, PoE+
> > - 802.3bt Type 3, 4PPoE or PoE++
> > - 802.3bt Type 4, 4PPoE or PoE++  
> 
> From the 2018 standard:
> 
>   1.4.407 Power Sourcing Equipment (PSE): A DTE or midspan device that
>   provides the power to a single link section. PSEs are defined for
>   use with two different types of balanced twisted-pair PHYs. When
>   used with 2 or 4 pair balanced twisted-pair (BASE-T) PHYs, (see IEEE
>   Std 802.3, Clause 33), DTE powering is intended to provide a single
>   10BASE-T, 100BASE-TX, or 1000BASE-T device with a unified interface
>   for both the data it requires and the power to process these
>   data. When used with single balanced twisted-pair (BASE-T1) PHYs
>   (see IEEE Std 802.3, Clause 104), DTE powering is intended to
>   provide a single 100BASE-T1 or 1000BASE-T1 device with a unified
>   interface for both the data it requires and the power to process
>   these data. A PSE used with balanced single twisted-pair PHYs is
>   also referred to as a PoDL PSE.
> 
> So it seems like, anything not PoDL PSE does not have a name :-(
> 
> However, everything not PoDL PSE seems to be clause 33. So how about:
> 
> 	enum ethtool_podl_pse_admin_state podl_admin_control;
> 	enum ethtool_c33_pse_admin_state c33_admin_control;  
> 
> At least inside the kernel we use c22, c45, c37 etc. I'm not sure they
> are visible to userspace, but if we don't have a better name, maybe we
> have to use c33 in userspace as well.
> 
> I do think naming like this makes it clear we are talking about two
> parallel technologies, not a generic layer and then extensions for
> podl.
> 
> What do you think?

If we decide to add a prefix, "c33" is precise but less easily understandable,
why not using simply "poe" prefix?
Maybe as POE were originally PMDI you prefer to use c33 which won't change over
time? 

Should I also modify the content of the enum?
ETHTOOL_PSE_ADMIN_STATE_* to ETHTOOL_C33_PSE_ADMIN_*
ETHTOOL_PSE_PW_D_STATUS_* to ETHTOOL_C33_PSE_PW_D_STATUS_*
Andrew Lunn Nov. 21, 2023, 2:19 p.m. UTC | #9
> > However, everything not PoDL PSE seems to be clause 33. So how about:
> > 
> > 	enum ethtool_podl_pse_admin_state podl_admin_control;
> > 	enum ethtool_c33_pse_admin_state c33_admin_control;  
> > 
> > At least inside the kernel we use c22, c45, c37 etc. I'm not sure they
> > are visible to userspace, but if we don't have a better name, maybe we
> > have to use c33 in userspace as well.
> > 
> > I do think naming like this makes it clear we are talking about two
> > parallel technologies, not a generic layer and then extensions for
> > podl.
> > 
> > What do you think?
> 
> If we decide to add a prefix, "c33" is precise but less easily understandable,
> why not using simply "poe" prefix?

I suspect poe has the exact opposite problem, its too imprecise. Its
too much of a marketing name, with no clear meaning. It could even be
some people call podl poe.

To some extent, this is a user space UX problem. We can be precises in
the kernel and the kAPI. What ethtool decides to show to the user
could be different. Although it basically is the same problem.

Do you have ethtool patches? What does the output look like?  Oleksij
did say a hybrid could be possible, so we probably want ethtool to
group these properties together and make it clear what is PoDL and
!PoDL.

> Maybe as POE were originally PMDI you prefer to use c33 which won't change over
> time? 
> 
> Should I also modify the content of the enum?
> ETHTOOL_PSE_ADMIN_STATE_* to ETHTOOL_C33_PSE_ADMIN_*
> ETHTOOL_PSE_PW_D_STATUS_* to ETHTOOL_C33_PSE_PW_D_STATUS_*

Yes. That will help avoid getting PODL and C33 properties missed up.

     Andrew
Kory Maincent Nov. 21, 2023, 2:56 p.m. UTC | #10
On Tue, 21 Nov 2023 15:19:19 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> > > However, everything not PoDL PSE seems to be clause 33. So how about:
> > > 
> > > 	enum ethtool_podl_pse_admin_state podl_admin_control;
> > > 	enum ethtool_c33_pse_admin_state c33_admin_control;  
> > > 
> > > At least inside the kernel we use c22, c45, c37 etc. I'm not sure they
> > > are visible to userspace, but if we don't have a better name, maybe we
> > > have to use c33 in userspace as well.
> > > 
> > > I do think naming like this makes it clear we are talking about two
> > > parallel technologies, not a generic layer and then extensions for
> > > podl.
> > > 
> > > What do you think?  
> > 
> > If we decide to add a prefix, "c33" is precise but less easily
> > understandable, why not using simply "poe" prefix?  
> 
> I suspect poe has the exact opposite problem, its too imprecise. Its
> too much of a marketing name, with no clear meaning. It could even be
> some people call podl poe.
> 
> To some extent, this is a user space UX problem. We can be precises in
> the kernel and the kAPI. What ethtool decides to show to the user
> could be different. Although it basically is the same problem.

Alright, thanks for your answer.

> Do you have ethtool patches? What does the output look like?  Oleksij
> did say a hybrid could be possible, so we probably want ethtool to
> group these properties together and make it clear what is PoDL and
> !PoDL.

No I don't, I am only using ynl for now.
I would be similar to podl:
https://kernel.googlesource.com/pub/scm/network/ethtool/ethtool/+/e6cc6807f87c74d4e5b1f1e9d21d3a74e75a258b/netlink/pse-pd.c

Duplicating the PoDL part with c33. Using the same --set-pse and --show-pse
options.

> > Maybe as POE were originally PMDI you prefer to use c33 which won't change
> > over time? 
> > 
> > Should I also modify the content of the enum?
> > ETHTOOL_PSE_ADMIN_STATE_* to ETHTOOL_C33_PSE_ADMIN_*
> > ETHTOOL_PSE_PW_D_STATUS_* to ETHTOOL_C33_PSE_PW_D_STATUS_*  
> 
> Yes. That will help avoid getting PODL and C33 properties missed up.

Alright.

Regards,
diff mbox series

Patch

diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 199cf4ae3cf2..25490d0c682d 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -17,9 +17,12 @@  struct pse_controller_dev;
  *
  * @podl_admin_control: set PoDL PSE admin control as described in
  *	IEEE 802.3-2018 30.15.1.2.1 acPoDLPSEAdminControl
+ * @admin_control: set PSE admin control as described in
+ *	IEEE 802.3-2022 30.9.1.2.1 acPSEAdminControl
  */
 struct pse_control_config {
 	enum ethtool_podl_pse_admin_state podl_admin_control;
+	enum ethtool_pse_admin_state admin_control;
 };
 
 /**
@@ -29,10 +32,16 @@  struct pse_control_config {
  *	functions. IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState
  * @podl_pw_status: power detection status of the PoDL PSE.
  *	IEEE 802.3-2018 30.15.1.1.3 aPoDLPSEPowerDetectionStatus:
+ * @admin_state: operational state of the PSE
+ *	functions. IEEE 802.3-2022 30.9.1.1.2 aPSEAdminState
+ * @pw_status: power detection status of the PSE.
+ *	IEEE 802.3-2022 30.9.1.1.5 aPSEPowerDetectionStatus:
  */
 struct pse_control_status {
 	enum ethtool_podl_pse_admin_state podl_admin_state;
 	enum ethtool_podl_pse_pw_d_status podl_pw_status;
+	enum ethtool_pse_admin_state admin_state;
+	enum ethtool_pse_pw_d_status pw_status;
 };
 
 /**
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index f7fba0dc87e5..eaf7f7ff41f1 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -752,6 +752,49 @@  enum ethtool_module_power_mode {
 	ETHTOOL_MODULE_POWER_MODE_HIGH,
 };
 
+/**
+ * enum ethtool_pse_admin_state - operational state of the PoDL PSE
+ *	functions. IEEE 802.3-2022 30.9.1.1.2 aPSEAdminState
+ * @ETHTOOL_PSE_ADMIN_STATE_UNKNOWN: state of PSE functions is unknown
+ * @ETHTOOL_PSE_ADMIN_STATE_DISABLED: PSE functions are disabled
+ * @ETHTOOL_PSE_ADMIN_STATE_ENABLED: PSE functions are enabled
+ */
+enum ethtool_pse_admin_state {
+	ETHTOOL_PSE_ADMIN_STATE_UNKNOWN = 1,
+	ETHTOOL_PSE_ADMIN_STATE_DISABLED,
+	ETHTOOL_PSE_ADMIN_STATE_ENABLED,
+};
+
+/**
+ * enum ethtool_pse_pw_d_status - power detection status of the PSE.
+ *	IEEE 802.3-2022 30.9.1.1.3 aPoDLPSEPowerDetectionStatus:
+ * @ETHTOOL_PSE_PW_D_STATUS_UNKNOWN: PSE status is unknown
+ * @ETHTOOL_PSE_PW_D_STATUS_DISABLED: "The enumeration “disabled”
+ *	indicates that the PSE State diagram is in the state DISABLED."
+ * @ETHTOOL_PSE_PW_D_STATUS_SEARCHING: "The enumeration “searching”
+ *	indicates the PSE State diagram is in a state other than those
+ *	listed."
+ * @ETHTOOL_PSE_PW_D_STATUS_DELIVERING: "The enumeration
+ *	“deliveringPower” indicates that the PSE State diagram is in the
+ *	state POWER_ON."
+ * @ETHTOOL_PSE_PW_D_STATUS_TEST: "The enumeration “test” indicates that
+ *	the PSE State diagram is in the state TEST_MODE."
+ * @ETHTOOL_PSE_PW_D_STATUS_FAULT: "The enumeration “fault” indicates that
+ *	the PSE State diagram is in the state TEST_ERROR."
+ * @ETHTOOL_PSE_PW_D_STATUS_OTHERFAULT: "The enumeration “otherFault”
+ *	indicates that the PSE State diagram is in the state IDLE due to
+ *	the variable error_condition = true."
+ */
+enum ethtool_pse_pw_d_status {
+	ETHTOOL_PSE_PW_D_STATUS_UNKNOWN = 1,
+	ETHTOOL_PSE_PW_D_STATUS_DISABLED,
+	ETHTOOL_PSE_PW_D_STATUS_SEARCHING,
+	ETHTOOL_PSE_PW_D_STATUS_DELIVERING,
+	ETHTOOL_PSE_PW_D_STATUS_TEST,
+	ETHTOOL_PSE_PW_D_STATUS_FAULT,
+	ETHTOOL_PSE_PW_D_STATUS_OTHERFAULT,
+};
+
 /**
  * enum ethtool_podl_pse_admin_state - operational state of the PoDL PSE
  *	functions. IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 73e2c10dc2cc..2a27f37c71f7 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -895,6 +895,9 @@  enum {
 	ETHTOOL_A_PODL_PSE_ADMIN_STATE,		/* u32 */
 	ETHTOOL_A_PODL_PSE_ADMIN_CONTROL,	/* u32 */
 	ETHTOOL_A_PODL_PSE_PW_D_STATUS,		/* u32 */
+	ETHTOOL_A_PSE_ADMIN_STATE,		/* u32 */
+	ETHTOOL_A_PSE_ADMIN_CONTROL,		/* u32 */
+	ETHTOOL_A_PSE_PW_D_STATUS,		/* u32 */
 
 	/* add new constants above here */
 	__ETHTOOL_A_PSE_CNT,