2022-08-19 12:04:23

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v1 0/7] add generic PSE support

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


2022-08-19 12:04:52

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v1 6/7] net: mdiobus: search for PSE nodes by parsing PHY nodes.

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 <[email protected]>
---
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 <[email protected]>");
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

2022-08-19 12:20:29

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v1 5/7] net: mdiobus: fwnode_mdiobus_register_phy() rework error handling

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

Signed-off-by: Oleksij Rempel <[email protected]>
---
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

2022-08-19 12:22:26

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v1 3/7] net: add framework to support Ethernet PSE and PDs devices

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 <[email protected]>
---
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 <[email protected]>
+//
+
+#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 <[email protected]>
+ */
+#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

2022-08-19 12:23:57

by Oleksij Rempel

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

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

Signed-off-by: Oleksij Rempel <[email protected]>
---
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

2022-08-19 12:46:57

by Oleksij Rempel

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

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 <[email protected]>
---
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 <[email protected]>
+//
+
+#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 <[email protected]>");
+MODULE_DESCRIPTION("Generic Ethernet Power Sourcing Equipment");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:pse-generic");
--
2.30.2

2022-08-19 21:08:12

by Andrew Lunn

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

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 <[email protected]>
> ---
> 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 <[email protected]>
> +//
> +
> +#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

2022-08-19 21:34:49

by Andrew Lunn

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

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

2022-08-20 12:50:04

by Oleksij Rempel

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

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 |

2022-08-20 13:30:40

by Oleksij Rempel

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

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 |

2022-08-22 18:49:56

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
> 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

2022-08-22 19:59:24

by Andrew Lunn

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

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 <[email protected]>
> > ---
> > 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

2022-08-23 16:53:13

by Oleksij Rempel

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

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 <[email protected]>
> > > ---
> > > 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 |