[PATCH net-next v2 0/7] add generic PSE support

[PATCH net-next v2 0/7] add generic PSE support

[Oleksij Rempel]

Add generic support for the Ethernet Power Sourcing Equipment.

changes are listed within patches.

Oleksij Rempel (7):
  dt-bindings: net: pse-dt: add bindings for generic PSE controller
  dt-bindings: net: phy: add PoDL PSE property
  net: add framework to support Ethernet PSE and PDs devices
  net: mdiobus: fwnode_mdiobus_register_phy() rework error handling
  net: mdiobus: search for PSE nodes by parsing PHY nodes.
  ethtool: add interface to interact with Ethernet Power Equipment
  net: pse-pd: add generic PSE driver

 .../devicetree/bindings/net/ethernet-phy.yaml |   5 +
 .../bindings/net/pse-pd/generic-pse.yaml      |  95 +++++
 Documentation/networking/ethtool-netlink.rst  |  60 +++
 drivers/net/Kconfig                           |   2 +
 drivers/net/Makefile                          |   1 +
 drivers/net/mdio/fwnode_mdio.c                |  58 ++-
 drivers/net/phy/phy_device.c                  |   2 +
 drivers/net/pse-pd/Kconfig                    |  22 +
 drivers/net/pse-pd/Makefile                   |   6 +
 drivers/net/pse-pd/pse_core.c                 | 381 ++++++++++++++++++
 drivers/net/pse-pd/pse_generic.c              | 148 +++++++
 include/linux/ethtool.h                       |  27 ++
 include/linux/phy.h                           |   2 +
 include/linux/pse-pd/pse.h                    | 134 ++++++
 include/uapi/linux/ethtool.h                  |  50 +++
 include/uapi/linux/ethtool_netlink.h          |  17 +
 net/ethtool/Makefile                          |   3 +-
 net/ethtool/common.c                          |  10 +
 net/ethtool/common.h                          |   1 +
 net/ethtool/netlink.c                         |  19 +
 net/ethtool/netlink.h                         |   4 +
 net/ethtool/pse-pd.c                          | 187 +++++++++
 22 files changed, 1222 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/pse-pd/generic-pse.yaml
 create mode 100644 drivers/net/pse-pd/Kconfig
 create mode 100644 drivers/net/pse-pd/Makefile
 create mode 100644 drivers/net/pse-pd/pse_core.c
 create mode 100644 drivers/net/pse-pd/pse_generic.c
 create mode 100644 include/linux/pse-pd/pse.h
 create mode 100644 net/ethtool/pse-pd.c

-- 
2.30.2

[PATCH net-next v2 1/7] dt-bindings: net: pse-dt: add bindings for generic PSE controller

[Oleksij Rempel]

Add binding for generic Ethernet PSE controller.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- rename compatible to more generic "ieee802.3-pse"
- add class and type properties for PoDL and PoE variants
- add pairs property
---
 .../bindings/net/pse-pd/generic-pse.yaml      | 95 +++++++++++++++++++
 1 file changed, 95 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/pse-pd/generic-pse.yaml

diff --git a/Documentation/devicetree/bindings/net/pse-pd/generic-pse.yaml b/Documentation/devicetree/bindings/net/pse-pd/generic-pse.yaml
new file mode 100644
index 0000000000000..ecd226df66f4e
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pse-pd/generic-pse.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/pse-pd/generic-pse.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic Power Sourcing Equipment
+
+maintainers:
+  - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: Generic PSE controller. The device must be referenced by the PHY
+  node to control power injection to the Ethernet cable.
+
+properties:
+  compatible:
+    const: ieee802.3-pse
+
+  '#pse-cells':
+    const: 0
+
+  ieee802.3-pse-supply:
+    description: Power supply for the PSE controller
+
+  ieee802.3-pairs:
+    $ref: /schemas/types.yaml#/definitions/int8-array
+    description: Array of number of twisted-pairs capable to deliver power.
+      Since not all circuits are able to support all pair variants, the array of
+      supported variants should be specified.
+      Note - single twisted-pair PSE is formally know as PoDL PSE.
+    items:
+      enum: [1, 2, 4]
+
+  ieee802.3-pse-type:
+    $ref: /schemas/types.yaml#/definitions/uint8
+    minimum: 1
+    maximum: 2
+    description: PSE Type. Describes classification- and class-capabilities.
+      Not compatible with PoDL PSE Type.
+      Type 1 - provides a Class 0, 1, 2, or 3 signature during Physical Layer
+      classification.
+      Type 2 - provides a Class 4 signature during Physical Layer
+      classification, understands 2-Event classification, and is capable of
+      Data Link Layer classification.
+
+  ieee802.3-pse-class:
+    $ref: /schemas/types.yaml#/definitions/int8-array
+    items:
+      enum: [0, 1, 2, 3, 4]
+    description: PSE Class. Array of supported classes by the 2 and 4 pair PSE.
+
+  ieee802.3-podl-pse-type:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [a, b, c, d, e]
+    description: PoDL PSE Type. Describes compatibility to physical Layer
+      specifications.
+      Type A - cost optimized for 100BASE-T1
+      Type B - cost optimized for 1000BASE-T1
+      Type C - works with 100BASE-T1 and 1000BASE-T1
+      Type D - optimized for 10BASE-T1S
+      Type E - optimized for 10BASE-T1L
+
+  ieee802.3-podl-pse-class:
+    $ref: /schemas/types.yaml#/definitions/int8-array
+    items:
+      enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
+    description: PoDL PSE Class. Array of supported classes by the
+      single twisted-pair PoDL PSE.
+
+additionalProperties: false
+
+required:
+  - compatible
+  - '#pse-cells'
+  - ieee802.3-pse-supply
+
+examples:
+  - |
+    pse_t1l2: ethernet-pse-1 {
+      compatible = "ieee802.3-pse";
+      ieee802.3-pse-supply = <&reg_t1l1>;
+      ieee802.3-podl-pse-type = "e";
+      ieee802.3-podl-pse-class = /bits/ 8 <0 1>;
+      ieee802.3-pairs = /bits/ 8 <1>;
+      #pse-cells = <0>;
+    };
+  - |
+    pse_poe: ethernet-pse-2 {
+      compatible = "ieee802.3-pse";
+      ieee802.3-pse-supply = <&reg_poe>;
+      ieee802.3-pairs = /bits/ 8 <2 4>;
+      ieee802.3-pse-type = /bits/ 8 <1>;
+      ieee802.3-pse-class = /bits/ 8 <0 1>;
+      #pse-cells = <0>;
+    };
-- 
2.30.2

[PATCH net-next v2 2/7] dt-bindings: net: phy: add PoDL PSE property

[Oleksij Rempel]

Add property to reference node representing a PoDL Power Sourcing Equipment.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 Documentation/devicetree/bindings/net/ethernet-phy.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index ed1415a4381f2..0b7b9dc69d454 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -144,6 +144,11 @@ properties:
       Mark the corresponding energy efficient ethernet mode as
       broken and request the ethernet to stop advertising it.
 
+  ieee802.3-pse:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Specifies a reference to a node representing a Power Sourcing Equipment.
+
   phy-is-integrated:
     $ref: /schemas/types.yaml#/definitions/flag
     description:
-- 
2.30.2

[PATCH net-next v2 3/7] net: add framework to support Ethernet PSE and PDs devices

[Oleksij Rempel]

This framework was create with intention to provide support for Ethernet PSE
(Power Sourcing Equipment) and PDs (Powered Device).

At current step this patch implements generic PSE support for PoDL (Power over
Data Lines 802.3bu) specification with reserving name space for PD devices as
well.

This framework can be extended to support 802.3af and 802.3at "Power via the
Media Dependent Interface" (or PoE/Power over Ethernet)

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- remove ethtool interface support from this patch
---
 drivers/net/Kconfig           |   2 +
 drivers/net/Makefile          |   1 +
 drivers/net/pse-pd/Kconfig    |  11 ++
 drivers/net/pse-pd/Makefile   |   4 +
 drivers/net/pse-pd/pse_core.c | 294 ++++++++++++++++++++++++++++++++++
 include/linux/pse-pd/pse.h    |  93 +++++++++++
 6 files changed, 405 insertions(+)
 create mode 100644 drivers/net/pse-pd/Kconfig
 create mode 100644 drivers/net/pse-pd/Makefile
 create mode 100644 drivers/net/pse-pd/pse_core.c
 create mode 100644 include/linux/pse-pd/pse.h

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 94c889802566a..15d4a38b1351d 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -500,6 +500,8 @@ config NET_SB1000
 
 source "drivers/net/phy/Kconfig"
 
+source "drivers/net/pse-pd/Kconfig"
+
 source "drivers/net/can/Kconfig"
 
 source "drivers/net/mctp/Kconfig"
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 3f1192d3c52d3..6ce076462dbfd 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_NET) += loopback.o
 obj-$(CONFIG_NETDEV_LEGACY_INIT) += Space.o
 obj-$(CONFIG_NETCONSOLE) += netconsole.o
 obj-y += phy/
+obj-y += pse-pd/
 obj-y += mdio/
 obj-y += pcs/
 obj-$(CONFIG_RIONET) += rionet.o
diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
new file mode 100644
index 0000000000000..49c7f0bcff526
--- /dev/null
+++ b/drivers/net/pse-pd/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Ethernet Power Sourcing Equipment drivers
+#
+
+menuconfig PSE_CONTROLLER
+	bool "Ethernet Power Sourcing Equipment Support"
+	help
+	  Generic Power Sourcing Equipment Controller support.
+
+	  If unsure, say no.
diff --git a/drivers/net/pse-pd/Makefile b/drivers/net/pse-pd/Makefile
new file mode 100644
index 0000000000000..cfa780c7801dd
--- /dev/null
+++ b/drivers/net/pse-pd/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# Makefile for Linux PSE drivers
+
+obj-$(CONFIG_PSE_CONTROLLER) += pse_core.o
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
new file mode 100644
index 0000000000000..7bf3acc721c0a
--- /dev/null
+++ b/drivers/net/pse-pd/pse_core.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Framework for Ethernet Power Sourcing Equipment
+//
+// Copyright (c) 2022 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+//
+
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/pse-pd/pse.h>
+
+static DEFINE_MUTEX(pse_list_mutex);
+static LIST_HEAD(pse_controller_list);
+
+/**
+ * struct pse_control - a PSE control
+ * @pcdev: a pointer to the PSE controller device
+ *         this PSE control belongs to
+ * @list: list entry for the pcdev's PSE controller list
+ * @id: ID of the PSE line in the PSE controller device
+ * @refcnt: Number of gets of this pse_control
+ */
+struct pse_control {
+	struct pse_controller_dev *pcdev;
+	struct list_head list;
+	unsigned int id;
+	struct kref refcnt;
+};
+
+/**
+ * of_pse_zero_xlate - dummy function for controllers with one only control
+ * @pcdev: a pointer to the PSE controller device
+ * @pse_spec: PSE line specifier as found in the device tree
+ *
+ * This static translation function is used by default if of_xlate in
+ * :c:type:`pse_controller_dev` is not set. It is useful for all PSE
+ * controllers with #pse-cells = <0>.
+ */
+static int of_pse_zero_xlate(struct pse_controller_dev *pcdev,
+			     const struct of_phandle_args *pse_spec)
+{
+	return 0;
+}
+
+/**
+ * of_pse_simple_xlate - translate pse_spec to the PSE line number
+ * @pcdev: a pointer to the PSE controller device
+ * @pse_spec: PSE line specifier as found in the device tree
+ *
+ * This static translation function is used by default if of_xlate in
+ * :c:type:`pse_controller_dev` is not set. It is useful for all PSE
+ * controllers with 1:1 mapping, where PSE lines can be indexed by number
+ * without gaps.
+ */
+static int of_pse_simple_xlate(struct pse_controller_dev *pcdev,
+			       const struct of_phandle_args *pse_spec)
+{
+	if (pse_spec->args[0] >= pcdev->nr_lines)
+		return -EINVAL;
+
+	return pse_spec->args[0];
+}
+
+/**
+ * pse_controller_register - register a PSE controller device
+ * @pcdev: a pointer to the initialized PSE controller device
+ */
+int pse_controller_register(struct pse_controller_dev *pcdev)
+{
+	if (!pcdev->of_xlate) {
+		if (pcdev->of_pse_n_cells == 0)
+			pcdev->of_xlate = of_pse_zero_xlate;
+		else if (pcdev->of_pse_n_cells == 1)
+			pcdev->of_xlate = of_pse_simple_xlate;
+	}
+
+	mutex_init(&pcdev->lock);
+	INIT_LIST_HEAD(&pcdev->pse_control_head);
+
+	mutex_lock(&pse_list_mutex);
+	list_add(&pcdev->list, &pse_controller_list);
+	mutex_unlock(&pse_list_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pse_controller_register);
+
+/**
+ * pse_controller_unregister - unregister a PSE controller device
+ * @pcdev: a pointer to the PSE controller device
+ */
+void pse_controller_unregister(struct pse_controller_dev *pcdev)
+{
+	mutex_lock(&pse_list_mutex);
+	list_del(&pcdev->list);
+	mutex_unlock(&pse_list_mutex);
+}
+EXPORT_SYMBOL_GPL(pse_controller_unregister);
+
+static void devm_pse_controller_release(struct device *dev, void *res)
+{
+	pse_controller_unregister(*(struct pse_controller_dev **)res);
+}
+
+/**
+ * devm_pse_controller_register - resource managed pse_controller_register()
+ * @dev: device that is registering this PSE controller
+ * @pcdev: a pointer to the initialized PSE controller device
+ *
+ * Managed pse_controller_register(). For PSE controllers registered by
+ * this function, pse_controller_unregister() is automatically called on
+ * driver detach. See pse_controller_register() for more information.
+ */
+int devm_pse_controller_register(struct device *dev,
+				 struct pse_controller_dev *pcdev)
+{
+	struct pse_controller_dev **pcdevp;
+	int ret;
+
+	pcdevp = devres_alloc(devm_pse_controller_release, sizeof(*pcdevp),
+			      GFP_KERNEL);
+	if (!pcdevp)
+		return -ENOMEM;
+
+	ret = pse_controller_register(pcdev);
+	if (ret) {
+		devres_free(pcdevp);
+		return ret;
+	}
+
+	*pcdevp = pcdev;
+	devres_add(dev, pcdevp);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_pse_controller_register);
+
+/* PSE control section */
+
+static void __pse_control_release(struct kref *kref)
+{
+	struct pse_control *psec = container_of(kref, struct pse_control,
+						  refcnt);
+
+	lockdep_assert_held(&pse_list_mutex);
+
+	module_put(psec->pcdev->owner);
+
+	list_del(&psec->list);
+	kfree(psec);
+}
+
+static void __pse_control_put_internal(struct pse_control *psec)
+{
+	lockdep_assert_held(&pse_list_mutex);
+
+	kref_put(&psec->refcnt, __pse_control_release);
+}
+
+/**
+ * pse_control_put - free the PSE control
+ * @psec: PSE control pointer
+ */
+void pse_control_put(struct pse_control *psec)
+{
+	if (IS_ERR_OR_NULL(psec))
+		return;
+
+	mutex_lock(&pse_list_mutex);
+	__pse_control_put_internal(psec);
+	mutex_unlock(&pse_list_mutex);
+}
+EXPORT_SYMBOL_GPL(pse_control_put);
+
+static struct pse_control *
+pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index)
+{
+	struct pse_control *psec;
+
+	lockdep_assert_held(&pse_list_mutex);
+
+	list_for_each_entry(psec, &pcdev->pse_control_head, list) {
+		if (psec->id == index) {
+			kref_get(&psec->refcnt);
+			return psec;
+		}
+	}
+
+	psec = kzalloc(sizeof(*psec), GFP_KERNEL);
+	if (!psec)
+		return ERR_PTR(-ENOMEM);
+
+	if (!try_module_get(pcdev->owner)) {
+		kfree(psec);
+		return ERR_PTR(-ENODEV);
+	}
+
+	psec->pcdev = pcdev;
+	list_add(&psec->list, &pcdev->pse_control_head);
+	psec->id = index;
+	kref_init(&psec->refcnt);
+
+	return psec;
+}
+
+struct pse_control *
+of_pse_control_get(struct device_node *node)
+{
+	struct pse_controller_dev *r, *pcdev;
+	struct of_phandle_args args;
+	struct pse_control *psec;
+	int psec_id;
+	int ret;
+
+	if (!node)
+		return ERR_PTR(-EINVAL);
+
+	ret = of_parse_phandle_with_args(node, "ieee802.3-pse",
+					 "#pse-cells", 0, &args);
+	if (ret)
+		return ERR_PTR(ret);
+
+	mutex_lock(&pse_list_mutex);
+	pcdev = NULL;
+	list_for_each_entry(r, &pse_controller_list, list) {
+		if (args.np == r->dev->of_node) {
+			pcdev = r;
+			break;
+		}
+	}
+
+	if (!pcdev) {
+		psec = ERR_PTR(-EPROBE_DEFER);
+		goto out;
+	}
+
+	if (WARN_ON(args.args_count != pcdev->of_pse_n_cells)) {
+		psec = ERR_PTR(-EINVAL);
+		goto out;
+	}
+
+	psec_id = pcdev->of_xlate(pcdev, &args);
+	if (psec_id < 0) {
+		psec = ERR_PTR(psec_id);
+		goto out;
+	}
+
+	/* pse_list_mutex also protects the pcdev's pse_control list */
+	psec = pse_control_get_internal(pcdev, psec_id);
+
+out:
+	mutex_unlock(&pse_list_mutex);
+	of_node_put(args.np);
+
+	return psec;
+}
+EXPORT_SYMBOL_GPL(of_pse_control_get);
+
+struct pse_control *pse_control_get(struct device *dev)
+{
+	if (!dev->of_node)
+		return ERR_PTR(-ENODEV);
+
+	return of_pse_control_get(dev->of_node);
+}
+EXPORT_SYMBOL_GPL(pse_control_get);
+
+static void devm_pse_control_release(struct device *dev, void *res)
+{
+	pse_control_put(*(struct pse_control **)res);
+}
+
+struct pse_control *
+devm_pse_control_get(struct device *dev)
+{
+	struct pse_control **ptr, *psec;
+
+	ptr = devres_alloc(devm_pse_control_release, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	psec = pse_control_get(dev);
+	if (IS_ERR_OR_NULL(psec)) {
+		devres_free(ptr);
+		return psec;
+	}
+
+	*ptr = psec;
+	devres_add(dev, ptr);
+
+	return psec;
+}
+EXPORT_SYMBOL_GPL(devm_pse_control_get);
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
new file mode 100644
index 0000000000000..8bf84288db418
--- /dev/null
+++ b/include/linux/pse-pd/pse.h
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+// Copyright (c) 2022 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+ */
+#ifndef _LINUX_PSE_CONTROLLER_H
+#define _LINUX_PSE_CONTROLLER_H
+
+#include <linux/ethtool.h>
+#include <linux/list.h>
+#include <uapi/linux/ethtool.h>
+
+struct module;
+struct device_node;
+struct of_phandle_args;
+struct pse_control;
+
+/**
+ * struct pse_controller_dev - PSE controller entity that might
+ *                             provide multiple PSE controls
+ * @ops: a pointer to device specific struct pse_controller_ops
+ * @owner: kernel module of the PSE controller driver
+ * @list: internal list of PSE controller devices
+ * @pse_control_head: head of internal list of requested PSE controls
+ * @dev: corresponding driver model device struct
+ * @of_pse_n_cells: number of cells in PSE line specifiers
+ * @of_xlate: translation function to translate from specifier as found in the
+ *            device tree to id as given to the PSE control ops
+ * @nr_lines: number of PSE controls in this controller device
+ */
+struct pse_controller_dev {
+	const struct pse_controller_ops *ops;
+	struct module *owner;
+	struct list_head list;
+	struct list_head pse_control_head;
+	struct device *dev;
+	int of_pse_n_cells;
+	int (*of_xlate)(struct pse_controller_dev *pcdev,
+			const struct of_phandle_args *pse_spec);
+	unsigned int nr_lines;
+	struct mutex lock;
+};
+
+#if IS_ENABLED(CONFIG_PSE_CONTROLLER)
+int pse_controller_register(struct pse_controller_dev *pcdev);
+void pse_controller_unregister(struct pse_controller_dev *pcdev);
+struct device;
+int devm_pse_controller_register(struct device *dev,
+				 struct pse_controller_dev *pcdev);
+
+struct pse_control *pse_control_get(struct device *dev);
+struct pse_control *devm_pse_control_get( struct device *dev);
+struct pse_control *of_pse_control_get(struct device_node *node);
+void pse_control_put(struct pse_control *psec);
+
+#else
+
+static inline int pse_controller_register(struct pse_controller_dev *pcdev)
+{
+	return -ENOTSUPP;
+}
+
+static inline void pse_controller_unregister(struct pse_controller_dev *pcdev)
+{
+}
+
+static inline int devm_pse_controller_register(struct device *dev,
+						 struct pse_controller_dev *pcdev)
+{
+	return -ENOTSUPP;
+}
+
+static inline struct pse_control *pse_control_get(struct device *dev)
+{
+	return ERR_PTR(-ENOTSUPP);
+}
+
+static inline struct pse_control *devm_pse_control_get( struct device *dev)
+{
+	return ERR_PTR(-ENOTSUPP);
+}
+
+static inline struct pse_control *of_pse_control_get(struct device_node *node)
+{
+	return ERR_PTR(-ENOTSUPP);
+}
+
+static inline void pse_control_put(struct pse_control *psec)
+{
+}
+
+#endif
+
+#endif
-- 
2.30.2

[PATCH net-next v2 4/7] net: mdiobus: fwnode_mdiobus_register_phy() rework error handling

[Oleksij Rempel]

Rework error handling as preparation for PSE patch. This patch should
make it easier to extend this function.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/mdio/fwnode_mdio.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 3e79c2c519298..e78ad55c0e091 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -108,8 +108,8 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 	else
 		phy = phy_device_create(bus, addr, phy_id, 0, NULL);
 	if (IS_ERR(phy)) {
-		unregister_mii_timestamper(mii_ts);
-		return PTR_ERR(phy);
+		rc = PTR_ERR(phy);
+		goto clean_mii_ts;
 	}
 
 	if (is_acpi_node(child)) {
@@ -123,17 +123,13 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 		/* All data is now stored in the phy struct, so register it */
 		rc = phy_device_register(phy);
 		if (rc) {
-			phy_device_free(phy);
 			fwnode_handle_put(phy->mdio.dev.fwnode);
-			return rc;
+			goto clean_phy;
 		}
 	} else if (is_of_node(child)) {
 		rc = fwnode_mdiobus_phy_device_register(bus, phy, child, addr);
-		if (rc) {
-			unregister_mii_timestamper(mii_ts);
-			phy_device_free(phy);
-			return rc;
-		}
+		if (rc)
+			goto clean_phy;
 	}
 
 	/* phy->mii_ts may already be defined by the PHY driver. A
@@ -143,5 +139,12 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 	if (mii_ts)
 		phy->mii_ts = mii_ts;
 	return 0;
+
+clean_phy:
+	phy_device_free(phy);
+clean_mii_ts:
+	unregister_mii_timestamper(mii_ts);
+
+	return rc;
 }
 EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
-- 
2.30.2

[PATCH net-next v2 5/7] net: mdiobus: search for PSE nodes by parsing PHY nodes.

[Oleksij Rempel]

Some PHYs can be linked with PSE (Power Sourcing Equipment), so search
for related nodes and attach it to the phydev.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/mdio/fwnode_mdio.c | 37 ++++++++++++++++++++++++++++++++--
 drivers/net/phy/phy_device.c   |  2 ++
 include/linux/phy.h            |  2 ++
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index e78ad55c0e091..1e775c449f5db 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -10,10 +10,31 @@
 #include <linux/fwnode_mdio.h>
 #include <linux/of.h>
 #include <linux/phy.h>
+#include <linux/pse-pd/pse.h>
 
 MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
 MODULE_LICENSE("GPL");
 
+static struct pse_control *
+fwnode_find_pse_control(struct fwnode_handle *fwnode)
+{
+	struct pse_control *psec;
+	struct device_node *np;
+
+	if (is_acpi_node(fwnode))
+		return NULL;
+
+	np = to_of_node(fwnode);
+	if (!np)
+		return NULL;
+
+	psec = of_pse_control_get(np);
+	if (IS_ERR_OR_NULL(psec))
+		return NULL;
+
+	return psec;
+}
+
 static struct mii_timestamper *
 fwnode_find_mii_timestamper(struct fwnode_handle *fwnode)
 {
@@ -89,14 +110,21 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 				struct fwnode_handle *child, u32 addr)
 {
 	struct mii_timestamper *mii_ts = NULL;
+	struct pse_control *psec = NULL;
 	struct phy_device *phy;
 	bool is_c45 = false;
 	u32 phy_id;
 	int rc;
 
+	psec = fwnode_find_pse_control(child);
+	if (IS_ERR(psec))
+		return PTR_ERR(psec);
+
 	mii_ts = fwnode_find_mii_timestamper(child);
-	if (IS_ERR(mii_ts))
-		return PTR_ERR(mii_ts);
+	if (IS_ERR(mii_ts)) {
+		rc = PTR_ERR(mii_ts);
+		goto clean_pse;
+	}
 
 	rc = fwnode_property_match_string(child, "compatible",
 					  "ethernet-phy-ieee802.3-c45");
@@ -132,18 +160,23 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 			goto clean_phy;
 	}
 
+	phy->psec = psec;
+
 	/* phy->mii_ts may already be defined by the PHY driver. A
 	 * mii_timestamper probed via the device tree will still have
 	 * precedence.
 	 */
 	if (mii_ts)
 		phy->mii_ts = mii_ts;
+
 	return 0;
 
 clean_phy:
 	phy_device_free(phy);
 clean_mii_ts:
 	unregister_mii_timestamper(mii_ts);
+clean_pse:
+	pse_control_put(psec);
 
 	return rc;
 }
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0c6efd7926907..221bc872ee2fb 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -26,6 +26,7 @@
 #include <linux/netdevice.h>
 #include <linux/phy.h>
 #include <linux/phy_led_triggers.h>
+#include <linux/pse-pd/pse.h>
 #include <linux/property.h>
 #include <linux/sfp.h>
 #include <linux/skbuff.h>
@@ -988,6 +989,7 @@ EXPORT_SYMBOL(phy_device_register);
  */
 void phy_device_remove(struct phy_device *phydev)
 {
+	pse_control_put(phydev->psec);
 	unregister_mii_timestamper(phydev->mii_ts);
 
 	device_del(&phydev->mdio.dev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 87638c55d8442..0c91870b82582 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -588,6 +588,7 @@ struct macsec_ops;
  * @master_slave_get: Current master/slave advertisement
  * @master_slave_state: Current master/slave configuration
  * @mii_ts: Pointer to time stamper callbacks
+ * @psec: Pointer to Power Sourcing Equipment control struct
  * @lock:  Mutex for serialization access to PHY
  * @state_queue: Work queue for state machine
  * @shared: Pointer to private data shared by phys in one package
@@ -701,6 +702,7 @@ struct phy_device {
 	struct phylink *phylink;
 	struct net_device *attached_dev;
 	struct mii_timestamper *mii_ts;
+	struct pse_control *psec;
 
 	u8 mdix;
 	u8 mdix_ctrl;
-- 
2.30.2

[PATCH net-next v2 6/7] ethtool: add interface to interact with Ethernet Power Equipment

[Oleksij Rempel]

Add interface to support Power Sourcing Equipment. At current step it
provides generic way to address all variants of PSE devices as defined
in IEEE 802.3-2018 but support only objects specified for IEEE 802.3-2018 104.4
PoDL Power Sourcing Equipment (PSE).

Currently supported and mandatory objects are:
IEEE 802.3-2018 30.15.1.1.3 aPoDLPSEPowerDetectionStatus
IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState
IEEE 802.3-2018 30.15.1.2.1 acPoDLPSEAdminControl

This is minimal interface needed to control PSE on each separate
ethernet port but it provides not all mandatory objects specified in
IEEE 802.3-2018.

Since "PoDL PSE" and "PSE" have similar names, but some different values
I decide to not merge them and keep separate naming schema. This should
allow as to be as close to IEEE 802.3 spec as possible and avoid name
conflicts in the future.

This implementation is connected to PHYs instead of MACs because PSE
auto classification can potentially interfere with PHY auto negotiation.
So, may be some extra PHY related initialization will be needed.

With WIP version of ethtools interaction with PSE capable link looks
as following:

$ ip l
...
5: t1l1@eth0: <BROADCAST,MULTICAST> ..
...

$ ethtool --show-pse t1l1
PSE attributs for t1l1:
PoDL PSE Admin State: disabled
PoDL PSE Power Detection Status: disabled

$ ethtool --set-pse t1l1 podl-pse-admin-control enable
$ ethtool --show-pse t1l1
PSE attributs for t1l1:
PoDL PSE Admin State: enabled
PoDL PSE Power Detection Status: delivering power

Signed-off-by: kernel test robot <lkp@intel.com>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
- rework PSE ethtool API. Reduce it to get_status and set_config cbs
- remove phydev locking. Use PSE own locking instead.
---
 Documentation/networking/ethtool-netlink.rst |  60 ++++++
 drivers/net/pse-pd/pse_core.c                |  87 +++++++++
 include/linux/ethtool.h                      |  27 +++
 include/linux/pse-pd/pse.h                   |  41 ++++
 include/uapi/linux/ethtool.h                 |  50 +++++
 include/uapi/linux/ethtool_netlink.h         |  17 ++
 net/ethtool/Makefile                         |   3 +-
 net/ethtool/common.c                         |  10 +
 net/ethtool/common.h                         |   1 +
 net/ethtool/netlink.c                        |  19 ++
 net/ethtool/netlink.h                        |   4 +
 net/ethtool/pse-pd.c                         | 187 +++++++++++++++++++
 12 files changed, 505 insertions(+), 1 deletion(-)
 create mode 100644 net/ethtool/pse-pd.c

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index dbca3e9ec782f..c8b09b57bd65e 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -220,6 +220,8 @@ Userspace to kernel:
   ``ETHTOOL_MSG_PHC_VCLOCKS_GET``       get PHC virtual clocks info
   ``ETHTOOL_MSG_MODULE_SET``            set transceiver module parameters
   ``ETHTOOL_MSG_MODULE_GET``            get transceiver module parameters
+  ``ETHTOOL_MSG_PSE_SET``               set PSE parameters
+  ``ETHTOOL_MSG_PSE_GET``               get PSE parameters
   ===================================== =================================
 
 Kernel to userspace:
@@ -260,6 +262,7 @@ Kernel to userspace:
   ``ETHTOOL_MSG_STATS_GET_REPLY``          standard statistics
   ``ETHTOOL_MSG_PHC_VCLOCKS_GET_REPLY``    PHC virtual clocks info
   ``ETHTOOL_MSG_MODULE_GET_REPLY``         transceiver module parameters
+  ``ETHTOOL_MSG_PSE_GET_REPLY``            PSE parameters
   ======================================== =================================
 
 ``GET`` requests are sent by userspace applications to retrieve device
@@ -1625,6 +1628,63 @@ For SFF-8636 modules, low power mode is forced by the host according to table
 For CMIS modules, low power mode is forced by the host according to table 6-12
 in revision 5.0 of the specification.
 
+PSE_GET
+=======
+
+Gets PSE attributes.
+
+Request contents:
+
+  =====================================  ======  ==========================
+  ``ETHTOOL_A_PSE_HEADER``               nested  request header
+  =====================================  ======  ==========================
+
+Kernel response contents:
+
+  ======================================  ======  ==========================
+  ``ETHTOOL_A_PSE_HEADER``                nested  reply header
+  ``ETHTOOL_A_PODL_PSE_ADMIN_STATE``          u8  Operational state of the PoDL
+                                                  PSE functions
+  ``ETHTOOL_A_PODL_PSE_PW_D_STATUS``          u8  power detection status of the
+                                                  PoDL PSE.
+  ======================================  ======  ==========================
+
+The ``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` identifies the operational state of the
+PoDL PSE functions.  The operational state of the PSE function can be changed
+using the ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL`` action. This option is
+corresponding to IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState. Possible values
+are:
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+    :identifiers: ethtool_podl_pse_admin_state
+
+The ``ETHTOOL_A_PODL_PSE_PW_D_STATUS`` identifies the power detection status of the
+PoDL PSE.  The status depend on internal PSE state machine and automatic
+PD classification support. This option is corresponding to IEEE 802.3-2018
+30.15.1.1.3 aPoDLPSEPowerDetectionStatus. Possible values are:
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+    :identifiers: ethtool_podl_pse_admin_state
+
+PSE_SET
+=======
+
+Sets PSE parameters.
+
+Request contents:
+
+  ======================================  ======  ==========================
+  ``ETHTOOL_A_PSE_HEADER``                nested  request header
+  ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL``        u8  Control PoDL PSE Admin state
+  ======================================  ======  ==========================
+
+When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL`` attribute is used
+to control PoDL PSE Admin functions. This option is implementing
+IEEE 802.3-2018 30.15.1.2.1 acPoDLPSEAdminControl. Possible values are:
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+    :identifiers: ethtool_podl_pse_admin_state
+
 Request translation
 ===================
 
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 7bf3acc721c0a..d0f8fb6ac038b 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -292,3 +292,90 @@ devm_pse_control_get(struct device *dev)
 	return psec;
 }
 EXPORT_SYMBOL_GPL(devm_pse_control_get);
+
+/**
+ * pse_ethtool_get_status - get status of PSE control
+ * @psec: PSE control pointer
+ * @extack: extack for reporting useful error messages
+ */
+static 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;
+
+	if (!psec)
+		return 0;
+
+	if (WARN_ON(IS_ERR(psec)))
+		return -EINVAL;
+
+	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);
+	mutex_unlock(&psec->pcdev->lock);
+
+	return err;
+}
+
+/**
+ * pse_ethtool_set_config - set PSE control configuration
+ * @psec: PSE control pointer
+ * @extack: extack for reporting useful error messages
+ * @config: Configuration of the test to run
+ */
+static int pse_ethtool_set_config(struct pse_control *psec,
+				  struct netlink_ext_ack *extack,
+				  const struct pse_control_config *config)
+{
+	const struct pse_controller_ops *ops;
+	int err;
+
+	if (!psec)
+		return 0;
+
+	if (WARN_ON(IS_ERR(psec)))
+		return -EINVAL;
+
+	ops = psec->pcdev->ops;
+
+	if (!ops->ethtool_set_config) {
+		NL_SET_ERR_MSG(extack,
+			       "PSE driver does not configuration");
+		return -EOPNOTSUPP;
+	}
+
+	mutex_lock(&psec->pcdev->lock);
+	err = ops->ethtool_set_config(psec->pcdev, psec->id, extack, config);
+	mutex_unlock(&psec->pcdev->lock);
+
+	return err;
+}
+
+static const struct ethtool_pse_ops pse_ethtool_pse_ops = {
+	.get_status = pse_ethtool_get_status,
+	.set_config = pse_ethtool_set_config,
+};
+
+static int __init pse_init(void)
+{
+	ethtool_set_ethtool_pse_ops(&pse_ethtool_pse_ops);
+
+	return 0;
+}
+
+static void __exit pse_exit(void)
+{
+	ethtool_set_ethtool_pse_ops(NULL);
+}
+
+subsys_initcall(pse_init);
+module_exit(pse_exit);
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 99dc7bfbcd3c3..0ab95703d5517 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -815,6 +815,33 @@ struct ethtool_phy_ops {
  */
 void ethtool_set_ethtool_phy_ops(const struct ethtool_phy_ops *ops);
 
+struct pse_control;
+struct pse_control_config;
+struct pse_control_status;
+
+/**
+ * struct ethtool_pse_ops - Optional PSE device options
+ * @get_status: Returns status of PSE.
+ * @set_config: Set PSE configuration.
+ *
+ * All operations are optional (i.e. the function pointer may be set to %NULL)
+ * and callers must take this into account. Callers must hold the RTNL lock.
+ */
+struct ethtool_pse_ops {
+	int (*get_status)(struct pse_control *psec,
+			  struct netlink_ext_ack *extack,
+			  struct pse_control_status *status);
+	int (*set_config)(struct pse_control *psec,
+			  struct netlink_ext_ack *extack,
+			  const struct pse_control_config *config);
+};
+
+/**
+ * ethtool_set_ethtool_pse_ops - Set the ethtool_pse_ops singleton
+ * @ops: Ethtool PHY operations to set
+ */
+void ethtool_set_ethtool_pse_ops(const struct ethtool_pse_ops *ops);
+
 /**
  * ethtool_params_from_link_mode - Derive link parameters from a given link mode
  * @link_ksettings: Link parameters to be derived from the link mode
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 8bf84288db418..d6daa525fbfd3 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -9,6 +9,47 @@
 #include <linux/list.h>
 #include <uapi/linux/ethtool.h>
 
+struct phy_device;
+struct pse_controller_dev;
+
+/**
+ * struct pse_control_config - PSE control/channel configuration.
+ *
+ * @admin_cotrol: set PoDL PSE admin control as described in
+ *	IEEE 802.3-2018 30.15.1.2.1 acPoDLPSEAdminControl
+ */
+struct pse_control_config {
+	enum ethtool_podl_pse_admin_state admin_cotrol;
+};
+
+/**
+ * struct pse_control_status - PSE control/channel status.
+ *
+ * @admin_state: operational state of the PoDL PSE
+ *	functions. IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState
+ * @pw_status: power detection status of the PoDL PSE.
+ *	IEEE 802.3-2018 30.15.1.1.3 aPoDLPSEPowerDetectionStatus:
+ */
+struct pse_control_status {
+	enum ethtool_podl_pse_admin_state podl_admin_state;
+	enum ethtool_podl_pse_pw_d_status podl_pw_status;
+};
+
+/**
+ * struct pse_controller_ops - PSE controller driver callbacks
+ *
+ * @get_status: get PSE control status for ethtool interface
+ * @set_config: set PSE control configuration over ethtool interface
+ */
+struct pse_controller_ops {
+	int (*ethtool_get_status)(struct pse_controller_dev *pcdev,
+		unsigned long id, struct netlink_ext_ack *extack,
+		struct pse_control_status *status);
+	int (*ethtool_set_config)(struct pse_controller_dev *pcdev,
+		unsigned long id, struct netlink_ext_ack *extack,
+		const struct pse_control_config *config);
+};
+
 struct module;
 struct device_node;
 struct of_phandle_args;
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 2d5741fd44bbc..783f19f78c633 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -736,6 +736,56 @@ enum ethtool_module_power_mode {
 	ETHTOOL_MODULE_POWER_MODE_HIGH,
 };
 
+/**
+ * enum ethtool_podl_pse_admin_state - operational state of the PoDL PSE
+ *	functions. IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState
+ * @ETHTOOL_PSE_MODE_POLICY_UNKNOWN: state of PoDL PSE functions are unknown
+ * @ETHTOOL_PSE_MODE_POLICY_HIGH: PoDL PSE functions are disabled
+ * @ETHTOOL_PSE_MODE_POLICY_AUTO: PoDL PSE functions are enabled
+ */
+enum ethtool_podl_pse_admin_state {
+	ETHTOOL_PODL_PSE_ADMIN_STATE_UNKNOWN = 1,
+	ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED,
+	ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED,
+
+	/* add new constants above here */
+	ETHTOOL_PODL_PSE_ADMIN_STATE_COUNT
+};
+
+/**
+ * enum ethtool_podl_pse_pw_d_status - power detection status of the PoDL PSE.
+ *	IEEE 802.3-2018 30.15.1.1.3 aPoDLPSEPowerDetectionStatus:
+ * @ETHTOOL_PODL_PSE_PW_D_STATUS_UNKNOWN: PoDL PSE
+ * @ETHTOOL_PODL_PSE_PW_D_STATUS_DISABLED: "The enumeration “disabled” is
+ *	asserted true when the PoDL PSE state diagram variable mr_pse_enable is
+ *	false"
+ * @ETHTOOL_PODL_PSE_PW_D_STATUS_SEARCHING: "The enumeration “searching” is
+ *	asserted true when either of the PSE state diagram variables
+ *	pi_detecting or pi_classifying is true."
+ * @ETHTOOL_PODL_PSE_PW_D_STATUS_DELIVERING: "The enumeration “deliveringPower”
+ *	is asserted true when the PoDL PSE state diagram variable pi_powered is
+ *	true."
+ * @ETHTOOL_PODL_PSE_PW_D_STATUS_SLEEP: "The enumeration “sleep” is asserted
+ *	true when the PoDL PSE state diagram variable pi_sleeping is true."
+ * @ETHTOOL_PODL_PSE_PW_D_STATUS_IDLE: "The enumeration “idle” is asserted true
+ *	when the logical combination of the PoDL PSE state diagram variables
+ *	pi_prebiased*!pi_sleeping is true."
+ * @ETHTOOL_PODL_PSE_PW_D_STATUS_ERROR: "The enumeration “error” is asserted
+ *	true when the PoDL PSE state diagram variable overload_held is true."
+ */
+enum ethtool_podl_pse_pw_d_status {
+	ETHTOOL_PODL_PSE_PW_D_STATUS_UNKNOWN = 1,
+	ETHTOOL_PODL_PSE_PW_D_STATUS_DISABLED,
+	ETHTOOL_PODL_PSE_PW_D_STATUS_SEARCHING,
+	ETHTOOL_PODL_PSE_PW_D_STATUS_DELIVERING,
+	ETHTOOL_PODL_PSE_PW_D_STATUS_SLEEP,
+	ETHTOOL_PODL_PSE_PW_D_STATUS_IDLE,
+	ETHTOOL_PODL_PSE_PW_D_STATUS_ERROR,
+
+	/* add new constants above here */
+	ETHTOOL_PODL_PSE_PW_D_STATUS_COUNT
+};
+
 /**
  * struct ethtool_gstrings - string set for data tagging
  * @cmd: Command number = %ETHTOOL_GSTRINGS
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index d2fb4f7be61b2..1c890a37a35b5 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -49,6 +49,8 @@ enum {
 	ETHTOOL_MSG_PHC_VCLOCKS_GET,
 	ETHTOOL_MSG_MODULE_GET,
 	ETHTOOL_MSG_MODULE_SET,
+	ETHTOOL_MSG_PSE_GET,
+	ETHTOOL_MSG_PSE_SET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -94,6 +96,8 @@ enum {
 	ETHTOOL_MSG_PHC_VCLOCKS_GET_REPLY,
 	ETHTOOL_MSG_MODULE_GET_REPLY,
 	ETHTOOL_MSG_MODULE_NTF,
+	ETHTOOL_MSG_PSE_GET_REPLY,
+	ETHTOOL_MSG_PSE_NTF,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
@@ -862,6 +866,19 @@ enum {
 	ETHTOOL_A_MODULE_MAX = (__ETHTOOL_A_MODULE_CNT - 1)
 };
 
+/* Power Sourcing Equipment */
+enum {
+	ETHTOOL_A_PSE_UNSPEC,
+	ETHTOOL_A_PSE_HEADER,			/* nest - _A_HEADER_* */
+	ETHTOOL_A_PODL_PSE_ADMIN_STATE,		/* u8 */
+	ETHTOOL_A_PODL_PSE_ADMIN_CONTROL,	/* u8 */
+	ETHTOOL_A_PODL_PSE_PW_D_STATUS,		/* u8 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_PSE_CNT,
+	ETHTOOL_A_PSE_MAX = (__ETHTOOL_A_PSE_CNT - 1)
+};
+
 /* generic netlink info */
 #define ETHTOOL_GENL_NAME "ethtool"
 #define ETHTOOL_GENL_VERSION 1
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index b76432e70e6ba..72ab0944262af 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -7,4 +7,5 @@ obj-$(CONFIG_ETHTOOL_NETLINK)	+= ethtool_nl.o
 ethtool_nl-y	:= netlink.o bitset.o strset.o linkinfo.o linkmodes.o \
 		   linkstate.o debug.o wol.o features.o privflags.o rings.o \
 		   channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \
-		   tunnels.o fec.o eeprom.o stats.o phc_vclocks.o module.o
+		   tunnels.o fec.o eeprom.o stats.o phc_vclocks.o module.o \
+		   pse-pd.o
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 566adf85e658d..c41a8599dc514 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -582,6 +582,16 @@ void ethtool_set_ethtool_phy_ops(const struct ethtool_phy_ops *ops)
 }
 EXPORT_SYMBOL_GPL(ethtool_set_ethtool_phy_ops);
 
+const struct ethtool_pse_ops *ethtool_pse_ops;
+
+void ethtool_set_ethtool_pse_ops(const struct ethtool_pse_ops *ops)
+{
+	rtnl_lock();
+	ethtool_pse_ops = ops;
+	rtnl_unlock();
+}
+EXPORT_SYMBOL_GPL(ethtool_set_ethtool_pse_ops);
+
 void
 ethtool_params_from_link_mode(struct ethtool_link_ksettings *link_ksettings,
 			      enum ethtool_link_mode_bit_indices link_mode)
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index 2dc2b80aea5f5..c1779657e074f 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -46,6 +46,7 @@ int ethtool_get_max_rxfh_channel(struct net_device *dev, u32 *max);
 int __ethtool_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info);
 
 extern const struct ethtool_phy_ops *ethtool_phy_ops;
+extern const struct ethtool_pse_ops *ethtool_pse_ops;
 
 int ethtool_get_module_info_call(struct net_device *dev,
 				 struct ethtool_modinfo *modinfo);
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index e26079e11835c..ec84a12ba4918 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -286,6 +286,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_STATS_GET]		= &ethnl_stats_request_ops,
 	[ETHTOOL_MSG_PHC_VCLOCKS_GET]	= &ethnl_phc_vclocks_request_ops,
 	[ETHTOOL_MSG_MODULE_GET]	= &ethnl_module_request_ops,
+	[ETHTOOL_MSG_PSE_GET]		= &ethnl_pse_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -598,6 +599,7 @@ ethnl_default_notify_ops[ETHTOOL_MSG_KERNEL_MAX + 1] = {
 	[ETHTOOL_MSG_EEE_NTF]		= &ethnl_eee_request_ops,
 	[ETHTOOL_MSG_FEC_NTF]		= &ethnl_fec_request_ops,
 	[ETHTOOL_MSG_MODULE_NTF]	= &ethnl_module_request_ops,
+	[ETHTOOL_MSG_PSE_NTF]		= &ethnl_pse_request_ops,
 };
 
 /* default notification handler */
@@ -691,6 +693,7 @@ static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
 	[ETHTOOL_MSG_EEE_NTF]		= ethnl_default_notify,
 	[ETHTOOL_MSG_FEC_NTF]		= ethnl_default_notify,
 	[ETHTOOL_MSG_MODULE_NTF]	= ethnl_default_notify,
+	[ETHTOOL_MSG_PSE_NTF]		= ethnl_default_notify,
 };
 
 void ethtool_notify(struct net_device *dev, unsigned int cmd, const void *data)
@@ -1020,6 +1023,22 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.policy = ethnl_module_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_module_set_policy) - 1,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_PSE_GET,
+		.doit	= ethnl_default_doit,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
+		.policy = ethnl_pse_get_policy,
+		.maxattr = ARRAY_SIZE(ethnl_pse_get_policy) - 1,
+	},
+	{
+		.cmd	= ETHTOOL_MSG_PSE_SET,
+		.flags	= GENL_UNS_ADMIN_PERM,
+		.doit	= ethnl_set_pse,
+		.policy = ethnl_pse_set_policy,
+		.maxattr = ARRAY_SIZE(ethnl_pse_set_policy) - 1,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index c0d5876118546..1bfd374f97188 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -345,6 +345,7 @@ extern const struct ethnl_request_ops ethnl_module_eeprom_request_ops;
 extern const struct ethnl_request_ops ethnl_stats_request_ops;
 extern const struct ethnl_request_ops ethnl_phc_vclocks_request_ops;
 extern const struct ethnl_request_ops ethnl_module_request_ops;
+extern const struct ethnl_request_ops ethnl_pse_request_ops;
 
 extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1];
 extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1];
@@ -383,6 +384,8 @@ extern const struct nla_policy ethnl_stats_get_policy[ETHTOOL_A_STATS_GROUPS + 1
 extern const struct nla_policy ethnl_phc_vclocks_get_policy[ETHTOOL_A_PHC_VCLOCKS_HEADER + 1];
 extern const struct nla_policy ethnl_module_get_policy[ETHTOOL_A_MODULE_HEADER + 1];
 extern const struct nla_policy ethnl_module_set_policy[ETHTOOL_A_MODULE_POWER_MODE_POLICY + 1];
+extern const struct nla_policy ethnl_pse_get_policy[ETHTOOL_A_PSE_HEADER + 1];
+extern const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1];
 
 int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info);
@@ -402,6 +405,7 @@ int ethnl_tunnel_info_start(struct netlink_callback *cb);
 int ethnl_tunnel_info_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
 int ethnl_set_fec(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_module(struct sk_buff *skb, struct genl_info *info);
+int ethnl_set_pse(struct sk_buff *skb, struct genl_info *info);
 
 extern const char stats_std_names[__ETHTOOL_STATS_CNT][ETH_GSTRING_LEN];
 extern const char stats_eth_phy_names[__ETHTOOL_A_STATS_ETH_PHY_CNT][ETH_GSTRING_LEN];
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
new file mode 100644
index 0000000000000..f11dadcde2cc4
--- /dev/null
+++ b/net/ethtool/pse-pd.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// ethtool interface for for Ethernet PSE (Power Sourcing Equipment)
+// and PD (Powered Device)
+//
+// Copyright (c) 2022 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+//
+
+#include "common.h"
+#include "linux/pse-pd/pse.h"
+#include "netlink.h"
+#include <linux/ethtool_netlink.h>
+#include <linux/ethtool.h>
+#include <linux/phy.h>
+
+struct pse_req_info {
+	struct ethnl_req_info base;
+};
+
+struct pse_reply_data {
+	struct ethnl_reply_data	base;
+	struct pse_control_status status;
+};
+
+#define PSE_REPDATA(__reply_base) \
+	container_of(__reply_base, struct pse_reply_data, base)
+
+/* PSE_GET */
+
+const struct nla_policy ethnl_pse_get_policy[ETHTOOL_A_PSE_HEADER + 1] = {
+	[ETHTOOL_A_PSE_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+};
+
+static int pse_get_pse_attributs(struct net_device *dev,
+				 struct netlink_ext_ack *extack,
+				 struct pse_reply_data *data)
+{
+	struct phy_device *phydev = dev->phydev;
+	const struct ethtool_pse_ops *ops;
+
+	if (!phydev)
+		return -EOPNOTSUPP;
+
+	if (!phydev->psec)
+		return -EOPNOTSUPP;
+
+	ops = ethtool_pse_ops;
+	if (!ops || !ops->get_status)
+		return -EOPNOTSUPP;
+
+	memset(&data->status, 0, sizeof(data->status));
+
+	return ops->get_status(phydev->psec, extack, &data->status);
+}
+
+static int pse_prepare_data(const struct ethnl_req_info *req_base,
+			       struct ethnl_reply_data *reply_base,
+			       struct genl_info *info)
+{
+	struct pse_reply_data *data = PSE_REPDATA(reply_base);
+	struct net_device *dev = reply_base->dev;
+	int ret;
+
+	ret = ethnl_ops_begin(dev);
+	if (ret < 0)
+		return 0;
+
+	ret = pse_get_pse_attributs(dev, info->extack, data);
+
+	ethnl_ops_complete(dev);
+
+	return ret;
+}
+
+static int pse_reply_size(const struct ethnl_req_info *req_base,
+			  const struct ethnl_reply_data *reply_base)
+{
+	const struct pse_reply_data *data = PSE_REPDATA(reply_base);
+	const struct pse_control_status *st = &data->status;
+	int len = 0;
+
+	if (st->podl_admin_state >= 0)
+		len += nla_total_size(sizeof(u8)); /* _PODL_PSE_ADMIN_STATE */
+	if (st->podl_pw_status >= 0)
+		len += nla_total_size(sizeof(u8)); /* _PODL_PSE_PW_D_STATUS */
+
+	return len;
+}
+
+static int pse_fill_reply(struct sk_buff *skb,
+			  const struct ethnl_req_info *req_base,
+			  const struct ethnl_reply_data *reply_base)
+{
+	const struct pse_reply_data *data = PSE_REPDATA(reply_base);
+	const struct pse_control_status *st = &data->status;
+
+	if (st->podl_admin_state > 0 &&
+	    nla_put_u8(skb, ETHTOOL_A_PODL_PSE_ADMIN_STATE,
+		       st->podl_admin_state))
+		return -EMSGSIZE;
+
+	if (st->podl_pw_status > 0 &&
+	    nla_put_u8(skb, ETHTOOL_A_PODL_PSE_PW_D_STATUS,
+		       st->podl_pw_status))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+const struct ethnl_request_ops ethnl_pse_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_PSE_GET,
+	.reply_cmd		= ETHTOOL_MSG_PSE_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_PSE_HEADER,
+	.req_info_size		= sizeof(struct pse_req_info),
+	.reply_data_size	= sizeof(struct pse_reply_data),
+
+	.prepare_data		= pse_prepare_data,
+	.reply_size		= pse_reply_size,
+	.fill_reply		= pse_fill_reply,
+};
+
+/* PSE_SET */
+
+const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1] = {
+	[ETHTOOL_A_PSE_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+	[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] =
+		NLA_POLICY_RANGE(NLA_U8, ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED,
+				 ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED),
+};
+
+static int pse_set_pse_config(struct net_device *dev,
+			      struct netlink_ext_ack *extack,
+			      struct nlattr **tb)
+{
+	struct phy_device *phydev = dev->phydev;
+	struct pse_control_config config = {};
+	const struct ethtool_pse_ops *ops;
+	int ret;
+
+	if (!tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL])
+		return 0;
+
+	ops = ethtool_pse_ops;
+	if (!ops || !ops->set_config)
+		return -EOPNOTSUPP;
+
+	config.admin_cotrol = nla_get_u8(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
+
+	if (!phydev)
+		return -EOPNOTSUPP;
+
+	// todo resolve phydev dependecy
+	if (!phydev->psec)
+		ret = -EOPNOTSUPP;
+	else
+		ret = ops->set_config(phydev->psec, extack, &config);
+
+	return ret;
+}
+
+int ethnl_set_pse(struct sk_buff *skb, struct genl_info *info)
+{
+	struct ethnl_req_info req_info = {};
+	struct nlattr **tb = info->attrs;
+	struct net_device *dev;
+	int ret;
+
+	ret = ethnl_parse_header_dev_get(&req_info, tb[ETHTOOL_A_PSE_HEADER],
+					 genl_info_net(info), info->extack,
+					 true);
+	if (ret < 0)
+		return ret;
+	dev = req_info.dev;
+
+	rtnl_lock();
+	ret = ethnl_ops_begin(dev);
+	if (ret < 0)
+		goto out_rtnl;
+
+	ret = pse_set_pse_config(dev, info->extack, tb);
+
+	ethnl_ops_complete(dev);
+out_rtnl:
+	rtnl_unlock();
+	ethnl_parse_header_dev_put(&req_info);
+	return ret;
+}
-- 
2.30.2

[PATCH net-next v2 7/7] net: pse-pd: add generic PSE driver

[Oleksij Rempel]

Add generic driver to support simple Power Sourcing Equipment without
automatic classification support.

This driver was tested on 10Bast-T1L switch with regulator based PoDL PSE.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- add regulator_enable test to the probe
- migrate to the new PSE ethtool API
---
 drivers/net/pse-pd/Kconfig       |  11 +++
 drivers/net/pse-pd/Makefile      |   2 +
 drivers/net/pse-pd/pse_generic.c | 148 +++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+)
 create mode 100644 drivers/net/pse-pd/pse_generic.c

diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
index 49c7f0bcff526..a804c9f1af2bc 100644
--- a/drivers/net/pse-pd/Kconfig
+++ b/drivers/net/pse-pd/Kconfig
@@ -9,3 +9,14 @@ menuconfig PSE_CONTROLLER
 	  Generic Power Sourcing Equipment Controller support.
 
 	  If unsure, say no.
+
+if PSE_CONTROLLER
+
+config PSE_GENERIC
+	tristate "Generic PSE driver"
+	help
+	  This module provides support for simple Ethernet Power Sourcing
+	  Equipment without automatic classification support. For example for
+	  PoDL (802.3bu) specification.
+
+endif
diff --git a/drivers/net/pse-pd/Makefile b/drivers/net/pse-pd/Makefile
index cfa780c7801dd..e3f2f434a9e5c 100644
--- a/drivers/net/pse-pd/Makefile
+++ b/drivers/net/pse-pd/Makefile
@@ -2,3 +2,5 @@
 # Makefile for Linux PSE drivers
 
 obj-$(CONFIG_PSE_CONTROLLER) += pse_core.o
+
+obj-$(CONFIG_PSE_GENERIC) += pse_generic.o
diff --git a/drivers/net/pse-pd/pse_generic.c b/drivers/net/pse-pd/pse_generic.c
new file mode 100644
index 0000000000000..2be1a8bf45b48
--- /dev/null
+++ b/drivers/net/pse-pd/pse_generic.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Driver for the Generic Ethernet Power Sourcing Equipment, without
+// auto classification support.
+//
+// Copyright (c) 2022 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+//
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pse-pd/pse.h>
+#include <linux/regulator/consumer.h>
+
+struct gen_pse_priv {
+	struct pse_controller_dev pcdev;
+	struct regulator *ps; /*power source */
+	enum ethtool_podl_pse_admin_state admin_state;
+};
+
+static struct gen_pse_priv *to_gen_pse(struct pse_controller_dev *pcdev)
+{
+	return container_of(pcdev, struct gen_pse_priv, pcdev);
+}
+
+static int
+gen_pse_ethtool_set_config(struct pse_controller_dev *pcdev, unsigned long id,
+			   struct netlink_ext_ack *extack,
+			   const struct pse_control_config *config)
+{
+	struct gen_pse_priv *priv = to_gen_pse(pcdev);
+	int ret;
+
+	if (priv->admin_state == config->admin_cotrol)
+		return 0;
+
+	switch (config->admin_cotrol) {
+	case ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED:
+		ret = regulator_enable(priv->ps);
+		break;
+	case ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED:
+		ret = regulator_disable(priv->ps);
+		break;
+	default:
+		dev_err(pcdev->dev, "Unknown admin state %i\n",
+			config->admin_cotrol);
+		ret = -ENOTSUPP;
+	}
+
+	if (ret)
+		return ret;
+
+	priv->admin_state = config->admin_cotrol;
+
+	return 0;
+}
+
+static int
+gen_pse_ethtool_get_status(struct pse_controller_dev *pcdev, unsigned long id,
+			   struct netlink_ext_ack *extack,
+			   struct pse_control_status *status)
+{
+	struct gen_pse_priv *priv = to_gen_pse(pcdev);
+	int ret;
+
+	ret = regulator_is_enabled(priv->ps);
+	if (ret < 0)
+		return ret;
+
+	if (!ret)
+		status->podl_pw_status = ETHTOOL_PODL_PSE_PW_D_STATUS_DISABLED;
+	else
+		status->podl_pw_status =
+			ETHTOOL_PODL_PSE_PW_D_STATUS_DELIVERING;
+
+	status->podl_admin_state = priv->admin_state;
+
+	return 0;
+}
+
+static const struct pse_controller_ops gen_pse_ops = {
+	.ethtool_get_status = gen_pse_ethtool_get_status,
+	.ethtool_set_config = gen_pse_ethtool_set_config,
+};
+
+static int
+gen_pse_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct gen_pse_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	if (!pdev->dev.of_node)
+		return -ENOENT;
+
+	priv->ps = devm_regulator_get_exclusive(dev, "ieee802.3-pse");
+	if (IS_ERR(priv->ps)) {
+		dev_err(dev, "failed to get PSE regulator (%pe)\n", priv->ps);
+		return PTR_ERR(priv->ps);
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	ret = regulator_is_enabled(priv->ps);
+	if (ret < 0)
+		return ret;
+
+	if (ret)
+		priv->admin_state = ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED;
+	else
+		priv->admin_state = ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED;
+
+	priv->pcdev.owner = THIS_MODULE;
+	priv->pcdev.ops = &gen_pse_ops;
+	priv->pcdev.dev = dev;
+	ret = devm_pse_controller_register(dev, &priv->pcdev);
+	if (ret) {
+		dev_err(dev, "failed to register PSE controller (%pe)\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id gen_pse_of_match[] = {
+	{ .compatible = "ieee802.3-pse", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, gen_pse_of_match);
+
+static struct platform_driver gen_pse_driver = {
+	.probe		= gen_pse_probe,
+	.driver		= {
+		.name		= "generic PSE",
+		.of_match_table = of_match_ptr(gen_pse_of_match),
+	},
+};
+module_platform_driver(gen_pse_driver);
+
+MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
+MODULE_DESCRIPTION("Generic Ethernet Power Sourcing Equipment");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:pse-generic");
-- 
2.30.2

Re: [PATCH net-next v2 5/7] net: mdiobus: search for PSE nodes by parsing PHY nodes.

[Russell King (Oracle)]

On Thu, Aug 25, 2022 at 03:02:09PM +0200, Oleksij Rempel wrote:
> +static struct pse_control *
> +fwnode_find_pse_control(struct fwnode_handle *fwnode)
> +{
> +	struct pse_control *psec;
> +	struct device_node *np;
> +
> +	if (is_acpi_node(fwnode))
> +		return NULL;
> +
> +	np = to_of_node(fwnode);
> +	if (!np)
> +		return NULL;

Doesn't to_of_node() confirm that the fwnode is a DT node? In other
words, isn't the "is_acpi_node()" entirely redundant?

> +
> +	psec = of_pse_control_get(np);
> +	if (IS_ERR_OR_NULL(psec))
> +		return NULL;
> +
> +	return psec;
> +}

So fwnode_find_pse_control() returns NULL on error.

> +	psec = fwnode_find_pse_control(child);
> +	if (IS_ERR(psec))
> +		return PTR_ERR(psec);

This usage expects it to return an error-pointer.

Clearly, there is some disagreement about what fwnode_find_pse_control()
returns on error.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 6/7] ethtool: add interface to interact with Ethernet Power Equipment

[Jakub Kicinski]

On Thu, 25 Aug 2022 15:02:10 +0200 Oleksij Rempel wrote:
> +void ethtool_set_ethtool_pse_ops(const struct ethtool_pse_ops *ops)
> +{
> +	rtnl_lock();
> +	ethtool_pse_ops = ops;
> +	rtnl_unlock();
> +}
> +EXPORT_SYMBOL_GPL(ethtool_set_ethtool_pse_ops);

Do we really need the loose linking on the PSE ops?
It's not a lot of code, and the pcdev->ops should be 
enough to decouple drivers, it seems.

> +static int pse_set_pse_config(struct net_device *dev,
> +			      struct netlink_ext_ack *extack,
> +			      struct nlattr **tb)
> +{
> +	struct phy_device *phydev = dev->phydev;
> +	struct pse_control_config config = {};
> +	const struct ethtool_pse_ops *ops;
> +	int ret;
> +
> +	if (!tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL])
> +		return 0;

If SET has no useful attrs the usual response is -EINVAL.

> +	ops = ethtool_pse_ops;
> +	if (!ops || !ops->set_config)
> +		return -EOPNOTSUPP;
> +
> +	config.admin_cotrol = nla_get_u8(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
> +
> +	if (!phydev)
> +		return -EOPNOTSUPP;
> +
> +	// todo resolve phydev dependecy

My lack of phydev understanding and laziness are likely the cause,
but I haven't found an explanation for this todo. What is it about?

> +	if (!phydev->psec)
> +		ret = -EOPNOTSUPP;
> +	else
> +		ret = ops->set_config(phydev->psec, extack, &config);
> +
> +	return ret;
> +}

Re: [PATCH net-next v2 6/7] ethtool: add interface to interact with Ethernet Power Equipment

[Jakub Kicinski]

On Thu, 25 Aug 2022 15:02:10 +0200 Oleksij Rempel wrote:
> +enum ethtool_podl_pse_admin_state {
> +	ETHTOOL_PODL_PSE_ADMIN_STATE_UNKNOWN = 1,

Why define UNKNOWN.. as 1? No real objection here, just in my head
somehow UNKNOWN = 0 or just start from 1.

> +	ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED,
> +	ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED,
> +
> +	/* add new constants above here */
> +	ETHTOOL_PODL_PSE_ADMIN_STATE_COUNT

Why define count for a value enum like this? For attrs we define it
because it's used to size tables, don't think anyone will size tables
based on states.

There's a bunch of kdoc warnings in the patches as well.

Re: [PATCH net-next v2 6/7] ethtool: add interface to interact with Ethernet Power Equipment

[Oleksij Rempel]

On Thu, Aug 25, 2022 at 11:10:19AM -0700, Jakub Kicinski wrote:
> On Thu, 25 Aug 2022 15:02:10 +0200 Oleksij Rempel wrote:
> > +enum ethtool_podl_pse_admin_state {
> > +	ETHTOOL_PODL_PSE_ADMIN_STATE_UNKNOWN = 1,
> 
> Why define UNKNOWN.. as 1? No real objection here, just in my head
> somehow UNKNOWN = 0 or just start from 1.

I need to keep difference between not supported functionality and
supported but unknown.

> > +	ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED,
> > +	ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED,
> > +
> > +	/* add new constants above here */
> > +	ETHTOOL_PODL_PSE_ADMIN_STATE_COUNT
> 
> Why define count for a value enum like this? For attrs we define it
> because it's used to size tables, don't think anyone will size tables
> based on states.

ok, i'll remove it.

> There's a bunch of kdoc warnings in the patches as well.

ok.

-- 
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 6/7] ethtool: add interface to interact with Ethernet Power Equipment

[Oleksij Rempel]

On Thu, Aug 25, 2022 at 11:07:56AM -0700, Jakub Kicinski wrote:
> On Thu, 25 Aug 2022 15:02:10 +0200 Oleksij Rempel wrote:
> > +void ethtool_set_ethtool_pse_ops(const struct ethtool_pse_ops *ops)
> > +{
> > +	rtnl_lock();
> > +	ethtool_pse_ops = ops;
> > +	rtnl_unlock();
> > +}
> > +EXPORT_SYMBOL_GPL(ethtool_set_ethtool_pse_ops);
> 
> Do we really need the loose linking on the PSE ops?
> It's not a lot of code, and the pcdev->ops should be 
> enough to decouple drivers, it seems.

Right now i have no good idea how to properly decouple pse-pd from phydev.

@Andrew, should i care about it on this stage or it is currently not a
big deal?

> > +static int pse_set_pse_config(struct net_device *dev,
> > +			      struct netlink_ext_ack *extack,
> > +			      struct nlattr **tb)
> > +{
> > +	struct phy_device *phydev = dev->phydev;
> > +	struct pse_control_config config = {};
> > +	const struct ethtool_pse_ops *ops;
> > +	int ret;
> > +
> > +	if (!tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL])
> > +		return 0;
> 
> If SET has no useful attrs the usual response is -EINVAL.

ack

> > +	ops = ethtool_pse_ops;
> > +	if (!ops || !ops->set_config)
> > +		return -EOPNOTSUPP;
> > +
> > +	config.admin_cotrol = nla_get_u8(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
> > +
> > +	if (!phydev)
> > +		return -EOPNOTSUPP;
> > +
> > +	// todo resolve phydev dependecy
> 
> My lack of phydev understanding and laziness are likely the cause,
> but I haven't found an explanation for this todo. What is it about?

sorry. old artifact, will be removed. It is part of phydev/phylink
related discussion in the last patch version.

-- 
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 1/7] dt-bindings: net: pse-dt: add bindings for generic PSE controller

[Andrew Lunn]

> +  ieee802.3-pairs:
> +    $ref: /schemas/types.yaml#/definitions/int8-array
> +    description: Array of number of twisted-pairs capable to deliver power.
> +      Since not all circuits are able to support all pair variants, the array of
> +      supported variants should be specified.
> +      Note - single twisted-pair PSE is formally know as PoDL PSE.
> +    items:
> +      enum: [1, 2, 4]

It is not clear to me what you are describing here. It looks like the
number of pairs? That does not seem like a hardware property. The
controller itself should be able to tell you how many pairs it can
feed.

A hardware property would be which pairs of the socket are connected
to a PSE and so can be used to deliver power. But i'm not sure how
that would be useful to know. I suppose a controller capable of
powering 4 pair, but connected to a socket only wired to supply 2, can
then disable 2 pairs?

> +
> +  ieee802.3-pse-type:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    minimum: 1
> +    maximum: 2
> +    description: PSE Type. Describes classification- and class-capabilities.
> +      Not compatible with PoDL PSE Type.
> +      Type 1 - provides a Class 0, 1, 2, or 3 signature during Physical Layer
> +      classification.
> +      Type 2 - provides a Class 4 signature during Physical Layer
> +      classification, understands 2-Event classification, and is capable of
> +      Data Link Layer classification.

Again, the controller should know what class it can support. Why do we
need to specify it?  What could make sense is we want to limit the
controller to a specific type? 

> +  ieee802.3-podl-pse-class:
> +    $ref: /schemas/types.yaml#/definitions/int8-array
> +    items:
> +      enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
> +    description: PoDL PSE Class. Array of supported classes by the
> +      single twisted-pair PoDL PSE.

Why? I could understand we might want to limit the higher classes,
because the board is not designed for them, but the controller on the
board can actually offer them. But if it tries to use them, the board
will melt/blow a fuse.

So i'm wondering if it should actually be something like:

> +  ieee802.3-podl-limit-pse-classes:
> +    $ref: /schemas/types.yaml#/definitions/int8-array
> +    items:
> +      enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
> +    description: PoDL PSE Class. Limit the PoDL PSE to only these classes,
         due to hardware design limitations. If not specified, the PoDL PSE
	 will offer all classes its supports.

Remember, DT describes the hardware, not software configuration of the
hardware.

	Andrew

Re: [PATCH net-next v2 1/7] dt-bindings: net: pse-dt: add bindings for generic PSE controller

[Oleksij Rempel]

On Fri, Aug 26, 2022 at 12:27:51AM +0200, Andrew Lunn wrote:
> > +  ieee802.3-pairs:
> > +    $ref: /schemas/types.yaml#/definitions/int8-array
> > +    description: Array of number of twisted-pairs capable to deliver power.
> > +      Since not all circuits are able to support all pair variants, the array of
> > +      supported variants should be specified.
> > +      Note - single twisted-pair PSE is formally know as PoDL PSE.
> > +    items:
> > +      enum: [1, 2, 4]
> 
> It is not clear to me what you are describing here. It looks like the
> number of pairs? That does not seem like a hardware property. The
> controller itself should be able to tell you how many pairs it can
> feed.
> 
> A hardware property would be which pairs of the socket are connected
> to a PSE and so can be used to deliver power.

Good point, this will be needed as well. But not right now.

> But i'm not sure how
> that would be useful to know. I suppose a controller capable of
> powering 4 pair, but connected to a socket only wired to supply 2, can
> then disable 2 pairs?

Not only. Here are following reasons:
- not all boards use a controller in form of IC. Some boards are the
  controller. So, there is no other place to describe, what kind of
  controller this board is. For example - currently there are no known
  ICs to support PoDL (ieee802.3-pairs == 1), early adopters are
  implementing it by using MOSFETs coupled with ADCs and some extra
  logic on CPU side:
  https://www.ti.com/lit/an/snla395/snla395.pdf
- not all ICs provide a way for advanced communication (I2C, SPI, MDIO).
  Some of them will provide only bootstrapping and some pin status
  feedback:
  https://www.analog.com/media/en/technical-documentation/data-sheets/4279fa.pdf
- Even if we are able to communicate with the IC, there are still board
  specific limitations.

I hope we can agree that some property is need to tell what kind of PSE
specification is used by this node.

The next challenge is to name it. We have following options:
1. PoE, PoE+, PoE++, 4PPoE, PoDL
2. 802.3af, 802.3at, 802.bt, 802.3bu, 802.3cg
3. Physical property of this specifications

Option 1 is mostly using marketing names, except of PoDL. This names are
not used in the ieee 802.3-2018 specification. Systematic research of
this marketing names would give following results:
- PoE is about delivering power over two twisted pairs and is related to
  802.3af and 802.3at specs.
- PoE+ is about delivering power over two twisted pairs and is related
  only to 802.3at.
- PoE++ is the same as 4PPoE or power over four twisted pairs and is related
  to 802.3bt.
- PoDL is related to 802.3bu and 802.3cg. Which is power over one
  twisted pair

All of this names combine different properties: number of twisted pairs
used to deliver power, maximal supported power by the system and
recommendation for digital interface to communicate with the PSE
controller (MDIO registers). Since system I currently use do not follow
all of this recommendations, it is needed to describe them separately.

Option 2 is interesting only for archaeological investigation. Final
snapshots of 802.3 specification do not provide mapping of extensions to
actual parts of the spec. I assume, no software developer will be able
to properly set the devicetree property by using specification extension
names.

Option 3 provide exact physical property of implementation by using same
wording provided by the  802.3-2018 spec. This option is easy to verify
by reviewing the board schematics and it is easy to understand without
doing historical analysis of 802.3 spec.

> > +
> > +  ieee802.3-pse-type:
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    minimum: 1
> > +    maximum: 2
> > +    description: PSE Type. Describes classification- and class-capabilities.
> > +      Not compatible with PoDL PSE Type.
> > +      Type 1 - provides a Class 0, 1, 2, or 3 signature during Physical Layer
> > +      classification.
> > +      Type 2 - provides a Class 4 signature during Physical Layer
> > +      classification, understands 2-Event classification, and is capable of
> > +      Data Link Layer classification.
> 
> Again, the controller should know what class it can support. Why do we
> need to specify it?  What could make sense is we want to limit the
> controller to a specific type? 

If we are using existing controller - yes. But this binding is designed for the
system where no special PSE IC is used. This Types and Classes depends on the
board implementation and should be provided by the vendor as part of
product certification. This information can be used by the system
administrators to verify compatibility between PSE and PD, especially if
no automatic classification is not implemented on this board.

And even if auto classification is implemented by the software, we need
to know which classes we should announce.

> > +  ieee802.3-podl-pse-class:
> > +    $ref: /schemas/types.yaml#/definitions/int8-array
> > +    items:
> > +      enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
> > +    description: PoDL PSE Class. Array of supported classes by the
> > +      single twisted-pair PoDL PSE.
> 
> Why? I could understand we might want to limit the higher classes,
> because the board is not designed for them, but the controller on the
> board can actually offer them. But if it tries to use them, the board
> will melt/blow a fuse.
> 
> So i'm wondering if it should actually be something like:
> 
> > +  ieee802.3-podl-limit-pse-classes:
> > +    $ref: /schemas/types.yaml#/definitions/int8-array
> > +    items:
> > +      enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
> > +    description: PoDL PSE Class. Limit the PoDL PSE to only these classes,
>          due to hardware design limitations. If not specified, the PoDL PSE
> 	 will offer all classes its supports.

Sounds good, this can be an extra property for PSE ICs, not for boards
without dedicated ICs.

> Remember, DT describes the hardware, not software configuration of the
> hardware.

Ack, I agree.

-- 
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 1/7] dt-bindings: net: pse-dt: add bindings for generic PSE controller

[Andrew Lunn]

On Fri, Aug 26, 2022 at 09:49:40AM +0200, Oleksij Rempel wrote:
> On Fri, Aug 26, 2022 at 12:27:51AM +0200, Andrew Lunn wrote:
> > > +  ieee802.3-pairs:
> > > +    $ref: /schemas/types.yaml#/definitions/int8-array
> > > +    description: Array of number of twisted-pairs capable to deliver power.
> > > +      Since not all circuits are able to support all pair variants, the array of
> > > +      supported variants should be specified.
> > > +      Note - single twisted-pair PSE is formally know as PoDL PSE.
> > > +    items:
> > > +      enum: [1, 2, 4]
> > 
> > It is not clear to me what you are describing here. It looks like the
> > number of pairs? That does not seem like a hardware property. The
> > controller itself should be able to tell you how many pairs it can
> > feed.
> > 
> > A hardware property would be which pairs of the socket are connected
> > to a PSE and so can be used to deliver power.
> 
> Good point, this will be needed as well. But not right now.

That is another point. You are adding properties which no driver
actually uses. That is unusual.

I think i would rename your current driver to regulator. That is all
it is, and it only needs one property, the regulator itself. Its yaml
description should only have the regulator, and nothing else.

When other drivers start to be added, we can think about each property
they add, and decided if they are generic, or specific to a
driver/board. Generic properties we can add to one shared .yaml file,
device/board specific properties get added to that drivers .yaml

> > But i'm not sure how
> > that would be useful to know. I suppose a controller capable of
> > powering 4 pair, but connected to a socket only wired to supply 2, can
> > then disable 2 pairs?
> 
> Not only. Here are following reasons:
> - not all boards use a controller in form of IC. Some boards are the
>   controller. So, there is no other place to describe, what kind of
>   controller this board is. For example - currently there are no known
>   ICs to support PoDL (ieee802.3-pairs == 1), early adopters are
>   implementing it by using MOSFETs coupled with ADCs and some extra
>   logic on CPU side:
>   https://www.ti.com/lit/an/snla395/snla395.pdf
> - not all ICs provide a way for advanced communication (I2C, SPI, MDIO).
>   Some of them will provide only bootstrapping and some pin status
>   feedback:
>   https://www.analog.com/media/en/technical-documentation/data-sheets/4279fa.pdf
> - Even if we are able to communicate with the IC, there are still board
>   specific limitations.

I expect each of these will provide some sort of driver. It could be
board specific, or it could be MCU specific if the same MCU is used
multiple times and each implementation looks the same to Linux. I
suppose there could even be a library which implements SCCP via a
bit-banging GPIO line, and it has a binding for the two GPIO?

And if there is no communication at all with it, you cannot represent
it in Linux, so you don't need to worry about it.

Each driver should come with its own .yaml file, and we should review
it, and decided are the properties common or not.

> I hope we can agree that some property is need to tell what kind of PSE
> specification is used by this node.
> 
> The next challenge is to name it. We have following options:
> 1. PoE, PoE+, PoE++, 4PPoE, PoDL
> 2. 802.3af, 802.3at, 802.bt, 802.3bu, 802.3cg
> 3. Physical property of this specifications
> 
> Option 1 is mostly using marketing names, except of PoDL. This names are
> not used in the ieee 802.3-2018 specification. Systematic research of
> this marketing names would give following results:
> - PoE is about delivering power over two twisted pairs and is related to
>   802.3af and 802.3at specs.
> - PoE+ is about delivering power over two twisted pairs and is related
>   only to 802.3at.
> - PoE++ is the same as 4PPoE or power over four twisted pairs and is related
>   to 802.3bt.
> - PoDL is related to 802.3bu and 802.3cg. Which is power over one
>   twisted pair
> 
> All of this names combine different properties: number of twisted pairs
> used to deliver power, maximal supported power by the system and
> recommendation for digital interface to communicate with the PSE
> controller (MDIO registers). Since system I currently use do not follow
> all of this recommendations, it is needed to describe them separately.
> 
> Option 2 is interesting only for archaeological investigation. Final
> snapshots of 802.3 specification do not provide mapping of extensions to
> actual parts of the spec. I assume, no software developer will be able
> to properly set the devicetree property by using specification extension
> names.
> 
> Option 3 provide exact physical property of implementation by using same
> wording provided by the  802.3-2018 spec. This option is easy to verify
> by reviewing the board schematics and it is easy to understand without
> doing historical analysis of 802.3 spec.

I would go for option 3. We want well defined concepts, and
specifications provide that.

> > > +
> > > +  ieee802.3-pse-type:
> > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > +    minimum: 1
> > > +    maximum: 2
> > > +    description: PSE Type. Describes classification- and class-capabilities.
> > > +      Not compatible with PoDL PSE Type.
> > > +      Type 1 - provides a Class 0, 1, 2, or 3 signature during Physical Layer
> > > +      classification.
> > > +      Type 2 - provides a Class 4 signature during Physical Layer
> > > +      classification, understands 2-Event classification, and is capable of
> > > +      Data Link Layer classification.
> > 
> > Again, the controller should know what class it can support. Why do we
> > need to specify it?  What could make sense is we want to limit the
> > controller to a specific type? 
> 
> If we are using existing controller - yes. But this binding is designed for the
> system where no special PSE IC is used.

I would expect a discreet implementation to also have a driver. The
specific discreet implementation should have a compatible, and a yaml
file describing whatever properties it needs. And since the driver is
specific to the discreet implementation, it should know what it can do
in terms of PSE Type, etc.

If the same discrete implementation is used on multiple boards, and
there are board specific limitations, then we need properties to limit
what it can do. Maybe those limits are then described in the shared
.yaml file, since limits like this probably are generic.

In general, i expect we will end up with two classes of properties:

Hardware controls: I2C bus address, SPI address, gpios for bit banging
SCCP, GPIOs for turning power on/off and sensing etc.

Board specific limitations: Max class, Max current, max Type etc.

      Andrew
 

Re: [PATCH net-next v2 1/7] dt-bindings: net: pse-dt: add bindings for generic PSE controller

[Oleksij Rempel]

On Sat, Aug 27, 2022 at 04:48:50PM +0200, Andrew Lunn wrote:
> On Fri, Aug 26, 2022 at 09:49:40AM +0200, Oleksij Rempel wrote:
> > On Fri, Aug 26, 2022 at 12:27:51AM +0200, Andrew Lunn wrote:
> > > > +  ieee802.3-pairs:
> > > > +    $ref: /schemas/types.yaml#/definitions/int8-array
> > > > +    description: Array of number of twisted-pairs capable to deliver power.
> > > > +      Since not all circuits are able to support all pair variants, the array of
> > > > +      supported variants should be specified.
> > > > +      Note - single twisted-pair PSE is formally know as PoDL PSE.
> > > > +    items:
> > > > +      enum: [1, 2, 4]
> > > 
> > > It is not clear to me what you are describing here. It looks like the
> > > number of pairs? That does not seem like a hardware property. The
> > > controller itself should be able to tell you how many pairs it can
> > > feed.
> > > 
> > > A hardware property would be which pairs of the socket are connected
> > > to a PSE and so can be used to deliver power.
> > 
> > Good point, this will be needed as well. But not right now.
> 
> That is another point. You are adding properties which no driver
> actually uses. That is unusual.
> 
> I think i would rename your current driver to regulator. That is all
> it is, and it only needs one property, the regulator itself. Its yaml
> description should only have the regulator, and nothing else.
> 
> When other drivers start to be added, we can think about each property
> they add, and decided if they are generic, or specific to a
> driver/board. Generic properties we can add to one shared .yaml file,
> device/board specific properties get added to that drivers .yaml
> 
> > > But i'm not sure how
> > > that would be useful to know. I suppose a controller capable of
> > > powering 4 pair, but connected to a socket only wired to supply 2, can
> > > then disable 2 pairs?
> > 
> > Not only. Here are following reasons:
> > - not all boards use a controller in form of IC. Some boards are the
> >   controller. So, there is no other place to describe, what kind of
> >   controller this board is. For example - currently there are no known
> >   ICs to support PoDL (ieee802.3-pairs == 1), early adopters are
> >   implementing it by using MOSFETs coupled with ADCs and some extra
> >   logic on CPU side:
> >   https://www.ti.com/lit/an/snla395/snla395.pdf
> > - not all ICs provide a way for advanced communication (I2C, SPI, MDIO).
> >   Some of them will provide only bootstrapping and some pin status
> >   feedback:
> >   https://www.analog.com/media/en/technical-documentation/data-sheets/4279fa.pdf
> > - Even if we are able to communicate with the IC, there are still board
> >   specific limitations.
> 
> I expect each of these will provide some sort of driver. It could be
> board specific, or it could be MCU specific if the same MCU is used
> multiple times and each implementation looks the same to Linux. I
> suppose there could even be a library which implements SCCP via a
> bit-banging GPIO line, and it has a binding for the two GPIO?
> 
> And if there is no communication at all with it, you cannot represent
> it in Linux, so you don't need to worry about it.
> 
> Each driver should come with its own .yaml file, and we should review
> it, and decided are the properties common or not.
> 
> > I hope we can agree that some property is need to tell what kind of PSE
> > specification is used by this node.
> > 
> > The next challenge is to name it. We have following options:
> > 1. PoE, PoE+, PoE++, 4PPoE, PoDL
> > 2. 802.3af, 802.3at, 802.bt, 802.3bu, 802.3cg
> > 3. Physical property of this specifications
> > 
> > Option 1 is mostly using marketing names, except of PoDL. This names are
> > not used in the ieee 802.3-2018 specification. Systematic research of
> > this marketing names would give following results:
> > - PoE is about delivering power over two twisted pairs and is related to
> >   802.3af and 802.3at specs.
> > - PoE+ is about delivering power over two twisted pairs and is related
> >   only to 802.3at.
> > - PoE++ is the same as 4PPoE or power over four twisted pairs and is related
> >   to 802.3bt.
> > - PoDL is related to 802.3bu and 802.3cg. Which is power over one
> >   twisted pair
> > 
> > All of this names combine different properties: number of twisted pairs
> > used to deliver power, maximal supported power by the system and
> > recommendation for digital interface to communicate with the PSE
> > controller (MDIO registers). Since system I currently use do not follow
> > all of this recommendations, it is needed to describe them separately.
> > 
> > Option 2 is interesting only for archaeological investigation. Final
> > snapshots of 802.3 specification do not provide mapping of extensions to
> > actual parts of the spec. I assume, no software developer will be able
> > to properly set the devicetree property by using specification extension
> > names.
> > 
> > Option 3 provide exact physical property of implementation by using same
> > wording provided by the  802.3-2018 spec. This option is easy to verify
> > by reviewing the board schematics and it is easy to understand without
> > doing historical analysis of 802.3 spec.
> 
> I would go for option 3. We want well defined concepts, and
> specifications provide that.
> 
> > > > +
> > > > +  ieee802.3-pse-type:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > +    minimum: 1
> > > > +    maximum: 2
> > > > +    description: PSE Type. Describes classification- and class-capabilities.
> > > > +      Not compatible with PoDL PSE Type.
> > > > +      Type 1 - provides a Class 0, 1, 2, or 3 signature during Physical Layer
> > > > +      classification.
> > > > +      Type 2 - provides a Class 4 signature during Physical Layer
> > > > +      classification, understands 2-Event classification, and is capable of
> > > > +      Data Link Layer classification.
> > > 
> > > Again, the controller should know what class it can support. Why do we
> > > need to specify it?  What could make sense is we want to limit the
> > > controller to a specific type? 
> > 
> > If we are using existing controller - yes. But this binding is designed for the
> > system where no special PSE IC is used.
> 
> I would expect a discreet implementation to also have a driver. The
> specific discreet implementation should have a compatible, and a yaml
> file describing whatever properties it needs. And since the driver is
> specific to the discreet implementation, it should know what it can do
> in terms of PSE Type, etc.
> 
> If the same discrete implementation is used on multiple boards, and
> there are board specific limitations, then we need properties to limit
> what it can do. Maybe those limits are then described in the shared
> .yaml file, since limits like this probably are generic.
> 
> In general, i expect we will end up with two classes of properties:
> 
> Hardware controls: I2C bus address, SPI address, gpios for bit banging
> SCCP, GPIOs for turning power on/off and sensing etc.
> 
> Board specific limitations: Max class, Max current, max Type etc.

Ok, so current plan is:
- rename this driver and binding to pse-regulator
- i will integrate the "ieee802.3-pairs" property, since this driver
  need to know which field it need to fill in the ethtool response (PSE
  vs PoDL PSE)
- compatible will be "ieee802.3-pse-regulator"

Correct?

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 1/7] dt-bindings: net: pse-dt: add bindings for generic PSE controller

[Andrew Lunn]

> Ok, so current plan is:
> - rename this driver and binding to pse-regulator
> - i will integrate the "ieee802.3-pairs" property, since this driver
>   need to know which field it need to fill in the ethtool response (PSE
>   vs PoDL PSE)

It seems odd to have a property which only purpose is to supply
userspace with some information. If all you have is a single
regulator, does it even matter if it is PSE vs PoDL PSE?

If you think it does matter, i would probably have two compatibles.

   Andrew