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

[PATCH v5 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 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/tipd/Kconfig                             |  20 +++++++-
 drivers/usb/typec/tipd/Makefile                            |  14 ++++--
 drivers/usb/typec/tipd/core.c                              | 246 ++++++++++++++++++++++--------------------------------------------------------------------------
 drivers/usb/typec/tipd/i2c.c                               | 106 ++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/typec/tipd/spmi.c                              | 300 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/typec/tipd/tps6598x.h                          | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 702 insertions(+), 196 deletions(-)
---
base-commit: 3d9eeb336131bc5a174367c384fa00c15c8744fd
change-id: 20260725-tipd-ace3-090bf570ef59

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

[PATCH v5 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 8014b9f8253e..62030ad60baa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2610,6 +2610,7 @@ F:	Documentation/devicetree/bindings/rtc/apple,smc-rtc.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 v5 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      | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/typec/tipd/tps6598x.h | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 280 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 522f56742aa9..36fccd45858b 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,31 +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;
-	__le16 cable_mode;
-} __packed;
-
 /* Standard Task return codes */
 #define TPS_TASK_TIMEOUT		1
 #define TPS_TASK_REJECTED		3
@@ -145,82 +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;
-	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,
@@ -1127,11 +1024,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)
@@ -1778,27 +1676,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),
@@ -1806,10 +1690,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) {
@@ -1819,13 +1699,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)
@@ -1861,7 +1734,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);
 
@@ -1887,14 +1760,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);
@@ -1905,13 +1778,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;
@@ -1932,15 +1804,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);
@@ -1955,29 +1826,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);
@@ -1991,25 +1859,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 |
@@ -2029,7 +1894,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 |
@@ -2048,7 +1913,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,
@@ -2066,7 +1931,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 |
@@ -2085,33 +1950,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_tps6598x_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..f852cdc327f5
--- /dev/null
+++ b/drivers/usb/typec/tipd/i2c.c
@@ -0,0 +1,106 @@
+// 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)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 = "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 d4140f4da5bb..6b9785c62afd 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;
+	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;
+	__le16 cable_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 v5 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/tipd/Kconfig    |  12 ++++++
 drivers/usb/typec/tipd/Makefile   |   3 ++
 drivers/usb/typec/tipd/core.c     |  20 ++++++++++
 drivers/usb/typec/tipd/spmi.c     | 300 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/typec/tipd/tps6598x.h |   9 +++++
 6 files changed, 345 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 62030ad60baa..afe38cb1156b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2641,6 +2641,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/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 36fccd45858b..296bc8adcc91 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -1950,11 +1950,31 @@ 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,
+	.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_tps6598x_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..15976d15d9bc
--- /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)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 6b9785c62afd..297be9738037 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 v5 2/3] usb: typec: tipd: Factor out i2c specifics

[sashiko-bot]

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

New issues:
- [High] The `ti,tps66993` compatible string was inadvertently removed from the OF match table during the I2C split.
- [High] The hardware variant mapping for `TIPD_VARIANT_TPS66993` incorrectly points to `tipd_tps6598x_data` instead of `tipd_tps66993_data`.
- [Medium] The supposedly bus-agnostic core still contains direct I2C struct downcasts, risking kernel crashes on non-I2C buses.
- [Low] Direct pointer-to-integer cast causes build warnings/errors on 64-bit systems.

Pre-existing issues:
- [Critical] Missing cancellation of delayed works in the probe error path leads to use-after-free.
- [High] Suspend and resume functions unconditionally disable/enable IRQ 0 when in polling mode, potentially causing system instability.
--

--- Patch [2]: [PATCH v5 2/3] usb: typec: tipd: Factor out i2c specifics ---
commit 691ce94a8de82880f19343c1fa6f0daf5755c9d5
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.

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

[Severity: Medium]
Will this bus-agnostic abstraction fail on non-I2C buses due to remaining I2C
struct downcasts?

For example, tps25750_write_firmware() in core.c still uses to_i2c_client():

	struct i2c_client *client = to_i2c_client(tps->dev);
	...
	slave_addr = client->addr;
	timeout = client->adapter->timeout;
	...
	client->addr = bpms_addr;

Could this cause memory corruption if a non-I2C variant calls
tps25750_apply_patch() -> tps25750_start_patch_burst_mode() ->
tps25750_write_firmware() in the future?

> 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

[ ... ]

> @@ -1887,14 +1760,14 @@ static int tps6598x_probe(struct i2c_client *client)
>  	if (tps->irq) {
> -		ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> +		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);
>  		queue_delayed_work(system_power_efficient_wq, &tps->wq_poll,
>  				   msecs_to_jiffies(POLL_INTERVAL));
>  	}
>  
>  	if (ret)
>  		goto err_disconnect;

[Severity: Critical]
This isn't a bug introduced by this patch, but could this error path leave
delayed works pending and cause a use-after-free?

If tipd_init() fails after queueing tps->wq_poll in polling mode, or after
connect() schedules cd321x->update_work, it jumps to err_disconnect. Since
cancel_delayed_work_sync() is missing in the cleanup, devres will free
the tps structure while the system workqueue might still execute the pending
work and dereference the freed memory.

[ ... ]

> @@ -1955,29 +1826,26 @@ static void tps6598x_remove(struct i2c_client *client)
> +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);
>  	}

[Severity: High]
This is a pre-existing issue, but does this unconditionally disable and enable
IRQ 0 when in polling mode?

If a device tree specifies a wakeup-source without providing an interrupt,
tps->irq remains 0 while tps->wakeup is set to true. During suspend and resume,
tipd_suspend() and tipd_resume() call disable_irq(0) and enable_irq_wake(0).
Manipulating IRQ 0 (typically the system timer on x86) can cause lockups or
panics.

[ ... ]

> @@ -2085,33 +1950,14 @@ static const struct tipd_data tps25750_data = {
> +const struct tipd_data *tipd_hw_variants[] = {
> +	[TIPD_VARIANT_CD321X] = &tipd_cd321x_data,
> +	[TIPD_VARIANT_TPS6598X] = &tipd_tps6598x_data,
> +	[TIPD_VARIANT_TPS66993] = &tipd_tps6598x_data,
> +	[TIPD_VARIANT_TPS25750] = &tipd_tps25750_data,
>  };

[Severity: High]
Is it intentional that TIPD_VARIANT_TPS66993 points to tipd_tps6598x_data
instead of tipd_tps66993_data?

Since tipd_tps66993_data is defined with the correct
tps66993_read_power_status() callback and IRQ masks, assigning
tipd_tps6598x_data here will cause TPS66993 devices to use the wrong
callbacks and experience hardware state parsing failures.

> diff --git a/drivers/usb/typec/tipd/i2c.c b/drivers/usb/typec/tipd/i2c.c
> new file mode 100644
> index 0000000000000..f852cdc327f5d
> --- /dev/null
> +++ b/drivers/usb/typec/tipd/i2c.c
> @@ -0,0 +1,106 @@

[ ... ]

> +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)i2c_get_match_data(client);

[Severity: Low]
Will this pointer-to-int cast trigger a -Wpointer-to-int-cast build warning on
64-bit architectures?

[ ... ]

> +static const struct of_device_id tps6598x_of_match[] = {
> +	{ .compatible = "ti,tps6598x", (void *)TIPD_VARIANT_TPS6598X},
> +	{ .compatible = "apple,cd321x",  (void *)TIPD_VARIANT_CD321X},
> +	{ .compatible = "ti,tps25750",  (void *)TIPD_VARIANT_TPS25750},
> +	{}
> +};

[Severity: High]
Did we accidentally drop the ti,tps66993 entry here during the refactoring?

Without this compatible string in the I2C driver, it seems we will no longer
bind to TPS66993 devices during boot.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-tipd-ace3-v5-0-3244ed3bdc3f@chaosmail.tech?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The `tipd_sn201202x_data` structure fails to initialize the `.read_power_status` callback, causing a NULL pointer dereference when invoked.
- [Medium] Enabling `CONFIG_TYPEC_SN201202X` does not guarantee compilation because the Makefile skips the `tipd/` directory if `CONFIG_TYPEC_TPS6598X` is disabled.
- [Medium] Missing `spmi_command_sleep()` on wakeup completion timeout leaves the controller awake on error paths.
- [Low] Pointer to integer cast of different size in `sn201202x_probe` triggers compiler warnings on 64-bit architectures.
- [Low] Implicit positional structure initialization in `sn201202x_of_match` bypasses designated initializer syntax.
--

--- Patch [3]: [PATCH v5 3/3] usb: typec: tipd: Add sn201202x support ---
commit cf60ef5ec1afc63ccae95643a77a2844f4dd0245
Author: Sasha Finkelstein <k@chaosmail.tech>

usb: typec: tipd: Add sn201202x support
    
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.

> diff --git a/drivers/usb/typec/tipd/Kconfig b/drivers/usb/typec/tipd/Kconfig
> index c87cbe6109ca6..8177f860f89de 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

[Severity: Medium]
Does enabling CONFIG_TYPEC_SN201202X guarantee that this driver will be
built?

It appears that the parent drivers/usb/typec/Makefile only descends into
the tipd/ directory if CONFIG_TYPEC_TPS6598X is enabled. If someone selects
CONFIG_TYPEC_SN201202X without CONFIG_TYPEC_TPS6598X, could this module
be silently excluded from the build entirely?

> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 36fccd45858bf..296bc8adcc91e 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
[ ... ]
> @@ -1950,11 +1950,31 @@ 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,
> +	.reset = cd321x_reset,
> +	.switch_power_state = cd321x_switch_power_state,
> +	.connect = cd321x_connect,
> +};

[Severity: High]
Is the .read_power_status callback intentionally omitted here?

Without it, the uninitialized callback defaults to NULL. 

Would this result in a NULL pointer dereference during device probe? In 
tipd_init(), if a plug is present (status & TPS_STATUS_PLUG_PRESENT), 
the code invokes the callback unconditionally:

    if (!tps->data->read_power_status(tps))
        goto err_unregister_port;

This same NULL dereference could also happen in cd321x_interrupt() when a
power status update is received (event & 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..15976d15d9bce
> --- /dev/null
> +++ b/drivers/usb/typec/tipd/spmi.c
[ ... ]
> +static const struct of_device_id sn201202x_of_match[] = {
> +	{ .compatible = "apple,sn201202x", (void *)TIPD_VARIANT_SN201202X},

[Severity: Low]
Is it intentional to use positional initialization for the data field here?

Mixing designated initializers (.compatible) with positional ones can
bypass designated initializer syntax and be error-prone if the structure
changes. Should this explicitly use .data = (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)match->data];

[Severity: Low]
Could this direct cast from a pointer (match->data) to an enum cause
compiler warnings?

On 64-bit architectures where pointers are 64-bit and enums are 32-bit
(like ARM64), this might trigger a -Wpointer-to-int-cast warning. Should
this cast through an unsigned long or uintptr_t first?

[ ... ]
> +	ret = spmi_command_wakeup(device);
> +	if (ret)
> +		return ret;
> +	if (!wait_for_completion_timeout(&sn->wake_completion, msecs_to_jiffies(100)))
> +		return -ETIMEDOUT;

[Severity: Medium]
If this wait_for_completion_timeout() times out, should the hardware be
put back to sleep?

By returning -ETIMEDOUT without calling spmi_command_sleep(), could the
device be left in an active state indefinitely and drain power?

[ ... ]
> +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;

[Severity: Medium]
Similarly here in the resume path, does this error return need to send a
sleep command to revert the hardware state if the wakeup times out?

> +	return tipd_resume(tps);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-tipd-ace3-v5-0-3244ed3bdc3f@chaosmail.tech?part=3

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

[Janne Grunau]

On Fri, Aug 21, 2026 at 10:26:20AM +0200, Sasha Finkelstein wrote:
> 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      | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/typec/tipd/tps6598x.h | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 280 insertions(+), 198 deletions(-)
> 
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 522f56742aa9..36fccd45858b 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -2085,33 +1950,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_tps6598x_data,

this should be &tipd_tps66993_data

Janne