Devicetree
 help / color / mirror / Atom feed
* [PATCH v5 0/2] usb: typec: ucsi: Add ITE IT8851/IT8853 support
@ 2026-07-24 10:09 Amber Kao
  2026-07-24 10:09 ` [PATCH v5 1/2] dt-bindings: usb: Add ITE IT8851/IT8853 Type-C PD controllers Amber Kao
  2026-07-24 10:09 ` [PATCH v5 2/2] usb: typec: ucsi: Add ITE IT8851/IT8853 Type-C PD controller driver Amber Kao
  0 siblings, 2 replies; 4+ messages in thread
From: Amber Kao @ 2026-07-24 10:09 UTC (permalink / raw)
  To: Jeson Yang, Yaode Fang, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heikki Krogerus
  Cc: linux-usb, devicetree, linux-kernel, Amber Kao, Bling Chiang,
	Eric Su, Doreen Lin

This series adds Device Tree bindings and minimal UCSI driver support 
for the ITE IT8851/IT8853 USB Type-C Power Delivery controller over I2C.

Changes in v5:
- dt-bindings: express compatible as ite,it8853 + ite,it8851 fallback
  instead of a plain enum, since the driver treats both parts
  identically (Krzysztof)
- dt-bindings: use consistent quoting for patternProperties key
  (Krzysztof)
- dt-bindings: add anyOf requiring at least one of connector@0/
  connector@1 (Krzysztof)
- Drop the unnecessary heap-allocated tx/rx buffers in
  itepd_read_reg()/itepd_write_reg(); use the caller-supplied buffer
  and a local variable directly instead (Heikki)
- Ack the interrupt in itepd_process_event() even when the CCI/
  msg-in I2C read fails, instead of bailing out before acking, to
  avoid an interrupt storm on transient I2C errors (Sashiko)
- Not changed: keep ucsi_unregister() before free_irq() in remove().
  the IRQ handler is the only event consumer/acker on this device,
  so freeing the IRQ first stalls in-flight commands until timeout.
  ucsi_unregister() disables notifications via async_control() first,
  narrowing the reported UAF window to the interval before free_irq() 
  masks the interrupt; IRQF_ONESHOT and free_irq()'s synchronous wait
  mean only a newly-arriving event in that narrow window remains exposed
  (Sashiko, not addressed)
- Not changed: keep itepd_read_reg()/itepd_write_reg() built on
  i2c_transfer() rather than switching to
  i2c_smbus_{read,write}_i2c_block_data(). The read path needs this
  regardless, since the message-in register can return up to 40
  bytes, exceeding the 32-byte SMBus block-read limit; write is kept
  on the same helper style for consistency rather than mixing APIs
  for a few lines of savings. (Heikki, not addressed)

Link to v4: https://lore.kernel.org/all/20260717-ucsi-itepd-v4-0-f3ba5addd0df@ite.com.tw/
Link to v3: https://lore.kernel.org/all/20260710-ucsi-itepd-feature-v3-0-8523e1e1c47a@ite.com.tw/
Link to v2: https://lore.kernel.org/all/20260710-ucsi-itepd-feature-v2-0-41943fd5df38@ite.com.tw/
Link to v1: https://lore.kernel.org/all/20260615-ucsi-itepd-feature-v1-0-a826cfd0df6a@ite.com.tw/

Signed-off-by: Amber Kao <amber.kao@ite.com.tw>
---
Amber Kao (2):
      dt-bindings: usb: Add ITE IT8851/IT8853 Type-C PD controllers
      usb: typec: ucsi: Add ITE IT8851/IT8853 Type-C PD controller driver

 .../devicetree/bindings/usb/ite,it8851.yaml        | 151 +++++++++
 MAINTAINERS                                        |   9 +
 drivers/usb/typec/ucsi/Kconfig                     |  10 +
 drivers/usb/typec/ucsi/Makefile                    |   1 +
 drivers/usb/typec/ucsi/ucsi_itepd.c                | 365 +++++++++++++++++++++
 5 files changed, 536 insertions(+)
---
base-commit: 8fde5d1d47f69db6082dfa34500c27f8485389a5
change-id: 20260717-ucsi-itepd-4e943e1382f0

Best regards,
--  
Amber Kao <amber.kao@ite.com.tw>


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

* [PATCH v5 1/2] dt-bindings: usb: Add ITE IT8851/IT8853 Type-C PD controllers
  2026-07-24 10:09 [PATCH v5 0/2] usb: typec: ucsi: Add ITE IT8851/IT8853 support Amber Kao
@ 2026-07-24 10:09 ` Amber Kao
  2026-07-24 10:09 ` [PATCH v5 2/2] usb: typec: ucsi: Add ITE IT8851/IT8853 Type-C PD controller driver Amber Kao
  1 sibling, 0 replies; 4+ messages in thread
From: Amber Kao @ 2026-07-24 10:09 UTC (permalink / raw)
  To: Jeson Yang, Yaode Fang, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heikki Krogerus
  Cc: linux-usb, devicetree, linux-kernel, Amber Kao, Bling Chiang,
	Eric Su, Doreen Lin

Add device tree binding for the ITE IT8851 and IT8853 I2C-based USB
Type-C Power Delivery controllers.
The IT8851 supports one Type-C port; the IT8853 is the two-port variant
sharing the same interface.

Add a MAINTAINERS entry for the binding.

Cc: Yaode Fang <Yaode.Fang@ite.com.tw>
Cc: Jeson Yang <jeson.yang@ite.com.tw>
Cc: Bling Chiang <Bling.Chiang@ite.com.tw>
Cc: Eric Su <Eric.Su@ite.com.tw>
Cc: Doreen Lin <doreen.lin@ite.com.tw>
Signed-off-by: Amber Kao <amber.kao@ite.com.tw>
---
 .../devicetree/bindings/usb/ite,it8851.yaml        | 151 +++++++++++++++++++++
 MAINTAINERS                                        |   8 ++
 2 files changed, 159 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ite,it8851.yaml b/Documentation/devicetree/bindings/usb/ite,it8851.yaml
new file mode 100644
index 000000000000..246d34734b4a
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ite,it8851.yaml
@@ -0,0 +1,151 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/ite,it8851.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ITE IT8851/IT8853 USB Type-C Power Delivery Controller
+
+maintainers:
+  - Jeson Yang <jeson.yang@ite.com.tw>
+
+description:
+  The ITE IT8851 is an I2C-based USB Type-C Power Delivery (PD) controller
+  supporting one Type-C port.
+  The IT8853 is the two-port variant sharing the same programming interface.
+
+properties:
+  compatible:
+    oneOf:
+      - const: ite,it8851
+      - items:
+          - const: ite,it8853
+          - const: ite,it8851
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wakeup-source: true
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+patternProperties:
+  '^connector@[01]$':
+    $ref: /schemas/connector/usb-connector.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+anyOf:
+  - required:
+      - connector@0
+  - required:
+      - connector@1
+
+allOf:
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: ite,it8853
+    then:
+      properties:
+        connector@1: false
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        typec@40 {
+            compatible = "ite,it8853", "ite,it8851";
+            reg = <0x40>;
+            interrupts-extended = <&tlmm 129 IRQ_TYPE_EDGE_FALLING>;
+            wakeup-source;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            connector@0 {
+                compatible = "usb-c-connector";
+                reg = <0>;
+                label = "USB-C-0";
+                power-role = "dual";
+                data-role = "dual";
+
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@0 {
+                        reg = <0>;
+                        endpoint {
+                            remote-endpoint = <&eud_con>;
+                        };
+                    };
+
+                    port@1 {
+                        reg = <1>;
+                        endpoint {
+                            remote-endpoint = <&redriver_ss_out>;
+                        };
+                    };
+
+                    port@2 {
+                        reg = <2>;
+                        endpoint {
+                            remote-endpoint = <&fsa4480_sbu_mux>;
+                        };
+                    };
+                };
+            };
+
+            connector@1 {
+                compatible = "usb-c-connector";
+                reg = <1>;
+                label = "USB-C-1";
+                power-role = "dual";
+                data-role = "dual";
+
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@0 {
+                        reg = <0>;
+                        endpoint {
+                          remote-endpoint = <&usb1_hs_con>;
+                        };
+                    };
+                    port@1 {
+                        reg = <1>;
+                        endpoint {
+                          remote-endpoint = <&redriver1_ss_out>;
+                        };
+                    };
+                    port@2 {
+                        reg = <2>;
+                        endpoint {
+                          remote-endpoint = <&sbu_mux1>;
+                        };
+                    };
+                };
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 461a3eed6129..ec0eeea79b9e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13720,6 +13720,14 @@ T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
 F:	Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
 F:	drivers/gpu/drm/bridge/ite-it66121.c
 
+ITE IT8851 TYPE-C PD CONTROLLER DRIVER
+M:	Amber Kao <amber.kao@ite.com.tw>
+R:	Jeson Yang <jeson.yang@ite.com.tw>
+R:	Yaode Fang <Yaode.Fang@ite.com.tw>
+L:	linux-usb@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/usb/ite,it8851.yaml
+
 IVTV VIDEO4LINUX DRIVER
 M:	Andy Walls <awalls@md.metrocast.net>
 L:	linux-media@vger.kernel.org

-- 
2.53.0


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

* [PATCH v5 2/2] usb: typec: ucsi: Add ITE IT8851/IT8853 Type-C PD controller driver
  2026-07-24 10:09 [PATCH v5 0/2] usb: typec: ucsi: Add ITE IT8851/IT8853 support Amber Kao
  2026-07-24 10:09 ` [PATCH v5 1/2] dt-bindings: usb: Add ITE IT8851/IT8853 Type-C PD controllers Amber Kao
@ 2026-07-24 10:09 ` Amber Kao
  2026-07-24 10:23   ` sashiko-bot
  1 sibling, 1 reply; 4+ messages in thread
From: Amber Kao @ 2026-07-24 10:09 UTC (permalink / raw)
  To: Jeson Yang, Yaode Fang, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heikki Krogerus
  Cc: linux-usb, devicetree, linux-kernel, Amber Kao, Bling Chiang,
	Eric Su, Doreen Lin

Add a UCSI driver for the ITE IT8851 and IT8853 I2C-based USB Type-C
Power Delivery controllers.
The controller signals UCSI through a shared interrupt line; events are
read and acknowledged via vendor-specific registers, and
UCSI events are forwarded to the UCSI core.

Add the driver file to the MAINTAINERS entry.

Cc: Yaode Fang <Yaode.Fang@ite.com.tw>
Cc: Jeson Yang <jeson.yang@ite.com.tw>
Cc: Bling Chiang <Bling.Chiang@ite.com.tw>
Cc: Eric Su <Eric.Su@ite.com.tw>
Cc: Doreen Lin <doreen.lin@ite.com.tw>
Signed-off-by: Amber Kao <amber.kao@ite.com.tw>
---
 MAINTAINERS                         |   1 +
 drivers/usb/typec/ucsi/Kconfig      |  10 +
 drivers/usb/typec/ucsi/Makefile     |   1 +
 drivers/usb/typec/ucsi/ucsi_itepd.c | 365 ++++++++++++++++++++++++++++++++++++
 4 files changed, 377 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ec0eeea79b9e..1e76e299e6c3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13727,6 +13727,7 @@ R:	Yaode Fang <Yaode.Fang@ite.com.tw>
 L:	linux-usb@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/usb/ite,it8851.yaml
+F:	drivers/usb/typec/ucsi/ucsi_itepd.c
 
 IVTV VIDEO4LINUX DRIVER
 M:	Andy Walls <awalls@md.metrocast.net>
diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
index 87dd992a4b9e..0f10dac9b34b 100644
--- a/drivers/usb/typec/ucsi/Kconfig
+++ b/drivers/usb/typec/ucsi/Kconfig
@@ -104,4 +104,14 @@ config UCSI_HUAWEI_GAOKUN
 	  To compile the driver as a module, choose M here: the module will be
 	  called ucsi_huawei_gaokun.
 
+config UCSI_ITEPD
+	tristate "UCSI Interface Driver for ITE IT8851/IT8853"
+	depends on I2C
+	help
+	  This driver enables UCSI support on platforms that expose an
+	  ITE IT8851/IT8853 Type-C Power Delivery controller over an I2C interface.
+
+	  To compile the driver as a module, choose M here: the module
+	  will be called ucsi_itepd.
+
 endif
diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
index c7e38bf01350..0903a1fd486b 100644
--- a/drivers/usb/typec/ucsi/Makefile
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_UCSI_PMIC_GLINK)		+= ucsi_glink.o
 obj-$(CONFIG_CROS_EC_UCSI)		+= cros_ec_ucsi.o
 obj-$(CONFIG_UCSI_LENOVO_YOGA_C630)	+= ucsi_yoga_c630.o
 obj-$(CONFIG_UCSI_HUAWEI_GAOKUN)	+= ucsi_huawei_gaokun.o
+obj-$(CONFIG_UCSI_ITEPD)		+= ucsi_itepd.o
diff --git a/drivers/usb/typec/ucsi/ucsi_itepd.c b/drivers/usb/typec/ucsi/ucsi_itepd.c
new file mode 100644
index 000000000000..b95f5f9b9122
--- /dev/null
+++ b/drivers/usb/typec/ucsi/ucsi_itepd.c
@@ -0,0 +1,365 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025-2026, ITE. All Rights Reserved
+ *
+ */
+#include <linux/bits.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/unaligned.h>
+
+#include "ucsi.h"
+
+#define ITEPD_UCSI_VERSION_REG	0x80
+#define ITEPD_UCSI_CCI_REG	0x84
+#define ITEPD_UCSI_MSG_IN_REG	0x88
+#define ITEPD_UCSI_CONTROL_REG	0x98
+
+#define ITEPD_VENDOR_WC_INT	0xbc
+#define ITEPD_VENDOR_INT	0xbd
+#define ITEPD_ALERT_VDM_EVENT	BIT(0)
+#define ITEPD_ALERT_UCSI_EVENT	BIT(1)
+
+#define ITEPD_MSG_IN_MAX_LEN	0x28
+
+#define ITEPD_EVENT_NONE	0
+#define ITEPD_EVENT_UCSI	1
+#define ITEPD_EVENT_VDM		2
+
+struct itepd {
+	struct i2c_client *client;
+	struct ucsi *ucsi;
+	struct mutex i2c_lock;		/* Serializes I2C accesses */
+	struct mutex event_lock;	/* Serializes event processing (IRQ vs poll) */
+	struct mutex received_lock;	/* Protects cci and msg_in */
+	u8 msg_in[ITEPD_MSG_IN_MAX_LEN];
+	u32 cci;
+};
+
+static u8 ucsi_itepd_get_len(u32 cci)
+{
+	if (cci & UCSI_CCI_COMMAND_COMPLETE)
+		return UCSI_CCI_LENGTH(cci);
+	return 0;
+}
+
+static int itepd_read_reg(struct itepd *itepd, u8 reg, void *data, u32 len)
+{
+	struct i2c_client *client = itepd->client;
+	struct i2c_msg msg[2] = {
+		{
+			.addr	= client->addr,
+			.flags	= 0,
+			.len	= 1,
+			.buf	= &reg,
+		},
+		{
+			.addr	= client->addr,
+			.flags	= I2C_M_RD,
+			.len	= len,
+			.buf	= data,
+		}
+
+	};
+	int ret;
+
+	mutex_lock(&itepd->i2c_lock);
+	ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+	mutex_unlock(&itepd->i2c_lock);
+	if (ret < 0) {
+		dev_err_ratelimited(&client->dev, "reg 0x%02x read failed: %d\n",
+				    reg, ret);
+		return ret;
+	}
+
+	return ret == ARRAY_SIZE(msg) ? 0 : -EIO;
+}
+
+static int itepd_write_reg(struct itepd *itepd, u8 reg, const void *data, u32 len)
+{
+	struct i2c_client *client = itepd->client;
+	u8 buf[sizeof(u64) + 1];
+	struct i2c_msg msg[1] = {
+		{
+			.addr	= client->addr,
+			.flags	= 0,
+			.len	= len + 1,
+			.buf	= buf,
+		}
+	};
+	int ret;
+
+	if (len > sizeof(buf) - 1)
+		return -EINVAL;
+
+	buf[0] = reg;
+	memcpy(&buf[1], data, len);
+
+	mutex_lock(&itepd->i2c_lock);
+	ret = i2c_transfer(client->adapter, msg, 1);
+	mutex_unlock(&itepd->i2c_lock);
+	if (ret < 0) {
+		dev_err_ratelimited(&client->dev, "reg 0x%02x write failed: %d\n",
+				    reg, ret);
+		return ret;
+	}
+
+	return ret == 1 ? 0 : -EIO;
+}
+
+static int itepd_process_event(struct itepd *itepd, u32 *cci)
+{
+	u8 msg_in[ITEPD_MSG_IN_MAX_LEN] = {};
+	__le32 le_cci;
+	u8 event, ack;
+	u8 len = 0;
+	int err = 0;
+	int ret;
+
+	mutex_lock(&itepd->event_lock);
+
+	ret = itepd_read_reg(itepd, ITEPD_VENDOR_INT, &event, sizeof(event));
+	if (ret)
+		goto out_unlock;
+
+	event &= ITEPD_ALERT_VDM_EVENT | ITEPD_ALERT_UCSI_EVENT;
+	if (!event) {
+		mutex_lock(&itepd->received_lock);
+		*cci = itepd->cci;
+		mutex_unlock(&itepd->received_lock);
+		ret = ITEPD_EVENT_NONE;
+		goto out_unlock;
+	}
+
+	if (event & ITEPD_ALERT_UCSI_EVENT) {
+		err = itepd_read_reg(itepd, ITEPD_UCSI_CCI_REG, &le_cci,
+				     sizeof(le_cci));
+		if (!err) {
+			*cci = le32_to_cpu(le_cci);
+			len = min_t(u8, ucsi_itepd_get_len(*cci),
+				    sizeof(msg_in));
+			if (len)
+				err = itepd_read_reg(itepd,
+						     ITEPD_UCSI_MSG_IN_REG,
+						     msg_in, len);
+		}
+	}
+
+	ack = event;
+	ret = itepd_write_reg(itepd, ITEPD_VENDOR_WC_INT, &ack, sizeof(ack));
+	if (ret)
+		goto out_unlock;
+
+	if (err) {
+		ret = err;
+		goto out_unlock;
+	}
+
+	if (event & ITEPD_ALERT_UCSI_EVENT) {
+		mutex_lock(&itepd->received_lock);
+		itepd->cci = *cci;
+		memset(itepd->msg_in, 0, sizeof(itepd->msg_in));
+		memcpy(itepd->msg_in, msg_in, len);
+		mutex_unlock(&itepd->received_lock);
+		ret = ITEPD_EVENT_UCSI;
+	} else {
+		mutex_lock(&itepd->received_lock);
+		*cci = itepd->cci;
+		mutex_unlock(&itepd->received_lock);
+		ret = ITEPD_EVENT_VDM;
+	}
+
+out_unlock:
+	mutex_unlock(&itepd->event_lock);
+	return ret;
+}
+
+static int ucsi_itepd_read_version(struct ucsi *ucsi, u16 *version)
+{
+	struct itepd *itepd = ucsi_get_drvdata(ucsi);
+	__le16 le_version;
+	int ret;
+
+	ret = itepd_read_reg(itepd, ITEPD_UCSI_VERSION_REG, &le_version,
+			     sizeof(le_version));
+	if (ret)
+		return ret;
+
+	*version = le16_to_cpu(le_version);
+
+	return 0;
+}
+
+static int ucsi_itepd_read_cci(struct ucsi *ucsi, u32 *cci)
+{
+	struct itepd *itepd = ucsi_get_drvdata(ucsi);
+
+	mutex_lock(&itepd->received_lock);
+	*cci = itepd->cci;
+	mutex_unlock(&itepd->received_lock);
+
+	return 0;
+}
+
+static int ucsi_itepd_poll_cci(struct ucsi *ucsi, u32 *cci)
+{
+	struct itepd *itepd = ucsi_get_drvdata(ucsi);
+	int ret;
+
+	ret = itepd_process_event(itepd, cci);
+	return ret < 0 ? ret : 0;
+}
+
+static int ucsi_itepd_read_message_in(struct ucsi *ucsi, void *val, size_t val_len)
+{
+	struct itepd *itepd = ucsi_get_drvdata(ucsi);
+
+	mutex_lock(&itepd->received_lock);
+	memcpy(val, itepd->msg_in, min(val_len, sizeof(itepd->msg_in)));
+	mutex_unlock(&itepd->received_lock);
+
+	return 0;
+}
+
+static int ucsi_itepd_async_control(struct ucsi *ucsi, u64 command)
+{
+	struct itepd *itepd = ucsi_get_drvdata(ucsi);
+	__le64 le_cmd = cpu_to_le64(command);
+
+	if (UCSI_COMMAND(command) == UCSI_PPM_RESET) {
+		mutex_lock(&itepd->received_lock);
+		itepd->cci = 0;
+		mutex_unlock(&itepd->received_lock);
+	}
+
+	return itepd_write_reg(itepd, ITEPD_UCSI_CONTROL_REG, &le_cmd,
+			       sizeof(le_cmd));
+}
+
+static const struct ucsi_operations ucsi_itepd_ops = {
+	.read_version		= ucsi_itepd_read_version,
+	.read_cci		= ucsi_itepd_read_cci,
+	.poll_cci		= ucsi_itepd_poll_cci,
+	.read_message_in	= ucsi_itepd_read_message_in,
+	.sync_control		= ucsi_sync_control_common,
+	.async_control		= ucsi_itepd_async_control,
+};
+
+static irqreturn_t itepd_irq_process(struct itepd *itepd)
+{
+	u32 cci;
+	int ret;
+
+	ret = itepd_process_event(itepd, &cci);
+	if (ret == ITEPD_EVENT_NONE)
+		return IRQ_NONE;
+
+	if (ret < 0)
+		return IRQ_HANDLED;
+
+	if (ret == ITEPD_EVENT_UCSI)
+		ucsi_notify_common(itepd->ucsi, cci);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t itepd_irq_thread_fn(int irq, void *data)
+{
+	struct itepd *itepd = data;
+
+	return itepd_irq_process(itepd);
+}
+
+static int itepd_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct itepd *itepd;
+	u32 cci;
+	int ret;
+
+	if (client->irq <= 0)
+		return dev_err_probe(dev, -ENODEV, "no IRQ provided\n");
+
+	itepd = devm_kzalloc(dev, sizeof(*itepd), GFP_KERNEL);
+	if (!itepd)
+		return -ENOMEM;
+
+	itepd->client = client;
+	mutex_init(&itepd->i2c_lock);
+	mutex_init(&itepd->event_lock);
+	mutex_init(&itepd->received_lock);
+	i2c_set_clientdata(client, itepd);
+
+	itepd->ucsi = ucsi_create(dev, &ucsi_itepd_ops);
+	if (IS_ERR(itepd->ucsi))
+		return dev_err_probe(dev, PTR_ERR(itepd->ucsi),
+				     "ucsi_create failed\n");
+
+	ucsi_set_drvdata(itepd->ucsi, itepd);
+
+	ret = request_threaded_irq(client->irq, NULL, itepd_irq_thread_fn,
+				   IRQF_ONESHOT, dev_name(dev), itepd);
+	if (ret) {
+		dev_err_probe(dev, ret, "request_threaded_irq failed\n");
+		goto out_ucsi_destroy;
+	}
+
+	ret = itepd_process_event(itepd, &cci);
+	if (ret < 0)
+		dev_warn(dev, "initial event processing failed: %d\n", ret);
+
+	ret = ucsi_register(itepd->ucsi);
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to register UCSI\n");
+		goto out_free_irq;
+	}
+
+	return 0;
+
+out_free_irq:
+	free_irq(client->irq, itepd);
+out_ucsi_destroy:
+	ucsi_destroy(itepd->ucsi);
+	return ret;
+}
+
+static void itepd_remove(struct i2c_client *client)
+{
+	struct itepd *itepd = i2c_get_clientdata(client);
+
+	ucsi_unregister(itepd->ucsi);
+	free_irq(client->irq, itepd);
+	ucsi_destroy(itepd->ucsi);
+}
+
+static const struct of_device_id itepd_of_match_table[] = {
+	{ .compatible = "ite,it8851" },
+	{ .compatible = "ite,it8853" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, itepd_of_match_table);
+
+static const struct i2c_device_id itepd_id_table[] = {
+	{ "ucsi_itepd", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, itepd_id_table);
+
+static struct i2c_driver itepd_driver = {
+	.driver = {
+		.name = "ucsi_itepd",
+		.of_match_table = itepd_of_match_table,
+	},
+	.probe = itepd_probe,
+	.remove = itepd_remove,
+	.id_table = itepd_id_table,
+};
+module_i2c_driver(itepd_driver);
+
+MODULE_AUTHOR("Jeson Yang <jeson.yang@ite.com.tw>");
+MODULE_DESCRIPTION("UCSI driver for ITE IT8851 and IT8853 Type-C PD controllers");
+MODULE_LICENSE("GPL");

-- 
2.53.0


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

* Re: [PATCH v5 2/2] usb: typec: ucsi: Add ITE IT8851/IT8853 Type-C PD controller driver
  2026-07-24 10:09 ` [PATCH v5 2/2] usb: typec: ucsi: Add ITE IT8851/IT8853 Type-C PD controller driver Amber Kao
@ 2026-07-24 10:23   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-07-24 10:23 UTC (permalink / raw)
  To: Amber Kao; +Cc: robh, devicetree, conor+dt

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Stack-allocated buffers are passed directly to `i2c_transfer`, violating DMA constraints.
- [High] Race condition during driver removal leads to a use-after-free of `ucsi->connector` due to improper teardown ordering.
- [High] Unhandled hardware interrupts or I2C read failures return IRQ_HANDLED, causing an interrupt storm.
--

--- Patch [2]: [PATCH v5 2/2] usb: typec: ucsi: Add ITE IT8851/IT8853 Type-C PD controller driver ---
commit e9bebde0ac95c5792e010b2c998820bde558bc90
Author: Amber Kao <amber.kao@ite.com.tw>

usb: typec: ucsi: Add ITE IT8851/IT8853 Type-C PD controller driver

This commit adds a UCSI driver for the ITE IT8851 and IT8853 I2C-based
USB Type-C Power Delivery controllers. The driver communicates with the
controller over an I2C interface and forwards UCSI events to the core.

> diff --git a/drivers/usb/typec/ucsi/ucsi_itepd.c b/drivers/usb/typec/ucsi/ucsi_itepd.c
> new file mode 100644
> index 0000000000000..b95f5f9b91228
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_itepd.c
[ ... ]
> +static int itepd_read_reg(struct itepd *itepd, u8 reg, void *data, u32 len)
> +{
> +	struct i2c_client *client = itepd->client;
> +	struct i2c_msg msg[2] = {
> +		{
> +			.addr	= client->addr,
> +			.flags	= 0,
> +			.len	= 1,
> +			.buf	= &reg,
> +		},

[Severity: High]
Does this code pass a stack-allocated buffer to the I2C subsystem?

The reg pointer and the data argument (which points to local stack variables
like event and le_cci in caller functions) are passed directly into the
i2c_msg array. 

When i2c_transfer() passes these structures to the underlying I2C host
controller driver, it may map them for DMA. Since the stack is often
virtually mapped in the kernel (CONFIG_VMAP_STACK=y), passing stack memory
to the DMA API causes mapping failures. 

Depending on the platform, this can trigger kernel panics, DMA API debug
warnings, or silent memory corruption.

> +		{
> +			.addr	= client->addr,
> +			.flags	= I2C_M_RD,
> +			.len	= len,
> +			.buf	= data,
> +		}
> +
> +	};
> +	int ret;
[ ... ]
> +static int itepd_write_reg(struct itepd *itepd, u8 reg, const void *data, u32 len)
> +{
> +	struct i2c_client *client = itepd->client;
> +	u8 buf[sizeof(u64) + 1];
> +	struct i2c_msg msg[1] = {
> +		{
> +			.addr	= client->addr,
> +			.flags	= 0,
> +			.len	= len + 1,
> +			.buf	= buf,
> +		}

[Severity: High]
Similarly, buf is a local stack array used as the write buffer for
i2c_transfer(). 

Is it possible to allocate these buffers dynamically or use a pre-allocated
DMA-safe buffer in the itepd structure to avoid DMA mapping issues on stack
memory?

[ ... ]
> +static irqreturn_t itepd_irq_process(struct itepd *itepd)
> +{
> +	u32 cci;
> +	int ret;
> +
> +	ret = itepd_process_event(itepd, &cci);
> +	if (ret == ITEPD_EVENT_NONE)
> +		return IRQ_NONE;
> +
> +	if (ret < 0)
> +		return IRQ_HANDLED;

[Severity: High]
Could this return value cause an interrupt storm if I2C communication fails?

If itepd_process_event() fails due to an I2C error, it propagates a negative
error code. At that point, the hardware interrupt source has not been
acknowledged or cleared via a write to the ITEPD_VENDOR_WC_INT register.

By returning IRQ_HANDLED, the handler falsely signals to the IRQ subsystem
that the interrupt was successfully processed. For a level-triggered interrupt,
this prevents the kernel's spurious interrupt detector from disabling the
unhandled IRQ, leading to an infinite interrupt loop that can starve the CPU.

Should this return IRQ_NONE on failure instead to allow the spurious IRQ
detector to properly mask it?

> +
> +	if (ret == ITEPD_EVENT_UCSI)
> +		ucsi_notify_common(itepd->ucsi, cci);
> +
> +	return IRQ_HANDLED;
> +}
[ ... ]
> +static void itepd_remove(struct i2c_client *client)
> +{
> +	struct itepd *itepd = i2c_get_clientdata(client);
> +
> +	ucsi_unregister(itepd->ucsi);
> +	free_irq(client->irq, itepd);

[Severity: High]
Is there a race condition between tearing down the UCSI subsystem and freeing
the interrupt?

ucsi_unregister() sends an asynchronous command to disable notifications and
frees the ucsi->connector array. Since free_irq() has not yet been called to
synchronize and disable the IRQ thread, itepd_irq_thread_fn() can run
concurrently.

If the interrupt fires after ucsi->connector is freed, the IRQ handler will
call ucsi_notify_common(), which reads the freed ucsi->connector array. 

Should free_irq() be called before ucsi_unregister() to guarantee that no
interrupts are processed while the subsystem dismantles its structures?

> +	ucsi_destroy(itepd->ucsi);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-ucsi-itepd-v5-0-d1a83e5c9c77@ite.com.tw?part=2

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

end of thread, other threads:[~2026-07-24 10:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-24 10:09 [PATCH v5 0/2] usb: typec: ucsi: Add ITE IT8851/IT8853 support Amber Kao
2026-07-24 10:09 ` [PATCH v5 1/2] dt-bindings: usb: Add ITE IT8851/IT8853 Type-C PD controllers Amber Kao
2026-07-24 10:09 ` [PATCH v5 2/2] usb: typec: ucsi: Add ITE IT8851/IT8853 Type-C PD controller driver Amber Kao
2026-07-24 10:23   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox