devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH net-next 0/2] Add support for the LTC4266 PSE Controller
@ 2025-06-03 23:04 Kyle Swenson
  2025-06-03 23:04 ` [RFC PATCH net-next 1/2] dt-bindings: net: pse-pd: Describe the LTC4266 PSE chipset Kyle Swenson
  2025-06-03 23:04 ` [RFC PATCH net-next 2/2] net: pse-pd: Add LTC4266 PSE controller driver Kyle Swenson
  0 siblings, 2 replies; 8+ messages in thread
From: Kyle Swenson @ 2025-06-03 23:04 UTC (permalink / raw)
  To: o.rempel@pengutronix.de, kory.maincent@bootlin.com,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org
  Cc: Kyle Swenson, netdev@vger.kernel.org, devicetree@vger.kernel.org

Hello,

This RFC series is intended as a starting point for adding support for the
LTC4266, an older PSE (IEEE 802.3at+) chipset from Linear Technology.

This chip has four individually controllable ports, each with its own
detection, classification and current-limiting abilities.  Currently, this
series integrates with the PSE controller core and supports enable/disable,
current and voltage reporting, classification results and will power up a valid
IEEE 802.3at or IEEE802.3af powered device.

It also (poorly) supports the power-limiting functionality in the PSE core.
The complexity here is that the LTC4266 only supports limiting the current, and
so deriving a "current limit" from this power limit while staying compliant
with the IEEE 802.3at/af specifications is tricky.  I'm curious if folks have a
better idea than the linear approximation I've used here.

It is RFC because I want to clarify some confusion I have around the
system-level flow for a port's power allocation.  It's very likely I've missed
something, but it appears as though the port needs to be delivering power in
order to set the power limit on the port.

Thanks for your time,
Kyle

Kyle Swenson (2):
  dt-bindings: net: pse-pd: Describe the LTC4266 PSE chipset
  net: pse-pd: Add LTC4266 PSE controller driver

 .../bindings/net/pse-pd/lltc,ltc4266.yaml     | 146 +++
 drivers/net/pse-pd/Kconfig                    |  10 +
 drivers/net/pse-pd/Makefile                   |   1 +
 drivers/net/pse-pd/ltc4266.c                  | 919 ++++++++++++++++++
 4 files changed, 1076 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/pse-pd/lltc,ltc4266.yaml
 create mode 100644 drivers/net/pse-pd/ltc4266.c

-- 
2.47.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC PATCH net-next 1/2] dt-bindings: net: pse-pd: Describe the LTC4266 PSE chipset
  2025-06-03 23:04 [RFC PATCH net-next 0/2] Add support for the LTC4266 PSE Controller Kyle Swenson
@ 2025-06-03 23:04 ` Kyle Swenson
  2025-06-04  6:43   ` Krzysztof Kozlowski
  2025-06-04  9:06   ` Kory Maincent
  2025-06-03 23:04 ` [RFC PATCH net-next 2/2] net: pse-pd: Add LTC4266 PSE controller driver Kyle Swenson
  1 sibling, 2 replies; 8+ messages in thread
From: Kyle Swenson @ 2025-06-03 23:04 UTC (permalink / raw)
  To: o.rempel@pengutronix.de, kory.maincent@bootlin.com,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org
  Cc: Kyle Swenson, netdev@vger.kernel.org, devicetree@vger.kernel.org

Add the LTC4266 PSE controller from Linear Technology to the device-tree
bindings.
---
 .../bindings/net/pse-pd/lltc,ltc4266.yaml     | 146 ++++++++++++++++++
 1 file changed, 146 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/pse-pd/lltc,ltc4266.yaml

diff --git a/Documentation/devicetree/bindings/net/pse-pd/lltc,ltc4266.yaml b/Documentation/devicetree/bindings/net/pse-pd/lltc,ltc4266.yaml
new file mode 100644
index 000000000000..874447ab6c84
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pse-pd/lltc,ltc4266.yaml
@@ -0,0 +1,146 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/pse-pd/lltc,ltc4266.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LTC LTC4266 Power Sourcing Equipment controller
+
+maintainers:
+  - Kyle Swenson <kyle.swenson@est.tech>
+
+allOf:
+  - $ref: pse-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - lltc,ltc4266
+
+  reg:
+    maxItems: 1
+
+  '#pse-cells':
+    const: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  channels:
+
+    description: This parameter describes the mapping between the logical ports
+      on the PSE controller and the physical ports.
+
+    type: object
+
+    additionalProperties: false
+
+    properties:
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+
+    patternProperties:
+      '^channel@[0-3]$':
+        type: object
+        additionalProperties: false
+
+        properties:
+          reg:
+            maxItems: 1
+
+          sense-resistor-micro-ohms:
+            description: Sense resistor connected to the channel's MOSFET, used
+              for current measurement for overcurrent detection.
+            enum: [250000, 500000]
+
+        required:
+          - reg
+
+    required:
+      - "#address-cells"
+      - "#size-cells"
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      ethernet-pse@2f {
+        compatible = "lltc,ltc4266";
+        status = "okay";
+
+        reg = <0x2f>;
+
+        channels {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          phys0: channel@0 {
+            reg = <0>;
+          };
+
+          phys1: channel@1 {
+            reg = <1>;
+          };
+
+          phys2: channel@2 {
+            reg = <2>;
+          };
+
+          phys3: channel@3 {
+            reg = <3>;
+          };
+        };
+        pse-pis {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          pse_pi0: pse-pi@0 {
+            reg = <0>;
+            #pse-cells = <0>;
+            pairset-names = "alternative-a";
+            pairsets = <&phys0>;
+            polarity-supported = "MDI";
+            vpwr-supply = <&vreg_pse_1>;
+          };
+
+          pse_pi1: pse-pi@1 {
+            reg = <1>;
+            #pse-cells = <0>;
+            pairset-names = "alternative-a";
+            pairsets = <&phys1>;
+            polarity-supported = "MDI";
+            vpwr-supply = <&vreg_pse_2>;
+          };
+
+          pse_pi2: pse-pi@2 {
+            reg = <2>;
+            #pse-cells = <0>;
+            pairset-names = "alternative-a";
+            pairsets = <&phys2>;
+            polarity-supported = "MDI";
+            vpwr-supply = <&vreg_pse_3>;
+          };
+
+          pse_pi3: pse-pi@3 {
+            reg = <3>;
+            #pse-cells = <0>;
+            pairset-names = "alternative-a";
+            pairsets = <&phys3>;
+            polarity-supported = "MDI";
+            vpwr-supply = <&vreg_pse_4>;
+          };
+        };
+      };
+    };
-- 
2.47.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH net-next 2/2] net: pse-pd: Add LTC4266 PSE controller driver
  2025-06-03 23:04 [RFC PATCH net-next 0/2] Add support for the LTC4266 PSE Controller Kyle Swenson
  2025-06-03 23:04 ` [RFC PATCH net-next 1/2] dt-bindings: net: pse-pd: Describe the LTC4266 PSE chipset Kyle Swenson
@ 2025-06-03 23:04 ` Kyle Swenson
  2025-06-04  8:39   ` Oleksij Rempel
  1 sibling, 1 reply; 8+ messages in thread
From: Kyle Swenson @ 2025-06-03 23:04 UTC (permalink / raw)
  To: o.rempel@pengutronix.de, kory.maincent@bootlin.com,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org
  Cc: Kyle Swenson, netdev@vger.kernel.org, devicetree@vger.kernel.org

Add a new driver for the Linear Technology LTC4266 I2C Power Sourcing
Equipment controller.  This driver integrates with the current PSE
controller core, implementing IEEE802.3af and IEEE802.3at PSE standards.
---
 drivers/net/pse-pd/Kconfig   |  10 +
 drivers/net/pse-pd/Makefile  |   1 +
 drivers/net/pse-pd/ltc4266.c | 919 +++++++++++++++++++++++++++++++++++
 3 files changed, 930 insertions(+)
 create mode 100644 drivers/net/pse-pd/ltc4266.c

diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
index 7fab916a7f46..a0f2eaadb4fb 100644
--- a/drivers/net/pse-pd/Kconfig
+++ b/drivers/net/pse-pd/Kconfig
@@ -18,10 +18,20 @@ config PSE_REGULATOR
 	help
 	  This module provides support for simple regulator based Ethernet Power
 	  Sourcing Equipment without automatic classification support. For
 	  example for basic implementation of PoDL (802.3bu) specification.
 
+config PSE_LTC4266
+	tristate "LTC4266 PSE controller"
+	depends on I2C
+	help
+	  This module provides support the LTC4266 regulator based Ethernet
+	  Power Sourcing Equipment.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ltc4266.
+
 config PSE_PD692X0
 	tristate "PD692X0 PSE controller"
 	depends on I2C
 	select FW_LOADER
 	select FW_UPLOAD
diff --git a/drivers/net/pse-pd/Makefile b/drivers/net/pse-pd/Makefile
index 9d2898b36737..a17e16467ae2 100644
--- a/drivers/net/pse-pd/Makefile
+++ b/drivers/net/pse-pd/Makefile
@@ -1,8 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0-only
 # Makefile for Linux PSE drivers
 
 obj-$(CONFIG_PSE_CONTROLLER) += pse_core.o
 
+obj-$(CONFIG_PSE_LTC4266) += ltc4266.o
 obj-$(CONFIG_PSE_REGULATOR) += pse_regulator.o
 obj-$(CONFIG_PSE_PD692X0) += pd692x0.o
 obj-$(CONFIG_PSE_TPS23881) += tps23881.o
diff --git a/drivers/net/pse-pd/ltc4266.c b/drivers/net/pse-pd/ltc4266.c
new file mode 100644
index 000000000000..858889c9ab75
--- /dev/null
+++ b/drivers/net/pse-pd/ltc4266.c
@@ -0,0 +1,919 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Linear LTC4266 PoE PSE Controller
+ *
+ * Original work:
+ *    Copyright 2019 Cradlepoint Technology, Inc.
+ *    Cradlepoint Technology, Inc.  <source@cradlepoint.com>
+ *
+ * Re-written in 2025:
+ *    Copyright 2025 Ericsson Software Technology
+ *    Kyle Swenson <kyle.swenson@est.tech>
+ *
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/ethtool.h>
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pse-pd/pse.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+#define LTC4266_REG_ID				0x1B
+#define LTC4266_ID				0x64
+
+#define TWO_BIT_WORD_OFFSET(_v, _pid)		((_v) << ((_pid) * 2))
+#define TWO_BIT_WORD_MASK(_pid)			TWO_BIT_WORD_OFFSET(0x03, (_pid))
+
+#define LTC4266_IPLSB_REG(_p)			(0x30 | ((_p) << 2))
+#define LTC4266_VPLSB_REG(_p)			(LTC4266_IPLSB_REG(_p) + 2)
+
+#define LTC4266_RSTPB_INTCLR			BIT(7)
+#define LTC4266_RSTPB_PINCLR			BIT(6)
+#define LTC4266_RSTPB_RSTALL			BIT(5)
+
+/* Register definitions */
+#define LTC4266_REG_INTSTAT			0x00
+#define LTC4266_REG_INTMASK			0x01
+#define LTC4266_REG_PWREVN_COR			0x03
+#define LTC4266_REG_DETEVN_COR			0x05
+#define LTC4266_REG_FLTEVN_COR			0x07
+#define LTC4266_REG_TSEVN_COR			0x09
+#define LTC4266_REG_SUPEVN_COR			0x0B
+#define LTC4266_REG_STAT(n)			(0x0C + (n))
+#define LTC4266_REG_STATPWR			0x10
+#define LTC4266_REG_OPMD			0x12
+#define LTC4266_REG_DISENA			0x13 /* Disconnect detect enable */
+#define LTC4266_REG_MCONF			0x17
+#define LTC4266_REG_DETPB			0x18
+#define LTC4266_REG_PWRPB			0x19
+#define LTC4266_REG_RSTPB			0x1A
+#define LTC4266_REG_HPEN			0x44
+#define LTC4266_REG_HPMD(_p)			(0x46 + (5 * (_p)))
+#define LTC4266_REG_ILIM(_p)			(LTC4266_REG_HPMD(_p) + 2)
+#define LTC4266_REG_TLIM12			0x1E
+#define LTC4266_REG_TLIM34			0x1F
+
+/* Register field definitions */
+#define LTC4266_HPMD_PONGEN			0x01
+
+/* For LTC4266_REG_TLIM* */
+#define LTC4266_TLIM_VALUE			0x01
+
+/* LTC4266_REG_HPEN, enable "High Power" mode (i.e. Type 2, 25.4W PDs) */
+#define LTC4266_HPEN(_p)			BIT(_p)
+
+/* LTC4266_REG_MCONF */
+#define LTC4266_MCONF_INTERRUPT_ENABLE		BIT(7)
+
+/* LTC4266_REG_STATPWR */
+#define LTC4266_STATPWR_PG(_p)			BIT((_p) + 4)
+#define LTC4266_STATPWR_PE(_p)			BIT(_p)
+#define LTC4266_PORT_CLASS(_stat)		FIELD_GET(GENMASK(6, 4), (_stat))
+
+#define LTC4266_REG_ICUT_HP(_p)			(LTC4266_REG_HPMD(_p) + 1)
+
+/* if R_sense = 0.25 Ohm, this should be set otherwise 0 */
+#define LTC4266_ICUT_RSENSE			BIT(7)
+/* if set, halve the range and double the precision */
+#define LTC4266_ICUT_RANGE			BIT(6)
+
+#define LTC4266_ILIM_AF_RSENSE_025		0x80
+#define LTC4266_ILIM_AF_RSENSE_050		0x00
+#define LTC4266_ILIM_AT_RSENSE_025		0xC0
+#define LTC4266_ILIM_AT_RSENSE_050		0x40
+
+/* LTC4266_REG_INTSTAT and LTC4266_REG_INTMASK */
+#define LTC4266_INT_SUPPLY			BIT(7)
+#define LTC4266_INT_TSTART			BIT(6)
+#define LTC4266_INT_TCUT			BIT(5)
+#define LTC4266_INT_CLASS			BIT(4)
+#define LTC4266_INT_DET				BIT(3)
+#define LTC4266_INT_DIS				BIT(2)
+#define LTC4266_INT_PWRGD			BIT(1)
+#define LTC4266_INT_PWRENA			BIT(0)
+
+#define LTC4266_MAX_PORTS 4
+
+/* Maximum and minimum power limits for a single port */
+#define LTC4266_PW_LIMIT_MAX 25400
+#define LTC4266_PW_LIMIT_MIN 1
+
+enum {
+	READ_CURRENT = 0,
+	READ_VOLTAGE = 2
+};
+
+enum {
+	LTC4266_OPMD_SHUTDOWN = 0,
+	LTC4266_OPMD_MANUAL,
+	LTC4266_OPMD_SEMI,
+	LTC4266_OPMD_AUTO
+};
+
+/* Map LTC4266 Classification result to PD class */
+static int ltc4266_class_map[] = {
+	0, /* Treat as class 3 */
+	1,
+	2,
+	3,
+	4,
+	-EINVAL,
+	3, /* Treat as class 3 */
+	-ERANGE
+};
+
+/* Convert a class 0-4 to icut register value */
+static int ltc4266_class_to_icut[] = {
+	375,
+	112,
+	206,
+	375,
+	638
+};
+
+enum sense_resistor {
+	LTC4266_RSENSE_500, /* Rsense 0.5 Ohm */
+	LTC4266_RSENSE_250 /* Rsense 0.25 Ohm */
+};
+
+struct ltc4266_port {
+	enum sense_resistor rsense;
+	struct device_node *node;
+	int current_limit;
+};
+
+struct ltc4266 {
+	struct i2c_client *client;
+	struct mutex lock; /* Protect Read-Modify-Write Sequences */
+	struct ltc4266_port *ports[LTC4266_MAX_PORTS];
+	struct device *dev;
+	struct device_node *np;
+	struct pse_controller_dev pcdev;
+};
+
+/* Read-modify-write sequence with value and mask.  Mask is expected to be
+ * shifted to the correct spot.
+ */
+static int ltc4266_write_reg(struct ltc4266 *ltc4266, u8 reg, u8 value, u8 mask)
+{
+	int ret;
+	u8 new;
+
+	mutex_lock(&ltc4266->lock);
+	ret = i2c_smbus_read_byte_data(ltc4266->client, reg);
+	if (ret < 0) {
+		dev_warn(ltc4266->dev, "Failed to read register 0x%02x, err=%d\n", reg, ret);
+		mutex_unlock(&ltc4266->lock);
+		return ret;
+	}
+	new = (u8)ret;
+	new &= ~mask;
+	new |= value & mask;
+	ret = i2c_smbus_write_byte_data(ltc4266->client, reg, new);
+	mutex_unlock(&ltc4266->lock);
+
+	return ret;
+}
+
+static int ltc4266_read_iv(struct ltc4266 *ltc4266, int port, u8 iv)
+{
+	int lsb;
+	int msb;
+	int result;
+	int lsb_reg;
+	u64 ivbits = 0;
+
+	if (iv == READ_CURRENT)
+		lsb_reg = LTC4266_IPLSB_REG(port);
+	else if (iv == READ_VOLTAGE)
+		lsb_reg = LTC4266_VPLSB_REG(port);
+	else
+		return -EINVAL;
+
+	result = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_STATPWR);
+	if (result < 0)
+		return result;
+
+	/*  LTC4266 IV readings are only valid if the port is powered. */
+	if (!(result & LTC4266_STATPWR_PG(port)))
+		return -EINVAL;
+
+	/* LTC4266 expects the MSB register to be read immediately following the LSB
+	 * register, so we need to ensure other parts aren't reading other registers in
+	 * this chip while we read the current/voltage regulators.
+	 */
+	mutex_lock(&ltc4266->lock);
+
+	lsb = i2c_smbus_read_byte_data(ltc4266->client, lsb_reg);
+	msb = i2c_smbus_read_byte_data(ltc4266->client, lsb_reg + 1);
+
+	mutex_unlock(&ltc4266->lock);
+
+	if (lsb < 0)
+		return lsb;
+
+	if (msb < 0)
+		return msb;
+
+	ivbits = 0;
+	ivbits |= ((u8)msb) << 8 | ((u8)lsb);
+
+	if (iv == READ_CURRENT)
+		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250) /* 122.07 uA/LSB */
+			result = DIV_ROUND_CLOSEST_ULL((ivbits * 122070), 1000);
+		else /* 61.035 uA/LSB */
+			result = DIV_ROUND_CLOSEST_ULL((ivbits * 61035), 1000);
+	else /* 5.835 mV/LSB == 5835 uV/LSB */
+		result = ivbits * 5835;
+
+	return result;
+}
+
+static int ltc4266_port_set_ilim(struct ltc4266 *ltc4266, int port, int class)
+{
+	if (class > 4 || class < 0)
+		return -EINVAL;
+
+	/* We want to set 425 mA for class 3 and lower; 850 mA otherwise for IEEE compliance */
+	if (class < 4) {
+		/* Write 0x80 for 0.25 Ohm sense otherwise 0 */
+		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
+			return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AF_RSENSE_025);
+		return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AF_RSENSE_050);
+	}
+
+	/* Class == 4 */
+	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
+		return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AT_RSENSE_025);
+	/* Class == 4 and the sense resistor is 0.5 */
+	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AT_RSENSE_050);
+}
+
+static int ltc4266_port_set_icut(struct ltc4266 *ltc4266, int port, int icut)
+{
+	u8 val;
+
+	if (icut > 850)
+		return -ERANGE;
+
+	val = (u8)(DIV_ROUND_CLOSEST((icut * 1000), 18750) & 0x3F);
+
+	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
+		val |= LTC4266_ICUT_RSENSE | LTC4266_ICUT_RANGE;
+
+	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ICUT_HP(port), val);
+}
+
+static int ltc4266_port_mode(struct ltc4266 *ltc4266, int port, u8 opmd)
+{
+	if (opmd >= LTC4266_OPMD_AUTO)
+		return -EINVAL;
+
+	return ltc4266_write_reg(ltc4266, LTC4266_REG_OPMD, TWO_BIT_WORD_OFFSET(opmd, port),
+				TWO_BIT_WORD_MASK(port));
+}
+
+static int ltc4266_port_powered(struct ltc4266 *ltc4266, int port)
+{
+	int result = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_STATPWR);
+
+	if (result < 0)
+		return result;
+
+	return !!((result & LTC4266_STATPWR_PG(port)) && (result & LTC4266_STATPWR_PE(port)));
+}
+
+static int ltc4266_port_init(struct ltc4266 *ltc4266, int port)
+{
+	int ret;
+	u8 tlim_reg;
+	u8 tlim_mask;
+
+	/* Reset the port */
+	ret = i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_RSTPB, BIT(port));
+	if (ret < 0)
+		return ret;
+
+	ret = ltc4266_port_mode(ltc4266, port, LTC4266_OPMD_SEMI);
+	if (ret < 0)
+		return ret;
+
+	/* Enable high power mode on the port (802.3at+) */
+	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_HPEN,
+				LTC4266_HPEN(port), LTC4266_HPEN(port));
+	if (ret < 0)
+		return ret;
+
+	/* Enable Ping-Pong Classification */
+	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_HPMD(port),
+				LTC4266_HPMD_PONGEN, LTC4266_HPMD_PONGEN);
+	if (ret < 0)
+		return ret;
+
+	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
+		ret = ltc4266_write_reg(ltc4266, LTC4266_REG_ICUT_HP(port),
+					LTC4266_ICUT_RSENSE, LTC4266_ICUT_RSENSE);
+	else
+		ret = ltc4266_write_reg(ltc4266, LTC4266_REG_ICUT_HP(port),
+					0, LTC4266_ICUT_RSENSE);
+
+	if (ret < 0)
+		return ret;
+
+	if (port <= 1)
+		tlim_reg = LTC4266_REG_TLIM12;
+	else
+		tlim_reg = LTC4266_REG_TLIM34;
+
+	if (port & BIT(0))
+		tlim_mask = GENMASK(7, 4);
+	else
+		tlim_mask = GENMASK(3, 0);
+
+	ret = ltc4266_write_reg(ltc4266, tlim_reg, LTC4266_TLIM_VALUE, tlim_mask);
+	if (ret < 0)
+		return ret;
+
+	/* Enable disconnect detect. */
+	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_DISENA, BIT(port), BIT(port));
+	if (ret < 0)
+		return ret;
+
+	/* Enable detection (low nibble), classification (high nibble) on the port */
+	ret = i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_DETPB,
+					BIT(port + 4) | BIT(port));
+
+	if (ret < 0)
+		return ret;
+
+	dev_dbg(ltc4266->dev, "Port %d has been initialized\n", port);
+	return 0;
+}
+
+static int ltc4266_get_opmode(struct ltc4266 *ltc4266, int port)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_OPMD);
+	if (ret < 0)
+		return ret;
+
+	switch (port) {
+	case 0:
+		return FIELD_GET(GENMASK(1, 0), ret);
+	case 1:
+		return FIELD_GET(GENMASK(3, 2), ret);
+	case 2:
+		return FIELD_GET(GENMASK(5, 4), ret);
+	case 3:
+		return FIELD_GET(GENMASK(7, 6), ret);
+	}
+	return -EINVAL;
+}
+
+static int ltc4266_pi_is_enabled(struct pse_controller_dev *pcdev, int id)
+{
+	int ret;
+	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
+
+	ret = ltc4266_get_opmode(ltc4266, id);
+	if (ret < 0)
+		return ret;
+
+	if (ret == LTC4266_OPMD_SEMI)
+		return 1; /*  If a port is in OPMODE SEMI, we'll just assume admin has it enabled */
+
+	return 0;
+}
+
+static int ltc4266_pi_get_pw_status(struct pse_controller_dev *pcdev, int id,
+				    struct pse_pw_status *pw_status)
+{
+	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
+	int ret = 0;
+
+	if (!ltc4266_pi_is_enabled(pcdev, id)) {
+		/* The port is disabled by configuration*/
+		pw_status->c33_pw_status = ETHTOOL_C33_PSE_PW_D_STATUS_DISABLED;
+		return 0;
+	}
+
+	if (ltc4266_port_powered(ltc4266, id)) {
+		pw_status->c33_pw_status = ETHTOOL_C33_PSE_PW_D_STATUS_DELIVERING;
+		return 0;
+	}
+
+	ret = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_STAT(id));
+	if (ret < 0) {
+		dev_warn(pcdev->dev, "Failed to read status register, err=%d\n", ret);
+		return ret;
+	}
+
+	pw_status->c33_pw_status = ETHTOOL_C33_PSE_PW_D_STATUS_SEARCHING;
+	return 0;
+}
+
+/* Allow a port to be powered */
+static int ltc4266_pi_enable(struct pse_controller_dev *pcdev, int port)
+{
+	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
+
+	ltc4266_port_init(ltc4266, port);
+	return 0;
+}
+
+static int ltc4266_pi_disable(struct pse_controller_dev *pcdev, int id)
+{
+	int ret = 0;
+	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
+
+	ret = i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_RSTPB, BIT(id));
+	if (ret)
+		return ret;
+
+	ltc4266->ports[id]->current_limit = 0;
+	return ltc4266_port_mode(ltc4266, id, LTC4266_OPMD_SHUTDOWN);
+}
+
+static int ltc4266_pi_get_voltage(struct pse_controller_dev *pcdev, int id)
+{
+	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
+
+	return ltc4266_read_iv(ltc4266, id, READ_VOLTAGE);
+}
+
+static int ltc4266_pi_get_admin_state(struct pse_controller_dev *pcdev, int id,
+				      struct pse_admin_state *admin_state)
+{
+	if (ltc4266_pi_is_enabled(pcdev, id))
+		admin_state->c33_admin_state =
+			ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED;
+	else
+		admin_state->c33_admin_state =
+			ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED;
+
+	return 0;
+}
+
+/* Get the PD Classification Result */
+static int ltc4266_pi_get_pw_class(struct pse_controller_dev *pcdev,
+				   int id)
+{
+	int ret;
+	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
+
+	ret = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_STAT(id));
+	if (ret < 0) {
+		dev_warn(ltc4266->dev, "Failed to read status register, err=%d\n", ret);
+		return ret;
+	}
+
+	return ltc4266_class_map[LTC4266_PORT_CLASS(ret)];
+}
+
+static int ltc4266_pi_get_actual_pw(struct pse_controller_dev *pcdev, int id)
+{
+	int uA, uV;
+	u64 uW;
+	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
+
+	uA = ltc4266_read_iv(ltc4266, id, READ_CURRENT);
+	uV = ltc4266_read_iv(ltc4266, id, READ_VOLTAGE);
+
+	if (uA < 0)
+		return uA;
+	if (uV < 0)
+		return uV;
+
+	/* Convert uA to mA and uV to mV; mA * mV = uW */
+	uW = DIV_ROUND_CLOSEST_ULL(uA, 1000) * DIV_ROUND_CLOSEST_ULL(uV, 1000);
+
+	return (int)DIV_ROUND_CLOSEST_ULL(uW, 1000);
+}
+
+static int
+ltc4266_pi_get_pw_limit_ranges(struct pse_controller_dev *pcdev, int id,
+			       struct pse_pw_limit_ranges *pw_limit_ranges)
+{
+	struct ethtool_c33_pse_pw_limit_range *c33_pw_limit_ranges;
+
+	c33_pw_limit_ranges = kzalloc(sizeof(*c33_pw_limit_ranges),
+				      GFP_KERNEL);
+	if (!c33_pw_limit_ranges)
+		return -ENOMEM;
+
+	c33_pw_limit_ranges[0].min = LTC4266_PW_LIMIT_MIN;
+	c33_pw_limit_ranges[0].max = LTC4266_PW_LIMIT_MAX;
+
+	pw_limit_ranges->c33_pw_limit_ranges = c33_pw_limit_ranges;
+	/* Return the number of ranges */
+	return 1;
+}
+
+static int ltc4266_pi_set_pw_limit(struct pse_controller_dev *pcdev,
+				   int id, int max_mW)
+{
+	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
+	u64 temp;
+
+	if (max_mW < 1 || max_mW > 25500) {
+		dev_err(ltc4266->dev, "power limit %d is out of range [%d, %d]\n",
+			max_mW, 1, 25500);
+		return -ERANGE;
+	}
+
+	/* Given the range, set a class-specific current limit:
+	 *
+	 * Class	Range (W)	Current Limit
+	 * 0		0			0mA
+	 * 1		4000			112mA
+	 * 2		7000			206mA
+	 * 3		15400			375mA
+	 * 4		25500			638mA
+	 *
+	 * Simple linear regression is probably good enough:
+	 * y = 0.0238856*x + 22.83371414
+	 * scale by 10^7 to get y = 238856 * x + 228337141
+	 */
+
+	temp = DIV_ROUND_CLOSEST_ULL((238856ULL * (uint64_t)max_mW + 22833714ULL), 10000000ULL);
+
+	dev_dbg(ltc4266->dev, "%s passed max_mW=%d, linear regression results in %d\n", __func__, max_mW, (int)temp);
+
+	ltc4266->ports[id]->current_limit = (int)temp;
+	return ltc4266_port_set_icut(ltc4266, id, (int)temp);
+}
+
+static int ltc4266_pi_get_pw_limit(struct pse_controller_dev *pcdev, int id)
+{
+	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
+	int ret = 0;
+	int icut = 0;
+	int uA = 0;
+	int mA;
+	int mV;
+	u64 mW = 0;
+
+	/* The LTC4266 offers a "current limit", not a power-limit */
+	ret = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_ICUT_HP(id));
+	if (ret < 0)
+		return ret;
+
+	icut = FIELD_GET(GENMASK(5, 0), ret);
+
+	if (ltc4266->ports[id]->rsense == LTC4266_RSENSE_250) {
+		if (ret & LTC4266_ICUT_RANGE)
+			uA = icut * 18750; /* 18.75 mA/LSB  = 18750 uA/LSB */
+		else
+			uA = icut * 37500;
+	} else {
+		if (ret & LTC4266_ICUT_RANGE)
+			uA = icut * 9380; /* 9.38 mA/LSB */
+		else
+			uA = icut * 18750;
+	}
+
+	mA = DIV_ROUND_CLOSEST(uA, 1000);
+	mV = ltc4266_read_iv(ltc4266, id, READ_VOLTAGE);
+	if (mV < 0)
+		return mV;
+
+	mW = DIV_ROUND_CLOSEST_ULL(((uint64_t)mV * mA), 1000000);
+	dev_dbg(ltc4266->dev, "%s(id=%d) current limit is 0x%02X, power limit is %llu\n", __func__, id, icut, mW);
+	return (int)mW;
+}
+
+static int ltc4266_setup_pi_matrix(struct pse_controller_dev *pcdev)
+{
+	u32 channel_id;
+	struct ltc4266_port *port;
+	u32 sense;
+	struct device_node *channels_node;
+
+	int i = 0;
+	int ret = 0;
+	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
+
+	channels_node = of_find_node_by_name(ltc4266->np, "channels");
+	if (!channels_node)
+		return -EINVAL;
+
+	for_each_child_of_node_scoped(channels_node, port_node) {
+		if (!of_node_name_eq(port_node, "channel"))
+			continue;
+
+		ret = of_property_read_u32(port_node, "reg", &channel_id);
+		if (ret)
+			goto out;
+
+		if (channel_id >= LTC4266_MAX_PORTS) {
+			ret = -EINVAL;
+			goto out;
+		}
+
+		if (ltc4266->ports[channel_id]) {
+			dev_err(ltc4266->dev,
+				"channel_id %d is already used, please check the reg property in node %pOF\n",
+				channel_id, port_node);
+			ret = -EINVAL;
+			goto out;
+		}
+
+		port = devm_kzalloc(ltc4266->dev, sizeof(struct ltc4266_port), GFP_KERNEL);
+		if (!port) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		ltc4266->ports[channel_id] = port;
+
+		ret = of_property_read_u32(port_node, "sense-resistor-micro-ohms", &sense);
+		if (ret)
+			goto out;
+
+		if (sense == 250000) {
+			port->rsense = LTC4266_RSENSE_250;
+		} else if (sense == 500000) {
+			port->rsense = LTC4266_RSENSE_500;
+		} else {
+			dev_err(ltc4266->dev, "sense resistor value of %d is invalid in node %pOF\n", sense, port_node);
+			ret = -EINVAL;
+			goto out;
+		}
+
+		port->node = of_node_get(port_node);
+	}
+	for (i = 0; i < LTC4266_MAX_PORTS; i++) {
+		ret = ltc4266_port_init(ltc4266, i);
+		if (ret < 0) {
+			dev_err(ltc4266->dev, "Failed to initialize port %d\n", i);
+			goto out;
+		}
+	}
+
+	return 0;
+out:
+	for (i = 0; i < LTC4266_MAX_PORTS; i++) {
+		if (!ltc4266->ports[i])
+			continue;
+
+		if (ltc4266->ports[i]->node)
+			of_node_put(ltc4266->ports[i]->node);
+	}
+	return ret;
+}
+
+static const struct pse_controller_ops ltc4266_ops = {
+	.setup_pi_matrix = ltc4266_setup_pi_matrix,
+	.pi_get_admin_state = ltc4266_pi_get_admin_state,
+	.pi_get_pw_status = ltc4266_pi_get_pw_status,
+	.pi_get_pw_class = ltc4266_pi_get_pw_class,
+	.pi_get_actual_pw = ltc4266_pi_get_actual_pw,
+	.pi_enable = ltc4266_pi_enable,
+	.pi_disable = ltc4266_pi_disable,
+	.pi_get_voltage = ltc4266_pi_get_voltage,
+	.pi_get_pw_limit = ltc4266_pi_get_pw_limit,
+	.pi_set_pw_limit = ltc4266_pi_set_pw_limit,
+	.pi_get_pw_limit_ranges = ltc4266_pi_get_pw_limit_ranges,
+};
+
+#define LTC4266_INTERRUPT_SOURCES	(LTC4266_INT_SUPPLY | LTC4266_INT_TSTART | LTC4266_INT_TCUT | LTC4266_INT_CLASS | LTC4266_INT_DIS | LTC4266_INT_PWRENA | LTC4266_INT_PWRGD)
+
+static void ltc4266_enable_interrupts(struct i2c_client *client)
+{
+	i2c_smbus_write_byte_data(client, LTC4266_REG_INTMASK, LTC4266_INTERRUPT_SOURCES); /* Unmask interrupts */
+}
+
+static int ltc4266_disable_interrupts(struct i2c_client *client)
+{
+	int r;
+	/* Mask out the chip's interrupt */
+	r = i2c_smbus_write_byte_data(client, LTC4266_REG_INTMASK, 0x00);
+	if (r < 0) {
+		dev_err(&client->dev, "Failed to disable interrupts, err %d\n", r);
+		return r;
+	}
+	/* Reset the (SMBus Alert) interrupt pin */
+	i2c_smbus_write_byte_data(client, LTC4266_REG_RSTPB, LTC4266_RSTPB_PINCLR);
+	return 0;
+}
+
+static void handle_classification_event(struct ltc4266 *ltc4266, int detect_event)
+{
+	u8 classified_ports;
+	int ret = 0;
+	int i = 0;
+
+	if (detect_event < 0)
+		return;
+
+	for (i = 0, classified_ports = FIELD_GET(GENMASK(7, 4), detect_event);
+			classified_ports && i < LTC4266_MAX_PORTS; classified_ports >>= 1, i++) {
+		if (!(classified_ports & BIT(0)))
+			continue;
+		if (!ltc4266_pi_is_enabled(&ltc4266->pcdev, i))
+			continue;
+
+		ret = ltc4266_pi_get_pw_class(&ltc4266->pcdev, i);
+		if (ret < 0) {
+			dev_warn(&ltc4266->client->dev, "Invalid class %d\n", ret);
+			continue;
+		}
+		dev_dbg(ltc4266->dev, "port %d has a classification result of %d\n", i, ret);
+		ltc4266_port_set_ilim(ltc4266, i, ret);
+
+		/* It is possible we're in this handler before the ports are non-null */
+		if (!ltc4266->ports[i])
+			continue;
+
+		dev_dbg(&ltc4266->client->dev, "%s is powering port %d because it's enabled\n", __func__, i);
+		if (ltc4266->ports[i]->current_limit > 0) {
+			dev_dbg(ltc4266->dev, "Port %d is using a previously set current limit of %d\n", i, ltc4266->ports[i]->current_limit);
+			ltc4266_port_set_icut(ltc4266, i, ltc4266->ports[i]->current_limit);
+		} else {
+			ltc4266_port_set_icut(ltc4266, i, ltc4266_class_to_icut[ret]);
+		}
+		i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_PWRPB, BIT(i));
+	}
+}
+
+static irqreturn_t ltc4266_irq_handler_thread(int irq, void *private)
+{
+	struct ltc4266 *ltc4266 = private;
+	int event, intstat;
+
+	ltc4266_disable_interrupts(ltc4266->client);
+
+	intstat = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_INTSTAT);
+	if (intstat < 0) {
+		dev_err(&ltc4266->client->dev, "Error %d reading register 0x%02X", intstat, LTC4266_REG_INTSTAT);
+		goto done;
+	}
+
+	if (!intstat) {
+		dev_dbg(ltc4266->dev, "Intstat is zero yet we're in the interrupt routine...\n");
+		goto done;
+	}
+
+	if (intstat & (LTC4266_INT_SUPPLY)) {
+		event = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_SUPEVN_COR);
+		if (event) {
+			dev_info(ltc4266->dev, "Supply event=0x%02X\n", event);
+			goto done;
+		}
+	}
+
+	/* There isn't anything we need to actually do for the
+	 * Tstart/Tcut/Disconnect events here, except for reading the clear-on-read
+	 * register
+	 */
+	if (intstat & (LTC4266_INT_TCUT | LTC4266_INT_TSTART | LTC4266_INT_DIS)) {
+		i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_TSEVN_COR);
+		i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_FLTEVN_COR);
+	}
+
+	if (intstat & (LTC4266_INT_DET | LTC4266_INT_CLASS))
+		handle_classification_event(ltc4266, i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_DETEVN_COR));
+
+	if (intstat & (LTC4266_INT_PWRGD | LTC4266_INT_PWRENA))
+		i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_PWREVN_COR);
+
+done:
+	ltc4266_enable_interrupts(ltc4266->client);
+	return IRQ_HANDLED;
+}
+
+static int ltc4266_probe(struct i2c_client *client)
+{
+	int ret;
+	u8 id_reg;
+	struct ltc4266 *ltc4266;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return dev_err_probe(&client->dev, -ENODEV, "I2C Bus Adapter needs I2C functionality\n");
+
+	/* The id_reg value here is not what was specified in the datasheet, nor
+	 * the include file. Lets make sure we've got something at address 0x2F that
+	 * happens to have a register at 0x1B that returns 0x64
+	 */
+	id_reg = i2c_smbus_read_byte_data(client, LTC4266_REG_ID);
+	if (id_reg != LTC4266_ID)
+		return dev_err_probe(&client->dev, -ENODEV, "Expected an ID of 0x64, saw 0x%02X\n", id_reg);
+
+	/* Reset the chip */
+	i2c_smbus_write_byte_data(client, LTC4266_REG_RSTPB,  LTC4266_RSTPB_INTCLR | LTC4266_RSTPB_RSTALL);
+
+	/* LTC4266 requires approximately 10 ms after reset to be stable; if it
+	 * isn't, then there is typically an undervoltage lockout/something pretty bad
+	 * going on. We give it 50 ms here so we don't need to poll the chip and use I2C bandwidth
+	 */
+	msleep(50);
+
+	/* Let's make sure the chip came out of reset (if not, the chip is probably
+	 * either (no longer?) present, in thermal shutdown, or watchdogged....either
+	 * way, there's nothing we can do in software to fix it)
+	 */
+	id_reg = i2c_smbus_read_byte_data(client, LTC4266_REG_ID);
+	if (id_reg != 0x64)
+		return dev_err_probe(&client->dev, -ENODEV, "Failed to re-read LTC4266 device ID after reset 0x%02X\n", id_reg);
+
+	ltc4266 = devm_kzalloc(&client->dev, sizeof(struct ltc4266), GFP_KERNEL);
+	if (!ltc4266)
+		return -ENOMEM;
+
+	mutex_init(&ltc4266->lock);
+
+	i2c_set_clientdata(client, ltc4266);
+	ltc4266->client = client;
+	ltc4266->np = client->dev.of_node;
+
+	/* After reset, the LTC4266 will interrupt with a (single) supply fault.
+	 * Clear it here and discard the result
+	 */
+	i2c_smbus_read_byte_data(client, LTC4266_REG_SUPEVN_COR);
+
+	ltc4266_disable_interrupts(client);
+
+	ltc4266->pcdev.owner = THIS_MODULE;
+	ltc4266->pcdev.ops = &ltc4266_ops;
+	ltc4266->dev = &client->dev;
+	ltc4266->pcdev.dev = &client->dev;
+	ltc4266->pcdev.types = ETHTOOL_PSE_C33;
+	ltc4266->pcdev.nr_lines = LTC4266_MAX_PORTS;
+
+	ret = devm_pse_controller_register(ltc4266->dev, &ltc4266->pcdev);
+	if (ret)
+		return dev_err_probe(&client->dev, ret,
+						"Failed to register PSE controller\n");
+
+	if (client->irq) {
+		dev_dbg(ltc4266->dev, "Client IRQ is set!\n");
+
+		/* Enable the interrupt pin */
+		ltc4266_write_reg(ltc4266, LTC4266_REG_MCONF,
+				  LTC4266_MCONF_INTERRUPT_ENABLE, LTC4266_MCONF_INTERRUPT_ENABLE);
+
+		ret = devm_request_threaded_irq(ltc4266->dev, client->irq, NULL,
+						ltc4266_irq_handler_thread,
+						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,  "ltc4266-irq", ltc4266);
+		if (ret)
+			dev_err_probe(&client->dev, ret, "Failed to request threaded IRQ\n");
+
+		ltc4266_enable_interrupts(client);
+	}
+
+	return 0;
+}
+
+static void ltc4266_remove(struct i2c_client *client)
+{
+	struct ltc4266 *ltc4266 = i2c_get_clientdata(client);
+	int i;
+	/*Put the LTC4266 into reset */
+	i2c_smbus_write_byte_data(client, LTC4266_REG_RSTPB, LTC4266_RSTPB_RSTALL);
+
+	for (i = 0; i < LTC4266_MAX_PORTS; i++) {
+		if (!ltc4266->ports[i])
+			continue;
+		if (ltc4266->ports[i]->node)
+			of_node_put(ltc4266->ports[i]->node);
+	}
+}
+
+static const struct i2c_device_id ltc4266_id[] = {
+	{.name = "ltc4266"},
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ltc4266_id);
+
+static const struct of_device_id ltc4266_of_match[] = {
+	{ .compatible = "lltc,ltc4266" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ltc4266_of_match);
+
+static struct i2c_driver ltc4266_driver = {
+	.driver		= {
+		.name	= "ltc4266",
+		.of_match_table = ltc4266_of_match,
+	},
+	.probe		= ltc4266_probe,
+	.remove		= ltc4266_remove,
+	.id_table	= ltc4266_id,
+};
+module_i2c_driver(ltc4266_driver);
+
+MODULE_AUTHOR("Kyle Swenson <kyle.swenson@est.tech>");
+MODULE_DESCRIPTION("LTC4266 PoE PSE Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.47.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH net-next 1/2] dt-bindings: net: pse-pd: Describe the LTC4266 PSE chipset
  2025-06-03 23:04 ` [RFC PATCH net-next 1/2] dt-bindings: net: pse-pd: Describe the LTC4266 PSE chipset Kyle Swenson
@ 2025-06-04  6:43   ` Krzysztof Kozlowski
  2025-06-04  9:06   ` Kory Maincent
  1 sibling, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-04  6:43 UTC (permalink / raw)
  To: Kyle Swenson, o.rempel@pengutronix.de, kory.maincent@bootlin.com,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org
  Cc: netdev@vger.kernel.org, devicetree@vger.kernel.org

On 04/06/2025 01:04, Kyle Swenson wrote:
> +allOf:
> +  - $ref: pse-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - lltc,ltc4266
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#pse-cells':
> +    const: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  channels:
> +

Drop blank line

> +    description: This parameter describes the mapping between the logical ports
> +      on the PSE controller and the physical ports.

Move description after additionalProperties, so entire block is together.

> +

> +    type: object
> +
Drop blank line


> +    additionalProperties: false
> +
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +

Only one blank line

> +    patternProperties:
> +      '^channel@[0-3]$':
> +        type: object
> +        additionalProperties: false
> +
> +        properties:
> +          reg:
> +            maxItems: 1
> +
> +          sense-resistor-micro-ohms:
> +            description: Sense resistor connected to the channel's MOSFET, used
> +              for current measurement for overcurrent detection.
> +            enum: [250000, 500000]
> +
> +        required:
> +          - reg
> +
> +    required:
> +      - "#address-cells"

Keep consistent quotes, either ' or "

> +      - "#size-cells"
> +
> +unevaluatedProperties: false

This goes after required block

> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      ethernet-pse@2f {
> +        compatible = "lltc,ltc4266";
> +        status = "okay";

Drop

> +

Drop blank line


> +        reg = <0x2f>;
> +
> +        channels {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          phys0: channel@0 {
> +            reg = <0>;
> +          };
> +
> +          phys1: channel@1 {
> +            reg = <1>;
> +          };
> +
> +          phys2: channel@2 {
> +            reg = <2>;
> +          };
> +
> +          phys3: channel@3 {
> +            reg = <3>;
> +          };
> +        };

Blank line... you really folllow entirely different style :/



Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH net-next 2/2] net: pse-pd: Add LTC4266 PSE controller driver
  2025-06-03 23:04 ` [RFC PATCH net-next 2/2] net: pse-pd: Add LTC4266 PSE controller driver Kyle Swenson
@ 2025-06-04  8:39   ` Oleksij Rempel
  2025-06-09 14:23     ` Kyle Swenson
  0 siblings, 1 reply; 8+ messages in thread
From: Oleksij Rempel @ 2025-06-04  8:39 UTC (permalink / raw)
  To: Kyle Swenson
  Cc: kory.maincent@bootlin.com, andrew+netdev@lunn.ch,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org

Hi Kyle,

thank you for your work!

Are there any way to get manual with register description? I would like
to make a deeper review :)

On Tue, Jun 03, 2025 at 11:04:39PM +0000, Kyle Swenson wrote:
> Add a new driver for the Linear Technology LTC4266 I2C Power Sourcing
> Equipment controller.  This driver integrates with the current PSE
> controller core, implementing IEEE802.3af and IEEE802.3at PSE standards.
> ---
>  drivers/net/pse-pd/Kconfig   |  10 +
>  drivers/net/pse-pd/Makefile  |   1 +
>  drivers/net/pse-pd/ltc4266.c | 919 +++++++++++++++++++++++++++++++++++
>  3 files changed, 930 insertions(+)
>  create mode 100644 drivers/net/pse-pd/ltc4266.c
> 
> diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
> index 7fab916a7f46..a0f2eaadb4fb 100644
> --- a/drivers/net/pse-pd/Kconfig
> +++ b/drivers/net/pse-pd/Kconfig
> @@ -18,10 +18,20 @@ config PSE_REGULATOR
>  	help
>  	  This module provides support for simple regulator based Ethernet Power
>  	  Sourcing Equipment without automatic classification support. For
>  	  example for basic implementation of PoDL (802.3bu) specification.
>  
> +config PSE_LTC4266
> +	tristate "LTC4266 PSE controller"
> +	depends on I2C
> +	help
> +	  This module provides support the LTC4266 regulator based Ethernet
> +	  Power Sourcing Equipment.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ltc4266.
> +
>  config PSE_PD692X0
>  	tristate "PD692X0 PSE controller"
>  	depends on I2C
>  	select FW_LOADER
>  	select FW_UPLOAD
> diff --git a/drivers/net/pse-pd/Makefile b/drivers/net/pse-pd/Makefile
> index 9d2898b36737..a17e16467ae2 100644
> --- a/drivers/net/pse-pd/Makefile
> +++ b/drivers/net/pse-pd/Makefile
> @@ -1,8 +1,9 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  # Makefile for Linux PSE drivers
>  
>  obj-$(CONFIG_PSE_CONTROLLER) += pse_core.o
>  
> +obj-$(CONFIG_PSE_LTC4266) += ltc4266.o
>  obj-$(CONFIG_PSE_REGULATOR) += pse_regulator.o
>  obj-$(CONFIG_PSE_PD692X0) += pd692x0.o
>  obj-$(CONFIG_PSE_TPS23881) += tps23881.o
> diff --git a/drivers/net/pse-pd/ltc4266.c b/drivers/net/pse-pd/ltc4266.c
> new file mode 100644
> index 000000000000..858889c9ab75
> --- /dev/null
> +++ b/drivers/net/pse-pd/ltc4266.c
> @@ -0,0 +1,919 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for Linear LTC4266 PoE PSE Controller
> + *
> + * Original work:
> + *    Copyright 2019 Cradlepoint Technology, Inc.
> + *    Cradlepoint Technology, Inc.  <source@cradlepoint.com>
> + *
> + * Re-written in 2025:
> + *    Copyright 2025 Ericsson Software Technology
> + *    Kyle Swenson <kyle.swenson@est.tech>
> + *
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/ethtool.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/math.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pse-pd/pse.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +
> +#define LTC4266_REG_ID				0x1B
> +#define LTC4266_ID				0x64
> +
> +#define TWO_BIT_WORD_OFFSET(_v, _pid)		((_v) << ((_pid) * 2))
> +#define TWO_BIT_WORD_MASK(_pid)			TWO_BIT_WORD_OFFSET(0x03, (_pid))
> +
> +#define LTC4266_IPLSB_REG(_p)			(0x30 | ((_p) << 2))
> +#define LTC4266_VPLSB_REG(_p)			(LTC4266_IPLSB_REG(_p) + 2)
> +
> +#define LTC4266_RSTPB_INTCLR			BIT(7)
> +#define LTC4266_RSTPB_PINCLR			BIT(6)
> +#define LTC4266_RSTPB_RSTALL			BIT(5)
> +
> +/* Register definitions */
> +#define LTC4266_REG_INTSTAT			0x00
> +#define LTC4266_REG_INTMASK			0x01
> +#define LTC4266_REG_PWREVN_COR			0x03
> +#define LTC4266_REG_DETEVN_COR			0x05
> +#define LTC4266_REG_FLTEVN_COR			0x07
> +#define LTC4266_REG_TSEVN_COR			0x09
> +#define LTC4266_REG_SUPEVN_COR			0x0B
> +#define LTC4266_REG_STAT(n)			(0x0C + (n))
> +#define LTC4266_REG_STATPWR			0x10
> +#define LTC4266_REG_OPMD			0x12
> +#define LTC4266_REG_DISENA			0x13 /* Disconnect detect enable */
> +#define LTC4266_REG_MCONF			0x17
> +#define LTC4266_REG_DETPB			0x18
> +#define LTC4266_REG_PWRPB			0x19
> +#define LTC4266_REG_RSTPB			0x1A
> +#define LTC4266_REG_HPEN			0x44
> +#define LTC4266_REG_HPMD(_p)			(0x46 + (5 * (_p)))
> +#define LTC4266_REG_ILIM(_p)			(LTC4266_REG_HPMD(_p) + 2)
> +#define LTC4266_REG_TLIM12			0x1E
> +#define LTC4266_REG_TLIM34			0x1F
> +
> +/* Register field definitions */
> +#define LTC4266_HPMD_PONGEN			0x01
> +
> +/* For LTC4266_REG_TLIM* */
> +#define LTC4266_TLIM_VALUE			0x01
> +
> +/* LTC4266_REG_HPEN, enable "High Power" mode (i.e. Type 2, 25.4W PDs) */

Type 2 Class 4? Probably not, datasheet claims:
"Supports Proprietary Power Levels Above 25W"

> +#define LTC4266_HPEN(_p)			BIT(_p)
> +
> +/* LTC4266_REG_MCONF */
> +#define LTC4266_MCONF_INTERRUPT_ENABLE		BIT(7)
> +
> +/* LTC4266_REG_STATPWR */
> +#define LTC4266_STATPWR_PG(_p)			BIT((_p) + 4)
> +#define LTC4266_STATPWR_PE(_p)			BIT(_p)
> +#define LTC4266_PORT_CLASS(_stat)		FIELD_GET(GENMASK(6, 4), (_stat))
> +
> +#define LTC4266_REG_ICUT_HP(_p)			(LTC4266_REG_HPMD(_p) + 1)
> +
> +/* if R_sense = 0.25 Ohm, this should be set otherwise 0 */
> +#define LTC4266_ICUT_RSENSE			BIT(7)

LTC4266_ICUT_RSENSE_025_OHM

> +/* if set, halve the range and double the precision */
> +#define LTC4266_ICUT_RANGE			BIT(6)
> +
> +#define LTC4266_ILIM_AF_RSENSE_025		0x80
> +#define LTC4266_ILIM_AF_RSENSE_050		0x00
> +#define LTC4266_ILIM_AT_RSENSE_025		0xC0
> +#define LTC4266_ILIM_AT_RSENSE_050		0x40

Consider renaming constants AF/AT mentions.

Replace _AF_ with _TYPE1_ (e.g., LTC4266_ILIM_TYPE1_RSENSE_025)
Replace _AT_ with _TYPE2_ (e.g., LTC4266_ILIM_TYPE2_RSENSE_025)

The terms "Type 1" and "Type 2" are how the official IEEE 802.3 standard refers
to the PoE capabilities and power levels that were introduced by the 802.3af
and 802.3at amendments, respectively. Using "Type1" and "Type2" in your code
will make it clearer and more aligned with the current, consolidated IEEE
terminology, which is helpful since direct access to the original "af" and "at"
amendment documents can be challenging for the open-source community.

Do you have access to this amendments?

> +/* LTC4266_REG_INTSTAT and LTC4266_REG_INTMASK */
> +#define LTC4266_INT_SUPPLY			BIT(7)
> +#define LTC4266_INT_TSTART			BIT(6)
> +#define LTC4266_INT_TCUT			BIT(5)
> +#define LTC4266_INT_CLASS			BIT(4)
> +#define LTC4266_INT_DET				BIT(3)
> +#define LTC4266_INT_DIS				BIT(2)
> +#define LTC4266_INT_PWRGD			BIT(1)
> +#define LTC4266_INT_PWRENA			BIT(0)
> +
> +#define LTC4266_MAX_PORTS 4
> +
> +/* Maximum and minimum power limits for a single port */
> +#define LTC4266_PW_LIMIT_MAX 25400
> +#define LTC4266_PW_LIMIT_MIN 1
> +
> +enum {
> +	READ_CURRENT = 0,
> +	READ_VOLTAGE = 2
> +};
> +
> +enum {
> +	LTC4266_OPMD_SHUTDOWN = 0,
> +	LTC4266_OPMD_MANUAL,
> +	LTC4266_OPMD_SEMI,
> +	LTC4266_OPMD_AUTO

Please add explanations to this port modes

> +};
> +
> +/* Map LTC4266 Classification result to PD class */
> +static int ltc4266_class_map[] = {
> +	0, /* Treat as class 3 */
> +	1,
> +	2,
> +	3,
> +	4,
> +	-EINVAL,
> +	3, /* Treat as class 3 */
> +	-ERANGE
> +};
> +
> +/* Convert a class 0-4 to icut register value */
> +static int ltc4266_class_to_icut[] = {
> +	375,

missing comment, index 0 is class 3.

> +	112,
> +	206,
> +	375,
> +	638
> +};

May be we should have a generic function in the framework providing conversion
from class to min/max Icut and Ilim, otherwise it makes additional work
validation this values.

> +
> +enum sense_resistor {
> +	LTC4266_RSENSE_500, /* Rsense 0.5 Ohm */
> +	LTC4266_RSENSE_250 /* Rsense 0.25 Ohm */
> +};
> +
> +struct ltc4266_port {
> +	enum sense_resistor rsense;
> +	struct device_node *node;
> +	int current_limit;
> +};
> +
> +struct ltc4266 {
> +	struct i2c_client *client;
> +	struct mutex lock; /* Protect Read-Modify-Write Sequences */
> +	struct ltc4266_port *ports[LTC4266_MAX_PORTS];
> +	struct device *dev;
> +	struct device_node *np;
> +	struct pse_controller_dev pcdev;
> +};
> +
> +/* Read-modify-write sequence with value and mask.  Mask is expected to be
> + * shifted to the correct spot.
> + */
> +static int ltc4266_write_reg(struct ltc4266 *ltc4266, u8 reg, u8 value, u8 mask)

If it is Read-modify-write type of function, it would be better to name
it ltc4266_rmw_reg(). Or use regmap instead, you will get some extra
functionality: register dump over sysfs interface, range validation,
caching if enabled, locking, etc.

> +{
> +	int ret;
> +	u8 new;
> +
> +	mutex_lock(&ltc4266->lock);
> +	ret = i2c_smbus_read_byte_data(ltc4266->client, reg);
> +	if (ret < 0) {
> +		dev_warn(ltc4266->dev, "Failed to read register 0x%02x, err=%d\n", reg, ret);
> +		mutex_unlock(&ltc4266->lock);
> +		return ret;
> +	}
> +	new = (u8)ret;
> +	new &= ~mask;
> +	new |= value & mask;
> +	ret = i2c_smbus_write_byte_data(ltc4266->client, reg, new);
> +	mutex_unlock(&ltc4266->lock);
> +
> +	return ret;
> +}
> +
> +static int ltc4266_read_iv(struct ltc4266 *ltc4266, int port, u8 iv)
> +{
> +	int lsb;
> +	int msb;
> +	int result;
> +	int lsb_reg;
> +	u64 ivbits = 0;
> +
> +	if (iv == READ_CURRENT)
> +		lsb_reg = LTC4266_IPLSB_REG(port);
> +	else if (iv == READ_VOLTAGE)
> +		lsb_reg = LTC4266_VPLSB_REG(port);
> +	else
> +		return -EINVAL;
> +
> +	result = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_STATPWR);
> +	if (result < 0)
> +		return result;
> +
> +	/*  LTC4266 IV readings are only valid if the port is powered. */
> +	if (!(result & LTC4266_STATPWR_PG(port)))
> +		return -EINVAL;

We have two states:
- admin enabled: admin enabled state
- delivering power: PSE is actually delivering power

Please use this words to clarify what is actually happening.

> +	/* LTC4266 expects the MSB register to be read immediately following the LSB
> +	 * register, so we need to ensure other parts aren't reading other registers in
> +	 * this chip while we read the current/voltage regulators.
> +	 */
> +	mutex_lock(&ltc4266->lock);

please use guard* variants for locking.

> +
> +	lsb = i2c_smbus_read_byte_data(ltc4266->client, lsb_reg);
> +	msb = i2c_smbus_read_byte_data(ltc4266->client, lsb_reg + 1);
> +
> +	mutex_unlock(&ltc4266->lock);
> +
> +	if (lsb < 0)
> +		return lsb;
> +
> +	if (msb < 0)
> +		return msb;
> +
> +	ivbits = 0;
> +	ivbits |= ((u8)msb) << 8 | ((u8)lsb);
> +
> +	if (iv == READ_CURRENT)
> +		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250) /* 122.07 uA/LSB */
> +			result = DIV_ROUND_CLOSEST_ULL((ivbits * 122070), 1000);
> +		else /* 61.035 uA/LSB */
> +			result = DIV_ROUND_CLOSEST_ULL((ivbits * 61035), 1000);
> +	else /* 5.835 mV/LSB == 5835 uV/LSB */
> +		result = ivbits * 5835;
> +
> +	return result;
> +}
> +i

> +static int ltc4266_port_set_ilim(struct ltc4266 *ltc4266, int port, int class)
> +{
> +	if (class > 4 || class < 0)
> +		return -EINVAL;
> +
> +	/* We want to set 425 mA for class 3 and lower; 850 mA otherwise for IEEE compliance */
> +	if (class < 4) {
> +		/* Write 0x80 for 0.25 Ohm sense otherwise 0 */
> +		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> +			return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AF_RSENSE_025);
> +		return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AF_RSENSE_050);
> +	}
> +
> +	/* Class == 4 */
> +	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> +		return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AT_RSENSE_025);
> +	/* Class == 4 and the sense resistor is 0.5 */
> +	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AT_RSENSE_050);

May be something like this:
/*
 * ltc4266_port_set_ilim - Set the active current limit (ILIM) for a port.
 * @ltc4266: Pointer to the LTC4266 device structure.
 * @port: The port number (0-3).
 * @class: The detected PD class (0-4).
 *
 * This function configures the ILIM register (0x48, 0x4D, 0x52, 0x57)
 * of the LTC4266. The ILIM value determines the threshold at which the
 * PSE actively limits current to the PD. The chosen values are based on
 * IEEE Std 802.3-2022 requirements and typical operational values for the
 * LTC4266 controller.
 *
 * IEEE Std 802.3-2022, Table 33-11 specifies ILIM parameter ranges:
 * - For Type 1 PSE operation (typically PD Classes 0-3):
 * The minimum ILIM is 0.400A. This driver uses 425mA. This value fits
 * within typical Type 1 ILIM specifications (e.g., 0.400A min to
 * around 0.440A-0.500A max for the programmed steady-state limit).
 *
 * - For Type 2 PSE operation (typically PD Class 4):
 * The minimum ILIM is 1.14 * ICable (or ~1.05 * IPort_max from other
 * interpretations, e.g., ~0.630A to ~0.684A). This driver uses 850mA.
 * This value meets the minimum requirement and is a supported operational
 * current limit for high power modes in the LTC4266.
 *
 * The overall PSE current output must not exceed the time-dependent PSE
 * upperbound template, IPSEUT(t), described in IEEE Std 802.3-2022,
 * Equation (33-6). The programmed ILIM values (425mA/850mA) serve as the
 * long-term current limit (Ilimmin segment of IPSEUT(t)) and are well
 * within the higher short-term current allowances of that template (e.g., 1.75A).
 *
 * The specific register values written depend on the sense resistor
 * (0.25 Ohm or 0.50 Ohm) as detailed in the LTC4266 datasheet (Table 5).
 *
 * Returns: ...
 */
static int ltc4266_port_set_ilim(struct ltc4266 *ltc4266, int port, int class)
{
	u8 ilim_reg_val;

	if (class > 4 || class < 0)
		return -EINVAL;

	if (class < 4) {
		/* PD Class is 0, 1, 2, or 3 (Type 1 PSE operation).
		 * Set ILIM to 425mA.
		 */
		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250) {
			/* Using 0.25 Ohm sense resistor. */
			ilim_reg_val = LTC4266_ILIM_TYPE1_RSENSE_025;
		} else {
			/* Using 0.50 Ohm sense resistor. */
			ilim_reg_val = LTC4266_ILIM_TYPE1_RSENSE_050;
		}
	} else {
		/* PD Class is 4 (Type 2 PSE operation).
		 * Set ILIM to 850mA.
		 */
		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250) {
			/* Using 0.25 Ohm sense resistor. */
			ilim_reg_val = LTC4266_ILIM_TYPE2_RSENSE_025;
		} else {
			/* Using 0.50 Ohm sense resistor. */
			ilim_reg_val = LTC4266_ILIM_TYPE2_RSENSE_050;
		}
	}

	/* Write the determined ILIM value to the appropriate port's ILIM register.
	 * The LTC4266_REG_ILIM(port) macro resolves to the correct register
	 * address for the given port (e.g., 0x48 for port 0, 0x4D for port 1, etc.).
	 */
	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), ilim_reg_val);
}

> +static int ltc4266_port_set_icut(struct ltc4266 *ltc4266, int port, int icut)
> +{
> +	u8 val;
> +
> +	if (icut > 850)

It looks like board specific limit:
From the LTC4266 datasheet:
"Non-standard applications that provide more current
than the 850mA IEEE maximum may require heat sinking and other MOSFET design
considerations."

> +		return -ERANGE;
> +
> +	val = (u8)(DIV_ROUND_CLOSEST((icut * 1000), 18750) & 0x3F);

I assume 18750 micro Amp, per step in the register value and 0x3f is the max
mask for the bit field. In this case this register supports
0x3f * 18750 / 1000 = 1181mA

Please use defines to make it readable.

> +
> +	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> +		val |= LTC4266_ICUT_RSENSE | LTC4266_ICUT_RANGE;
> +
> +	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ICUT_HP(port), val);
> +}
> +
> +static int ltc4266_port_mode(struct ltc4266 *ltc4266, int port, u8 opmd)
> +{
> +	if (opmd >= LTC4266_OPMD_AUTO)
> +		return -EINVAL;
> +
> +	return ltc4266_write_reg(ltc4266, LTC4266_REG_OPMD, TWO_BIT_WORD_OFFSET(opmd, port),
> +				TWO_BIT_WORD_MASK(port));
> +}
> +
> +static int ltc4266_port_powered(struct ltc4266 *ltc4266, int port)

delivering or enabled?

> +{
> +	int result = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_STATPWR);
> +
> +	if (result < 0)
> +		return result;
> +
> +	return !!((result & LTC4266_STATPWR_PG(port)) && (result & LTC4266_STATPWR_PE(port)));
> +}
> +
> +static int ltc4266_port_init(struct ltc4266 *ltc4266, int port)
> +{
> +	int ret;
> +	u8 tlim_reg;
> +	u8 tlim_mask;
> +
> +	/* Reset the port */
> +	ret = i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_RSTPB, BIT(port));
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = ltc4266_port_mode(ltc4266, port, LTC4266_OPMD_SEMI);

Should we have disabled mode before all current limits configured?

> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable high power mode on the port (802.3at+) */

802.3at+? "Proprietary Power Levels Above 25W"?. Here we will need a discussion
how to reflect a Proprietary Power levels in the UAPI.

> +	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_HPEN,
> +				LTC4266_HPEN(port), LTC4266_HPEN(port));
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable Ping-Pong Classification */

This is probably "2-event classification" described in Clause 33 of the
IEEE Std 802.3-2022.

> +	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_HPMD(port),
> +				LTC4266_HPMD_PONGEN, LTC4266_HPMD_PONGEN);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> +		ret = ltc4266_write_reg(ltc4266, LTC4266_REG_ICUT_HP(port),
> +					LTC4266_ICUT_RSENSE, LTC4266_ICUT_RSENSE);
> +	else
> +		ret = ltc4266_write_reg(ltc4266, LTC4266_REG_ICUT_HP(port),
> +					0, LTC4266_ICUT_RSENSE);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	if (port <= 1)
> +		tlim_reg = LTC4266_REG_TLIM12;
> +	else
> +		tlim_reg = LTC4266_REG_TLIM34;
> +
> +	if (port & BIT(0))
> +		tlim_mask = GENMASK(7, 4);
> +	else
> +		tlim_mask = GENMASK(3, 0);
> +
> +	ret = ltc4266_write_reg(ltc4266, tlim_reg, LTC4266_TLIM_VALUE, tlim_mask);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable disconnect detect. */
> +	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_DISENA, BIT(port), BIT(port));
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable detection (low nibble), classification (high nibble) on the port */

This seems to correspond to ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED 

> +	ret = i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_DETPB,
> +					BIT(port + 4) | BIT(port));
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	dev_dbg(ltc4266->dev, "Port %d has been initialized\n", port);
> +	return 0;
> +}
> +
> +static int ltc4266_get_opmode(struct ltc4266 *ltc4266, int port)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_OPMD);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (port) {
> +	case 0:
> +		return FIELD_GET(GENMASK(1, 0), ret);
> +	case 1:
> +		return FIELD_GET(GENMASK(3, 2), ret);
> +	case 2:
> +		return FIELD_GET(GENMASK(5, 4), ret);
> +	case 3:
> +		return FIELD_GET(GENMASK(7, 6), ret);
> +	}
> +	return -EINVAL;
> +}
> +
> +static int ltc4266_pi_is_enabled(struct pse_controller_dev *pcdev, int id)
> +{
> +	int ret;
> +	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
> +
> +	ret = ltc4266_get_opmode(ltc4266, id);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret == LTC4266_OPMD_SEMI)
> +		return 1; /*  If a port is in OPMODE SEMI, we'll just assume admin has it enabled */

From HW perspective, every mode except of LTC4266_OPMD_SHUTDOWN can be seen as
admin state enabled. LTC4266_OPMD_MANUAL - is forced mode controlling
power delivery manually.

I need to make stop here, i'll try to review the rest later.

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] 8+ messages in thread

* Re: [RFC PATCH net-next 1/2] dt-bindings: net: pse-pd: Describe the LTC4266 PSE chipset
  2025-06-03 23:04 ` [RFC PATCH net-next 1/2] dt-bindings: net: pse-pd: Describe the LTC4266 PSE chipset Kyle Swenson
  2025-06-04  6:43   ` Krzysztof Kozlowski
@ 2025-06-04  9:06   ` Kory Maincent
  2025-06-04 14:53     ` Kyle Swenson
  1 sibling, 1 reply; 8+ messages in thread
From: Kory Maincent @ 2025-06-04  9:06 UTC (permalink / raw)
  To: Kyle Swenson
  Cc: o.rempel@pengutronix.de, andrew+netdev@lunn.ch,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org

Le Tue, 3 Jun 2025 23:04:37 +0000,
Kyle Swenson <kyle.swenson@est.tech> a écrit :

> Add the LTC4266 PSE controller from Linear Technology to the device-tree
> bindings.

Hello Kyle,

Nice to see your patch!

Your Sign-off-by tag is missing on all your patches.

> ---
>  .../bindings/net/pse-pd/lltc,ltc4266.yaml     | 146 ++++++++++++++++++
>  1 file changed, 146 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/net/pse-pd/lltc,ltc4266.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/pse-pd/lltc,ltc4266.yaml
> b/Documentation/devicetree/bindings/net/pse-pd/lltc,ltc4266.yaml new file
> mode 100644 index 000000000000..874447ab6c84
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pse-pd/lltc,ltc4266.yaml
> @@ -0,0 +1,146 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/pse-pd/lltc,ltc4266.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LTC LTC4266 Power Sourcing Equipment controller
> +
> +maintainers:
> +  - Kyle Swenson <kyle.swenson@est.tech>
> +
> +allOf:
> +  - $ref: pse-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - lltc,ltc4266
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#pse-cells':
> +    const: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  channels:
> +
> +    description: This parameter describes the mapping between the logical
> ports
> +      on the PSE controller and the physical ports.

The channels parameter is not describing the mapping but only the channels
list. I also have to change the tps23881 binding doc similarly.
We discussed about this here
https://lore.kernel.org/netdev/20250517003525.2f6a5005@kmaincent-XPS-13-7390/

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH net-next 1/2] dt-bindings: net: pse-pd: Describe the LTC4266 PSE chipset
  2025-06-04  9:06   ` Kory Maincent
@ 2025-06-04 14:53     ` Kyle Swenson
  0 siblings, 0 replies; 8+ messages in thread
From: Kyle Swenson @ 2025-06-04 14:53 UTC (permalink / raw)
  To: Kory Maincent
  Cc: o.rempel@pengutronix.de, andrew+netdev@lunn.ch,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org

On Wed, Jun 04, 2025 at 11:06:54AM +0200, Kory Maincent wrote:
> Le Tue, 3 Jun 2025 23:04:37 +0000,
> Kyle Swenson <kyle.swenson@est.tech> a écrit :
> 
> Hello Kyle,
> 
> Nice to see your patch!

Thanks!  A bit later than I had intended but late is better than never I
guess.

> 
> Your Sign-off-by tag is missing on all your patches.

My mistake.  I had intended to add that but evidently didn't.

> > +  channels:
> > +
> > +    description: This parameter describes the mapping between the logical
> > ports
> > +      on the PSE controller and the physical ports.
> 
> The channels parameter is not describing the mapping but only the channels
> list. I also have to change the tps23881 binding doc similarly.
> We discussed about this here
> https://lore.kernel.org/netdev/20250517003525.2f6a5005@kmaincent-XPS-13-7390/

Oh, thanks for the link.  I'll fix this (and all the other issues in the
binding) in v2.

Thanks,
Kyle

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH net-next 2/2] net: pse-pd: Add LTC4266 PSE controller driver
  2025-06-04  8:39   ` Oleksij Rempel
@ 2025-06-09 14:23     ` Kyle Swenson
  0 siblings, 0 replies; 8+ messages in thread
From: Kyle Swenson @ 2025-06-09 14:23 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: kory.maincent@bootlin.com, andrew+netdev@lunn.ch,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org

On Wed, Jun 04, 2025 at 10:39:17AM +0200, Oleksij Rempel wrote:
> Hi Kyle,
> 
> thank you for your work!
> 
> Are there any way to get manual with register description? I would like
> to make a deeper review :)

Yes, but unfortunately I think you'll need to make an account on the Analog Devices
website and go through the "Request Software" process for what they call
the "Software Interface" datasheet.

> On Tue, Jun 03, 2025 at 11:04:39PM +0000, Kyle Swenson wrote:
> > Add a new driver for the Linear Technology LTC4266 I2C Power Sourcing
> > Equipment controller.  This driver integrates with the current PSE
> > controller core, implementing IEEE802.3af and IEEE802.3at PSE standards.
[snip]
> > +#define LTC4266_TLIM_VALUE			0x01
> > +
> > +/* LTC4266_REG_HPEN, enable "High Power" mode (i.e. Type 2, 25.4W PDs) */
> 
> Type 2 Class 4? Probably not, datasheet claims:
> "Supports Proprietary Power Levels Above 25W"
Yes, Type 2 in this context means a Class 4 device.  I think the
comment in the datasheet is referring to the "4-Point PD detection
circuitry" but it's a little unclear.  That said, I've no intent in
supporting these proprietary power levels unless they're the same as
what's described in the IEEE spec.

> > +#define LTC4266_HPEN(_p)			BIT(_p)
> > +
> > +/* LTC4266_REG_MCONF */
> > +#define LTC4266_MCONF_INTERRUPT_ENABLE		BIT(7)
> > +
> > +/* LTC4266_REG_STATPWR */
> > +#define LTC4266_STATPWR_PG(_p)			BIT((_p) + 4)
> > +#define LTC4266_STATPWR_PE(_p)			BIT(_p)
> > +#define LTC4266_PORT_CLASS(_stat)		FIELD_GET(GENMASK(6, 4), (_stat))
> > +
> > +#define LTC4266_REG_ICUT_HP(_p)			(LTC4266_REG_HPMD(_p) + 1)
> > +
> > +/* if R_sense = 0.25 Ohm, this should be set otherwise 0 */
> > +#define LTC4266_ICUT_RSENSE			BIT(7)
> 
> LTC4266_ICUT_RSENSE_025_OHM
Ack.  I'll also fix up the indentation that's (now) obviously
misaligned.

> > +/* if set, halve the range and double the precision */
> > +#define LTC4266_ICUT_RANGE			BIT(6)
> > +
> > +#define LTC4266_ILIM_AF_RSENSE_025		0x80
> > +#define LTC4266_ILIM_AF_RSENSE_050		0x00
> > +#define LTC4266_ILIM_AT_RSENSE_025		0xC0
> > +#define LTC4266_ILIM_AT_RSENSE_050		0x40
> 
> Consider renaming constants AF/AT mentions.
> 
> Replace _AF_ with _TYPE1_ (e.g., LTC4266_ILIM_TYPE1_RSENSE_025)
> Replace _AT_ with _TYPE2_ (e.g., LTC4266_ILIM_TYPE2_RSENSE_025)
Sounds good, will do.

> The terms "Type 1" and "Type 2" are how the official IEEE 802.3 standard refers
> to the PoE capabilities and power levels that were introduced by the 802.3af
> and 802.3at amendments, respectively. Using "Type1" and "Type2" in your code
> will make it clearer and more aligned with the current, consolidated IEEE
> terminology, which is helpful since direct access to the original "af" and "at"
> amendment documents can be challenging for the open-source community.
Ah, makes sense.  I'll change AF/AT related bits to Type 1 and Type 2.

> Do you have access to this amendments?
I don't have them currently, but I'll ask around my organization and see if I can get access.

> > +/* LTC4266_REG_INTSTAT and LTC4266_REG_INTMASK */
> > +#define LTC4266_INT_SUPPLY			BIT(7)
> > +#define LTC4266_INT_TSTART			BIT(6)
> > +#define LTC4266_INT_TCUT			BIT(5)
> > +#define LTC4266_INT_CLASS			BIT(4)
> > +#define LTC4266_INT_DET				BIT(3)
> > +#define LTC4266_INT_DIS				BIT(2)
> > +#define LTC4266_INT_PWRGD			BIT(1)
> > +#define LTC4266_INT_PWRENA			BIT(0)
> > +
> > +#define LTC4266_MAX_PORTS 4
> > +
> > +/* Maximum and minimum power limits for a single port */
> > +#define LTC4266_PW_LIMIT_MAX 25400
> > +#define LTC4266_PW_LIMIT_MIN 1
> > +
> > +enum {
> > +	READ_CURRENT = 0,
> > +	READ_VOLTAGE = 2
> > +};
> > +
> > +enum {
> > +	LTC4266_OPMD_SHUTDOWN = 0,
> > +	LTC4266_OPMD_MANUAL,
> > +	LTC4266_OPMD_SEMI,
> > +	LTC4266_OPMD_AUTO
> 
> Please add explanations to this port modes
Will do, I see now the confusion this has caused.  Sorry for that.

> > +};
> > +
> > +/* Map LTC4266 Classification result to PD class */
> > +static int ltc4266_class_map[] = {
> > +	0, /* Treat as class 3 */
> > +	1,
> > +	2,
> > +	3,
> > +	4,
> > +	-EINVAL,
> > +	3, /* Treat as class 3 */
> > +	-ERANGE
> > +};
> > +
> > +/* Convert a class 0-4 to icut register value */
> > +static int ltc4266_class_to_icut[] = {
> > +	375,
> 
> missing comment, index 0 is class 3.
Ack.

> > +	112,
> > +	206,
> > +	375,
> > +	638
> > +};
> 
> May be we should have a generic function in the framework providing conversion
> from class to min/max Icut and Ilim, otherwise it makes additional work
> validation this values.
Sure, I'm open to something like this, even more so if other PSE
chipsets use a current limit instead of a power limit.

> > +
> > +enum sense_resistor {
> > +	LTC4266_RSENSE_500, /* Rsense 0.5 Ohm */
> > +	LTC4266_RSENSE_250 /* Rsense 0.25 Ohm */
> > +};
> > +
> > +struct ltc4266_port {
> > +	enum sense_resistor rsense;
> > +	struct device_node *node;
> > +	int current_limit;
> > +};
> > +
> > +struct ltc4266 {
> > +	struct i2c_client *client;
> > +	struct mutex lock; /* Protect Read-Modify-Write Sequences */
> > +	struct ltc4266_port *ports[LTC4266_MAX_PORTS];
> > +	struct device *dev;
> > +	struct device_node *np;
> > +	struct pse_controller_dev pcdev;
> > +};
> > +
> > +/* Read-modify-write sequence with value and mask.  Mask is expected to be
> > + * shifted to the correct spot.
> > + */
> > +static int ltc4266_write_reg(struct ltc4266 *ltc4266, u8 reg, u8 value, u8 mask)
> 
> If it is Read-modify-write type of function, it would be better to name
> it ltc4266_rmw_reg(). Or use regmap instead, you will get some extra
> functionality: register dump over sysfs interface, range validation,
> caching if enabled, locking, etc.
Oh, good call- I'll use regmap.

> > +{
> > +	int ret;
> > +	u8 new;
> > +
> > +	mutex_lock(&ltc4266->lock);
> > +	ret = i2c_smbus_read_byte_data(ltc4266->client, reg);
> > +	if (ret < 0) {
> > +		dev_warn(ltc4266->dev, "Failed to read register 0x%02x, err=%d\n", reg, ret);
> > +		mutex_unlock(&ltc4266->lock);
> > +		return ret;
> > +	}
> > +	new = (u8)ret;
> > +	new &= ~mask;
> > +	new |= value & mask;
> > +	ret = i2c_smbus_write_byte_data(ltc4266->client, reg, new);
> > +	mutex_unlock(&ltc4266->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ltc4266_read_iv(struct ltc4266 *ltc4266, int port, u8 iv)
> > +{
> > +	int lsb;
> > +	int msb;
> > +	int result;
> > +	int lsb_reg;
> > +	u64 ivbits = 0;
> > +
> > +	if (iv == READ_CURRENT)
> > +		lsb_reg = LTC4266_IPLSB_REG(port);
> > +	else if (iv == READ_VOLTAGE)
> > +		lsb_reg = LTC4266_VPLSB_REG(port);
> > +	else
> > +		return -EINVAL;
> > +
> > +	result = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_STATPWR);
> > +	if (result < 0)
> > +		return result;
> > +
> > +	/*  LTC4266 IV readings are only valid if the port is powered. */
> > +	if (!(result & LTC4266_STATPWR_PG(port)))
> > +		return -EINVAL;
> 
> We have two states:
> - admin enabled: admin enabled state
> - delivering power: PSE is actually delivering power
> 
> Please use this words to clarify what is actually happening.
Will do.

> > +	/* LTC4266 expects the MSB register to be read immediately following the LSB
> > +	 * register, so we need to ensure other parts aren't reading other registers in
> > +	 * this chip while we read the current/voltage regulators.
> > +	 */
> > +	mutex_lock(&ltc4266->lock);
> 
> please use guard* variants for locking.
Ack.

> 
> > +
> > +	lsb = i2c_smbus_read_byte_data(ltc4266->client, lsb_reg);
> > +	msb = i2c_smbus_read_byte_data(ltc4266->client, lsb_reg + 1);
> > +
> > +	mutex_unlock(&ltc4266->lock);
> > +
> > +	if (lsb < 0)
> > +		return lsb;
> > +
> > +	if (msb < 0)
> > +		return msb;
> > +
> > +	ivbits = 0;
> > +	ivbits |= ((u8)msb) << 8 | ((u8)lsb);
> > +
> > +	if (iv == READ_CURRENT)
> > +		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250) /* 122.07 uA/LSB */
> > +			result = DIV_ROUND_CLOSEST_ULL((ivbits * 122070), 1000);
> > +		else /* 61.035 uA/LSB */
> > +			result = DIV_ROUND_CLOSEST_ULL((ivbits * 61035), 1000);
> > +	else /* 5.835 mV/LSB == 5835 uV/LSB */
> > +		result = ivbits * 5835;
> > +
> > +	return result;
> > +}
> 
> > +static int ltc4266_port_set_ilim(struct ltc4266 *ltc4266, int port, int class)
> > +{
> > +	if (class > 4 || class < 0)
> > +		return -EINVAL;
> > +
> > +	/* We want to set 425 mA for class 3 and lower; 850 mA otherwise for IEEE compliance */
> > +	if (class < 4) {
> > +		/* Write 0x80 for 0.25 Ohm sense otherwise 0 */
> > +		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> > +			return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AF_RSENSE_025);
> > +		return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AF_RSENSE_050);
> > +	}
> > +
> > +	/* Class == 4 */
> > +	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> > +		return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AT_RSENSE_025);
> > +	/* Class == 4 and the sense resistor is 0.5 */
> > +	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AT_RSENSE_050);
> 
> May be something like this:
> /*
>  * ltc4266_port_set_ilim - Set the active current limit (ILIM) for a port.
>  * @ltc4266: Pointer to the LTC4266 device structure.
>  * @port: The port number (0-3).
>  * @class: The detected PD class (0-4).
>  *
>  * This function configures the ILIM register (0x48, 0x4D, 0x52, 0x57)
>  * of the LTC4266. The ILIM value determines the threshold at which the
>  * PSE actively limits current to the PD. The chosen values are based on
>  * IEEE Std 802.3-2022 requirements and typical operational values for the
>  * LTC4266 controller.
>  *
>  * IEEE Std 802.3-2022, Table 33-11 specifies ILIM parameter ranges:
>  * - For Type 1 PSE operation (typically PD Classes 0-3):
>  * The minimum ILIM is 0.400A. This driver uses 425mA. This value fits
>  * within typical Type 1 ILIM specifications (e.g., 0.400A min to
>  * around 0.440A-0.500A max for the programmed steady-state limit).
>  *
>  * - For Type 2 PSE operation (typically PD Class 4):
>  * The minimum ILIM is 1.14 * ICable (or ~1.05 * IPort_max from other
>  * interpretations, e.g., ~0.630A to ~0.684A). This driver uses 850mA.
>  * This value meets the minimum requirement and is a supported operational
>  * current limit for high power modes in the LTC4266.
>  *
>  * The overall PSE current output must not exceed the time-dependent PSE
>  * upperbound template, IPSEUT(t), described in IEEE Std 802.3-2022,
>  * Equation (33-6). The programmed ILIM values (425mA/850mA) serve as the
>  * long-term current limit (Ilimmin segment of IPSEUT(t)) and are well
>  * within the higher short-term current allowances of that template (e.g., 1.75A).
>  *
>  * The specific register values written depend on the sense resistor
>  * (0.25 Ohm or 0.50 Ohm) as detailed in the LTC4266 datasheet (Table 5).
>  *
>  * Returns: ...
>  */
> static int ltc4266_port_set_ilim(struct ltc4266 *ltc4266, int port, int class)
> {
> 	u8 ilim_reg_val;
> 
> 	if (class > 4 || class < 0)
> 		return -EINVAL;
> 
> 	if (class < 4) {
> 		/* PD Class is 0, 1, 2, or 3 (Type 1 PSE operation).
> 		 * Set ILIM to 425mA.
> 		 */
> 		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250) {
> 			/* Using 0.25 Ohm sense resistor. */
> 			ilim_reg_val = LTC4266_ILIM_TYPE1_RSENSE_025;
> 		} else {
> 			/* Using 0.50 Ohm sense resistor. */
> 			ilim_reg_val = LTC4266_ILIM_TYPE1_RSENSE_050;
> 		}
> 	} else {
> 		/* PD Class is 4 (Type 2 PSE operation).
> 		 * Set ILIM to 850mA.
> 		 */
> 		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250) {
> 			/* Using 0.25 Ohm sense resistor. */
> 			ilim_reg_val = LTC4266_ILIM_TYPE2_RSENSE_025;
> 		} else {
> 			/* Using 0.50 Ohm sense resistor. */
> 			ilim_reg_val = LTC4266_ILIM_TYPE2_RSENSE_050;
> 		}
> 	}
> 
> 	/* Write the determined ILIM value to the appropriate port's ILIM register.
> 	 * The LTC4266_REG_ILIM(port) macro resolves to the correct register
> 	 * address for the given port (e.g., 0x48 for port 0, 0x4D for port 1, etc.).
> 	 */
> 	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), ilim_reg_val);
> }
Wow, that is much more clear.  Thanks so much, I'll change accordingly.

> > +static int ltc4266_port_set_icut(struct ltc4266 *ltc4266, int port, int icut)
> > +{
> > +	u8 val;
> > +
> > +	if (icut > 850)
> 
> It looks like board specific limit:
> From the LTC4266 datasheet:
> "Non-standard applications that provide more current
> than the 850mA IEEE maximum may require heat sinking and other MOSFET design
> considerations."
Yes, it is board specific but also sounded to me like above 850mA would
violate the spec.

> > +		return -ERANGE;
> > +
> > +	val = (u8)(DIV_ROUND_CLOSEST((icut * 1000), 18750) & 0x3F);
> 
> I assume 18750 micro Amp, per step in the register value and 0x3f is the max
> mask for the bit field. In this case this register supports
> 0x3f * 18750 / 1000 = 1181mA
> 
> Please use defines to make it readable.
Will do.  I'll do this for all the other current/voltage conversions as
well. 

> > +
> > +	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> > +		val |= LTC4266_ICUT_RSENSE | LTC4266_ICUT_RANGE;
> > +
> > +	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ICUT_HP(port), val);
> > +}
> > +
> > +static int ltc4266_port_mode(struct ltc4266 *ltc4266, int port, u8 opmd)
> > +{
> > +	if (opmd >= LTC4266_OPMD_AUTO)
> > +		return -EINVAL;
> > +
> > +	return ltc4266_write_reg(ltc4266, LTC4266_REG_OPMD, TWO_BIT_WORD_OFFSET(opmd, port),
> > +				TWO_BIT_WORD_MASK(port));
> > +}
> > +
> > +static int ltc4266_port_powered(struct ltc4266 *ltc4266, int port)
> 
> delivering or enabled?
Delivering.  I'll change this per your earlier comment.

> > +{
> > +	int result = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_STATPWR);
> > +
> > +	if (result < 0)
> > +		return result;
> > +
> > +	return !!((result & LTC4266_STATPWR_PG(port)) && (result & LTC4266_STATPWR_PE(port)));
> > +}
> > +
> > +static int ltc4266_port_init(struct ltc4266 *ltc4266, int port)
> > +{
> > +	int ret;
> > +	u8 tlim_reg;
> > +	u8 tlim_mask;
> > +
> > +	/* Reset the port */
> > +	ret = i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_RSTPB, BIT(port));
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = ltc4266_port_mode(ltc4266, port, LTC4266_OPMD_SEMI);
> 
> Should we have disabled mode before all current limits configured?
Well, OPMD_SEMI means "Semi-Automatic" mode, which will detect and
classify connected PDs but not power them until instructed to do so by
software. 

> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Enable high power mode on the port (802.3at+) */
> 
> 802.3at+? "Proprietary Power Levels Above 25W"?. Here we will need a discussion
> how to reflect a Proprietary Power levels in the UAPI.
I don't know there is value in this discussion because a) I don't know what the
datasheet is referring to about these "Proprietary Power Levels" and b)
I'm not really interested in adding support for any proprietary power
levels because I can't test them.  The LTC4266 can power Type 1 and Type
2 PDs.  There is a support for high-capacitance devices (seen in really
old Cisco IP Phones), but this support I left (or intended to) disabled
because I can't test that, either.
> 
> > +	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_HPEN,
> > +				LTC4266_HPEN(port), LTC4266_HPEN(port));
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Enable Ping-Pong Classification */
> 
> This is probably "2-event classification" described in Clause 33 of the
> IEEE Std 802.3-2022.
Yes, this is exactly that.
> 
> > +	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_HPMD(port),
> > +				LTC4266_HPMD_PONGEN, LTC4266_HPMD_PONGEN);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> > +		ret = ltc4266_write_reg(ltc4266, LTC4266_REG_ICUT_HP(port),
> > +					LTC4266_ICUT_RSENSE, LTC4266_ICUT_RSENSE);
> > +	else
> > +		ret = ltc4266_write_reg(ltc4266, LTC4266_REG_ICUT_HP(port),
> > +					0, LTC4266_ICUT_RSENSE);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	if (port <= 1)
> > +		tlim_reg = LTC4266_REG_TLIM12;
> > +	else
> > +		tlim_reg = LTC4266_REG_TLIM34;
> > +
> > +	if (port & BIT(0))
> > +		tlim_mask = GENMASK(7, 4);
> > +	else
> > +		tlim_mask = GENMASK(3, 0);
> > +
> > +	ret = ltc4266_write_reg(ltc4266, tlim_reg, LTC4266_TLIM_VALUE, tlim_mask);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Enable disconnect detect. */
> > +	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_DISENA, BIT(port), BIT(port));
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Enable detection (low nibble), classification (high nibble) on the port */
> 
> This seems to correspond to ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED 
Yes, the combination of OPMD_SEMI, PD Detection enabled and PD
classification enabled corresponds to ADMIN_STATE_ENABLED.
> 
> > +	ret = i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_DETPB,
> > +					BIT(port + 4) | BIT(port));
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	dev_dbg(ltc4266->dev, "Port %d has been initialized\n", port);
> > +	return 0;
> > +}
> > +
> > +static int ltc4266_get_opmode(struct ltc4266 *ltc4266, int port)
> > +{
> > +	int ret;
> > +
> > +	ret = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_OPMD);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	switch (port) {
> > +	case 0:
> > +		return FIELD_GET(GENMASK(1, 0), ret);
> > +	case 1:
> > +		return FIELD_GET(GENMASK(3, 2), ret);
> > +	case 2:
> > +		return FIELD_GET(GENMASK(5, 4), ret);
> > +	case 3:
> > +		return FIELD_GET(GENMASK(7, 6), ret);
> > +	}
> > +	return -EINVAL;
> > +}
> > +
> > +static int ltc4266_pi_is_enabled(struct pse_controller_dev *pcdev, int id)
> > +{
> > +	int ret;
> > +	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
> > +
> > +	ret = ltc4266_get_opmode(ltc4266, id);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	if (ret == LTC4266_OPMD_SEMI)
> > +		return 1; /*  If a port is in OPMODE SEMI, we'll just assume admin has it enabled */
> 
> From HW perspective, every mode except of LTC4266_OPMD_SHUTDOWN can be seen as
> admin state enabled. LTC4266_OPMD_MANUAL - is forced mode controlling
> power delivery manually.
In V2, I'll add a lot more information in comments and such that
describe these modes a little better, but until then:

OPMD_SHUTDOWN means that port is off and will not run any detection or
classification cycles regardless of other configuration.

OPMD_MANUAL means the port will run a single detect cycle after the
corresponding bit is set;  additionally, it will run a single
classification cycle after that bit is set.  The problem with
OPMD_MANUAL is that there is no state enforcement in hardware, meaning
software could instruct the PSE to apply power to a port that doesn't
have a valid PD attached, and will apply power regardless of the result
(if any) from detection/classification.  Frankly, OPMD_MANUAL scares me
and I see supporting it as only adding risk with no value.

OPMD_SEMI means the chip will detect and classify PDs on enabled ports
and will wait until software sets the corresponding bit to power on the
port and (critically) only will power on a port with a valid detection
and classification result.

OPMD_AUTO requires the chip be powered on with the "auto" pin high, and
will automatically detect, classify, and power any valid PD.  I don't
have a board with this configuration so I've not included that support.
It's a little unclear to me what is possible to configure from software
when the chip is in this mode.

> 
> I need to make stop here, i'll try to review the rest later.
Thanks so much for the amazing review thus far, I think I've got more
than enough to adjust for a V2.  

> Regards,
> Oleksij

Thanks again,
Kyle

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-06-09 14:23 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-03 23:04 [RFC PATCH net-next 0/2] Add support for the LTC4266 PSE Controller Kyle Swenson
2025-06-03 23:04 ` [RFC PATCH net-next 1/2] dt-bindings: net: pse-pd: Describe the LTC4266 PSE chipset Kyle Swenson
2025-06-04  6:43   ` Krzysztof Kozlowski
2025-06-04  9:06   ` Kory Maincent
2025-06-04 14:53     ` Kyle Swenson
2025-06-03 23:04 ` [RFC PATCH net-next 2/2] net: pse-pd: Add LTC4266 PSE controller driver Kyle Swenson
2025-06-04  8:39   ` Oleksij Rempel
2025-06-09 14:23     ` Kyle Swenson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).