* [PATCH net-next v6 01/12] net: ethtool: Add support for ethnl_info_init_ntf helper function
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
@ 2025-03-04 10:18 ` Kory Maincent
2025-03-04 10:18 ` [PATCH net-next v6 02/12] net: pse-pd: Add support for reporting events Kory Maincent
` (10 subsequent siblings)
11 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:18 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project)
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
Introduce support for the ethnl_info_init_ntf helper function to enable
initialization of ethtool notifications outside of the netlink.c file.
This change allows for more flexible notification handling.
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
Changes in v4:
- Use the new helper in ethnl_default_notify function.
Changes in v2:
- new patch.
---
net/ethtool/netlink.c | 7 ++++++-
net/ethtool/netlink.h | 2 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index b4c45207fa32e..bb1a35494935f 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -758,7 +758,7 @@ static void ethnl_default_notify(struct net_device *dev, unsigned int cmd,
int reply_len;
int ret;
- genl_info_init_ntf(&info, ðtool_genl_family, cmd);
+ ethnl_info_init_ntf(&info, cmd);
if (WARN_ONCE(cmd > ETHTOOL_MSG_KERNEL_MAX ||
!ethnl_default_notify_ops[cmd],
@@ -825,6 +825,11 @@ static void ethnl_default_notify(struct net_device *dev, unsigned int cmd,
typedef void (*ethnl_notify_handler_t)(struct net_device *dev, unsigned int cmd,
const void *data);
+void ethnl_info_init_ntf(struct genl_info *info, u8 cmd)
+{
+ genl_info_init_ntf(info, ðtool_genl_family, cmd);
+}
+
static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
[ETHTOOL_MSG_LINKINFO_NTF] = ethnl_default_notify,
[ETHTOOL_MSG_LINKMODES_NTF] = ethnl_default_notify,
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index ff69ca0715dea..af20a175e1112 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -322,6 +322,8 @@ struct ethnl_sock_priv {
int ethnl_sock_priv_set(struct sk_buff *skb, struct net_device *dev, u32 portid,
enum ethnl_sock_type type);
+void ethnl_info_init_ntf(struct genl_info *info, u8 cmd);
+
/**
* struct ethnl_request_ops - unified handling of GET and SET requests
* @request_cmd: command id for request (GET)
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v6 02/12] net: pse-pd: Add support for reporting events
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
2025-03-04 10:18 ` [PATCH net-next v6 01/12] net: ethtool: Add support for ethnl_info_init_ntf helper function Kory Maincent
@ 2025-03-04 10:18 ` Kory Maincent
2025-03-07 1:43 ` Jakub Kicinski
2025-03-17 9:28 ` Oleksij Rempel
2025-03-04 10:18 ` [PATCH net-next v6 03/12] net: pse-pd: tps23881: Add support for PSE events and interrupts Kory Maincent
` (9 subsequent siblings)
11 siblings, 2 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:18 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project)
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
Add support for devm_pse_irq_helper() to register PSE interrupts and report
events such as over-current or over-temperature conditions. This follows a
similar approach to the regulator API but also sends notifications using a
dedicated PSE ethtool netlink socket.
Introduce an attached_phydev field in the pse_control structure to store
the phydev attached to the PSE PI, ensuring that PSE ethtool notifications
are sent to the correct network interface.
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
Change in v6:
- Update pse-ntf netlink to u32 instead of bitset.
- Update commit message.
Change in v4:
- Fix netlink notification message issues.
- Use netlink bitset in ethtool_pse_send_ntf.
- Add kdoc.
Change in v3:
- Remove C33 prefix when it is not in the standards.
- Fix pse_to_regulator_notifs which could not report regulator events
together.
- Fix deadlock issue.
- Save interrupt in pcdev structure for later use.
Change in v2:
- Add support for PSE ethtool notification.
- Saved the attached phy_device in the pse_control structure to know which
interface should have the notification.
- Rethink devm_pse_irq_helper() without devm_regulator_irq_helper() call.
---
Documentation/netlink/specs/ethtool.yaml | 21 ++++
Documentation/networking/ethtool-netlink.rst | 19 +++
drivers/net/mdio/fwnode_mdio.c | 26 ++--
drivers/net/pse-pd/pse_core.c | 157 ++++++++++++++++++++++++-
include/linux/ethtool_netlink.h | 9 ++
include/linux/pse-pd/pse.h | 24 +++-
include/uapi/linux/ethtool.h | 15 +++
include/uapi/linux/ethtool_netlink_generated.h | 9 ++
net/ethtool/pse-pd.c | 42 +++++++
9 files changed, 305 insertions(+), 17 deletions(-)
diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 655d8d10fe248..2417a8bf7f10b 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -1526,6 +1526,17 @@ attribute-sets:
name: hwtstamp-flags
type: nest
nested-attributes: bitset
+ -
+ name: pse-ntf
+ attr-cnt-name: __ethtool-a-pse-ntf-cnt
+ attributes:
+ -
+ name: header
+ type: nest
+ nested-attributes: header
+ -
+ name: events
+ type: u32
operations:
enum-model: directional
@@ -2382,3 +2393,13 @@ operations:
attributes: *tsconfig
reply:
attributes: *tsconfig
+ -
+ name: pse-ntf
+ doc: Notification for PSE events.
+
+ attribute-set: pse-ntf
+
+ event:
+ attributes:
+ - header
+ - events
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index b6e9af4d0f1b9..60f5ec7b80dda 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -290,6 +290,7 @@ Kernel to userspace:
``ETHTOOL_MSG_PHY_NTF`` Ethernet PHY information change
``ETHTOOL_MSG_TSCONFIG_GET_REPLY`` hw timestamping configuration
``ETHTOOL_MSG_TSCONFIG_SET_REPLY`` new hw timestamping configuration
+ ``ETHTOOL_MSG_PSE_NTF`` PSE events notification
======================================== =================================
``GET`` requests are sent by userspace applications to retrieve device
@@ -1896,6 +1897,24 @@ various existing products that document power consumption in watts rather than
classes. If power limit configuration based on classes is needed, the
conversion can be done in user space, for example by ethtool.
+PSE_NTF
+=======
+
+Notify PSE events.
+
+Notification contents:
+
+ =============================== ====== ========================
+ ``ETHTOOL_A_PSE_HEADER`` nested request header
+ ``ETHTOOL_A_PSE_EVENTS`` bitset PSE events
+ =============================== ====== ========================
+
+When set, the optional ``ETHTOOL_A_PSE_EVENTS`` attribute identifies the
+PSE events.
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+ :identifiers: ethtool_pse_events
+
RSS_GET
=======
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index aea0f03575689..9b41d4697a405 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -18,7 +18,8 @@ MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("FWNODE MDIO bus (Ethernet PHY) accessors");
static struct pse_control *
-fwnode_find_pse_control(struct fwnode_handle *fwnode)
+fwnode_find_pse_control(struct fwnode_handle *fwnode,
+ struct phy_device *phydev)
{
struct pse_control *psec;
struct device_node *np;
@@ -30,7 +31,7 @@ fwnode_find_pse_control(struct fwnode_handle *fwnode)
if (!np)
return NULL;
- psec = of_pse_control_get(np);
+ psec = of_pse_control_get(np, phydev);
if (PTR_ERR(psec) == -ENOENT)
return NULL;
@@ -128,15 +129,9 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
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)) {
- rc = PTR_ERR(mii_ts);
- goto clean_pse;
- }
+ if (IS_ERR(mii_ts))
+ return PTR_ERR(mii_ts);
is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45");
if (is_c45 || fwnode_get_phy_id(child, &phy_id))
@@ -169,6 +164,12 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
goto clean_phy;
}
+ psec = fwnode_find_pse_control(child, phy);
+ if (IS_ERR(psec)) {
+ rc = PTR_ERR(psec);
+ goto unregister_phy;
+ }
+
phy->psec = psec;
/* phy->mii_ts may already be defined by the PHY driver. A
@@ -180,12 +181,13 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
return 0;
+unregister_phy:
+ if (is_acpi_node(child) || is_of_node(child))
+ phy_device_remove(phy);
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/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 4602e26eb8c86..baccec984486c 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -7,6 +7,7 @@
#include <linux/device.h>
#include <linux/ethtool.h>
+#include <linux/ethtool_netlink.h>
#include <linux/of.h>
#include <linux/pse-pd/pse.h>
#include <linux/regulator/driver.h>
@@ -23,6 +24,7 @@ static LIST_HEAD(pse_controller_list);
* @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
+ * @attached_phydev: PHY device pointer attached by the PSE control
*/
struct pse_control {
struct pse_controller_dev *pcdev;
@@ -30,6 +32,7 @@ struct pse_control {
struct list_head list;
unsigned int id;
struct kref refcnt;
+ struct phy_device *attached_phydev;
};
static int of_load_single_pse_pi_pairset(struct device_node *node,
@@ -557,6 +560,151 @@ int devm_pse_controller_register(struct device *dev,
}
EXPORT_SYMBOL_GPL(devm_pse_controller_register);
+struct pse_irq {
+ struct pse_controller_dev *pcdev;
+ struct pse_irq_desc desc;
+ unsigned long *notifs;
+};
+
+/**
+ * pse_to_regulator_notifs - Convert PSE notifications to Regulator
+ * notifications
+ * @notifs: PSE notifications
+ *
+ * Return: Regulator notifications
+ */
+static unsigned long pse_to_regulator_notifs(unsigned long notifs)
+{
+ unsigned long rnotifs = 0;
+
+ if (notifs & ETHTOOL_PSE_EVENT_OVER_CURRENT)
+ rnotifs |= REGULATOR_EVENT_OVER_CURRENT;
+ if (notifs & ETHTOOL_PSE_EVENT_OVER_TEMP)
+ rnotifs |= REGULATOR_EVENT_OVER_TEMP;
+
+ return rnotifs;
+}
+
+/**
+ * pse_control_find_phy_by_id - Find PHY attached to the pse control id
+ * @pcdev: a pointer to the PSE
+ * @id: index of the PSE control
+ *
+ * Return: PHY device pointer or NULL
+ */
+static struct phy_device *
+pse_control_find_phy_by_id(struct pse_controller_dev *pcdev, int id)
+{
+ struct pse_control *psec;
+
+ mutex_lock(&pse_list_mutex);
+ list_for_each_entry(psec, &pcdev->pse_control_head, list) {
+ if (psec->id == id) {
+ mutex_unlock(&pse_list_mutex);
+ return psec->attached_phydev;
+ }
+ }
+ mutex_unlock(&pse_list_mutex);
+
+ return NULL;
+}
+
+/**
+ * pse_isr - IRQ handler for PSE
+ * @irq: irq number
+ * @data: pointer to user interrupt structure
+ *
+ * Return: irqreturn_t - status of IRQ
+ */
+static irqreturn_t pse_isr(int irq, void *data)
+{
+ struct netlink_ext_ack extack = {};
+ struct pse_controller_dev *pcdev;
+ unsigned long notifs_mask = 0;
+ struct pse_irq_desc *desc;
+ struct pse_irq *h = data;
+ int ret, i;
+
+ desc = &h->desc;
+ pcdev = h->pcdev;
+
+ /* Clear notifs mask */
+ memset(h->notifs, 0, pcdev->nr_lines * sizeof(*h->notifs));
+ mutex_lock(&pcdev->lock);
+ ret = desc->map_event(irq, pcdev, h->notifs, ¬ifs_mask);
+ mutex_unlock(&pcdev->lock);
+ if (ret || !notifs_mask)
+ return IRQ_NONE;
+
+ for_each_set_bit(i, ¬ifs_mask, pcdev->nr_lines) {
+ struct phy_device *phydev;
+ unsigned long notifs, rnotifs;
+
+ /* Do nothing PI not described */
+ if (!pcdev->pi[i].rdev)
+ continue;
+
+ notifs = h->notifs[i];
+ dev_dbg(h->pcdev->dev,
+ "Sending PSE notification EVT 0x%lx\n", notifs);
+
+ phydev = pse_control_find_phy_by_id(pcdev, i);
+ if (phydev)
+ ethnl_pse_send_ntf(phydev, notifs, &extack);
+ rnotifs = pse_to_regulator_notifs(notifs);
+ regulator_notifier_call_chain(pcdev->pi[i].rdev, rnotifs,
+ NULL);
+ }
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * devm_pse_irq_helper - Register IRQ based PSE event notifier
+ *
+ * @pcdev: a pointer to the PSE
+ * @irq: the irq value to be passed to request_irq
+ * @irq_flags: the flags to be passed to request_irq
+ * @d: PSE interrupt description
+ *
+ * Return: 0 on success and failure value on error
+ */
+int devm_pse_irq_helper(struct pse_controller_dev *pcdev, int irq,
+ int irq_flags, const struct pse_irq_desc *d)
+{
+ struct device *dev = pcdev->dev;
+ struct pse_irq *h;
+ int ret;
+
+ if (!d || !d->map_event || !d->name)
+ return -EINVAL;
+
+ h = devm_kzalloc(dev, sizeof(*h), GFP_KERNEL);
+ if (!h)
+ return -ENOMEM;
+
+ h->pcdev = pcdev;
+ h->desc = *d;
+ h->desc.name = devm_kstrdup(dev, d->name, GFP_KERNEL);
+ if (!h->desc.name)
+ return -ENOMEM;
+
+ h->notifs = devm_kcalloc(pcdev->dev, pcdev->nr_lines,
+ sizeof(*h->notifs), GFP_KERNEL);
+ if (!h->notifs)
+ return -ENOMEM;
+
+ ret = devm_request_threaded_irq(dev, irq, NULL, pse_isr,
+ IRQF_ONESHOT | irq_flags,
+ h->desc.name, h);
+ if (ret)
+ dev_err(pcdev->dev, "Failed to request IRQ %d\n", irq);
+
+ pcdev->irq = irq;
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devm_pse_irq_helper);
+
/* PSE control section */
static void __pse_control_release(struct kref *kref)
@@ -599,7 +747,8 @@ void pse_control_put(struct pse_control *psec)
EXPORT_SYMBOL_GPL(pse_control_put);
static struct pse_control *
-pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index)
+pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
+ struct phy_device *phydev)
{
struct pse_control *psec;
int ret;
@@ -638,6 +787,7 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index)
psec->pcdev = pcdev;
list_add(&psec->list, &pcdev->pse_control_head);
psec->id = index;
+ psec->attached_phydev = phydev;
kref_init(&psec->refcnt);
return psec;
@@ -693,7 +843,8 @@ static int psec_id_xlate(struct pse_controller_dev *pcdev,
return pse_spec->args[0];
}
-struct pse_control *of_pse_control_get(struct device_node *node)
+struct pse_control *of_pse_control_get(struct device_node *node,
+ struct phy_device *phydev)
{
struct pse_controller_dev *r, *pcdev;
struct of_phandle_args args;
@@ -743,7 +894,7 @@ struct pse_control *of_pse_control_get(struct device_node *node)
}
/* pse_list_mutex also protects the pcdev's pse_control list */
- psec = pse_control_get_internal(pcdev, psec_id);
+ psec = pse_control_get_internal(pcdev, psec_id, phydev);
out:
mutex_unlock(&pse_list_mutex);
diff --git a/include/linux/ethtool_netlink.h b/include/linux/ethtool_netlink.h
index aba91335273af..0fa1d8f59cf24 100644
--- a/include/linux/ethtool_netlink.h
+++ b/include/linux/ethtool_netlink.h
@@ -43,6 +43,9 @@ void ethtool_aggregate_rmon_stats(struct net_device *dev,
struct ethtool_rmon_stats *rmon_stats);
bool ethtool_dev_mm_supported(struct net_device *dev);
+void ethnl_pse_send_ntf(struct phy_device *phydev, unsigned long notif,
+ struct netlink_ext_ack *extack);
+
#else
static inline int ethnl_cable_test_alloc(struct phy_device *phydev, u8 cmd)
{
@@ -120,6 +123,12 @@ static inline bool ethtool_dev_mm_supported(struct net_device *dev)
return false;
}
+static inline void ethnl_pse_send_ntf(struct phy_device *phydev,
+ unsigned long notif,
+ struct netlink_ext_ack *extack)
+{
+}
+
#endif /* IS_ENABLED(CONFIG_ETHTOOL_NETLINK) */
static inline int ethnl_cable_test_result(struct phy_device *phydev, u8 pair,
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index c773eeb92d041..5d41a1c984bd4 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -7,6 +7,7 @@
#include <linux/list.h>
#include <uapi/linux/ethtool.h>
+#include <linux/regulator/driver.h>
/* Maximum current in uA according to IEEE 802.3-2022 Table 145-1 */
#define MAX_PI_CURRENT 1920000
@@ -37,6 +38,19 @@ struct ethtool_c33_pse_pw_limit_range {
u32 max;
};
+/**
+ * struct pse_irq_desc - notification sender description for IRQ based events.
+ *
+ * @name: the visible name for the IRQ
+ * @map_event: driver callback to map IRQ status into PSE devices with events.
+ */
+struct pse_irq_desc {
+ const char *name;
+ int (*map_event)(int irq, struct pse_controller_dev *pcdev,
+ unsigned long *notifs,
+ unsigned long *notifs_mask);
+};
+
/**
* struct pse_control_config - PSE control/channel configuration.
*
@@ -228,6 +242,7 @@ struct pse_pi {
* @types: types of the PSE controller
* @pi: table of PSE PIs described in this controller device
* @no_of_pse_pi: flag set if the pse_pis devicetree node is not used
+ * @irq: PSE interrupt
*/
struct pse_controller_dev {
const struct pse_controller_ops *ops;
@@ -241,6 +256,7 @@ struct pse_controller_dev {
enum ethtool_pse_types types;
struct pse_pi *pi;
bool no_of_pse_pi;
+ int irq;
};
#if IS_ENABLED(CONFIG_PSE_CONTROLLER)
@@ -249,8 +265,11 @@ void pse_controller_unregister(struct pse_controller_dev *pcdev);
struct device;
int devm_pse_controller_register(struct device *dev,
struct pse_controller_dev *pcdev);
+int devm_pse_irq_helper(struct pse_controller_dev *pcdev, int irq,
+ int irq_flags, const struct pse_irq_desc *d);
-struct pse_control *of_pse_control_get(struct device_node *node);
+struct pse_control *of_pse_control_get(struct device_node *node,
+ struct phy_device *phydev);
void pse_control_put(struct pse_control *psec);
int pse_ethtool_get_status(struct pse_control *psec,
@@ -268,7 +287,8 @@ bool pse_has_c33(struct pse_control *psec);
#else
-static inline struct pse_control *of_pse_control_get(struct device_node *node)
+static inline struct pse_control *of_pse_control_get(struct device_node *node,
+ struct phy_device *phydev)
{
return ERR_PTR(-ENOENT);
}
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 84833cca29fec..5e99daf42d440 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1002,6 +1002,21 @@ enum ethtool_c33_pse_pw_d_status {
ETHTOOL_C33_PSE_PW_D_STATUS_OTHERFAULT,
};
+/**
+ * enum ethtool_pse_events - event list of the PSE controller.
+ * @ETHTOOL_PSE_EVENT_OVER_CURRENT: PSE output current is too high.
+ * @ETHTOOL_PSE_EVENT_OVER_TEMP: PSE in over temperature state.
+ *
+ * @ETHTOOL_PSE_EVENT_LAST: Last PSE event of the enum.
+ */
+
+enum ethtool_pse_events {
+ ETHTOOL_PSE_EVENT_OVER_CURRENT = 1 << 0,
+ ETHTOOL_PSE_EVENT_OVER_TEMP = 1 << 1,
+
+ ETHTOOL_PSE_EVENT_LAST = ETHTOOL_PSE_EVENT_OVER_TEMP,
+};
+
/**
* enum ethtool_podl_pse_admin_state - operational state of the PoDL PSE
* functions. IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState
diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
index fe24c3459ac0f..549f01ea7a109 100644
--- a/include/uapi/linux/ethtool_netlink_generated.h
+++ b/include/uapi/linux/ethtool_netlink_generated.h
@@ -709,6 +709,14 @@ enum {
ETHTOOL_A_TSCONFIG_MAX = (__ETHTOOL_A_TSCONFIG_CNT - 1)
};
+enum {
+ ETHTOOL_A_PSE_NTF_HEADER = 1,
+ ETHTOOL_A_PSE_NTF_EVENTS,
+
+ __ETHTOOL_A_PSE_NTF_CNT,
+ ETHTOOL_A_PSE_NTF_MAX = (__ETHTOOL_A_PSE_NTF_CNT - 1)
+};
+
enum {
ETHTOOL_MSG_USER_NONE = 0,
ETHTOOL_MSG_STRSET_GET = 1,
@@ -813,6 +821,7 @@ enum {
ETHTOOL_MSG_PHY_NTF,
ETHTOOL_MSG_TSCONFIG_GET_REPLY,
ETHTOOL_MSG_TSCONFIG_SET_REPLY,
+ ETHTOOL_MSG_PSE_NTF,
__ETHTOOL_MSG_KERNEL_CNT,
ETHTOOL_MSG_KERNEL_MAX = (__ETHTOOL_MSG_KERNEL_CNT - 1)
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index 2819e2ba6be2d..6a9b18134736e 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -12,6 +12,7 @@
#include <linux/ethtool_netlink.h>
#include <linux/ethtool.h>
#include <linux/phy.h>
+#include "bitset.h"
struct pse_req_info {
struct ethnl_req_info base;
@@ -315,3 +316,44 @@ const struct ethnl_request_ops ethnl_pse_request_ops = {
.set = ethnl_set_pse,
/* PSE has no notification */
};
+
+void ethnl_pse_send_ntf(struct phy_device *phydev, unsigned long notifs,
+ struct netlink_ext_ack *extack)
+{
+ struct net_device *netdev = phydev->attached_dev;
+ struct genl_info info;
+ void *reply_payload;
+ struct sk_buff *skb;
+ int reply_len;
+ int ret;
+
+ if (!netdev || !notifs)
+ return;
+
+ ethnl_info_init_ntf(&info, ETHTOOL_MSG_PSE_NTF);
+ info.extack = extack;
+
+ reply_len = ethnl_reply_header_size() +
+ nla_total_size(sizeof(u32)); /* _PSE_NTF_EVENTS */
+
+ skb = genlmsg_new(reply_len, GFP_KERNEL);
+ reply_payload = ethnl_bcastmsg_put(skb, ETHTOOL_MSG_PSE_NTF);
+ if (!reply_payload)
+ goto err_skb;
+
+ ret = ethnl_fill_reply_header(skb, netdev,
+ ETHTOOL_A_PSE_NTF_HEADER);
+ if (ret < 0)
+ goto err_skb;
+
+ if (nla_put_u32(skb, ETHTOOL_A_PSE_NTF_EVENTS, notifs))
+ goto err_skb;
+
+ genlmsg_end(skb, reply_payload);
+ ethnl_multicast(skb, netdev);
+ return;
+
+err_skb:
+ nlmsg_free(skb);
+}
+EXPORT_SYMBOL_GPL(ethnl_pse_send_ntf);
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 02/12] net: pse-pd: Add support for reporting events
2025-03-04 10:18 ` [PATCH net-next v6 02/12] net: pse-pd: Add support for reporting events Kory Maincent
@ 2025-03-07 1:43 ` Jakub Kicinski
2025-03-07 9:08 ` Kory Maincent
2025-03-17 9:28 ` Oleksij Rempel
1 sibling, 1 reply; 36+ messages in thread
From: Jakub Kicinski @ 2025-03-07 1:43 UTC (permalink / raw)
To: Kory Maincent
Cc: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Tue, 04 Mar 2025 11:18:51 +0100 Kory Maincent wrote:
> + -
> + name: events
> + type: u32
type: uint
enum: your-enum-name
and you need to define the events like eg. c33-pse-ext-state
in the "definitions" section
BTW I was hoping you'd CC hwmon etc. Did you forget or some
of the CCed individuals are representing other subsystems?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 02/12] net: pse-pd: Add support for reporting events
2025-03-07 1:43 ` Jakub Kicinski
@ 2025-03-07 9:08 ` Kory Maincent
0 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-07 9:08 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Thu, 6 Mar 2025 17:43:45 -0800
Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue, 04 Mar 2025 11:18:51 +0100 Kory Maincent wrote:
> > + -
> > + name: events
> > + type: u32
>
> type: uint
> enum: your-enum-name
>
> and you need to define the events like eg. c33-pse-ext-state
> in the "definitions" section
Oh indeed, I forgot to use enum in the specs. Thanks for spotting it.
> BTW I was hoping you'd CC hwmon etc. Did you forget or some
> of the CCed individuals are representing other subsystems?
I have Cced Regulator maintainers Mark and Liam. hwmon are not related the
regulator events.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 02/12] net: pse-pd: Add support for reporting events
2025-03-04 10:18 ` [PATCH net-next v6 02/12] net: pse-pd: Add support for reporting events Kory Maincent
2025-03-07 1:43 ` Jakub Kicinski
@ 2025-03-17 9:28 ` Oleksij Rempel
2025-03-20 13:43 ` Kory Maincent
1 sibling, 1 reply; 36+ messages in thread
From: Oleksij Rempel @ 2025-03-17 9:28 UTC (permalink / raw)
To: Kory Maincent
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
Hi Köry,
sorry for late review.
On Tue, Mar 04, 2025 at 11:18:51AM +0100, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
...
> diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
> index aea0f03575689..9b41d4697a405 100644
> --- a/drivers/net/mdio/fwnode_mdio.c
> +++ b/drivers/net/mdio/fwnode_mdio.c
> @@ -18,7 +18,8 @@ MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("FWNODE MDIO bus (Ethernet PHY) accessors");
>
> static struct pse_control *
> -fwnode_find_pse_control(struct fwnode_handle *fwnode)
> +fwnode_find_pse_control(struct fwnode_handle *fwnode,
> + struct phy_device *phydev)
> {
> struct pse_control *psec;
> struct device_node *np;
> @@ -30,7 +31,7 @@ fwnode_find_pse_control(struct fwnode_handle *fwnode)
> if (!np)
> return NULL;
>
> - psec = of_pse_control_get(np);
> + psec = of_pse_control_get(np, phydev);
> if (PTR_ERR(psec) == -ENOENT)
> return NULL;
>
> @@ -128,15 +129,9 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> 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)) {
> - rc = PTR_ERR(mii_ts);
> - goto clean_pse;
> - }
> + if (IS_ERR(mii_ts))
> + return PTR_ERR(mii_ts);
>
> is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45");
> if (is_c45 || fwnode_get_phy_id(child, &phy_id))
> @@ -169,6 +164,12 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> goto clean_phy;
> }
>
> + psec = fwnode_find_pse_control(child, phy);
> + if (IS_ERR(psec)) {
> + rc = PTR_ERR(psec);
> + goto unregister_phy;
> + }
Hm... we are starting to dereference the phydev pointer to a different
framework without managing the ref-counting.
We will need to have some form of get_device(&phydev->mdio.dev).
Normally it is done by phy_attach_direct().
And the counterpart: put_device() or phy_device_free()
> static int of_load_single_pse_pi_pairset(struct device_node *node,
> @@ -557,6 +560,151 @@ int devm_pse_controller_register(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(devm_pse_controller_register);
>
> +struct pse_irq {
> + struct pse_controller_dev *pcdev;
> + struct pse_irq_desc desc;
> + unsigned long *notifs;
> +};
> +
> +/**
> + * pse_to_regulator_notifs - Convert PSE notifications to Regulator
> + * notifications
> + * @notifs: PSE notifications
> + *
> + * Return: Regulator notifications
> + */
> +static unsigned long pse_to_regulator_notifs(unsigned long notifs)
> +{
> + unsigned long rnotifs = 0;
> +
> + if (notifs & ETHTOOL_PSE_EVENT_OVER_CURRENT)
> + rnotifs |= REGULATOR_EVENT_OVER_CURRENT;
> + if (notifs & ETHTOOL_PSE_EVENT_OVER_TEMP)
> + rnotifs |= REGULATOR_EVENT_OVER_TEMP;
> +
> + return rnotifs;
> +}
> +
> +/**
> + * pse_control_find_phy_by_id - Find PHY attached to the pse control id
> + * @pcdev: a pointer to the PSE
> + * @id: index of the PSE control
> + *
> + * Return: PHY device pointer or NULL
> + */
> +static struct phy_device *
> +pse_control_find_phy_by_id(struct pse_controller_dev *pcdev, int id)
> +{
> + struct pse_control *psec;
> +
> + mutex_lock(&pse_list_mutex);
> + list_for_each_entry(psec, &pcdev->pse_control_head, list) {
> + if (psec->id == id) {
> + mutex_unlock(&pse_list_mutex);
> + return psec->attached_phydev;
> + }
> + }
> + mutex_unlock(&pse_list_mutex);
> +
Here we should increase ref-counting withing the mutex protection:
mutex_lock(&pse_list_mutex);
list_for_each_entry(psec, &pcdev->pse_control_head, list) {
if (psec->id == id) {
phydev = psec->attached_phydev;
if (phydev)
get_device(&phydev->mdio.dev); // Increase reference count
break;
}
}
mutex_unlock(&pse_list_mutex);
May be we will need a generic function for the PHYlib: phy_device_get() ?
--
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 |
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 02/12] net: pse-pd: Add support for reporting events
2025-03-17 9:28 ` Oleksij Rempel
@ 2025-03-20 13:43 ` Kory Maincent
0 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-20 13:43 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
Hello Oleksij,
On Mon, 17 Mar 2025 10:28:58 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> Hi Köry,
>
> sorry for late review.
No worry, thank you for the review! :)
> On Tue, Mar 04, 2025 at 11:18:51AM +0100, Kory Maincent wrote:
> > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> ...
> > diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
> > index aea0f03575689..9b41d4697a405 100644
> > --- a/drivers/net/mdio/fwnode_mdio.c
> > +++ b/drivers/net/mdio/fwnode_mdio.c
> > @@ -18,7 +18,8 @@ MODULE_LICENSE("GPL");
> > MODULE_DESCRIPTION("FWNODE MDIO bus (Ethernet PHY) accessors");
> >
> > static struct pse_control *
> > -fwnode_find_pse_control(struct fwnode_handle *fwnode)
> > +fwnode_find_pse_control(struct fwnode_handle *fwnode,
> > + struct phy_device *phydev)
> > {
> > struct pse_control *psec;
> > struct device_node *np;
> > @@ -30,7 +31,7 @@ fwnode_find_pse_control(struct fwnode_handle *fwnode)
> > if (!np)
> > return NULL;
> >
> > - psec = of_pse_control_get(np);
> > + psec = of_pse_control_get(np, phydev);
> > if (PTR_ERR(psec) == -ENOENT)
> > return NULL;
> >
> > @@ -128,15 +129,9 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> > 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)) {
> > - rc = PTR_ERR(mii_ts);
> > - goto clean_pse;
> > - }
> > + if (IS_ERR(mii_ts))
> > + return PTR_ERR(mii_ts);
> >
> > is_c45 = fwnode_device_is_compatible(child,
> > "ethernet-phy-ieee802.3-c45"); if (is_c45 || fwnode_get_phy_id(child,
> > &phy_id)) @@ -169,6 +164,12 @@ int fwnode_mdiobus_register_phy(struct
> > mii_bus *bus, goto clean_phy;
> > }
> >
> > + psec = fwnode_find_pse_control(child, phy);
> > + if (IS_ERR(psec)) {
> > + rc = PTR_ERR(psec);
> > + goto unregister_phy;
> > + }
>
> Hm... we are starting to dereference the phydev pointer to a different
> framework without managing the ref-counting.
>
> We will need to have some form of get_device(&phydev->mdio.dev).
> Normally it is done by phy_attach_direct().
>
> And the counterpart: put_device() or phy_device_free()
The thing is that the pse_control is already related to the PHY. It is created
(pse_control_get_internal) when a PHY is
registered (fwnode_mdiobus_register_phy), and removed when the PHY is removed
(phy_device_remove).
If we add a get_device it will increase the refcount of the PHY device but we
won't be able to decrease the refcount as it will never enter the remove
callback due to that refcount.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v6 03/12] net: pse-pd: tps23881: Add support for PSE events and interrupts
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
2025-03-04 10:18 ` [PATCH net-next v6 01/12] net: ethtool: Add support for ethnl_info_init_ntf helper function Kory Maincent
2025-03-04 10:18 ` [PATCH net-next v6 02/12] net: pse-pd: Add support for reporting events Kory Maincent
@ 2025-03-04 10:18 ` Kory Maincent
2025-03-17 9:43 ` Oleksij Rempel
2025-03-04 10:18 ` [PATCH net-next v6 04/12] net: pse-pd: Add support for PSE power domains Kory Maincent
` (8 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:18 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project)
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
Add support for PSE event reporting through interrupts. Set up the newly
introduced devm_pse_irq_helper helper to register the interrupt. Events are
reported for over-current and over-temperature conditions.
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
Change in v4:
- Small rename of a function.
Change in v3:
- Loop over interruption register to be sure the interruption pin is
freed before exiting the interrupt handler function.
- Add exist variable to not report event for undescribed PIs.
- Used helpers to convert the chan number to the PI port number.
Change in v2:
- Remove support for OSS pin and TPC23881 specific port priority management
---
drivers/net/pse-pd/tps23881.c | 178 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 177 insertions(+), 1 deletion(-)
diff --git a/drivers/net/pse-pd/tps23881.c b/drivers/net/pse-pd/tps23881.c
index 5e9dda2c0eac7..1226667192977 100644
--- a/drivers/net/pse-pd/tps23881.c
+++ b/drivers/net/pse-pd/tps23881.c
@@ -17,6 +17,13 @@
#define TPS23881_MAX_CHANS 8
+#define TPS23881_REG_IT 0x0
+#define TPS23881_REG_IT_MASK 0x1
+#define TPS23881_REG_IT_IFAULT BIT(5)
+#define TPS23881_REG_IT_SUPF BIT(7)
+#define TPS23881_REG_FAULT 0x7
+#define TPS23881_REG_SUPF_EVENT 0xb
+#define TPS23881_REG_TSD BIT(7)
#define TPS23881_REG_PW_STATUS 0x10
#define TPS23881_REG_OP_MODE 0x12
#define TPS23881_OP_MODE_SEMIAUTO 0xaaaa
@@ -24,6 +31,7 @@
#define TPS23881_REG_DET_CLA_EN 0x14
#define TPS23881_REG_GEN_MASK 0x17
#define TPS23881_REG_NBITACC BIT(5)
+#define TPS23881_REG_INTEN BIT(7)
#define TPS23881_REG_PW_EN 0x19
#define TPS23881_REG_2PAIR_POL1 0x1e
#define TPS23881_REG_PORT_MAP 0x26
@@ -51,6 +59,7 @@ struct tps23881_port_desc {
u8 chan[2];
bool is_4p;
int pw_pol;
+ bool exist;
};
struct tps23881_priv {
@@ -782,8 +791,10 @@ tps23881_write_port_matrix(struct tps23881_priv *priv,
hw_chan = port_matrix[i].hw_chan[0] % 4;
/* Set software port matrix for existing ports */
- if (port_matrix[i].exist)
+ if (port_matrix[i].exist) {
priv->port[pi_id].chan[0] = lgcl_chan;
+ priv->port[pi_id].exist = true;
+ }
/* Initialize power policy internal value */
priv->port[pi_id].pw_pol = -1;
@@ -1017,6 +1028,165 @@ static int tps23881_flash_sram_fw(struct i2c_client *client)
return 0;
}
+/* Convert interrupt events to 0xff to be aligned with the chan
+ * number.
+ */
+static u8 tps23881_irq_export_chans_helper(u16 reg_val, u8 field_offset)
+{
+ u8 val;
+
+ val = (reg_val >> (4 + field_offset) & 0xf0) |
+ (reg_val >> field_offset & 0x0f);
+
+ return val;
+}
+
+/* Convert chan number to port number */
+static void tps23881_set_notifs_helper(struct tps23881_priv *priv,
+ u8 chans,
+ unsigned long *notifs,
+ unsigned long *notifs_mask,
+ enum ethtool_pse_events event)
+{
+ u8 chan;
+ int i;
+
+ if (!chans)
+ return;
+
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ if (!priv->port[i].exist)
+ continue;
+ /* No need to look at the 2nd channel in case of PoE4 as
+ * both registers are set.
+ */
+ chan = priv->port[i].chan[0];
+
+ if (BIT(chan) & chans) {
+ *notifs_mask |= BIT(i);
+ notifs[i] |= event;
+ }
+ }
+}
+
+static void tps23881_irq_event_over_temp(struct tps23881_priv *priv,
+ u16 reg_val,
+ unsigned long *notifs,
+ unsigned long *notifs_mask)
+{
+ int i;
+
+ if (reg_val & TPS23881_REG_TSD) {
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ if (!priv->port[i].exist)
+ continue;
+
+ *notifs_mask |= BIT(i);
+ notifs[i] |= ETHTOOL_PSE_EVENT_OVER_TEMP;
+ }
+ }
+}
+
+static void tps23881_irq_event_over_current(struct tps23881_priv *priv,
+ u16 reg_val,
+ unsigned long *notifs,
+ unsigned long *notifs_mask)
+{
+ u8 chans;
+
+ chans = tps23881_irq_export_chans_helper(reg_val, 0);
+ if (chans)
+ tps23881_set_notifs_helper(priv, chans, notifs, notifs_mask,
+ ETHTOOL_PSE_EVENT_OVER_CURRENT);
+}
+
+static int tps23881_irq_event_handler(struct tps23881_priv *priv, u16 reg,
+ unsigned long *notifs,
+ unsigned long *notifs_mask)
+{
+ struct i2c_client *client = priv->client;
+ int ret;
+
+ /* The Supply event bit is repeated twice so we only need to read
+ * the one from the first byte.
+ */
+ if (reg & TPS23881_REG_IT_SUPF) {
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_SUPF_EVENT);
+ if (ret < 0)
+ return ret;
+ tps23881_irq_event_over_temp(priv, ret, notifs, notifs_mask);
+ }
+
+ if (reg & (TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_IFAULT << 8)) {
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_FAULT);
+ if (ret < 0)
+ return ret;
+ tps23881_irq_event_over_current(priv, ret, notifs, notifs_mask);
+ }
+
+ return 0;
+}
+
+static int tps23881_irq_handler(int irq, struct pse_controller_dev *pcdev,
+ unsigned long *notifs,
+ unsigned long *notifs_mask)
+{
+ struct tps23881_priv *priv = to_tps23881_priv(pcdev);
+ struct i2c_client *client = priv->client;
+ int ret, it_mask;
+
+ /* Get interruption mask */
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_IT_MASK);
+ if (ret < 0)
+ return ret;
+ it_mask = ret;
+
+ /* Read interrupt register until it frees the interruption pin. */
+ while (true) {
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_IT);
+ if (ret < 0)
+ return ret;
+
+ /* No more relevant interruption */
+ if (!(ret & it_mask))
+ return 0;
+
+ ret = tps23881_irq_event_handler(priv, (u16)ret, notifs,
+ notifs_mask);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
+static int tps23881_setup_irq(struct tps23881_priv *priv, int irq)
+{
+ struct i2c_client *client = priv->client;
+ struct pse_irq_desc irq_desc = {
+ .name = "tps23881-irq",
+ .map_event = tps23881_irq_handler,
+ };
+ int ret;
+ u16 val;
+
+ val = TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_SUPF;
+ val |= val << 8;
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_IT_MASK, val);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_GEN_MASK);
+ if (ret < 0)
+ return ret;
+
+ val = (u16)(ret | TPS23881_REG_INTEN | TPS23881_REG_INTEN << 8);
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_GEN_MASK, val);
+ if (ret < 0)
+ return ret;
+
+ return devm_pse_irq_helper(&priv->pcdev, irq, 0, &irq_desc);
+}
+
static int tps23881_i2c_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -1097,6 +1267,12 @@ static int tps23881_i2c_probe(struct i2c_client *client)
"failed to register PSE controller\n");
}
+ if (client->irq) {
+ ret = tps23881_setup_irq(priv, client->irq);
+ if (ret)
+ return ret;
+ }
+
return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 03/12] net: pse-pd: tps23881: Add support for PSE events and interrupts
2025-03-04 10:18 ` [PATCH net-next v6 03/12] net: pse-pd: tps23881: Add support for PSE events and interrupts Kory Maincent
@ 2025-03-17 9:43 ` Oleksij Rempel
2025-03-20 14:09 ` Kory Maincent
0 siblings, 1 reply; 36+ messages in thread
From: Oleksij Rempel @ 2025-03-17 9:43 UTC (permalink / raw)
To: Kory Maincent
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Tue, Mar 04, 2025 at 11:18:52AM +0100, Kory Maincent wrote:
> +static int tps23881_irq_handler(int irq, struct pse_controller_dev *pcdev,
> + unsigned long *notifs,
> + unsigned long *notifs_mask)
> +{
> + struct tps23881_priv *priv = to_tps23881_priv(pcdev);
> + struct i2c_client *client = priv->client;
> + int ret, it_mask;
> +
> + /* Get interruption mask */
> + ret = i2c_smbus_read_word_data(client, TPS23881_REG_IT_MASK);
> + if (ret < 0)
> + return ret;
> + it_mask = ret;
> +
> + /* Read interrupt register until it frees the interruption pin. */
> + while (true) {
If the hardware has a stuck interrupt, this could result in an infinite
loop. max_retries with some sane value would be good.
> + ret = i2c_smbus_read_word_data(client, TPS23881_REG_IT);
> + if (ret < 0)
> + return ret;
> +
> + /* No more relevant interruption */
> + if (!(ret & it_mask))
> + return 0;
> +
> + ret = tps23881_irq_event_handler(priv, (u16)ret, notifs,
> + notifs_mask);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int tps23881_setup_irq(struct tps23881_priv *priv, int irq)
> +{
> + struct i2c_client *client = priv->client;
> + struct pse_irq_desc irq_desc = {
> + .name = "tps23881-irq",
here or in devm_pse_irq_helper() it would be good to add intex suffix to
the irq handler name.
v> + .map_event = tps23881_irq_handler,
> + };
> + int ret;
> + u16 val;
> +
> + val = TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_SUPF;
> + val |= val << 8;
> + ret = i2c_smbus_write_word_data(client, TPS23881_REG_IT_MASK, val);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_read_word_data(client, TPS23881_REG_GEN_MASK);
> + if (ret < 0)
> + return ret;
> +
> + val = (u16)(ret | TPS23881_REG_INTEN | TPS23881_REG_INTEN << 8);
> + ret = i2c_smbus_write_word_data(client, TPS23881_REG_GEN_MASK, val);
> + if (ret < 0)
> + return ret;
> +
> + return devm_pse_irq_helper(&priv->pcdev, irq, 0, &irq_desc);
> +}
--
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 |
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 03/12] net: pse-pd: tps23881: Add support for PSE events and interrupts
2025-03-17 9:43 ` Oleksij Rempel
@ 2025-03-20 14:09 ` Kory Maincent
0 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-20 14:09 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Mon, 17 Mar 2025 10:43:58 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> On Tue, Mar 04, 2025 at 11:18:52AM +0100, Kory Maincent wrote:
> > +static int tps23881_irq_handler(int irq, struct pse_controller_dev *pcdev,
> > + unsigned long *notifs,
> > + unsigned long *notifs_mask)
> > +{
> > + struct tps23881_priv *priv = to_tps23881_priv(pcdev);
> > + struct i2c_client *client = priv->client;
> > + int ret, it_mask;
> > +
> > + /* Get interruption mask */
> > + ret = i2c_smbus_read_word_data(client, TPS23881_REG_IT_MASK);
> > + if (ret < 0)
> > + return ret;
> > + it_mask = ret;
> > +
> > + /* Read interrupt register until it frees the interruption pin. */
> > + while (true) {
>
> If the hardware has a stuck interrupt, this could result in an infinite
> loop. max_retries with some sane value would be good.
Ack I will. Do you have a value in mind as a sane value?
> > +
> > +static int tps23881_setup_irq(struct tps23881_priv *priv, int irq)
> > +{
> > + struct i2c_client *client = priv->client;
> > + struct pse_irq_desc irq_desc = {
> > + .name = "tps23881-irq",
>
> here or in devm_pse_irq_helper() it would be good to add intex suffix to
> the irq handler name.
You mean index? Like the PSE index?
So I will need to add back the support of the PSE index in the series.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v6 04/12] net: pse-pd: Add support for PSE power domains
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
` (2 preceding siblings ...)
2025-03-04 10:18 ` [PATCH net-next v6 03/12] net: pse-pd: tps23881: Add support for PSE events and interrupts Kory Maincent
@ 2025-03-04 10:18 ` Kory Maincent
2025-03-17 9:59 ` Oleksij Rempel
2025-03-04 10:18 ` [PATCH net-next v6 05/12] net: ethtool: Add support for new power domains index description Kory Maincent
` (7 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:18 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project)
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
Introduce PSE power domain support as groundwork for upcoming port
priority features. Multiple PSE PIs can now be grouped under a single
PSE power domain, enabling future enhancements like defining available
power budgets, port priority modes, and disconnection policies. This
setup will allow the system to assess whether activating a port would
exceed the available power budget, preventing over-budget states
proactively.
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
Changes in v6:
- nitpick change.
Changes in v4:
- Add kdoc.
- Fix null dereference in pse_flush_pw_ds function.
Changes in v3:
- Remove pw_budget variable.
Changes in v2:
- new patch.
---
drivers/net/pse-pd/pse_core.c | 113 ++++++++++++++++++++++++++++++++++++++++++
include/linux/pse-pd/pse.h | 2 +
2 files changed, 115 insertions(+)
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index baccec984486c..0f8a198f9f3b8 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -15,6 +15,7 @@
static DEFINE_MUTEX(pse_list_mutex);
static LIST_HEAD(pse_controller_list);
+static DEFINE_XARRAY_ALLOC(pse_pw_d_map);
/**
* struct pse_control - a PSE control
@@ -35,6 +36,16 @@ struct pse_control {
struct phy_device *attached_phydev;
};
+/**
+ * struct pse_power_domain - a PSE power domain
+ * @id: ID of the power domain
+ * @supply: Power supply the Power Domain
+ */
+struct pse_power_domain {
+ int id;
+ struct regulator *supply;
+};
+
static int of_load_single_pse_pi_pairset(struct device_node *node,
struct pse_pi *pi,
int pairset_num)
@@ -440,6 +451,103 @@ devm_pse_pi_regulator_register(struct pse_controller_dev *pcdev,
return 0;
}
+/**
+ * pse_flush_pw_ds - flush all PSE power domains of a PSE
+ * @pcdev: a pointer to the initialized PSE controller device
+ */
+static void pse_flush_pw_ds(struct pse_controller_dev *pcdev)
+{
+ struct pse_power_domain *pw_d;
+ int i;
+
+ for (i = 0; i < pcdev->nr_lines; i++) {
+ if (!pcdev->pi[i].pw_d)
+ continue;
+
+ pw_d = xa_load(&pse_pw_d_map, pcdev->pi[i].pw_d->id);
+ if (pw_d) {
+ regulator_put(pw_d->supply);
+ xa_erase(&pse_pw_d_map, pw_d->id);
+ }
+ }
+}
+
+/**
+ * devm_pse_alloc_pw_d - allocate a new PSE power domain for a device
+ * @dev: device that is registering this PSE power domain
+ *
+ * Return: Pointer to the newly allocated PSE power domain or error pointers
+ */
+static struct pse_power_domain *devm_pse_alloc_pw_d(struct device *dev)
+{
+ struct pse_power_domain *pw_d;
+ int index, ret;
+
+ pw_d = devm_kzalloc(dev, sizeof(*pw_d), GFP_KERNEL);
+ if (!pw_d)
+ return ERR_PTR(-ENOMEM);
+
+ ret = xa_alloc(&pse_pw_d_map, &index, pw_d, XA_LIMIT(1, INT_MAX), GFP_KERNEL);
+ if (ret)
+ return ERR_PTR(ret);
+
+ pw_d->id = index;
+ return pw_d;
+}
+
+/**
+ * pse_register_pw_ds - register the PSE power domains for a PSE
+ * @pcdev: a pointer to the PSE controller device
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int pse_register_pw_ds(struct pse_controller_dev *pcdev)
+{
+ int i;
+
+ for (i = 0; i < pcdev->nr_lines; i++) {
+ struct regulator_dev *rdev = pcdev->pi[i].rdev;
+ struct pse_power_domain *pw_d;
+ struct regulator *supply;
+ bool present = false;
+ unsigned long index;
+
+ /* No regulator or regulator parent supply registered.
+ * We need a regulator parent to register a PSE power domain
+ */
+ if (!rdev || !rdev->supply)
+ continue;
+
+ xa_for_each(&pse_pw_d_map, index, pw_d) {
+ /* Power supply already registered as a PSE power
+ * domain.
+ */
+ if (regulator_is_equal(pw_d->supply, rdev->supply)) {
+ present = true;
+ pcdev->pi[i].pw_d = pw_d;
+ break;
+ }
+ }
+ if (present)
+ continue;
+
+ pw_d = devm_pse_alloc_pw_d(pcdev->dev);
+ if (IS_ERR_OR_NULL(pw_d))
+ return PTR_ERR(pw_d);
+
+ supply = regulator_get(&rdev->dev, rdev->supply_name);
+ if (IS_ERR(supply)) {
+ xa_erase(&pse_pw_d_map, pw_d->id);
+ return PTR_ERR(supply);
+ }
+
+ pw_d->supply = supply;
+ pcdev->pi[i].pw_d = pw_d;
+ }
+
+ return 0;
+}
+
/**
* pse_controller_register - register a PSE controller device
* @pcdev: a pointer to the initialized PSE controller device
@@ -499,6 +607,10 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
return ret;
}
+ ret = pse_register_pw_ds(pcdev);
+ if (ret)
+ return ret;
+
mutex_lock(&pse_list_mutex);
list_add(&pcdev->list, &pse_controller_list);
mutex_unlock(&pse_list_mutex);
@@ -513,6 +625,7 @@ EXPORT_SYMBOL_GPL(pse_controller_register);
*/
void pse_controller_unregister(struct pse_controller_dev *pcdev)
{
+ pse_flush_pw_ds(pcdev);
pse_release_pis(pcdev);
mutex_lock(&pse_list_mutex);
list_del(&pcdev->list);
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 5d41a1c984bd4..5201a0fb3d744 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -220,12 +220,14 @@ struct pse_pi_pairset {
* @np: device node pointer of the PSE PI node
* @rdev: regulator represented by the PSE PI
* @admin_state_enabled: PI enabled state
+ * @pw_d: Power domain of the PSE PI
*/
struct pse_pi {
struct pse_pi_pairset pairset[2];
struct device_node *np;
struct regulator_dev *rdev;
bool admin_state_enabled;
+ struct pse_power_domain *pw_d;
};
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 04/12] net: pse-pd: Add support for PSE power domains
2025-03-04 10:18 ` [PATCH net-next v6 04/12] net: pse-pd: Add support for PSE power domains Kory Maincent
@ 2025-03-17 9:59 ` Oleksij Rempel
2025-03-20 15:46 ` Kory Maincent
0 siblings, 1 reply; 36+ messages in thread
From: Oleksij Rempel @ 2025-03-17 9:59 UTC (permalink / raw)
To: Kory Maincent
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Tue, Mar 04, 2025 at 11:18:53AM +0100, Kory Maincent wrote:
> +/**
> + * pse_flush_pw_ds - flush all PSE power domains of a PSE
> + * @pcdev: a pointer to the initialized PSE controller device
> + */
> +static void pse_flush_pw_ds(struct pse_controller_dev *pcdev)
> +{
> + struct pse_power_domain *pw_d;
> + int i;
> +
> + for (i = 0; i < pcdev->nr_lines; i++) {
> + if (!pcdev->pi[i].pw_d)
> + continue;
> +
> + pw_d = xa_load(&pse_pw_d_map, pcdev->pi[i].pw_d->id);
> + if (pw_d) {
> + regulator_put(pw_d->supply);
> + xa_erase(&pse_pw_d_map, pw_d->id);
> + }
> + }
> +}
> +
> +/**
> + * devm_pse_alloc_pw_d - allocate a new PSE power domain for a device
> + * @dev: device that is registering this PSE power domain
> + *
> + * Return: Pointer to the newly allocated PSE power domain or error pointers
> + */
> +static struct pse_power_domain *devm_pse_alloc_pw_d(struct device *dev)
> +{
> + struct pse_power_domain *pw_d;
> + int index, ret;
> +
> + pw_d = devm_kzalloc(dev, sizeof(*pw_d), GFP_KERNEL);
> + if (!pw_d)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = xa_alloc(&pse_pw_d_map, &index, pw_d, XA_LIMIT(1, INT_MAX), GFP_KERNEL);
#define PSE_PW_D_LIMIT INT_MAX
XA_LIMIT(1, PSE_PW_D_LIMIT)
> + if (ret)
> + return ERR_PTR(ret);
> +
> + pw_d->id = index;
> + return pw_d;
> +}
> +
> +/**
> + * pse_register_pw_ds - register the PSE power domains for a PSE
> + * @pcdev: a pointer to the PSE controller device
> + *
> + * Return: 0 on success and failure value on error
> + */
> +static int pse_register_pw_ds(struct pse_controller_dev *pcdev)
> +{
> + int i;
> +
> + for (i = 0; i < pcdev->nr_lines; i++) {
> + struct regulator_dev *rdev = pcdev->pi[i].rdev;
> + struct pse_power_domain *pw_d;
> + struct regulator *supply;
> + bool present = false;
> + unsigned long index;
> +
> + /* No regulator or regulator parent supply registered.
> + * We need a regulator parent to register a PSE power domain
> + */
> + if (!rdev || !rdev->supply)
> + continue;
> +
Should we use xa_lock() before iteration over the map?
> + xa_for_each(&pse_pw_d_map, index, pw_d) {
> + /* Power supply already registered as a PSE power
> + * domain.
> + */
> + if (regulator_is_equal(pw_d->supply, rdev->supply)) {
> + present = true;
> + pcdev->pi[i].pw_d = pw_d;
> + break;
> + }
> + }
> + if (present)
> + continue;
> +
> + pw_d = devm_pse_alloc_pw_d(pcdev->dev);
> + if (IS_ERR_OR_NULL(pw_d))
> + return PTR_ERR(pw_d);
It is better to break the loop and roll back previous allocations.
> +
> + supply = regulator_get(&rdev->dev, rdev->supply_name);
> + if (IS_ERR(supply)) {
> + xa_erase(&pse_pw_d_map, pw_d->id);
> + return PTR_ERR(supply);
same here.
> + }
> +
> + pw_d->supply = supply;
> + pcdev->pi[i].pw_d = pw_d;
> + }
> +
> + return 0;
> +}
--
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 |
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 04/12] net: pse-pd: Add support for PSE power domains
2025-03-17 9:59 ` Oleksij Rempel
@ 2025-03-20 15:46 ` Kory Maincent
0 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-20 15:46 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Mon, 17 Mar 2025 10:59:02 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> On Tue, Mar 04, 2025 at 11:18:53AM +0100, Kory Maincent wrote:
> > +static int pse_register_pw_ds(struct pse_controller_dev *pcdev)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < pcdev->nr_lines; i++) {
> > + struct regulator_dev *rdev = pcdev->pi[i].rdev;
> > + struct pse_power_domain *pw_d;
> > + struct regulator *supply;
> > + bool present = false;
> > + unsigned long index;
> > +
> > + /* No regulator or regulator parent supply registered.
> > + * We need a regulator parent to register a PSE power
> > domain
> > + */
> > + if (!rdev || !rdev->supply)
> > + continue;
> > +
>
> Should we use xa_lock() before iteration over the map?
I am scratching my head to find out the case where it may be needed.
The only case I think of is two PSE controller with PSE PI using the same power
supply. Which is not a good idea for well distributing power.
In this case we could have two pse_register_pw_ds running concurrently and we
could have issues. Not only the iteration over the map should be protected but
also the content of the pw_d entry. Also when we unbind one of the PSE
controller it will remove all the PSE power domains event if they are used
by another PSE. :/
I need to add refcount and lock on the PSE power domains to fix this case!
> > + xa_for_each(&pse_pw_d_map, index, pw_d) {
> > + /* Power supply already registered as a PSE power
> > + * domain.
> > + */
> > + if (regulator_is_equal(pw_d->supply,
> > rdev->supply)) {
> > + present = true;
> > + pcdev->pi[i].pw_d = pw_d;
> > + break;
> > + }
> > + }
> > + if (present)
> > + continue;
> > +
> > + pw_d = devm_pse_alloc_pw_d(pcdev->dev);
> > + if (IS_ERR_OR_NULL(pw_d))
> > + return PTR_ERR(pw_d);
>
> It is better to break the loop and roll back previous allocations.
This will be done by pse_flush_pw_ds called by devm_pse_controller_release as
the pse_controller_register failed.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v6 05/12] net: ethtool: Add support for new power domains index description
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
` (3 preceding siblings ...)
2025-03-04 10:18 ` [PATCH net-next v6 04/12] net: pse-pd: Add support for PSE power domains Kory Maincent
@ 2025-03-04 10:18 ` Kory Maincent
2025-03-04 10:18 ` [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies Kory Maincent
` (6 subsequent siblings)
11 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:18 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project)
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
Report the index of the newly introduced PSE power domain to the user,
enabling improved management of the power budget for PSE devices.
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
Changes in v3:
- Do not support power domain id = 0 because we can't differentiate with
no PSE power domain.
Changes in v2:
- new patch.
---
Documentation/netlink/specs/ethtool.yaml | 5 +++++
Documentation/networking/ethtool-netlink.rst | 4 ++++
drivers/net/pse-pd/pse_core.c | 3 +++
include/linux/pse-pd/pse.h | 2 ++
include/uapi/linux/ethtool_netlink_generated.h | 1 +
net/ethtool/pse-pd.c | 7 +++++++
6 files changed, 22 insertions(+)
diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 2417a8bf7f10b..cad52cf43b938 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -1366,6 +1366,10 @@ attribute-sets:
type: nest
multi-attr: true
nested-attributes: c33-pse-pw-limit
+ -
+ name: pse-pw-d-id
+ type: u32
+ name-prefix: ethtool-a-
-
name: rss
attr-cnt-name: __ethtool-a-rss-cnt
@@ -2185,6 +2189,7 @@ operations:
- c33-pse-ext-substate
- c33-pse-avail-pw-limit
- c33-pse-pw-limit-ranges
+ - pse-pw-d-id
dump: *pse-get-op
-
name: pse-set
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 60f5ec7b80dda..49c9a0e79af56 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1789,6 +1789,7 @@ Kernel response contents:
limit of the PoE PSE.
``ETHTOOL_A_C33_PSE_PW_LIMIT_RANGES`` nested Supported power limit
configuration ranges.
+ ``ETHTOOL_A_PSE_PW_D_ID`` u32 Index of the PSE power domain
========================================== ====== =============================
When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` attribute identifies
@@ -1862,6 +1863,9 @@ identifies the C33 PSE power limit ranges through
If the controller works with fixed classes, the min and max values will be
equal.
+The ``ETHTOOL_A_PSE_PW_D_ID`` attribute identifies the index of PSE power
+domain.
+
PSE_SET
=======
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 0f8a198f9f3b8..cb3477e4bd67b 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -1038,6 +1038,9 @@ int pse_ethtool_get_status(struct pse_control *psec,
pcdev = psec->pcdev;
ops = pcdev->ops;
mutex_lock(&pcdev->lock);
+ if (pcdev->pi[psec->id].pw_d)
+ status->pw_d_id = pcdev->pi[psec->id].pw_d->id;
+
ret = ops->pi_get_admin_state(pcdev, psec->id, &admin_state);
if (ret)
goto out;
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 5201a0fb3d744..ffa6cf9a0072f 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -112,6 +112,7 @@ struct pse_pw_limit_ranges {
/**
* struct ethtool_pse_control_status - PSE control/channel status.
*
+ * @pw_d_id: PSE power domain index.
* @podl_admin_state: operational state of the PoDL PSE
* functions. IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState
* @podl_pw_status: power detection status of the PoDL PSE.
@@ -133,6 +134,7 @@ struct pse_pw_limit_ranges {
* ranges
*/
struct ethtool_pse_control_status {
+ u32 pw_d_id;
enum ethtool_podl_pse_admin_state podl_admin_state;
enum ethtool_podl_pse_pw_d_status podl_pw_status;
enum ethtool_c33_pse_admin_state c33_admin_state;
diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
index 549f01ea7a109..ccd7ab3bf1b11 100644
--- a/include/uapi/linux/ethtool_netlink_generated.h
+++ b/include/uapi/linux/ethtool_netlink_generated.h
@@ -633,6 +633,7 @@ enum {
ETHTOOL_A_C33_PSE_EXT_SUBSTATE,
ETHTOOL_A_C33_PSE_AVAIL_PW_LIMIT,
ETHTOOL_A_C33_PSE_PW_LIMIT_RANGES,
+ ETHTOOL_A_PSE_PW_D_ID,
__ETHTOOL_A_PSE_CNT,
ETHTOOL_A_PSE_MAX = (__ETHTOOL_A_PSE_CNT - 1)
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index 6a9b18134736e..950c1d4ef4e06 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -84,6 +84,8 @@ static int pse_reply_size(const struct ethnl_req_info *req_base,
const struct ethtool_pse_control_status *st = &data->status;
int len = 0;
+ if (st->pw_d_id > 0)
+ len += nla_total_size(sizeof(u32)); /* _PSE_PW_D_ID */
if (st->podl_admin_state > 0)
len += nla_total_size(sizeof(u32)); /* _PODL_PSE_ADMIN_STATE */
if (st->podl_pw_status > 0)
@@ -149,6 +151,11 @@ static int pse_fill_reply(struct sk_buff *skb,
const struct pse_reply_data *data = PSE_REPDATA(reply_base);
const struct ethtool_pse_control_status *st = &data->status;
+ if (st->pw_d_id > 0 &&
+ nla_put_u32(skb, ETHTOOL_A_PSE_PW_D_ID,
+ st->pw_d_id))
+ return -EMSGSIZE;
+
if (st->podl_admin_state > 0 &&
nla_put_u32(skb, ETHTOOL_A_PODL_PSE_ADMIN_STATE,
st->podl_admin_state))
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
` (4 preceding siblings ...)
2025-03-04 10:18 ` [PATCH net-next v6 05/12] net: ethtool: Add support for new power domains index description Kory Maincent
@ 2025-03-04 10:18 ` Kory Maincent
2025-03-07 1:46 ` Jakub Kicinski
2025-03-17 12:40 ` Oleksij Rempel
2025-03-04 10:18 ` [PATCH net-next v6 07/12] net: ethtool: Add PSE new budget evaluation strategy support feature Kory Maincent
` (5 subsequent siblings)
11 siblings, 2 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:18 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project)
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
This patch introduces the ability to configure the PSE PI budget evaluation
strategies. Budget evaluation strategies is utilized by PSE controllers to
determine which ports to turn off first in scenarios such as power budget
exceedance.
The pis_prio_max value is used to define the maximum priority level
supported by the controller. Both the current priority and the maximum
priority are exposed to the user through the pse_ethtool_get_status call.
This patch add support for two mode of budget evaluation strategies.
1. Static Method:
This method involves distributing power based on PD classification.
It’s straightforward and stable, the PSE core keeping track of the
budget and subtracting the power requested by each PD’s class.
Advantages: Every PD gets its promised power at any time, which
guarantees reliability.
Disadvantages: PD classification steps are large, meaning devices
request much more power than they actually need. As a result, the power
supply may only operate at, say, 50% capacity, which is inefficient and
wastes money.
Priority max value is matching the number of PSE PIs within the PSE.
2. Dynamic Method:
To address the inefficiencies of the static method, vendors like
Microchip have introduced dynamic power budgeting, as seen in the
PD692x0 firmware. This method monitors the current consumption per port
and subtracts it from the available power budget. When the budget is
exceeded, lower-priority ports are shut down.
Advantages: This method optimizes resource utilization, saving costs.
Disadvantages: Low-priority devices may experience instability.
Priority max value is set by the PSE controller driver.
For now, budget evaluation methods are not configurable and cannot be
mixed. They are hardcoded in the PSE driver itself, as no current PSE
controller supports both methods.
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
Change in v6:
- Remove Budget evaluation strategy from ethtool_pse_control_status struct.
Change in v5:
- Save PI previous power allocated in set current limit to be able to
restore the power allocated in case of error.
Change in v4:
- Remove disconnection policy features.
- Rename port priority to budget evaluation strategy.
- Add kdoc
Change in v3:
- Add disconnection policy.
- Add management of disabled port priority in the interrupt handler.
- Move port prio mode in the power domain instead of the PSE.
Change in v2:
- Rethink the port priority support.
---
drivers/net/pse-pd/pse_core.c | 624 ++++++++++++++++++++++++++++++++++++++----
include/linux/pse-pd/pse.h | 44 +++
include/uapi/linux/ethtool.h | 39 ++-
3 files changed, 660 insertions(+), 47 deletions(-)
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index cb3477e4bd67b..e5b40e684fff5 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -40,10 +40,13 @@ struct pse_control {
* struct pse_power_domain - a PSE power domain
* @id: ID of the power domain
* @supply: Power supply the Power Domain
+ * @budget_eval_strategy: Current power budget evaluation strategy of the
+ * power domain
*/
struct pse_power_domain {
int id;
struct regulator *supply;
+ u32 budget_eval_strategy;
};
static int of_load_single_pse_pi_pairset(struct device_node *node,
@@ -222,6 +225,29 @@ static int of_load_pse_pis(struct pse_controller_dev *pcdev)
return ret;
}
+/**
+ * pse_pw_d_is_sw_pw_control - Is power control software managed
+ * @pcdev: a pointer to the PSE controller device
+ * @pw_d: a pointer to the PSE power domain
+ *
+ * Return: true if the power control of the power domain is managed from
+ * the software in the interrupt handler
+ */
+static bool pse_pw_d_is_sw_pw_control(struct pse_controller_dev *pcdev,
+ struct pse_power_domain *pw_d)
+{
+ if (!pw_d)
+ return false;
+
+ if (pw_d->budget_eval_strategy == ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC)
+ return true;
+ if (pw_d->budget_eval_strategy == ETHTOOL_PSE_BUDGET_EVAL_STRAT_DISABLED &&
+ pcdev->ops->pi_enable && pcdev->irq)
+ return true;
+
+ return false;
+}
+
static int pse_pi_is_enabled(struct regulator_dev *rdev)
{
struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
@@ -235,6 +261,11 @@ static int pse_pi_is_enabled(struct regulator_dev *rdev)
id = rdev_get_id(rdev);
mutex_lock(&pcdev->lock);
+ if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[id].pw_d)) {
+ ret = pcdev->pi[id].admin_state_enabled;
+ goto out;
+ }
+
ret = ops->pi_get_admin_state(pcdev, id, &admin_state);
if (ret)
goto out;
@@ -249,11 +280,260 @@ static int pse_pi_is_enabled(struct regulator_dev *rdev)
return ret;
}
+/**
+ * pse_pi_deallocate_pw_budget - Deallocate power budget of the PI
+ * @pi: a pointer to the PSE PI
+ */
+static void pse_pi_deallocate_pw_budget(struct pse_pi *pi)
+{
+ if (!pi->pw_d || !pi->pw_allocated_mW)
+ return;
+
+ regulator_free_power_budget(pi->pw_d->supply, pi->pw_allocated_mW);
+ pi->pw_allocated_mW = 0;
+}
+
+/**
+ * _pse_pi_disable - Call disable operation. Assumes the PSE lock has been
+ * acquired.
+ * @pcdev: a pointer to the PSE
+ * @id: index of the PSE control
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int _pse_pi_disable(struct pse_controller_dev *pcdev, int id)
+{
+ const struct pse_controller_ops *ops = pcdev->ops;
+ int ret;
+
+ if (!ops->pi_disable)
+ return -EOPNOTSUPP;
+
+ ret = ops->pi_disable(pcdev, id);
+ if (ret)
+ return ret;
+
+ pse_pi_deallocate_pw_budget(&pcdev->pi[id]);
+
+ return 0;
+}
+
+/**
+ * pse_control_find_phy_by_id - Find PHY attached to the pse control id
+ * @pcdev: a pointer to the PSE
+ * @id: index of the PSE control
+ *
+ * Return: PHY device pointer or NULL
+ */
+static struct phy_device *
+pse_control_find_phy_by_id(struct pse_controller_dev *pcdev, int id)
+{
+ struct pse_control *psec;
+
+ mutex_lock(&pse_list_mutex);
+ list_for_each_entry(psec, &pcdev->pse_control_head, list) {
+ if (psec->id == id) {
+ mutex_unlock(&pse_list_mutex);
+ return psec->attached_phydev;
+ }
+ }
+ mutex_unlock(&pse_list_mutex);
+ return NULL;
+}
+
+/**
+ * pse_disable_pi_pol - Disable a PI on a power budget policy
+ * @pcdev: a pointer to the PSE
+ * @id: index of the PSE PI
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int pse_disable_pi_pol(struct pse_controller_dev *pcdev, int id)
+{
+ unsigned long notifs = ETHTOOL_PSE_EVENT_OVER_BUDGET;
+ struct netlink_ext_ack extack = {};
+ struct phy_device *phydev;
+ int ret;
+
+ dev_dbg(pcdev->dev, "Disabling PI %d to free power budget\n", id);
+
+ NL_SET_ERR_MSG_FMT(&extack,
+ "Disabling PI %d to free power budget", id);
+
+ ret = _pse_pi_disable(pcdev, id);
+ if (ret) {
+ notifs |= ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR;
+ } else {
+ pcdev->pi[id].admin_state_enabled = 0;
+ pcdev->pi[id]._isr_counter_mismatch = 1;
+ }
+
+ phydev = pse_control_find_phy_by_id(pcdev, id);
+ if (phydev)
+ ethnl_pse_send_ntf(phydev, notifs, &extack);
+
+ return ret;
+}
+
+/**
+ * pse_disable_pi_prio - Disable all PIs of a given priority inside a PSE
+ * power domain
+ * @pcdev: a pointer to the PSE
+ * @pw_d: a pointer to the PSE power domain
+ * @prio: priority
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int pse_disable_pi_prio(struct pse_controller_dev *pcdev,
+ struct pse_power_domain *pw_d,
+ int prio)
+{
+ int i;
+
+ for (i = 0; i < pcdev->nr_lines; i++) {
+ int ret;
+
+ if (pcdev->pi[i].prio != prio ||
+ pcdev->pi[i].pw_d != pw_d ||
+ !pcdev->pi[i].admin_state_enabled)
+ continue;
+
+ ret = pse_disable_pi_pol(pcdev, i);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * pse_pi_allocate_pw_budget_static_prio - Allocate power budget for the PI
+ * when the budget eval strategy is
+ * static
+ * @pcdev: a pointer to the PSE
+ * @id: index of the PSE control
+ * @pw_req: power requested in mW
+ * @extack: extack for error reporting
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int
+pse_pi_allocate_pw_budget_static_prio(struct pse_controller_dev *pcdev, int id,
+ int pw_req, struct netlink_ext_ack *extack)
+{
+ struct pse_pi *pi = &pcdev->pi[id];
+ int ret, _prio;
+
+ _prio = pcdev->nr_lines;
+ while (regulator_request_power_budget(pi->pw_d->supply, pw_req) == -ERANGE) {
+ if (_prio <= pi->prio) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "PI %d: not enough power budget available",
+ id);
+ return -ERANGE;
+ }
+
+ ret = pse_disable_pi_prio(pcdev, pi->pw_d, _prio);
+ if (ret < 0)
+ return ret;
+
+ _prio--;
+ }
+
+ pi->pw_allocated_mW = pw_req;
+ return 0;
+}
+
+/**
+ * pse_pi_allocate_pw_budget - Allocate power budget for the PI
+ * @pcdev: a pointer to the PSE
+ * @id: index of the PSE control
+ * @pw_req: power requested in mW
+ * @extack: extack for error reporting
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int pse_pi_allocate_pw_budget(struct pse_controller_dev *pcdev, int id,
+ int pw_req, struct netlink_ext_ack *extack)
+{
+ struct pse_pi *pi = &pcdev->pi[id];
+
+ if (!pi->pw_d)
+ return 0;
+
+ /* ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC */
+ if (pi->pw_d->budget_eval_strategy == ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC)
+ return pse_pi_allocate_pw_budget_static_prio(pcdev, id, pw_req,
+ extack);
+
+ return 0;
+}
+
+/**
+ * _pse_pi_enable_sw_pw_ctrl - Enable PSE PI in case of software power control.
+ * Assumes the PSE lock has been acquired
+ * @pcdev: a pointer to the PSE
+ * @id: index of the PSE control
+ * @extack: extack for error reporting
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int _pse_pi_enable_sw_pw_ctrl(struct pse_controller_dev *pcdev, int id,
+ struct netlink_ext_ack *extack)
+{
+ const struct pse_controller_ops *ops = pcdev->ops;
+ struct pse_pi *pi = &pcdev->pi[id];
+ int ret, pw_req;
+
+ if (!ops->pi_get_pw_req) {
+ /* No power allocation management */
+ ret = ops->pi_enable(pcdev, id);
+ if (ret)
+ NL_SET_ERR_MSG_FMT(extack,
+ "PI %d: enable error %d",
+ id, ret);
+ return ret;
+ }
+
+ ret = ops->pi_get_pw_req(pcdev, id);
+ if (ret < 0)
+ return ret;
+
+ pw_req = ret;
+
+ /* Compare requested power with port power limit and use the lowest
+ * one.
+ */
+ if (ops->pi_get_pw_limit) {
+ ret = ops->pi_get_pw_limit(pcdev, id);
+ if (ret < 0)
+ return ret;
+
+ if (ret < pw_req)
+ pw_req = ret;
+ }
+
+ ret = pse_pi_allocate_pw_budget(pcdev, id, pw_req, extack);
+ if (ret)
+ return ret;
+
+ ret = ops->pi_enable(pcdev, id);
+ if (ret) {
+ pse_pi_deallocate_pw_budget(pi);
+ NL_SET_ERR_MSG_FMT(extack,
+ "PI %d: enable error %d",
+ id, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
static int pse_pi_enable(struct regulator_dev *rdev)
{
struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
const struct pse_controller_ops *ops;
- int id, ret;
+ int id, ret = 0;
ops = pcdev->ops;
if (!ops->pi_enable)
@@ -261,6 +541,23 @@ static int pse_pi_enable(struct regulator_dev *rdev)
id = rdev_get_id(rdev);
mutex_lock(&pcdev->lock);
+ if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[id].pw_d)) {
+ /* Manage enabled status by software.
+ * Real enable process will happen if a port is connected.
+ */
+ if (pcdev->pi[id].isr_pd_detected) {
+ struct netlink_ext_ack extack;
+
+ ret = _pse_pi_enable_sw_pw_ctrl(pcdev, id, &extack);
+ if (!ret)
+ pcdev->pi[id].admin_state_enabled = 1;
+ } else {
+ pcdev->pi[id].admin_state_enabled = 1;
+ }
+ mutex_unlock(&pcdev->lock);
+ return ret;
+ }
+
ret = ops->pi_enable(pcdev, id);
if (!ret)
pcdev->pi[id].admin_state_enabled = 1;
@@ -272,21 +569,20 @@ static int pse_pi_enable(struct regulator_dev *rdev)
static int pse_pi_disable(struct regulator_dev *rdev)
{
struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
- const struct pse_controller_ops *ops;
+ struct pse_pi *pi;
int id, ret;
- ops = pcdev->ops;
- if (!ops->pi_disable)
- return -EOPNOTSUPP;
-
id = rdev_get_id(rdev);
+ pi = &pcdev->pi[id];
mutex_lock(&pcdev->lock);
- ret = ops->pi_disable(pcdev, id);
- if (!ret)
- pcdev->pi[id].admin_state_enabled = 0;
- mutex_unlock(&pcdev->lock);
+ ret = _pse_pi_disable(pcdev, id);
+ if (!ret) {
+ pi->admin_state_enabled = 0;
+ pcdev->pi[id]._isr_counter_mismatch = 0;
+ }
- return ret;
+ mutex_unlock(&pcdev->lock);
+ return 0;
}
static int _pse_pi_get_voltage(struct regulator_dev *rdev)
@@ -542,6 +838,10 @@ static int pse_register_pw_ds(struct pse_controller_dev *pcdev)
}
pw_d->supply = supply;
+ if (pcdev->supp_budget_eval_strategies)
+ pw_d->budget_eval_strategy = pcdev->supp_budget_eval_strategies;
+ else
+ pw_d->budget_eval_strategy = ETHTOOL_PSE_BUDGET_EVAL_STRAT_DISABLED;
pcdev->pi[i].pw_d = pw_d;
}
@@ -699,27 +999,45 @@ static unsigned long pse_to_regulator_notifs(unsigned long notifs)
}
/**
- * pse_control_find_phy_by_id - Find PHY attached to the pse control id
+ * pse_set_config_isr - Set PSE control config according to the PSE
+ * notifications
* @pcdev: a pointer to the PSE
* @id: index of the PSE control
+ * @notifs: PSE event notifications
+ * @extack: extack for error reporting
*
- * Return: PHY device pointer or NULL
+ * Return: 0 on success and failure value on error
*/
-static struct phy_device *
-pse_control_find_phy_by_id(struct pse_controller_dev *pcdev, int id)
+static int pse_set_config_isr(struct pse_controller_dev *pcdev, int id,
+ unsigned long notifs,
+ struct netlink_ext_ack *extack)
{
- struct pse_control *psec;
+ int ret = 0;
- mutex_lock(&pse_list_mutex);
- list_for_each_entry(psec, &pcdev->pse_control_head, list) {
- if (psec->id == id) {
- mutex_unlock(&pse_list_mutex);
- return psec->attached_phydev;
- }
+ if (notifs & ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC)
+ return 0;
+
+ if ((notifs & ETHTOOL_C33_PSE_EVENT_DISCONNECTION) &&
+ ((notifs & ETHTOOL_C33_PSE_EVENT_DETECTION) ||
+ (notifs & ETHTOOL_C33_PSE_EVENT_CLASSIFICATION))) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "PI %d: error, connection and disconnection reported simultaneously",
+ id);
+ return -EINVAL;
}
- mutex_unlock(&pse_list_mutex);
- return NULL;
+ if (notifs & ETHTOOL_C33_PSE_EVENT_CLASSIFICATION) {
+ pcdev->pi[id].isr_pd_detected = true;
+ if (pcdev->pi[id].admin_state_enabled)
+ ret = _pse_pi_enable_sw_pw_ctrl(pcdev, id, extack);
+ } else if (notifs & ETHTOOL_C33_PSE_EVENT_DISCONNECTION) {
+ if (pcdev->pi[id].admin_state_enabled &&
+ pcdev->pi[id].isr_pd_detected)
+ ret = _pse_pi_disable(pcdev, id);
+ pcdev->pi[id].isr_pd_detected = false;
+ }
+
+ return ret;
}
/**
@@ -745,9 +1063,10 @@ static irqreturn_t pse_isr(int irq, void *data)
memset(h->notifs, 0, pcdev->nr_lines * sizeof(*h->notifs));
mutex_lock(&pcdev->lock);
ret = desc->map_event(irq, pcdev, h->notifs, ¬ifs_mask);
- mutex_unlock(&pcdev->lock);
- if (ret || !notifs_mask)
+ if (ret || !notifs_mask) {
+ mutex_unlock(&pcdev->lock);
return IRQ_NONE;
+ }
for_each_set_bit(i, ¬ifs_mask, pcdev->nr_lines) {
struct phy_device *phydev;
@@ -758,6 +1077,12 @@ static irqreturn_t pse_isr(int irq, void *data)
continue;
notifs = h->notifs[i];
+ if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[i].pw_d)) {
+ ret = pse_set_config_isr(pcdev, i, notifs, &extack);
+ if (ret)
+ notifs |= ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR;
+ }
+
dev_dbg(h->pcdev->dev,
"Sending PSE notification EVT 0x%lx\n", notifs);
@@ -769,6 +1094,8 @@ static irqreturn_t pse_isr(int irq, void *data)
NULL);
}
+ mutex_unlock(&pcdev->lock);
+
return IRQ_HANDLED;
}
@@ -863,6 +1190,7 @@ static struct pse_control *
pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
struct phy_device *phydev)
{
+ struct pse_admin_state admin_state = {0};
struct pse_control *psec;
int ret;
@@ -884,6 +1212,23 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
goto free_psec;
}
+ if (!pcdev->ops->pi_get_admin_state) {
+ ret = -EOPNOTSUPP;
+ goto free_psec;
+ }
+
+ /* Initialize admin_state_enabled before the regulator_get. This
+ * aims to have the right value reported in the first is_enabled
+ * call in case of control managed by software.
+ */
+ ret = pcdev->ops->pi_get_admin_state(pcdev, index, &admin_state);
+ if (ret)
+ goto free_psec;
+
+ if (admin_state.podl_admin_state == ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED ||
+ admin_state.c33_admin_state == ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED)
+ pcdev->pi[index].admin_state_enabled = 1;
+
psec->ps = devm_regulator_get_exclusive(pcdev->dev,
rdev_get_name(pcdev->pi[index].rdev));
if (IS_ERR(psec->ps)) {
@@ -891,12 +1236,6 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
goto put_module;
}
- ret = regulator_is_enabled(psec->ps);
- if (ret < 0)
- goto regulator_put;
-
- pcdev->pi[index].admin_state_enabled = ret;
-
psec->pcdev = pcdev;
list_add(&psec->list, &pcdev->pse_control_head);
psec->id = index;
@@ -905,8 +1244,6 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
return psec;
-regulator_put:
- devm_regulator_put(psec->ps);
put_module:
module_put(pcdev->owner);
free_psec:
@@ -1017,6 +1354,35 @@ struct pse_control *of_pse_control_get(struct device_node *node,
}
EXPORT_SYMBOL_GPL(of_pse_control_get);
+/**
+ * pse_get_sw_admin_state - Convert the software admin state to c33 or podl
+ * admin state value used in the standard
+ * @psec: PSE control pointer
+ * @admin_state: a pointer to the admin_state structure
+ */
+static void pse_get_sw_admin_state(struct pse_control *psec,
+ struct pse_admin_state *admin_state)
+{
+ struct pse_pi *pi = &psec->pcdev->pi[psec->id];
+
+ if (pse_has_podl(psec)) {
+ if (pi->admin_state_enabled)
+ admin_state->podl_admin_state =
+ ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED;
+ else
+ admin_state->podl_admin_state =
+ ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED;
+ }
+ if (pse_has_c33(psec)) {
+ if (pi->admin_state_enabled)
+ admin_state->c33_admin_state =
+ ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED;
+ else
+ admin_state->c33_admin_state =
+ ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED;
+ }
+}
+
/**
* pse_ethtool_get_status - get status of PSE control
* @psec: PSE control pointer
@@ -1033,19 +1399,46 @@ int pse_ethtool_get_status(struct pse_control *psec,
struct pse_pw_status pw_status = {0};
const struct pse_controller_ops *ops;
struct pse_controller_dev *pcdev;
+ struct pse_pi *pi;
int ret;
pcdev = psec->pcdev;
ops = pcdev->ops;
+
+ pi = &pcdev->pi[psec->id];
mutex_lock(&pcdev->lock);
- if (pcdev->pi[psec->id].pw_d)
- status->pw_d_id = pcdev->pi[psec->id].pw_d->id;
+ if (pi->pw_d) {
+ status->pw_d_id = pi->pw_d->id;
+ if (pse_pw_d_is_sw_pw_control(pcdev, pi->pw_d)) {
+ pse_get_sw_admin_state(psec, &admin_state);
+ } else {
+ ret = ops->pi_get_admin_state(pcdev, psec->id,
+ &admin_state);
+ if (ret)
+ goto out;
+ }
+ status->podl_admin_state = admin_state.podl_admin_state;
+ status->c33_admin_state = admin_state.c33_admin_state;
- ret = ops->pi_get_admin_state(pcdev, psec->id, &admin_state);
- if (ret)
- goto out;
- status->podl_admin_state = admin_state.podl_admin_state;
- status->c33_admin_state = admin_state.c33_admin_state;
+ switch (pi->pw_d->budget_eval_strategy) {
+ case ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC:
+ status->prio_max = pcdev->nr_lines;
+ status->prio = pi->prio;
+ break;
+ case ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC:
+ status->prio_max = pcdev->pis_prio_max;
+ if (ops->pi_get_prio) {
+ ret = ops->pi_get_prio(pcdev, psec->id);
+ if (ret < 0)
+ goto out;
+
+ status->prio = ret;
+ }
+ break;
+ default:
+ break;
+ }
+ }
ret = ops->pi_get_pw_status(pcdev, psec->id, &pw_status);
if (ret)
@@ -1120,11 +1513,15 @@ static int pse_ethtool_c33_set_config(struct pse_control *psec,
case ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED:
/* We could have mismatch between admin_state_enabled and
* state reported by regulator_is_enabled. This can occur when
- * the PI is forcibly turn off by the controller. Call
+ * the PI is forcibly turn off by the controller or in power
+ * off case in the interrupt context. Call
* regulator_disable on that case to fix the counters state.
+ * disable action might be called two times consecutively
+ * but that is not a real issue.
*/
- if (psec->pcdev->pi[psec->id].admin_state_enabled &&
- !regulator_is_enabled(psec->ps)) {
+ if ((psec->pcdev->pi[psec->id].admin_state_enabled &&
+ !regulator_is_enabled(psec->ps)) ||
+ psec->pcdev->pi[psec->id]._isr_counter_mismatch) {
err = regulator_disable(psec->ps);
if (err)
break;
@@ -1194,6 +1591,52 @@ int pse_ethtool_set_config(struct pse_control *psec,
}
EXPORT_SYMBOL_GPL(pse_ethtool_set_config);
+/**
+ * pse_pi_update_pw_budget - Update PSE power budget allocated with new
+ * power in mW
+ * @pcdev: a pointer to the PSE controller device
+ * @id: index of the PSE PI
+ * @pw_req: power requested
+ * @extack: extack for reporting useful error messages
+ *
+ * Return: Previous power allocated on success and failure value on error
+ */
+static int pse_pi_update_pw_budget(struct pse_controller_dev *pcdev, int id,
+ const unsigned int pw_req,
+ struct netlink_ext_ack *extack)
+{
+ struct pse_pi *pi = &pcdev->pi[id];
+ int previous_pw_allocated;
+ int pw_diff, ret = 0;
+
+ /* We don't want pw_allocated_mW value change in the middle of an
+ * power budget update
+ */
+ mutex_lock(&pcdev->lock);
+ previous_pw_allocated = pi->pw_allocated_mW;
+ pw_diff = pw_req - previous_pw_allocated;
+ if (!pw_diff) {
+ goto out;
+ } else if (pw_diff > 0) {
+ ret = regulator_request_power_budget(pi->pw_d->supply, pw_diff);
+ if (ret) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "PI %d: not enough power budget available",
+ id);
+ goto out;
+ }
+
+ } else {
+ regulator_free_power_budget(pi->pw_d->supply, -pw_diff);
+ }
+ pi->pw_allocated_mW = pw_req;
+ ret = previous_pw_allocated;
+
+out:
+ mutex_unlock(&pcdev->lock);
+ return ret;
+}
+
/**
* pse_ethtool_set_pw_limit - set PSE control power limit
* @psec: PSE control pointer
@@ -1206,7 +1649,7 @@ int pse_ethtool_set_pw_limit(struct pse_control *psec,
struct netlink_ext_ack *extack,
const unsigned int pw_limit)
{
- int uV, uA, ret;
+ int uV, uA, ret, previous_pw_allocated = 0;
s64 tmp_64;
if (pw_limit > MAX_PI_PW)
@@ -1230,10 +1673,99 @@ int pse_ethtool_set_pw_limit(struct pse_control *psec,
/* uA = mW * 1000000000 / uV */
uA = DIV_ROUND_CLOSEST_ULL(tmp_64, uV);
- return regulator_set_current_limit(psec->ps, 0, uA);
+ /* Update power budget only in software power control case and
+ * if a Power Device is powered.
+ */
+ if (pse_pw_d_is_sw_pw_control(psec->pcdev,
+ psec->pcdev->pi[psec->id].pw_d) &&
+ psec->pcdev->pi[psec->id].admin_state_enabled &&
+ psec->pcdev->pi[psec->id].isr_pd_detected) {
+ ret = pse_pi_update_pw_budget(psec->pcdev, psec->id,
+ pw_limit, extack);
+ if (ret < 0)
+ return ret;
+ previous_pw_allocated = ret;
+ }
+
+ ret = regulator_set_current_limit(psec->ps, 0, uA);
+ if (ret < 0 && previous_pw_allocated) {
+ pse_pi_update_pw_budget(psec->pcdev, psec->id,
+ previous_pw_allocated, extack);
+ }
+
+ return ret;
}
EXPORT_SYMBOL_GPL(pse_ethtool_set_pw_limit);
+/**
+ * pse_ethtool_set_prio - Set PSE PI priority according to the budget
+ * evaluation strategy
+ * @psec: PSE control pointer
+ * @extack: extack for reporting useful error messages
+ * @prio: priovity value
+ *
+ * Return: 0 on success and failure value on error
+ */
+int pse_ethtool_set_prio(struct pse_control *psec,
+ struct netlink_ext_ack *extack,
+ unsigned int prio)
+{
+ struct pse_controller_dev *pcdev = psec->pcdev;
+ const struct pse_controller_ops *ops;
+ int ret = 0;
+
+ if (!pcdev->pi[psec->id].pw_d) {
+ NL_SET_ERR_MSG(extack, "no power domain attached");
+ return -EOPNOTSUPP;
+ }
+
+ /* We don't want priority change in the middle of an
+ * enable/disable call or a priority mode change
+ */
+ mutex_lock(&pcdev->lock);
+ switch (pcdev->pi[psec->id].pw_d->budget_eval_strategy) {
+ case ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC:
+ if (prio > pcdev->nr_lines) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "priority %d exceed priority max %d",
+ prio, pcdev->nr_lines);
+ ret = -ERANGE;
+ goto out;
+ }
+
+ pcdev->pi[psec->id].prio = prio;
+ break;
+
+ case ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC:
+ ops = psec->pcdev->ops;
+ if (!ops->pi_set_prio) {
+ NL_SET_ERR_MSG(extack,
+ "pse driver does not support setting port priority");
+ ret = -EOPNOTSUPP;
+ goto out;
+ }
+
+ if (prio > pcdev->pis_prio_max) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "priority %d exceed priority max %d",
+ prio, pcdev->pis_prio_max);
+ ret = -ERANGE;
+ goto out;
+ }
+
+ ret = ops->pi_set_prio(pcdev, psec->id, prio);
+ break;
+
+ default:
+ ret = -EOPNOTSUPP;
+ }
+
+out:
+ mutex_unlock(&pcdev->lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(pse_ethtool_set_prio);
+
bool pse_has_podl(struct pse_control *psec)
{
return psec->pcdev->types & ETHTOOL_PSE_PODL;
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index ffa6cf9a0072f..b47d2d2c807e9 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -132,6 +132,9 @@ struct pse_pw_limit_ranges {
* is in charge of the memory allocation
* @c33_pw_limit_nb_ranges: number of supported power limit configuration
* ranges
+ * @prio_max: max priority allowed for the c33_prio variable value.
+ * @prio: priority of the PSE. Managed by PSE core in case of static budget
+ * evaluation strategy.
*/
struct ethtool_pse_control_status {
u32 pw_d_id;
@@ -145,6 +148,8 @@ struct ethtool_pse_control_status {
u32 c33_avail_pw_limit;
struct ethtool_c33_pse_pw_limit_range *c33_pw_limit_ranges;
u32 c33_pw_limit_nb_ranges;
+ u32 prio_max;
+ u32 prio;
};
/**
@@ -168,6 +173,11 @@ struct ethtool_pse_control_status {
* range. The driver is in charge of the memory
* allocation and should return the number of
* ranges.
+ * @pi_get_prio: Get the PSE PI priority.
+ * @pi_set_prio: Configure the PSE PI priority.
+ * @pi_get_pw_req: Get the power requested by a PD before enabling the PSE PI.
+ * This is only relevant when an interrupt is registered using
+ * devm_pse_irq_helper helper.
*/
struct pse_controller_ops {
int (*setup_pi_matrix)(struct pse_controller_dev *pcdev);
@@ -188,6 +198,10 @@ struct pse_controller_ops {
int id, int max_mW);
int (*pi_get_pw_limit_ranges)(struct pse_controller_dev *pcdev, int id,
struct pse_pw_limit_ranges *pw_limit_ranges);
+ int (*pi_get_prio)(struct pse_controller_dev *pcdev, int id);
+ int (*pi_set_prio)(struct pse_controller_dev *pcdev, int id,
+ unsigned int prio);
+ int (*pi_get_pw_req)(struct pse_controller_dev *pcdev, int id);
};
struct module;
@@ -223,6 +237,17 @@ struct pse_pi_pairset {
* @rdev: regulator represented by the PSE PI
* @admin_state_enabled: PI enabled state
* @pw_d: Power domain of the PSE PI
+ * @prio: Priority of the PSE PI. Used in static budget evaluation strategy
+ * @isr_pd_detected: PSE PI detection status managed by the interruption
+ * handler. This variable is relevant when the power enabled
+ * management is managed in software like the static
+ * budget evaluation strategy.
+ * @pw_allocated_mW: Power allocated to a PSE PI to manage power budget in
+ * static budget evaluation strategy.
+ * @_isr_counter_mismatch: Internal flag used in PSE core in case of a
+ * counter mismatch between regulator and PSE API.
+ * This is caused by a disable call in the interrupt
+ * context handler.
*/
struct pse_pi {
struct pse_pi_pairset pairset[2];
@@ -230,6 +255,10 @@ struct pse_pi {
struct regulator_dev *rdev;
bool admin_state_enabled;
struct pse_power_domain *pw_d;
+ int prio;
+ bool isr_pd_detected;
+ int pw_allocated_mW;
+ bool _isr_counter_mismatch;
};
/**
@@ -247,6 +276,9 @@ struct pse_pi {
* @pi: table of PSE PIs described in this controller device
* @no_of_pse_pi: flag set if the pse_pis devicetree node is not used
* @irq: PSE interrupt
+ * @pis_prio_max: Maximum value allowed for the PSE PIs priority
+ * @supp_budget_eval_strategies: budget evaluation strategies supported
+ * by the PSE
*/
struct pse_controller_dev {
const struct pse_controller_ops *ops;
@@ -261,6 +293,8 @@ struct pse_controller_dev {
struct pse_pi *pi;
bool no_of_pse_pi;
int irq;
+ unsigned int pis_prio_max;
+ u32 supp_budget_eval_strategies;
};
#if IS_ENABLED(CONFIG_PSE_CONTROLLER)
@@ -285,6 +319,9 @@ int pse_ethtool_set_config(struct pse_control *psec,
int pse_ethtool_set_pw_limit(struct pse_control *psec,
struct netlink_ext_ack *extack,
const unsigned int pw_limit);
+int pse_ethtool_set_prio(struct pse_control *psec,
+ struct netlink_ext_ack *extack,
+ unsigned int prio);
bool pse_has_podl(struct pse_control *psec);
bool pse_has_c33(struct pse_control *psec);
@@ -322,6 +359,13 @@ static inline int pse_ethtool_set_pw_limit(struct pse_control *psec,
return -EOPNOTSUPP;
}
+static inline int pse_ethtool_set_prio(struct pse_control *psec,
+ struct netlink_ext_ack *extack,
+ unsigned int prio)
+{
+ return -EOPNOTSUPP;
+}
+
static inline bool pse_has_podl(struct pse_control *psec)
{
return false;
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 5e99daf42d440..0c6aef700a8ff 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1006,6 +1006,20 @@ enum ethtool_c33_pse_pw_d_status {
* enum ethtool_pse_events - event list of the PSE controller.
* @ETHTOOL_PSE_EVENT_OVER_CURRENT: PSE output current is too high.
* @ETHTOOL_PSE_EVENT_OVER_TEMP: PSE in over temperature state.
+ * @ETHTOOL_C33_PSE_EVENT_DETECTION: detection process occur on the PSE.
+ * IEEE 802.3-2022 33.2.5 and 145.2.6 PSE detection of PDs.
+ * IEEE 802.3-202 30.9.1.1.5 aPSEPowerDetectionStatus.
+ * @ETHTOOL_C33_PSE_EVENT_CLASSIFICATION: classification process occur on
+ * the PSE. IEEE 802.3-2022 33.2.6 and 145.2.8 classification of PDs and
+ * mutual identification.
+ * IEEE 802.3-2022 30.9.1.1.8 aPSEPowerClassification.
+ * @ETHTOOL_C33_PSE_EVENT_DISCONNECTION: PD has been disconnected on the PSE.
+ * IEEE 802.3-2022 33.3.8 and 145.3.9 PD Maintain Power Signature.
+ * IEEE 802.3-2022 33.5.1.2.9 MPS Absent.
+ * IEEE 802.3-2022 30.9.1.1.20 aPSEMPSAbsentCounter.
+ * @ETHTOOL_PSE_EVENT_OVER_BUDGET: PSE turned off due to over budget situation.
+ * @ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR: PSE faced an error managing the
+ * power control from software.
*
* @ETHTOOL_PSE_EVENT_LAST: Last PSE event of the enum.
*/
@@ -1013,8 +1027,31 @@ enum ethtool_c33_pse_pw_d_status {
enum ethtool_pse_events {
ETHTOOL_PSE_EVENT_OVER_CURRENT = 1 << 0,
ETHTOOL_PSE_EVENT_OVER_TEMP = 1 << 1,
+ ETHTOOL_C33_PSE_EVENT_DETECTION = 1 << 2,
+ ETHTOOL_C33_PSE_EVENT_CLASSIFICATION = 1 << 3,
+ ETHTOOL_C33_PSE_EVENT_DISCONNECTION = 1 << 4,
+ ETHTOOL_PSE_EVENT_OVER_BUDGET = 1 << 5,
+ ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR = 1 << 6,
- ETHTOOL_PSE_EVENT_LAST = ETHTOOL_PSE_EVENT_OVER_TEMP,
+ ETHTOOL_PSE_EVENT_LAST = ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR,
+};
+
+/**
+ * enum ethtool_pse_budget_eval_strategies - PSE budget evaluation strategies.
+ * @ETHTOOL_PSE_BUDGET_EVAL_STRAT_DISABLED: Budget evaluation strategy disabled.
+ * @ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC: PSE static budget evaluation strategy.
+ * Budget evaluation strategy based on the power requested during PD
+ * classification. This strategy is managed by the PSE core.
+ * @ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC: PSE dynamic budget evaluation
+ * strategy. Budget evaluation strategy based on the current consumption
+ * per ports compared to the total power budget. This mode is managed by
+ * the PSE controller.
+ */
+
+enum ethtool_pse_budget_eval_strategies {
+ ETHTOOL_PSE_BUDGET_EVAL_STRAT_DISABLED = 1 << 0,
+ ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC = 1 << 1,
+ ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC = 1 << 2,
};
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-04 10:18 ` [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies Kory Maincent
@ 2025-03-07 1:46 ` Jakub Kicinski
2025-03-07 9:10 ` Kory Maincent
2025-03-17 12:40 ` Oleksij Rempel
1 sibling, 1 reply; 36+ messages in thread
From: Jakub Kicinski @ 2025-03-07 1:46 UTC (permalink / raw)
To: Kory Maincent
Cc: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Tue, 04 Mar 2025 11:18:55 +0100 Kory Maincent wrote:
> +/**
> + * enum ethtool_pse_budget_eval_strategies - PSE budget evaluation strategies.
> + * @ETHTOOL_PSE_BUDGET_EVAL_STRAT_DISABLED: Budget evaluation strategy disabled.
> + * @ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC: PSE static budget evaluation strategy.
> + * Budget evaluation strategy based on the power requested during PD
> + * classification. This strategy is managed by the PSE core.
> + * @ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC: PSE dynamic budget evaluation
> + * strategy. Budget evaluation strategy based on the current consumption
> + * per ports compared to the total power budget. This mode is managed by
> + * the PSE controller.
> + */
> +
> +enum ethtool_pse_budget_eval_strategies {
> + ETHTOOL_PSE_BUDGET_EVAL_STRAT_DISABLED = 1 << 0,
> + ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC = 1 << 1,
> + ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC = 1 << 2,
> };
Leftover?
--
pw-bot: cr
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-07 1:46 ` Jakub Kicinski
@ 2025-03-07 9:10 ` Kory Maincent
0 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-07 9:10 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Thu, 6 Mar 2025 17:46:19 -0800
Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue, 04 Mar 2025 11:18:55 +0100 Kory Maincent wrote:
> > +/**
> > + * enum ethtool_pse_budget_eval_strategies - PSE budget evaluation
> > strategies.
> > + * @ETHTOOL_PSE_BUDGET_EVAL_STRAT_DISABLED: Budget evaluation strategy
> > disabled.
> > + * @ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC: PSE static budget evaluation
> > strategy.
> > + * Budget evaluation strategy based on the power requested during PD
> > + * classification. This strategy is managed by the PSE core.
> > + * @ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC: PSE dynamic budget evaluation
> > + * strategy. Budget evaluation strategy based on the current
> > consumption
> > + * per ports compared to the total power budget. This mode
> > is managed by
> > + * the PSE controller.
> > + */
> > +
> > +enum ethtool_pse_budget_eval_strategies {
> > + ETHTOOL_PSE_BUDGET_EVAL_STRAT_DISABLED = 1 << 0,
> > + ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC = 1 << 1,
> > + ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC = 1 << 2,
> > };
>
> Leftover?
We still need these to know the PSE method but they shall not be in the uAPI
for now. I will move it out of it.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-04 10:18 ` [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies Kory Maincent
2025-03-07 1:46 ` Jakub Kicinski
@ 2025-03-17 12:40 ` Oleksij Rempel
2025-03-20 16:35 ` Kory Maincent
1 sibling, 1 reply; 36+ messages in thread
From: Oleksij Rempel @ 2025-03-17 12:40 UTC (permalink / raw)
To: Kory Maincent
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Tue, Mar 04, 2025 at 11:18:55AM +0100, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> +/**
> + * pse_disable_pi_prio - Disable all PIs of a given priority inside a PSE
> + * power domain
> + * @pcdev: a pointer to the PSE
> + * @pw_d: a pointer to the PSE power domain
> + * @prio: priority
> + *
> + * Return: 0 on success and failure value on error
> + */
> +static int pse_disable_pi_prio(struct pse_controller_dev *pcdev,
> + struct pse_power_domain *pw_d,
> + int prio)
> +{
> + int i;
> +
Should we lock the pi[] array at some level?
> + for (i = 0; i < pcdev->nr_lines; i++) {
> + int ret;
> +
> + if (pcdev->pi[i].prio != prio ||
> + pcdev->pi[i].pw_d != pw_d ||
> + !pcdev->pi[i].admin_state_enabled)
> + continue;
> +
> + ret = pse_disable_pi_pol(pcdev, i);
If the PSE has many lower-priority ports, the same set of ports could be
repeatedly shut down while higher-priority ports keep power
indefinitely.
This could result in a starvation issue, where lower-priority group of
ports may never get a chance to stay enabled, even if power briefly
becomes available.
To fix this, we could:
- Disallow identical priorities in static mode to ensure a clear
shutdown order.
- Modify pse_disable_pi_prio() to track freed power and stop
disabling once enough power is recovered.
static int pse_disable_pi_prio(struct pse_controller_dev *pcdev,
struct pse_power_domain *pw_d,
int prio, int required_power)
{
int i, ret;
int freed_power = 0;
mutex_lock(&pcdev->lock);
for (i = 0; i < pcdev->nr_lines; i++) {
if (pcdev->pi[i].prio != prio ||
pcdev->pi[i].pw_d != pw_d ||
!pcdev->pi[i].admin_state_enabled)
continue;
ret = pse_disable_pi_pol(pcdev, i);
if (ret == 0)
freed_power += pcdev->pi[i].pw_allocated_mW;
/* Stop once we have freed enough power */
if (freed_power >= required_power)
break;
}
mutex_unlock(&pcdev->lock);
return ret;
}
The current approach still introduces an implicit priority based on loop
execution order, but since it's predictable, it can serve as a reasonable
default.
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
....
> +/**
> + * pse_ethtool_set_prio - Set PSE PI priority according to the budget
> + * evaluation strategy
> + * @psec: PSE control pointer
> + * @extack: extack for reporting useful error messages
> + * @prio: priovity value
> + *
> + * Return: 0 on success and failure value on error
> + */
> +int pse_ethtool_set_prio(struct pse_control *psec,
> + struct netlink_ext_ack *extack,
> + unsigned int prio)
> +{
> + struct pse_controller_dev *pcdev = psec->pcdev;
> + const struct pse_controller_ops *ops;
> + int ret = 0;
> +
> + if (!pcdev->pi[psec->id].pw_d) {
> + NL_SET_ERR_MSG(extack, "no power domain attached");
> + return -EOPNOTSUPP;
> + }
> +
> + /* We don't want priority change in the middle of an
> + * enable/disable call or a priority mode change
> + */
> + mutex_lock(&pcdev->lock);
> + switch (pcdev->pi[psec->id].pw_d->budget_eval_strategy) {
> + case ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC:
> + if (prio > pcdev->nr_lines) {
> + NL_SET_ERR_MSG_FMT(extack,
> + "priority %d exceed priority max %d",
> + prio, pcdev->nr_lines);
> + ret = -ERANGE;
> + goto out;
> + }
> +
> + pcdev->pi[psec->id].prio = prio;
In case we already out of the budget, we will need to re-evaluate the
prios. New configuration may affect state of ports.
Potentially we may need a bulk interface to assign prios, to speed-up
reconfiguration. But it is not needed right now.
> + break;
> +
> + case ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC:
> + ops = psec->pcdev->ops;
> + if (!ops->pi_set_prio) {
> + NL_SET_ERR_MSG(extack,
> + "pse driver does not support setting port priority");
> + ret = -EOPNOTSUPP;
> + goto out;
> + }
> +
> + if (prio > pcdev->pis_prio_max) {
> + NL_SET_ERR_MSG_FMT(extack,
> + "priority %d exceed priority max %d",
> + prio, pcdev->pis_prio_max);
> + ret = -ERANGE;
> + goto out;
> + }
> +
> + ret = ops->pi_set_prio(pcdev, psec->id, prio);
Here too, but in case of microchip PSE it will happen in the firmware.
May be add here a comment that currently it is done in firmware and to
be extended for the kernel based implementation.
> + break;
> +
> + default:
> + ret = -EOPNOTSUPP;
> + }
> +
> +out:
> + mutex_unlock(&pcdev->lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(pse_ethtool_set_prio);
--
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 |
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-17 12:40 ` Oleksij Rempel
@ 2025-03-20 16:35 ` Kory Maincent
2025-03-24 16:39 ` Kory Maincent
0 siblings, 1 reply; 36+ messages in thread
From: Kory Maincent @ 2025-03-20 16:35 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Mon, 17 Mar 2025 13:40:45 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> On Tue, Mar 04, 2025 at 11:18:55AM +0100, Kory Maincent wrote:
> > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> > +/**
> > + * pse_disable_pi_prio - Disable all PIs of a given priority inside a PSE
> > + * power domain
> > + * @pcdev: a pointer to the PSE
> > + * @pw_d: a pointer to the PSE power domain
> > + * @prio: priority
> > + *
> > + * Return: 0 on success and failure value on error
> > + */
> > +static int pse_disable_pi_prio(struct pse_controller_dev *pcdev,
> > + struct pse_power_domain *pw_d,
> > + int prio)
> > +{
> > + int i;
> > +
>
> Should we lock the pi[] array at some level?
Lock the pi array?
pcdev is already locked in the irq thread handler.
> > + for (i = 0; i < pcdev->nr_lines; i++) {
> > + int ret;
> > +
> > + if (pcdev->pi[i].prio != prio ||
> > + pcdev->pi[i].pw_d != pw_d ||
> > + !pcdev->pi[i].admin_state_enabled)
> > + continue;
> > +
> > + ret = pse_disable_pi_pol(pcdev, i);
>
> If the PSE has many lower-priority ports, the same set of ports could be
> repeatedly shut down while higher-priority ports keep power
> indefinitely.
That is the point of priority. It is not a problem as there is as many priority
value as ports. Disabling all the ports configured with a specific priority is
understandable.
> This could result in a starvation issue, where lower-priority group of
> ports may never get a chance to stay enabled, even if power briefly
> becomes available.
>
> To fix this, we could:
> - Disallow identical priorities in static mode to ensure a clear
> shutdown order.
We can't do this as this imply priority shutdown method is always used and
can't be disabled.
Having all the ports to the same priority means that the priority strategy is
not used and if new PD is plugged and exceed the budget it simply won't be
powered.
> > +int pse_ethtool_set_prio(struct pse_control *psec,
> > + struct netlink_ext_ack *extack,
> > + unsigned int prio)
> > +{
> > + struct pse_controller_dev *pcdev = psec->pcdev;
> > + const struct pse_controller_ops *ops;
> > + int ret = 0;
> > +
> > + if (!pcdev->pi[psec->id].pw_d) {
> > + NL_SET_ERR_MSG(extack, "no power domain attached");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + /* We don't want priority change in the middle of an
> > + * enable/disable call or a priority mode change
> > + */
> > + mutex_lock(&pcdev->lock);
> > + switch (pcdev->pi[psec->id].pw_d->budget_eval_strategy) {
> > + case ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC:
> > + if (prio > pcdev->nr_lines) {
> > + NL_SET_ERR_MSG_FMT(extack,
> > + "priority %d exceed priority
> > max %d",
> > + prio, pcdev->nr_lines);
> > + ret = -ERANGE;
> > + goto out;
> > + }
> > +
> > + pcdev->pi[psec->id].prio = prio;
>
> In case we already out of the budget, we will need to re-evaluate the
> prios. New configuration may affect state of ports.
>
> Potentially we may need a bulk interface to assign prios, to speed-up
> reconfiguration. But it is not needed right now.
Oh indeed, I missed that. /o\
I will try to add something but lets keep it not too complex! ^^ Don't add me
more work!! ;)
> > + break;
> > +
> > + case ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC:
> > + ops = psec->pcdev->ops;
> > + if (!ops->pi_set_prio) {
> > + NL_SET_ERR_MSG(extack,
> > + "pse driver does not support
> > setting port priority");
> > + ret = -EOPNOTSUPP;
> > + goto out;
> > + }
> > +
> > + if (prio > pcdev->pis_prio_max) {
> > + NL_SET_ERR_MSG_FMT(extack,
> > + "priority %d exceed priority
> > max %d",
> > + prio, pcdev->pis_prio_max);
> > + ret = -ERANGE;
> > + goto out;
> > + }
> > +
> > + ret = ops->pi_set_prio(pcdev, psec->id, prio);
>
> Here too, but in case of microchip PSE it will happen in the firmware.
> May be add here a comment that currently it is done in firmware and to
> be extended for the kernel based implementation.
Ack, I will add a comment.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-20 16:35 ` Kory Maincent
@ 2025-03-24 16:39 ` Kory Maincent
2025-03-24 17:33 ` Kyle Swenson
0 siblings, 1 reply; 36+ messages in thread
From: Kory Maincent @ 2025-03-24 16:39 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
Hello Kyle, Oleksij,
On Thu, 20 Mar 2025 17:35:35 +0100
Kory Maincent <kory.maincent@bootlin.com> wrote:
> On Mon, 17 Mar 2025 13:40:45 +0100
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> > On Tue, Mar 04, 2025 at 11:18:55AM +0100, Kory Maincent wrote:
> > > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> > > +int pse_ethtool_set_prio(struct pse_control *psec,
> > > + struct netlink_ext_ack *extack,
> > > + unsigned int prio)
> > > +{
> > > + struct pse_controller_dev *pcdev = psec->pcdev;
> > > + const struct pse_controller_ops *ops;
> > > + int ret = 0;
> > > +
> > > + if (!pcdev->pi[psec->id].pw_d) {
> > > + NL_SET_ERR_MSG(extack, "no power domain attached");
> > > + return -EOPNOTSUPP;
> > > + }
> > > +
> > > + /* We don't want priority change in the middle of an
> > > + * enable/disable call or a priority mode change
> > > + */
> > > + mutex_lock(&pcdev->lock);
> > > + switch (pcdev->pi[psec->id].pw_d->budget_eval_strategy) {
> > > + case ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC:
> > > + if (prio > pcdev->nr_lines) {
> > > + NL_SET_ERR_MSG_FMT(extack,
> > > + "priority %d exceed priority
> > > max %d",
> > > + prio, pcdev->nr_lines);
> > > + ret = -ERANGE;
> > > + goto out;
> > > + }
> > > +
> > > + pcdev->pi[psec->id].prio = prio;
> >
> > In case we already out of the budget, we will need to re-evaluate the
> > prios. New configuration may affect state of ports.
> >
> > Potentially we may need a bulk interface to assign prios, to speed-up
> > reconfiguration. But it is not needed right now.
>
> Oh indeed, I missed that. /o\
> I will try to add something but lets keep it not too complex! ^^ Don't add me
> more work!! ;)
Small question on PSE core behavior for PoE users.
If we want to enable a port but we can't due to over budget.
Should we :
- Report an error (or not) and save the enable action from userspace. On that
case, if enough budget is available later due to priority change or port
disconnected the PSE core will try automatically to re enable the PoE port.
The port will then be enabled without any action from the user.
- Report an error but do nothing. The user will need to rerun the enable
command later to try to enable the port again.
How is it currently managed in PoE poprietary userspace tools?
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-24 16:39 ` Kory Maincent
@ 2025-03-24 17:33 ` Kyle Swenson
2025-03-25 5:34 ` Oleksij Rempel
0 siblings, 1 reply; 36+ messages in thread
From: Kyle Swenson @ 2025-03-24 17:33 UTC (permalink / raw)
To: Kory Maincent
Cc: Oleksij Rempel, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Thomas Petazzoni, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, Dent Project, kernel@pengutronix.de,
Maxime Chevallier, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Hello Kory,
On Mon, Mar 24, 2025 at 05:39:07PM +0100, Kory Maincent wrote:
> Hello Kyle, Oleksij,
...
>
> Small question on PSE core behavior for PoE users.
>
> If we want to enable a port but we can't due to over budget.
> Should we :
> - Report an error (or not) and save the enable action from userspace. On that
> case, if enough budget is available later due to priority change or port
> disconnected the PSE core will try automatically to re enable the PoE port.
> The port will then be enabled without any action from the user.
> - Report an error but do nothing. The user will need to rerun the enable
> command later to try to enable the port again.
>
> How is it currently managed in PoE poprietary userspace tools?
So in our implementation, we're using the first option you've presented.
That is, we save the enable action from the user and if we can't power
the device due to insufficient budget remaining, we'll indicate that status to the
user. If enough power budget becomes available later, we'll power up
the device automatically.
Hope this helps,
Kyle
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-24 17:33 ` Kyle Swenson
@ 2025-03-25 5:34 ` Oleksij Rempel
2025-03-25 15:25 ` Kory Maincent
0 siblings, 1 reply; 36+ messages in thread
From: Oleksij Rempel @ 2025-03-25 5:34 UTC (permalink / raw)
To: Kyle Swenson
Cc: Kory Maincent, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Thomas Petazzoni, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, Dent Project, kernel@pengutronix.de,
Maxime Chevallier, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi,
On Mon, Mar 24, 2025 at 05:33:18PM +0000, Kyle Swenson wrote:
> Hello Kory,
>
> On Mon, Mar 24, 2025 at 05:39:07PM +0100, Kory Maincent wrote:
> > Hello Kyle, Oleksij,
> ...
> >
> > Small question on PSE core behavior for PoE users.
> >
> > If we want to enable a port but we can't due to over budget.
> > Should we :
> > - Report an error (or not) and save the enable action from userspace. On that
> > case, if enough budget is available later due to priority change or port
> > disconnected the PSE core will try automatically to re enable the PoE port.
> > The port will then be enabled without any action from the user.
> > - Report an error but do nothing. The user will need to rerun the enable
> > command later to try to enable the port again.
> >
> > How is it currently managed in PoE poprietary userspace tools?
>
> So in our implementation, we're using the first option you've presented.
> That is, we save the enable action from the user and if we can't power
> the device due to insufficient budget remaining, we'll indicate that status to the
> user. If enough power budget becomes available later, we'll power up
> the device automatically.
It seems to be similar to administrative UP state - "ip link set dev lan1 up".
I'm ok with this behavior.
--
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 |
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-25 5:34 ` Oleksij Rempel
@ 2025-03-25 15:25 ` Kory Maincent
2025-03-25 20:40 ` Kyle Swenson
0 siblings, 1 reply; 36+ messages in thread
From: Kory Maincent @ 2025-03-25 15:25 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Kyle Swenson, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Thomas Petazzoni, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, Dent Project, kernel@pengutronix.de,
Maxime Chevallier, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
On Tue, 25 Mar 2025 06:34:17 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> Hi,
>
> On Mon, Mar 24, 2025 at 05:33:18PM +0000, Kyle Swenson wrote:
> > Hello Kory,
> >
> > On Mon, Mar 24, 2025 at 05:39:07PM +0100, Kory Maincent wrote:
> > > Hello Kyle, Oleksij,
> > ...
> > >
> > > Small question on PSE core behavior for PoE users.
> > >
> > > If we want to enable a port but we can't due to over budget.
> > > Should we :
> > > - Report an error (or not) and save the enable action from userspace. On
> > > that case, if enough budget is available later due to priority change or
> > > port disconnected the PSE core will try automatically to re enable the
> > > PoE port. The port will then be enabled without any action from the user.
> > > - Report an error but do nothing. The user will need to rerun the enable
> > > command later to try to enable the port again.
> > >
> > > How is it currently managed in PoE poprietary userspace tools?
> >
> > So in our implementation, we're using the first option you've presented.
> > That is, we save the enable action from the user and if we can't power
> > the device due to insufficient budget remaining, we'll indicate that status
> > to the user. If enough power budget becomes available later, we'll power up
> > the device automatically.
>
> It seems to be similar to administrative UP state - "ip link set dev lan1 up".
> I'm ok with this behavior.
Ack I will go for it then, thank you!
Other question to both of you:
If we configure manually the current limit for a port. Then we plug a Powered
Device and we detect (during the classification) a smaller current limit
supported. Should we change the current limit to the one detected. On that case
we should not let the user set a power limit greater than the one detected after
the PD has been plugged.
What do you think? Could we let a user burn a PD?
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-25 15:25 ` Kory Maincent
@ 2025-03-25 20:40 ` Kyle Swenson
2025-03-26 10:01 ` Oleksij Rempel
0 siblings, 1 reply; 36+ messages in thread
From: Kyle Swenson @ 2025-03-25 20:40 UTC (permalink / raw)
To: Kory Maincent
Cc: Oleksij Rempel, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Thomas Petazzoni, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, Dent Project, kernel@pengutronix.de,
Maxime Chevallier, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Hello Kory,
On Tue, Mar 25, 2025 at 04:25:34PM +0100, Kory Maincent wrote:
> On Tue, 25 Mar 2025 06:34:17 +0100
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> > Hi,
> >
> > On Mon, Mar 24, 2025 at 05:33:18PM +0000, Kyle Swenson wrote:
> > > Hello Kory,
> > >
> > > On Mon, Mar 24, 2025 at 05:39:07PM +0100, Kory Maincent wrote:
> > > > Hello Kyle, Oleksij,
> > > ...
> > > >
> > > > Small question on PSE core behavior for PoE users.
> > > >
> > > > If we want to enable a port but we can't due to over budget.
> > > > Should we :
> > > > - Report an error (or not) and save the enable action from userspace. On
> > > > that case, if enough budget is available later due to priority change or
> > > > port disconnected the PSE core will try automatically to re enable the
> > > > PoE port. The port will then be enabled without any action from the user.
> > > > - Report an error but do nothing. The user will need to rerun the enable
> > > > command later to try to enable the port again.
> > > >
> > > > How is it currently managed in PoE poprietary userspace tools?
> > >
> > > So in our implementation, we're using the first option you've presented.
> > > That is, we save the enable action from the user and if we can't power
> > > the device due to insufficient budget remaining, we'll indicate that status
> > > to the user. If enough power budget becomes available later, we'll power up
> > > the device automatically.
> >
> > It seems to be similar to administrative UP state - "ip link set dev lan1 up".
> > I'm ok with this behavior.
>
> Ack I will go for it then, thank you!
>
> Other question to both of you:
> If we configure manually the current limit for a port. Then we plug a Powered
> Device and we detect (during the classification) a smaller current limit
> supported. Should we change the current limit to the one detected. On that case
> we should not let the user set a power limit greater than the one detected after
> the PD has been plugged.
I don't know that we want to prevent the user from setting a higher
current than a device's classification current because that would
prevent the PD and PSE negotiating a higher current via LLDP.
That said, I'm struggling to think of a use-case where the user would be
setting a current limit before a PD is connected, so maybe we can reset
the current limit when the PD is classified to the classification
result, but also allow it to be adjusted after a PD is powered for the
LLDP negotiation case.
In our implementation, don't really let the user specify something like,
"Only class 3 and lower devices on this port" because we've not seen
customers need this. We have, however, implemented the LLDP negotiation
support after several requests from customers, but this only makes sense
when a PD is powered at it's initial classification result. The PD can
then request more power (via LLDP) and then we adjust the current limit
assuming the system has budget available for the request.
>
> What do you think? Could we let a user burn a PD?
This seems like a very rare case, and if the PD is designed such that
it's reliant on the PSE's current limiting ability then seems like it's
just an accident waiting to happen with any PSE.
Very rarely have we seen a device actually pull more current than it's
classification result allows (except for LLDP negotiation). What's more
likely is a dual-channel 802.3bt device is incorrectly classified as a
single-channel 802.3at device; the device pulls more current than
allocated and gets shut off promptly, but no magic smoke escaped.
Hope this helps!
Kyle
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-25 20:40 ` Kyle Swenson
@ 2025-03-26 10:01 ` Oleksij Rempel
2025-03-26 14:35 ` Kory Maincent
0 siblings, 1 reply; 36+ messages in thread
From: Oleksij Rempel @ 2025-03-26 10:01 UTC (permalink / raw)
To: Kyle Swenson
Cc: Kory Maincent, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Thomas Petazzoni, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, Dent Project, kernel@pengutronix.de,
Maxime Chevallier, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi folks,
On Tue, Mar 25, 2025 at 08:40:54PM +0000, Kyle Swenson wrote:
> Hello Kory,
>
> On Tue, Mar 25, 2025 at 04:25:34PM +0100, Kory Maincent wrote:
> > On Tue, 25 Mar 2025 06:34:17 +0100
> > Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> >
> > > Hi,
> > >
> > > On Mon, Mar 24, 2025 at 05:33:18PM +0000, Kyle Swenson wrote:
> > > > Hello Kory,
> > > >
> > > > On Mon, Mar 24, 2025 at 05:39:07PM +0100, Kory Maincent wrote:
> > > > > Hello Kyle, Oleksij,
> > > > ...
> > > > >
> > > > > Small question on PSE core behavior for PoE users.
> > > > >
> > > > > If we want to enable a port but we can't due to over budget.
> > > > > Should we :
> > > > > - Report an error (or not) and save the enable action from userspace. On
> > > > > that case, if enough budget is available later due to priority change or
> > > > > port disconnected the PSE core will try automatically to re enable the
> > > > > PoE port. The port will then be enabled without any action from the user.
> > > > > - Report an error but do nothing. The user will need to rerun the enable
> > > > > command later to try to enable the port again.
> > > > >
> > > > > How is it currently managed in PoE poprietary userspace tools?
> > > >
> > > > So in our implementation, we're using the first option you've presented.
> > > > That is, we save the enable action from the user and if we can't power
> > > > the device due to insufficient budget remaining, we'll indicate that status
> > > > to the user. If enough power budget becomes available later, we'll power up
> > > > the device automatically.
> > >
> > > It seems to be similar to administrative UP state - "ip link set dev lan1 up".
> > > I'm ok with this behavior.
> >
> > Ack I will go for it then, thank you!
> >
> > Other question to both of you:
> > If we configure manually the current limit for a port. Then we plug a Powered
> > Device and we detect (during the classification) a smaller current limit
> > supported. Should we change the current limit to the one detected. On that case
> > we should not let the user set a power limit greater than the one detected after
> > the PD has been plugged.
>
> I don't know that we want to prevent the user from setting a higher
> current than a device's classification current because that would
> prevent the PD and PSE negotiating a higher current via LLDP.
>
> That said, I'm struggling to think of a use-case where the user would be
> setting a current limit before a PD is connected, so maybe we can reset
> the current limit when the PD is classified to the classification
> result, but also allow it to be adjusted after a PD is powered for the
> LLDP negotiation case.
>
> In our implementation, don't really let the user specify something like,
> "Only class 3 and lower devices on this port" because we've not seen
> customers need this. We have, however, implemented the LLDP negotiation
> support after several requests from customers, but this only makes sense
> when a PD is powered at it's initial classification result. The PD can
> then request more power (via LLDP) and then we adjust the current limit
> assuming the system has budget available for the request.
>
> >
> > What do you think? Could we let a user burn a PD?
>
> This seems like a very rare case, and if the PD is designed such that
> it's reliant on the PSE's current limiting ability then seems like it's
> just an accident waiting to happen with any PSE.
>
> Very rarely have we seen a device actually pull more current than it's
> classification result allows (except for LLDP negotiation). What's more
> likely is a dual-channel 802.3bt device is incorrectly classified as a
> single-channel 802.3at device; the device pulls more current than
> allocated and gets shut off promptly, but no magic smoke escaped.
Here’s my understanding of the use cases described so far, and a proposal for
how we could handle them in the kernel to avoid conflicts between different
actors.
We have multiple components that may affect power delivery:
- The kernel, which reacts to detection and classification
- The admin, who might want to override or restrict power for policy or safety reasons
- The LLDP daemon, which may request more power dynamically based on what the PD asks for
To avoid races and make things more predictable, I think it's best if each
actor has its own dedicated input.
## Use Cases
### Use Case 1: Classification-based power (default behavior)
- Kernel detects PD and performs classification
- Power is applied according to classification and hardware limits
- No override used
Steps:
1. Detection runs
2. Classification result obtained (e.g. Class 2 → 7W)
3. Kernel computes:
effective_limit = min(
classification_result,
controller_capability,
board_limit,
dynamic_budget
)
4. Power applied up to `effective_limit`
### Use Case 2: Admin-configured upper bound (non-override)
- Admin sets a policy limit that restricts all power delivery
- Does not override classification, only bounds it
Steps:
1. Admin sets `ETHTOOL_A_C33_PSE_AVAIL_PWR_LIMIT = 15000`
2. Detection + classification run normally
3. Kernel computes:
effective_limit = min(
classification_result,
AVAIL_PWR_LIMIT,
controller_capability,
board_limit,
dynamic_budget
)
4. Classification is respected, but never exceeds admin limit
This value is always included in power computation — even if classification
or LLDP overrides are active.
### Use Case 3: Persistent classification override (admin)
- Admin sets a persistent limit that overrides classification
- Power is always based on this override
Steps:
1. Admin sets `CLASS_OVERRIDE_PERSISTENT = 25000` (mW)
2. Detection/classification may run, but classification result is ignored
3. Kernel computes:
effective_limit = min(
CLASS_OVERRIDE_PERSISTENT,
AVAIL_PWR_LIMIT,
controller_capability,
board_limit,
dynamic_budget
)
4. Power applied accordingly
5. Override persists until cleared
### Use Case 4: Temporary classification override (LLDP)
- LLDP daemon overrides classification for current PD session only
- Cleared automatically on PD disconnect
Steps:
1. PD connects, detection + classification runs (e.g. 7W)
2. LLDP daemon receives PD request for 25000 mW
3. LLDP daemon sets `CLASS_OVERRIDE_TEMPORARY = 25000`
4. Kernel computes:
effective_limit = min(
CLASS_OVERRIDE_TEMPORARY,
AVAIL_PWR_LIMIT,
controller_capability,
board_limit,
dynamic_budget
)
5. Power is increased for this session
6. On PD disconnect, override is cleared automatically
---
### Use Case 5: Ignore detection and classification (force-on)
- Admin forces the port on, ignoring detection
- Useful for passive/non-802.3 devices or bring-up
Steps:
1. Admin sets:
- `DETECTION_IGNORE = true`
- `CLASS_OVERRIDE_PERSISTENT = 5000`
2. Kernel skips detection and classification
3. Kernel computes:
effective_limit = min(
CLASS_OVERRIDE_PERSISTENT,
AVAIL_PWR_LIMIT,
controller_capability,
board_limit,
dynamic_budget
)
4. Power is applied immediately
## Proposed kernel UAPI
### SET attributes (configuration input)
| Attribute | Type | Lifetime | Owner | Description |
|-------------------------------------------|----------|------------------------|------------------|-------------|
| `ETHTOOL_A_PSE_CLASS_OVERRIDE_PERSISTENT` | u32 (mW) | Until cleared | Admin | Persistent classification override |
| `ETHTOOL_A_PSE_CLASS_OVERRIDE_TEMPORARY` | u32 (mW) | Cleared on detection failure / PD replug | LLDP daemon / test tool | Temporary override of classification |
| `ETHTOOL_A_PSE_DETECTION_IGNORE` | bool | Until cleared | Admin | Ignore detection phase |
| `ETHTOOL_A_C33_PSE_AVAIL_PWR_LIMIT` | u32 (mW) | Until changed | Admin | Static admin-defined max power cap (non-override) |
### GET attributes (status and diagnostics)
| Attribute | Type | Description |
|--------------------------------------------|----------|-------------|
| `ETHTOOL_A_PSE_EFFECTIVE_PWR_LIMIT` | u32 (mW) | Final power limit applied by kernel |
| `ETHTOOL_A_PSE_CLASS_OVERRIDE_PERSISTENT` | u32 (mW) | Current persistent override (if set) |
| `ETHTOOL_A_PSE_CLASS_OVERRIDE_TEMPORARY` | u32 (mW) | Current temporary override (if active) |
| `ETHTOOL_A_PSE_DETECTION_IGNORE` | bool | Current detection ignore state |
### Power Limit Priority
Since we now have multiple sources that can influence how much power is
delivered to a PD, we need to define a clear and deterministic priority
order for all these values. This avoids confusion and ensures that the kernel
behaves consistently, even when different actors (e.g. admin, LLDP daemon,
hardware limits) are active at the same time.
Below is the proposed priority list — values higher in the list take precedence
over those below:
| Priority | Source / Field | Description |
|----------|------------------------------------------|-------------|
| 1 | Hardware/board-specific limit | Maximum allowed by controller or board design (e.g. via device tree or driver constraints) |
| 2 | Dynamic power budget | Current system-level or PSE-level power availability (shared with other ports) |
| 3 | `ETHTOOL_A_C33_PSE_AVAIL_PWR_LIMIT` | Admin-configured upper bound — applies even when classification or override is used |
| 4 | `ETHTOOL_A_PSE_CLASS_OVERRIDE_TEMPORARY` | Temporary override, e.g. set by LLDP daemon, cleared on PD disconnect or detection loss |
| 5 | `ETHTOOL_A_PSE_CLASS_OVERRIDE_PERSISTENT` | Admin override that persists until cleared |
| 6 | `ETHTOOL_A_PSE_CLASSIFICATION_RESULT` | Result of PD classification, used when no override is present |
The effective power limit used by the kernel will always be the minimum of the
values above.
This way, even if the LLDP daemon requests more power, or classification result
is high, power delivery will still be constrained by admin policies, hardware
limits, and current budget.
Best regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies
2025-03-26 10:01 ` Oleksij Rempel
@ 2025-03-26 14:35 ` Kory Maincent
0 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-26 14:35 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Kyle Swenson, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Thomas Petazzoni, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, Dent Project, kernel@pengutronix.de,
Maxime Chevallier, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
On Wed, 26 Mar 2025 11:01:19 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> Hi folks,
>
> On Tue, Mar 25, 2025 at 08:40:54PM +0000, Kyle Swenson wrote:
> > Hello Kory,
> >
> > On Tue, Mar 25, 2025 at 04:25:34PM +0100, Kory Maincent wrote:
> > > On Tue, 25 Mar 2025 06:34:17 +0100
> > > Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > >
> [...]
> [...]
> [...]
> [...]
> [...]
> [...]
> [...]
> > >
> > > Ack I will go for it then, thank you!
> > >
> > > Other question to both of you:
> > > If we configure manually the current limit for a port. Then we plug a
> > > Powered Device and we detect (during the classification) a smaller
> > > current limit supported. Should we change the current limit to the one
> > > detected. On that case we should not let the user set a power limit
> > > greater than the one detected after the PD has been plugged.
> >
> > I don't know that we want to prevent the user from setting a higher
> > current than a device's classification current because that would
> > prevent the PD and PSE negotiating a higher current via LLDP.
> >
> > That said, I'm struggling to think of a use-case where the user would be
> > setting a current limit before a PD is connected, so maybe we can reset
> > the current limit when the PD is classified to the classification
> > result, but also allow it to be adjusted after a PD is powered for the
> > LLDP negotiation case.
> >
> > In our implementation, don't really let the user specify something like,
> > "Only class 3 and lower devices on this port" because we've not seen
> > customers need this. We have, however, implemented the LLDP negotiation
> > support after several requests from customers, but this only makes sense
> > when a PD is powered at it's initial classification result. The PD can
> > then request more power (via LLDP) and then we adjust the current limit
> > assuming the system has budget available for the request.
> >
> > >
> > > What do you think? Could we let a user burn a PD?
> >
> > This seems like a very rare case, and if the PD is designed such that
> > it's reliant on the PSE's current limiting ability then seems like it's
> > just an accident waiting to happen with any PSE.
> >
> > Very rarely have we seen a device actually pull more current than it's
> > classification result allows (except for LLDP negotiation). What's more
> > likely is a dual-channel 802.3bt device is incorrectly classified as a
> > single-channel 802.3at device; the device pulls more current than
> > allocated and gets shut off promptly, but no magic smoke escaped.
>
> Here’s my understanding of the use cases described so far, and a proposal for
> how we could handle them in the kernel to avoid conflicts between different
> actors.
>
> We have multiple components that may affect power delivery:
> - The kernel, which reacts to detection and classification
> - The admin, who might want to override or restrict power for policy or
> safety reasons
> - The LLDP daemon, which may request more power dynamically based on what the
> PD asks for
>
> To avoid races and make things more predictable, I think it's best if each
> actor has its own dedicated input.
>
> ## Use Cases
>
> ### Use Case 1: Classification-based power (default behavior)
> - Kernel detects PD and performs classification
> - Power is applied according to classification and hardware limits
> - No override used
>
> Steps:
> 1. Detection runs
> 2. Classification result obtained (e.g. Class 2 → 7W)
> 3. Kernel computes:
>
> effective_limit = min(
> classification_result,
> controller_capability,
> board_limit,
> dynamic_budget
> )
>
> 4. Power applied up to `effective_limit`
>
> ### Use Case 2: Admin-configured upper bound (non-override)
> - Admin sets a policy limit that restricts all power delivery
> - Does not override classification, only bounds it
>
> Steps:
> 1. Admin sets `ETHTOOL_A_C33_PSE_AVAIL_PWR_LIMIT = 15000`
> 2. Detection + classification run normally
> 3. Kernel computes:
>
> effective_limit = min(
> classification_result,
> AVAIL_PWR_LIMIT,
> controller_capability,
> board_limit,
> dynamic_budget
> )
>
> 4. Classification is respected, but never exceeds admin limit
>
> This value is always included in power computation — even if classification
> or LLDP overrides are active.
>
> ### Use Case 3: Persistent classification override (admin)
> - Admin sets a persistent limit that overrides classification
> - Power is always based on this override
>
> Steps:
> 1. Admin sets `CLASS_OVERRIDE_PERSISTENT = 25000` (mW)
> 2. Detection/classification may run, but classification result is ignored
> 3. Kernel computes:
>
> effective_limit = min(
> CLASS_OVERRIDE_PERSISTENT,
> AVAIL_PWR_LIMIT,
> controller_capability,
> board_limit,
> dynamic_budget
> )
>
> 4. Power applied accordingly
> 5. Override persists until cleared
>
> ### Use Case 4: Temporary classification override (LLDP)
> - LLDP daemon overrides classification for current PD session only
> - Cleared automatically on PD disconnect
>
> Steps:
> 1. PD connects, detection + classification runs (e.g. 7W)
> 2. LLDP daemon receives PD request for 25000 mW
> 3. LLDP daemon sets `CLASS_OVERRIDE_TEMPORARY = 25000`
> 4. Kernel computes:
>
> effective_limit = min(
> CLASS_OVERRIDE_TEMPORARY,
> AVAIL_PWR_LIMIT,
> controller_capability,
> board_limit,
> dynamic_budget
> )
>
> 5. Power is increased for this session
> 6. On PD disconnect, override is cleared automatically
>
> ---
>
> ### Use Case 5: Ignore detection and classification (force-on)
> - Admin forces the port on, ignoring detection
> - Useful for passive/non-802.3 devices or bring-up
>
> Steps:
> 1. Admin sets:
> - `DETECTION_IGNORE = true`
> - `CLASS_OVERRIDE_PERSISTENT = 5000`
> 2. Kernel skips detection and classification
> 3. Kernel computes:
>
> effective_limit = min(
> CLASS_OVERRIDE_PERSISTENT,
> AVAIL_PWR_LIMIT,
> controller_capability,
> board_limit,
> dynamic_budget
> )
>
> 4. Power is applied immediately
>
> ## Proposed kernel UAPI
>
> ### SET attributes (configuration input)
>
> | Attribute | Type | Lifetime
> | Owner | Description |
> |-------------------------------------------|----------|------------------------|------------------|-------------|
> | `ETHTOOL_A_PSE_CLASS_OVERRIDE_PERSISTENT` | u32 (mW) | Until cleared
> | Admin | Persistent classification override | |
> `ETHTOOL_A_PSE_CLASS_OVERRIDE_TEMPORARY` | u32 (mW) | Cleared on detection
> failure / PD replug | LLDP daemon / test tool | Temporary override of
> classification | | `ETHTOOL_A_PSE_DETECTION_IGNORE` | bool |
> Until cleared | Admin | Ignore detection phase | |
> `ETHTOOL_A_C33_PSE_AVAIL_PWR_LIMIT` | u32 (mW) | Until changed
> | Admin | Static admin-defined max power cap (non-override) |
>
> ### GET attributes (status and diagnostics)
>
> | Attribute | Type | Description |
> |--------------------------------------------|----------|-------------|
> | `ETHTOOL_A_PSE_EFFECTIVE_PWR_LIMIT` | u32 (mW) | Final power limit
> applied by kernel | | `ETHTOOL_A_PSE_CLASS_OVERRIDE_PERSISTENT` | u32 (mW) |
> Current persistent override (if set) | |
> `ETHTOOL_A_PSE_CLASS_OVERRIDE_TEMPORARY` | u32 (mW) | Current temporary
> override (if active) | | `ETHTOOL_A_PSE_DETECTION_IGNORE` | bool
> | Current detection ignore state |
>
> ### Power Limit Priority
>
> Since we now have multiple sources that can influence how much power is
> delivered to a PD, we need to define a clear and deterministic priority
> order for all these values. This avoids confusion and ensures that the kernel
> behaves consistently, even when different actors (e.g. admin, LLDP daemon,
> hardware limits) are active at the same time.
>
> Below is the proposed priority list — values higher in the list take
> precedence over those below:
>
> | Priority | Source / Field | Description |
> |----------|------------------------------------------|-------------|
> | 1 | Hardware/board-specific limit | Maximum allowed by
> controller or board design (e.g. via device tree or driver constraints) | | 2
> | Dynamic power budget | Current system-level or
> PSE-level power availability (shared with other ports) | | 3 |
> `ETHTOOL_A_C33_PSE_AVAIL_PWR_LIMIT` | Admin-configured upper bound —
> applies even when classification or override is used | | 4 |
> `ETHTOOL_A_PSE_CLASS_OVERRIDE_TEMPORARY` | Temporary override, e.g. set by
> LLDP daemon, cleared on PD disconnect or detection loss | | 5 |
> `ETHTOOL_A_PSE_CLASS_OVERRIDE_PERSISTENT` | Admin override that persists
> until cleared | | 6 | `ETHTOOL_A_PSE_CLASSIFICATION_RESULT` |
> Result of PD classification, used when no override is present |
>
> The effective power limit used by the kernel will always be the minimum of the
> values above.
>
> This way, even if the LLDP daemon requests more power, or classification
> result is high, power delivery will still be constrained by admin policies,
> hardware limits, and current budget.
Kyle thanks for your PoE user side answer!
Oleksij, thanks, as usual you have done an intense brainstorm! ^^
These two replies could be helpful in the future.
I asked this because I found out that the over current event of the TPS23881
was resetting the power limit register of the port. I think I will simply
fix it by reconfiguring the power limit if this event happens.
So it won't change the current behavior where the user setting of power limit
prevail over the power limit detected during classification.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v6 07/12] net: ethtool: Add PSE new budget evaluation strategy support feature
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
` (5 preceding siblings ...)
2025-03-04 10:18 ` [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies Kory Maincent
@ 2025-03-04 10:18 ` Kory Maincent
2025-03-04 10:18 ` [PATCH net-next v6 08/12] net: pse-pd: pd692x0: Add support for PSE PI priority feature Kory Maincent
` (4 subsequent siblings)
11 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:18 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project)
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
This patch expands the status information provided by ethtool for PSE c33
with current port priority and max port priority. It also adds a call to
pse_ethtool_set_prio() to configure the PSE port priority.
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
Change in v6:
- Remove c33 standard reference from new netlink field in documentation.
- Remove report of budget evaluation strategy.
Change in v4:
- Remove disconnection policy features.
- Rename port priority to budget evaluation strategy.
Change in v3:
- Add disconnection policy.
Change in v2:
- Improve port priority documentation.
- Add port priority modes.
---
Documentation/netlink/specs/ethtool.yaml | 11 +++++++++++
Documentation/networking/ethtool-netlink.rst | 26 ++++++++++++++++++++++++++
include/uapi/linux/ethtool_netlink_generated.h | 2 ++
net/ethtool/pse-pd.c | 18 ++++++++++++++++++
4 files changed, 57 insertions(+)
diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index cad52cf43b938..830678db3081d 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -1370,6 +1370,14 @@ attribute-sets:
name: pse-pw-d-id
type: u32
name-prefix: ethtool-a-
+ -
+ name: pse-prio-max
+ type: u32
+ name-prefix: ethtool-a-
+ -
+ name: pse-prio
+ type: u32
+ name-prefix: ethtool-a-
-
name: rss
attr-cnt-name: __ethtool-a-rss-cnt
@@ -2190,6 +2198,8 @@ operations:
- c33-pse-avail-pw-limit
- c33-pse-pw-limit-ranges
- pse-pw-d-id
+ - pse-prio-max
+ - pse-prio
dump: *pse-get-op
-
name: pse-set
@@ -2204,6 +2214,7 @@ operations:
- podl-pse-admin-control
- c33-pse-admin-control
- c33-pse-avail-pw-limit
+ - pse-prio
-
name: rss-get
doc: Get RSS params.
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 49c9a0e79af56..efc410ae26c87 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1790,6 +1790,10 @@ Kernel response contents:
``ETHTOOL_A_C33_PSE_PW_LIMIT_RANGES`` nested Supported power limit
configuration ranges.
``ETHTOOL_A_PSE_PW_D_ID`` u32 Index of the PSE power domain
+ ``ETHTOOL_A_PSE_PRIO_MAX`` u32 Priority maximum configurable
+ on the PoE PSE
+ ``ETHTOOL_A_PSE_PRIO`` u32 Priority of the PoE PSE
+ currently configured
========================================== ====== =============================
When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` attribute identifies
@@ -1866,6 +1870,12 @@ equal.
The ``ETHTOOL_A_PSE_PW_D_ID`` attribute identifies the index of PSE power
domain.
+When set, the optional ``ETHTOOL_A_PSE_PRIO_MAX`` attribute identifies
+the PSE maximum priority value.
+When set, the optional ``ETHTOOL_A_PSE_PRIO`` attributes is used to
+identifies the currently configured PSE priority.
+For a description of PSE priority attributes, see ``PSE_SET``.
+
PSE_SET
=======
@@ -1879,6 +1889,8 @@ Request contents:
``ETHTOOL_A_C33_PSE_ADMIN_CONTROL`` u32 Control PSE Admin state
``ETHTOOL_A_C33_PSE_AVAIL_PWR_LIMIT`` u32 Control PoE PSE available
power limit
+ ``ETHTOOL_A_PSE_PRIO`` u32 Control priority of the
+ PoE PSE
====================================== ====== =============================
When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL`` attribute is used
@@ -1901,6 +1913,20 @@ various existing products that document power consumption in watts rather than
classes. If power limit configuration based on classes is needed, the
conversion can be done in user space, for example by ethtool.
+When set, the optional ``ETHTOOL_A_PSE_PRIO`` attributes is used to
+control the PSE priority. Allowed priority value are between zero and
+the value of ``ETHTOOL_A_PSE_PRIO_MAX`` attribute.
+
+A lower value indicates a higher priority, meaning that a priority value
+of 0 corresponds to the highest port priority.
+Port priority serves two functions:
+
+ - Power-up Order: After a reset, ports are powered up in order of their
+ priority from highest to lowest. Ports with higher priority
+ (lower values) power up first.
+ - Shutdown Order: When the power budget is exceeded, ports with lower
+ priority (higher values) are turned off first.
+
PSE_NTF
=======
diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
index ccd7ab3bf1b11..0220cd83728ab 100644
--- a/include/uapi/linux/ethtool_netlink_generated.h
+++ b/include/uapi/linux/ethtool_netlink_generated.h
@@ -634,6 +634,8 @@ enum {
ETHTOOL_A_C33_PSE_AVAIL_PW_LIMIT,
ETHTOOL_A_C33_PSE_PW_LIMIT_RANGES,
ETHTOOL_A_PSE_PW_D_ID,
+ ETHTOOL_A_PSE_PRIO_MAX,
+ ETHTOOL_A_PSE_PRIO,
__ETHTOOL_A_PSE_CNT,
ETHTOOL_A_PSE_MAX = (__ETHTOOL_A_PSE_CNT - 1)
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index 950c1d4ef4e06..bc6f582645778 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -112,6 +112,9 @@ static int pse_reply_size(const struct ethnl_req_info *req_base,
len += st->c33_pw_limit_nb_ranges *
(nla_total_size(0) +
nla_total_size(sizeof(u32)) * 2);
+ if (st->prio_max)
+ /* _PSE_PRIO_MAX + _PSE_PRIO */
+ len += nla_total_size(sizeof(u32)) * 2;
return len;
}
@@ -206,6 +209,11 @@ static int pse_fill_reply(struct sk_buff *skb,
pse_put_pw_limit_ranges(skb, st))
return -EMSGSIZE;
+ if (st->prio_max > 0 &&
+ (nla_put_u32(skb, ETHTOOL_A_PSE_PRIO_MAX, st->prio_max) ||
+ nla_put_u32(skb, ETHTOOL_A_PSE_PRIO, st->prio)))
+ return -EMSGSIZE;
+
return 0;
}
@@ -227,6 +235,7 @@ const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1] = {
NLA_POLICY_RANGE(NLA_U32, ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED,
ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED),
[ETHTOOL_A_C33_PSE_AVAIL_PW_LIMIT] = { .type = NLA_U32 },
+ [ETHTOOL_A_PSE_PRIO] = { .type = NLA_U32 },
};
static int
@@ -275,6 +284,15 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
if (ret)
return ret;
+ if (tb[ETHTOOL_A_PSE_PRIO]) {
+ unsigned int prio;
+
+ prio = nla_get_u32(tb[ETHTOOL_A_PSE_PRIO]);
+ ret = pse_ethtool_set_prio(phydev->psec, info->extack, prio);
+ if (ret)
+ return ret;
+ }
+
if (tb[ETHTOOL_A_C33_PSE_AVAIL_PW_LIMIT]) {
unsigned int pw_limit;
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v6 08/12] net: pse-pd: pd692x0: Add support for PSE PI priority feature
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
` (6 preceding siblings ...)
2025-03-04 10:18 ` [PATCH net-next v6 07/12] net: ethtool: Add PSE new budget evaluation strategy support feature Kory Maincent
@ 2025-03-04 10:18 ` Kory Maincent
2025-03-17 13:20 ` Oleksij Rempel
2025-03-04 10:18 ` [PATCH net-next v6 09/12] net: pse-pd: pd692x0: Add support for controller and manager power supplies Kory Maincent
` (3 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:18 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project)
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
This patch extends the PSE callbacks by adding support for the newly
introduced pi_set_prio() callback, enabling the configuration of PSE PI
priorities. The current port priority is now also included in the status
information returned to users.
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
Changes in v3:
- New patch
---
drivers/net/pse-pd/pd692x0.c | 205 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 205 insertions(+)
diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
index 7d60a714ca536..44ded2aa6fcab 100644
--- a/drivers/net/pse-pd/pd692x0.c
+++ b/drivers/net/pse-pd/pd692x0.c
@@ -12,6 +12,8 @@
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pse-pd/pse.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
#define PD692X0_PSE_NAME "pd692x0_pse"
@@ -76,6 +78,8 @@ enum {
PD692X0_MSG_GET_PORT_CLASS,
PD692X0_MSG_GET_PORT_MEAS,
PD692X0_MSG_GET_PORT_PARAM,
+ PD692X0_MSG_GET_POWER_BANK,
+ PD692X0_MSG_SET_POWER_BANK,
/* add new message above here */
PD692X0_MSG_CNT
@@ -95,6 +99,8 @@ struct pd692x0_priv {
unsigned long last_cmd_key_time;
enum ethtool_c33_pse_admin_state admin_state[PD692X0_MAX_PIS];
+ struct regulator_dev *manager_reg[PD692X0_MAX_MANAGERS];
+ int manager_pw_budget[PD692X0_MAX_MANAGERS];
};
/* Template list of communication messages. The non-null bytes defined here
@@ -170,6 +176,16 @@ static const struct pd692x0_msg pd692x0_msg_template_list[PD692X0_MSG_CNT] = {
.data = {0x4e, 0x4e, 0x4e, 0x4e,
0x4e, 0x4e, 0x4e, 0x4e},
},
+ [PD692X0_MSG_GET_POWER_BANK] = {
+ .key = PD692X0_KEY_REQ,
+ .sub = {0x07, 0x0b, 0x57},
+ .data = { 0, 0x4e, 0x4e, 0x4e,
+ 0x4e, 0x4e, 0x4e, 0x4e},
+ },
+ [PD692X0_MSG_SET_POWER_BANK] = {
+ .key = PD692X0_KEY_CMD,
+ .sub = {0x07, 0x0b, 0x57},
+ },
};
static u8 pd692x0_build_msg(struct pd692x0_msg *msg, u8 echo)
@@ -739,6 +755,29 @@ pd692x0_pi_get_actual_pw(struct pse_controller_dev *pcdev, int id)
return (buf.data[0] << 4 | buf.data[1]) * 100;
}
+static int
+pd692x0_pi_get_prio(struct pse_controller_dev *pcdev, int id)
+{
+ struct pd692x0_priv *priv = to_pd692x0_priv(pcdev);
+ struct pd692x0_msg msg, buf = {0};
+ int ret;
+
+ ret = pd692x0_fw_unavailable(priv);
+ if (ret)
+ return ret;
+
+ msg = pd692x0_msg_template_list[PD692X0_MSG_GET_PORT_PARAM];
+ msg.sub[2] = id;
+ ret = pd692x0_sendrecv_msg(priv, &msg, &buf);
+ if (ret < 0)
+ return ret;
+ if (buf.data[2] < 1 || 3 < buf.data[2])
+ return -ERANGE;
+
+ /* PSE core priority start at 0 */
+ return buf.data[2] - 1;
+}
+
static struct pd692x0_msg_ver pd692x0_get_sw_version(struct pd692x0_priv *priv)
{
struct device *dev = &priv->client->dev;
@@ -766,6 +805,7 @@ static struct pd692x0_msg_ver pd692x0_get_sw_version(struct pd692x0_priv *priv)
struct pd692x0_manager {
struct device_node *port_node[PD692X0_MAX_MANAGER_PORTS];
+ struct device_node *node;
int nports;
};
@@ -857,6 +897,8 @@ pd692x0_of_get_managers(struct pd692x0_priv *priv,
if (ret)
goto out;
+ of_node_get(node);
+ manager[manager_id].node = node;
nmanagers++;
}
@@ -869,6 +911,8 @@ pd692x0_of_get_managers(struct pd692x0_priv *priv,
of_node_put(manager[i].port_node[j]);
manager[i].port_node[j] = NULL;
}
+ of_node_put(manager[i].node);
+ manager[i].node = NULL;
}
of_node_put(node);
@@ -876,6 +920,130 @@ pd692x0_of_get_managers(struct pd692x0_priv *priv,
return ret;
}
+static const struct regulator_ops dummy_ops;
+
+static struct regulator_dev *
+pd692x0_register_manager_regulator(struct device *dev, char *reg_name,
+ struct device_node *node)
+{
+ struct regulator_init_data *rinit_data;
+ struct regulator_config rconfig = {0};
+ struct regulator_desc *rdesc;
+ struct regulator_dev *rdev;
+
+ rinit_data = devm_kzalloc(dev, sizeof(*rinit_data),
+ GFP_KERNEL);
+ if (!rinit_data)
+ return ERR_PTR(-ENOMEM);
+
+ rdesc = devm_kzalloc(dev, sizeof(*rdesc), GFP_KERNEL);
+ if (!rdesc)
+ return ERR_PTR(-ENOMEM);
+
+ rdesc->name = reg_name;
+ rdesc->type = REGULATOR_VOLTAGE;
+ rdesc->ops = &dummy_ops;
+ rdesc->owner = THIS_MODULE;
+
+ rinit_data->supply_regulator = "vmain";
+
+ rconfig.dev = dev;
+ rconfig.init_data = rinit_data;
+ rconfig.of_node = node;
+
+ rdev = devm_regulator_register(dev, rdesc, &rconfig);
+ if (IS_ERR(rdev)) {
+ dev_err_probe(dev, PTR_ERR(rdev),
+ "Failed to register regulator\n");
+ return rdev;
+ }
+
+ return rdev;
+}
+
+static int
+pd692x0_register_managers_regulator(struct pd692x0_priv *priv,
+ const struct pd692x0_manager *manager,
+ int nmanagers)
+{
+ struct device *dev = &priv->client->dev;
+ size_t reg_name_len;
+ int i;
+
+ /* Each regulator name len is dev name + 12 char +
+ * int max digit number (10) + 1
+ */
+ reg_name_len = strlen(dev_name(dev)) + 23;
+
+ for (i = 0; i < nmanagers; i++) {
+ struct regulator_dev *rdev;
+ char *reg_name;
+
+ reg_name = devm_kzalloc(dev, reg_name_len, GFP_KERNEL);
+ if (!reg_name)
+ return -ENOMEM;
+ snprintf(reg_name, 26, "pse-%s-manager%d", dev_name(dev), i);
+ rdev = pd692x0_register_manager_regulator(dev, reg_name,
+ manager[i].node);
+ if (IS_ERR(rdev))
+ return PTR_ERR(rdev);
+
+ priv->manager_reg[i] = rdev;
+ }
+
+ return 0;
+}
+
+static int
+pd692x0_conf_manager_power_budget(struct pd692x0_priv *priv, int id, int pw)
+{
+ struct pd692x0_msg msg, buf;
+ int ret, pw_mW = pw / 1000;
+
+ msg = pd692x0_msg_template_list[PD692X0_MSG_GET_POWER_BANK];
+ msg.data[0] = id;
+ ret = pd692x0_sendrecv_msg(priv, &msg, &buf);
+ if (ret < 0)
+ return ret;
+
+ msg = pd692x0_msg_template_list[PD692X0_MSG_SET_POWER_BANK];
+ msg.data[0] = id;
+ msg.data[1] = pw_mW >> 8;
+ msg.data[2] = pw_mW & 0xff;
+ msg.data[3] = buf.sub[2];
+ msg.data[4] = buf.data[0];
+ msg.data[5] = buf.data[1];
+ msg.data[6] = buf.data[2];
+ msg.data[7] = buf.data[3];
+ return pd692x0_sendrecv_msg(priv, &msg, &buf);
+}
+
+static int
+pd692x0_configure_managers(struct pd692x0_priv *priv, int nmanagers)
+{
+ int i, ret;
+
+ for (i = 0; i < nmanagers; i++) {
+ struct regulator *supply = priv->manager_reg[i]->supply;
+ int pw_budget;
+
+ pw_budget = regulator_get_unclaimed_power_budget(supply);
+ /* Max power budget per manager */
+ if (pw_budget > 6000000)
+ pw_budget = 6000000;
+ ret = regulator_request_power_budget(supply, pw_budget);
+ if (ret < 0)
+ return ret;
+
+ priv->manager_pw_budget[i] = pw_budget;
+ ret = pd692x0_conf_manager_power_budget(priv, i, pw_budget);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
static int
pd692x0_set_port_matrix(const struct pse_pi_pairset *pairset,
const struct pd692x0_manager *manager,
@@ -998,6 +1166,14 @@ static int pd692x0_setup_pi_matrix(struct pse_controller_dev *pcdev)
return ret;
nmanagers = ret;
+ ret = pd692x0_register_managers_regulator(priv, manager, nmanagers);
+ if (ret)
+ goto out;
+
+ ret = pd692x0_configure_managers(priv, nmanagers);
+ if (ret)
+ goto out;
+
ret = pd692x0_set_ports_matrix(priv, manager, nmanagers, port_matrix);
if (ret)
goto out;
@@ -1008,8 +1184,14 @@ static int pd692x0_setup_pi_matrix(struct pse_controller_dev *pcdev)
out:
for (i = 0; i < nmanagers; i++) {
+ struct regulator *supply = priv->manager_reg[i]->supply;
+
+ regulator_free_power_budget(supply,
+ priv->manager_pw_budget[i]);
+
for (j = 0; j < manager[i].nports; j++)
of_node_put(manager[i].port_node[j]);
+ of_node_put(manager[i].node);
}
return ret;
}
@@ -1071,6 +1253,25 @@ static int pd692x0_pi_set_pw_limit(struct pse_controller_dev *pcdev,
return pd692x0_sendrecv_msg(priv, &msg, &buf);
}
+static int pd692x0_pi_set_prio(struct pse_controller_dev *pcdev, int id,
+ unsigned int prio)
+{
+ struct pd692x0_priv *priv = to_pd692x0_priv(pcdev);
+ struct pd692x0_msg msg, buf = {0};
+ int ret;
+
+ ret = pd692x0_fw_unavailable(priv);
+ if (ret)
+ return ret;
+
+ msg = pd692x0_msg_template_list[PD692X0_MSG_SET_PORT_PARAM];
+ msg.sub[2] = id;
+ /* Controller priority from 1 to 3 */
+ msg.data[4] = prio + 1;
+
+ return pd692x0_sendrecv_msg(priv, &msg, &buf);
+}
+
static const struct pse_controller_ops pd692x0_ops = {
.setup_pi_matrix = pd692x0_setup_pi_matrix,
.pi_get_admin_state = pd692x0_pi_get_admin_state,
@@ -1084,6 +1285,8 @@ static const struct pse_controller_ops pd692x0_ops = {
.pi_get_pw_limit = pd692x0_pi_get_pw_limit,
.pi_set_pw_limit = pd692x0_pi_set_pw_limit,
.pi_get_pw_limit_ranges = pd692x0_pi_get_pw_limit_ranges,
+ .pi_get_prio = pd692x0_pi_get_prio,
+ .pi_set_prio = pd692x0_pi_set_prio,
};
#define PD692X0_FW_LINE_MAX_SZ 0xff
@@ -1500,6 +1703,8 @@ static int pd692x0_i2c_probe(struct i2c_client *client)
priv->pcdev.ops = &pd692x0_ops;
priv->pcdev.dev = dev;
priv->pcdev.types = ETHTOOL_PSE_C33;
+ priv->pcdev.supp_budget_eval_strategies = ETHTOOL_PSE_BUDGET_EVAL_STRAT_DYNAMIC;
+ priv->pcdev.pis_prio_max = 2;
ret = devm_pse_controller_register(dev, &priv->pcdev);
if (ret)
return dev_err_probe(dev, ret,
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 08/12] net: pse-pd: pd692x0: Add support for PSE PI priority feature
2025-03-04 10:18 ` [PATCH net-next v6 08/12] net: pse-pd: pd692x0: Add support for PSE PI priority feature Kory Maincent
@ 2025-03-17 13:20 ` Oleksij Rempel
2025-03-20 16:36 ` Kory Maincent
0 siblings, 1 reply; 36+ messages in thread
From: Oleksij Rempel @ 2025-03-17 13:20 UTC (permalink / raw)
To: Kory Maincent
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Tue, Mar 04, 2025 at 11:18:57AM +0100, Kory Maincent wrote:
> static u8 pd692x0_build_msg(struct pd692x0_msg *msg, u8 echo)
> @@ -739,6 +755,29 @@ pd692x0_pi_get_actual_pw(struct pse_controller_dev *pcdev, int id)
> return (buf.data[0] << 4 | buf.data[1]) * 100;
> }
>
> +static int
> +pd692x0_pi_get_prio(struct pse_controller_dev *pcdev, int id)
> +{
> + struct pd692x0_priv *priv = to_pd692x0_priv(pcdev);
> + struct pd692x0_msg msg, buf = {0};
> + int ret;
> +
> + ret = pd692x0_fw_unavailable(priv);
> + if (ret)
> + return ret;
> +
> + msg = pd692x0_msg_template_list[PD692X0_MSG_GET_PORT_PARAM];
> + msg.sub[2] = id;
> + ret = pd692x0_sendrecv_msg(priv, &msg, &buf);
> + if (ret < 0)
> + return ret;
> + if (buf.data[2] < 1 || 3 < buf.data[2])
if (!buf.data[2] || buf.data[2] > pcdev->pis_prio_max + 1)
--
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 |
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 08/12] net: pse-pd: pd692x0: Add support for PSE PI priority feature
2025-03-17 13:20 ` Oleksij Rempel
@ 2025-03-20 16:36 ` Kory Maincent
0 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-20 16:36 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Mon, 17 Mar 2025 14:20:02 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> On Tue, Mar 04, 2025 at 11:18:57AM +0100, Kory Maincent wrote:
> > static u8 pd692x0_build_msg(struct pd692x0_msg *msg, u8 echo)
> > @@ -739,6 +755,29 @@ pd692x0_pi_get_actual_pw(struct pse_controller_dev
> > *pcdev, int id) return (buf.data[0] << 4 | buf.data[1]) * 100;
> > }
> >
> > +static int
> > +pd692x0_pi_get_prio(struct pse_controller_dev *pcdev, int id)
> > +{
> > + struct pd692x0_priv *priv = to_pd692x0_priv(pcdev);
> > + struct pd692x0_msg msg, buf = {0};
> > + int ret;
> > +
> > + ret = pd692x0_fw_unavailable(priv);
> > + if (ret)
> > + return ret;
> > +
> > + msg = pd692x0_msg_template_list[PD692X0_MSG_GET_PORT_PARAM];
> > + msg.sub[2] = id;
> > + ret = pd692x0_sendrecv_msg(priv, &msg, &buf);
> > + if (ret < 0)
> > + return ret;
> > + if (buf.data[2] < 1 || 3 < buf.data[2])
>
> if (!buf.data[2] || buf.data[2] > pcdev->pis_prio_max + 1)
Oh indeed that is better, thanks!
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v6 09/12] net: pse-pd: pd692x0: Add support for controller and manager power supplies
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
` (7 preceding siblings ...)
2025-03-04 10:18 ` [PATCH net-next v6 08/12] net: pse-pd: pd692x0: Add support for PSE PI priority feature Kory Maincent
@ 2025-03-04 10:18 ` Kory Maincent
2025-03-04 10:18 ` [PATCH net-next v6 10/12] dt-bindings: net: pse-pd: microchip,pd692x0: Add manager regulator supply Kory Maincent
` (2 subsequent siblings)
11 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:18 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project)
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
Add support for managing the VDD and VDDA power supplies for the PD692x0
PSE controller, as well as the VAUX5 and VAUX3P3 power supplies for the
PD6920x PSE managers.
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
Changes in v5:
- New patch
---
drivers/net/pse-pd/pd692x0.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
index 44ded2aa6fcab..c9fa60b314ce9 100644
--- a/drivers/net/pse-pd/pd692x0.c
+++ b/drivers/net/pse-pd/pd692x0.c
@@ -976,8 +976,10 @@ pd692x0_register_managers_regulator(struct pd692x0_priv *priv,
reg_name_len = strlen(dev_name(dev)) + 23;
for (i = 0; i < nmanagers; i++) {
+ static const char * const regulators[] = { "vaux5", "vaux3p3" };
struct regulator_dev *rdev;
char *reg_name;
+ int ret;
reg_name = devm_kzalloc(dev, reg_name_len, GFP_KERNEL);
if (!reg_name)
@@ -988,6 +990,17 @@ pd692x0_register_managers_regulator(struct pd692x0_priv *priv,
if (IS_ERR(rdev))
return PTR_ERR(rdev);
+ /* VMAIN is described as main supply for the manager.
+ * Add other VAUX power supplies and link them to the
+ * virtual device rdev->dev.
+ */
+ ret = devm_regulator_bulk_get_enable(&rdev->dev,
+ ARRAY_SIZE(regulators),
+ regulators);
+ if (ret)
+ return dev_err_probe(&rdev->dev, ret,
+ "Failed to enable regulators\n");
+
priv->manager_reg[i] = rdev;
}
@@ -1640,6 +1653,7 @@ static const struct fw_upload_ops pd692x0_fw_ops = {
static int pd692x0_i2c_probe(struct i2c_client *client)
{
+ static const char * const regulators[] = { "vdd", "vdda" };
struct pd692x0_msg msg, buf = {0}, zero = {0};
struct device *dev = &client->dev;
struct pd692x0_msg_ver ver;
@@ -1647,6 +1661,12 @@ static int pd692x0_i2c_probe(struct i2c_client *client)
struct fw_upload *fwl;
int ret;
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
+ regulators);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to enable regulators\n");
+
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
dev_err(dev, "i2c check functionality failed\n");
return -ENXIO;
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v6 10/12] dt-bindings: net: pse-pd: microchip,pd692x0: Add manager regulator supply
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
` (8 preceding siblings ...)
2025-03-04 10:18 ` [PATCH net-next v6 09/12] net: pse-pd: pd692x0: Add support for controller and manager power supplies Kory Maincent
@ 2025-03-04 10:18 ` Kory Maincent
2025-03-04 10:19 ` [PATCH net-next v6 11/12] net: pse-pd: tps23881: Add support for static port priority feature Kory Maincent
2025-03-04 10:19 ` [PATCH net-next v6 12/12] dt-bindings: net: pse-pd: ti,tps23881: Add interrupt description Kory Maincent
11 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:18 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project), Krzysztof Kozlowski
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
Adds the regulator supply parameter of the managers.
Update also the example as the regulator supply of the PSE PIs
should be the managers itself and not an external regulator.
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v5:
- Add description of others power supplies.
Changes in v3:
- New patch
---
.../bindings/net/pse-pd/microchip,pd692x0.yaml | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml b/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml
index fd4244fceced9..ca61cc37a7902 100644
--- a/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml
+++ b/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml
@@ -22,6 +22,12 @@ properties:
reg:
maxItems: 1
+ vdd-supply:
+ description: Regulator that provides 3.3V VDD power supply.
+
+ vdda-supply:
+ description: Regulator that provides 3.3V VDDA power supply.
+
managers:
type: object
additionalProperties: false
@@ -68,6 +74,15 @@ properties:
"#size-cells":
const: 0
+ vmain-supply:
+ description: Regulator that provides 44-57V VMAIN power supply.
+
+ vaux5-supply:
+ description: Regulator that provides 5V VAUX5 power supply.
+
+ vaux3p3-supply:
+ description: Regulator that provides 3.3V VAUX3P3 power supply.
+
patternProperties:
'^port@[0-7]$':
type: object
@@ -106,10 +121,11 @@ examples:
#address-cells = <1>;
#size-cells = <0>;
- manager@0 {
+ manager0: manager@0 {
reg = <0>;
#address-cells = <1>;
#size-cells = <0>;
+ vmain-supply = <&pse1_supply>;
phys0: port@0 {
reg = <0>;
@@ -161,7 +177,7 @@ examples:
pairset-names = "alternative-a", "alternative-b";
pairsets = <&phys0>, <&phys1>;
polarity-supported = "MDI", "S";
- vpwr-supply = <&vpwr1>;
+ vpwr-supply = <&manager0>;
};
pse_pi1: pse-pi@1 {
reg = <1>;
@@ -169,7 +185,7 @@ examples:
pairset-names = "alternative-a";
pairsets = <&phys2>;
polarity-supported = "MDI";
- vpwr-supply = <&vpwr2>;
+ vpwr-supply = <&manager0>;
};
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v6 11/12] net: pse-pd: tps23881: Add support for static port priority feature
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
` (9 preceding siblings ...)
2025-03-04 10:18 ` [PATCH net-next v6 10/12] dt-bindings: net: pse-pd: microchip,pd692x0: Add manager regulator supply Kory Maincent
@ 2025-03-04 10:19 ` Kory Maincent
2025-03-17 13:33 ` Oleksij Rempel
2025-03-04 10:19 ` [PATCH net-next v6 12/12] dt-bindings: net: pse-pd: ti,tps23881: Add interrupt description Kory Maincent
11 siblings, 1 reply; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:19 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project)
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
This patch enhances PSE callbacks by introducing support for the static
port priority feature. It extends interrupt management to handle and report
detection, classification, and disconnection events. Additionally, it
introduces the pi_get_pw_req() callback, which provides information about
the power requested by the Powered Devices.
Interrupt support is essential for the proper functioning of the TPS23881
controller. Without it, after a power-on (PWON), the controller will
no longer perform detection and classification. This could lead to
potential hazards, such as connecting a non-PoE device after a PoE device,
which might result in magic smoke.
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
---
We may need a fix for the interrupt support in old version of Linux.
Change in v4:
- Fix variable type nit.
Change in v3:
- New patch
---
drivers/net/pse-pd/tps23881.c | 204 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 194 insertions(+), 10 deletions(-)
diff --git a/drivers/net/pse-pd/tps23881.c b/drivers/net/pse-pd/tps23881.c
index 1226667192977..6012c58b47e8a 100644
--- a/drivers/net/pse-pd/tps23881.c
+++ b/drivers/net/pse-pd/tps23881.c
@@ -19,20 +19,30 @@
#define TPS23881_REG_IT 0x0
#define TPS23881_REG_IT_MASK 0x1
+#define TPS23881_REG_IT_DISF BIT(2)
+#define TPS23881_REG_IT_DETC BIT(3)
+#define TPS23881_REG_IT_CLASC BIT(4)
#define TPS23881_REG_IT_IFAULT BIT(5)
#define TPS23881_REG_IT_SUPF BIT(7)
+#define TPS23881_REG_DET_EVENT 0x5
#define TPS23881_REG_FAULT 0x7
#define TPS23881_REG_SUPF_EVENT 0xb
#define TPS23881_REG_TSD BIT(7)
+#define TPS23881_REG_DISC 0xc
#define TPS23881_REG_PW_STATUS 0x10
#define TPS23881_REG_OP_MODE 0x12
+#define TPS23881_REG_DISC_EN 0x13
#define TPS23881_OP_MODE_SEMIAUTO 0xaaaa
#define TPS23881_REG_DIS_EN 0x13
#define TPS23881_REG_DET_CLA_EN 0x14
#define TPS23881_REG_GEN_MASK 0x17
+#define TPS23881_REG_CLCHE BIT(2)
+#define TPS23881_REG_DECHE BIT(3)
#define TPS23881_REG_NBITACC BIT(5)
#define TPS23881_REG_INTEN BIT(7)
#define TPS23881_REG_PW_EN 0x19
+#define TPS23881_REG_RESET 0x1a
+#define TPS23881_REG_CLRAIN BIT(7)
#define TPS23881_REG_2PAIR_POL1 0x1e
#define TPS23881_REG_PORT_MAP 0x26
#define TPS23881_REG_PORT_POWER 0x29
@@ -177,6 +187,7 @@ static int tps23881_pi_enable(struct pse_controller_dev *pcdev, int id)
struct i2c_client *client = priv->client;
u8 chan;
u16 val;
+ int ret;
if (id >= TPS23881_MAX_CHANS)
return -ERANGE;
@@ -190,7 +201,22 @@ static int tps23881_pi_enable(struct pse_controller_dev *pcdev, int id)
BIT(chan % 4));
}
- return i2c_smbus_write_word_data(client, TPS23881_REG_PW_EN, val);
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_PW_EN, val);
+ if (ret)
+ return ret;
+
+ /* Enable DC disconnect*/
+ chan = priv->port[id].chan[0];
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_DISC_EN);
+ if (ret < 0)
+ return ret;
+
+ val = tps23881_set_val(ret, chan, 0, BIT(chan % 4), BIT(chan % 4));
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_DISC_EN, val);
+ if (ret)
+ return ret;
+
+ return 0;
}
static int tps23881_pi_disable(struct pse_controller_dev *pcdev, int id)
@@ -223,6 +249,17 @@ static int tps23881_pi_disable(struct pse_controller_dev *pcdev, int id)
*/
mdelay(5);
+ /* Disable DC disconnect*/
+ chan = priv->port[id].chan[0];
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_DISC_EN);
+ if (ret < 0)
+ return ret;
+
+ val = tps23881_set_val(ret, chan, 0, 0, BIT(chan % 4));
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_DISC_EN, val);
+ if (ret)
+ return ret;
+
/* Enable detection and classification */
ret = i2c_smbus_read_word_data(client, TPS23881_REG_DET_CLA_EN);
if (ret < 0)
@@ -918,6 +955,47 @@ static int tps23881_setup_pi_matrix(struct pse_controller_dev *pcdev)
return ret;
}
+static int tps23881_power_class_table[] = {
+ -ERANGE,
+ 4000,
+ 7000,
+ 15500,
+ 30000,
+ 15500,
+ 15500,
+ -ERANGE,
+ 45000,
+ 60000,
+ 75000,
+ 90000,
+ 15500,
+ 45000,
+ -ERANGE,
+ -ERANGE,
+};
+
+static int tps23881_pi_get_pw_req(struct pse_controller_dev *pcdev, int id)
+{
+ struct tps23881_priv *priv = to_tps23881_priv(pcdev);
+ struct i2c_client *client = priv->client;
+ u8 reg, chan;
+ int ret;
+ u16 val;
+
+ /* For a 4-pair the classification need 5ms to be completed */
+ if (priv->port[id].is_4p)
+ mdelay(5);
+
+ chan = priv->port[id].chan[0];
+ reg = TPS23881_REG_DISC + (chan % 4);
+ ret = i2c_smbus_read_word_data(client, reg);
+ if (ret < 0)
+ return ret;
+
+ val = tps23881_calc_val(ret, chan, 4, 0xf);
+ return tps23881_power_class_table[val];
+}
+
static const struct pse_controller_ops tps23881_ops = {
.setup_pi_matrix = tps23881_setup_pi_matrix,
.pi_enable = tps23881_pi_enable,
@@ -930,6 +1008,7 @@ static const struct pse_controller_ops tps23881_ops = {
.pi_get_pw_limit = tps23881_pi_get_pw_limit,
.pi_set_pw_limit = tps23881_pi_set_pw_limit,
.pi_get_pw_limit_ranges = tps23881_pi_get_pw_limit_ranges,
+ .pi_get_pw_req = tps23881_pi_get_pw_req,
};
static const char fw_parity_name[] = "ti/tps23881/tps23881-parity-14.bin";
@@ -1100,12 +1179,83 @@ static void tps23881_irq_event_over_current(struct tps23881_priv *priv,
ETHTOOL_PSE_EVENT_OVER_CURRENT);
}
+static void tps23881_irq_event_disconnection(struct tps23881_priv *priv,
+ u16 reg_val,
+ unsigned long *notifs,
+ unsigned long *notifs_mask)
+{
+ u8 chans;
+
+ chans = tps23881_irq_export_chans_helper(reg_val, 4);
+ if (chans)
+ tps23881_set_notifs_helper(priv, chans, notifs, notifs_mask,
+ ETHTOOL_C33_PSE_EVENT_DISCONNECTION);
+}
+
+static int tps23881_irq_event_detection(struct tps23881_priv *priv,
+ u16 reg_val,
+ unsigned long *notifs,
+ unsigned long *notifs_mask)
+{
+ enum ethtool_pse_events event;
+ int reg, ret, i, val;
+ unsigned long chans;
+
+ chans = tps23881_irq_export_chans_helper(reg_val, 0);
+ for_each_set_bit(i, &chans, TPS23881_MAX_CHANS) {
+ reg = TPS23881_REG_DISC + (i % 4);
+ ret = i2c_smbus_read_word_data(priv->client, reg);
+ if (ret < 0)
+ return ret;
+
+ val = tps23881_calc_val(ret, i, 0, 0xf);
+ /* If detection valid */
+ if (val == 0x4)
+ event = ETHTOOL_C33_PSE_EVENT_DETECTION;
+ else
+ event = ETHTOOL_C33_PSE_EVENT_DISCONNECTION;
+
+ tps23881_set_notifs_helper(priv, BIT(i), notifs,
+ notifs_mask, event);
+ }
+
+ return 0;
+}
+
+static int tps23881_irq_event_classification(struct tps23881_priv *priv,
+ u16 reg_val,
+ unsigned long *notifs,
+ unsigned long *notifs_mask)
+{
+ int reg, ret, val, i;
+ unsigned long chans;
+
+ chans = tps23881_irq_export_chans_helper(reg_val, 4);
+ for_each_set_bit(i, &chans, TPS23881_MAX_CHANS) {
+ reg = TPS23881_REG_DISC + (i % 4);
+ ret = i2c_smbus_read_word_data(priv->client, reg);
+ if (ret < 0)
+ return ret;
+
+ val = tps23881_calc_val(ret, i, 4, 0xf);
+ /* Do not report classification event for unknown class */
+ if (!val || val == 0x8 || val == 0xf)
+ continue;
+
+ tps23881_set_notifs_helper(priv, BIT(i), notifs,
+ notifs_mask,
+ ETHTOOL_C33_PSE_EVENT_CLASSIFICATION);
+ }
+
+ return 0;
+}
+
static int tps23881_irq_event_handler(struct tps23881_priv *priv, u16 reg,
unsigned long *notifs,
unsigned long *notifs_mask)
{
struct i2c_client *client = priv->client;
- int ret;
+ int ret, val;
/* The Supply event bit is repeated twice so we only need to read
* the one from the first byte.
@@ -1117,13 +1267,33 @@ static int tps23881_irq_event_handler(struct tps23881_priv *priv, u16 reg,
tps23881_irq_event_over_temp(priv, ret, notifs, notifs_mask);
}
- if (reg & (TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_IFAULT << 8)) {
+ if (reg & (TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_IFAULT << 8 |
+ TPS23881_REG_IT_DISF | TPS23881_REG_IT_DISF << 8)) {
ret = i2c_smbus_read_word_data(client, TPS23881_REG_FAULT);
if (ret < 0)
return ret;
tps23881_irq_event_over_current(priv, ret, notifs, notifs_mask);
+
+ tps23881_irq_event_disconnection(priv, ret, notifs, notifs_mask);
}
+ if (reg & (TPS23881_REG_IT_DETC | TPS23881_REG_IT_DETC << 8 |
+ TPS23881_REG_IT_CLASC | TPS23881_REG_IT_CLASC << 8)) {
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_DET_EVENT);
+ if (ret < 0)
+ return ret;
+
+ val = ret;
+ ret = tps23881_irq_event_detection(priv, val, notifs,
+ notifs_mask);
+ if (ret)
+ return ret;
+
+ ret = tps23881_irq_event_classification(priv, val, notifs,
+ notifs_mask);
+ if (ret)
+ return ret;
+ }
return 0;
}
@@ -1169,7 +1339,14 @@ static int tps23881_setup_irq(struct tps23881_priv *priv, int irq)
int ret;
u16 val;
- val = TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_SUPF;
+ if (!irq) {
+ dev_err(&client->dev, "interrupt is missing");
+ return -EINVAL;
+ }
+
+ val = TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_SUPF |
+ TPS23881_REG_IT_DETC | TPS23881_REG_IT_CLASC |
+ TPS23881_REG_IT_DISF;
val |= val << 8;
ret = i2c_smbus_write_word_data(client, TPS23881_REG_IT_MASK, val);
if (ret)
@@ -1179,11 +1356,19 @@ static int tps23881_setup_irq(struct tps23881_priv *priv, int irq)
if (ret < 0)
return ret;
- val = (u16)(ret | TPS23881_REG_INTEN | TPS23881_REG_INTEN << 8);
+ val = TPS23881_REG_INTEN | TPS23881_REG_CLCHE | TPS23881_REG_DECHE;
+ val |= val << 8;
+ val |= (u16)ret;
ret = i2c_smbus_write_word_data(client, TPS23881_REG_GEN_MASK, val);
if (ret < 0)
return ret;
+ /* Reset interrupts registers */
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_RESET,
+ TPS23881_REG_CLRAIN);
+ if (ret < 0)
+ return ret;
+
return devm_pse_irq_helper(&priv->pcdev, irq, 0, &irq_desc);
}
@@ -1261,17 +1446,16 @@ static int tps23881_i2c_probe(struct i2c_client *client)
priv->pcdev.dev = dev;
priv->pcdev.types = ETHTOOL_PSE_C33;
priv->pcdev.nr_lines = TPS23881_MAX_CHANS;
+ priv->pcdev.supp_budget_eval_strategies = ETHTOOL_PSE_BUDGET_EVAL_STRAT_STATIC;
ret = devm_pse_controller_register(dev, &priv->pcdev);
if (ret) {
return dev_err_probe(dev, ret,
"failed to register PSE controller\n");
}
- if (client->irq) {
- ret = tps23881_setup_irq(priv, client->irq);
- if (ret)
- return ret;
- }
+ ret = tps23881_setup_irq(priv, client->irq);
+ if (ret)
+ return ret;
return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 11/12] net: pse-pd: tps23881: Add support for static port priority feature
2025-03-04 10:19 ` [PATCH net-next v6 11/12] net: pse-pd: tps23881: Add support for static port priority feature Kory Maincent
@ 2025-03-17 13:33 ` Oleksij Rempel
2025-03-20 17:22 ` Kory Maincent
0 siblings, 1 reply; 36+ messages in thread
From: Oleksij Rempel @ 2025-03-17 13:33 UTC (permalink / raw)
To: Kory Maincent
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Tue, Mar 04, 2025 at 11:19:00AM +0100, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
>
> This patch enhances PSE callbacks by introducing support for the static
> port priority feature. It extends interrupt management to handle and report
> detection, classification, and disconnection events. Additionally, it
> introduces the pi_get_pw_req() callback, which provides information about
> the power requested by the Powered Devices.
>
> Interrupt support is essential for the proper functioning of the TPS23881
> controller. Without it, after a power-on (PWON), the controller will
> no longer perform detection and classification. This could lead to
> potential hazards, such as connecting a non-PoE device after a PoE device,
> which might result in magic smoke.
>
> Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> ---
>
> We may need a fix for the interrupt support in old version of Linux.
>
> Change in v4:
> - Fix variable type nit.
>
> Change in v3:
> - New patch
> ---
> drivers/net/pse-pd/tps23881.c | 204 +++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 194 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/pse-pd/tps23881.c b/drivers/net/pse-pd/tps23881.c
> index 1226667192977..6012c58b47e8a 100644
> --- a/drivers/net/pse-pd/tps23881.c
> +++ b/drivers/net/pse-pd/tps23881.c
> @@ -19,20 +19,30 @@
>
> #define TPS23881_REG_IT 0x0
> #define TPS23881_REG_IT_MASK 0x1
> +#define TPS23881_REG_IT_DISF BIT(2)
> +#define TPS23881_REG_IT_DETC BIT(3)
> +#define TPS23881_REG_IT_CLASC BIT(4)
> #define TPS23881_REG_IT_IFAULT BIT(5)
> #define TPS23881_REG_IT_SUPF BIT(7)
> +#define TPS23881_REG_DET_EVENT 0x5
> #define TPS23881_REG_FAULT 0x7
> #define TPS23881_REG_SUPF_EVENT 0xb
> #define TPS23881_REG_TSD BIT(7)
> +#define TPS23881_REG_DISC 0xc
> #define TPS23881_REG_PW_STATUS 0x10
> #define TPS23881_REG_OP_MODE 0x12
> +#define TPS23881_REG_DISC_EN 0x13
> #define TPS23881_OP_MODE_SEMIAUTO 0xaaaa
> #define TPS23881_REG_DIS_EN 0x13
> #define TPS23881_REG_DET_CLA_EN 0x14
> #define TPS23881_REG_GEN_MASK 0x17
> +#define TPS23881_REG_CLCHE BIT(2)
> +#define TPS23881_REG_DECHE BIT(3)
> #define TPS23881_REG_NBITACC BIT(5)
> #define TPS23881_REG_INTEN BIT(7)
> #define TPS23881_REG_PW_EN 0x19
> +#define TPS23881_REG_RESET 0x1a
> +#define TPS23881_REG_CLRAIN BIT(7)
> #define TPS23881_REG_2PAIR_POL1 0x1e
> #define TPS23881_REG_PORT_MAP 0x26
> #define TPS23881_REG_PORT_POWER 0x29
> @@ -177,6 +187,7 @@ static int tps23881_pi_enable(struct pse_controller_dev *pcdev, int id)
> struct i2c_client *client = priv->client;
> u8 chan;
> u16 val;
> + int ret;
>
> if (id >= TPS23881_MAX_CHANS)
> return -ERANGE;
> @@ -190,7 +201,22 @@ static int tps23881_pi_enable(struct pse_controller_dev *pcdev, int id)
> BIT(chan % 4));
> }
>
> - return i2c_smbus_write_word_data(client, TPS23881_REG_PW_EN, val);
> + ret = i2c_smbus_write_word_data(client, TPS23881_REG_PW_EN, val);
> + if (ret)
> + return ret;
> +
> + /* Enable DC disconnect*/
> + chan = priv->port[id].chan[0];
> + ret = i2c_smbus_read_word_data(client, TPS23881_REG_DISC_EN);
> + if (ret < 0)
> + return ret;
Here we have RMW operation without lock on two paths: pi_enable and
pi_disable.
> + val = tps23881_set_val(ret, chan, 0, BIT(chan % 4), BIT(chan % 4));
> + ret = i2c_smbus_write_word_data(client, TPS23881_REG_DISC_EN, val);
> + if (ret)
> + return ret;
> +
> + return 0;
> }
>
> static int tps23881_pi_disable(struct pse_controller_dev *pcdev, int id)
> @@ -223,6 +249,17 @@ static int tps23881_pi_disable(struct pse_controller_dev *pcdev, int id)
> */
> mdelay(5);
>
> + /* Disable DC disconnect*/
> + chan = priv->port[id].chan[0];
> + ret = i2c_smbus_read_word_data(client, TPS23881_REG_DISC_EN);
> + if (ret < 0)
> + return ret;
dito
> + val = tps23881_set_val(ret, chan, 0, 0, BIT(chan % 4));
> + ret = i2c_smbus_write_word_data(client, TPS23881_REG_DISC_EN, val);
> + if (ret)
> + return ret;
> +
--
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 |
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v6 11/12] net: pse-pd: tps23881: Add support for static port priority feature
2025-03-17 13:33 ` Oleksij Rempel
@ 2025-03-20 17:22 ` Kory Maincent
0 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-20 17:22 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Donald Hunter, Rob Herring,
Andrew Lunn, Simon Horman, Heiner Kallweit, Russell King,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Thomas Petazzoni, netdev, linux-doc, Kyle Swenson, Dent Project,
kernel, Maxime Chevallier, devicetree, linux-kernel
On Mon, 17 Mar 2025 14:33:09 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> On Tue, Mar 04, 2025 at 11:19:00AM +0100, Kory Maincent wrote:
> > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
...
> > @@ -190,7 +201,22 @@ static int tps23881_pi_enable(struct
> > pse_controller_dev *pcdev, int id) BIT(chan % 4));
> > }
> >
> > - return i2c_smbus_write_word_data(client, TPS23881_REG_PW_EN, val);
> > + ret = i2c_smbus_write_word_data(client, TPS23881_REG_PW_EN, val);
> > + if (ret)
> > + return ret;
> > +
> > + /* Enable DC disconnect*/
> > + chan = priv->port[id].chan[0];
> > + ret = i2c_smbus_read_word_data(client, TPS23881_REG_DISC_EN);
> > + if (ret < 0)
> > + return ret;
>
> Here we have RMW operation without lock on two paths: pi_enable and
> pi_disable.
I don't understand, pi_enable and pi_disable are called with pcdev->lock
acquired thanks to the pse core.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v6 12/12] dt-bindings: net: pse-pd: ti,tps23881: Add interrupt description
2025-03-04 10:18 [PATCH net-next v6 00/12] Add support for PSE budget evaluation strategy Kory Maincent
` (10 preceding siblings ...)
2025-03-04 10:19 ` [PATCH net-next v6 11/12] net: pse-pd: tps23881: Add support for static port priority feature Kory Maincent
@ 2025-03-04 10:19 ` Kory Maincent
11 siblings, 0 replies; 36+ messages in thread
From: Kory Maincent @ 2025-03-04 10:19 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jonathan Corbet, Donald Hunter,
Rob Herring, Andrew Lunn, Simon Horman, Heiner Kallweit,
Russell King, Krzysztof Kozlowski, Conor Dooley
Cc: Liam Girdwood, Mark Brown, Thomas Petazzoni, netdev, linux-doc,
Kyle Swenson, Dent Project, kernel, Maxime Chevallier, devicetree,
linux-kernel, Kory Maincent (Dent Project), Krzysztof Kozlowski
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
Add an interrupt property to the device tree bindings for the TI TPS23881
PSE controller. The interrupt is primarily used to detect classification
and disconnection events, which are essential for managing the PSE
controller in compliance with the PoE standard.
Interrupt support is essential for the proper functioning of the TPS23881
controller. Without it, after a power-on (PWON), the controller will
no longer perform detection and classification. This could lead to
potential hazards, such as connecting a non-PoE device after a PoE device,
which might result in magic smoke.
Signed-off-by: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Change in v5:
- Use standard interrupt flag in the example.
Change in v3:
- New patch
---
Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml b/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
index d08abcb012113..3a5f960d8489a 100644
--- a/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
+++ b/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
@@ -20,6 +20,9 @@ properties:
reg:
maxItems: 1
+ interrupts:
+ maxItems: 1
+
'#pse-cells':
const: 1
@@ -62,9 +65,12 @@ unevaluatedProperties: false
required:
- compatible
- reg
+ - interrupts
examples:
- |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
i2c {
#address-cells = <1>;
#size-cells = <0>;
@@ -72,6 +78,8 @@ examples:
ethernet-pse@20 {
compatible = "ti,tps23881";
reg = <0x20>;
+ interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&gpiog>;
channels {
#address-cells = <1>;
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread