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

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

[Oleksij Rempel]

Add generic support for the Ethernet Power Sourcing Equipment.

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: pse-pd: add generic PSE driver
  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

 .../devicetree/bindings/net/ethernet-phy.yaml |   6 +
 .../bindings/net/pse-pd/generic-pse.yaml      |  40 ++
 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                 | 387 ++++++++++++++++++
 drivers/net/pse-pd/pse_generic.c              | 146 +++++++
 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/netlink.c                         |  19 +
 net/ethtool/netlink.h                         |   4 +
 net/ethtool/pse-pd.c                          | 194 +++++++++
 19 files changed, 1141 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 v1 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>
---
 .../bindings/net/pse-pd/generic-pse.yaml      | 40 +++++++++++++++++++
 1 file changed, 40 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..64f91efa79a56
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pse-pd/generic-pse.yaml
@@ -0,0 +1,40 @@
+# 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-podl-pse-generic
+
+  '#pse-cells':
+    const: 0
+
+  ieee802.3-podl-pse-supply:
+    description: |
+      Power supply for the PSE controller
+
+additionalProperties: false
+
+required:
+  - compatible
+  - '#pse-cells'
+  - ieee802.3-podl-pse-supply
+
+examples:
+  - |
+    ethernet-pse-1 {
+      compatible = "ieee802.3-podl-pse-generic";
+      ieee802.3-podl-pse-supply = <&reg_t1l1>;
+      #pse-cells = <0>;
+    };
-- 
2.30.2

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

[Rob Herring]

On Fri, Aug 19, 2022 at 02:01:03PM +0200, Oleksij Rempel wrote:
> Add binding for generic Ethernet PSE controller.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../bindings/net/pse-pd/generic-pse.yaml      | 40 +++++++++++++++++++
>  1 file changed, 40 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..64f91efa79a56
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pse-pd/generic-pse.yaml
> @@ -0,0 +1,40 @@
> +# 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.

Isn't this separate from the PHY other than you need to associate 
supplies with ethernet ports?

Is there a controller here? Or it is just a regulator consumer 
associated with an ethernet port?

> +
> +properties:
> +  compatible:
> +    const: ieee802.3-podl-pse-generic

Is this for 802.3bu only (which is where PoDL comes from) or all the 
flavors? If all, do they need to be distinguished?

'generic' is redundant.

> +
> +  '#pse-cells':

What's this for? You don't have a consumer.

> +    const: 0
> +
> +  ieee802.3-podl-pse-supply:

Seems a bit long

> +    description: |

Don't need '|' if no formatting to maintain.

> +      Power supply for the PSE controller
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - '#pse-cells'
> +  - ieee802.3-podl-pse-supply
> +
> +examples:
> +  - |
> +    ethernet-pse-1 {
> +      compatible = "ieee802.3-podl-pse-generic";
> +      ieee802.3-podl-pse-supply = <&reg_t1l1>;
> +      #pse-cells = <0>;
> +    };
> -- 
> 2.30.2
> 
> 

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

[Oleksij Rempel]

On Mon, Aug 22, 2022 at 01:41:12PM -0500, Rob Herring wrote:
> On Fri, Aug 19, 2022 at 02:01:03PM +0200, Oleksij Rempel wrote:
> > Add binding for generic Ethernet PSE controller.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  .../bindings/net/pse-pd/generic-pse.yaml      | 40 +++++++++++++++++++
> >  1 file changed, 40 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..64f91efa79a56
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/pse-pd/generic-pse.yaml
> > @@ -0,0 +1,40 @@
> > +# 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.
> 
> Isn't this separate from the PHY other than you need to associate 
> supplies with ethernet ports?
> 
> Is there a controller here? Or it is just a regulator consumer 
> associated with an ethernet port?

Current version has only regulator. It will be extended with IEEE 802.3
specific power source classification information, wich will be overkill for the
regulator framework. I can add it to the v2 version.

> > +properties:
> > +  compatible:
> > +    const: ieee802.3-podl-pse-generic
> 
> Is this for 802.3bu only (which is where PoDL comes from) or all the 
> flavors? If all, do they need to be distinguished?

yes. ieee802.3 defines type and class with different enumeration and
meanings for PSE and PoDL PSE. 

So far we have two different modes:
 - 802.3bu (PoDL PSE). Has own types and classes
 - 802.3af  is extended by 802.3at, and the extended by 802.3bt
   all of them are named as PSE and has own types and classes as well.

I worry more about the fact is some one will implement HW supporting both
modes. IMO, it is possible to take usual ethernet PHY, configure to
10Bit half-duplex and run over single pair. In this case it is possible
to use only PoDL PSE mode.

In this case I need single generic compatible but different properties
to describe supported PSE and PoDL PSE modes.

> 'generic' is redundant.

ok

> > +
> > +  '#pse-cells':
> 
> What's this for? You don't have a consumer.

the consumer is PHY.

> > +    const: 0
> > +
> > +  ieee802.3-podl-pse-supply:
> 
> Seems a bit long

ok. Reduce it to pse-supply ?

> > +    description: |
> 
> Don't need '|' if no formatting to maintain.

ok

> > +      Power supply for the PSE controller
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - '#pse-cells'
> > +  - ieee802.3-podl-pse-supply
> > +
> > +examples:
> > +  - |
> > +    ethernet-pse-1 {
> > +      compatible = "ieee802.3-podl-pse-generic";
> > +      ieee802.3-podl-pse-supply = <&reg_t1l1>;
> > +      #pse-cells = <0>;
> > +    };
> > -- 
> > 2.30.2
> > 
> > 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH net-next v1 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 | 6 ++++++
 1 file changed, 6 insertions(+)

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

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

[Rob Herring]

On Fri, Aug 19, 2022 at 02:01:04PM +0200, Oleksij Rempel wrote:
> 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 | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index ed1415a4381f2..49c74e177c788 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -144,6 +144,12 @@ properties:
>        Mark the corresponding energy efficient ethernet mode as
>        broken and request the ethernet to stop advertising it.
>  
> +  ieee802.3-podl-pse:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Specifies a reference to a node representing a Power over Data Lines
> +      Power Sourcing Equipment.

Ah, here is the consumer.

Why do you anything more than just a -supply property here for the 
PoE/PoDL supply? The only reason I see is you happen to want a separate 
driver for this and a separate node happens to be a convenient way to 
instantiate drivers in Linux. Convince me otherwise.

Rob

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

[Andrew Lunn]

On Mon, Aug 22, 2022 at 01:45:34PM -0500, Rob Herring wrote:
> On Fri, Aug 19, 2022 at 02:01:04PM +0200, Oleksij Rempel wrote:
> > 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 | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > index ed1415a4381f2..49c74e177c788 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > @@ -144,6 +144,12 @@ properties:
> >        Mark the corresponding energy efficient ethernet mode as
> >        broken and request the ethernet to stop advertising it.
> >  
> > +  ieee802.3-podl-pse:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      Specifies a reference to a node representing a Power over Data Lines
> > +      Power Sourcing Equipment.
> 
> Ah, here is the consumer.
> 
> Why do you anything more than just a -supply property here for the 
> PoE/PoDL supply? The only reason I see is you happen to want a separate 
> driver for this and a separate node happens to be a convenient way to 
> instantiate drivers in Linux. Convince me otherwise.

The regulator binding provides a lot of very useful properties, which
look to do a good job describing the regulator part of a PoE/PeDL
supplier side. What however is missing is the communication part, the
power provider and the power consumer communicate with each other, via
a serial protocol. They negotiate the supply of power, a sleep mode
where power is reduced, but not removed, etc.

So a Power Sourcing Equipment driver is very likely to have a
regulator embedded in it, but its more than a regulator.

	 Andrew

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

[Oleksij Rempel]

On Mon, Aug 22, 2022 at 09:34:47PM +0200, Andrew Lunn wrote:
> On Mon, Aug 22, 2022 at 01:45:34PM -0500, Rob Herring wrote:
> > On Fri, Aug 19, 2022 at 02:01:04PM +0200, Oleksij Rempel wrote:
> > > 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 | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > > index ed1415a4381f2..49c74e177c788 100644
> > > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > > @@ -144,6 +144,12 @@ properties:
> > >        Mark the corresponding energy efficient ethernet mode as
> > >        broken and request the ethernet to stop advertising it.
> > >  
> > > +  ieee802.3-podl-pse:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description:
> > > +      Specifies a reference to a node representing a Power over Data Lines
> > > +      Power Sourcing Equipment.
> > 
> > Ah, here is the consumer.
> > 
> > Why do you anything more than just a -supply property here for the 
> > PoE/PoDL supply? The only reason I see is you happen to want a separate 
> > driver for this and a separate node happens to be a convenient way to 
> > instantiate drivers in Linux. Convince me otherwise.
> 
> The regulator binding provides a lot of very useful properties, which
> look to do a good job describing the regulator part of a PoE/PeDL
> supplier side. What however is missing is the communication part, the
> power provider and the power consumer communicate with each other, via
> a serial protocol. They negotiate the supply of power, a sleep mode
> where power is reduced, but not removed, etc.
> 
> So a Power Sourcing Equipment driver is very likely to have a
> regulator embedded in it, but its more than a regulator.

@Rob, is it enough to convince?

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 |

[PATCH net-next v1 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>
---
 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 | 387 ++++++++++++++++++++++++++++++++++
 include/linux/pse-pd/pse.h    | 134 ++++++++++++
 6 files changed, 539 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..cbb79fc2e2706
--- /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..a0b42dc46d029
--- /dev/null
+++ b/drivers/net/pse-pd/pse-core.c
@@ -0,0 +1,387 @@
+// 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;
+	}
+
+	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-podl-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);
+
+/**
+ * pse_podl_get_admin_sate - get operational state of the PoDL PSE functions.
+ * @pcdev: a pointer to the PSE controller device
+ *
+ * IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState:
+ * A read-only value that identifies the operational state of the PoDL PSE
+ * functions.
+ *
+ * Return values:
+ * see enum ethtool_podl_pse_admin_state
+ * Negative value is an error.
+ */
+int pse_podl_get_admin_sate(struct pse_control *psec)
+{
+	const struct pse_control_ops *ops;
+
+	if (!psec)
+		return 0;
+
+	if (WARN_ON(IS_ERR(psec)))
+		return -EINVAL;
+
+	ops = psec->pcdev->ops;
+
+	if (!ops->get_podl_pse_admin_sate)
+		return -ENOTSUPP;
+
+	return ops->get_podl_pse_admin_sate(psec->pcdev, psec->id);
+}
+EXPORT_SYMBOL_GPL(pse_podl_get_admin_sate);
+
+/**
+ * pse_podl_set_admin_control - set operational state of the PoDL PSE functions.
+ * @pcdev: a pointer to the PSE controller device
+ *
+ * IEEE 802.3-2018 30.15.1.2.1 acPoDLPSEAdminControl
+ * This action provides a means to alter aPoDLPSEAdminState.
+ *
+ * Return values:
+ * Negative value is an error.
+ */
+int pse_podl_set_admin_control(struct pse_control *psec,
+			       enum ethtool_podl_pse_admin_state state)
+{
+	const struct pse_control_ops *ops;
+
+	if (!psec)
+		return 0;
+
+	if (WARN_ON(IS_ERR(psec)))
+		return -EINVAL;
+
+	ops = psec->pcdev->ops;
+
+	if (!ops->set_podl_pse_admin_control)
+		return -ENOTSUPP;
+
+	if (state >= ETHTOOL_PODL_PSE_ADMIN_STATE_COUNT)
+		return -ENOTSUPP;
+
+	return ops->set_podl_pse_admin_control(psec->pcdev, psec->id, state);
+}
+EXPORT_SYMBOL_GPL(pse_podl_set_admin_control);
+
+/**
+ * pse_podl_get_pw_d_status - get a PoDL PSE power detection status
+ * @pcdev: a pointer to the PSE controller device
+ *
+ * IEEE 802.3-2018 30.15.1.1.3 aPoDLPSEPowerDetectionStatus:
+ * A read-only value that indicates the current status of the PoDL PSE.
+ *
+ * Return values:
+ * see enum ethtool_podl_pse_pw_d_status
+ * Negative value is an error.
+ */
+int pse_podl_get_pw_d_status(struct pse_control *psec)
+{
+	const struct pse_control_ops *ops;
+
+	if (!psec)
+		return 0;
+
+	if (WARN_ON(IS_ERR(psec)))
+		return -EINVAL;
+
+	ops = psec->pcdev->ops;
+
+	if (!ops->get_podl_pse_pw_d_status)
+		return -ENOTSUPP;
+
+	return ops->get_podl_pse_pw_d_status(psec->pcdev, psec->id);
+}
+EXPORT_SYMBOL_GPL(pse_podl_get_pw_d_status);
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
new file mode 100644
index 0000000000000..77d1d64f3e831
--- /dev/null
+++ b/include/linux/pse-pd/pse.h
@@ -0,0 +1,134 @@
+// 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/list.h>
+#include <uapi/linux/ethtool.h>
+
+struct phy_device;
+struct pse_controller_dev;
+
+/**
+ * struct pse_control_ops - PSE controller driver callbacks
+ *
+ * @get_podl_pse_admin_sate: return a PoDL PSE admin state as described in
+ *	IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState
+ * @set_podl_pse_admin_control: set PoDL PSE admin control as described in
+ *	IEEE 802.3-2018 30.15.1.2.1 acPoDLPSEAdminControl
+ * @get_podl_pse_pw_d_status: return a PoDL PSE power detection status as
+ *	described in: IEEE 802.3-2018 30.15.1.1.3 aPoDLPSEPowerDetectionStatus
+ */
+struct pse_control_ops {
+	int (*get_podl_pse_admin_sate)(struct pse_controller_dev *pcdev,
+		unsigned long id);
+	int (*set_podl_pse_admin_control)(struct pse_controller_dev *pcdev,
+		unsigned long id, enum ethtool_podl_pse_admin_state state);
+	int (*get_podl_pse_pw_d_status)(struct pse_controller_dev *pcdev,
+		unsigned long id);
+};
+
+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_control_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_control_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;
+};
+
+#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);
+int pse_podl_get_admin_sate(struct pse_control *psec);
+int pse_podl_set_admin_control(struct pse_control *psec,
+			       enum ethtool_podl_pse_admin_state state);
+int pse_podl_get_pw_d_status(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)
+{
+}
+
+static inline int pse_podl_get_admin_sate(struct pse_control *psec)
+{
+	return -ENOTSUPP;
+}
+
+static inline int
+pse_podl_set_admin_control(struct pse_control *psec,
+			   enum ethtool_podl_pse_admin_state state)
+{
+	return -ENOTSUPP;
+}
+
+static inline int pse_podl_get_pw_d_status(struct pse_control *psec)
+{
+	return -ENOTSUPP;
+}
+
+#endif
+
+#endif
-- 
2.30.2

[PATCH net-next v1 4/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>
---
 drivers/net/pse-pd/Kconfig       |  11 +++
 drivers/net/pse-pd/Makefile      |   2 +
 drivers/net/pse-pd/pse_generic.c | 146 +++++++++++++++++++++++++++++++
 3 files changed, 159 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 cbb79fc2e2706..80ef39ad68f10 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..f264d4d589f59
--- /dev/null
+++ b/drivers/net/pse-pd/pse_generic.c
@@ -0,0 +1,146 @@
+// 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_podl_get_admin_sate(struct pse_controller_dev *pcdev, unsigned long id)
+{
+	struct gen_pse_priv *priv = to_gen_pse(pcdev);
+
+	/* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
+	 * which is provided by the regulator.
+	 */
+	return priv->admin_state;
+}
+
+static int
+gen_pse_podl_set_admin_control(struct pse_controller_dev *pcdev,
+			       unsigned long id,
+			       enum ethtool_podl_pse_admin_state state)
+{
+	struct gen_pse_priv *priv = to_gen_pse(pcdev);
+	int ret;
+
+	if (priv->admin_state == state)
+		goto set_state;
+
+	switch (state) {
+	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", state);
+		ret = -ENOTSUPP;
+	}
+
+	if (ret)
+		return ret;
+
+set_state:
+	priv->admin_state = state;
+
+	return 0;
+}
+
+static int
+gen_pse_podl_get_pw_d_status(struct pse_controller_dev *pcdev, unsigned long id)
+{
+	struct gen_pse_priv *priv = to_gen_pse(pcdev);
+	int ret;
+
+	ret = regulator_is_enabled(priv->ps);
+	if (ret < 0)
+		return ret;
+
+	if (!ret)
+		return ETHTOOL_PODL_PSE_PW_D_STATUS_DISABLED;
+
+	return ETHTOOL_PODL_PSE_PW_D_STATUS_DELIVERING;
+}
+
+static const struct pse_control_ops gen_pse_ops = {
+	.get_podl_pse_admin_sate = gen_pse_podl_get_admin_sate,
+	.set_podl_pse_admin_control = gen_pse_podl_set_admin_control,
+	.get_podl_pse_pw_d_status = gen_pse_podl_get_pw_d_status,
+};
+
+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(dev, "ieee802.3-podl-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);
+
+	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-podl-pse-generic", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, gen_pse_of_match);
+
+static struct platform_driver gen_pse_driver = {
+	.probe		= gen_pse_probe,
+	.driver		= {
+		.name		= "PSE Generic",
+		.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 v1 4/7] net: pse-pd: add generic PSE driver

[Andrew Lunn]

On Fri, Aug 19, 2022 at 02:01:06PM +0200, Oleksij Rempel wrote:
> 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>
> ---
>  drivers/net/pse-pd/Kconfig       |  11 +++
>  drivers/net/pse-pd/Makefile      |   2 +
>  drivers/net/pse-pd/pse_generic.c | 146 +++++++++++++++++++++++++++++++
>  3 files changed, 159 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 cbb79fc2e2706..80ef39ad68f10 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..f264d4d589f59
> --- /dev/null
> +++ b/drivers/net/pse-pd/pse_generic.c
> @@ -0,0 +1,146 @@
> +// 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_podl_get_admin_sate(struct pse_controller_dev *pcdev, unsigned long id)

Should that be state?

> +{
> +	struct gen_pse_priv *priv = to_gen_pse(pcdev);
> +
> +	/* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
> +	 * which is provided by the regulator.
> +	 */
> +	return priv->admin_state;
> +}
> +
> +static int
> +gen_pse_podl_set_admin_control(struct pse_controller_dev *pcdev,
> +			       unsigned long id,
> +			       enum ethtool_podl_pse_admin_state state)
> +{
> +	struct gen_pse_priv *priv = to_gen_pse(pcdev);
> +	int ret;
> +
> +	if (priv->admin_state == state)
> +		goto set_state;

return 0; ?

> +
> +	switch (state) {
> +	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", state);
> +		ret = -ENOTSUPP;
> +	}
> +
> +	if (ret)
> +		return ret;
> +
> +set_state:
> +	priv->admin_state = state;
> +
> +	return 0;
> +}
> +
> +static int
> +gen_pse_podl_get_pw_d_status(struct pse_controller_dev *pcdev, unsigned long id)
> +{
> +	struct gen_pse_priv *priv = to_gen_pse(pcdev);
> +	int ret;
> +
> +	ret = regulator_is_enabled(priv->ps);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!ret)
> +		return ETHTOOL_PODL_PSE_PW_D_STATUS_DISABLED;
> +
> +	return ETHTOOL_PODL_PSE_PW_D_STATUS_DELIVERING;
> +}
> +
> +static const struct pse_control_ops gen_pse_ops = {
> +	.get_podl_pse_admin_sate = gen_pse_podl_get_admin_sate,
> +	.set_podl_pse_admin_control = gen_pse_podl_set_admin_control,
> +	.get_podl_pse_pw_d_status = gen_pse_podl_get_pw_d_status,
> +};
> +
> +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(dev, "ieee802.3-podl-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);
> +
> +	priv->admin_state = ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED;

There is the comment earlier:

	/* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
	 * which is provided by the regulator.

Is this because the regulator might of been turned on, but it has
detected a short and turned itself off? So it is administratively on,
but off in order to stop the magic smoke escaping?

But what about the other way around? Something has already turned the
regulator on, e.g. the bootloader. Should the default be
ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED even thought power is being
delivered? Do we want to put the regulator into the off state at
probe, so it is in a well defined state? Or set priv->admin_state to
whatever regulator_is_enabled() indicates?

	 Andrew

Re: [PATCH net-next v1 4/7] net: pse-pd: add generic PSE driver

[Oleksij Rempel]

On Fri, Aug 19, 2022 at 10:54:14PM +0200, Andrew Lunn wrote:
> > +static int
> > +gen_pse_podl_get_admin_sate(struct pse_controller_dev *pcdev, unsigned long id)
> 
> Should that be state?

ack. fixed.

> > +{
> > +	struct gen_pse_priv *priv = to_gen_pse(pcdev);
> > +
> > +	/* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
> > +	 * which is provided by the regulator.
> > +	 */
> > +	return priv->admin_state;
> > +}
> > +
> > +static int
> > +gen_pse_podl_set_admin_control(struct pse_controller_dev *pcdev,
> > +			       unsigned long id,
> > +			       enum ethtool_podl_pse_admin_state state)
> > +{
> > +	struct gen_pse_priv *priv = to_gen_pse(pcdev);
> > +	int ret;
> > +
> > +	if (priv->admin_state == state)
> > +		goto set_state;
> 
> return 0; ?

ack. done

> > +	platform_set_drvdata(pdev, priv);
> > +
> > +	priv->admin_state = ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED;
> 
> There is the comment earlier:
> 
> 	/* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
> 	 * which is provided by the regulator.
> 
> Is this because the regulator might of been turned on, but it has
> detected a short and turned itself off? So it is administratively on,
> but off in order to stop the magic smoke escaping?

ack. According to 30.15.1.1.3 aPoDLPSEPowerDetectionStatus, we may have
following:
/**
 * 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."
 */

Probably all of them, except of ETHTOOL_PODL_PSE_PW_D_STATUS_SEARCHING can be
potentially implemented in this driver on top of regulator framework.

> But what about the other way around? Something has already turned the
> regulator on, e.g. the bootloader. Should the default be
> ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED even thought power is being
> delivered? Do we want to put the regulator into the off state at
> probe, so it is in a well defined state? Or set priv->admin_state to
> whatever regulator_is_enabled() indicates?

Good question. I assume, automotive folks would love be able to enable
regulator in the boot loader on the PSE to let the Powered Device boot parallel
to the PSE.

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 v1 4/7] net: pse-pd: add generic PSE driver

[Andrew Lunn]

On Fri, Aug 19, 2022 at 02:01:06PM +0200, Oleksij Rempel wrote:
> 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.

Do you have access to a PHY which implements clause 45.2.9? That seems
like a better reference implementation?

I don't know the market, what is more likely, a simple regulator, or
something more capable with an interface like 45.2.9?

netlink does allow us to keep adding more attributes, so we don't need
to be perfect first time, but it seems like 45.2.9 is what IEEE expect
vendors to provide, so at some point Linux should implement it.

	  Andrew

Re: [PATCH net-next v1 4/7] net: pse-pd: add generic PSE driver

[Oleksij Rempel]

On Fri, Aug 19, 2022 at 11:32:00PM +0200, Andrew Lunn wrote:
> On Fri, Aug 19, 2022 at 02:01:06PM +0200, Oleksij Rempel wrote:
> > 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.
> 
> Do you have access to a PHY which implements clause 45.2.9? That seems
> like a better reference implementation?

Suddenly I do not have access to any of them. 

> I don't know the market, what is more likely, a simple regulator, or
> something more capable with an interface like 45.2.9?

So far I was not able to find any PoDL PES IC (with classification
support). Or PHY with PoDL PSE on one package. If some one know any,
please let me know.

> netlink does allow us to keep adding more attributes, so we don't need
> to be perfect first time, but it seems like 45.2.9 is what IEEE expect
> vendors to provide, so at some point Linux should implement it.

ack.

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 |

[PATCH net-next v1 5/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 v1 6/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 v1 7/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: Oleksij Rempel <o.rempel@pengutronix.de>
---
 Documentation/networking/ethtool-netlink.rst |  60 ++++++
 include/uapi/linux/ethtool.h                 |  50 +++++
 include/uapi/linux/ethtool_netlink.h         |  17 ++
 net/ethtool/Makefile                         |   3 +-
 net/ethtool/netlink.c                        |  19 ++
 net/ethtool/netlink.h                        |   4 +
 net/ethtool/pse-pd.c                         | 194 +++++++++++++++++++
 7 files changed, 346 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/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..7247769829641 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.o
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..216c6b0d327b5
--- /dev/null
+++ b/net/ethtool/pse-pd.c
@@ -0,0 +1,194 @@
+// 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 "netlink.h"
+#include "common.h"
+#include <linux/phy.h>
+#include "linux/pse-pd/pse.h"
+
+struct pse_req_info {
+	struct ethnl_req_info base;
+};
+
+struct pse_reply_data {
+	struct ethnl_reply_data	base;
+	int podl_pse_admin_state;
+	int podl_pse_pw_d_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 pse_reply_data *data)
+{
+	struct phy_device *phydev = dev->phydev;
+	int ret;
+
+	if (!phydev)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&phydev->lock);
+	if (!phydev->psec) {
+		ret = -EOPNOTSUPP;
+		goto error_unlock;
+	}
+
+	ret = pse_podl_get_admin_sate(phydev->psec);
+	if (ret < 0)
+		goto error_unlock;
+
+	data->podl_pse_admin_state = ret;
+
+	ret = pse_podl_get_pw_d_status(phydev->psec);
+	if (ret < 0)
+		goto error_unlock;
+
+	data->podl_pse_pw_d_status = ret;
+
+error_unlock:
+	mutex_unlock(&phydev->lock);
+
+	return ret;
+}
+
+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 ret;
+
+	ret = pse_get_pse_attributs(dev, 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)
+{
+	struct pse_reply_data *data = PSE_REPDATA(reply_base);
+	int len = 0;
+
+	if (data->podl_pse_admin_state >= 0)
+		len += nla_total_size(sizeof(u8)); /* _PODL_PSE_ADMIN_STATE */
+	if (data->podl_pse_pw_d_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);
+
+	if (data->podl_pse_admin_state >= 0 &&
+	    nla_put_u8(skb, ETHTOOL_A_PODL_PSE_ADMIN_STATE,
+		       data->podl_pse_admin_state))
+		return -EMSGSIZE;
+
+	if (data->podl_pse_pw_d_status >= 0 &&
+	    nla_put_u8(skb, ETHTOOL_A_PODL_PSE_PW_D_STATUS,
+		       data->podl_pse_pw_d_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_mode(struct net_device *dev, struct nlattr **tb)
+{
+	enum ethtool_podl_pse_admin_state state;
+	struct phy_device *phydev = dev->phydev;
+	int ret;
+
+	if (!tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL])
+		return 0;
+
+	state = nla_get_u8(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
+
+	if (!phydev)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&phydev->lock);
+	if (!phydev->psec)
+		ret = -EOPNOTSUPP;
+	else
+		ret = pse_podl_set_admin_control(phydev->psec, state);
+	mutex_unlock(&phydev->lock);
+
+	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_mode(dev, tb);
+	if (ret < 0)
+		goto out_ops;
+
+	ethtool_notify(dev, ETHTOOL_MSG_PSE_NTF, NULL);
+
+out_ops:
+	ethnl_ops_complete(dev);
+out_rtnl:
+	rtnl_unlock();
+	ethnl_parse_header_dev_put(&req_info);
+	return ret;
+}
-- 
2.30.2

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

[kernel test robot]

Hi Oleksij,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Oleksij-Rempel/add-generic-PSE-support/20220819-200408
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 268603d79cc48dba671e9caf108fab32315b86a2
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20220820/202208200039.UVzayOII-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/ac5a14669dbe6cac4972ff01ea6291d12152e78f
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Oleksij-Rempel/add-generic-PSE-support/20220819-200408
        git checkout ac5a14669dbe6cac4972ff01ea6291d12152e78f
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> make[3]: *** No rule to make target 'net/ethtool/pse.o', needed by 'net/ethtool/built-in.a'.
   make[3]: Target '__build' not remade because of errors.

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[kernel test robot]

Hi Oleksij,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Oleksij-Rempel/add-generic-PSE-support/20220819-200408
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 268603d79cc48dba671e9caf108fab32315b86a2
config: x86_64-randconfig-a014 (https://download.01.org/0day-ci/archive/20220820/202208200202.pmwCMvZd-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 0ac597f3cacf60479ffd36b03766fa7462dabd78)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ac5a14669dbe6cac4972ff01ea6291d12152e78f
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Oleksij-Rempel/add-generic-PSE-support/20220819-200408
        git checkout ac5a14669dbe6cac4972ff01ea6291d12152e78f
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> make[3]: *** No rule to make target 'net/ethtool/pse.o', needed by 'net/ethtool/built-in.a'.
   make[3]: Target '__build' not remade because of errors.

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[Andrew Lunn]

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

Here you seem to indicate that delivering power is totally independent
of the interface admin status, <BROADCAST,MULTICAST>. The interface is
admin down, yet you can make it deliver power. I thought there might
be a link between interface admin status and power? Do the standards
say anything about this? Is there some sort of industrial norm?

I'm also wondering about the defaults. It seems like the defaults you
are proposing is power is off by default, and you have to use ethtool
to enable power. That does not seem like the most friendly
settings. Why not an 'auto' mode where if the PHY has PoDL PSE
capabilities, on ifup it is enabled, on ifdown it is disabled? And you
can put it into a 'manual' mode where you control it independent of
administrative status of the interface?

    Andrew

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

[Oleksij Rempel]

On Fri, Aug 19, 2022 at 11:15:09PM +0200, Andrew Lunn wrote:
> > $ 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
> 
> Here you seem to indicate that delivering power is totally independent
> of the interface admin status, <BROADCAST,MULTICAST>. The interface is
> admin down, yet you can make it deliver power. I thought there might
> be a link between interface admin status and power? Do the standards
> say anything about this? Is there some sort of industrial norm?
> 
> I'm also wondering about the defaults. It seems like the defaults you
> are proposing is power is off by default, and you have to use ethtool
> to enable power. That does not seem like the most friendly
> settings. Why not an 'auto' mode where if the PHY has PoDL PSE
> capabilities, on ifup it is enabled, on ifdown it is disabled? And you
> can put it into a 'manual' mode where you control it independent of
> administrative status of the interface?

Hm. I would say, safe option is to enable PSE manually. Here are my
reasons:
- some system may require to have power be enabled on boot, before we
  start to care about administrative state of the interface.
- in some cases powered device should stay enabled, even if we do
  ifup/ifdown

I assume, safe defaults should be:
- keep PSE always off, except system was configured to enable it on
  boot.
- keep PSE on after it was enabled, even on if up/down
- bind PSE admin state to the interface state only if user explicitly
  requested it.

At this round is only default, manual mode is implemented. Automatic
mode can be added later if needed.

These are my points, but i'm open for discussion.

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

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 3163 bytes --]

On Fri, Aug 19, 2022 at 02:01:09PM +0200, Oleksij Rempel wrote:
> +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.
> +  ======================================  ======  ==========================
> +

I don't see malformed table warnings on my htmldocs build, although the
table border for the third column is not long enough to cover the
contents.

> +Request contents:
> +
> +  ======================================  ======  ==========================
> +  ``ETHTOOL_A_PSE_HEADER``                nested  request header
> +  ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL``        u8  Control PoDL PSE Admin state
> +  ======================================  ======  ==========================
> +

Same here too.

In that case, I'd like to extend the border, like:

---- >8 ----

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index c8b09b57bd65ea..2560cf62d14f4e 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1641,13 +1641,13 @@ Request contents:
 
 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
@@ -1673,10 +1673,10 @@ 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

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Andrew Lunn]

On Fri, Aug 19, 2022 at 02:01:09PM +0200, Oleksij Rempel wrote:
> 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.

> +static int pse_get_pse_attributs(struct net_device *dev,
> +				 struct pse_reply_data *data)
> +{
> +	struct phy_device *phydev = dev->phydev;
> +	int ret;
> +
> +	if (!phydev)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&phydev->lock);
> +	if (!phydev->psec) {
> +		ret = -EOPNOTSUPP;
> +		goto error_unlock;
> +	}
> +
> +	ret = pse_podl_get_admin_sate(phydev->psec);
> +	if (ret < 0)
> +		goto error_unlock;
> +
> +	data->podl_pse_admin_state = ret;
> +
> +	ret = pse_podl_get_pw_d_status(phydev->psec);
> +	if (ret < 0)
> +		goto error_unlock;
> +
> +	data->podl_pse_pw_d_status = ret;

I'm wondering how this is going to scale. At some point, i expect
there will be an implementation that follows C45.2.9. I see 14 values
which could be returned. I don't think 14 ops in the driver structure
makes sense. Plus c30.15.1 defines other values.

The nice thing about netlink is you can have as many or are little
attributes in the message as you want. For cable testing, i made use
of this. There is no standardisation, different PHYs offer different
sorts of results. So i made the API flexible. The PHY puts whatever
results it has into the message, and ethtool(1) just walks the message
and prints what is in it.

I'm wondering if we want a similar sort of API here?
net/ethtool/pse-pd.c allocates the netlink messages, adds the header,
and then passes it to the driver. The driver then uses helpers from
ethtool to add whatever attributes it wants to the message. pse-pd
then completes the message, and returns it to user space? This seems
like it will scale better.

     Andrew

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

[Oleksij Rempel]

On Sat, Aug 20, 2022 at 08:16:18PM +0200, Andrew Lunn wrote:
> On Fri, Aug 19, 2022 at 02:01:09PM +0200, Oleksij Rempel wrote:
> > 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.
> 
> > +static int pse_get_pse_attributs(struct net_device *dev,
> > +				 struct pse_reply_data *data)
> > +{
> > +	struct phy_device *phydev = dev->phydev;
> > +	int ret;
> > +
> > +	if (!phydev)
> > +		return -EOPNOTSUPP;
> > +
> > +	mutex_lock(&phydev->lock);
> > +	if (!phydev->psec) {
> > +		ret = -EOPNOTSUPP;
> > +		goto error_unlock;
> > +	}
> > +
> > +	ret = pse_podl_get_admin_sate(phydev->psec);
> > +	if (ret < 0)
> > +		goto error_unlock;
> > +
> > +	data->podl_pse_admin_state = ret;
> > +
> > +	ret = pse_podl_get_pw_d_status(phydev->psec);
> > +	if (ret < 0)
> > +		goto error_unlock;
> > +
> > +	data->podl_pse_pw_d_status = ret;
> 
> I'm wondering how this is going to scale. At some point, i expect
> there will be an implementation that follows C45.2.9. I see 14 values
> which could be returned. I don't think 14 ops in the driver structure
> makes sense. Plus c30.15.1 defines other values.
> 
> The nice thing about netlink is you can have as many or are little
> attributes in the message as you want. For cable testing, i made use
> of this. There is no standardisation, different PHYs offer different
> sorts of results. So i made the API flexible. The PHY puts whatever
> results it has into the message, and ethtool(1) just walks the message
> and prints what is in it.
> 
> I'm wondering if we want a similar sort of API here?
> net/ethtool/pse-pd.c allocates the netlink messages, adds the header,
> and then passes it to the driver. The driver then uses helpers from
> ethtool to add whatever attributes it wants to the message. pse-pd
> then completes the message, and returns it to user space? This seems
> like it will scale better.

Yes. Sounds good. I'll make a new version.

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

[Andrew Lunn]

> +static int pse_get_pse_attributs(struct net_device *dev,
> +				 struct pse_reply_data *data)
> +{
> +	struct phy_device *phydev = dev->phydev;
> +	int ret;
> +
> +	if (!phydev)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&phydev->lock);
> +	if (!phydev->psec) {
> +		ret = -EOPNOTSUPP;
> +		goto error_unlock;
> +	}
> +
> +	ret = pse_podl_get_admin_sate(phydev->psec);
> +	if (ret < 0)
> +		goto error_unlock;

The locking is triggering all sorts of questions in my mind... I don't
have the answers yet, so consider this more a discussion.

You need somewhere to place the psec. Since we are talking power over
copper lines, there will be some sort of PHY, so phydev->psec seems
reasonable. However, there is a general trend of moving all DSA
Ethernet switches to phylink, which is going to make this a bit
tricker to actually get to the phydev object.

But using phydev->lock? Humm. At least in the PoE world, there seems
to be lots of I2C or SPI controllers. Why hold the phydev lock when
performing an I2C transaction? You have a generic linux regulator
driver. How would you see a generic C45.2.9 driver? If it calls in the
PHY driver, the lock is already held, and we have to be careful to not
deadlock.

I'm more thinking along the lines of psec should have a lock of its
own.  pse_podl_get_admin_state(), pse_podl_get_pw_d_status() etc
should take that mutex before calling to the actual driver.

For a PHY which actually supports C45.2.9, i hope that the phylib core
looks at the phy driver structure, sees that some pse_podl ops are
implemented, and instantiates and registers a psec object. The phylib
core provides wrappers, which take the phylib lock before calling into
the driver. And if the PHY strictly follows C45.2.9, the calls are
actually into phylib helpers. Otherwise the PHY driver can do its own
implementation.

     Andrew