[PATCH net-next v3 0/7] net: pse-pd: Add new PSE c33 features

[PATCH net-next v3 0/7] net: pse-pd: Add new PSE c33 features

[Kory Maincent]

From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

This patch series adds new c33 features to the PSE API.
- Expand the PSE PI informations status with power, class and failure
  reason
- Add the possibility to get and set the PSE PIs power limit

Changes in v3:
- Use u32 instead of u8 size for c33 pse extended state and substate.
- Reformat the state and substate enumeration to follow Oleksij proposal which
  is more IEEE 802.3 standard compliant
- Sent the first patch standalone in net.
- Link to v2: https://lore.kernel.org/r/20240607-feature_poe_power_cap-v2-0-c03c2deb83ab@bootlin.com

Changes in v2:
- Use uA and uV instead of mA and mV to have more precision in the power
  calculation. Need to use 64bit variables for the calculation.
- Modify the pd-92x0behavior in case of setting the current out of the
  available ranges. Report an error now.
- Link to v1: https://lore.kernel.org/r/20240529-feature_poe_power_cap-v1-0-0c4b1d5953b8@bootlin.com

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Kory Maincent (7):
      net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state
      netlink: specs: Expand the PSE netlink command with C33 new features
      net: pse-pd: pd692x0: Expand ethtool status message
      net: pse-pd: Add new power limit get and set c33 features
      net: ethtool: Add new power limit get and set features
      netlink: specs: Expand the PSE netlink command with C33 pw-limit attributes
      net: pse-pd: pd692x0: Enhance with new current limit and voltage read callbacks

 Documentation/netlink/specs/ethtool.yaml     |  40 ++++
 Documentation/networking/ethtool-netlink.rst |  45 +++++
 drivers/net/pse-pd/pd692x0.c                 | 292 ++++++++++++++++++++++++++-
 drivers/net/pse-pd/pse_core.c                | 172 +++++++++++++++-
 include/linux/ethtool.h                      |  16 ++
 include/linux/pse-pd/pse.h                   |  42 ++++
 include/uapi/linux/ethtool.h                 | 212 +++++++++++++++++++
 include/uapi/linux/ethtool_netlink.h         |   5 +
 net/ethtool/pse-pd.c                         |  69 ++++++-
 9 files changed, 874 insertions(+), 19 deletions(-)
---
base-commit: 94720a40bd1c6de0780f90f4f68d76be1d2c6bd8
change-id: 20240425-feature_poe_power_cap-18e90ba7294b

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

[PATCH net-next v3 1/7] net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state

[Kory Maincent]

From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

This update expands the status information provided by ethtool for PSE c33.
It includes details such as the detected class, current power delivered,
and extended state information.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Change in v2:
- Move on PSE string error messages to ETHTOOL_LINK_EXT_STATE and
  ETHTOOL_LINK_EXT_SUBSTATE with fixed enumeration in aim to unify
  interface diagnostic.

Change in v3:
- Add ethtool netlink documentation and kdoc.
- Move on to u32 for state and substate.
- Update C33 pse extended state and substate following Oleksij proposal
---
 Documentation/networking/ethtool-netlink.rst |  37 +++++
 include/linux/ethtool.h                      |  16 ++
 include/linux/pse-pd/pse.h                   |   8 +
 include/uapi/linux/ethtool.h                 | 212 +++++++++++++++++++++++++++
 include/uapi/linux/ethtool_netlink.h         |   4 +
 net/ethtool/pse-pd.c                         |  29 +++-
 6 files changed, 305 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 160bfb0ae8ba..7dbf2ef3ac0e 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1730,6 +1730,13 @@ Kernel response contents:
                                                   PSE functions.
   ``ETHTOOL_A_C33_PSE_PW_D_STATUS``          u32  power detection status of the
                                                   PoE PSE.
+  ``ETHTOOL_A_C33_PSE_PW_CLASS``             u32  power class of the PoE PSE.
+  ``ETHTOOL_A_C33_PSE_ACTUAL_PW``            u32  actual power drawn on the
+                                                  PoE PSE.
+  ``ETHTOOL_A_C33_PSE_EXT_STATE``            u32  power extended state of the
+                                                  PoE PSE.
+  ``ETHTOOL_A_C33_PSE_EXT_SUBSTATE``         u32  power extended substatus of
+                                                  the PoE PSE.
   ======================================  ======  =============================
 
 When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` attribute identifies
@@ -1762,6 +1769,36 @@ The same goes for ``ETHTOOL_A_C33_PSE_ADMIN_PW_D_STATUS`` implementing
 .. kernel-doc:: include/uapi/linux/ethtool.h
     :identifiers: ethtool_c33_pse_pw_d_status
 
+When set, the optional ``ETHTOOL_A_C33_PSE_PW_CLASS`` attribute identifies
+the power class of the C33 PSE. It depends on the class negotiated between
+the PSE and the PD. This option is corresponding to ``IEEE 802.3-2022``
+30.9.1.1.8 aPSEPowerClassification.
+
+When set, the optional ``ETHTOOL_A_C33_PSE_ACTUAL_PW`` attribute identifies
+This option is corresponding to ``IEEE 802.3-2022`` 30.9.1.1.23 aPSEActualPower.
+Actual power is reported in mW.
+
+When set, the optional ``ETHTOOL_A_C33_PSE_EXT_STATE`` attribute identifies
+the extended error state of the C33 PSE. Possible values are:
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+    :identifiers: ethtool_c33_pse_ext_state
+
+When set, the optional ``ETHTOOL_A_C33_PSE_EXT_SUBSTATE`` attribute identifies
+the extended error state of the C33 PSE. Possible values are:
+Possible values are:
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+    :identifiers: ethtool_c33_pse_ext_substate_class_num_events
+		  ethtool_c33_pse_ext_substate_error_condition
+		  ethtool_c33_pse_ext_substate_mr_pse_enable
+		  ethtool_c33_pse_ext_substate_option_detect_ted
+		  ethtool_c33_pse_ext_substate_option_vport_lim
+		  ethtool_c33_pse_ext_substate_ovld_detected
+		  ethtool_c33_pse_ext_substate_pd_dll_power_type
+		  ethtool_c33_pse_ext_substate_power_not_available
+		  ethtool_c33_pse_ext_substate_short_detected
+
 PSE_SET
 =======
 
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 6fd9107d3cc0..38a07e7a71d4 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -1155,4 +1155,20 @@ struct ethtool_forced_speed_map {
 
 void
 ethtool_forced_speed_maps_init(struct ethtool_forced_speed_map *maps, u32 size);
+
+/* C33 PSE extended state and substate. */
+struct ethtool_c33_pse_ext_state_info {
+	enum ethtool_c33_pse_ext_state c33_pse_ext_state;
+	union {
+		enum ethtool_c33_pse_ext_substate_error_condition error_condition;
+		enum ethtool_c33_pse_ext_substate_mr_pse_enable mr_pse_enable;
+		enum ethtool_c33_pse_ext_substate_option_detect_ted option_detect_ted;
+		enum ethtool_c33_pse_ext_substate_option_vport_lim option_vport_lim;
+		enum ethtool_c33_pse_ext_substate_ovld_detected ovld_detected;
+		enum ethtool_c33_pse_ext_substate_pd_dll_power_type pd_dll_power_type;
+		enum ethtool_c33_pse_ext_substate_power_not_available power_not_available;
+		enum ethtool_c33_pse_ext_substate_short_detected short_detected;
+		u32 __c33_pse_ext_substate;
+	};
+};
 #endif /* _LINUX_ETHTOOL_H */
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 6eec24ffa866..38b9308e5e7a 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -36,12 +36,20 @@ struct pse_control_config {
  *	functions. IEEE 802.3-2022 30.9.1.1.2 aPSEAdminState
  * @c33_pw_status: power detection status of the PSE.
  *	IEEE 802.3-2022 30.9.1.1.5 aPSEPowerDetectionStatus:
+ * @c33_pw_class: detected class of a powered PD
+ *	IEEE 802.3-2022 30.9.1.1.8 aPSEPowerClassification
+ * @c33_actual_pw: power currently delivered by the PSE in mW
+ *	IEEE 802.3-2022 30.9.1.1.23 aPSEActualPower
+ * @c33_ext_state_info: extended state information of the PSE
  */
 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_c33_pse_admin_state c33_admin_state;
 	enum ethtool_c33_pse_pw_d_status c33_pw_status;
+	u32 c33_pw_class;
+	u32 c33_actual_pw;
+	struct ethtool_c33_pse_ext_state_info c33_ext_state_info;
 };
 
 /**
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 8733a3117902..5bf8cfc4e9ad 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -752,6 +752,218 @@ enum ethtool_module_power_mode {
 	ETHTOOL_MODULE_POWER_MODE_HIGH,
 };
 
+/**
+ * enum ethtool_c33_pse_ext_state - groups of PSE extended states
+ *      functions. IEEE 802.3-2022 33.2.4.4 Variables
+ *
+ * @ETHTOOL_C33_PSE_EXT_STATE_CLASS_NUM_EVENTS: Group of class_num_events states
+ * @ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION: Group of error_condition states
+ * @ETHTOOL_C33_PSE_EXT_STATE_MR_PSE_ENABLE: Group of mr_pse_enable states
+ * @ETHTOOL_C33_PSE_EXT_STATE_OPTION_DETECT_TED: Group of option_detect_ted
+ *	states
+ * @ETHTOOL_C33_PSE_EXT_STATE_OPTION_VPORT_LIM: Group of option_vport_lim states
+ * @ETHTOOL_C33_PSE_EXT_STATE_OVLD_DETECTED: Group of ovld_detected states
+ * @ETHTOOL_C33_PSE_EXT_STATE_PD_DLL_POWER_TYPE: Group of pd_dll_power_type
+ *	states
+ * @ETHTOOL_C33_PSE_EXT_STATE_POWER_NOT_AVAILABLE: Group of power_not_available
+ *	states
+ * @ETHTOOL_C33_PSE_EXT_STATE_SHORT_DETECTED: Group of short_detected states
+ */
+enum ethtool_c33_pse_ext_state {
+	ETHTOOL_C33_PSE_EXT_STATE_CLASS_NUM_EVENTS = 1,
+	ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+	ETHTOOL_C33_PSE_EXT_STATE_MR_PSE_ENABLE,
+	ETHTOOL_C33_PSE_EXT_STATE_OPTION_DETECT_TED,
+	ETHTOOL_C33_PSE_EXT_STATE_OPTION_VPORT_LIM,
+	ETHTOOL_C33_PSE_EXT_STATE_OVLD_DETECTED,
+	ETHTOOL_C33_PSE_EXT_STATE_PD_DLL_POWER_TYPE,
+	ETHTOOL_C33_PSE_EXT_STATE_POWER_NOT_AVAILABLE,
+	ETHTOOL_C33_PSE_EXT_STATE_SHORT_DETECTED,
+};
+
+/**
+ * enum ethtool_c33_pse_ext_substate_class_num_events - class_num_events states
+ *      functions. IEEE 802.3-2022 33.2.4.4 Variables
+ *
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_CLASS_NUM_EVENTS_CLASS_ERROR: Illegal class
+ *
+ * class_num_events is variable indicating the number of classification events
+ * performed by the PSE. A variable that is set in an implementation-dependent
+ * manner.
+ */
+enum ethtool_c33_pse_ext_substate_class_num_events {
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_CLASS_NUM_EVENTS_CLASS_ERROR = 1,
+};
+
+/**
+ * enum ethtool_c33_pse_ext_substate_error_condition - error_condition states
+ *      functions. IEEE 802.3-2022 33.2.4.4 Variables
+ *
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_NON_EXISTING_PORT: Non-existing
+ *	port number
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNDEFINED_PORT: Undefined port
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_INTERNAL_HW_FAULT: Internal
+ *	hardware fault
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_COMM_ERROR_AFTER_FORCE_ON:
+ *	Communication error after force on
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNKNOWN_PORT_STATUS: Unknown
+ *	port status
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_HOST_CRASH_TURN_OFF: Host
+ *	crash turn off
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_HOST_CRASH_FORCE_SHUTDOWN:
+ *	Host crash force shutdown
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_UNDERLOAD: Underload
+ *	state
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONFIG_CHANGE: Configuration
+ *	change
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_OVER_TEMP: Over
+ *	temperature detected
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONNECTION_OPEN: Port is
+ *	not connected
+ *
+ * error_condition is a variable indicating the status of
+ * implementation-specific fault conditions or optionally other system faults
+ * that prevent the PSE from meeting the specifications in Table 33–11 and that
+ * require the PSE not to source power. These error conditions are different
+ * from those monitored by the state diagrams in Figure 33–10.
+ */
+enum ethtool_c33_pse_ext_substate_error_condition {
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_NON_EXISTING_PORT = 1,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNDEFINED_PORT,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_INTERNAL_HW_FAULT,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_COMM_ERROR_AFTER_FORCE_ON,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNKNOWN_PORT_STATUS,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_HOST_CRASH_TURN_OFF,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_HOST_CRASH_FORCE_SHUTDOWN,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_UNDERLOAD,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONFIG_CHANGE,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_OVER_TEMP,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONNECTION_OPEN,
+};
+
+/**
+ * enum ethtool_c33_pse_ext_substate_mr_pse_enable - mr_pse_enable states
+ *      functions. IEEE 802.3-2022 33.2.4.4 Variables
+ *
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_MR_PSE_ENABLE_DISABLE_PIN_ACTIVE: Disable
+ *	pin active
+ *
+ * mr_pse_enable is control variable that selects PSE operation and test
+ * functions.
+ */
+enum ethtool_c33_pse_ext_substate_mr_pse_enable {
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_MR_PSE_ENABLE_DISABLE_PIN_ACTIVE = 1,
+};
+
+/**
+ * enum ethtool_c33_pse_ext_substate_option_detect_ted - option_detect_ted
+ *	states functions. IEEE 802.3-2022 33.2.4.4 Variables
+ *
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_DET_IN_PROCESS: Detection
+ *	in process
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_IMPROPER_CAP_DET: Improper
+ *	capacitor Detection
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_CONNECTION_CHECK_ERROR:
+ *	Connection check error
+ *
+ * option_detect_ted is a variable indicating if detection can be performed
+ * by the PSE during the ted_timer interval.
+ */
+enum ethtool_c33_pse_ext_substate_option_detect_ted {
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_DET_IN_PROCESS = 1,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_IMPROPER_CAP_DET,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_CONNECTION_CHECK_ERROR,
+};
+
+/**
+ * enum ethtool_c33_pse_ext_substate_option_vport_lim - option_vport_lim states
+ *      functions. IEEE 802.3-2022 33.2.4.4 Variables
+ *
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_HIGH_VOLTAGE: Main supply
+ *	voltage is high
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_LOW_VOLTAGE: Main supply
+ *	voltage is low
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_VOLTAGE_INJECTION: Voltage
+ *	injection into the port
+ *
+ * option_vport_lim is an optional variable indicates if VPSE is out of the
+ * operating range during normal operating state.
+ */
+enum ethtool_c33_pse_ext_substate_option_vport_lim {
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_HIGH_VOLTAGE = 1,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_LOW_VOLTAGE,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_VOLTAGE_INJECTION,
+};
+
+/**
+ * enum ethtool_c33_pse_ext_substate_ovld_detected - ovld_detected states
+ *      functions. IEEE 802.3-2022 33.2.4.4 Variables
+ *
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OVLD_DETECTED_OVERLOAD: Overload state
+ *
+ * ovld_detected is a variable indicating if the PSE output current has been
+ * in an overload condition (see 33.2.7.6) for at least TCUT of a one-second
+ * sliding time.
+ */
+enum ethtool_c33_pse_ext_substate_ovld_detected {
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_OVLD_DETECTED_OVERLOAD = 1,
+};
+
+/**
+ * enum ethtool_c33_pse_ext_substate_pd_dll_power_type - pd_dll_power_type
+ *	states functions. IEEE 802.3-2022 33.2.4.4 Variables
+ *
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_PD_DLL_POWER_TYPE_NON_802_3AF_AT_DEVICE:
+ *	Non-802.3AF/AT powered device
+ *
+ * pd_dll_power_type is a control variable initially output by the PSE power
+ * control state diagram (Figure 33–27), which can be updated by LLDP
+ * (see Table 33–26), that indicates the type of PD as advertised through
+ * Data Link Layer classification.
+ */
+enum ethtool_c33_pse_ext_substate_pd_dll_power_type {
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_PD_DLL_POWER_TYPE_NON_802_3AF_AT_DEVICE = 1,
+};
+
+/**
+ * enum ethtool_c33_pse_ext_substate_power_not_available - power_not_available
+ *	states functions. IEEE 802.3-2022 33.2.4.4 Variables
+ *
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_BUDGET_EXCEEDED: Power
+ *	budget exceeded
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC: Power
+ *	Management-Static
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC_OVL: Power
+ *	Management-Static-ovl
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_HW_PW_LIMIT: Power
+ * denied due to Hardware power limit
+ *
+ * power_not_available is a variable that is asserted in an
+ * implementation-dependent manner when the PSE is no longer capable of
+ * sourcing sufficient power to support the attached PD. Sufficient power
+ * is defined by classification; see 33.2.6.
+ */
+enum ethtool_c33_pse_ext_substate_power_not_available {
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_BUDGET_EXCEEDED =  1,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC_OVL,
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_HW_PW_LIMIT,
+};
+
+/**
+ * enum ethtool_c33_pse_ext_substate_short_detected - short_detected states
+ *      functions. IEEE 802.3-2022 33.2.4.4 Variables
+ *
+ * @ETHTOOL_C33_PSE_EXT_SUBSTATE_SHORT_DETECTED_SHORT_CONDITION: Short
+ *	condition was detected
+ *
+ * short_detected is a variable indicating if the PSE output current has been
+ * in a short circuit condition for TLIM within a sliding window (see 33.2.7.7).
+ */
+enum ethtool_c33_pse_ext_substate_short_detected {
+	ETHTOOL_C33_PSE_EXT_SUBSTATE_SHORT_DETECTED_SHORT_CONDITION = 1,
+};
+
 /**
  * enum ethtool_pse_types - Types of PSE controller.
  * @ETHTOOL_PSE_UNKNOWN: Type of PSE controller is unknown
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index b49b804b9495..398a0aa8daad 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -915,6 +915,10 @@ enum {
 	ETHTOOL_A_C33_PSE_ADMIN_STATE,		/* u32 */
 	ETHTOOL_A_C33_PSE_ADMIN_CONTROL,	/* u32 */
 	ETHTOOL_A_C33_PSE_PW_D_STATUS,		/* u32 */
+	ETHTOOL_A_C33_PSE_PW_CLASS,		/* u32 */
+	ETHTOOL_A_C33_PSE_ACTUAL_PW,		/* u32 */
+	ETHTOOL_A_C33_PSE_EXT_STATE,		/* u32 */
+	ETHTOOL_A_C33_PSE_EXT_SUBSTATE,		/* u32 */
 
 	/* add new constants above here */
 	__ETHTOOL_A_PSE_CNT,
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index 2c981d443f27..fec56db557d3 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -86,7 +86,14 @@ static int pse_reply_size(const struct ethnl_req_info *req_base,
 		len += nla_total_size(sizeof(u32)); /* _C33_PSE_ADMIN_STATE */
 	if (st->c33_pw_status > 0)
 		len += nla_total_size(sizeof(u32)); /* _C33_PSE_PW_D_STATUS */
-
+	if (st->c33_pw_class > 0)
+		len += nla_total_size(sizeof(u32)); /* _C33_PSE_PW_CLASS */
+	if (st->c33_actual_pw > 0)
+		len += nla_total_size(sizeof(u32)); /* _C33_PSE_ACTUAL_PW */
+	if (st->c33_ext_state_info.c33_pse_ext_state > 0)
+		len += nla_total_size(sizeof(u32)); /* _C33_PSE_EXT_STATE */
+	if (st->c33_ext_state_info.__c33_pse_ext_substate > 0)
+		len += nla_total_size(sizeof(u32)); /* _C33_PSE_EXT_SUBSTATE */
 	return len;
 }
 
@@ -117,6 +124,26 @@ static int pse_fill_reply(struct sk_buff *skb,
 			st->c33_pw_status))
 		return -EMSGSIZE;
 
+	if (st->c33_pw_class > 0 &&
+	    nla_put_u32(skb, ETHTOOL_A_C33_PSE_PW_CLASS,
+			st->c33_pw_class))
+		return -EMSGSIZE;
+
+	if (st->c33_actual_pw > 0 &&
+	    nla_put_u32(skb, ETHTOOL_A_C33_PSE_ACTUAL_PW,
+			st->c33_actual_pw))
+		return -EMSGSIZE;
+
+	if (st->c33_ext_state_info.c33_pse_ext_state > 0 &&
+	    nla_put_u32(skb, ETHTOOL_A_C33_PSE_EXT_STATE,
+			st->c33_ext_state_info.c33_pse_ext_state))
+		return -EMSGSIZE;
+
+	if (st->c33_ext_state_info.__c33_pse_ext_substate > 0 &&
+	    nla_put_u32(skb, ETHTOOL_A_C33_PSE_EXT_SUBSTATE,
+			st->c33_ext_state_info.__c33_pse_ext_substate))
+		return -EMSGSIZE;
+
 	return 0;
 }
 

-- 
2.34.1

Re: [PATCH net-next v3 1/7] net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state

[Oleksij Rempel]

Hi Köry,

Overall, it looks good. Some fields need clarification, so don't be
surprised if I critique things I proposed myself. There are still
aspects I don't fully understand :)

On Fri, Jun 14, 2024 at 04:33:17PM +0200, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

> +/**
> + * enum ethtool_c33_pse_ext_substate_class_num_events - class_num_events states
> + *      functions. IEEE 802.3-2022 33.2.4.4 Variables
> + *
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_CLASS_NUM_EVENTS_CLASS_ERROR: Illegal class
> + *
> + * class_num_events is variable indicating the number of classification events
> + * performed by the PSE. A variable that is set in an implementation-dependent
> + * manner.
> + */
> +enum ethtool_c33_pse_ext_substate_class_num_events {
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_CLASS_NUM_EVENTS_CLASS_ERROR = 1,
> +};

I'm still not 100% sure by this name. class_num_events seems to be more
PSE side configuration variable. The pd692x0 0x43 value says "Illegal
class" without providing additional information. If I see it correctly,
typical classification will end with POWER_NOT_AVAILABLE if we will
detect not supported class. Something other should fail to detect an
illegal class.

According to 33.2.4.7
State diagrams we have CLASSIFICATION_EVAL function which evaluates
results of classification.
In case of class_num_events = 1, we have only tpdc_timer. In case of
error, will we get some timer related error?

In case of class_num_events = 2, if i see it correctly, PSE is doing
double classification and if results do not match, PSE will go to faul
state. See CLASS_EV2->(mr_pd_class_detected != temp_var) case.

Is it what we have here?

> +/**
> + * enum ethtool_c33_pse_ext_substate_error_condition - error_condition states
> + *      functions. IEEE 802.3-2022 33.2.4.4 Variables
> + *
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_NON_EXISTING_PORT: Non-existing
> + *	port number
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNDEFINED_PORT: Undefined port
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_INTERNAL_HW_FAULT: Internal
> + *	hardware fault
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_COMM_ERROR_AFTER_FORCE_ON:
> + *	Communication error after force on
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNKNOWN_PORT_STATUS: Unknown
> + *	port status
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_HOST_CRASH_TURN_OFF: Host
> + *	crash turn off
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_HOST_CRASH_FORCE_SHUTDOWN:
> + *	Host crash force shutdown
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_UNDERLOAD: Underload
> + *	state

pd692x0 documentation says, underload condition is related to Iport < Imin.
Sofar, I was not able to find Imin in the final IEEE 802.3 2022 spec.

There are some historical traces:
https://www.ieee802.org/3/af/public/mar01/darshan_3_0301.pdf

Instead, underload condition seems to be part of Maintain Power Signature (MPS)
monitoring. See 33.2.9 PSE power removal and 33.2.9.1.2 PSE DC MPS component
requirements.

Probably, it should go to the ETHTOOL_C33_PSE_EXT_SUBSTATE_MPS

> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONFIG_CHANGE: Configuration
> + *	change
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_OVER_TEMP: Over
> + *	temperature detected
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONNECTION_OPEN: Port is
> + *	not connected

This seems to reflect DETECT_EVAL->(signature = open_circuit) case. So,
it is probably not vendor specific error condition?

The difference between open and underload is probably:
- open: Iport = 0, detection state
- underload: Iport < Imin (or Ihold?), Iport can be 0. related to powered/MPS
  state.

> + *
> + * error_condition is a variable indicating the status of
> + * implementation-specific fault conditions or optionally other system faults
> + * that prevent the PSE from meeting the specifications in Table 33–11 and that
> + * require the PSE not to source power. These error conditions are different
> + * from those monitored by the state diagrams in Figure 33–10.
> + */
> +enum ethtool_c33_pse_ext_substate_error_condition {
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_NON_EXISTING_PORT = 1,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNDEFINED_PORT,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_INTERNAL_HW_FAULT,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_COMM_ERROR_AFTER_FORCE_ON,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNKNOWN_PORT_STATUS,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_HOST_CRASH_TURN_OFF,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_HOST_CRASH_FORCE_SHUTDOWN,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_UNDERLOAD,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONFIG_CHANGE,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_OVER_TEMP,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONNECTION_OPEN,
> +};
> +
> +/**
> + * enum ethtool_c33_pse_ext_substate_mr_pse_enable - mr_pse_enable states
> + *      functions. IEEE 802.3-2022 33.2.4.4 Variables
> + *
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_MR_PSE_ENABLE_DISABLE_PIN_ACTIVE: Disable
> + *	pin active
> + *
> + * mr_pse_enable is control variable that selects PSE operation and test
> + * functions.
> + */
> +enum ethtool_c33_pse_ext_substate_mr_pse_enable {
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_MR_PSE_ENABLE_DISABLE_PIN_ACTIVE = 1,
> +};
> +
> +/**
> + * enum ethtool_c33_pse_ext_substate_option_detect_ted - option_detect_ted
> + *	states functions. IEEE 802.3-2022 33.2.4.4 Variables
> + *
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_DET_IN_PROCESS: Detection
> + *	in process
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_IMPROPER_CAP_DET: Improper
> + *	capacitor Detection
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_CONNECTION_CHECK_ERROR:
> + *	Connection check error
> + *
> + * option_detect_ted is a variable indicating if detection can be performed
> + * by the PSE during the ted_timer interval.
> + */
> +enum ethtool_c33_pse_ext_substate_option_detect_ted {
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_DET_IN_PROCESS = 1,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_IMPROPER_CAP_DET,

The pd692x0 0x25 may be reported in two cases:
Fail due to out-of-range capacitor value or
Fail due to detected short value

On one side, this seems to be related to MONITOR_INRUSH function.
"33.2.7.5 Output current in POWER_UP mode

The PSE shall limit the maximum current sourced at the PI during
POWER_UP. The maximum inrush current sourced by the PSE shall not exceed the
PSE inrush template in Figure 33–13."

On other side, pd692x0 documentation is using 0x1C or 0x25 or 0xA7
values together with "INVALID SIG" description. In this case, this
values are related to signature detection stage, not power up or
tinrush_timer stage. In this case, i assume:
0x25 and 0xa7 refers to Table 33–6 or Table 145–8 Invalid PD detection signature
electrical characteristics.

Not sure about 0x1c - Non-802.3AF/AT powered device. Is it something
between Table 33–5 and Table 33–6? 

CCing UNGLinuxDriver@microchip.com

May be you will need to contact Microchip directly. Usually it helps :)

> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_CONNECTION_CHECK_ERROR,
> +};
> +
> +/**
> + * enum ethtool_c33_pse_ext_substate_option_vport_lim - option_vport_lim states
> + *      functions. IEEE 802.3-2022 33.2.4.4 Variables
> + *
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_HIGH_VOLTAGE: Main supply
> + *	voltage is high
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_LOW_VOLTAGE: Main supply
> + *	voltage is low
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_VOLTAGE_INJECTION: Voltage
> + *	injection into the port
> + *
> + * option_vport_lim is an optional variable indicates if VPSE is out of the
> + * operating range during normal operating state.
> + */
> +enum ethtool_c33_pse_ext_substate_option_vport_lim {
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_HIGH_VOLTAGE = 1,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_LOW_VOLTAGE,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_VOLTAGE_INJECTION,
> +};
> +
> +/**
> + * enum ethtool_c33_pse_ext_substate_ovld_detected - ovld_detected states
> + *      functions. IEEE 802.3-2022 33.2.4.4 Variables
> + *
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_OVLD_DETECTED_OVERLOAD: Overload state
> + *
> + * ovld_detected is a variable indicating if the PSE output current has been
> + * in an overload condition (see 33.2.7.6) for at least TCUT of a one-second
> + * sliding time.
> + */
> +enum ethtool_c33_pse_ext_substate_ovld_detected {
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_OVLD_DETECTED_OVERLOAD = 1,
> +};
> +
> +/**
> + * enum ethtool_c33_pse_ext_substate_pd_dll_power_type - pd_dll_power_type
> + *	states functions. IEEE 802.3-2022 33.2.4.4 Variables
> + *
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_PD_DLL_POWER_TYPE_NON_802_3AF_AT_DEVICE:
> + *	Non-802.3AF/AT powered device
> + *
> + * pd_dll_power_type is a control variable initially output by the PSE power
> + * control state diagram (Figure 33–27), which can be updated by LLDP
> + * (see Table 33–26), that indicates the type of PD as advertised through
> + * Data Link Layer classification.
> + */
> +enum ethtool_c33_pse_ext_substate_pd_dll_power_type {
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_PD_DLL_POWER_TYPE_NON_802_3AF_AT_DEVICE = 1,
> +};

Here i was potentially wrong. LLDP stage is after power up, and this
values was probably set on early stage of signature detection. How can
we detect a device which is not conform to the 802.3AF/AT standard? Is
it something pre-802.3AF/AT, micorosemi specific vendor specific signature?

> +/**
> + * enum ethtool_c33_pse_ext_substate_power_not_available - power_not_available
> + *	states functions. IEEE 802.3-2022 33.2.4.4 Variables
> + *
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_BUDGET_EXCEEDED: Power
> + *	budget exceeded
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC: Power
> + *	Management-Static

> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC_OVL: Power
> + *	Management-Static-ovl

Here we need some comment updates. Here is my understanding, taken out
of thin air:
0x20 - We have per controller limit, but no limit per port is configured,
       in this case, if PD classification request more power then
       allowed by per controller budget, we will get this error.
       AllPortsPower + NewPortPower > ControllerMaxPower
0x3c - We have per port limit configured and it is over the controller
       budget.
       AllPortsMaxPower + NewPortMaxPower > ControllerMaxPower
0x3D - PD Class requesting more power that the Port configured port limit.
       PDClassPower > PortMaxPower

How about:
 * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_CONTROLLER_BUDGET_EXCEEDED: Power
 *   budget exceeded for the controller
 * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PORT_POWER_LIMIT_EXCEEDS_CONTROLLER_BUDGET: Configured
 *   port power limit exceeded controller power budget
 * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PD_REQUEST_EXCEEDS_PORT_LIMIT: Power
 *   request from PD exceeds port limit

> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_HW_PW_LIMIT: Power
> + * denied due to Hardware power limit

Not sure i understand this one correctly. Is it something like - all previous
errors can be solved by proper configuration, but on this one we can't do
anything. The HW is the limit. Correct? :)

> + * power_not_available is a variable that is asserted in an
> + * implementation-dependent manner when the PSE is no longer capable of
> + * sourcing sufficient power to support the attached PD. Sufficient power
> + * is defined by classification; see 33.2.6.
> + */
> +enum ethtool_c33_pse_ext_substate_power_not_available {
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_BUDGET_EXCEEDED =  1,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC_OVL,
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_HW_PW_LIMIT,
> +};
> +
> +/**
> + * enum ethtool_c33_pse_ext_substate_short_detected - short_detected states
> + *      functions. IEEE 802.3-2022 33.2.4.4 Variables
> + *
> + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_SHORT_DETECTED_SHORT_CONDITION: Short
> + *	condition was detected
> + *
> + * short_detected is a variable indicating if the PSE output current has been
> + * in a short circuit condition for TLIM within a sliding window (see 33.2.7.7).
> + */
> +enum ethtool_c33_pse_ext_substate_short_detected {
> +	ETHTOOL_C33_PSE_EXT_SUBSTATE_SHORT_DETECTED_SHORT_CONDITION = 1,
> +};
> +
>  /**
>   * enum ethtool_pse_types - Types of PSE controller.
>   * @ETHTOOL_PSE_UNKNOWN: Type of PSE controller is unknown
> diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
> index b49b804b9495..398a0aa8daad 100644
> --- a/include/uapi/linux/ethtool_netlink.h
> +++ b/include/uapi/linux/ethtool_netlink.h
> @@ -915,6 +915,10 @@ enum {
>  	ETHTOOL_A_C33_PSE_ADMIN_STATE,		/* u32 */
>  	ETHTOOL_A_C33_PSE_ADMIN_CONTROL,	/* u32 */
>  	ETHTOOL_A_C33_PSE_PW_D_STATUS,		/* u32 */
> +	ETHTOOL_A_C33_PSE_PW_CLASS,		/* u32 */
> +	ETHTOOL_A_C33_PSE_ACTUAL_PW,		/* u32 */
> +	ETHTOOL_A_C33_PSE_EXT_STATE,		/* u32 */
> +	ETHTOOL_A_C33_PSE_EXT_SUBSTATE,		/* u32 */
>  
>  	/* add new constants above here */
>  	__ETHTOOL_A_PSE_CNT,
> diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
> index 2c981d443f27..fec56db557d3 100644
> --- a/net/ethtool/pse-pd.c
> +++ b/net/ethtool/pse-pd.c
> @@ -86,7 +86,14 @@ static int pse_reply_size(const struct ethnl_req_info *req_base,
>  		len += nla_total_size(sizeof(u32)); /* _C33_PSE_ADMIN_STATE */
>  	if (st->c33_pw_status > 0)
>  		len += nla_total_size(sizeof(u32)); /* _C33_PSE_PW_D_STATUS */
> -
> +	if (st->c33_pw_class > 0)
> +		len += nla_total_size(sizeof(u32)); /* _C33_PSE_PW_CLASS */
> +	if (st->c33_actual_pw > 0)
> +		len += nla_total_size(sizeof(u32)); /* _C33_PSE_ACTUAL_PW */
> +	if (st->c33_ext_state_info.c33_pse_ext_state > 0)
> +		len += nla_total_size(sizeof(u32)); /* _C33_PSE_EXT_STATE */
> +	if (st->c33_ext_state_info.__c33_pse_ext_substate > 0)
> +		len += nla_total_size(sizeof(u32)); /* _C33_PSE_EXT_SUBSTATE */

Hm, we still may include __c33_pse_ext_substate even if c33_pse_ext_state == 0.

>  	return len;
>  }
>  
> @@ -117,6 +124,26 @@ static int pse_fill_reply(struct sk_buff *skb,
>  			st->c33_pw_status))
>  		return -EMSGSIZE;
>  
> +	if (st->c33_pw_class > 0 &&
> +	    nla_put_u32(skb, ETHTOOL_A_C33_PSE_PW_CLASS,
> +			st->c33_pw_class))
> +		return -EMSGSIZE;
> +
> +	if (st->c33_actual_pw > 0 &&
> +	    nla_put_u32(skb, ETHTOOL_A_C33_PSE_ACTUAL_PW,
> +			st->c33_actual_pw))
> +		return -EMSGSIZE;
> +
> +	if (st->c33_ext_state_info.c33_pse_ext_state > 0 &&
> +	    nla_put_u32(skb, ETHTOOL_A_C33_PSE_EXT_STATE,
> +			st->c33_ext_state_info.c33_pse_ext_state))
> +		return -EMSGSIZE;
> +
> +	if (st->c33_ext_state_info.__c33_pse_ext_substate > 0 &&
> +	    nla_put_u32(skb, ETHTOOL_A_C33_PSE_EXT_SUBSTATE,
> +			st->c33_ext_state_info.__c33_pse_ext_substate))
> +		return -EMSGSIZE;
> +

Same here. 

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

Re: [PATCH net-next v3 1/7] net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state

[Kory Maincent]

Hello Oleksij,

Thanks for your complete reviews.

On Sat, 15 Jun 2024 13:22:36 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Hi Köry,
> 
> Overall, it looks good. Some fields need clarification, so don't be
> surprised if I critique things I proposed myself. There are still
> aspects I don't fully understand :)

Me neither, lets continue dig this up together.

Figured out we could also base our substate according to 33.8 tables values.

> On Fri, Jun 14, 2024 at 04:33:17PM +0200, Kory Maincent wrote:
>  [...]  
> 
> > +/**
> > + * enum ethtool_c33_pse_ext_substate_class_num_events - class_num_events
> > states
> > + *      functions. IEEE 802.3-2022 33.2.4.4 Variables
> > + *
> > + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_CLASS_NUM_EVENTS_CLASS_ERROR: Illegal
> > class
> > + *
> > + * class_num_events is variable indicating the number of classification
> > events
> > + * performed by the PSE. A variable that is set in an
> > implementation-dependent
> > + * manner.
> > + */
> > +enum ethtool_c33_pse_ext_substate_class_num_events {
> > +	ETHTOOL_C33_PSE_EXT_SUBSTATE_CLASS_NUM_EVENTS_CLASS_ERROR = 1,
> > +};  
> 
> I'm still not 100% sure by this name. class_num_events seems to be more
> PSE side configuration variable. The pd692x0 0x43 value says "Illegal
> class" without providing additional information. If I see it correctly,
> typical classification will end with POWER_NOT_AVAILABLE if we will
> detect not supported class. Something other should fail to detect an
> illegal class.
> 
> According to 33.2.4.7
> State diagrams we have CLASSIFICATION_EVAL function which evaluates
> results of classification.
> In case of class_num_events = 1, we have only tpdc_timer. In case of
> error, will we get some timer related error?
> 
> In case of class_num_events = 2, if i see it correctly, PSE is doing
> double classification and if results do not match, PSE will go to faul
> state. See CLASS_EV2->(mr_pd_class_detected != temp_var) case.
> 
> Is it what we have here?

Mmh not really indeed, maybe we can put it in error_condition substate?

> > + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_UNDERLOAD:
> > Underload
> > + *	state  
> 
> pd692x0 documentation says, underload condition is related to Iport < Imin.
> Sofar, I was not able to find Imin in the final IEEE 802.3 2022 spec.
> 
> There are some historical traces:
> https://www.ieee802.org/3/af/public/mar01/darshan_3_0301.pdf
> 
> Instead, underload condition seems to be part of Maintain Power Signature
> (MPS) monitoring. See 33.2.9 PSE power removal and 33.2.9.1.2 PSE DC MPS
> component requirements.
> 
> Probably, it should go to the ETHTOOL_C33_PSE_EXT_SUBSTATE_MPS

Ok.
 
> > + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONFIG_CHANGE:
> > Configuration
> > + *	change
> > + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_OVER_TEMP: Over
> > + *	temperature detected
> > + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONNECTION_OPEN: Port is
> > + *	not connected  
> 
> This seems to reflect DETECT_EVAL->(signature = open_circuit) case. So,
> it is probably not vendor specific error condition?
> 
> The difference between open and underload is probably:
> - open: Iport = 0, detection state
> - underload: Iport < Imin (or Ihold?), Iport can be 0. related to powered/MPS
>   state.

Should I put it under MPS substate then?

> > +enum ethtool_c33_pse_ext_substate_option_detect_ted {
> > +	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_DET_IN_PROCESS = 1,
> > +	ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_IMPROPER_CAP_DET,  
> 
> The pd692x0 0x25 may be reported in two cases:
> Fail due to out-of-range capacitor value or
> Fail due to detected short value
> 
> On one side, this seems to be related to MONITOR_INRUSH function.
> "33.2.7.5 Output current in POWER_UP mode
> 
> The PSE shall limit the maximum current sourced at the PI during
> POWER_UP. The maximum inrush current sourced by the PSE shall not exceed the
> PSE inrush template in Figure 33–13."
> 
> On other side, pd692x0 documentation is using 0x1C or 0x25 or 0xA7
> values together with "INVALID SIG" description. In this case, this
> values are related to signature detection stage, not power up or
> tinrush_timer stage. In this case, i assume:
> 0x25 and 0xa7 refers to Table 33–6 or Table 145–8 Invalid PD detection
> signature electrical characteristics.
> 
> Not sure about 0x1c - Non-802.3AF/AT powered device. Is it something
> between Table 33–5 and Table 33–6? 
> 
> CCing UNGLinuxDriver@microchip.com
> 
> May be you will need to contact Microchip directly. Usually it helps :)

Lets keep it like that for now?

> > +enum ethtool_c33_pse_ext_substate_pd_dll_power_type {
> > +
> > ETHTOOL_C33_PSE_EXT_SUBSTATE_PD_DLL_POWER_TYPE_NON_802_3AF_AT_DEVICE = 1,
> > +};  
> 
> Here i was potentially wrong. LLDP stage is after power up, and this
> values was probably set on early stage of signature detection. How can
> we detect a device which is not conform to the 802.3AF/AT standard? Is
> it something pre-802.3AF/AT, micorosemi specific vendor specific signature?

Don't really know.
 
> > +/**
> > + * enum ethtool_c33_pse_ext_substate_power_not_available -
> > power_not_available
> > + *	states functions. IEEE 802.3-2022 33.2.4.4 Variables
> > + *
> > + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_BUDGET_EXCEEDED: Power
> > + *	budget exceeded
> > + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC: Power
> > + *	Management-Static  
> 
> > + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC_OVL: Power
> > + *	Management-Static-ovl  
> 
> Here we need some comment updates. Here is my understanding, taken out
> of thin air:
> 0x20 - We have per controller limit, but no limit per port is configured,
>        in this case, if PD classification request more power then
>        allowed by per controller budget, we will get this error.
>        AllPortsPower + NewPortPower > ControllerMaxPower
> 0x3c - We have per port limit configured and it is over the controller
>        budget.
>        AllPortsMaxPower + NewPortMaxPower > ControllerMaxPower
> 0x3D - PD Class requesting more power that the Port configured port limit.
>        PDClassPower > PortMaxPower
> 
> How about:
>  *
> @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_CONTROLLER_BUDGET_EXCEEDED:
> Power
>  *   budget exceeded for the controller
>  *
> @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PORT_POWER_LIMIT_EXCEEDS_CONTROLLER_BUDGET:
> Configured
>  *   port power limit exceeded controller power budget
>  *
> @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PD_REQUEST_EXCEEDS_PORT_LIMIT:
> Power
>  *   request from PD exceeds port limit

Yes seems right.
 
> > + * @ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_HW_PW_LIMIT: Power
> > + * denied due to Hardware power limit  
> 
> Not sure i understand this one correctly. Is it something like - all previous
> errors can be solved by proper configuration, but on this one we can't do
> anything. The HW is the limit. Correct? :)

Suppose so. ;)
 
> > +	if (st->c33_pw_class > 0)
> > +		len += nla_total_size(sizeof(u32)); /* _C33_PSE_PW_CLASS */
> > +	if (st->c33_actual_pw > 0)
> > +		len += nla_total_size(sizeof(u32)); /* _C33_PSE_ACTUAL_PW
> > */
> > +	if (st->c33_ext_state_info.c33_pse_ext_state > 0)
> > +		len += nla_total_size(sizeof(u32)); /* _C33_PSE_EXT_STATE
> > */
> > +	if (st->c33_ext_state_info.__c33_pse_ext_substate > 0)
> > +		len += nla_total_size(sizeof(u32)); /*
> > _C33_PSE_EXT_SUBSTATE */  
> 
> Hm, we still may include __c33_pse_ext_substate even if c33_pse_ext_state ==
> 0.

Right indeed. Will fix it.

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

Re: [PATCH net-next v3 1/7] net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state

[Oleksij Rempel]

On Mon, Jun 17, 2024 at 03:47:12PM +0200, Kory Maincent wrote:
> > According to 33.2.4.7
> > State diagrams we have CLASSIFICATION_EVAL function which evaluates
> > results of classification.
> > In case of class_num_events = 1, we have only tpdc_timer. In case of
> > error, will we get some timer related error?
> > 
> > In case of class_num_events = 2, if i see it correctly, PSE is doing
> > double classification and if results do not match, PSE will go to faul
> > state. See CLASS_EV2->(mr_pd_class_detected != temp_var) case.
> > 
> > Is it what we have here?
> 
> Mmh not really indeed, maybe we can put it in error_condition substate?

I'm not sure how this error can help user, if even we do not understand
what is says. May be map everything what is not clear right not to
unsupported error value. This give us some time to communicate with
vendor and prevent us from making pointless UAPi?

> > The difference between open and underload is probably:
> > - open: Iport = 0, detection state
> > - underload: Iport < Imin (or Ihold?), Iport can be 0. related to powered/MPS
> >   state.
> 
> Should I put it under MPS substate then?

If my understand is correct, then yes. Can you test it? Do you have PD
with adjustable load?

> > May be you will need to contact Microchip directly. Usually it helps :)
> 
> Lets keep it like that for now?

let's map it to unsupported error for now

> > > +enum ethtool_c33_pse_ext_substate_pd_dll_power_type {
> > > +
> > > ETHTOOL_C33_PSE_EXT_SUBSTATE_PD_DLL_POWER_TYPE_NON_802_3AF_AT_DEVICE = 1,
> > > +};  
> > 
> > Here i was potentially wrong. LLDP stage is after power up, and this
> > values was probably set on early stage of signature detection. How can
> > we detect a device which is not conform to the 802.3AF/AT standard? Is
> > it something pre-802.3AF/AT, micorosemi specific vendor specific signature?
> 
> Don't really know.

Same here, if we do not really know what it is, make it unsupported error value

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 |

Re: [PATCH net-next v3 1/7] net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state

[Kory Maincent]

On Mon, 17 Jun 2024 21:55:25 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> On Mon, Jun 17, 2024 at 03:47:12PM +0200, Kory Maincent wrote:
>  [...]  
> > 
> > Mmh not really indeed, maybe we can put it in error_condition substate?  
> 
> I'm not sure how this error can help user, if even we do not understand
> what is says. May be map everything what is not clear right not to
> unsupported error value. This give us some time to communicate with
> vendor and prevent us from making pointless UAPi?

Is it ok for you if I use this substate for unsupported value:
ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNKNOWN_PORT_STATUS
or do you prefer another one.

> > Should I put it under MPS substate then?  
> 
> If my understand is correct, then yes. Can you test it? Do you have PD
> with adjustable load?

Yes I will test it.

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

Re: [PATCH net-next v3 1/7] net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state

[Oleksij Rempel]

On Fri, Jun 21, 2024 at 06:29:15PM +0200, Kory Maincent wrote:
> On Mon, 17 Jun 2024 21:55:25 +0200
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > On Mon, Jun 17, 2024 at 03:47:12PM +0200, Kory Maincent wrote:
> >  [...]  
> > > 
> > > Mmh not really indeed, maybe we can put it in error_condition substate?  
> > 
> > I'm not sure how this error can help user, if even we do not understand
> > what is says. May be map everything what is not clear right not to
> > unsupported error value. This give us some time to communicate with
> > vendor and prevent us from making pointless UAPi?
> 
> Is it ok for you if I use this substate for unsupported value:
> ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNKNOWN_PORT_STATUS
> or do you prefer another one.

Ack, sounds good.

> > > Should I put it under MPS substate then?  
> > 
> > If my understand is correct, then yes. Can you test it? Do you have PD
> > with adjustable load?
> 
> Yes I will test it.

Thx!

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 |

Re: [PATCH net-next v3 1/7] net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state

[Kory Maincent]

Hello Oleksij,

On Mon, 17 Jun 2024 21:55:25 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> 
> > > The difference between open and underload is probably:
> > > - open: Iport = 0, detection state
> > > - underload: Iport < Imin (or Ihold?), Iport can be 0. related to
> > > powered/MPS state.  
> > 
> > Should I put it under MPS substate then?  
> 
> If my understand is correct, then yes. Can you test it? Do you have PD
> with adjustable load?

In fact I can't test it, I have a splitter and an adjustable load, not a
splitter that can adjust it's own load. So I can't decrease the load of the
splitter itself and reach this error condition.

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

Re: [PATCH net-next v3 1/7] net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state

[Oleksij Rempel]

Hi Köry,

On Tue, Jun 25, 2024 at 11:18:35AM +0200, Kory Maincent wrote:
> Hello Oleksij,
> 
> On Mon, 17 Jun 2024 21:55:25 +0200
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > 
> > > > The difference between open and underload is probably:
> > > > - open: Iport = 0, detection state
> > > > - underload: Iport < Imin (or Ihold?), Iport can be 0. related to
> > > > powered/MPS state.  
> > > 
> > > Should I put it under MPS substate then?  
> > 
> > If my understand is correct, then yes. Can you test it? Do you have PD
> > with adjustable load?
> 
> In fact I can't test it, I have a splitter and an adjustable load, not a
> splitter that can adjust it's own load. So I can't decrease the load of the
> splitter itself and reach this error condition.

Hm.. how about this setup:
------>>-----x--------->>----
PSE          |-load      splitter
------>>-----x--------->>----

Attach the load directly to the ethernet line after PSE did
classification with splitter. Then remove splitter. As long as load is
high enough, PSE will not turn the port off. Then reduce load until it drops
below the threshold.

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 |

Re: [PATCH net-next v3 1/7] net: ethtool: pse-pd: Expand C33 PSE status with class, power and extended state

[Kory Maincent]

On Tue, 25 Jun 2024 12:33:50 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Hi Köry,
> 
> On Tue, Jun 25, 2024 at 11:18:35AM +0200, Kory Maincent wrote:
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
> > 
> > In fact I can't test it, I have a splitter and an adjustable load, not a
> > splitter that can adjust it's own load. So I can't decrease the load of the
> > splitter itself and reach this error condition.  
> 
> Hm.. how about this setup:
> ------>>-----x--------->>----  
> PSE          |-load      splitter
> ------>>-----x--------->>----  
> 
> Attach the load directly to the ethernet line after PSE did
> classification with splitter. Then remove splitter. As long as load is
> high enough, PSE will not turn the port off. Then reduce load until it drops
> below the threshold.

That was a good idea but I can't managed do test it.
This is what I try: 
                       /---ethernet cable---PD
PSE===passive splitter=+
                       \---barrel cable---load

The active PD is well powered with PoE negotiation but there is no voltage at
the barrel cable of the passive splitter so the load is useless. :/ Maybe
passive splitter works only with passive injector.

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

[PATCH net-next v3 2/7] netlink: specs: Expand the PSE netlink command with C33 new features

[Kory Maincent]

From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

Expand the c33 PSE attributes with PSE class, extended state information
and power consumption.

./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do pse-get
	     --json '{"header":{"dev-name":"eth0"}}'
{'c33-pse-actual-pw': 1700,
 'c33-pse-admin-state': 3,
 'c33-pse-pw-class': 4,
 'c33-pse-pw-d-status': 4,
 'header': {'dev-index': 4, 'dev-name': 'eth0'}}

./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do pse-get
	     --json '{"header":{"dev-name":"eth0"}}'
{'c33-pse-admin-state': 3,
 'c33-pse-ext-state': 5,
 'c33-pse-ext-substate': 5,
 'c33-pse-pw-d-status': 2,
 'header': {'dev-index': 4, 'dev-name': 'eth0'}}

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Change in v3:
- Update c33-pse-ext-state and c33-pse-ext-substate to u32
- Add enum for listing c33-pse-ext-state
---
 Documentation/netlink/specs/ethtool.yaml | 35 ++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 00dc61358be8..0ff27319856c 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -20,6 +20,20 @@ definitions:
     name: header-flags
     type: flags
     entries: [ compact-bitsets, omit-reply, stats ]
+  -
+    name: c33-pse-ext-state
+    enum-name:
+    type: enum
+    entries: [ none,
+               ethtool_c33_pse_ext_state_class_num_events,
+               ethtool_c33_pse_ext_state_error_condition,
+               ethtool_c33_pse_ext_state_mr_pse_enable,
+               ethtool_c33_pse_ext_state_option_detect_ted,
+               ethtool_c33_pse_ext_state_option_vport_lim,
+               ethtool_c33_pse_ext_state_ovld_detected,
+               ethtool_c33_pse_ext_state_pd_dll_power_type,
+               ethtool_c33_pse_ext_state_power_not_available,
+               ethtool_c33_pse_ext_state_short_detected ]
 
 attribute-sets:
   -
@@ -922,6 +936,23 @@ attribute-sets:
         name: c33-pse-pw-d-status
         type: u32
         name-prefix: ethtool-a-
+      -
+        name: c33-pse-pw-class
+        type: u32
+        name-prefix: ethtool-a-
+      -
+        name: c33-pse-actual-pw
+        type: u32
+        name-prefix: ethtool-a-
+      -
+        name: c33-pse-ext-state
+        type: u32
+        name-prefix: ethtool-a-
+        enum: c33-pse-ext-state
+      -
+        name: c33-pse-ext-substate
+        type: u32
+        name-prefix: ethtool-a-
   -
     name: rss
     attributes:
@@ -1611,6 +1642,10 @@ operations:
             - c33-pse-admin-state
             - c33-pse-admin-control
             - c33-pse-pw-d-status
+            - c33-pse-pw-class
+            - c33-pse-actual-pw
+            - c33-pse-ext-state
+            - c33-pse-ext-substate
       dump: *pse-get-op
     -
       name: pse-set

-- 
2.34.1

Re: [PATCH net-next v3 2/7] netlink: specs: Expand the PSE netlink command with C33 new features

[Donald Hunter]

Kory Maincent <kory.maincent@bootlin.com> writes:

> diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
> index 00dc61358be8..0ff27319856c 100644
> --- a/Documentation/netlink/specs/ethtool.yaml
> +++ b/Documentation/netlink/specs/ethtool.yaml
> @@ -20,6 +20,20 @@ definitions:
>      name: header-flags
>      type: flags
>      entries: [ compact-bitsets, omit-reply, stats ]
> +  -
> +    name: c33-pse-ext-state
> +    enum-name:
> +    type: enum
> +    entries: [ none,
> +               ethtool_c33_pse_ext_state_class_num_events,
> +               ethtool_c33_pse_ext_state_error_condition,
> +               ethtool_c33_pse_ext_state_mr_pse_enable,
> +               ethtool_c33_pse_ext_state_option_detect_ted,
> +               ethtool_c33_pse_ext_state_option_vport_lim,
> +               ethtool_c33_pse_ext_state_ovld_detected,
> +               ethtool_c33_pse_ext_state_pd_dll_power_type,
> +               ethtool_c33_pse_ext_state_power_not_available,
> +               ethtool_c33_pse_ext_state_short_detected ]

It looks like this should use name-prefix: ethtool-c33-pse-ext-state- so
that all the entries can be shortened.

The entries should be in hyphen-case, not snake_case.

The preferred format for a list that spills over many lines is to use
the yaml list format:

    entries:
      - none
      - class-num-events
      - error-condition
      - mr-pse-enable
      ...

Thanks!
--
Donald Hunter.

[PATCH net-next v3 3/7] net: pse-pd: pd692x0: Expand ethtool status message

[Kory Maincent]

From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

This update expands pd692x0_ethtool_get_status() callback with newly
introduced details such as the detected class, current power delivered,
and extended state information.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Change in v2:
- Move in from string status message to c33_pse_ext_state_info.

Change in v3:
- Update extended state and substate list.
---
 drivers/net/pse-pd/pd692x0.c | 101 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
index 6488b941703c..c5e0456e7103 100644
--- a/drivers/net/pse-pd/pd692x0.c
+++ b/drivers/net/pse-pd/pd692x0.c
@@ -73,6 +73,7 @@ enum {
 	PD692X0_MSG_SET_PORT_PARAM,
 	PD692X0_MSG_GET_PORT_STATUS,
 	PD692X0_MSG_DOWNLOAD_CMD,
+	PD692X0_MSG_GET_PORT_CLASS,
 
 	/* add new message above here */
 	PD692X0_MSG_CNT
@@ -149,6 +150,12 @@ static const struct pd692x0_msg pd692x0_msg_template_list[PD692X0_MSG_CNT] = {
 		.data = {0x16, 0x16, 0x99, 0x4e,
 			 0x4e, 0x4e, 0x4e, 0x4e},
 	},
+	[PD692X0_MSG_GET_PORT_CLASS] = {
+		.key = PD692X0_KEY_REQ,
+		.sub = {0x05, 0xc4},
+		.data = {0x4e, 0x4e, 0x4e, 0x4e,
+			 0x4e, 0x4e, 0x4e, 0x4e},
+	},
 };
 
 static u8 pd692x0_build_msg(struct pd692x0_msg *msg, u8 echo)
@@ -435,6 +442,84 @@ static int pd692x0_pi_is_enabled(struct pse_controller_dev *pcdev, int id)
 	}
 }
 
+struct pd692x0_pse_ext_state_mapping {
+	u32 status_code;
+	enum ethtool_c33_pse_ext_state pse_ext_state;
+	u32 pse_ext_substate;
+};
+
+static const struct pd692x0_pse_ext_state_mapping
+pd692x0_pse_ext_state_map[] = {
+	{0x06, ETHTOOL_C33_PSE_EXT_STATE_OPTION_VPORT_LIM,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_HIGH_VOLTAGE},
+	{0x07, ETHTOOL_C33_PSE_EXT_STATE_OPTION_VPORT_LIM,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_LOW_VOLTAGE},
+	{0x08, ETHTOOL_C33_PSE_EXT_STATE_MR_PSE_ENABLE,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_MR_PSE_ENABLE_DISABLE_PIN_ACTIVE},
+	{0x0C, ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_NON_EXISTING_PORT},
+	{0x11, ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNDEFINED_PORT},
+	{0x12, ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_INTERNAL_HW_FAULT},
+	{0x1B, ETHTOOL_C33_PSE_EXT_STATE_OPTION_DETECT_TED,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_DET_IN_PROCESS},
+	{0x1C, ETHTOOL_C33_PSE_EXT_STATE_PD_DLL_POWER_TYPE,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_PD_DLL_POWER_TYPE_NON_802_3AF_AT_DEVICE},
+	{0x1E, ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_UNDERLOAD},
+	{0x1F, ETHTOOL_C33_PSE_EXT_STATE_OVLD_DETECTED,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_OVLD_DETECTED_OVERLOAD},
+	{0x20, ETHTOOL_C33_PSE_EXT_STATE_POWER_NOT_AVAILABLE,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_BUDGET_EXCEEDED},
+	{0x21, ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_INTERNAL_HW_FAULT},
+	{0x22, ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONFIG_CHANGE},
+	{0x24, ETHTOOL_C33_PSE_EXT_STATE_OPTION_VPORT_LIM,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_VPORT_LIM_VOLTAGE_INJECTION},
+	{0x25, ETHTOOL_C33_PSE_EXT_STATE_OPTION_DETECT_TED,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_IMPROPER_CAP_DET},
+	{0x34, ETHTOOL_C33_PSE_EXT_STATE_SHORT_DETECTED,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_SHORT_DETECTED_SHORT_CONDITION},
+	{0x35, ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_OVER_TEMP},
+	{0x36, ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_DETECTED_OVER_TEMP},
+	{0x37, ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_UNKNOWN_PORT_STATUS},
+	{0x3C, ETHTOOL_C33_PSE_EXT_STATE_POWER_NOT_AVAILABLE,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC},
+	{0x3D, ETHTOOL_C33_PSE_EXT_STATE_POWER_NOT_AVAILABLE,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_PM_STATIC_OVL},
+	{0x41, ETHTOOL_C33_PSE_EXT_STATE_POWER_NOT_AVAILABLE,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_POWER_NOT_AVAILABLE_HW_PW_LIMIT},
+	{0x43, ETHTOOL_C33_PSE_EXT_STATE_CLASS_NUM_EVENTS,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_CLASS_NUM_EVENTS_CLASS_ERROR},
+	{0xA7, ETHTOOL_C33_PSE_EXT_STATE_OPTION_DETECT_TED,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_OPTION_DETECT_TED_CONNECTION_CHECK_ERROR},
+	{0xA8, ETHTOOL_C33_PSE_EXT_STATE_ERROR_CONDITION,
+		ETHTOOL_C33_PSE_EXT_SUBSTATE_ERROR_CONDITION_CONNECTION_OPEN},
+	{ /* sentinel */ }
+};
+
+static void
+pd692x0_get_ext_state(struct ethtool_c33_pse_ext_state_info *c33_ext_state_info,
+		      u32 status_code)
+{
+	const struct pd692x0_pse_ext_state_mapping *ext_state_map;
+
+	ext_state_map = pd692x0_pse_ext_state_map;
+	while (ext_state_map->status_code) {
+		if (ext_state_map->status_code == status_code) {
+			c33_ext_state_info->c33_pse_ext_state = ext_state_map->pse_ext_state;
+			c33_ext_state_info->__c33_pse_ext_substate = ext_state_map->pse_ext_substate;
+			return;
+		}
+		ext_state_map++;
+	}
+}
+
 static int pd692x0_ethtool_get_status(struct pse_controller_dev *pcdev,
 				      unsigned long id,
 				      struct netlink_ext_ack *extack,
@@ -442,6 +527,7 @@ static int pd692x0_ethtool_get_status(struct pse_controller_dev *pcdev,
 {
 	struct pd692x0_priv *priv = to_pd692x0_priv(pcdev);
 	struct pd692x0_msg msg, buf = {0};
+	u32 class;
 	int ret;
 
 	ret = pd692x0_fw_unavailable(priv);
@@ -471,6 +557,21 @@ static int pd692x0_ethtool_get_status(struct pse_controller_dev *pcdev,
 
 	priv->admin_state[id] = status->c33_admin_state;
 
+	pd692x0_get_ext_state(&status->c33_ext_state_info, buf.sub[0]);
+
+	status->c33_actual_pw = (buf.data[0] << 4 | buf.data[1]) * 100;
+
+	memset(&buf, 0, sizeof(buf));
+	msg = pd692x0_msg_template_list[PD692X0_MSG_GET_PORT_CLASS];
+	msg.sub[2] = id;
+	ret = pd692x0_sendrecv_msg(priv, &msg, &buf);
+	if (ret < 0)
+		return ret;
+
+	class = buf.data[3] >> 4;
+	if (class <= 8)
+		status->c33_pw_class = class;
+
 	return 0;
 }
 

-- 
2.34.1

[PATCH net-next v3 4/7] net: pse-pd: Add new power limit get and set c33 features

[Kory Maincent]

From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

This patch add a way to get and set the power limit of a PSE PI.
For that it uses regulator API callbacks wrapper like get_voltage() and
get/set_current_limit() as power is simply V * I.
We used mW unit as defined by the IEEE 802.3-2022 standards.

set_current_limit() uses the voltage return by get_voltage() and the
desired power limit to calculate the current limit. get_voltage() callback
is then mandatory to set the power limit.

get_current_limit() callback is by default looking at a driver callback
and fallback to extracting the current limit from _pse_ethtool_get_status()
if the driver does not set its callback. We prefer let the user the choice
because ethtool_get_status return much more information than the current
limit.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Change in v2:
- Use uA and uV instead of mA and mV to have more precision in the power
  calculation. Need to use 64bit variables for the calculation.
---
 drivers/net/pse-pd/pse_core.c | 172 +++++++++++++++++++++++++++++++++++++++---
 include/linux/pse-pd/pse.h    |  34 +++++++++
 2 files changed, 195 insertions(+), 11 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 795ab264eaf2..98da4488a8fe 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -265,10 +265,113 @@ static int pse_pi_disable(struct regulator_dev *rdev)
 	return ret;
 }
 
+static int _pse_pi_get_voltage(struct regulator_dev *rdev)
+{
+	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
+	const struct pse_controller_ops *ops;
+	int id;
+
+	ops = pcdev->ops;
+	if (!ops->pi_get_voltage)
+		return -EOPNOTSUPP;
+
+	id = rdev_get_id(rdev);
+	return ops->pi_get_voltage(pcdev, id);
+}
+
+static int pse_pi_get_voltage(struct regulator_dev *rdev)
+{
+	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
+	int ret;
+
+	mutex_lock(&pcdev->lock);
+	ret = _pse_pi_get_voltage(rdev);
+	mutex_unlock(&pcdev->lock);
+
+	return ret;
+}
+
+static int _pse_ethtool_get_status(struct pse_controller_dev *pcdev,
+				   int id,
+				   struct netlink_ext_ack *extack,
+				   struct pse_control_status *status);
+
+static int pse_pi_get_current_limit(struct regulator_dev *rdev)
+{
+	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
+	const struct pse_controller_ops *ops;
+	struct netlink_ext_ack extack = {};
+	struct pse_control_status st = {};
+	int id, uV, ret;
+	s64 tmp_64;
+
+	ops = pcdev->ops;
+	id = rdev_get_id(rdev);
+	mutex_lock(&pcdev->lock);
+	if (ops->pi_get_current_limit) {
+		ret = ops->pi_get_current_limit(pcdev, id);
+		goto out;
+	}
+
+	/* If pi_get_current_limit() callback not populated get voltage
+	 * from pi_get_voltage() and power limit from ethtool_get_status()
+	 *  to calculate current limit.
+	 */
+	ret = _pse_pi_get_voltage(rdev);
+	if (!ret) {
+		dev_err(pcdev->dev, "Voltage null\n");
+		ret = -ERANGE;
+		goto out;
+	}
+	if (ret < 0)
+		goto out;
+	uV = ret;
+
+	ret = _pse_ethtool_get_status(pcdev, id, &extack, &st);
+	if (ret)
+		goto out;
+
+	if (!st.c33_pw_limit) {
+		ret = -ENODATA;
+		goto out;
+	}
+
+	tmp_64 = st.c33_pw_limit;
+	tmp_64 *= 1000000000ull;
+	/* uA = mW * 1000000000 / uV */
+	ret = DIV_ROUND_CLOSEST_ULL(tmp_64, uV);
+
+out:
+	mutex_unlock(&pcdev->lock);
+	return ret;
+}
+
+static int pse_pi_set_current_limit(struct regulator_dev *rdev, int min_uA,
+				    int max_uA)
+{
+	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
+	const struct pse_controller_ops *ops;
+	int id, ret;
+
+	ops = pcdev->ops;
+	if (!ops->pi_set_current_limit)
+		return -EOPNOTSUPP;
+
+	id = rdev_get_id(rdev);
+	mutex_lock(&pcdev->lock);
+	ret = ops->pi_set_current_limit(pcdev, id, max_uA);
+	mutex_unlock(&pcdev->lock);
+
+	return ret;
+}
+
 static const struct regulator_ops pse_pi_ops = {
 	.is_enabled = pse_pi_is_enabled,
 	.enable = pse_pi_enable,
 	.disable = pse_pi_disable,
+	.get_voltage = pse_pi_get_voltage,
+	.get_current_limit = pse_pi_get_current_limit,
+	.set_current_limit = pse_pi_set_current_limit,
 };
 
 static int
@@ -298,7 +401,9 @@ devm_pse_pi_regulator_register(struct pse_controller_dev *pcdev,
 	rdesc->ops = &pse_pi_ops;
 	rdesc->owner = pcdev->owner;
 
-	rinit_data->constraints.valid_ops_mask = REGULATOR_CHANGE_STATUS;
+	rinit_data->constraints.valid_ops_mask = REGULATOR_CHANGE_STATUS |
+						 REGULATOR_CHANGE_CURRENT;
+	rinit_data->constraints.max_uA = MAX_PI_CURRENT;
 	rinit_data->supply_regulator = "vpwr";
 
 	rconfig.dev = pcdev->dev;
@@ -626,6 +731,23 @@ struct pse_control *of_pse_control_get(struct device_node *node)
 }
 EXPORT_SYMBOL_GPL(of_pse_control_get);
 
+static int _pse_ethtool_get_status(struct pse_controller_dev *pcdev,
+				   int id,
+				   struct netlink_ext_ack *extack,
+				   struct pse_control_status *status)
+{
+	const struct pse_controller_ops *ops;
+
+	ops = pcdev->ops;
+	if (!ops->ethtool_get_status) {
+		NL_SET_ERR_MSG(extack,
+			       "PSE driver does not support status report");
+		return -EOPNOTSUPP;
+	}
+
+	return ops->ethtool_get_status(pcdev, id, extack, status);
+}
+
 /**
  * pse_ethtool_get_status - get status of PSE control
  * @psec: PSE control pointer
@@ -638,19 +760,10 @@ int pse_ethtool_get_status(struct pse_control *psec,
 			   struct netlink_ext_ack *extack,
 			   struct pse_control_status *status)
 {
-	const struct pse_controller_ops *ops;
 	int err;
 
-	ops = psec->pcdev->ops;
-
-	if (!ops->ethtool_get_status) {
-		NL_SET_ERR_MSG(extack,
-			       "PSE driver does not support status report");
-		return -EOPNOTSUPP;
-	}
-
 	mutex_lock(&psec->pcdev->lock);
-	err = ops->ethtool_get_status(psec->pcdev, psec->id, extack, status);
+	err = _pse_ethtool_get_status(psec->pcdev, psec->id, extack, status);
 	mutex_unlock(&psec->pcdev->lock);
 
 	return err;
@@ -732,6 +845,43 @@ int pse_ethtool_set_config(struct pse_control *psec,
 }
 EXPORT_SYMBOL_GPL(pse_ethtool_set_config);
 
+/**
+ * pse_ethtool_set_pw_limit - set PSE control power limit
+ * @psec: PSE control pointer
+ * @extack: extack for reporting useful error messages
+ * @pw_limit: power limit value in mW
+ *
+ * Return: 0 on success and failure value on error
+ */
+int pse_ethtool_set_pw_limit(struct pse_control *psec,
+			     struct netlink_ext_ack *extack,
+			     const unsigned int pw_limit)
+{
+	int uV, uA, ret;
+	s64 tmp_64;
+
+	ret = regulator_get_voltage(psec->ps);
+	if (!ret) {
+		NL_SET_ERR_MSG(extack,
+			       "Can't read current voltage");
+		return ret;
+	}
+	if (ret < 0) {
+		NL_SET_ERR_MSG(extack,
+			       "Error reading current voltage");
+		return ret;
+	}
+	uV = ret;
+
+	tmp_64 = pw_limit;
+	tmp_64 *= 1000000000ull;
+	/* uA = mW * 1000000000 / uV */
+	uA = DIV_ROUND_CLOSEST_ULL(tmp_64, uV);
+
+	return regulator_set_current_limit(psec->ps, 0, uA);
+}
+EXPORT_SYMBOL_GPL(pse_ethtool_set_pw_limit);
+
 bool pse_has_podl(struct pse_control *psec)
 {
 	return psec->pcdev->types & ETHTOOL_PSE_PODL;
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 38b9308e5e7a..e9a7f0e5e555 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -9,6 +9,9 @@
 #include <linux/list.h>
 #include <uapi/linux/ethtool.h>
 
+/* Maximum current in uA according to IEEE 802.3-2022 Table 145-1 */
+#define MAX_PI_CURRENT 1920000
+
 struct phy_device;
 struct pse_controller_dev;
 
@@ -41,6 +44,7 @@ struct pse_control_config {
  * @c33_actual_pw: power currently delivered by the PSE in mW
  *	IEEE 802.3-2022 30.9.1.1.23 aPSEActualPower
  * @c33_ext_state_info: extended state information of the PSE
+ * @c33_pw_limit: power limit of the PSE
  */
 struct pse_control_status {
 	enum ethtool_podl_pse_admin_state podl_admin_state;
@@ -50,6 +54,7 @@ struct pse_control_status {
 	u32 c33_pw_class;
 	u32 c33_actual_pw;
 	struct ethtool_c33_pse_ext_state_info c33_ext_state_info;
+	u32 c33_pw_limit;
 };
 
 /**
@@ -61,6 +66,12 @@ struct pse_control_status {
  *		   May also return negative errno.
  * @pi_enable: Configure the PSE PI as enabled.
  * @pi_disable: Configure the PSE PI as disabled.
+ * @pi_get_voltage: Return voltage similarly to get_voltage regulator
+ *		    callback.
+ * @pi_get_current_limit: Get the configured current limit similarly to
+ *			  get_current_limit regulator callback.
+ * @pi_set_current_limit: Configure the current limit similarly to
+ *			  set_current_limit regulator callback.
  */
 struct pse_controller_ops {
 	int (*ethtool_get_status)(struct pse_controller_dev *pcdev,
@@ -70,6 +81,11 @@ struct pse_controller_ops {
 	int (*pi_is_enabled)(struct pse_controller_dev *pcdev, int id);
 	int (*pi_enable)(struct pse_controller_dev *pcdev, int id);
 	int (*pi_disable)(struct pse_controller_dev *pcdev, int id);
+	int (*pi_get_voltage)(struct pse_controller_dev *pcdev, int id);
+	int (*pi_get_current_limit)(struct pse_controller_dev *pcdev,
+				    int id);
+	int (*pi_set_current_limit)(struct pse_controller_dev *pcdev,
+				    int id, int max_uA);
 };
 
 struct module;
@@ -156,6 +172,11 @@ int pse_ethtool_get_status(struct pse_control *psec,
 int pse_ethtool_set_config(struct pse_control *psec,
 			   struct netlink_ext_ack *extack,
 			   const struct pse_control_config *config);
+int pse_ethtool_set_pw_limit(struct pse_control *psec,
+			     struct netlink_ext_ack *extack,
+			     const unsigned int pw_limit);
+int pse_ethtool_get_pw_limit(struct pse_control *psec,
+			     struct netlink_ext_ack *extack);
 
 bool pse_has_podl(struct pse_control *psec);
 bool pse_has_c33(struct pse_control *psec);
@@ -185,6 +206,19 @@ static inline int pse_ethtool_set_config(struct pse_control *psec,
 	return -EOPNOTSUPP;
 }
 
+static inline int pse_ethtool_set_pw_limit(struct pse_control *psec,
+					   struct netlink_ext_ack *extack,
+					   const unsigned int pw_limit)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int pse_ethtool_get_pw_limit(struct pse_control *psec,
+					   struct netlink_ext_ack *extack)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline bool pse_has_podl(struct pse_control *psec)
 {
 	return false;

-- 
2.34.1

[PATCH net-next v3 5/7] net: ethtool: Add new power limit get and set features

[Kory Maincent]

From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

This patch expands the status information provided by ethtool for PSE c33
with power limit. It also adds a call to pse_ethtool_set_pw_limit() to
configure the PSE control power limit.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Change in v3:
- Add ethtool netlink documentation.
---
 Documentation/networking/ethtool-netlink.rst |  8 ++++++
 include/uapi/linux/ethtool_netlink.h         |  1 +
 net/ethtool/pse-pd.c                         | 42 +++++++++++++++++++++++-----
 3 files changed, 44 insertions(+), 7 deletions(-)

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.
+
 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.
+
 RSS_GET
 =======
 
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 398a0aa8daad..62ac9e1001bf 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -919,6 +919,7 @@ enum {
 	ETHTOOL_A_C33_PSE_ACTUAL_PW,		/* u32 */
 	ETHTOOL_A_C33_PSE_EXT_STATE,		/* u32 */
 	ETHTOOL_A_C33_PSE_EXT_SUBSTATE,		/* u32 */
+	ETHTOOL_A_C33_PSE_PW_LIMIT,		/* u32 */
 
 	/* add new constants above here */
 	__ETHTOOL_A_PSE_CNT,
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index fec56db557d3..5c5eccd5b32b 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -94,6 +94,9 @@ static int pse_reply_size(const struct ethnl_req_info *req_base,
 		len += nla_total_size(sizeof(u32)); /* _C33_PSE_EXT_STATE */
 	if (st->c33_ext_state_info.__c33_pse_ext_substate > 0)
 		len += nla_total_size(sizeof(u32)); /* _C33_PSE_EXT_SUBSTATE */
+	if (st->c33_pw_limit > 0)
+		len += nla_total_size(sizeof(u32)); /* _C33_PSE_PW_LIMIT */
+
 	return len;
 }
 
@@ -144,6 +147,11 @@ static int pse_fill_reply(struct sk_buff *skb,
 			st->c33_ext_state_info.__c33_pse_ext_substate))
 		return -EMSGSIZE;
 
+	if (st->c33_pw_limit > 0 &&
+	    nla_put_u32(skb, ETHTOOL_A_C33_PSE_PW_LIMIT,
+			st->c33_pw_limit))
+		return -EMSGSIZE;
+
 	return 0;
 }
 
@@ -157,6 +165,7 @@ const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1] = {
 	[ETHTOOL_A_C33_PSE_ADMIN_CONTROL] =
 		NLA_POLICY_RANGE(NLA_U32, ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED,
 				 ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED),
+	[ETHTOOL_A_C33_PSE_PW_LIMIT] = { .type = NLA_U32 },
 };
 
 static int
@@ -199,19 +208,38 @@ static int
 ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
 {
 	struct net_device *dev = req_info->dev;
-	struct pse_control_config config = {};
 	struct nlattr **tb = info->attrs;
 	struct phy_device *phydev;
+	int ret = 0;
 
 	phydev = dev->phydev;
+
+	if (tb[ETHTOOL_A_C33_PSE_PW_LIMIT]) {
+		unsigned int pw_limit = nla_get_u32(tb[ETHTOOL_A_C33_PSE_PW_LIMIT]);
+
+		ret = pse_ethtool_set_pw_limit(phydev->psec, info->extack,
+					       pw_limit);
+		if (ret)
+			return ret;
+	}
+
 	/* These values are already validated by the ethnl_pse_set_policy */
-	if (pse_has_podl(phydev->psec))
-		config.podl_admin_control = nla_get_u32(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
-	if (pse_has_c33(phydev->psec))
-		config.c33_admin_control = nla_get_u32(tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL]);
+	if (tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] ||
+	    tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL]) {
+		struct pse_control_config config = {};
+
+		if (pse_has_podl(phydev->psec))
+			config.podl_admin_control = nla_get_u32(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
+		if (pse_has_c33(phydev->psec))
+			config.c33_admin_control = nla_get_u32(tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL]);
+
+		ret = pse_ethtool_set_config(phydev->psec, info->extack,
+					     &config);
+		if (ret)
+			return ret;
+	}
 
-	/* Return errno directly - PSE has no notification */
-	return pse_ethtool_set_config(phydev->psec, info->extack, &config);
+	return ret;
 }
 
 const struct ethnl_request_ops ethnl_pse_request_ops = {

-- 
2.34.1

Re: [PATCH net-next v3 5/7] net: ethtool: Add new power limit get and set features

[Oleksij Rempel]

Hi Köry,

On Fri, Jun 14, 2024 at 04:33:21PM +0200, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> 
> This patch expands the status information provided by ethtool for PSE c33
> with power limit. It also adds a call to pse_ethtool_set_pw_limit() to
> configure the PSE control power limit.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
> 
> Change in v3:
> - Add ethtool netlink documentation.
> ---
>  Documentation/networking/ethtool-netlink.rst |  8 ++++++
>  include/uapi/linux/ethtool_netlink.h         |  1 +
>  net/ethtool/pse-pd.c                         | 42 +++++++++++++++++++++++-----
>  3 files changed, 44 insertions(+), 7 deletions(-)
> 
> 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.
> +
>  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? 

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 |

Re: [PATCH net-next v3 5/7] net: ethtool: Add new power limit get and set features

[Oleksij Rempel]

On Sat, Jun 15, 2024 at 05:59:36PM +0200, Oleksij Rempel wrote:
> Hi Köry,
> 
> On Fri, Jun 14, 2024 at 04:33:21PM +0200, Kory Maincent wrote:
> > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> > 
> > This patch expands the status information provided by ethtool for PSE c33
> > with power limit. It also adds a call to pse_ethtool_set_pw_limit() to
> > configure the PSE control power limit.
> > 
> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> > ---
> > 
> > Change in v3:
> > - Add ethtool netlink documentation.
> > ---
> >  Documentation/networking/ethtool-netlink.rst |  8 ++++++
> >  include/uapi/linux/ethtool_netlink.h         |  1 +
> >  net/ethtool/pse-pd.c                         | 42 +++++++++++++++++++++++-----
> >  3 files changed, 44 insertions(+), 7 deletions(-)
> > 
> > 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.
> > +
> >  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.

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

Re: [PATCH net-next v3 5/7] net: ethtool: Add new power limit get and set features

[Oleksij Rempel]

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 |

Re: [PATCH net-next v3 5/7] net: ethtool: Add new power limit get and set features

[Kory Maincent]

On Sun, 16 Jun 2024 08:07:20 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> On Sat, Jun 15, 2024 at 08:28:30PM +0200, Oleksij Rempel wrote:
>  [...]  
> 
> 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 

Yes, good idea.
 
> 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)   |
> +-+-+--------------------------------------+--------+----------------------------+

Not sure the ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_RANGE_ENTRY bring anything
interesting.

 +--------------------------------------------+--------+----------------------------+
 | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_RANGES`` | nested | array of power limit ranges|
 +-+------------------------------------------+--------+----------------------------+
 | | ``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)   |
 +-+------------------------------------------+--------+----------------------------+

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

Thanks for the rephrasing!

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

Re: [PATCH net-next v3 5/7] net: ethtool: Add new power limit get and set features

[Oleksij Rempel]

On Mon, Jun 17, 2024 at 06:14:13PM +0200, Kory Maincent wrote:
> >  +------------------------------------------+--------+----------------------------+
> >  | ``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)   |
> > +-+-+--------------------------------------+--------+----------------------------+
> 
> Not sure the ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_RANGE_ENTRY bring anything
> interesting.
> 
>  +--------------------------------------------+--------+----------------------------+
>  | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_RANGES`` | nested | array of power limit ranges|
>  +-+------------------------------------------+--------+----------------------------+
>  | | ``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)   |
>  +-+------------------------------------------+--------+----------------------------+

Ack. Good point.
-- 
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 |

[PATCH net-next v3 6/7] netlink: specs: Expand the PSE netlink command with C33 pw-limit attributes

[Kory Maincent]

From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

Expand the c33 PSE attributes with power limit to be able to set and get
the PSE Power Interface power limit.

./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do pse-get
             --json '{"header":{"dev-name":"eth2"}}'
{'c33-pse-actual-pw': 1700,
 'c33-pse-admin-state': 3,
 'c33-pse-pw-class': 4,
 'c33-pse-pw-d-status': 4,
 'c33-pse-pw-limit': 90000,
 'header': {'dev-index': 6, 'dev-name': 'eth2'}}

./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do pse-set
             --json '{"header":{"dev-name":"eth2"},
                      "c33-pse-pw-limit":19000}'
None

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 Documentation/netlink/specs/ethtool.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 0ff27319856c..d5a91265cb85 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -953,6 +953,10 @@ attribute-sets:
         name: c33-pse-ext-substate
         type: u32
         name-prefix: ethtool-a-
+      -
+        name: c33-pse-pw-limit
+        type: u32
+        name-prefix: ethtool-a-
   -
     name: rss
     attributes:
@@ -1646,6 +1650,7 @@ operations:
             - c33-pse-actual-pw
             - c33-pse-ext-state
             - c33-pse-ext-substate
+            - c33-pse-pw-limit
       dump: *pse-get-op
     -
       name: pse-set

-- 
2.34.1

Re: [PATCH net-next v3 6/7] netlink: specs: Expand the PSE netlink command with C33 pw-limit attributes

[Donald Hunter]

Kory Maincent <kory.maincent@bootlin.com> writes:

> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
>
> Expand the c33 PSE attributes with power limit to be able to set and get
> the PSE Power Interface power limit.
>
> ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do pse-get
>              --json '{"header":{"dev-name":"eth2"}}'
> {'c33-pse-actual-pw': 1700,
>  'c33-pse-admin-state': 3,
>  'c33-pse-pw-class': 4,
>  'c33-pse-pw-d-status': 4,
>  'c33-pse-pw-limit': 90000,
>  'header': {'dev-index': 6, 'dev-name': 'eth2'}}
>
> ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --do pse-set
>              --json '{"header":{"dev-name":"eth2"},
>                       "c33-pse-pw-limit":19000}'
> None
>
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

Re: [PATCH net-next v3 6/7] netlink: specs: Expand the PSE netlink command with C33 pw-limit attributes

[Kory Maincent]

On Mon, 17 Jun 2024 09:03:12 +0100
Donald Hunter <donald.hunter@gmail.com> wrote:

> Kory Maincent <kory.maincent@bootlin.com> writes:
> 
>  [...]  
> 
> Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

Hello Donald,

Thanks for your Reviewed-by. I won't add the tag to the next version of the
patch series because the change asked by Oleskij on patch 5 will change the
specs.

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

[PATCH net-next v3 7/7] net: pse-pd: pd692x0: Enhance with new current limit and voltage read callbacks

[Kory Maincent]

From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

This patch expands PSE callbacks with newly introduced
pi_get/set_current_limit() and pi_get_voltage() callback.
The only way to set ps692x0 port power limit is by configure the power
class plus a small power supplement which maximum depends on each class.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Change in v2:
- Use uA and uV instead of mA and mV to have more precision in the power
  calculation. Need to use 64bit variables for the calculation.
- Modify the behavior in case of setting the current out of the available
  ranges. Report an error now.
---
 drivers/net/pse-pd/pd692x0.c | 193 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 191 insertions(+), 2 deletions(-)

diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
index c5e0456e7103..5b7d9fc83af7 100644
--- a/drivers/net/pse-pd/pd692x0.c
+++ b/drivers/net/pse-pd/pd692x0.c
@@ -74,6 +74,8 @@ enum {
 	PD692X0_MSG_GET_PORT_STATUS,
 	PD692X0_MSG_DOWNLOAD_CMD,
 	PD692X0_MSG_GET_PORT_CLASS,
+	PD692X0_MSG_GET_PORT_MEAS,
+	PD692X0_MSG_GET_PORT_PARAM,
 
 	/* add new message above here */
 	PD692X0_MSG_CNT
@@ -135,7 +137,7 @@ static const struct pd692x0_msg pd692x0_msg_template_list[PD692X0_MSG_CNT] = {
 	[PD692X0_MSG_SET_PORT_PARAM] = {
 		.key = PD692X0_KEY_CMD,
 		.sub = {0x05, 0xc0},
-		.data = {   0, 0xff, 0xff, 0xff,
+		.data = { 0xf, 0xff, 0xff, 0xff,
 			 0x4e, 0x4e, 0x4e, 0x4e},
 	},
 	[PD692X0_MSG_GET_PORT_STATUS] = {
@@ -156,6 +158,18 @@ static const struct pd692x0_msg pd692x0_msg_template_list[PD692X0_MSG_CNT] = {
 		.data = {0x4e, 0x4e, 0x4e, 0x4e,
 			 0x4e, 0x4e, 0x4e, 0x4e},
 	},
+	[PD692X0_MSG_GET_PORT_MEAS] = {
+		.key = PD692X0_KEY_REQ,
+		.sub = {0x05, 0xc5},
+		.data = {0x4e, 0x4e, 0x4e, 0x4e,
+			 0x4e, 0x4e, 0x4e, 0x4e},
+	},
+	[PD692X0_MSG_GET_PORT_PARAM] = {
+		.key = PD692X0_KEY_REQ,
+		.sub = {0x05, 0xc0},
+		.data = {0x4e, 0x4e, 0x4e, 0x4e,
+			 0x4e, 0x4e, 0x4e, 0x4e},
+	},
 };
 
 static u8 pd692x0_build_msg(struct pd692x0_msg *msg, u8 echo)
@@ -520,6 +534,85 @@ pd692x0_get_ext_state(struct ethtool_c33_pse_ext_state_info *c33_ext_state_info,
 	}
 }
 
+struct pd692x0_class_pw {
+	int class;
+	int class_cfg_value;
+	int class_pw;
+	int max_added_class_pw;
+};
+
+/* 4/2 pairs class configuration power table in compliance mode.
+ * Need to be arranged in ascending order of power support.
+ */
+static const struct pd692x0_class_pw pd692x0_class_pw_table[] = {
+	{.class = 3, .class_cfg_value = 0x3, .class_pw = 15000, .max_added_class_pw = 3100},
+	{.class = 4, .class_cfg_value = 0x2, .class_pw = 30000, .max_added_class_pw = 8000},
+	{.class = 6, .class_cfg_value = 0x1, .class_pw = 60000, .max_added_class_pw = 5000},
+	{.class = 8, .class_cfg_value = 0x0, .class_pw = 90000, .max_added_class_pw = 7500},
+	{ /* sentinel */ }
+};
+
+static int pd692x0_pi_get_pw_from_table(int op_mode, int added_pw)
+{
+	const struct pd692x0_class_pw *pw_table;
+
+	pw_table = pd692x0_class_pw_table;
+	while (pw_table->class) {
+		if (pw_table->class_cfg_value == op_mode)
+			return pw_table->class_pw + added_pw * 100;
+
+		pw_table++;
+	}
+
+	return -ERANGE;
+}
+
+static int pd692x0_pi_set_pw_from_table(struct device *dev,
+					struct pd692x0_msg *msg, int pw)
+{
+	const struct pd692x0_class_pw *pw_table;
+
+	pw_table = pd692x0_class_pw_table;
+	if (pw < pw_table->class_pw) {
+		dev_err(dev,
+			"Power limit %dmW not supported. Ranges minimal available: [%d-%d]\n",
+			pw,
+			pw_table->class_pw,
+			pw_table->class_pw + pw_table->max_added_class_pw);
+		return -ERANGE;
+	}
+
+	while (pw_table->class) {
+		if (pw > (pw_table->class_pw + pw_table->max_added_class_pw)) {
+			pw_table++;
+			continue;
+		}
+
+		if (pw < pw_table->class_pw) {
+			dev_err(dev,
+				"Power limit %dmW not supported. Ranges availables: [%d-%d] or [%d-%d]\n",
+				pw,
+				(pw_table - 1)->class_pw,
+				(pw_table - 1)->class_pw + (pw_table - 1)->max_added_class_pw,
+				pw_table->class_pw,
+				pw_table->class_pw + pw_table->max_added_class_pw);
+			return -ERANGE;
+		}
+
+		msg->data[2] = pw_table->class_cfg_value;
+		msg->data[3] = (pw - pw_table->class_pw) / 100;
+		return 0;
+	}
+
+	pw_table--;
+	dev_warn(dev,
+		 "Power limit %dmW not supported. Set to highest power limit %dmW\n",
+		 pw, pw_table->class_pw + pw_table->max_added_class_pw);
+	msg->data[2] = pw_table->class_cfg_value;
+	msg->data[3] = pw_table->max_added_class_pw / 100;
+	return 0;
+}
+
 static int pd692x0_ethtool_get_status(struct pse_controller_dev *pcdev,
 				      unsigned long id,
 				      struct netlink_ext_ack *extack,
@@ -558,9 +651,20 @@ static int pd692x0_ethtool_get_status(struct pse_controller_dev *pcdev,
 	priv->admin_state[id] = status->c33_admin_state;
 
 	pd692x0_get_ext_state(&status->c33_ext_state_info, buf.sub[0]);
-
 	status->c33_actual_pw = (buf.data[0] << 4 | buf.data[1]) * 100;
 
+	msg = pd692x0_msg_template_list[PD692X0_MSG_GET_PORT_PARAM];
+	msg.sub[2] = id;
+	memset(&buf, 0, sizeof(buf));
+	ret = pd692x0_sendrecv_msg(priv, &msg, &buf);
+	if (ret < 0)
+		return ret;
+
+	ret = pd692x0_pi_get_pw_from_table(buf.data[0], buf.data[1]);
+	if (ret < 0)
+		return ret;
+	status->c33_pw_limit = ret;
+
 	memset(&buf, 0, sizeof(buf));
 	msg = pd692x0_msg_template_list[PD692X0_MSG_GET_PORT_CLASS];
 	msg.sub[2] = id;
@@ -850,12 +954,97 @@ static int pd692x0_setup_pi_matrix(struct pse_controller_dev *pcdev)
 	return ret;
 }
 
+static int pd692x0_pi_get_voltage(struct pse_controller_dev *pcdev, int id)
+{
+	struct pd692x0_priv *priv = to_pd692x0_priv(pcdev);
+	struct pd692x0_msg msg, buf = {0};
+	int ret;
+
+	ret = pd692x0_fw_unavailable(priv);
+	if (ret)
+		return ret;
+
+	msg = pd692x0_msg_template_list[PD692X0_MSG_GET_PORT_MEAS];
+	msg.sub[2] = id;
+	ret = pd692x0_sendrecv_msg(priv, &msg, &buf);
+	if (ret < 0)
+		return ret;
+
+	/* Convert 0.1V unit to uV */
+	return (buf.sub[0] << 8 | buf.sub[1]) * 100000;
+}
+
+static int pd692x0_pi_get_current_limit(struct pse_controller_dev *pcdev,
+					int id)
+{
+	struct pd692x0_priv *priv = to_pd692x0_priv(pcdev);
+	struct pd692x0_msg msg, buf = {0};
+	int mW, uV, uA, ret;
+	s64 tmp_64;
+
+	msg = pd692x0_msg_template_list[PD692X0_MSG_GET_PORT_PARAM];
+	msg.sub[2] = id;
+	ret = pd692x0_sendrecv_msg(priv, &msg, &buf);
+	if (ret < 0)
+		return ret;
+
+	ret = pd692x0_pi_get_pw_from_table(buf.data[2], buf.data[3]);
+	if (ret < 0)
+		return ret;
+	mW = ret;
+
+	ret = pd692x0_pi_get_voltage(pcdev, id);
+	if (ret < 0)
+		return ret;
+	uV = ret;
+
+	tmp_64 = mW;
+	tmp_64 *= 1000000000ull;
+	/* uA = mW * 1000000000 / uV */
+	uA = DIV_ROUND_CLOSEST_ULL(tmp_64, uV);
+	return uA;
+}
+
+static int pd692x0_pi_set_current_limit(struct pse_controller_dev *pcdev,
+					int id, int max_uA)
+{
+	struct pd692x0_priv *priv = to_pd692x0_priv(pcdev);
+	struct device *dev = &priv->client->dev;
+	struct pd692x0_msg msg, buf = {0};
+	int uV, ret, mW;
+	s64 tmp_64;
+
+	ret = pd692x0_fw_unavailable(priv);
+	if (ret)
+		return ret;
+
+	ret = pd692x0_pi_get_voltage(pcdev, id);
+	if (ret < 0)
+		return ret;
+	uV = ret;
+
+	msg = pd692x0_msg_template_list[PD692X0_MSG_SET_PORT_PARAM];
+	msg.sub[2] = id;
+	tmp_64 = uV;
+	tmp_64 *= max_uA;
+	/* mW = uV * uA / 1000000000 */
+	mW = DIV_ROUND_CLOSEST_ULL(tmp_64, 1000000000);
+	ret = pd692x0_pi_set_pw_from_table(dev, &msg, mW);
+	if (ret)
+		return ret;
+
+	return pd692x0_sendrecv_msg(priv, &msg, &buf);
+}
+
 static const struct pse_controller_ops pd692x0_ops = {
 	.setup_pi_matrix = pd692x0_setup_pi_matrix,
 	.ethtool_get_status = pd692x0_ethtool_get_status,
 	.pi_enable = pd692x0_pi_enable,
 	.pi_disable = pd692x0_pi_disable,
 	.pi_is_enabled = pd692x0_pi_is_enabled,
+	.pi_get_voltage = pd692x0_pi_get_voltage,
+	.pi_get_current_limit = pd692x0_pi_get_current_limit,
+	.pi_set_current_limit = pd692x0_pi_set_current_limit,
 };
 
 #define PD692X0_FW_LINE_MAX_SZ 0xff

-- 
2.34.1