[PATCH v7 0/3] usb: typec: tipd: Add sn201202x (ACE3) support

[PATCH v7 0/3] usb: typec: tipd: Add sn201202x (ACE3) support

[Sasha Finkelstein]

Add support for sn201202x (a.k.a ACE3), an Apple-specific tps6598x
variant that wraps the tps6598x core register set with a SPMI based
regmap-like protocol, first appearing in Apple M3 devices. This series
first makes the core parts bus-agnostic, and then adds the new SPMI
interface as a custom regmap implementation.

To simplify the merge strategy, the device tree entries will be sent
in a future patch series.

Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
---
Changes in v7:
- Rebase on usb-testing
- Link to v6: https://patch.msgid.link/20260826-tipd-ace3-v6-0-e919f39a93a2@chaosmail.tech

Changes in v6:
- Fix rebase issues
- Link to v5: https://patch.msgid.link/20260821-tipd-ace3-v5-0-3244ed3bdc3f@chaosmail.tech

Changes in v5:
- Rebase on usb-next
- Link to v4: https://patch.msgid.link/20260806-tipd-ace3-v4-0-79855b150914@chaosmail.tech

Changes in v4:
- Unexport variant data
- One more debugfs-safety fix
- Link to v3: https://patch.msgid.link/20260801-tipd-ace3-v3-0-8332f2eca4fc@chaosmail.tech

Changes in v3:
- Restore accidentally deleted static
- Fix building with tracepoints
- Sleep the device on failed probe
- Link to v2: https://patch.msgid.link/20260727-tipd-ace3-v2-0-1345957bd635@chaosmail.tech

Changes in v2:
- Use a -core module instead of building everything twice
- Fix completion handling
- Make regmaps debugfs-safe
- Link to v1: https://patch.msgid.link/20260725-tipd-ace3-v1-0-73202d36138a@chaosmail.tech

---
Alyssa Milburn (1):
      usb: typec: tipd: Factor out i2c specifics

Sasha Finkelstein (2):
      dt-bindings: usb: tps6598x: Add sn201202x/ACE3
      usb: typec: tipd: Add sn201202x support

 Documentation/devicetree/bindings/usb/apple,sn201202x.yaml |  78 +++++++++++++++++++++++++++++++
 MAINTAINERS                                                |   2 +
 drivers/usb/typec/Makefile                                 |   1 +
 drivers/usb/typec/tipd/Kconfig                             |  20 +++++++-
 drivers/usb/typec/tipd/Makefile                            |  14 ++++--
 drivers/usb/typec/tipd/core.c                              | 247 +++++++++++++++++++++++--------------------------------------------------------------------------
 drivers/usb/typec/tipd/i2c.c                               | 107 ++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/typec/tipd/spmi.c                              | 300 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/typec/tipd/tps6598x.h                          | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 705 insertions(+), 196 deletions(-)
---
base-commit: bbed88f72dbbb960080282de3b90ca0933fa6226
change-id: 20260725-tipd-ace3-090bf570ef59

Best regards,
--  
Sasha Finkelstein <k@chaosmail.tech>

[PATCH v7 1/3] dt-bindings: usb: tps6598x: Add sn201202x/ACE3

[Sasha Finkelstein]

A variant of tps6598x that is attached to a SPMI bus and is found on
Apple devices starting from the M3 series

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
---
 Documentation/devicetree/bindings/usb/apple,sn201202x.yaml | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 MAINTAINERS                                                |  1 +
 2 files changed, 79 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/apple,sn201202x.yaml b/Documentation/devicetree/bindings/usb/apple,sn201202x.yaml
new file mode 100644
index 000000000000..133a71bbedb7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/apple,sn201202x.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/apple,sn201202x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments sn201202x Type-C Power Delivery controller
+
+maintainers:
+  - Sasha Finkelstein <k@chaosmail.tech>
+
+description: |
+  Texas Instruments sn201202x Type-C Power Delivery controller
+
+  A variant of tps6598x controller also known as Apple ACE3
+  present on hardware with Apple SoCs starting from the M3.
+
+properties:
+  compatible:
+    enum:
+      - apple,sn201202x
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: Primary irq used for tps6598x events
+      - description: Logical register selection completed
+      - description: Standby command completed
+      - description: Wakeup command completed
+
+  interrupt-names:
+    items:
+      - const: irq
+      - const: select
+      - const: sleep
+      - const: wake
+
+  connector:
+    $ref: /schemas/connector/usb-connector.yaml#
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/spmi/spmi.h>
+    spmi {
+        #address-cells = <2>;
+        #size-cells = <0>;
+        usb-pd@c {
+            compatible = "apple,sn201202x";
+            reg = <0xc SPMI_USID>;
+            interrupts = <11 IRQ_TYPE_EDGE_RISING>,
+                         <13 IRQ_TYPE_EDGE_RISING>,
+                         <17 IRQ_TYPE_EDGE_RISING>,
+                         <19 IRQ_TYPE_EDGE_RISING>;
+            interrupt-names = "irq", "select", "sleep", "wake";
+
+            connector {
+                compatible = "usb-c-connector";
+                label = "USB-C";
+
+                port {
+                    typec_ep: endpoint {
+                        remote-endpoint = <&otg_ep>;
+                    };
+                };
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 6215fcb07770..ba4274ed9c79 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2659,6 +2659,7 @@ F:	Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml
 F:	Documentation/devicetree/bindings/spi/apple,spi.yaml
 F:	Documentation/devicetree/bindings/spmi/apple,spmi.yaml
 F:	Documentation/devicetree/bindings/usb/apple,dwc3.yaml
+F:	Documentation/devicetree/bindings/usb/apple,sn201202x.yaml
 F:	Documentation/devicetree/bindings/watchdog/apple,wdt.yaml
 F:	Documentation/hwmon/macsmc-hwmon.rst
 F:	arch/arm64/boot/dts/apple/

-- 
2.55.0

[PATCH v7 2/3] usb: typec: tipd: Factor out i2c specifics

[Sasha Finkelstein]

From: Alyssa Milburn <amilburn@zall.org>

Make the core driver more bus-agnostic to prepare for SPMI variants of
the tipd chip

Signed-off-by: Alyssa Milburn <amilburn@zall.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
---
 drivers/usb/typec/tipd/Kconfig    |   8 +++--
 drivers/usb/typec/tipd/Makefile   |  11 ++++---
 drivers/usb/typec/tipd/core.c     | 230 ++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------------------------
 drivers/usb/typec/tipd/i2c.c      | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/typec/tipd/tps6598x.h | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 281 insertions(+), 198 deletions(-)

diff --git a/drivers/usb/typec/tipd/Kconfig b/drivers/usb/typec/tipd/Kconfig
index b82715293072..c87cbe6109ca 100644
--- a/drivers/usb/typec/tipd/Kconfig
+++ b/drivers/usb/typec/tipd/Kconfig
@@ -1,9 +1,13 @@
+config TYPEC_TPS6598X_CORE
+	tristate
+	select USB_ROLE_SWITCH
+	select POWER_SUPPLY
+
 config TYPEC_TPS6598X
 	tristate "TI TPS6598x USB Power Delivery controller driver"
 	depends on I2C
-	select POWER_SUPPLY
 	select REGMAP_I2C
-	select USB_ROLE_SWITCH
+	select TYPEC_TPS6598X_CORE
 	help
 	  Say Y or M here if your system has TI TPS65982 or TPS65983 USB Power
 	  Delivery controller.
diff --git a/drivers/usb/typec/tipd/Makefile b/drivers/usb/typec/tipd/Makefile
index aa439f80a889..c741bf44f52f 100644
--- a/drivers/usb/typec/tipd/Makefile
+++ b/drivers/usb/typec/tipd/Makefile
@@ -1,6 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
-CFLAGS_trace.o			:= -I$(src)
+CFLAGS_trace.o				:= -I$(src)
 
-obj-$(CONFIG_TYPEC_TPS6598X)	+= tps6598x.o
-tps6598x-y			:= core.o
-tps6598x-$(CONFIG_TRACING)	+= trace.o
+obj-$(CONFIG_TYPEC_TPS6598X_CORE)	+= tps6598x-core.o
+tps6598x-core-y				:= core.o
+tps6598x-core-$(CONFIG_TRACING)		+= trace.o
+
+obj-$(CONFIG_TYPEC_TPS6598X)		+= tps6598x.o
+tps6598x-y				:= i2c.o
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 9248f0dd3f49..bdcc6f75d93f 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -9,11 +9,9 @@
 #include <linux/i2c.h>
 #include <linux/acpi.h>
 #include <linux/gpio/consumer.h>
-#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/power_supply.h>
 #include <linux/regmap.h>
-#include <linux/interrupt.h>
 #include <linux/usb/typec.h>
 #include <linux/usb/typec_altmode.h>
 #include <linux/usb/typec_dp.h>
@@ -92,30 +90,6 @@ struct tps6598x_rx_identity_reg {
 	struct usb_pd_identity identity;
 } __packed;
 
-/* TPS_REG_USB4_STATUS */
-struct tps6598x_usb4_status_reg {
-	u8 mode_status;
-	__le32 eudo;
-	__le32 unknown;
-} __packed;
-
-/* TPS_REG_DP_SID_STATUS */
-struct tps6598x_dp_sid_status_reg {
-	u8 mode_status;
-	__le32 status_tx;
-	__le32 status_rx;
-	__le32 configure;
-	__le32 mode_data;
-} __packed;
-
-/* TPS_REG_INTEL_VID_STATUS */
-struct tps6598x_intel_vid_status_reg {
-	u8 mode_status;
-	__le32 attention_vdo;
-	__le16 enter_vdo;
-	__le16 device_mode;
-} __packed;
-
 /* Standard Task return codes */
 #define TPS_TASK_TIMEOUT		1
 #define TPS_TASK_REJECTED		3
@@ -144,83 +118,6 @@ static const char *const modes[] = {
 /* Unrecognized commands will be replaced with "!CMD" */
 #define INVALID_CMD(_cmd_)		(_cmd_ == 0x444d4321)
 
-struct tps6598x;
-
-struct tipd_data {
-	irq_handler_t irq_handler;
-	u64 irq_mask1;
-	size_t tps_struct_size;
-	bool no_mode_control;
-	void (*remove)(struct tps6598x *tps);
-	int (*register_port)(struct tps6598x *tps, struct fwnode_handle *node);
-	void (*unregister_port)(struct tps6598x *tps);
-	void (*trace_data_status)(u32 status);
-	void (*trace_power_status)(u16 status);
-	void (*trace_status)(u32 status);
-	int (*apply_patch)(struct tps6598x *tps);
-	int (*init)(struct tps6598x *tps);
-	int (*switch_power_state)(struct tps6598x *tps, u8 target_state);
-	bool (*read_data_status)(struct tps6598x *tps);
-	bool (*read_power_status)(struct tps6598x *tps);
-	int (*reset)(struct tps6598x *tps);
-	int (*connect)(struct tps6598x *tps, u32 status);
-};
-
-struct tps6598x {
-	struct device *dev;
-	struct regmap *regmap;
-	struct mutex lock; /* device lock */
-	u8 i2c_protocol:1;
-
-	struct gpio_desc *reset;
-	struct typec_port *port;
-	struct typec_partner *partner;
-	struct usb_pd_identity partner_identity;
-	struct usb_role_switch *role_sw;
-	struct typec_capability typec_cap;
-
-	struct power_supply *psy;
-	struct power_supply_desc psy_desc;
-	enum power_supply_usb_type usb_type;
-
-	int wakeup;
-	u32 status; /* status reg */
-	u32 data_status;
-	u16 pwr_status;
-	struct delayed_work	wq_poll;
-
-	const struct tipd_data *data;
-};
-
-struct cd321x_status {
-	u32 status;
-	u32 pwr_status;
-	u32 data_status;
-	u32 status_changed;
-	struct usb_pd_identity partner_identity;
-	struct tps6598x_dp_sid_status_reg dp_sid_status;
-	struct tps6598x_intel_vid_status_reg intel_vid_status;
-	struct tps6598x_usb4_status_reg usb4_status;
-};
-
-struct cd321x {
-	struct tps6598x tps;
-
-	struct tps6598x_dp_sid_status_reg dp_sid_status;
-	struct tps6598x_intel_vid_status_reg intel_vid_status;
-	struct tps6598x_usb4_status_reg usb4_status;
-
-	struct typec_altmode *port_altmode_dp;
-	struct typec_altmode *port_altmode_tbt;
-
-	struct typec_mux *mux;
-	struct typec_mux_state state;
-
-	struct cd321x_status update_status;
-	struct delayed_work update_work;
-	struct usb_pd_identity cur_partner_identity;
-};
-
 static enum power_supply_property tps6598x_psy_props[] = {
 	POWER_SUPPLY_PROP_USB_TYPE,
 	POWER_SUPPLY_PROP_ONLINE,
@@ -1137,11 +1034,12 @@ static int tps6598x_check_mode(struct tps6598x *tps)
 	return -ENODEV;
 }
 
-static const struct regmap_config tps6598x_regmap_config = {
+const struct regmap_config tps6598x_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 	.max_register = 0x7F,
 };
+EXPORT_SYMBOL_GPL(tps6598x_regmap_config);
 
 static int tps6598x_psy_get_online(struct tps6598x *tps,
 				   union power_supply_propval *val)
@@ -1789,27 +1687,13 @@ static void cd321x_remove(struct tps6598x *tps)
 	cancel_delayed_work_sync(&cd321x->update_work);
 }
 
-static int tps6598x_probe(struct i2c_client *client)
+int tipd_init(struct tps6598x *tps)
 {
-	const struct tipd_data *data;
-	struct tps6598x *tps;
 	struct fwnode_handle *fwnode;
 	u32 status;
 	u32 vid = 0;
 	int ret;
 
-	data = i2c_get_match_data(client);
-	if (!data)
-		return -EINVAL;
-
-	tps = devm_kzalloc(&client->dev, data->tps_struct_size, GFP_KERNEL);
-	if (!tps)
-		return -ENOMEM;
-
-	mutex_init(&tps->lock);
-	tps->dev = &client->dev;
-	tps->data = data;
-
 	tps->reset = devm_gpiod_get_optional(tps->dev, "reset", GPIOD_OUT_LOW);
 	if (IS_ERR(tps->reset))
 		return dev_err_probe(tps->dev, PTR_ERR(tps->reset),
@@ -1817,10 +1701,6 @@ static int tps6598x_probe(struct i2c_client *client)
 	if (tps->reset)
 		msleep(TPS_SETUP_MS);
 
-	tps->regmap = devm_regmap_init_i2c(client, &tps6598x_regmap_config);
-	if (IS_ERR(tps->regmap))
-		return PTR_ERR(tps->regmap);
-
 	if (!device_is_compatible(tps->dev, "ti,tps25750")) {
 		ret = tps6598x_read32(tps, TPS_REG_VID, &vid);
 		if (ret < 0 || !vid) {
@@ -1830,13 +1710,6 @@ static int tps6598x_probe(struct i2c_client *client)
 		}
 	}
 
-	/*
-	 * Checking can the adapter handle SMBus protocol. If it can not, the
-	 * driver needs to take care of block reads separately.
-	 */
-	if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
-		tps->i2c_protocol = true;
-
 	if (tps->data->switch_power_state) {
 		ret = tps->data->switch_power_state(tps, TPS_SYSTEM_POWER_STATE_S0);
 		if (ret)
@@ -1872,7 +1745,7 @@ static int tps6598x_probe(struct i2c_client *client)
 	 * with existing DT files, we work around this by deleting any
 	 * fwnode_links to/from this fwnode.
 	 */
-	fwnode = device_get_named_child_node(&client->dev, "connector");
+	fwnode = device_get_named_child_node(tps->dev, "connector");
 	if (fwnode)
 		fw_devlink_purge_absent_suppliers(fwnode);
 
@@ -1898,14 +1771,14 @@ static int tps6598x_probe(struct i2c_client *client)
 			goto err_unregister_port;
 		ret = tps->data->connect(tps, status);
 		if (ret)
-			dev_err(&client->dev, "failed to register partner\n");
+			dev_err(tps->dev, "failed to register partner\n");
 	}
 
-	if (client->irq) {
-		ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+	if (tps->irq) {
+		ret = devm_request_threaded_irq(tps->dev, tps->irq, NULL,
 						tps->data->irq_handler,
 						IRQF_SHARED | IRQF_ONESHOT,
-						dev_name(&client->dev), tps);
+						dev_name(tps->dev), tps);
 	} else {
 		dev_dbg(tps->dev, "no IRQ specified, using polling mode\n");
 		INIT_DELAYED_WORK(&tps->wq_poll, tps6598x_poll_work);
@@ -1916,13 +1789,12 @@ static int tps6598x_probe(struct i2c_client *client)
 	if (ret)
 		goto err_disconnect;
 
-	i2c_set_clientdata(client, tps);
 	fwnode_handle_put(fwnode);
 
 	tps->wakeup = device_property_read_bool(tps->dev, "wakeup-source");
-	if (tps->wakeup && client->irq) {
-		devm_device_init_wakeup(&client->dev);
-		enable_irq_wake(client->irq);
+	if (tps->wakeup && tps->irq) {
+		devm_device_init_wakeup(tps->dev);
+		enable_irq_wake(tps->irq);
 	}
 
 	return 0;
@@ -1943,15 +1815,14 @@ static int tps6598x_probe(struct i2c_client *client)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(tipd_init);
 
-static void tps6598x_remove(struct i2c_client *client)
+void tipd_remove(struct tps6598x *tps)
 {
-	struct tps6598x *tps = i2c_get_clientdata(client);
-
-	if (!client->irq)
+	if (!tps->irq)
 		cancel_delayed_work_sync(&tps->wq_poll);
 	else
-		devm_free_irq(tps->dev, client->irq, tps);
+		devm_free_irq(tps->dev, tps->irq, tps);
 
 	if (tps->data->remove)
 		tps->data->remove(tps);
@@ -1966,29 +1837,26 @@ static void tps6598x_remove(struct i2c_client *client)
 	if (tps->reset)
 		gpiod_set_value_cansleep(tps->reset, 1);
 }
+EXPORT_SYMBOL_GPL(tipd_remove);
 
-static int __maybe_unused tps6598x_suspend(struct device *dev)
+int tipd_suspend(struct tps6598x *tps)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct tps6598x *tps = i2c_get_clientdata(client);
-
 	if (tps->wakeup) {
-		disable_irq(client->irq);
-		enable_irq_wake(client->irq);
+		disable_irq(tps->irq);
+		enable_irq_wake(tps->irq);
 	} else if (tps->reset) {
 		gpiod_set_value_cansleep(tps->reset, 1);
 	}
 
-	if (!client->irq)
+	if (!tps->irq)
 		cancel_delayed_work_sync(&tps->wq_poll);
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(tipd_suspend);
 
-static int __maybe_unused tps6598x_resume(struct device *dev)
+int tipd_resume(struct tps6598x *tps)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct tps6598x *tps = i2c_get_clientdata(client);
 	int ret;
 
 	ret = tps6598x_check_mode(tps);
@@ -2002,25 +1870,22 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
 	}
 
 	if (tps->wakeup) {
-		disable_irq_wake(client->irq);
-		enable_irq(client->irq);
+		disable_irq_wake(tps->irq);
+		enable_irq(tps->irq);
 	} else if (tps->reset) {
 		gpiod_set_value_cansleep(tps->reset, 0);
 		msleep(TPS_SETUP_MS);
 	}
 
-	if (!client->irq)
+	if (!tps->irq)
 		queue_delayed_work(system_power_efficient_wq, &tps->wq_poll,
 				   msecs_to_jiffies(POLL_INTERVAL));
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(tipd_resume);
 
-static const struct dev_pm_ops tps6598x_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(tps6598x_suspend, tps6598x_resume)
-};
-
-static const struct tipd_data cd321x_data = {
+static const struct tipd_data tipd_cd321x_data = {
 	.irq_handler = cd321x_interrupt,
 	.irq_mask1 = APPLE_CD_REG_INT_POWER_STATUS_UPDATE |
 		     APPLE_CD_REG_INT_DATA_STATUS_UPDATE |
@@ -2041,7 +1906,7 @@ static const struct tipd_data cd321x_data = {
 	.connect = cd321x_connect,
 };
 
-static const struct tipd_data tps6598x_data = {
+static const struct tipd_data tipd_tps6598x_data = {
 	.irq_handler = tps6598x_interrupt,
 	.irq_mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
 		     TPS_REG_INT_DATA_STATUS_UPDATE |
@@ -2060,7 +1925,7 @@ static const struct tipd_data tps6598x_data = {
 	.connect = tps6598x_connect,
 };
 
-static const struct tipd_data tps66993_data = {
+static const struct tipd_data tipd_tps66993_data = {
 	.irq_handler = tps6598x_interrupt,
 	.irq_mask1 = TPS_REG_INT_DATA_STATUS_UPDATE |
 		     TPS_REG_INT_PLUG_EVENT,
@@ -2078,7 +1943,7 @@ static const struct tipd_data tps66993_data = {
 	.connect = tps6598x_connect,
 };
 
-static const struct tipd_data tps25750_data = {
+static const struct tipd_data tipd_tps25750_data = {
 	.irq_handler = tps25750_interrupt,
 	.irq_mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
 		     TPS_REG_INT_DATA_STATUS_UPDATE |
@@ -2097,33 +1962,14 @@ static const struct tipd_data tps25750_data = {
 	.connect = tps6598x_connect,
 };
 
-static const struct of_device_id tps6598x_of_match[] = {
-	{ .compatible = "ti,tps6598x", &tps6598x_data},
-	{ .compatible = "ti,tps66993", &tps66993_data},
-	{ .compatible = "apple,cd321x", &cd321x_data},
-	{ .compatible = "ti,tps25750", &tps25750_data},
-	{}
-};
-MODULE_DEVICE_TABLE(of, tps6598x_of_match);
-
-static const struct i2c_device_id tps6598x_id[] = {
-	{ .name = "tps6598x", .driver_data = (kernel_ulong_t)&tps6598x_data },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, tps6598x_id);
-
-static struct i2c_driver tps6598x_i2c_driver = {
-	.driver = {
-		.name = "tps6598x",
-		.pm = &tps6598x_pm_ops,
-		.of_match_table = tps6598x_of_match,
-	},
-	.probe = tps6598x_probe,
-	.remove = tps6598x_remove,
-	.id_table = tps6598x_id,
+const struct tipd_data *tipd_hw_variants[] = {
+	[TIPD_VARIANT_CD321X] = &tipd_cd321x_data,
+	[TIPD_VARIANT_TPS6598X] = &tipd_tps6598x_data,
+	[TIPD_VARIANT_TPS66993] = &tipd_tps66993_data,
+	[TIPD_VARIANT_TPS25750] = &tipd_tps25750_data,
 };
-module_i2c_driver(tps6598x_i2c_driver);
+EXPORT_SYMBOL_GPL(tipd_hw_variants);
 
 MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
-MODULE_LICENSE("GPL v2");
-MODULE_DESCRIPTION("TI TPS6598x USB Power Delivery Controller Driver");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("TI TPS6598x USB Power Delivery Controller Core Functions");
diff --git a/drivers/usb/typec/tipd/i2c.c b/drivers/usb/typec/tipd/i2c.c
new file mode 100644
index 000000000000..a51fc6c39771
--- /dev/null
+++ b/drivers/usb/typec/tipd/i2c.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for TI TPS6598x USB Power Delivery controller family
+ *
+ * Copyright (C) The Asahi Linux Contributors
+ */
+
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "tps6598x.h"
+
+static int tps6598x_probe_i2c(struct i2c_client *client)
+{
+	enum tipd_hw_variant variant;
+	const struct tipd_data *data;
+	struct tps6598x *tps;
+	int ret;
+
+	variant = (enum tipd_hw_variant)(uintptr_t)i2c_get_match_data(client);
+	if (!variant || variant >= TIPD_VARIANT_MAX)
+		return -EINVAL;
+	data = tipd_hw_variants[variant];
+
+	tps = devm_kzalloc(&client->dev, data->tps_struct_size, GFP_KERNEL);
+	if (!tps)
+		return -ENOMEM;
+
+	mutex_init(&tps->lock);
+	tps->dev = &client->dev;
+	tps->data = data;
+	tps->irq = client->irq;
+
+	tps->regmap = devm_regmap_init_i2c(client, &tps6598x_regmap_config);
+	if (IS_ERR(tps->regmap))
+		return PTR_ERR(tps->regmap);
+
+	/*
+	 * Checking can the adapter handle SMBus protocol. If it can not, the
+	 * driver needs to take care of block reads separately.
+	 */
+	if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		tps->i2c_protocol = true;
+
+	ret = tipd_init(tps);
+
+	if (ret == 0)
+		i2c_set_clientdata(client, tps);
+
+	return ret;
+}
+
+static void tps6598x_remove_i2c(struct i2c_client *client)
+{
+	struct tps6598x *tps = i2c_get_clientdata(client);
+
+	tipd_remove(tps);
+}
+
+static int __maybe_unused tps6598x_suspend(struct device *dev)
+{
+	struct tps6598x *tps = dev_get_drvdata(dev);
+
+	return tipd_suspend(tps);
+}
+
+static int __maybe_unused tps6598x_resume(struct device *dev)
+{
+	struct tps6598x *tps = dev_get_drvdata(dev);
+
+	return tipd_resume(tps);
+}
+
+static const struct dev_pm_ops tps6598x_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(tps6598x_suspend, tps6598x_resume)
+};
+
+static const struct of_device_id tps6598x_of_match[] = {
+	{ .compatible = "ti,tps6598x", (void *)TIPD_VARIANT_TPS6598X},
+	{ .compatible = "ti,tps66993", (void *)TIPD_VARIANT_TPS66993},
+	{ .compatible = "apple,cd321x",  (void *)TIPD_VARIANT_CD321X},
+	{ .compatible = "ti,tps25750",  (void *)TIPD_VARIANT_TPS25750},
+	{}
+};
+MODULE_DEVICE_TABLE(of, tps6598x_of_match);
+
+static const struct i2c_device_id tps6598x_id[] = {
+	{ .name = "tps6598x", .driver_data = (kernel_ulong_t)TIPD_VARIANT_TPS6598X },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, tps6598x_id);
+
+static struct i2c_driver tps6598x_i2c_driver = {
+	.driver = {
+		.name = "tps6598x",
+		.pm = &tps6598x_pm_ops,
+		.of_match_table = tps6598x_of_match,
+	},
+	.probe = tps6598x_probe_i2c,
+	.remove = tps6598x_remove_i2c,
+	.id_table = tps6598x_id,
+};
+module_i2c_driver(tps6598x_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("TI TPS6598x USB Power Delivery Controller Driver");
diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
index 11ab58ba9a18..5af575d18354 100644
--- a/drivers/usb/typec/tipd/tps6598x.h
+++ b/drivers/usb/typec/tipd/tps6598x.h
@@ -8,6 +8,10 @@
 
 #include <linux/bits.h>
 #include <linux/bitfield.h>
+#include <linux/interrupt.h>
+#include <linux/power_supply.h>
+#include <linux/usb/typec.h>
+#include <linux/usb/typec_mux.h>
 
 #ifndef __TPS6598X_H__
 #define __TPS6598X_H__
@@ -273,4 +277,123 @@
 #define TPS_65981_2_6_INTEVENT_LEN             8
 #define TPS_65987_8_INTEVENT_LEN               11
 
+enum tipd_hw_variant {
+	TIPD_VARIANT_INVALID = 0,
+	TIPD_VARIANT_CD321X,
+	TIPD_VARIANT_TPS6598X,
+	TIPD_VARIANT_TPS66993,
+	TIPD_VARIANT_TPS25750,
+	TIPD_VARIANT_MAX,
+};
+
+struct tps6598x;
+
+struct tipd_data {
+	irq_handler_t irq_handler;
+	u64 irq_mask1;
+	size_t tps_struct_size;
+	bool no_mode_control;
+	void (*remove)(struct tps6598x *tps);
+	int (*register_port)(struct tps6598x *tps, struct fwnode_handle *node);
+	void (*unregister_port)(struct tps6598x *tps);
+	void (*trace_data_status)(u32 status);
+	void (*trace_power_status)(u16 status);
+	void (*trace_status)(u32 status);
+	int (*apply_patch)(struct tps6598x *tps);
+	int (*init)(struct tps6598x *tps);
+	int (*switch_power_state)(struct tps6598x *tps, u8 target_state);
+	bool (*read_data_status)(struct tps6598x *tps);
+	bool (*read_power_status)(struct tps6598x *tps);
+	int (*reset)(struct tps6598x *tps);
+	int (*connect)(struct tps6598x *tps, u32 status);
+};
+
+struct tps6598x {
+	struct device *dev;
+	struct regmap *regmap;
+	struct mutex lock; /* device lock */
+	int irq;
+	u8 i2c_protocol:1;
+
+	struct gpio_desc *reset;
+	struct typec_port *port;
+	struct typec_partner *partner;
+	struct usb_pd_identity partner_identity;
+	struct usb_role_switch *role_sw;
+	struct typec_capability typec_cap;
+
+	struct power_supply *psy;
+	struct power_supply_desc psy_desc;
+	enum power_supply_usb_type usb_type;
+
+	int wakeup;
+	u32 status; /* status reg */
+	u32 data_status;
+	u16 pwr_status;
+	struct delayed_work	wq_poll;
+
+	const struct tipd_data *data;
+};
+
+/* TPS_REG_USB4_STATUS */
+struct tps6598x_usb4_status_reg {
+	u8 mode_status;
+	__le32 eudo;
+	__le32 unknown;
+} __packed;
+
+/* TPS_REG_DP_SID_STATUS */
+struct tps6598x_dp_sid_status_reg {
+	u8 mode_status;
+	__le32 status_tx;
+	__le32 status_rx;
+	__le32 configure;
+	__le32 mode_data;
+} __packed;
+
+/* TPS_REG_INTEL_VID_STATUS */
+struct tps6598x_intel_vid_status_reg {
+	u8 mode_status;
+	__le32 attention_vdo;
+	__le16 enter_vdo;
+	__le16 device_mode;
+} __packed;
+
+struct cd321x_status {
+	u32 status;
+	u32 pwr_status;
+	u32 data_status;
+	u32 status_changed;
+	struct usb_pd_identity partner_identity;
+	struct tps6598x_dp_sid_status_reg dp_sid_status;
+	struct tps6598x_intel_vid_status_reg intel_vid_status;
+	struct tps6598x_usb4_status_reg usb4_status;
+};
+
+struct cd321x {
+	struct tps6598x tps;
+
+	struct tps6598x_dp_sid_status_reg dp_sid_status;
+	struct tps6598x_intel_vid_status_reg intel_vid_status;
+	struct tps6598x_usb4_status_reg usb4_status;
+
+	struct typec_altmode *port_altmode_dp;
+	struct typec_altmode *port_altmode_tbt;
+
+	struct typec_mux *mux;
+	struct typec_mux_state state;
+
+	struct cd321x_status update_status;
+	struct delayed_work update_work;
+	struct usb_pd_identity cur_partner_identity;
+};
+
+extern const struct tipd_data *tipd_hw_variants[];
+extern const struct regmap_config tps6598x_regmap_config;
+
+int tipd_init(struct tps6598x *tps);
+void tipd_remove(struct tps6598x *tps);
+int tipd_suspend(struct tps6598x *tps);
+int tipd_resume(struct tps6598x *tps);
+
 #endif /* __TPS6598X_H__ */

-- 
2.55.0

[PATCH v7 3/3] usb: typec: tipd: Add sn201202x support

[Sasha Finkelstein]

Add support for sn201202x (aka ACE3), a tipd variant that uses a very
similar register map, that is exposed over a "logical register"
interface on the SPMI bus.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Co-developed-by: Alyssa Milburn <amilburn@zall.org>
Signed-off-by: Alyssa Milburn <amilburn@zall.org>
Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
---
 MAINTAINERS                       |   1 +
 drivers/usb/typec/Makefile        |   1 +
 drivers/usb/typec/tipd/Kconfig    |  12 ++++++
 drivers/usb/typec/tipd/Makefile   |   3 ++
 drivers/usb/typec/tipd/core.c     |  21 ++++++++++
 drivers/usb/typec/tipd/spmi.c     | 300 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/typec/tipd/tps6598x.h |   9 +++++
 7 files changed, 347 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ba4274ed9c79..5e631428a6c9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2690,6 +2690,7 @@ F:	drivers/soc/apple/*
 F:	drivers/spi/spi-apple.c
 F:	drivers/spmi/spmi-apple-controller.c
 F:	drivers/usb/dwc3/dwc3-apple.c
+F:	drivers/usb/typec/tipd/spmi.c
 F:	drivers/video/backlight/apple_dwi_bl.c
 F:	drivers/watchdog/apple_wdt.c
 F:	include/dt-bindings/interrupt-controller/apple-aic.h
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 8a6a1c663eb6..80d6767667a7 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_TYPEC)		+= altmodes/
 obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)	+= tipd/
+obj-$(CONFIG_TYPEC_SN201202X)	+= tipd/
 obj-$(CONFIG_TYPEC_ANX7411)	+= anx7411.o
 obj-$(CONFIG_TYPEC_HD3SS3220)	+= hd3ss3220.o
 obj-$(CONFIG_TYPEC_STUSB160X) 	+= stusb160x.o
diff --git a/drivers/usb/typec/tipd/Kconfig b/drivers/usb/typec/tipd/Kconfig
index c87cbe6109ca..8177f860f89d 100644
--- a/drivers/usb/typec/tipd/Kconfig
+++ b/drivers/usb/typec/tipd/Kconfig
@@ -14,3 +14,15 @@ config TYPEC_TPS6598X
 
 	  If you choose to build this driver as a dynamically linked module, the
 	  module will be called tps6598x.ko.
+
+config TYPEC_SN201202X
+	tristate "TI SN201202x USB Power Delivery controller support"
+	depends on SPMI
+	select TYPEC_TPS6598X_CORE
+	help
+	  Say Y here to enable support for SN201202x, a TPS6598x variant
+	  that uses SPMI as a host bus, and is found on Apple devices starting
+	  from the M3 series.
+
+	  If you choose to build this driver as a dynamically linked module, the
+	  module will be called sn201202x.ko.
diff --git a/drivers/usb/typec/tipd/Makefile b/drivers/usb/typec/tipd/Makefile
index c741bf44f52f..6f5ebc1e1a5f 100644
--- a/drivers/usb/typec/tipd/Makefile
+++ b/drivers/usb/typec/tipd/Makefile
@@ -7,3 +7,6 @@ tps6598x-core-$(CONFIG_TRACING)		+= trace.o
 
 obj-$(CONFIG_TYPEC_TPS6598X)		+= tps6598x.o
 tps6598x-y				:= i2c.o
+
+obj-$(CONFIG_TYPEC_SN201202X)		+= sn201202x.o
+sn201202x-y				:= spmi.o
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index bdcc6f75d93f..82145c9f1cc8 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -1962,11 +1962,32 @@ static const struct tipd_data tipd_tps25750_data = {
 	.connect = tps6598x_connect,
 };
 
+static const struct tipd_data tipd_sn201202x_data = {
+	.irq_handler = cd321x_interrupt,
+	.irq_mask1 = APPLE_CD_REG_INT_POWER_STATUS_UPDATE |
+		     APPLE_CD_REG_INT_DATA_STATUS_UPDATE |
+		     APPLE_CD_REG_INT_PLUG_EVENT,
+	.tps_struct_size = sizeof(struct sn201202x),
+	.remove = cd321x_remove,
+	.register_port = cd321x_register_port,
+	.unregister_port = cd321x_unregister_port,
+	.trace_data_status = trace_cd321x_data_status,
+	.trace_power_status = trace_tps6598x_power_status,
+	.trace_status = trace_tps6598x_status,
+	.init = cd321x_init,
+	.read_data_status = cd321x_read_data_status,
+	.read_power_status = tps6598x_read_power_status,
+	.reset = cd321x_reset,
+	.switch_power_state = cd321x_switch_power_state,
+	.connect = cd321x_connect,
+};
+
 const struct tipd_data *tipd_hw_variants[] = {
 	[TIPD_VARIANT_CD321X] = &tipd_cd321x_data,
 	[TIPD_VARIANT_TPS6598X] = &tipd_tps6598x_data,
 	[TIPD_VARIANT_TPS66993] = &tipd_tps66993_data,
 	[TIPD_VARIANT_TPS25750] = &tipd_tps25750_data,
+	[TIPD_VARIANT_SN201202X] = &tipd_sn201202x_data,
 };
 EXPORT_SYMBOL_GPL(tipd_hw_variants);
 
diff --git a/drivers/usb/typec/tipd/spmi.c b/drivers/usb/typec/tipd/spmi.c
new file mode 100644
index 000000000000..a165ec39a8ed
--- /dev/null
+++ b/drivers/usb/typec/tipd/spmi.c
@@ -0,0 +1,300 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/interrupt.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/regmap.h>
+#include <linux/spmi.h>
+
+#include "tps6598x.h"
+
+#define tps_to_sn(tps) container_of_const((tps), struct sn201202x, cd.tps)
+
+static int regmap_sn201202x_select_reg(struct spmi_device *sdev, u8 reg)
+{
+	int err;
+	u8 val;
+	bool warned = false;
+	int attempts = 5;
+	struct tps6598x *tps = spmi_device_get_drvdata(sdev);
+	struct sn201202x *sn = tps_to_sn(tps);
+
+	if (reg >= 0x80) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+	reinit_completion(&sn->select_completion);
+	err = spmi_register_zero_write(sdev, reg);
+	if (err)
+		return err;
+
+	if (!wait_for_completion_timeout(&sn->select_completion, msecs_to_jiffies(100)))
+		return -ETIMEDOUT;
+
+	while (--attempts) {
+		err = spmi_register_read(sdev, 0, &val);
+		if (err)
+			return err;
+		if (val == (reg | 0x80)) {
+			if (!warned) {
+				dev_warn(tps->dev,
+					 "Got interrupt but selection not complete?\n");
+				warned = true;
+			}
+			msleep(20);
+			continue;
+		}
+		if (val == reg)
+			return 0;
+		return -EIO;
+	}
+
+	return -EIO;
+}
+
+static int regmap_sn201202x_read(void *context,
+				 const void *reg, size_t reg_size,
+				 void *val, size_t val_size)
+{
+	int err;
+	unsigned int offset = 0x20;
+	size_t len;
+	u8 addr;
+
+	if (reg_size != 1) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+	if (val_size > 0x40) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	addr = *(u8 *)reg;
+
+	err = regmap_sn201202x_select_reg(context, addr);
+	if (err)
+		return err;
+
+	while (val_size) {
+		len = min_t(size_t, val_size, 16);
+		err = spmi_ext_register_read(context, offset, val, len);
+		if (err)
+			return err;
+		offset += len;
+		val += len;
+		val_size -= len;
+	}
+
+	return 0;
+}
+
+static int regmap_sn201202x_write(void *context, const void *data,
+				  size_t count)
+{
+	int err = 0;
+	unsigned int offset = 0xa0;
+	size_t len;
+	u8 addr;
+
+	if (count < 1) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	addr = *(u8 *)data;
+	data += 1;
+	count -= 1;
+
+	if (count > 0x40) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	err = regmap_sn201202x_select_reg(context, addr);
+	if (err)
+		return err;
+
+	while (count) {
+		len = min_t(size_t, count, 16);
+		err = spmi_ext_register_write(context, offset, data, len);
+		if (err)
+			return err;
+		offset += len;
+		data += len;
+		count -= len;
+	}
+
+	return err;
+}
+
+static irqreturn_t sn201202x_irq(int irq, void *data)
+{
+	struct completion *c = data;
+
+	complete(c);
+	return IRQ_HANDLED;
+}
+
+static const struct regmap_bus regmap_sn201202x = {
+	.read				= regmap_sn201202x_read,
+	.write				= regmap_sn201202x_write,
+	.reg_format_endian_default	= REGMAP_ENDIAN_NATIVE,
+	.val_format_endian_default	= REGMAP_ENDIAN_NATIVE,
+};
+
+static struct regmap *__devm_regmap_init_sn201202x(struct spmi_device *sdev,
+						   const struct regmap_config *config,
+						   struct lock_class_key *lock_key,
+						   const char *lock_name)
+{
+	return __devm_regmap_init(&sdev->dev, &regmap_sn201202x, sdev, config,
+				  lock_key, lock_name);
+}
+
+#define devm_regmap_init_sn201202x(dev, config)				\
+	__regmap_lockdep_wrapper(__devm_regmap_init_sn201202x, #config,	\
+				dev, config)
+
+static const struct of_device_id sn201202x_of_match[] = {
+	{ .compatible = "apple,sn201202x", (void *)TIPD_VARIANT_SN201202X},
+	{}
+};
+
+static int sn201202x_probe(struct spmi_device *device)
+{
+	const struct of_device_id *match;
+	const struct tipd_data *data;
+	struct sn201202x *sn;
+	struct tps6598x *tps;
+	int irq_select, irq_sleep, irq_wake;
+	int ret;
+
+	match = of_match_device(sn201202x_of_match, &device->dev);
+	if (!match)
+		return -EINVAL;
+	data = tipd_hw_variants[(enum tipd_hw_variant)(uintptr_t)match->data];
+
+	sn = devm_kzalloc(&device->dev, data->tps_struct_size, GFP_KERNEL);
+	if (!sn)
+		return -ENOMEM;
+	sn->sdev = device;
+	tps = &sn->cd.tps;
+
+	mutex_init(&tps->lock);
+	tps->dev = &device->dev;
+	tps->data = data;
+
+	tps->irq = of_irq_get_byname(device->dev.of_node, "irq");
+	if (tps->irq < 0)
+		return tps->irq;
+	irq_select = of_irq_get_byname(device->dev.of_node, "select");
+	if (irq_select < 0)
+		return irq_select;
+	irq_sleep = of_irq_get_byname(device->dev.of_node, "sleep");
+	if (irq_sleep < 0)
+		return irq_sleep;
+	irq_wake = of_irq_get_byname(device->dev.of_node, "wake");
+	if (irq_wake < 0)
+		return irq_wake;
+
+	init_completion(&sn->select_completion);
+	init_completion(&sn->sleep_completion);
+	init_completion(&sn->wake_completion);
+
+	ret = devm_request_irq(&device->dev, irq_select, sn201202x_irq,
+			       0, NULL, &sn->select_completion);
+	if (ret)
+		return ret;
+	ret = devm_request_irq(&device->dev, irq_sleep, sn201202x_irq,
+			       0, NULL, &sn->sleep_completion);
+	if (ret)
+		return ret;
+	ret = devm_request_irq(&device->dev, irq_wake, sn201202x_irq,
+			       0, NULL, &sn->wake_completion);
+	if (ret)
+		return ret;
+
+	spmi_device_set_drvdata(device, tps);
+	tps->regmap = devm_regmap_init_sn201202x(device, &tps6598x_regmap_config);
+	if (IS_ERR(tps->regmap))
+		return PTR_ERR(tps->regmap);
+
+	ret = spmi_command_wakeup(device);
+	if (ret)
+		return ret;
+	if (!wait_for_completion_timeout(&sn->wake_completion, msecs_to_jiffies(100)))
+		return -ETIMEDOUT;
+
+	ret = tipd_init(tps);
+	if (ret)
+		spmi_command_sleep(device);
+	return ret;
+}
+
+static void sn201202x_remove(struct spmi_device *device)
+{
+	struct tps6598x *tps = spmi_device_get_drvdata(device);
+	struct sn201202x *sn = tps_to_sn(tps);
+
+	tipd_remove(tps);
+	spmi_command_sleep(sn->sdev);
+}
+
+static int __maybe_unused sn201202x_resume(struct device *dev)
+{
+	struct tps6598x *tps = dev_get_drvdata(dev);
+	struct sn201202x *sn = tps_to_sn(tps);
+	int err;
+
+	reinit_completion(&sn->wake_completion);
+	err = spmi_command_wakeup(sn->sdev);
+	if (err)
+		return err;
+	if (!wait_for_completion_timeout(&sn->wake_completion, msecs_to_jiffies(100)))
+		return -ETIMEDOUT;
+	return tipd_resume(tps);
+}
+
+static int __maybe_unused sn201202x_suspend(struct device *dev)
+{
+	struct tps6598x *tps = dev_get_drvdata(dev);
+	struct sn201202x *sn = tps_to_sn(tps);
+	int err;
+
+	err = tipd_suspend(tps);
+	if (err)
+		return err;
+	reinit_completion(&sn->sleep_completion);
+	err = spmi_command_sleep(sn->sdev);
+	if (err)
+		goto out_resume;
+	if (!wait_for_completion_timeout(&sn->sleep_completion, msecs_to_jiffies(100)))
+		dev_err(dev, "Waiting for sleep complete irq timed out");
+
+	return 0;
+
+out_resume:
+	tipd_resume(tps);
+	return err;
+}
+
+MODULE_DEVICE_TABLE(of, sn201202x_of_match);
+
+static const struct dev_pm_ops sn201202x_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(sn201202x_suspend, sn201202x_resume)
+};
+
+static struct spmi_driver sn201202x_driver = {
+	.driver = {
+		.name = "sn201202x",
+		.pm = &sn201202x_pm_ops,
+		.of_match_table = sn201202x_of_match,
+	},
+	.probe = sn201202x_probe,
+	.remove = sn201202x_remove,
+};
+module_spmi_driver(sn201202x_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("TI SN201202x USB Power Delivery Controller Driver");
diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
index 5af575d18354..a411392deb39 100644
--- a/drivers/usb/typec/tipd/tps6598x.h
+++ b/drivers/usb/typec/tipd/tps6598x.h
@@ -283,6 +283,7 @@ enum tipd_hw_variant {
 	TIPD_VARIANT_TPS6598X,
 	TIPD_VARIANT_TPS66993,
 	TIPD_VARIANT_TPS25750,
+	TIPD_VARIANT_SN201202X,
 	TIPD_VARIANT_MAX,
 };
 
@@ -388,6 +389,14 @@ struct cd321x {
 	struct usb_pd_identity cur_partner_identity;
 };
 
+struct sn201202x {
+	struct cd321x cd;
+	struct completion select_completion;
+	struct completion sleep_completion;
+	struct completion wake_completion;
+	struct spmi_device *sdev;
+};
+
 extern const struct tipd_data *tipd_hw_variants[];
 extern const struct regmap_config tps6598x_regmap_config;
 

-- 
2.55.0

Re: [PATCH v7 2/3] usb: typec: tipd: Factor out i2c specifics

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] `tipd_suspend()` and `tipd_resume()` blindly operate on IRQ 0 if the device has a `wakeup-source` property but no valid interrupt line.
- [High] Probe failure in polling mode causes a Use-After-Free because `tps->wq_poll` is queued but never canceled in the error path.
- [High] Device is accessed over I2C before being taken out of reset during resume, causing a persistent failure.
--

commit 483e340a7e2db3c0bcd0902d89a807ddfbd80a8f
Author: Alyssa Milburn <amilburn@zall.org>

usb: typec: tipd: Factor out i2c specifics

Make the core driver more bus-agnostic to prepare for SPMI variants of
the tipd chip.

> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -1898,14 +1771,14 @@ static int tps6598x_probe(struct i2c_client *client)
[ ... ]
>  	if (tps->irq) {
>  		ret = devm_request_threaded_irq(tps->dev, tps->irq, NULL,
>  						tps->data->irq_handler,
>  						IRQF_SHARED | IRQF_ONESHOT,
>  						dev_name(tps->dev), tps);
>  	} else {
>  		dev_dbg(tps->dev, "no IRQ specified, using polling mode\n");
>  		INIT_DELAYED_WORK(&tps->wq_poll, tps6598x_poll_work);
> @@ -1916,13 +1789,12 @@ static int tps6598x_probe(struct i2c_client *client)
>  	if (ret)
>  		goto err_disconnect;

[Severity: High]
This is a pre-existing issue, but does this leak memory or cause a
use-after-free if the probe fails while in polling mode?

If the device is configured in polling mode (no IRQ specified) and the call
to tps->data->connect() fails, ret holds a non-zero error code. The polling
work tps->wq_poll is queued here, and then the code jumps to err_disconnect.

Looking at the err_disconnect path in tipd_init():

err_disconnect:
        tps6598x_disconnect(tps, 0);
err_unregister_port:
        tps->data->unregister_port(tps);
...
        return ret;

The error path returns without canceling the queued work. When devres frees
the tps structure, the delayed work timer can still fire 500ms later, which
would lead to a use-after-free when it accesses the freed memory.

Could we add a cancel_delayed_work_sync() call in the error handling path?

[ ... ]

> -static int __maybe_unused tps6598x_suspend(struct device *dev)
> +int tipd_suspend(struct tps6598x *tps)
>  {
>  	if (tps->wakeup) {
> -		disable_irq(client->irq);
> -		enable_irq_wake(client->irq);
> +		disable_irq(tps->irq);
> +		enable_irq_wake(tps->irq);

[Severity: High]
This isn't a bug introduced by this patch, but could this blindly operate
on IRQ 0 during system suspend?

If a device is configured in polling mode, tps->irq is 0. If that device
also has the wakeup-source property set in its device tree, tipd_suspend()
will call disable_irq(0) and enable_irq_wake(0). On architectures where
IRQ 0 is a system interrupt (such as the timer tick on x86), disabling it
can cause a hang during suspend.

[ ... ]

>  	} else if (tps->reset) {
>  		gpiod_set_value_cansleep(tps->reset, 1);
>  	}
>  
> -	if (!client->irq)
> +	if (!tps->irq)
>  		cancel_delayed_work_sync(&tps->wq_poll);
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(tipd_suspend);
>  
> -static int __maybe_unused tps6598x_resume(struct device *dev)
> +int tipd_resume(struct tps6598x *tps)
>  {
>  	int ret;
>  
>  	ret = tps6598x_check_mode(tps);

[Severity: High]
This is a pre-existing issue, but will this I2C read persistently fail
during resume?

In tipd_suspend(), if the device lacks a wakeup source but has a reset GPIO,
it is put into reset. However, during tipd_resume(), the very first operation
is a call to tps6598x_check_mode(tps), which attempts an I2C read.

Since the device is still in reset when this I2C read is attempted, the read
will fail and return an error early, leaving the device permanently in reset
after a system suspend.

[ ... ]

>  	if (ret < 0)
>  		return ret;
>  
> @@ -2002,25 +1870,22 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
>  	}
>  
>  	if (tps->wakeup) {
> -		disable_irq_wake(client->irq);
> -		enable_irq(client->irq);
> +		disable_irq_wake(tps->irq);
> +		enable_irq(tps->irq);

[Severity: High]
This isn't a bug introduced by this patch, but similar to the suspend path,
will this operate on IRQ 0 if the device has a wakeup-source property but
no valid interrupt line?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-tipd-ace3-v7-0-6c6ebe2193f2@chaosmail.tech?part=2

Re: [PATCH v7 3/3] usb: typec: tipd: Add sn201202x support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] Probe function fails to catch a 0 return value from of_irq_get_byname(), erroneously passing IRQ 0 to devm_request_irq().
- [Medium] spmi_command_sleep() is called without waiting for the corresponding sleep interrupt in sn201202x_remove and the probe error path, causing a teardown race.
- [Low] devm_request_irq() is called with a NULL devname argument.

Pre-existing issues:
- [High] Calling disable_irq(0) when tps->irq is 0 and wakeup-source is defined.
--

commit 50826c85c86e2d8bfcd4558e78d047469a9a4cbf
Author: Sasha Finkelstein <k@chaosmail.tech>

usb: typec: tipd: Add sn201202x support

This commit adds support for the sn201202x (ACE3) controller, a tipd
variant that uses a similar register map exposed over a logical register
interface on the SPMI bus.

[ ... ]
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index bdcc6f75d93fe..82145c9f1cc8c 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -1962,11 +1962,32 @@ static const struct tipd_data tipd_tps25750_data = {
>  	.connect = tps6598x_connect,
>  };
>  
> +static const struct tipd_data tipd_sn201202x_data = {
> +	.irq_handler = cd321x_interrupt,

[Severity: High]
This is a pre-existing issue, but utilizing the tipd core suspend/resume 
infrastructure with a device that might not have an IRQ mapped can cause 
a system hang.

If a device tree or ACPI config defines "wakeup-source" but lacks an "irq" 
property, tps->irq will default to 0. During system suspend, tipd_suspend() 
only checks tps->wakeup:

drivers/usb/typec/tipd/core.c:tipd_suspend() {
    ...
    if (tps->wakeup) {
        disable_irq(tps->irq);
        enable_irq_wake(tps->irq);
    }
    ...
}

Will this cause disable_irq(0) and enable_irq_wake(0) to be called? On 
architectures like x86 where IRQ 0 is a critical system interrupt (the PIT 
timer), disabling it can hang the kernel.

> +	.irq_mask1 = APPLE_CD_REG_INT_POWER_STATUS_UPDATE |
[ ... ]
> diff --git a/drivers/usb/typec/tipd/spmi.c b/drivers/usb/typec/tipd/spmi.c
> new file mode 100644
> index 0000000000000..a165ec39a8ed8
> --- /dev/null
> +++ b/drivers/usb/typec/tipd/spmi.c
[ ... ]
> +static int sn201202x_probe(struct spmi_device *device)
> +{
[ ... ]
> +	tps->irq = of_irq_get_byname(device->dev.of_node, "irq");
> +	if (tps->irq < 0)
> +		return tps->irq;

[Severity: Medium]
Does this check miss the case where of_irq_get_byname() returns 0? 

If the device tree is malformed or IRQ mapping fails, of_irq_get_byname() 
can return 0. Passing IRQ 0 to devm_request_irq() can conflict with critical 
system interrupts instead of aborting the probe gracefully. Should this check 
be <= 0?

> +	irq_select = of_irq_get_byname(device->dev.of_node, "select");
> +	if (irq_select < 0)
> +		return irq_select;
> +	irq_sleep = of_irq_get_byname(device->dev.of_node, "sleep");
> +	if (irq_sleep < 0)
> +		return irq_sleep;
> +	irq_wake = of_irq_get_byname(device->dev.of_node, "wake");
> +	if (irq_wake < 0)
> +		return irq_wake;

[Severity: Medium]
Similarly, do these checks also need to verify that the returned IRQs 
are not 0?

> +
> +	init_completion(&sn->select_completion);
> +	init_completion(&sn->sleep_completion);
> +	init_completion(&sn->wake_completion);
> +
> +	ret = devm_request_irq(&device->dev, irq_select, sn201202x_irq,
> +			       0, NULL, &sn->select_completion);

[Severity: Low]
Is it intentional to pass NULL as the devname argument here?

The IRQ name will appear as "(null)" in /proc/interrupts and tracing output, 
which might degrade observability and debugging tools. Could a descriptive 
string be provided instead?

> +	if (ret)
> +		return ret;
> +	ret = devm_request_irq(&device->dev, irq_sleep, sn201202x_irq,
> +			       0, NULL, &sn->sleep_completion);

[Severity: Low]
Does this also need a valid devname instead of NULL?

> +	if (ret)
> +		return ret;
> +	ret = devm_request_irq(&device->dev, irq_wake, sn201202x_irq,
> +			       0, NULL, &sn->wake_completion);

[Severity: Low]
Does this also need a valid devname instead of NULL?

> +	if (ret)
> +		return ret;
> +
> +	spmi_device_set_drvdata(device, tps);
> +	tps->regmap = devm_regmap_init_sn201202x(device, &tps6598x_regmap_config);
> +	if (IS_ERR(tps->regmap))
> +		return PTR_ERR(tps->regmap);
> +
> +	ret = spmi_command_wakeup(device);
> +	if (ret)
> +		return ret;
> +	if (!wait_for_completion_timeout(&sn->wake_completion, msecs_to_jiffies(100)))
> +		return -ETIMEDOUT;
> +
> +	ret = tipd_init(tps);
> +	if (ret)
> +		spmi_command_sleep(device);

[Severity: Medium]
Could this create a race condition during driver teardown on the error path?

If the hardware is slow to enter sleep, the sleep interrupt might fire after 
the IRQ handler is freed by devm when probe fails, potentially causing an 
unhandled interrupt. Should this wait for the sleep completion like the 
wakeup path does?

> +	return ret;
> +}
> +
> +static void sn201202x_remove(struct spmi_device *device)
> +{
> +	struct tps6598x *tps = spmi_device_get_drvdata(device);
> +	struct sn201202x *sn = tps_to_sn(tps);
> +
> +	tipd_remove(tps);
> +	spmi_command_sleep(sn->sdev);

[Severity: Medium]
Similarly, does this need synchronization with the sleep interrupt before 
remove completes?

> +}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-tipd-ace3-v7-0-6c6ebe2193f2@chaosmail.tech?part=3