All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] usb: typec: add drivers for TUSB320xA and TS5USBA224
@ 2022-03-01 13:20 Alvin Šipraga
  2022-03-01 13:20 ` [PATCH 1/4] dt-bindings: usb: add TUSB320xA Type-C port controller Alvin Šipraga
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Alvin Šipraga @ 2022-03-01 13:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Heikki Krogerus, �ipraga
  Cc: linux-usb, devicetree, linux-kernel

From: Alvin Šipraga <alsi@bang-olufsen.dk>

This series adds a new typec class driver for the TUSB320xA family of
Type-C port controllers and a typec_mux class driver for the TS5USBA224
switch mux.

This series was bourne out of frustration with the existing extcon
driver for the TUSB320, which did not offer a convenient driver model
for the Audio Accessory mode muxing offered by the TS5USBA224. I found
the typec subsystem to be more suitable.

I have tested this on my i.MX8MM platform with USB OTG support and it
works as desired. However I am not very familiar with this part of the
kernel, so I welcome your critical feedback to this series. Thanks in
advance.


Alvin Šipraga (4):
  dt-bindings: usb: add TUSB320xA Type-C port controller
  dt-bindings: usb: add TS5USBA224 USB/Audio switch mux
  usb: typec: add TUSB320xA driver
  usb: typec: mux: add TS5USBA224 driver

 .../bindings/usb/ti,ts5usba224.yaml           |  56 ++
 .../devicetree/bindings/usb/ti,tusb320xa.yaml |  78 +++
 drivers/usb/typec/Kconfig                     |  12 +
 drivers/usb/typec/Makefile                    |   1 +
 drivers/usb/typec/mux/Kconfig                 |  10 +
 drivers/usb/typec/mux/Makefile                |   1 +
 drivers/usb/typec/mux/ts5usba224.c            | 102 ++++
 drivers/usb/typec/tusb320xa.c                 | 517 ++++++++++++++++++
 8 files changed, 777 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
 create mode 100644 Documentation/devicetree/bindings/usb/ti,tusb320xa.yaml
 create mode 100644 drivers/usb/typec/mux/ts5usba224.c
 create mode 100644 drivers/usb/typec/tusb320xa.c

-- 
2.35.1


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

* [PATCH 1/4] dt-bindings: usb: add TUSB320xA Type-C port controller
  2022-03-01 13:20 [PATCH 0/4] usb: typec: add drivers for TUSB320xA and TS5USBA224 Alvin Šipraga
@ 2022-03-01 13:20 ` Alvin Šipraga
  2022-03-02 18:16   ` Rob Herring
  2022-03-01 13:20 ` [PATCH 2/4] dt-bindings: usb: add TS5USBA224 USB/Audio switch mux Alvin Šipraga
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Alvin Šipraga @ 2022-03-01 13:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Heikki Krogerus, �ipraga
  Cc: linux-usb, devicetree, linux-kernel

From: Alvin Šipraga <alsi@bang-olufsen.dk>

The TUSB320xA is a non-PD Type-C port controller managed over I2C.

Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 .../devicetree/bindings/usb/ti,tusb320xa.yaml | 78 +++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,tusb320xa.yaml

diff --git a/Documentation/devicetree/bindings/usb/ti,tusb320xa.yaml b/Documentation/devicetree/bindings/usb/ti,tusb320xa.yaml
new file mode 100644
index 000000000000..a93d53ddd01c
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,tusb320xa.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/ti,tusb320xa.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TUSB320xA Type-C port controller DT bindings
+
+description:
+  The Texas Instruments TUSB320xA is a USB Type-C port controller which
+  supports role and plug orientation detection using the CC pins. It is
+  compatible with the USB Type-C Cable and Connector Specification v1.1.
+
+maintainers:
+  - Alvin Šipraga <alsi@bang-olufsen.dk>
+
+properties:
+  compatible:
+    enum:
+      - ti,tusb320la
+      - ti,tusb320ha
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    patternProperties:
+      '^port@':
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          OF graph bindings modelling any "usb-role-switch" or "accessory" mux.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      tcpc@47 {
+        compatible = "ti,tusb320la";
+        reg = <0x47>;
+        interrupt-parent = <&gpio5>;
+        interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+
+        ports {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          port@0 {
+            reg = <0>;
+            typec1_mux: endpoint {
+              remote-endpoint = <&usb_audio_mux1>;
+            };
+          };
+
+          port@1 {
+            reg = <1>;
+            typec1_dr_sw: endpoint {
+              remote-endpoint = <&usbotg1_drd_sw>;
+            };
+          };
+        };
+      };
+    };
-- 
2.35.1


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

* [PATCH 2/4] dt-bindings: usb: add TS5USBA224 USB/Audio switch mux
  2022-03-01 13:20 [PATCH 0/4] usb: typec: add drivers for TUSB320xA and TS5USBA224 Alvin Šipraga
  2022-03-01 13:20 ` [PATCH 1/4] dt-bindings: usb: add TUSB320xA Type-C port controller Alvin Šipraga
@ 2022-03-01 13:20 ` Alvin Šipraga
  2022-03-02 18:21   ` Rob Herring
  2022-03-01 13:20 ` [PATCH 3/4] usb: typec: add TUSB320xA driver Alvin Šipraga
  2022-03-01 13:20 ` [PATCH 4/4] usb: typec: mux: add TS5USBA224 driver Alvin Šipraga
  3 siblings, 1 reply; 15+ messages in thread
From: Alvin Šipraga @ 2022-03-01 13:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Heikki Krogerus, �ipraga
  Cc: linux-usb, devicetree, linux-kernel

From: Alvin Šipraga <alsi@bang-olufsen.dk>

The TS5USBA224 is a USB High Speed/Audio switch mux IC controlled via
GPIO. It is typically composed with a Type-C port controller with Audio
Accessory mode detection.

Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 .../bindings/usb/ti,ts5usba224.yaml           | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml

diff --git a/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml b/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
new file mode 100644
index 000000000000..0a488b961906
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/ti,ts5usba224.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TS5USBA224 USB 2.0 High Speed and Audio mux DT bindings
+
+description:
+  The Texas Instruments TS5USBA224 is a double-pole, double throw
+  (DPDT) multiplexer that includes a low-distortion audio switch and a
+  USB 2.0 High Speed switch in the same package.
+
+maintainers:
+  - Alvin Šipraga <alsi@bang-olufsen.dk>
+
+properties:
+  compatible:
+    enum:
+      - ti,ts5usba224
+
+  asel-gpio:
+    description: Output GPIO for A_SEL signal
+    maxItems: 1
+
+  accessory:
+    type: boolean
+    description:
+      Indicates that this is an Accessory Mode mux.
+
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description:
+      OF graph bindings modelling a Type-C port controller.
+
+required:
+  - compatible
+  - asel-gpio
+  - accessory
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    usbaudiomux@0 {
+      compatible = "ti,ts5usba224";
+      asel-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
+      accessory;
+
+      port {
+        usb_audio_mux1: endpoint {
+          remote-endpoint = <&typec1_mux>;
+        };
+      };
+    };
-- 
2.35.1


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

* [PATCH 3/4] usb: typec: add TUSB320xA driver
  2022-03-01 13:20 [PATCH 0/4] usb: typec: add drivers for TUSB320xA and TS5USBA224 Alvin Šipraga
  2022-03-01 13:20 ` [PATCH 1/4] dt-bindings: usb: add TUSB320xA Type-C port controller Alvin Šipraga
  2022-03-01 13:20 ` [PATCH 2/4] dt-bindings: usb: add TS5USBA224 USB/Audio switch mux Alvin Šipraga
@ 2022-03-01 13:20 ` Alvin Šipraga
  2022-03-01 16:58   ` kernel test robot
  2022-03-07 14:36   ` Heikki Krogerus
  2022-03-01 13:20 ` [PATCH 4/4] usb: typec: mux: add TS5USBA224 driver Alvin Šipraga
  3 siblings, 2 replies; 15+ messages in thread
From: Alvin Šipraga @ 2022-03-01 13:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Heikki Krogerus, �ipraga
  Cc: linux-usb, devicetree, linux-kernel

From: Alvin Šipraga <alsi@bang-olufsen.dk>

The TUSB320LA and TUSB320HA (or LAI, HAI) chips are I2C controlled
non-PD Type-C port controllers. They support detection of cable
orientation, port attachment state, and role, including Audio Accessory
and Debug Accessory modes. Add a typec class driver for this family.

Note that there already exists an extcon driver for the TUSB320 (a
slightly older revision that does not support setting role preference or
disabling the CC state machine). This driver is loosely based on that
one.

Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 drivers/usb/typec/Kconfig     |  12 +
 drivers/usb/typec/Makefile    |   1 +
 drivers/usb/typec/tusb320xa.c | 517 ++++++++++++++++++++++++++++++++++
 3 files changed, 530 insertions(+)
 create mode 100644 drivers/usb/typec/tusb320xa.c

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 8f921213b17d..fd752524814d 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -75,6 +75,18 @@ config TYPEC_HD3SS3220
 	  If you choose to build this driver as a dynamically linked module, the
 	  module will be called hd3ss3220.ko.
 
+config TYPEC_TUSB320XA
+	tristate "TI TUSB320xA Type-C DRP Port controller driver"
+	depends on USB_ROLE_SWITCH
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say Y or M here if your system has a TUSB320xA Type-C DRP port
+	  controller.
+
+	  If you choose to build this driver as a dynamically linked module, the
+	  module will be called tusb320xa.ko.
+
 config TYPEC_STUSB160X
 	tristate "STMicroelectronics STUSB160x Type-C controller driver"
 	depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 43626acc0aaf..493e4796ca65 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)	+= tipd/
 obj-$(CONFIG_TYPEC_HD3SS3220)	+= hd3ss3220.o
+obj-$(CONFIG_TYPEC_TUSB320XA)	+= tusb320xa.o
 obj-$(CONFIG_TYPEC_QCOM_PMIC)	+= qcom-pmic-typec.o
 obj-$(CONFIG_TYPEC_STUSB160X) 	+= stusb160x.o
 obj-$(CONFIG_TYPEC_RT1719)	+= rt1719.o
diff --git a/drivers/usb/typec/tusb320xa.c b/drivers/usb/typec/tusb320xa.c
new file mode 100644
index 000000000000..b955143e24fc
--- /dev/null
+++ b/drivers/usb/typec/tusb320xa.c
@@ -0,0 +1,517 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI TUSB320LA/TUSB320HA Type-C DRP Port controller driver
+ *
+ * Based on the drivers/extcon/extcon-tusb320.c driver by Michael Auchter.
+ *
+ * Copyright (c) 2021 Alvin Šipraga <alsi@bang-olufsen.dk>
+ * Copyright (c) 2020 Michael Auchter <michael.auchter@ni.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/usb/role.h>
+#include <linux/usb/typec.h>
+#include <linux/usb/typec_altmode.h>
+
+#define TUSB320XA_REG8				0x08
+#define TUSB320XA_REG8_ACCESSORY_CONNECTED	GENMASK(3, 1)
+
+#define TUSB320XA_REG9				0x09
+#define TUSB320XA_REG9_ATTACHED_STATE		GENMASK(7, 6)
+#define TUSB320XA_REG9_CABLE_DIR		BIT(5)
+#define TUSB320XA_REG9_INTERRUPT_STATUS		BIT(4)
+
+#define TUSB320XA_REGA				0x0A
+#define TUSB320XA_REGA_MODE_SELECT		GENMASK(5, 4)
+#define TUSB320XA_REGA_I2C_SOFT_RESET		BIT(3)
+#define TUSB320XA_REGA_SOURCE_PREF		GENMASK(2, 1)
+#define TUSB320XA_REGA_DISABLE_TERM		BIT(0)
+
+enum tusb320xa_accessory {
+	TUSB320XA_ACCESSORY_NONE = 0,
+	/* 0b001 ~ 0b011 are reserved */
+	TUSB320XA_ACCESSORY_AUDIO = 4,
+	TUSB320XA_ACCESSORY_AUDIO_CHARGETHRU = 5,
+	TUSB320XA_ACCESSORY_DEBUG_DFP = 6,
+	TUSB320XA_ACCESSORY_DEBUG_UFP = 7,
+};
+
+static const char *const tusb320xa_accessories[] = {
+	[TUSB320XA_ACCESSORY_NONE]             = "none",
+	[TUSB320XA_ACCESSORY_AUDIO]            = "audio",
+	[TUSB320XA_ACCESSORY_AUDIO_CHARGETHRU] = "audio with charge thru",
+	[TUSB320XA_ACCESSORY_DEBUG_DFP]        = "debug while DFP",
+	[TUSB320XA_ACCESSORY_DEBUG_UFP]        = "debug while UFP",
+};
+
+enum tusb320xa_attached_state {
+	TUSB320XA_ATTACHED_STATE_NONE = 0,
+	TUSB320XA_ATTACHED_STATE_DFP = 1,
+	TUSB320XA_ATTACHED_STATE_UFP = 2,
+	TUSB320XA_ATTACHED_STATE_ACC = 3,
+};
+
+static const char *const tusb320xa_attached_states[] = {
+	[TUSB320XA_ATTACHED_STATE_NONE] = "not attached",
+	[TUSB320XA_ATTACHED_STATE_DFP]  = "downstream facing port",
+	[TUSB320XA_ATTACHED_STATE_UFP]  = "upstream facing port",
+	[TUSB320XA_ATTACHED_STATE_ACC]  = "accessory",
+};
+
+enum tusb320xa_cable_dir {
+	TUSB320XA_CABLE_DIR_CC1 = 0,
+	TUSB320XA_CABLE_DIR_CC2 = 1,
+};
+
+static const char *const tusb320xa_cable_directions[] = {
+	[TUSB320XA_CABLE_DIR_CC1] = "CC1",
+	[TUSB320XA_CABLE_DIR_CC2] = "CC2",
+};
+
+enum tusb320xa_mode {
+	TUSB320XA_MODE_PORT = 0,
+	TUSB320XA_MODE_UFP = 1,
+	TUSB320XA_MODE_DFP = 2,
+	TUSB320XA_MODE_DRP = 3,
+};
+
+enum tusb320xa_source_pref {
+	TUSB320XA_SOURCE_PREF_DRP = 0,
+	TUSB320XA_SOURCE_PREF_TRY_SNK = 1,
+	/* 0b10 is reserved */
+	TUSB320XA_SOURCE_PREF_TRY_SRC = 3,
+};
+
+struct tusb320xa {
+	struct device *dev;
+	struct regmap *regmap;
+	struct typec_port *port;
+	struct usb_role_switch *role_sw;
+	struct mutex lock;
+	int irq;
+};
+
+static int tusb320xa_check_signature(struct tusb320xa *tusb)
+{
+	static const char sig[] = { '\0', 'T', 'U', 'S', 'B', '3', '2', '0' };
+	unsigned int val;
+	int i, ret;
+
+	mutex_lock(&tusb->lock);
+
+	for (i = 0; i < sizeof(sig); i++) {
+		ret = regmap_read(tusb->regmap, sizeof(sig) - 1 - i, &val);
+		if (ret)
+			goto done;
+
+		if (val != sig[i]) {
+			dev_err(tusb->dev, "signature mismatch!\n");
+			ret = -ENODEV;
+			goto done;
+		}
+	}
+
+done:
+	mutex_unlock(&tusb->lock);
+
+	return ret;
+}
+
+static int tusb320xa_reset(struct tusb320xa *tusb)
+{
+	int ret;
+
+	mutex_lock(&tusb->lock);
+
+	/* Disable CC state machine */
+	ret = regmap_write_bits(tusb->regmap, TUSB320XA_REGA,
+				TUSB320XA_REGA_DISABLE_TERM,
+				FIELD_PREP(TUSB320XA_REGA_DISABLE_TERM, 1));
+	if (ret)
+		goto done;
+
+	/* Set to DRP by default, overriding any hardwired PORT setting */
+	ret = regmap_write_bits(tusb->regmap, TUSB320XA_REGA,
+				TUSB320XA_REGA_MODE_SELECT,
+				FIELD_PREP(TUSB320XA_REGA_MODE_SELECT,
+					   TUSB320XA_MODE_DRP));
+	if (ret)
+		goto done;
+
+	/* Wait 5 ms per datasheet specification */
+	usleep_range(5000, 10000);
+
+	/* Perform soft reset */
+	ret = regmap_write_bits(tusb->regmap, TUSB320XA_REGA,
+				TUSB320XA_REGA_I2C_SOFT_RESET,
+				FIELD_PREP(TUSB320XA_REGA_I2C_SOFT_RESET, 1));
+	if (ret)
+		goto done;
+
+	/* Wait 95 ms for chip to reset per datasheet specification */
+	msleep(95);
+
+	/* Clear any old interrupt status bit */
+	ret = regmap_write_bits(tusb->regmap, TUSB320XA_REG9,
+				TUSB320XA_REG9_INTERRUPT_STATUS,
+				FIELD_PREP(TUSB320XA_REG9_INTERRUPT_STATUS, 1));
+	if (ret)
+		goto done;
+
+	/* Re-enable CC state machine */
+	ret = regmap_write_bits(tusb->regmap, TUSB320XA_REGA,
+				TUSB320XA_REGA_DISABLE_TERM,
+				FIELD_PREP(TUSB320XA_REGA_DISABLE_TERM, 0));
+	if (ret)
+		goto done;
+
+done:
+	mutex_unlock(&tusb->lock);
+
+	return ret;
+}
+
+static int tusb320xa_sync_state(struct tusb320xa *tusb)
+{
+	enum tusb320xa_attached_state attached_state;
+	enum tusb320xa_cable_dir cable_dir;
+	enum tusb320xa_accessory accessory;
+	enum typec_orientation typec_orientation;
+	enum typec_data_role typec_data_role;
+	int typec_mode;
+	enum usb_role usb_role;
+	unsigned int reg8;
+	unsigned int reg9;
+	int ret;
+
+	ret = regmap_read(tusb->regmap, TUSB320XA_REG8, &reg8);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(tusb->regmap, TUSB320XA_REG9, &reg9);
+	if (ret)
+		return ret;
+
+	attached_state = FIELD_GET(TUSB320XA_REG9_ATTACHED_STATE, reg9);
+	cable_dir = FIELD_GET(TUSB320XA_REG9_CABLE_DIR, reg9);
+	accessory = FIELD_GET(TUSB320XA_REG8_ACCESSORY_CONNECTED, reg8);
+
+	dev_dbg(tusb->dev,
+		"attached state: %s, cable direction: %s, accessory: %s\n",
+		tusb320xa_attached_states[attached_state],
+		tusb320xa_cable_directions[cable_dir],
+		tusb320xa_accessories[accessory]);
+
+	if (cable_dir == TUSB320XA_CABLE_DIR_CC1)
+		typec_orientation = TYPEC_ORIENTATION_NORMAL;
+	else
+		typec_orientation = TYPEC_ORIENTATION_REVERSE;
+
+	switch (attached_state) {
+	case TUSB320XA_ATTACHED_STATE_NONE:
+		typec_orientation = TYPEC_ORIENTATION_NONE;
+		typec_data_role = TYPEC_HOST;
+		typec_mode = TYPEC_STATE_USB;
+		usb_role = USB_ROLE_NONE;
+		break;
+	case TUSB320XA_ATTACHED_STATE_DFP:
+		typec_data_role = TYPEC_HOST;
+		typec_mode = TYPEC_STATE_USB;
+		usb_role = USB_ROLE_HOST;
+		break;
+	case TUSB320XA_ATTACHED_STATE_UFP:
+		typec_data_role = TYPEC_DEVICE;
+		typec_mode = TYPEC_STATE_USB;
+		usb_role = USB_ROLE_DEVICE;
+		break;
+	case TUSB320XA_ATTACHED_STATE_ACC:
+		typec_data_role = TYPEC_HOST;
+		usb_role = USB_ROLE_HOST;
+
+		if (accessory == TUSB320XA_ACCESSORY_AUDIO ||
+		    accessory == TUSB320XA_ACCESSORY_AUDIO_CHARGETHRU) {
+			typec_mode = TYPEC_MODE_AUDIO;
+		} else if (accessory == TUSB320XA_ACCESSORY_DEBUG_DFP ||
+			   accessory == TUSB320XA_ACCESSORY_DEBUG_UFP) {
+			typec_mode = TYPEC_MODE_DEBUG;
+		} else {
+			dev_warn(tusb->dev, "unknown accessory type: %d\n",
+				 accessory);
+			typec_mode = TYPEC_STATE_USB;
+		}
+
+		break;
+	}
+
+	typec_set_orientation(tusb->port, typec_orientation);
+	typec_set_data_role(tusb->port, typec_data_role);
+	typec_set_mode(tusb->port, typec_mode);
+	usb_role_switch_set_role(tusb->role_sw, usb_role);
+
+	return ret;
+}
+
+static int tusb320xa_set_source_pref(struct tusb320xa *tusb,
+				     enum tusb320xa_source_pref pref)
+{
+	int ret;
+
+	mutex_lock(&tusb->lock);
+
+	ret = regmap_update_bits(tusb->regmap, TUSB320XA_REGA,
+				 TUSB320XA_REGA_SOURCE_PREF,
+				 FIELD_PREP(TUSB320XA_REGA_SOURCE_PREF, pref));
+
+	mutex_unlock(&tusb->lock);
+
+	return ret;
+}
+
+static int tusb320xa_set_mode(struct tusb320xa *tusb, enum tusb320xa_mode mode)
+{
+	int ret;
+
+	mutex_lock(&tusb->lock);
+
+	/* Disable CC state machine */
+	ret = regmap_write_bits(tusb->regmap, TUSB320XA_REGA,
+				TUSB320XA_REGA_DISABLE_TERM,
+				FIELD_PREP(TUSB320XA_REGA_DISABLE_TERM, 1));
+	if (ret)
+		goto done;
+
+	/* Set the desired port mode */
+	ret = regmap_write_bits(tusb->regmap, TUSB320XA_REGA,
+				TUSB320XA_REGA_MODE_SELECT,
+				FIELD_PREP(TUSB320XA_REGA_MODE_SELECT, mode));
+	if (ret)
+		goto done;
+
+	/* Wait 5 ms per datasheet specification */
+	usleep_range(5000, 10000);
+
+	/* Re-enable CC state machine */
+	ret = regmap_write_bits(tusb->regmap, TUSB320XA_REGA,
+				TUSB320XA_REGA_DISABLE_TERM,
+				FIELD_PREP(TUSB320XA_REGA_DISABLE_TERM, 0));
+	if (ret)
+		goto done;
+
+done:
+	mutex_unlock(&tusb->lock);
+
+	return ret;
+}
+
+static int tusb320xa_try_role(struct typec_port *port, int role)
+{
+	struct tusb320xa *tusb = typec_get_drvdata(port);
+	enum tusb320xa_source_pref pref;
+
+	switch (role) {
+	case TYPEC_NO_PREFERRED_ROLE:
+		pref = TUSB320XA_SOURCE_PREF_DRP;
+		break;
+	case TYPEC_SINK:
+		pref = TUSB320XA_SOURCE_PREF_TRY_SNK;
+		break;
+	case TYPEC_SOURCE:
+		pref = TUSB320XA_SOURCE_PREF_TRY_SRC;
+		break;
+	default:
+		dev_warn(tusb->dev, "unknown port role %d\n", role);
+		return -EINVAL;
+	}
+
+	return tusb320xa_set_source_pref(tusb, pref);
+}
+
+static int tusb320xa_port_type_set(struct typec_port *port,
+				   enum typec_port_type type)
+{
+	struct tusb320xa *tusb = typec_get_drvdata(port);
+	enum tusb320xa_mode mode;
+
+	switch (type) {
+	case TYPEC_PORT_SRC:
+		mode = TUSB320XA_MODE_DFP;
+		break;
+	case TYPEC_PORT_SNK:
+		mode = TUSB320XA_MODE_UFP;
+		break;
+	case TYPEC_PORT_DRP:
+		mode = TUSB320XA_MODE_DRP;
+		break;
+	default:
+		dev_warn(tusb->dev, "unknown port type %d\n", type);
+		return -EINVAL;
+	}
+
+	return tusb320xa_set_mode(tusb, mode);
+}
+
+static const struct typec_operations tusb320xa_ops = {
+	.try_role = tusb320xa_try_role,
+	.port_type_set = tusb320xa_port_type_set,
+};
+
+static irqreturn_t tusb320xa_irq_handler_thread(int irq, void *dev_id)
+{
+	struct tusb320xa *tusb = dev_id;
+	unsigned int reg;
+	int ret;
+
+	mutex_lock(&tusb->lock);
+
+	/* Check interrupt status bit */
+	ret = regmap_read(tusb->regmap, TUSB320XA_REG9, &reg);
+	if (ret)
+		goto unhandled;
+
+	if (!(reg & TUSB320XA_REG9_INTERRUPT_STATUS))
+		goto unhandled;
+
+	/* Clear interrupt status bit */
+	ret = regmap_write(tusb->regmap, TUSB320XA_REG9, reg);
+	if (ret)
+		goto unhandled;
+
+	ret = tusb320xa_sync_state(tusb);
+	if (ret)
+		dev_err_ratelimited(tusb->dev,
+				    "failed to sync state in irq: %d\n", ret);
+
+	mutex_unlock(&tusb->lock);
+
+	return IRQ_HANDLED;
+
+unhandled:
+	mutex_unlock(&tusb->lock);
+
+	return IRQ_NONE;
+}
+
+static const struct regmap_config tusb320xa_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.disable_locking = true,
+};
+
+void tusb320xa_action_role_sw_put(void *data)
+{
+	struct usb_role_switch *role_sw = data;
+
+	usb_role_switch_put(role_sw);
+}
+
+void tusb320xa_action_unregister_port(void *data)
+{
+	struct typec_port *port = data;
+
+	typec_unregister_port(port);
+}
+
+static int tusb320xa_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	struct tusb320xa *tusb;
+	struct typec_capability typec_cap = {};
+	int ret;
+
+	tusb = devm_kzalloc(&client->dev, sizeof(*tusb), GFP_KERNEL);
+	if (!tusb)
+		return -ENOMEM;
+
+	tusb->dev = &client->dev;
+	mutex_init(&tusb->lock);
+
+	tusb->irq = client->irq;
+	if (!tusb->irq)
+		return -EINVAL;
+
+	tusb->regmap = devm_regmap_init_i2c(client, &tusb320xa_regmap_config);
+	if (IS_ERR(tusb->regmap))
+		return PTR_ERR(tusb->regmap);
+
+	tusb->role_sw = usb_role_switch_get(tusb->dev);
+	if (IS_ERR(tusb->role_sw))
+		return PTR_ERR(tusb->role_sw);
+
+	ret = devm_add_action_or_reset(tusb->dev, tusb320xa_action_role_sw_put,
+				       tusb->role_sw);
+	if (ret)
+		return ret;
+
+	ret = tusb320xa_check_signature(tusb);
+	if (ret)
+		return ret;
+
+	ret = tusb320xa_reset(tusb);
+	if (ret)
+		return ret;
+
+	typec_cap.type = TYPEC_PORT_DRP;
+	typec_cap.data = TYPEC_PORT_DRD;
+	typec_cap.revision = USB_TYPEC_REV_1_1;
+	typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+	typec_cap.accessory[TYPEC_ACCESSORY_NONE] = 1;
+	typec_cap.accessory[TYPEC_ACCESSORY_AUDIO] = 1;
+	typec_cap.accessory[TYPEC_ACCESSORY_DEBUG] = 1;
+	typec_cap.orientation_aware = 1;
+	typec_cap.fwnode = dev_fwnode(tusb->dev);
+	typec_cap.driver_data = tusb;
+	typec_cap.ops = &tusb320xa_ops;
+
+	tusb->port = typec_register_port(tusb->dev, &typec_cap);
+	if (IS_ERR(tusb->port))
+		return PTR_ERR(tusb->port);
+
+	ret = devm_add_action_or_reset(tusb->dev,
+				       tusb320xa_action_unregister_port,
+				       tusb->port);
+	if (ret)
+		return ret;
+
+	ret = devm_request_threaded_irq(tusb->dev, tusb->irq, NULL,
+					tusb320xa_irq_handler_thread,
+					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
+					"tusb320xa", tusb);
+	if (ret)
+		return ret;
+
+	i2c_set_clientdata(client, tusb);
+
+	return 0;
+}
+
+static const struct of_device_id tusb320xa_dt_match[] = {
+	{
+		.compatible = "ti,tusb320la",
+	},
+	{
+		.compatible = "ti,tusb320ha",
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tusb320xa_dt_match);
+
+static struct i2c_driver tusb320xa_driver = {
+	.driver	= {
+		.name = "tusb320xa",
+		.of_match_table = tusb320xa_dt_match,
+	},
+	.probe  = tusb320xa_probe,
+};
+
+module_i2c_driver(tusb320xa_driver);
+
+MODULE_AUTHOR("Alvin Šipraga <alsi@bang-olufsen.dk>");
+MODULE_DESCRIPTION("TI TUSB320xA USB Type-C controller driver");
+MODULE_LICENSE("GPL v2");
-- 
2.35.1


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

* [PATCH 4/4] usb: typec: mux: add TS5USBA224 driver
  2022-03-01 13:20 [PATCH 0/4] usb: typec: add drivers for TUSB320xA and TS5USBA224 Alvin Šipraga
                   ` (2 preceding siblings ...)
  2022-03-01 13:20 ` [PATCH 3/4] usb: typec: add TUSB320xA driver Alvin Šipraga
@ 2022-03-01 13:20 ` Alvin Šipraga
  3 siblings, 0 replies; 15+ messages in thread
From: Alvin Šipraga @ 2022-03-01 13:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Heikki Krogerus, �ipraga
  Cc: linux-usb, devicetree, linux-kernel

From: Alvin Šipraga <alsi@bang-olufsen.dk>

The TS5USBA224 USB High Speed/Audio switch mux is typically used
together with a Type-C port controller such as the TUSB320LAI. Below is
a simplified typical application on an embedded platform:

 ┌─────────────────────┐                      ┌───────────────────┐
 │                     │          I2C         │                   │
 │                     ├──────────────────────┤ I2C               │
 │      TUSB320LAI     │         INT_N        │                   │
 │                     ├──────────────────────┤     System-on-    │
 │                     │                      │             Chip  │
 │ CC1 CC2             │           ┌──────────┤ GPIO              │
 └──┬───┬──────────────┘           │          │                   │
    │   │                          │          │       USBOTG      │
    │   │                          │          │    VBUS   D+  D-  │
    │   │                          │          └──────┬─────┬───┬──┘
    │   │   ┌──────────────────────│─────────────────┘     │   │
    │   │   │          ┌───────────┴──┐                    │   │
    │   │   │          │        A_SEL │                    │   │
    │   │   │          │              │                    │   │
    │   │   │          │           D+ ├────────────────────┘   │
    │   │   │          │              │                        │
    │   │   │   ┌──────┤ D+/L      D- ├────────────────────────┘
    │   │   │   │      │              │
    │   │   │   │   ┌──┤ D-/R         │                   Line_L
    │   │   │   │   │  │            L ├─────────────────────────
    ┴   ┴   ┴   ┴   ┴  │              │                   Line_R
     USB Type-C port   │            R ├─────────────────────────
                       │              │
                       │  TS5USBA224  │
                       └──────────────┘

This driver controls the TS5USBA224 via the A_SEL pin. In addition, it
registers itself as a Type-C mux device. In the above scenario, the
driver may be notified via the port controller driver if the CC pins
indicate that an Audio Accessory is connected or not.

Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 drivers/usb/typec/mux/Kconfig      |  10 +++
 drivers/usb/typec/mux/Makefile     |   1 +
 drivers/usb/typec/mux/ts5usba224.c | 102 +++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+)
 create mode 100644 drivers/usb/typec/mux/ts5usba224.c

diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
index edead555835e..82c25b117652 100644
--- a/drivers/usb/typec/mux/Kconfig
+++ b/drivers/usb/typec/mux/Kconfig
@@ -19,4 +19,14 @@ config TYPEC_MUX_INTEL_PMC
 	  control the USB role switch and also the multiplexer/demultiplexer
 	  switches used with USB Type-C Alternate Modes.
 
+config TYPEC_MUX_TS5USBA224
+	tristate "TI TS5USBA224 USB High Speed/Audio switch mux driver"
+	select GPIOLIB
+	help
+	  Say Y or M if your system has a TI TS5USBA224 USB High Speed/Audio
+	  switch mux controller paired with a USB Type-C port controller.
+
+	  If you choose to build this driver as a dynamically linked module, the
+	  module will be called ts5usba224.ko.
+
 endmenu
diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile
index 280a6f553115..4b5f318656a0 100644
--- a/drivers/usb/typec/mux/Makefile
+++ b/drivers/usb/typec/mux/Makefile
@@ -2,3 +2,4 @@
 
 obj-$(CONFIG_TYPEC_MUX_PI3USB30532)	+= pi3usb30532.o
 obj-$(CONFIG_TYPEC_MUX_INTEL_PMC)	+= intel_pmc_mux.o
+obj-$(CONFIG_TYPEC_MUX_TS5USBA224)	+= ts5usba224.o
diff --git a/drivers/usb/typec/mux/ts5usba224.c b/drivers/usb/typec/mux/ts5usba224.c
new file mode 100644
index 000000000000..4935dc8a0725
--- /dev/null
+++ b/drivers/usb/typec/mux/ts5usba224.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * TI TS5USBA224 USB 2.0/audio switch mux driver
+ *
+ * Copyright (c) 2021 Alvin Šipraga <alsi@bang-olufsen.dk>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/usb/typec_altmode.h>
+#include <linux/usb/typec_mux.h>
+
+struct ts5usba224 {
+	struct device *dev;
+	struct typec_mux *mux;
+	struct gpio_desc *a_sel;
+};
+
+static int ts5usba224_mux_set(struct typec_mux *mux,
+			      struct typec_mux_state *state)
+{
+	struct ts5usba224 *chip = typec_mux_get_drvdata(mux);
+
+	switch (state->mode) {
+	case TYPEC_MODE_AUDIO:
+		gpiod_set_value_cansleep(chip->a_sel, 1);
+		dev_dbg(chip->dev, "audio switch enabled\n");
+		break;
+	case TYPEC_STATE_USB:
+	case TYPEC_MODE_USB2:
+	default:
+		dev_dbg(chip->dev, "audio switch disabled\n");
+		gpiod_set_value_cansleep(chip->a_sel, 0);
+		break;
+	}
+
+	return 0;
+}
+
+static int ts5usba224_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct typec_mux_desc mux_desc = {};
+	struct ts5usba224 *chip;
+	int ret;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->dev = dev;
+
+	chip->a_sel = devm_gpiod_get(dev, "asel", GPIOD_OUT_LOW);
+	if (IS_ERR(chip->a_sel)) {
+		ret = PTR_ERR(chip->a_sel);
+		return dev_err_probe(dev, ret, "failed to get A_SEL GPIO\n");
+	}
+
+	mux_desc.drvdata = chip;
+	mux_desc.fwnode = dev->fwnode;
+	mux_desc.set = ts5usba224_mux_set;
+
+	chip->mux = typec_mux_register(dev, &mux_desc);
+	if (IS_ERR(chip->mux))
+		return PTR_ERR(chip->mux);
+
+	platform_set_drvdata(pdev, chip);
+
+	return 0;
+}
+
+static int ts5usba224_remove(struct platform_device *pdev)
+{
+	struct ts5usba224 *chip = platform_get_drvdata(pdev);
+
+	typec_mux_unregister(chip->mux);
+
+	return 0;
+}
+
+static const struct of_device_id ts5usba224_of_match[] = {
+	{ .compatible = "ti,ts5usba224", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ts5usba224_of_match);
+
+static struct platform_driver ts5usba224_driver = {
+	.driver = {
+		.name = "ts5usba224",
+		.of_match_table = of_match_ptr(ts5usba224_of_match),
+	},
+	.probe  = ts5usba224_probe,
+	.remove = ts5usba224_remove,
+};
+
+module_platform_driver(ts5usba224_driver);
+
+MODULE_AUTHOR("Alvin Šipraga <alsi@bang-olufsen.dk>");
+MODULE_DESCRIPTION("TI TS5USBA224 mux driver");
+MODULE_LICENSE("GPL");
-- 
2.35.1


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

* Re: [PATCH 3/4] usb: typec: add TUSB320xA driver
  2022-03-01 13:20 ` [PATCH 3/4] usb: typec: add TUSB320xA driver Alvin Šipraga
@ 2022-03-01 16:58   ` kernel test robot
  2022-03-01 17:20       ` Alvin Šipraga
  2022-03-07 14:36   ` Heikki Krogerus
  1 sibling, 1 reply; 15+ messages in thread
From: kernel test robot @ 2022-03-01 16:58 UTC (permalink / raw)
  To: Alvin Šipraga, Greg Kroah-Hartman, Rob Herring,
	Heikki Krogerus, �ipraga
  Cc: kbuild-all, linux-usb, devicetree, linux-kernel

Hi "Alvin,

I love your patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on robh/for-next v5.17-rc6 next-20220301]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Alvin-ipraga/usb-typec-add-drivers-for-TUSB320xA-and-TS5USBA224/20220301-212251
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220302/202203020056.igXsHYzi-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c60eb902fdeaef99cf2c10e84e0369ee6787753a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Alvin-ipraga/usb-typec-add-drivers-for-TUSB320xA-and-TS5USBA224/20220301-212251
        git checkout c60eb902fdeaef99cf2c10e84e0369ee6787753a
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/usb/typec/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/usb/typec/tusb320xa.c:407:6: warning: no previous prototype for 'tusb320xa_action_role_sw_put' [-Wmissing-prototypes]
     407 | void tusb320xa_action_role_sw_put(void *data)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/usb/typec/tusb320xa.c:414:6: warning: no previous prototype for 'tusb320xa_action_unregister_port' [-Wmissing-prototypes]
     414 | void tusb320xa_action_unregister_port(void *data)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/tusb320xa_action_role_sw_put +407 drivers/usb/typec/tusb320xa.c

   406	
 > 407	void tusb320xa_action_role_sw_put(void *data)
   408	{
   409		struct usb_role_switch *role_sw = data;
   410	
   411		usb_role_switch_put(role_sw);
   412	}
   413	
 > 414	void tusb320xa_action_unregister_port(void *data)
   415	{
   416		struct typec_port *port = data;
   417	
   418		typec_unregister_port(port);
   419	}
   420	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH 3/4] usb: typec: add TUSB320xA driver
  2022-03-01 16:58   ` kernel test robot
@ 2022-03-01 17:20       ` Alvin Šipraga
  0 siblings, 0 replies; 15+ messages in thread
From: Alvin Šipraga @ 2022-03-01 17:20 UTC (permalink / raw)
  To: kernel test robot
  Cc: Alvin Šipraga, Greg Kroah-Hartman, Rob Herring,
	Heikki Krogerus, kbuild-all, linux-usb, devicetree, linux-kernel

kernel test robot <lkp@intel.com> writes:

> Hi "Alvin,
>
> I love your patch! Perhaps something to improve:
>

<snip>

>
> All warnings (new ones prefixed by >>):
>
>>> drivers/usb/typec/tusb320xa.c:407:6: warning: no previous prototype for 'tusb320xa_action_role_sw_put' [-Wmissing-prototypes]
>      407 | void tusb320xa_action_role_sw_put(void *data)
>          |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/usb/typec/tusb320xa.c:414:6: warning: no previous prototype for 'tusb320xa_action_unregister_port' [-Wmissing-prototypes]
>      414 | void tusb320xa_action_unregister_port(void *data)
>          |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Ah yes, these should be static, will fix in v2. Thank you, test robot.

>
>
> vim +/tusb320xa_action_role_sw_put +407 drivers/usb/typec/tusb320xa.c
>
>    406	
>  > 407	void tusb320xa_action_role_sw_put(void *data)
>    408	{
>    409		struct usb_role_switch *role_sw = data;
>    410	
>    411		usb_role_switch_put(role_sw);
>    412	}
>    413	
>  > 414	void tusb320xa_action_unregister_port(void *data)
>    415	{
>    416		struct typec_port *port = data;
>    417	
>    418		typec_unregister_port(port);
>    419	}
>    420	
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH 3/4] usb: typec: add TUSB320xA driver
@ 2022-03-01 17:20       ` Alvin Šipraga
  0 siblings, 0 replies; 15+ messages in thread
From: Alvin Šipraga @ 2022-03-01 17:20 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

kernel test robot <lkp@intel.com> writes:

> Hi "Alvin,
>
> I love your patch! Perhaps something to improve:
>

<snip>

>
> All warnings (new ones prefixed by >>):
>
>>> drivers/usb/typec/tusb320xa.c:407:6: warning: no previous prototype for 'tusb320xa_action_role_sw_put' [-Wmissing-prototypes]
>      407 | void tusb320xa_action_role_sw_put(void *data)
>          |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/usb/typec/tusb320xa.c:414:6: warning: no previous prototype for 'tusb320xa_action_unregister_port' [-Wmissing-prototypes]
>      414 | void tusb320xa_action_unregister_port(void *data)
>          |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Ah yes, these should be static, will fix in v2. Thank you, test robot.

>
>
> vim +/tusb320xa_action_role_sw_put +407 drivers/usb/typec/tusb320xa.c
>
>    406	
>  > 407	void tusb320xa_action_role_sw_put(void *data)
>    408	{
>    409		struct usb_role_switch *role_sw = data;
>    410	
>    411		usb_role_switch_put(role_sw);
>    412	}
>    413	
>  > 414	void tusb320xa_action_unregister_port(void *data)
>    415	{
>    416		struct typec_port *port = data;
>    417	
>    418		typec_unregister_port(port);
>    419	}
>    420	
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

* Re: [PATCH 1/4] dt-bindings: usb: add TUSB320xA Type-C port controller
  2022-03-01 13:20 ` [PATCH 1/4] dt-bindings: usb: add TUSB320xA Type-C port controller Alvin Šipraga
@ 2022-03-02 18:16   ` Rob Herring
  0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2022-03-02 18:16 UTC (permalink / raw)
  To: Alvin Šipraga
  Cc: Greg Kroah-Hartman, Heikki Krogerus, �ipraga, linux-usb,
	devicetree, linux-kernel

On Tue, Mar 01, 2022 at 02:20:05PM +0100, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
> 
> The TUSB320xA is a non-PD Type-C port controller managed over I2C.
> 
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> ---
>  .../devicetree/bindings/usb/ti,tusb320xa.yaml | 78 +++++++++++++++++++
>  1 file changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ti,tusb320xa.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/ti,tusb320xa.yaml b/Documentation/devicetree/bindings/usb/ti,tusb320xa.yaml
> new file mode 100644
> index 000000000000..a93d53ddd01c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ti,tusb320xa.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/ti,tusb320xa.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TUSB320xA Type-C port controller DT bindings
> +
> +description:
> +  The Texas Instruments TUSB320xA is a USB Type-C port controller which
> +  supports role and plug orientation detection using the CC pins. It is
> +  compatible with the USB Type-C Cable and Connector Specification v1.1.
> +
> +maintainers:
> +  - Alvin Šipraga <alsi@bang-olufsen.dk>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,tusb320la
> +      - ti,tusb320ha
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    patternProperties:
> +      '^port@':

Exact port numbers need to be defined. What does port@0 represent? 
port@1?

> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          OF graph bindings modelling any "usb-role-switch" or "accessory" mux.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      tcpc@47 {
> +        compatible = "ti,tusb320la";
> +        reg = <0x47>;
> +        interrupt-parent = <&gpio5>;
> +        interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
> +
> +        ports {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          port@0 {
> +            reg = <0>;
> +            typec1_mux: endpoint {
> +              remote-endpoint = <&usb_audio_mux1>;
> +            };
> +          };
> +
> +          port@1 {
> +            reg = <1>;
> +            typec1_dr_sw: endpoint {
> +              remote-endpoint = <&usbotg1_drd_sw>;
> +            };
> +          };
> +        };
> +      };
> +    };
> -- 
> 2.35.1
> 

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

* Re: [PATCH 2/4] dt-bindings: usb: add TS5USBA224 USB/Audio switch mux
  2022-03-01 13:20 ` [PATCH 2/4] dt-bindings: usb: add TS5USBA224 USB/Audio switch mux Alvin Šipraga
@ 2022-03-02 18:21   ` Rob Herring
  2022-03-02 18:58     ` Alvin Šipraga
  0 siblings, 1 reply; 15+ messages in thread
From: Rob Herring @ 2022-03-02 18:21 UTC (permalink / raw)
  To: Alvin Šipraga
  Cc: Greg Kroah-Hartman, Heikki Krogerus, �ipraga, linux-usb,
	devicetree, linux-kernel

On Tue, Mar 01, 2022 at 02:20:06PM +0100, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
> 
> The TS5USBA224 is a USB High Speed/Audio switch mux IC controlled via
> GPIO. It is typically composed with a Type-C port controller with Audio
> Accessory mode detection.
> 
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> ---
>  .../bindings/usb/ti,ts5usba224.yaml           | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml b/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
> new file mode 100644
> index 000000000000..0a488b961906
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/ti,ts5usba224.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TS5USBA224 USB 2.0 High Speed and Audio mux DT bindings
> +
> +description:
> +  The Texas Instruments TS5USBA224 is a double-pole, double throw
> +  (DPDT) multiplexer that includes a low-distortion audio switch and a
> +  USB 2.0 High Speed switch in the same package.
> +
> +maintainers:
> +  - Alvin Šipraga <alsi@bang-olufsen.dk>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,ts5usba224
> +
> +  asel-gpio:
> +    description: Output GPIO for A_SEL signal
> +    maxItems: 1
> +
> +  accessory:
> +    type: boolean
> +    description:
> +      Indicates that this is an Accessory Mode mux.
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description:
> +      OF graph bindings modelling a Type-C port controller.

What does that mean?

There's a couple of Type-C bindings on the list ATM. Without block 
diagrams of all the relevant components and the corresponding graph, I 
can't say whether any of this graph usage makes sense.

> +
> +required:
> +  - compatible
> +  - asel-gpio
> +  - accessory
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    usbaudiomux@0 {
> +      compatible = "ti,ts5usba224";
> +      asel-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
> +      accessory;
> +
> +      port {
> +        usb_audio_mux1: endpoint {
> +          remote-endpoint = <&typec1_mux>;
> +        };
> +      };
> +    };
> -- 
> 2.35.1
> 

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

* Re: [PATCH 2/4] dt-bindings: usb: add TS5USBA224 USB/Audio switch mux
  2022-03-02 18:21   ` Rob Herring
@ 2022-03-02 18:58     ` Alvin Šipraga
  0 siblings, 0 replies; 15+ messages in thread
From: Alvin Šipraga @ 2022-03-02 18:58 UTC (permalink / raw)
  To: Rob Herring
  Cc: Alvin Šipraga, Greg Kroah-Hartman, Heikki Krogerus,
	linux-usb, devicetree, linux-kernel

Rob Herring <robh@kernel.org> writes:

> On Tue, Mar 01, 2022 at 02:20:06PM +0100, Alvin Šipraga wrote:
>> From: Alvin Šipraga <alsi@bang-olufsen.dk>
>> 
>> The TS5USBA224 is a USB High Speed/Audio switch mux IC controlled via
>> GPIO. It is typically composed with a Type-C port controller with Audio
>> Accessory mode detection.
>> 
>> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
>> ---
>>  .../bindings/usb/ti,ts5usba224.yaml           | 56 +++++++++++++++++++
>>  1 file changed, 56 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml b/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
>> new file mode 100644
>> index 000000000000..0a488b961906
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
>> @@ -0,0 +1,56 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/usb/ti,ts5usba224.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TS5USBA224 USB 2.0 High Speed and Audio mux DT bindings
>> +
>> +description:
>> +  The Texas Instruments TS5USBA224 is a double-pole, double throw
>> +  (DPDT) multiplexer that includes a low-distortion audio switch and a
>> +  USB 2.0 High Speed switch in the same package.
>> +
>> +maintainers:
>> +  - Alvin Šipraga <alsi@bang-olufsen.dk>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ti,ts5usba224
>> +
>> +  asel-gpio:
>> +    description: Output GPIO for A_SEL signal
>> +    maxItems: 1
>> +
>> +  accessory:
>> +    type: boolean
>> +    description:
>> +      Indicates that this is an Accessory Mode mux.
>> +
>> +  port:
>> +    $ref: /schemas/graph.yaml#/properties/port
>> +    description:
>> +      OF graph bindings modelling a Type-C port controller.
>
> What does that mean?
>
> There's a couple of Type-C bindings on the list ATM. Without block 
> diagrams of all the relevant components and the corresponding graph, I 
> can't say whether any of this graph usage makes sense.

Thanks for your review. Maybe I need some help with this but I'll try
and explain according to my understanding.

The series adds two drivers:

- tusb320xa - a USB Type-C port controller driver
- ts5usba224 - a USB Type-C multiplexer (for Audio Accessory mode)

These handle different classes of device: typec (the port controller)
and typec_mux (the multiplexer) respectively.

The typec driver is responsible for determining what is connected on the
other end of the Type-C port. It informs the typec subsystem of what
role is detected (UFP or DFP), the orientation, and whether or not
certain accessories (Audio or Debug) are connected.

The typec_mux driver is interested in whether an Audio Accessory has
been connected to the Type-C port. Depending on whether it is or not, it
will assert a GPIO controlling the TS5USBA224 mux chip.

Additionally, an OTG capable USB controller might like to be informed so
that it can configure itself correctly as a host (and enable VBUS etc.)
or peripheral.

Here is a simplified block diagram with my i.MX8MM SoC:

     ┌─────────────────────┐                    ┌───────────────────┐
     │                     │          I2C       │                   │
     │                     ├────────────────────┤ I2C               │
     │      TUSB320LAI     │         INT_N      │                   │
     │                     ├────────────────────┤         i.MX8MM   │
     │                     │                    │                   │
     │ CC1 CC2             │           ┌────────┤ GPIO              │
     └──┬───┬──────────────┘           │        │                   │
        │   │                          │        │       USBOTG      │
        │   │                          │        │    VBUS   D+  D-  │
        │   │                          │        └──────┬─────┬───┬──┘
        │   │   ┌──────────────────────│───────────────┘     │   │
        │   │   │          ┌───────────┴──┐                  │   │
        │   │   │          │        A_SEL │                  │   │
        │   │   │          │              │                  │   │
        │   │   │          │           D+ ├──────────────────┘   │
        │   │   │          │              │                      │
        │   │   │   ┌──────┤ D+/L      D- ├──────────────────────┘
        │   │   │   │      │              │
        │   │   │   │   ┌──┤ D-/R         │                 Line_L
        │   │   │   │   │  │            L ├───────────────────────
        ┴   ┴   ┴   ┴   ┴  │              │                 Line_R
         USB Type-C port   │            R ├───────────────────────
                           │              │
                           │  TS5USBA224  │
                           └──────────────┘


Line_{L,R} here are handled by some alternative circuitry which is not
relevant to this discussion.

Now back to the graph: I noticed while reading the generic typec code
that it is walking the child nodes and looking for particular endpoints
when registering a typec port. If a remote endpoint has the 'accessory'
property, it will pick up the corresponding device and inform it when
the port mode changes. If the new mode is TYPEC_MODE_AUDIO (as a result
of an Audio Accessory being detected by the port), then the typec_mux
driver will assert the A_SEL GPIO, otherwise it will deassert it.

Since this worked pretty automatically, my understanding was that this
was the correct way to "connect" these two devices in the device tree.

For the USB host/peripheral role switching, I use the
usb_role_switch_get() API in the probe function of the tusb320xa driver
to similarly walk the device tree and find a remote endpoint with the
'usb-role-switch' property.

So it could be a misunderstanding on my part, but what I'm trying to do
here with the bindings is offer the option to connect the various
devices together via the graph bindings. They are strictly optional
though - one might have a non-OTG capable USB controller, or perhaps not
care about Audio Accessories, etc. - so I did not specify specific port
numbering. I can of course do that, but it will be an arbitrary choice.

I hope that explains a bit better what I'm trying to do here. I don't
claim to have got it right though. Do you have any comments? Maybe
Heikki can chip in and explain if this is the right way to look at
things.

Kind regards,
Alvin

>
>> +
>> +required:
>> +  - compatible
>> +  - asel-gpio
>> +  - accessory
>> +  - port
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    usbaudiomux@0 {
>> +      compatible = "ti,ts5usba224";
>> +      asel-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
>> +      accessory;
>> +
>> +      port {
>> +        usb_audio_mux1: endpoint {
>> +          remote-endpoint = <&typec1_mux>;
>> +        };
>> +      };
>> +    };
>> -- 
>> 2.35.1
>> 

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

* Re: [PATCH 3/4] usb: typec: add TUSB320xA driver
  2022-03-01 13:20 ` [PATCH 3/4] usb: typec: add TUSB320xA driver Alvin Šipraga
  2022-03-01 16:58   ` kernel test robot
@ 2022-03-07 14:36   ` Heikki Krogerus
  2022-03-07 22:17     ` Alvin Šipraga
  1 sibling, 1 reply; 15+ messages in thread
From: Heikki Krogerus @ 2022-03-07 14:36 UTC (permalink / raw)
  To: Alvin Šipraga
  Cc: Greg Kroah-Hartman, Rob Herring, �ipraga, linux-usb,
	devicetree, linux-kernel

Hi,

On Tue, Mar 01, 2022 at 02:20:07PM +0100, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
> 
> The TUSB320LA and TUSB320HA (or LAI, HAI) chips are I2C controlled
> non-PD Type-C port controllers. They support detection of cable
> orientation, port attachment state, and role, including Audio Accessory
> and Debug Accessory modes. Add a typec class driver for this family.
> 
> Note that there already exists an extcon driver for the TUSB320 (a
> slightly older revision that does not support setting role preference or
> disabling the CC state machine). This driver is loosely based on that
> one.

This looked mostly OK to me. There is one question below.

<snip>

> +static int tusb320xa_check_signature(struct tusb320xa *tusb)
> +{
> +	static const char sig[] = { '\0', 'T', 'U', 'S', 'B', '3', '2', '0' };
> +	unsigned int val;
> +	int i, ret;
> +
> +	mutex_lock(&tusb->lock);
> +
> +	for (i = 0; i < sizeof(sig); i++) {
> +		ret = regmap_read(tusb->regmap, sizeof(sig) - 1 - i, &val);
> +		if (ret)
> +			goto done;
> +
> +		if (val != sig[i]) {
> +			dev_err(tusb->dev, "signature mismatch!\n");
> +			ret = -ENODEV;
> +			goto done;
> +		}
> +	}
> +
> +done:
> +	mutex_unlock(&tusb->lock);
> +
> +	return ret;
> +}

Couldn't that be done with a single read?

        char sig[8];
        u64 val;

        strcpy(sig, "TUSB320")

        mutex_lock(&tusb->lock);

        ret = regmap_raw_read(tusb->regmap, 0, &val, sizeof(val));
        ...
        if (val != cpu_to_le64(*(u64 *)sig)) {
        ...

Something like that?

thanks,

-- 
heikki

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

* Re: [PATCH 3/4] usb: typec: add TUSB320xA driver
  2022-03-07 14:36   ` Heikki Krogerus
@ 2022-03-07 22:17     ` Alvin Šipraga
  2022-03-08 11:49       ` Heikki Krogerus
  0 siblings, 1 reply; 15+ messages in thread
From: Alvin Šipraga @ 2022-03-07 22:17 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Alvin Šipraga, Greg Kroah-Hartman, Rob Herring, linux-usb,
	devicetree, linux-kernel

Hi Heikki,

Heikki Krogerus <heikki.krogerus@linux.intel.com> writes:

> Hi,
>
> On Tue, Mar 01, 2022 at 02:20:07PM +0100, Alvin Šipraga wrote:
>> From: Alvin Šipraga <alsi@bang-olufsen.dk>
>> 
>> The TUSB320LA and TUSB320HA (or LAI, HAI) chips are I2C controlled
>> non-PD Type-C port controllers. They support detection of cable
>> orientation, port attachment state, and role, including Audio Accessory
>> and Debug Accessory modes. Add a typec class driver for this family.
>> 
>> Note that there already exists an extcon driver for the TUSB320 (a
>> slightly older revision that does not support setting role preference or
>> disabling the CC state machine). This driver is loosely based on that
>> one.
>
> This looked mostly OK to me. There is one question below.
>
> <snip>
>
>> +static int tusb320xa_check_signature(struct tusb320xa *tusb)
>> +{
>> +	static const char sig[] = { '\0', 'T', 'U', 'S', 'B', '3', '2', '0' };
>> +	unsigned int val;
>> +	int i, ret;
>> +
>> +	mutex_lock(&tusb->lock);
>> +
>> +	for (i = 0; i < sizeof(sig); i++) {
>> +		ret = regmap_read(tusb->regmap, sizeof(sig) - 1 - i, &val);
>> +		if (ret)
>> +			goto done;
>> +
>> +		if (val != sig[i]) {
>> +			dev_err(tusb->dev, "signature mismatch!\n");
>> +			ret = -ENODEV;
>> +			goto done;
>> +		}
>> +	}
>> +
>> +done:
>> +	mutex_unlock(&tusb->lock);
>> +
>> +	return ret;
>> +}
>
> Couldn't that be done with a single read?
>
>         char sig[8];
>         u64 val;
>
>         strcpy(sig, "TUSB320")
>
>         mutex_lock(&tusb->lock);
>
>         ret = regmap_raw_read(tusb->regmap, 0, &val, sizeof(val));
>         ...
>         if (val != cpu_to_le64(*(u64 *)sig)) {
>         ...
>
> Something like that?

I think it's a bit cryptic - are you sure it's worth it just to save 8
one-off regmap_read()s? I could also just remove this check... I see it
mostly as a courtesy to the user in case the I2C address in his device
tree mistakenly points to some other unsuspecting chip.

BTW, do you have any feedback on the device tree bindings of this
series? Rob had some questions and I am not sure that my proposed
bindings are fully aligned with the typec subsystem expectations. Any
feedback would be welcome.

I will wait for more comments and send a v2 in ~a week.

Kind regards,
Alvin

>
> thanks,

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

* Re: [PATCH 3/4] usb: typec: add TUSB320xA driver
  2022-03-07 22:17     ` Alvin Šipraga
@ 2022-03-08 11:49       ` Heikki Krogerus
  2022-03-08 12:30         ` Alvin Šipraga
  0 siblings, 1 reply; 15+ messages in thread
From: Heikki Krogerus @ 2022-03-08 11:49 UTC (permalink / raw)
  To: Alvin Šipraga
  Cc: Alvin Šipraga, Greg Kroah-Hartman, Rob Herring, linux-usb,
	devicetree, linux-kernel

On Mon, Mar 07, 2022 at 10:17:04PM +0000, Alvin Šipraga wrote:
> Hi Heikki,
> 
> Heikki Krogerus <heikki.krogerus@linux.intel.com> writes:
> 
> > Hi,
> >
> > On Tue, Mar 01, 2022 at 02:20:07PM +0100, Alvin Šipraga wrote:
> >> From: Alvin Šipraga <alsi@bang-olufsen.dk>
> >> 
> >> The TUSB320LA and TUSB320HA (or LAI, HAI) chips are I2C controlled
> >> non-PD Type-C port controllers. They support detection of cable
> >> orientation, port attachment state, and role, including Audio Accessory
> >> and Debug Accessory modes. Add a typec class driver for this family.
> >> 
> >> Note that there already exists an extcon driver for the TUSB320 (a
> >> slightly older revision that does not support setting role preference or
> >> disabling the CC state machine). This driver is loosely based on that
> >> one.
> >
> > This looked mostly OK to me. There is one question below.
> >
> > <snip>
> >
> >> +static int tusb320xa_check_signature(struct tusb320xa *tusb)
> >> +{
> >> +	static const char sig[] = { '\0', 'T', 'U', 'S', 'B', '3', '2', '0' };
> >> +	unsigned int val;
> >> +	int i, ret;
> >> +
> >> +	mutex_lock(&tusb->lock);
> >> +
> >> +	for (i = 0; i < sizeof(sig); i++) {
> >> +		ret = regmap_read(tusb->regmap, sizeof(sig) - 1 - i, &val);
> >> +		if (ret)
> >> +			goto done;
> >> +
> >> +		if (val != sig[i]) {
> >> +			dev_err(tusb->dev, "signature mismatch!\n");
> >> +			ret = -ENODEV;
> >> +			goto done;
> >> +		}
> >> +	}
> >> +
> >> +done:
> >> +	mutex_unlock(&tusb->lock);
> >> +
> >> +	return ret;
> >> +}
> >
> > Couldn't that be done with a single read?
> >
> >         char sig[8];
> >         u64 val;
> >
> >         strcpy(sig, "TUSB320")
> >
> >         mutex_lock(&tusb->lock);
> >
> >         ret = regmap_raw_read(tusb->regmap, 0, &val, sizeof(val));
> >         ...
> >         if (val != cpu_to_le64(*(u64 *)sig)) {
> >         ...
> >
> > Something like that?
> 
> I think it's a bit cryptic - are you sure it's worth it just to save 8
> one-off regmap_read()s? I could also just remove this check... I see it
> mostly as a courtesy to the user in case the I2C address in his device
> tree mistakenly points to some other unsuspecting chip.
> 
> BTW, do you have any feedback on the device tree bindings of this
> series? Rob had some questions and I am not sure that my proposed
> bindings are fully aligned with the typec subsystem expectations. Any
> feedback would be welcome.

I don't think I understand DT well enough to comment. I'm not
completely sure what he's asking..

> I will wait for more comments and send a v2 in ~a week.

thanks,

-- 
heikki

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

* Re: [PATCH 3/4] usb: typec: add TUSB320xA driver
  2022-03-08 11:49       ` Heikki Krogerus
@ 2022-03-08 12:30         ` Alvin Šipraga
  0 siblings, 0 replies; 15+ messages in thread
From: Alvin Šipraga @ 2022-03-08 12:30 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Alvin Šipraga, Greg Kroah-Hartman, Rob Herring, linux-usb,
	devicetree, linux-kernel

Heikki Krogerus <heikki.krogerus@linux.intel.com> writes:

> On Mon, Mar 07, 2022 at 10:17:04PM +0000, Alvin Šipraga wrote:
>> Hi Heikki,
>> 
>> Heikki Krogerus <heikki.krogerus@linux.intel.com> writes:
>> 
>> > Hi,
>> >
>> > On Tue, Mar 01, 2022 at 02:20:07PM +0100, Alvin Šipraga wrote:
>> >> From: Alvin Šipraga <alsi@bang-olufsen.dk>
>> >> 
>> >> The TUSB320LA and TUSB320HA (or LAI, HAI) chips are I2C controlled
>> >> non-PD Type-C port controllers. They support detection of cable
>> >> orientation, port attachment state, and role, including Audio Accessory
>> >> and Debug Accessory modes. Add a typec class driver for this family.
>> >> 
>> >> Note that there already exists an extcon driver for the TUSB320 (a
>> >> slightly older revision that does not support setting role preference or
>> >> disabling the CC state machine). This driver is loosely based on that
>> >> one.
>> >
>> > This looked mostly OK to me. There is one question below.
>> >
>> > <snip>
>> >
>> >> +static int tusb320xa_check_signature(struct tusb320xa *tusb)
>> >> +{
>> >> +	static const char sig[] = { '\0', 'T', 'U', 'S', 'B', '3', '2', '0' };
>> >> +	unsigned int val;
>> >> +	int i, ret;
>> >> +
>> >> +	mutex_lock(&tusb->lock);
>> >> +
>> >> +	for (i = 0; i < sizeof(sig); i++) {
>> >> +		ret = regmap_read(tusb->regmap, sizeof(sig) - 1 - i, &val);
>> >> +		if (ret)
>> >> +			goto done;
>> >> +
>> >> +		if (val != sig[i]) {
>> >> +			dev_err(tusb->dev, "signature mismatch!\n");
>> >> +			ret = -ENODEV;
>> >> +			goto done;
>> >> +		}
>> >> +	}
>> >> +
>> >> +done:
>> >> +	mutex_unlock(&tusb->lock);
>> >> +
>> >> +	return ret;
>> >> +}
>> >
>> > Couldn't that be done with a single read?
>> >
>> >         char sig[8];
>> >         u64 val;
>> >
>> >         strcpy(sig, "TUSB320")
>> >
>> >         mutex_lock(&tusb->lock);
>> >
>> >         ret = regmap_raw_read(tusb->regmap, 0, &val, sizeof(val));
>> >         ...
>> >         if (val != cpu_to_le64(*(u64 *)sig)) {
>> >         ...
>> >
>> > Something like that?
>> 
>> I think it's a bit cryptic - are you sure it's worth it just to save 8
>> one-off regmap_read()s? I could also just remove this check... I see it
>> mostly as a courtesy to the user in case the I2C address in his device
>> tree mistakenly points to some other unsuspecting chip.
>> 
>> BTW, do you have any feedback on the device tree bindings of this
>> series? Rob had some questions and I am not sure that my proposed
>> bindings are fully aligned with the typec subsystem expectations. Any
>> feedback would be welcome.
>
> I don't think I understand DT well enough to comment. I'm not
> completely sure what he's asking..

OK, no problem! Thanks for your reply.

Kind regards,
Alvin

>
>> I will wait for more comments and send a v2 in ~a week.
>
> thanks,

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

end of thread, other threads:[~2022-03-08 12:30 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01 13:20 [PATCH 0/4] usb: typec: add drivers for TUSB320xA and TS5USBA224 Alvin Šipraga
2022-03-01 13:20 ` [PATCH 1/4] dt-bindings: usb: add TUSB320xA Type-C port controller Alvin Šipraga
2022-03-02 18:16   ` Rob Herring
2022-03-01 13:20 ` [PATCH 2/4] dt-bindings: usb: add TS5USBA224 USB/Audio switch mux Alvin Šipraga
2022-03-02 18:21   ` Rob Herring
2022-03-02 18:58     ` Alvin Šipraga
2022-03-01 13:20 ` [PATCH 3/4] usb: typec: add TUSB320xA driver Alvin Šipraga
2022-03-01 16:58   ` kernel test robot
2022-03-01 17:20     ` Alvin Šipraga
2022-03-01 17:20       ` Alvin Šipraga
2022-03-07 14:36   ` Heikki Krogerus
2022-03-07 22:17     ` Alvin Šipraga
2022-03-08 11:49       ` Heikki Krogerus
2022-03-08 12:30         ` Alvin Šipraga
2022-03-01 13:20 ` [PATCH 4/4] usb: typec: mux: add TS5USBA224 driver Alvin Šipraga

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.