[PATCH net-next v2 0/4] net: pse-pd: pd692x0: Add permanent configuration management support

[PATCH net-next v2 0/4] net: pse-pd: pd692x0: Add permanent configuration management support

[Kory Maincent]

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

This patch series adds support for saving and resetting the PD692x0
device's permanent configuration through driver devlink param attributes:
PD692X0_DEVLINK_PARAM_ID_SAVE_CONF and PD692X0_DEVLINK_PARAM_ID_RESET_CONF.

The permanent configuration allows settings to persist across device
resets and power cycles, providing better control over PSE behavior
in production environments.

Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
Changes in v2:
- Move from sysfs interface to devlink interface for the permanent
  configuration support
- Remove the __free macro from pd692x0 driver following net policy.
- Link to v1: https://lore.kernel.org/r/20250822-feature_poe_permanent_conf-v1-0-dcd41290254d@bootlin.com

---
Kory Maincent (4):
      net: pse-pd: pd692x0: Replace __free macro with explicit kfree calls
      net: pse-pd: pd692x0: Separate configuration parsing from hardware setup
      docs: devlink: Sort table of contents alphabetically
      net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

 Documentation/networking/devlink/index.rst   |  21 +-
 Documentation/networking/devlink/pd692x0.rst |  32 +++
 drivers/net/pse-pd/pd692x0.c                 | 325 +++++++++++++++++++++++----
 3 files changed, 325 insertions(+), 53 deletions(-)
---
base-commit: 01a8d87c13cc9ccd3692d3e750075a772b3626da
change-id: 20250813-feature_poe_permanent_conf-ec640dace1f2

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

[PATCH net-next v2 1/4] net: pse-pd: pd692x0: Replace __free macro with explicit kfree calls

[Kory Maincent]

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

Replace __free(kfree) with explicit kfree() calls to follow the net
subsystem policy of avoiding automatic cleanup macros as described in
the documentation.

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

Changes in v2:
- New patch
---
 drivers/net/pse-pd/pd692x0.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
index f4e91ba64a666..055e925c853ef 100644
--- a/drivers/net/pse-pd/pd692x0.c
+++ b/drivers/net/pse-pd/pd692x0.c
@@ -1200,9 +1200,9 @@ static void pd692x0_managers_free_pw_budget(struct pd692x0_priv *priv)
 
 static int pd692x0_setup_pi_matrix(struct pse_controller_dev *pcdev)
 {
-	struct pd692x0_manager *manager __free(kfree) = NULL;
 	struct pd692x0_priv *priv = to_pd692x0_priv(pcdev);
 	struct pd692x0_matrix port_matrix[PD692X0_MAX_PIS];
+	struct pd692x0_manager *manager;
 	int ret, nmanagers;
 
 	/* Should we flash the port matrix */
@@ -1216,7 +1216,7 @@ static int pd692x0_setup_pi_matrix(struct pse_controller_dev *pcdev)
 
 	ret = pd692x0_of_get_managers(priv, manager);
 	if (ret < 0)
-		return ret;
+		goto err_free_manager;
 
 	nmanagers = ret;
 	ret = pd692x0_register_managers_regulator(priv, manager, nmanagers);
@@ -1236,12 +1236,15 @@ static int pd692x0_setup_pi_matrix(struct pse_controller_dev *pcdev)
 		goto err_managers_req_pw;
 
 	pd692x0_of_put_managers(priv, manager, nmanagers);
+	kfree(manager);
 	return 0;
 
 err_managers_req_pw:
 	pd692x0_managers_free_pw_budget(priv);
 err_of_managers:
 	pd692x0_of_put_managers(priv, manager, nmanagers);
+err_free_manager:
+	kfree(manager);
 	return ret;
 }
 

-- 
2.43.0

[PATCH net-next v2 2/4] net: pse-pd: pd692x0: Separate configuration parsing from hardware setup

[Kory Maincent]

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

Cache the port matrix configuration in driver private data to enable
PSE controller reconfiguration. This refactoring separates device tree
parsing from hardware configuration application, allowing settings to be
reapplied without reparsing the device tree.

This prepares for upcoming permanent configuration support where cached
settings can be restored after device resets or firmware updates.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 drivers/net/pse-pd/pd692x0.c | 115 ++++++++++++++++++++++++++++---------------
 1 file changed, 76 insertions(+), 39 deletions(-)

diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
index 055e925c853ef..782b1abf94cb1 100644
--- a/drivers/net/pse-pd/pd692x0.c
+++ b/drivers/net/pse-pd/pd692x0.c
@@ -85,6 +85,11 @@ enum {
 	PD692X0_MSG_CNT
 };
 
+struct pd692x0_matrix {
+	u8 hw_port_a;
+	u8 hw_port_b;
+};
+
 struct pd692x0_priv {
 	struct i2c_client *client;
 	struct pse_controller_dev pcdev;
@@ -101,6 +106,8 @@ struct pd692x0_priv {
 	enum ethtool_c33_pse_admin_state admin_state[PD692X0_MAX_PIS];
 	struct regulator_dev *manager_reg[PD692X0_MAX_MANAGERS];
 	int manager_pw_budget[PD692X0_MAX_MANAGERS];
+	int nmanagers;
+	struct pd692x0_matrix *port_matrix;
 };
 
 /* Template list of communication messages. The non-null bytes defined here
@@ -809,11 +816,6 @@ struct pd692x0_manager {
 	int nports;
 };
 
-struct pd692x0_matrix {
-	u8 hw_port_a;
-	u8 hw_port_b;
-};
-
 static int
 pd692x0_of_get_ports_manager(struct pd692x0_priv *priv,
 			     struct pd692x0_manager *manager,
@@ -903,7 +905,8 @@ pd692x0_of_get_managers(struct pd692x0_priv *priv,
 	}
 
 	of_node_put(managers_node);
-	return nmanagers;
+	priv->nmanagers = nmanagers;
+	return 0;
 
 out:
 	for (i = 0; i < nmanagers; i++) {
@@ -963,8 +966,7 @@ pd692x0_register_manager_regulator(struct device *dev, char *reg_name,
 
 static int
 pd692x0_register_managers_regulator(struct pd692x0_priv *priv,
-				    const struct pd692x0_manager *manager,
-				    int nmanagers)
+				    const struct pd692x0_manager *manager)
 {
 	struct device *dev = &priv->client->dev;
 	size_t reg_name_len;
@@ -975,7 +977,7 @@ pd692x0_register_managers_regulator(struct pd692x0_priv *priv,
 	 */
 	reg_name_len = strlen(dev_name(dev)) + 23;
 
-	for (i = 0; i < nmanagers; i++) {
+	for (i = 0; i < priv->nmanagers; i++) {
 		static const char * const regulators[] = { "vaux5", "vaux3p3" };
 		struct regulator_dev *rdev;
 		char *reg_name;
@@ -1008,10 +1010,14 @@ pd692x0_register_managers_regulator(struct pd692x0_priv *priv,
 }
 
 static int
-pd692x0_conf_manager_power_budget(struct pd692x0_priv *priv, int id, int pw)
+pd692x0_conf_manager_power_budget(struct pd692x0_priv *priv, int id)
 {
 	struct pd692x0_msg msg, buf;
-	int ret, pw_mW = pw / 1000;
+	int ret, pw_mW;
+
+	pw_mW = priv->manager_pw_budget[id] / 1000;
+	if (!pw_mW)
+		return 0;
 
 	msg = pd692x0_msg_template_list[PD692X0_MSG_GET_POWER_BANK];
 	msg.data[0] = id;
@@ -1032,11 +1038,11 @@ pd692x0_conf_manager_power_budget(struct pd692x0_priv *priv, int id, int pw)
 }
 
 static int
-pd692x0_configure_managers(struct pd692x0_priv *priv, int nmanagers)
+pd692x0_req_managers_pw_budget(struct pd692x0_priv *priv)
 {
 	int i, ret;
 
-	for (i = 0; i < nmanagers; i++) {
+	for (i = 0; i < priv->nmanagers; i++) {
 		struct regulator *supply = priv->manager_reg[i]->supply;
 		int pw_budget;
 
@@ -1053,7 +1059,18 @@ pd692x0_configure_managers(struct pd692x0_priv *priv, int nmanagers)
 			return ret;
 
 		priv->manager_pw_budget[i] = pw_budget;
-		ret = pd692x0_conf_manager_power_budget(priv, i, pw_budget);
+	}
+
+	return 0;
+}
+
+static int
+pd692x0_configure_managers(struct pd692x0_priv *priv)
+{
+	int i, ret;
+
+	for (i = 0; i < priv->nmanagers; i++) {
+		ret = pd692x0_conf_manager_power_budget(priv, i);
 		if (ret < 0)
 			return ret;
 	}
@@ -1101,10 +1118,9 @@ pd692x0_set_port_matrix(const struct pse_pi_pairset *pairset,
 
 static int
 pd692x0_set_ports_matrix(struct pd692x0_priv *priv,
-			 const struct pd692x0_manager *manager,
-			 int nmanagers,
-			 struct pd692x0_matrix port_matrix[PD692X0_MAX_PIS])
+			 const struct pd692x0_manager *manager)
 {
+	struct pd692x0_matrix *port_matrix = priv->port_matrix;
 	struct pse_controller_dev *pcdev = &priv->pcdev;
 	int i, ret;
 
@@ -1117,7 +1133,7 @@ pd692x0_set_ports_matrix(struct pd692x0_priv *priv,
 	/* Update with values for every PSE PIs */
 	for (i = 0; i < pcdev->nr_lines; i++) {
 		ret = pd692x0_set_port_matrix(&pcdev->pi[i].pairset[0],
-					      manager, nmanagers,
+					      manager, priv->nmanagers,
 					      &port_matrix[i]);
 		if (ret) {
 			dev_err(&priv->client->dev,
@@ -1126,7 +1142,7 @@ pd692x0_set_ports_matrix(struct pd692x0_priv *priv,
 		}
 
 		ret = pd692x0_set_port_matrix(&pcdev->pi[i].pairset[1],
-					      manager, nmanagers,
+					      manager, priv->nmanagers,
 					      &port_matrix[i]);
 		if (ret) {
 			dev_err(&priv->client->dev,
@@ -1139,9 +1155,9 @@ pd692x0_set_ports_matrix(struct pd692x0_priv *priv,
 }
 
 static int
-pd692x0_write_ports_matrix(struct pd692x0_priv *priv,
-			   const struct pd692x0_matrix port_matrix[PD692X0_MAX_PIS])
+pd692x0_write_ports_matrix(struct pd692x0_priv *priv)
 {
+	struct pd692x0_matrix *port_matrix = priv->port_matrix;
 	struct pd692x0_msg msg, buf;
 	int ret, i;
 
@@ -1166,13 +1182,32 @@ pd692x0_write_ports_matrix(struct pd692x0_priv *priv,
 	return 0;
 }
 
+static int pd692x0_hw_conf_init(struct pd692x0_priv *priv)
+{
+	int ret;
+
+	/* Is PD692x0 ready to be configured? */
+	if (priv->fw_state != PD692X0_FW_OK &&
+	    priv->fw_state != PD692X0_FW_COMPLETE)
+		return 0;
+
+	ret = pd692x0_configure_managers(priv);
+	if (ret)
+		return ret;
+
+	ret = pd692x0_write_ports_matrix(priv);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static void pd692x0_of_put_managers(struct pd692x0_priv *priv,
-				    struct pd692x0_manager *manager,
-				    int nmanagers)
+				    struct pd692x0_manager *manager)
 {
 	int i, j;
 
-	for (i = 0; i < nmanagers; i++) {
+	for (i = 0; i < priv->nmanagers; i++) {
 		for (j = 0; j < manager[i].nports; j++)
 			of_node_put(manager[i].port_node[j]);
 		of_node_put(manager[i].node);
@@ -1201,48 +1236,50 @@ static void pd692x0_managers_free_pw_budget(struct pd692x0_priv *priv)
 static int pd692x0_setup_pi_matrix(struct pse_controller_dev *pcdev)
 {
 	struct pd692x0_priv *priv = to_pd692x0_priv(pcdev);
-	struct pd692x0_matrix port_matrix[PD692X0_MAX_PIS];
+	struct pd692x0_matrix *port_matrix;
 	struct pd692x0_manager *manager;
-	int ret, nmanagers;
-
-	/* Should we flash the port matrix */
-	if (priv->fw_state != PD692X0_FW_OK &&
-	    priv->fw_state != PD692X0_FW_COMPLETE)
-		return 0;
+	int ret;
 
 	manager = kcalloc(PD692X0_MAX_MANAGERS, sizeof(*manager), GFP_KERNEL);
 	if (!manager)
 		return -ENOMEM;
 
+	port_matrix = devm_kcalloc(&priv->client->dev, PD692X0_MAX_PIS,
+				   sizeof(*port_matrix), GFP_KERNEL);
+	if (!port_matrix) {
+		ret = -ENOMEM;
+		goto err_free_manager;
+	}
+	priv->port_matrix = port_matrix;
+
 	ret = pd692x0_of_get_managers(priv, manager);
 	if (ret < 0)
 		goto err_free_manager;
 
-	nmanagers = ret;
-	ret = pd692x0_register_managers_regulator(priv, manager, nmanagers);
+	ret = pd692x0_register_managers_regulator(priv, manager);
 	if (ret)
 		goto err_of_managers;
 
-	ret = pd692x0_configure_managers(priv, nmanagers);
+	ret = pd692x0_req_managers_pw_budget(priv);
 	if (ret)
 		goto err_of_managers;
 
-	ret = pd692x0_set_ports_matrix(priv, manager, nmanagers, port_matrix);
+	ret = pd692x0_set_ports_matrix(priv, manager);
 	if (ret)
 		goto err_managers_req_pw;
 
-	ret = pd692x0_write_ports_matrix(priv, port_matrix);
+	ret = pd692x0_hw_conf_init(priv);
 	if (ret)
 		goto err_managers_req_pw;
 
-	pd692x0_of_put_managers(priv, manager, nmanagers);
+	pd692x0_of_put_managers(priv, manager);
 	kfree(manager);
 	return 0;
 
 err_managers_req_pw:
 	pd692x0_managers_free_pw_budget(priv);
 err_of_managers:
-	pd692x0_of_put_managers(priv, manager, nmanagers);
+	pd692x0_of_put_managers(priv, manager);
 err_free_manager:
 	kfree(manager);
 	return ret;
@@ -1647,7 +1684,7 @@ static enum fw_upload_err pd692x0_fw_poll_complete(struct fw_upload *fwl)
 		return FW_UPLOAD_ERR_FW_INVALID;
 	}
 
-	ret = pd692x0_setup_pi_matrix(&priv->pcdev);
+	ret = pd692x0_hw_conf_init(priv);
 	if (ret < 0) {
 		dev_err(&client->dev, "Error configuring ports matrix (%pe)\n",
 			ERR_PTR(ret));

-- 
2.43.0

[PATCH net-next v2 3/4] docs: devlink: Sort table of contents alphabetically

[Kory Maincent]

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

Sort devlink documentation table of contents alphabetically to improve
readability and make it easier to locate specific chapters.

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

Changes in v2:
- New patch
---
 Documentation/networking/devlink/index.rst | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index 270a65a014111..0c58e5c729d92 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -56,18 +56,18 @@ general.
    :maxdepth: 1
 
    devlink-dpipe
+   devlink-eswitch-attr
+   devlink-flash
    devlink-health
    devlink-info
-   devlink-flash
+   devlink-linecard
    devlink-params
    devlink-port
    devlink-region
-   devlink-resource
    devlink-reload
+   devlink-resource
    devlink-selftests
    devlink-trap
-   devlink-linecard
-   devlink-eswitch-attr
 
 Driver-specific documentation
 -----------------------------
@@ -78,12 +78,14 @@ parameters, info versions, and other features it supports.
 .. toctree::
    :maxdepth: 1
 
+   am65-nuss-cpsw-switch
    bnxt
    etas_es58x
    hns3
    i40e
-   ionic
    ice
+   ionic
+   iosm
    ixgbe
    kvaser_pciefd
    kvaser_usb
@@ -93,11 +95,9 @@ parameters, info versions, and other features it supports.
    mv88e6xxx
    netdevsim
    nfp
-   qed
-   ti-cpsw-switch
-   am65-nuss-cpsw-switch
-   prestera
-   iosm
    octeontx2
+   prestera
+   qed
    sfc
+   ti-cpsw-switch
    zl3073x

-- 
2.43.0

[PATCH net-next v2 4/4] net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

[Kory Maincent]

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

Add devlink param attributes PD692X0_DEVLINK_PARAM_ID_SAVE_CONF and
PD692X0_DEVLINK_PARAM_ID_RESET_CONF to enable userspace management of the
PSE's permanent configuration stored in non-volatile memory.

The save_conf attribute with the '1' value allows saving the current
configuration to non-volatile memory. The reset_conf attribute restores
factory defaults configurations.

Skip hardware configuration initialization on probe when a saved
configuration is already present in non-volatile memory (detected via user
byte 42).

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

Changes in v2:
- Move on from sysfs to devlink param for userspace management.
---
 Documentation/networking/devlink/index.rst   |   1 +
 Documentation/networking/devlink/pd692x0.rst |  32 +++++
 drivers/net/pse-pd/pd692x0.c                 | 205 ++++++++++++++++++++++++++-
 3 files changed, 235 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index 0c58e5c729d92..6db7d9b45f7aa 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -96,6 +96,7 @@ parameters, info versions, and other features it supports.
    netdevsim
    nfp
    octeontx2
+   pd692x0
    prestera
    qed
    sfc
diff --git a/Documentation/networking/devlink/pd692x0.rst b/Documentation/networking/devlink/pd692x0.rst
new file mode 100644
index 0000000000000..3f3edd0ac0361
--- /dev/null
+++ b/Documentation/networking/devlink/pd692x0.rst
@@ -0,0 +1,32 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========================
+PSE PD692x0 devlink support
+===========================
+
+This document describes the devlink features implemented by the PSE ``PD692x0``
+device drivers.
+
+Parameters
+==========
+
+The ``PD692x0`` drivers implement the following driver-specific parameters.
+
+.. list-table:: Driver-specific parameters implemented
+   :widths: 5 5 5 85
+
+   * - Name
+     - Type
+     - Mode
+     - Description
+   * - ``save_conf``
+     - bool
+     - runtime
+     - Save the current configuration to non-volatile memory using ``1``
+       attribute value.
+   * - ``reset_conf``
+     - bool
+     - runtime
+     - Reset the current and saved configuration using ``1`` attribute
+       value.
+
diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
index 782b1abf94cb1..eb4b911d438b3 100644
--- a/drivers/net/pse-pd/pd692x0.c
+++ b/drivers/net/pse-pd/pd692x0.c
@@ -14,6 +14,7 @@
 #include <linux/pse-pd/pse.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
+#include <net/devlink.h>
 
 #define PD692X0_PSE_NAME "pd692x0_pse"
 
@@ -30,6 +31,8 @@
 #define PD692X0_FW_MIN_VER	5
 #define PD692X0_FW_PATCH_VER	5
 
+#define PD692X0_USER_BYTE	42
+
 enum pd692x0_fw_state {
 	PD692X0_FW_UNKNOWN,
 	PD692X0_FW_OK,
@@ -80,6 +83,9 @@ enum {
 	PD692X0_MSG_GET_PORT_PARAM,
 	PD692X0_MSG_GET_POWER_BANK,
 	PD692X0_MSG_SET_POWER_BANK,
+	PD692X0_MSG_SET_USER_BYTE,
+	PD692X0_MSG_SAVE_SYS_SETTINGS,
+	PD692X0_MSG_RESTORE_FACTORY,
 
 	/* add new message above here */
 	PD692X0_MSG_CNT
@@ -103,11 +109,13 @@ struct pd692x0_priv {
 	bool last_cmd_key;
 	unsigned long last_cmd_key_time;
 
+	bool cfg_saved;
 	enum ethtool_c33_pse_admin_state admin_state[PD692X0_MAX_PIS];
 	struct regulator_dev *manager_reg[PD692X0_MAX_MANAGERS];
 	int manager_pw_budget[PD692X0_MAX_MANAGERS];
 	int nmanagers;
 	struct pd692x0_matrix *port_matrix;
+	struct devlink *dl;
 };
 
 /* Template list of communication messages. The non-null bytes defined here
@@ -193,6 +201,24 @@ static const struct pd692x0_msg pd692x0_msg_template_list[PD692X0_MSG_CNT] = {
 		.key = PD692X0_KEY_CMD,
 		.sub = {0x07, 0x0b, 0x57},
 	},
+	[PD692X0_MSG_SET_USER_BYTE] = {
+		.key = PD692X0_KEY_PRG,
+		.sub = {0x41, PD692X0_USER_BYTE},
+		.data = {0x4e, 0x4e, 0x4e, 0x4e,
+			 0x4e, 0x4e, 0x4e, 0x4e},
+	},
+	[PD692X0_MSG_SAVE_SYS_SETTINGS] = {
+		.key = PD692X0_KEY_PRG,
+		.sub = {0x06, 0x0f},
+		.data = {0x4e, 0x4e, 0x4e, 0x4e,
+			 0x4e, 0x4e, 0x4e, 0x4e},
+	},
+	[PD692X0_MSG_RESTORE_FACTORY] = {
+		.key = PD692X0_KEY_PRG,
+		.sub = {0x2d, 0x4e},
+		.data = {0x4e, 0x4e, 0x4e, 0x4e,
+			 0x4e, 0x4e, 0x4e, 0x4e},
+	},
 };
 
 static u8 pd692x0_build_msg(struct pd692x0_msg *msg, u8 echo)
@@ -1268,9 +1294,12 @@ static int pd692x0_setup_pi_matrix(struct pse_controller_dev *pcdev)
 	if (ret)
 		goto err_managers_req_pw;
 
-	ret = pd692x0_hw_conf_init(priv);
-	if (ret)
-		goto err_managers_req_pw;
+	/* Do not init the conf if it is already saved */
+	if (!priv->cfg_saved) {
+		ret = pd692x0_hw_conf_init(priv);
+		if (ret)
+			goto err_managers_req_pw;
+	}
 
 	pd692x0_of_put_managers(priv, manager);
 	kfree(manager);
@@ -1727,14 +1756,148 @@ static const struct fw_upload_ops pd692x0_fw_ops = {
 	.cleanup = pd692x0_fw_cleanup,
 };
 
+/* Devlink Params APIs */
+enum pd692x0_devlink_param_id {
+	PD692X0_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
+	PD692X0_DEVLINK_PARAM_ID_SAVE_CONF,
+	PD692X0_DEVLINK_PARAM_ID_RESET_CONF,
+};
+
+struct pd692x0_devlink {
+	struct pd692x0_priv *priv;
+};
+
+static int pd692x0_dl_validate(struct devlink *devlink, u32 id,
+			       union devlink_param_value val,
+			       struct netlink_ext_ack *extack)
+{
+	if (!val.vbool) {
+		NL_SET_ERR_MSG_FMT(extack, "0 is not a valid value");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int pd692x0_dl_save_conf_set(struct devlink *devlink, u32 id,
+				    struct devlink_param_gset_ctx *ctx,
+				    struct netlink_ext_ack *extack)
+{
+	struct pd692x0_devlink *dl = devlink_priv(devlink);
+	struct pd692x0_priv *priv = dl->priv;
+	struct pd692x0_msg msg, buf = {0};
+	int ret;
+
+	mutex_lock(&priv->pcdev.lock);
+	ret = pd692x0_fw_unavailable(priv);
+	if (ret)
+		goto out;
+
+	msg = pd692x0_msg_template_list[PD692X0_MSG_SET_USER_BYTE];
+	ret = pd692x0_sendrecv_msg(priv, &msg, &buf);
+	if (ret)
+		goto out;
+
+	msg = pd692x0_msg_template_list[PD692X0_MSG_SAVE_SYS_SETTINGS];
+	ret = pd692x0_send_msg(priv, &msg);
+	if (ret) {
+		dev_err(&priv->client->dev,
+			"Failed to save the configuration (%pe)\n",
+			ERR_PTR(ret));
+		goto out;
+	}
+
+	msleep(50); /* Sleep 50ms as described in the datasheet */
+
+	ret = i2c_master_recv(priv->client, (u8 *)&buf, sizeof(buf));
+	if (ret != sizeof(buf)) {
+		if (ret >= 0)
+			ret = -EIO;
+		goto out;
+	}
+
+	ret = 0;
+
+out:
+	mutex_unlock(&priv->pcdev.lock);
+	return ret;
+}
+
+static int pd692x0_dl_reset_conf_set(struct devlink *devlink, u32 id,
+				     struct devlink_param_gset_ctx *ctx,
+				     struct netlink_ext_ack *extack)
+{
+	struct pd692x0_devlink *dl = devlink_priv(devlink);
+	struct pd692x0_priv *priv = dl->priv;
+	struct pd692x0_msg msg, buf = {0};
+	int ret;
+
+	mutex_lock(&priv->pcdev.lock);
+	ret = pd692x0_fw_unavailable(priv);
+	if (ret)
+		goto out;
+
+	msg = pd692x0_msg_template_list[PD692X0_MSG_RESTORE_FACTORY];
+	ret = pd692x0_send_msg(priv, &msg);
+	if (ret) {
+		dev_err(&priv->client->dev,
+			"Failed to reset the configuration (%pe)\n",
+			ERR_PTR(ret));
+		goto out;
+	}
+
+	msleep(100); /* Sleep 100ms as described in the datasheet */
+
+	ret = i2c_master_recv(priv->client, (u8 *)&buf, sizeof(buf));
+	if (ret != sizeof(buf)) {
+		if (ret >= 0)
+			ret = -EIO;
+		goto out;
+	}
+
+	ret = pd692x0_hw_conf_init(priv);
+	if (ret) {
+		dev_err(&priv->client->dev,
+			"Error configuring ports matrix (%pe)\n",
+			ERR_PTR(ret));
+	}
+
+out:
+	mutex_unlock(&priv->pcdev.lock);
+	return ret;
+}
+
+static int pd692x0_dl_dummy_get(struct devlink *devlink, u32 id,
+				struct devlink_param_gset_ctx *ctx)
+{
+	return 0;
+}
+
+static const struct devlink_param pd692x0_dl_params[] = {
+	DEVLINK_PARAM_DRIVER(PD692X0_DEVLINK_PARAM_ID_SAVE_CONF,
+			     "save_conf", DEVLINK_PARAM_TYPE_BOOL,
+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			     pd692x0_dl_dummy_get, pd692x0_dl_save_conf_set,
+			     pd692x0_dl_validate),
+	DEVLINK_PARAM_DRIVER(PD692X0_DEVLINK_PARAM_ID_RESET_CONF,
+			     "reset_conf", DEVLINK_PARAM_TYPE_BOOL,
+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			     pd692x0_dl_dummy_get, pd692x0_dl_reset_conf_set,
+			     pd692x0_dl_validate),
+};
+
+static const struct devlink_ops pd692x0_dl_ops = { };
+
 static int pd692x0_i2c_probe(struct i2c_client *client)
 {
 	static const char * const regulators[] = { "vdd", "vdda" };
 	struct pd692x0_msg msg, buf = {0}, zero = {0};
+	struct pd692x0_devlink *pd692x0_dl;
 	struct device *dev = &client->dev;
 	struct pd692x0_msg_ver ver;
 	struct pd692x0_priv *priv;
 	struct fw_upload *fwl;
+	struct devlink *dl;
 	int ret;
 
 	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
@@ -1793,6 +1956,9 @@ static int pd692x0_i2c_probe(struct i2c_client *client)
 		}
 	}
 
+	if (buf.data[2] == PD692X0_USER_BYTE)
+		priv->cfg_saved = true;
+
 	priv->np = dev->of_node;
 	priv->pcdev.nr_lines = PD692X0_MAX_PIS;
 	priv->pcdev.owner = THIS_MODULE;
@@ -1813,14 +1979,47 @@ static int pd692x0_i2c_probe(struct i2c_client *client)
 				     "failed to register to the Firmware Upload API\n");
 	priv->fwl = fwl;
 
+	dl = devlink_alloc(&pd692x0_dl_ops,
+			   sizeof(struct pd692x0_devlink), dev);
+	if (!dl) {
+		dev_err(dev, "devlink_alloc failed\n");
+		ret = -ENOMEM;
+		goto err_unregister_fw;
+	}
+
+	pd692x0_dl = devlink_priv(dl);
+	pd692x0_dl->priv = priv;
+	priv->dl = dl;
+
+	ret = devlink_params_register(dl, pd692x0_dl_params,
+				      ARRAY_SIZE(pd692x0_dl_params));
+	if (ret) {
+		dev_err(dev,
+			"devlink params register failed with error %d",
+			ret);
+		goto err_free_dl;
+	}
+
+	devlink_register(dl);
 	return 0;
+
+err_free_dl:
+	devlink_free(dl);
+err_unregister_fw:
+	firmware_upload_unregister(priv->fwl);
+
+	return ret;
 }
 
 static void pd692x0_i2c_remove(struct i2c_client *client)
 {
 	struct pd692x0_priv *priv = i2c_get_clientdata(client);
+	struct devlink *dl = priv->dl;
 
 	pd692x0_managers_free_pw_budget(priv);
+	devlink_params_unregister(dl, pd692x0_dl_params,
+				  ARRAY_SIZE(pd692x0_dl_params));
+	devlink_unregister(dl);
 	firmware_upload_unregister(priv->fwl);
 }
 

-- 
2.43.0

Re: [PATCH net-next v2 4/4] net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

[Jakub Kicinski]

On Fri, 29 Aug 2025 18:28:46 +0200 Kory Maincent wrote:
> +The ``PD692x0`` drivers implement the following driver-specific parameters.
> +
> +.. list-table:: Driver-specific parameters implemented
> +   :widths: 5 5 5 85
> +
> +   * - Name
> +     - Type
> +     - Mode
> +     - Description
> +   * - ``save_conf``
> +     - bool
> +     - runtime
> +     - Save the current configuration to non-volatile memory using ``1``
> +       attribute value.
> +   * - ``reset_conf``
> +     - bool
> +     - runtime
> +     - Reset the current and saved configuration using ``1`` attribute
> +       value.

Sorry for not offering a clear alternative, but I'm not aware of any
precedent for treating devlink params as action triggers. devlink params
should be values that can be set and read, which is clearly not
the case here:

> +static int pd692x0_dl_dummy_get(struct devlink *devlink, u32 id,
> +				struct devlink_param_gset_ctx *ctx)
> +{
> +	return 0;
> +}
-- 
pw-bot: cr

Re: [PATCH net-next v2 4/4] net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

[Kory Maincent]

On Mon, 1 Sep 2025 13:31:00 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Fri, 29 Aug 2025 18:28:46 +0200 Kory Maincent wrote:
> > +The ``PD692x0`` drivers implement the following driver-specific parameters.
> > +
> > +.. list-table:: Driver-specific parameters implemented
> > +   :widths: 5 5 5 85
> > +
> > +   * - Name
> > +     - Type
> > +     - Mode
> > +     - Description
> > +   * - ``save_conf``
> > +     - bool
> > +     - runtime
> > +     - Save the current configuration to non-volatile memory using ``1``
> > +       attribute value.
> > +   * - ``reset_conf``
> > +     - bool
> > +     - runtime
> > +     - Reset the current and saved configuration using ``1`` attribute
> > +       value.  
> 
> Sorry for not offering a clear alternative, but I'm not aware of any
> precedent for treating devlink params as action triggers. devlink params
> should be values that can be set and read, which is clearly not
> the case here:

Ok.
We could save the configuration for every config change and add a reset-conf
action to devlink reload uAPI? The drawback it that it will bring a bit of
latency (about 110ms) for every config change.

Or adding a new devlink uAPI like a devlink conf but maybe we don't have enough
cases to add such generic new uAPI.
Or get back to the first proposition to use sysfs. 

What do you think?

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

Re: [PATCH net-next v2 4/4] net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

[Jakub Kicinski]

On Tue, 2 Sep 2025 16:43:14 +0200 Kory Maincent wrote:
> > On Fri, 29 Aug 2025 18:28:46 +0200 Kory Maincent wrote:  
> > > +The ``PD692x0`` drivers implement the following driver-specific parameters.
> > > +
> > > +.. list-table:: Driver-specific parameters implemented
> > > +   :widths: 5 5 5 85
> > > +
> > > +   * - Name
> > > +     - Type
> > > +     - Mode
> > > +     - Description
> > > +   * - ``save_conf``
> > > +     - bool
> > > +     - runtime
> > > +     - Save the current configuration to non-volatile memory using ``1``
> > > +       attribute value.
> > > +   * - ``reset_conf``
> > > +     - bool
> > > +     - runtime
> > > +     - Reset the current and saved configuration using ``1`` attribute
> > > +       value.    
> > 
> > Sorry for not offering a clear alternative, but I'm not aware of any
> > precedent for treating devlink params as action triggers. devlink params
> > should be values that can be set and read, which is clearly not
> > the case here:  
> 
> Ok.
> We could save the configuration for every config change and add a reset-conf
> action to devlink reload uAPI? The drawback it that it will bring a bit of
> latency (about 110ms) for every config change.
> 
> Or adding a new devlink uAPI like a devlink conf but maybe we don't have enough
> cases to add such generic new uAPI.
> Or get back to the first proposition to use sysfs. 
> 
> What do you think?

If you are asking for my real preference, abstracting away whether it's
doable and justifiable amount of effort for you -- I'd explore using
flags in the ethtool header to control whether setting is written to
the flash.

Re: [PATCH net-next v2 4/4] net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

[Jakub Kicinski]

On Tue, 2 Sep 2025 13:42:12 -0700 Jakub Kicinski wrote:
> On Tue, 2 Sep 2025 16:43:14 +0200 Kory Maincent wrote:
> > > Sorry for not offering a clear alternative, but I'm not aware of any
> > > precedent for treating devlink params as action triggers. devlink params
> > > should be values that can be set and read, which is clearly not
> > > the case here:    
> > 
> > Ok.
> > We could save the configuration for every config change and add a reset-conf
> > action to devlink reload uAPI? The drawback it that it will bring a bit of
> > latency (about 110ms) for every config change.
> > 
> > Or adding a new devlink uAPI like a devlink conf but maybe we don't have enough
> > cases to add such generic new uAPI.
> > Or get back to the first proposition to use sysfs. 
> > 
> > What do you think?  
> 
> If you are asking for my real preference, abstracting away whether it's
> doable and justifiable amount of effort for you -- I'd explore using
> flags in the ethtool header to control whether setting is written to
> the flash.

PS. failing that the less uAPI the better. Tho, given that the whole
point here is giving user the ability to write the flash -- asking for
uAPI-light approach feels contradictory.

Taking a step back -- the "save to flash" is something that OEM FW
often supports. But for Linux-based control the "save to flash" should
really be equivalent to updating some user space config. When user
configures interfaces in OpenWRT we're not flashing them into the
device tree... Could you perhaps explain what makes updating the
in-flash config a high-priority requirement for PoE?

Re: [PATCH net-next v2 4/4] net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

[Oleksij Rempel]

On Tue, Sep 02, 2025 at 01:48:44PM -0700, Jakub Kicinski wrote:
> On Tue, 2 Sep 2025 13:42:12 -0700 Jakub Kicinski wrote:
> > On Tue, 2 Sep 2025 16:43:14 +0200 Kory Maincent wrote:
> > > > Sorry for not offering a clear alternative, but I'm not aware of any
> > > > precedent for treating devlink params as action triggers. devlink params
> > > > should be values that can be set and read, which is clearly not
> > > > the case here:    
> > > 
> > > Ok.
> > > We could save the configuration for every config change and add a reset-conf
> > > action to devlink reload uAPI? The drawback it that it will bring a bit of
> > > latency (about 110ms) for every config change.
> > > 
> > > Or adding a new devlink uAPI like a devlink conf but maybe we don't have enough
> > > cases to add such generic new uAPI.
> > > Or get back to the first proposition to use sysfs. 
> > > 
> > > What do you think?  
> > 
> > If you are asking for my real preference, abstracting away whether it's
> > doable and justifiable amount of effort for you -- I'd explore using
> > flags in the ethtool header to control whether setting is written to
> > the flash.
> 
> PS. failing that the less uAPI the better. Tho, given that the whole
> point here is giving user the ability to write the flash -- asking for
> uAPI-light approach feels contradictory.
> 
> Taking a step back -- the "save to flash" is something that OEM FW
> often supports. But for Linux-based control the "save to flash" should
> really be equivalent to updating some user space config. When user
> configures interfaces in OpenWRT we're not flashing them into the
> device tree... Could you perhaps explain what makes updating the
> in-flash config a high-priority requirement for PoE?
> 

I think the main use case question is: what happens if the application
CPU reboots?
Do we go back to “safe defaults”? But what are safe defaults - that can
vary a lot between systems.

In many setups, if the CPU reboots it also means the bridge is down, so
there is no packet forwarding. In that case, does it even make sense to
keep providing PoE power if the networking part is non-functional?

Another angle: does it make sense to overwrite the hardware power-on
defaults each time the system starts? Or should we rather be able to
read back the stored defaults from the hardware into the driver and work
with them?

Does anyone here have field experience with similar devices? How are
these topics usually handled outside of my bubble?

Best 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 v2 4/4] net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

[Kory Maincent]

On Wed, 3 Sep 2025 09:10:28 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> On Tue, Sep 02, 2025 at 01:48:44PM -0700, Jakub Kicinski wrote:
> > On Tue, 2 Sep 2025 13:42:12 -0700 Jakub Kicinski wrote:  
> > > On Tue, 2 Sep 2025 16:43:14 +0200 Kory Maincent wrote:  
> > > > > Sorry for not offering a clear alternative, but I'm not aware of any
> > > > > precedent for treating devlink params as action triggers. devlink
> > > > > params should be values that can be set and read, which is clearly not
> > > > > the case here:      
> > > > 
> > > > Ok.
> > > > We could save the configuration for every config change and add a
> > > > reset-conf action to devlink reload uAPI? The drawback it that it will
> > > > bring a bit of latency (about 110ms) for every config change.
> > > > 
> > > > Or adding a new devlink uAPI like a devlink conf but maybe we don't
> > > > have enough cases to add such generic new uAPI.
> > > > Or get back to the first proposition to use sysfs. 
> > > > 
> > > > What do you think?    
> > > 
> > > If you are asking for my real preference, abstracting away whether it's
> > > doable and justifiable amount of effort for you -- I'd explore using
> > > flags in the ethtool header to control whether setting is written to
> > > the flash.  
> > 
> > PS. failing that the less uAPI the better. Tho, given that the whole
> > point here is giving user the ability to write the flash -- asking for
> > uAPI-light approach feels contradictory.
> > 
> > Taking a step back -- the "save to flash" is something that OEM FW
> > often supports. But for Linux-based control the "save to flash" should
> > really be equivalent to updating some user space config. When user
> > configures interfaces in OpenWRT we're not flashing them into the
> > device tree... Could you perhaps explain what makes updating the
> > in-flash config a high-priority requirement for PoE?
> >   
> 
> I think the main use case question is: what happens if the application
> CPU reboots?
> Do we go back to “safe defaults”? But what are safe defaults - that can
> vary a lot between systems.

In case of CPU reboot, the port matrix will be flashed, which means the
controller is restarted and the ports get disconnected.
Therefore indeed we will go back to default settings.
 
> In many setups, if the CPU reboots it also means the bridge is down, so
> there is no packet forwarding. In that case, does it even make sense to
> keep providing PoE power if the networking part is non-functional?

It depends, we might not want to reboot the Powered Devices if the switch
reboot. I don't currently have specific case in mind which could need this
behavior.
Mainly, the Dent Project final aim was to support mainline all the features
supported in their poed tool.
https://github.com/dentproject/poed/blob/main/dentos-poe-agent/opt/poeagent/docs/Userguide

> Another angle: does it make sense to overwrite the hardware power-on
> defaults each time the system starts? Or should we rather be able to
> read back the stored defaults from the hardware into the driver and work
> with them?

Yes that is one of the design proposition, but we will still need a way to
reset the conf as said before.

> Does anyone here have field experience with similar devices? How are
> these topics usually handled outside of my bubble?

Kyle any field experience on this?

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

Re: [PATCH net-next v2 4/4] net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

[Oleksij Rempel]

On Wed, Sep 03, 2025 at 11:10:25AM +0200, Kory Maincent wrote:
> On Wed, 3 Sep 2025 09:10:28 +0200
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > On Tue, Sep 02, 2025 at 01:48:44PM -0700, Jakub Kicinski wrote:
> > > On Tue, 2 Sep 2025 13:42:12 -0700 Jakub Kicinski wrote:  
> > > > On Tue, 2 Sep 2025 16:43:14 +0200 Kory Maincent wrote:  
> > > > > > Sorry for not offering a clear alternative, but I'm not aware of any
> > > > > > precedent for treating devlink params as action triggers. devlink
> > > > > > params should be values that can be set and read, which is clearly not
> > > > > > the case here:      
> > > > > 
> > > > > Ok.
> > > > > We could save the configuration for every config change and add a
> > > > > reset-conf action to devlink reload uAPI? The drawback it that it will
> > > > > bring a bit of latency (about 110ms) for every config change.
> > > > > 
> > > > > Or adding a new devlink uAPI like a devlink conf but maybe we don't
> > > > > have enough cases to add such generic new uAPI.
> > > > > Or get back to the first proposition to use sysfs. 
> > > > > 
> > > > > What do you think?    
> > > > 
> > > > If you are asking for my real preference, abstracting away whether it's
> > > > doable and justifiable amount of effort for you -- I'd explore using
> > > > flags in the ethtool header to control whether setting is written to
> > > > the flash.  
> > > 
> > > PS. failing that the less uAPI the better. Tho, given that the whole
> > > point here is giving user the ability to write the flash -- asking for
> > > uAPI-light approach feels contradictory.
> > > 
> > > Taking a step back -- the "save to flash" is something that OEM FW
> > > often supports. But for Linux-based control the "save to flash" should
> > > really be equivalent to updating some user space config. When user
> > > configures interfaces in OpenWRT we're not flashing them into the
> > > device tree... Could you perhaps explain what makes updating the
> > > in-flash config a high-priority requirement for PoE?
> > >   
> > 
> > I think the main use case question is: what happens if the application
> > CPU reboots?
> > Do we go back to “safe defaults”? But what are safe defaults - that can
> > vary a lot between systems.
> 
> In case of CPU reboot, the port matrix will be flashed, which means the
> controller is restarted and the ports get disconnected.
> Therefore indeed we will go back to default settings.
>  
> > In many setups, if the CPU reboots it also means the bridge is down, so
> > there is no packet forwarding. In that case, does it even make sense to
> > keep providing PoE power if the networking part is non-functional?
> 
> It depends, we might not want to reboot the Powered Devices if the switch
> reboot. I don't currently have specific case in mind which could need this
> behavior.
> Mainly, the Dent Project final aim was to support mainline all the features
> supported in their poed tool.
> https://github.com/dentproject/poed/blob/main/dentos-poe-agent/opt/poeagent/docs/Userguide
> 
> > Another angle: does it make sense to overwrite the hardware power-on
> > defaults each time the system starts? Or should we rather be able to
> > read back the stored defaults from the hardware into the driver and work
> > with them?
> 
> Yes that is one of the design proposition, but we will still need a way to
> reset the conf as said before.
> 
> > Does anyone here have field experience with similar devices? How are
> > these topics usually handled outside of my bubble?
> 
> Kyle any field experience on this?

I can confirm a field case from industrial/medical gear. Closed system,
several modules on SPE, PoDL for power. Requirement: power the PDs as
early as possible, even before Linux. The box boots faster if power-up
and Linux init run in parallel. In this setup the power-on state is
pre-designed by the product team and should not be changed by Linux at
runtime.

So the question is how to communicate and control this:

Option A - Vendor tool writes a fixed config to NVM (EEPROM)
Pro: matches "pre-designed, don't touch" model; PDs come up early
without Linux.
Con: needs extra vendor tooling; hard to keep in sync with what
userspace shows; Linux may not know what is in NVM unless we
read/reflect it.

Option B - Do all changes in RAM, then one explicit "commit to NVM"
Pro: one write; predictable latency hit only on commit; maps to a
"transaction/commit" model.
Con: what if the controller discarded some changes during the session?
We would need a clear commit status and a way to report which settings
actually stuck.

Option C - Write-through: every change also goes to NVM
Pro: if the system resets, config is always up to date.
Con: adds about 50-110 ms per change on this hardware; may be too slow
for interactive tools or batch reconfig.

From API side, if write-through is possible on this hardware, we can
likely make this per-port and per-setting:

ethtool per-port setters can take a "persist=1" flag. Driver applies the
change and also writes it to NVM for that port.

If a particular setting (bit/field) cannot be persisted by the
controller/NVM, the driver returns an error for the whole request. Userspace
then knows persistence is not supported for that item.

Factory reset:
If hardware supports per-port defaults in NVM, provide a per-port
factory_reset op.

Do PD692x0 supports per-port save/restore functionality?

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 v2 4/4] net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

[Kory Maincent]

On Wed, 3 Sep 2025 12:59:42 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> On Wed, Sep 03, 2025 at 11:10:25AM +0200, Kory Maincent wrote:
> > On Wed, 3 Sep 2025 09:10:28 +0200
> > Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> >   
> > > On Tue, Sep 02, 2025 at 01:48:44PM -0700, Jakub Kicinski wrote:  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
> > > 
> > > I think the main use case question is: what happens if the application
> > > CPU reboots?
> > > Do we go back to “safe defaults”? But what are safe defaults - that can
> > > vary a lot between systems.  
> > 
> > In case of CPU reboot, the port matrix will be flashed, which means the
> > controller is restarted and the ports get disconnected.
> > Therefore indeed we will go back to default settings.
> >    
> > > In many setups, if the CPU reboots it also means the bridge is down, so
> > > there is no packet forwarding. In that case, does it even make sense to
> > > keep providing PoE power if the networking part is non-functional?  
> > 
> > It depends, we might not want to reboot the Powered Devices if the switch
> > reboot. I don't currently have specific case in mind which could need this
> > behavior.
> > Mainly, the Dent Project final aim was to support mainline all the features
> > supported in their poed tool.
> > https://github.com/dentproject/poed/blob/main/dentos-poe-agent/opt/poeagent/docs/Userguide
> >   
> > > Another angle: does it make sense to overwrite the hardware power-on
> > > defaults each time the system starts? Or should we rather be able to
> > > read back the stored defaults from the hardware into the driver and work
> > > with them?  
> > 
> > Yes that is one of the design proposition, but we will still need a way to
> > reset the conf as said before.
> >   
> > > Does anyone here have field experience with similar devices? How are
> > > these topics usually handled outside of my bubble?  
> > 
> > Kyle any field experience on this?  
> 
> I can confirm a field case from industrial/medical gear. Closed system,
> several modules on SPE, PoDL for power. Requirement: power the PDs as
> early as possible, even before Linux. The box boots faster if power-up
> and Linux init run in parallel. In this setup the power-on state is
> pre-designed by the product team and should not be changed by Linux at
> runtime.
> 
> So the question is how to communicate and control this:
> 
> Option A - Vendor tool writes a fixed config to NVM (EEPROM)
> Pro: matches "pre-designed, don't touch" model; PDs come up early
> without Linux.
> Con: needs extra vendor tooling; hard to keep in sync with what
> userspace shows; Linux may not know what is in NVM unless we
> read/reflect it.
> 
> Option B - Do all changes in RAM, then one explicit "commit to NVM"
> Pro: one write; predictable latency hit only on commit; maps to a
> "transaction/commit" model.
> Con: what if the controller discarded some changes during the session?
> We would need a clear commit status and a way to report which settings
> actually stuck.
> 
> Option C - Write-through: every change also goes to NVM
> Pro: if the system resets, config is always up to date.
> Con: adds about 50-110 ms per change on this hardware; may be too slow
> for interactive tools or batch reconfig.
> 
> From API side, if write-through is possible on this hardware, we can
> likely make this per-port and per-setting:
> 
> ethtool per-port setters can take a "persist=1" flag. Driver applies the
> change and also writes it to NVM for that port.

The thing is, as it is not per port save if we use one time this ethtool flag
all the other ports configurations will also be saved. This could mislead users
by letting them believe they can save configuration for one specific port
without saving all the other ports config.
I think saving the config to NVM globally on each config change is a better
idea.

> If a particular setting (bit/field) cannot be persisted by the
> controller/NVM, the driver returns an error for the whole request. Userspace
> then knows persistence is not supported for that item.
>
> Factory reset:
> If hardware supports per-port defaults in NVM, provide a per-port
> factory_reset op.
>
> Do PD692x0 supports per-port save/restore functionality?

No it is not per port but at the PSE controller level for both.

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