linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] USB Type-C mode selection
@ 2025-06-16 13:31 Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 01/10] usb: typec: Add alt_mode_override field to port property Andrei Kuchynski
                   ` (10 more replies)
  0 siblings, 11 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

This patch series introduces a flexible mechanism for USB Type-C mode
selection, enabling into USB4 mode, Thunderbolt alternate mode, or
DisplayPort alternate mode.
Two new sysfs attributes are exposed to provide user control over mode
selection:
`altmode_priorities`: Allows users to define their preferred order for
attempting mode entry.
`mode_selection`: Initiates an automatic mode entry process based on the
configured priorities and reports the outcome.
The mode selection logic attempts to enter prioritized modes sequentially.
A mode is considered successfully negotiated only when its alternate mode
driver explicitly reports a positive status. Alternate mode drivers are
required to report their mode entry status (either successful or failed).
If an alt-mode driver does not report its status within a defined timeout
period, the system automatically proceeds to attempt entry into the next
preferred mode.

This series was tested on a ChromeOS Brya device running kernel 6.6, and on
an Android OS device with kernel 6.12.

Andrei Kuchynski (10):
  usb: typec: Add alt_mode_override field to port property
  platform/chrome: cros_ec_typec: Set alt_mode_override flag
  usb: typec: ucsi: Set alt_mode_override flag
  usb: typec: Expose alternate mode priorities via sysfs
  usb: typec: Implement automated alternate mode selection
  Revert "usb: typec: displayport: Receive DP Status Update NAK request
    exit dp altmode"
  usb: typec: Report altmode entry status via callback
  usb: typec: ucsi: displayport: Propagate DP altmode entry result
  platform/chrome: cros_ec_typec: Propagate altmode entry result
  platform/chrome: cros_ec_typec: Report USB4 mode entry status via
    callback

 Documentation/ABI/testing/sysfs-class-typec  |  34 ++
 drivers/platform/chrome/cros_ec_typec.c      |  11 +
 drivers/platform/chrome/cros_typec_altmode.c |  32 +-
 drivers/platform/chrome/cros_typec_altmode.h |   6 +
 drivers/usb/typec/Makefile                   |   2 +-
 drivers/usb/typec/altmodes/displayport.c     |  17 +-
 drivers/usb/typec/altmodes/thunderbolt.c     |   6 +
 drivers/usb/typec/class.c                    |  95 +++-
 drivers/usb/typec/class.h                    |  16 +
 drivers/usb/typec/mode_selection.c           | 505 +++++++++++++++++++
 drivers/usb/typec/mode_selection.h           |  42 ++
 drivers/usb/typec/ucsi/displayport.c         |   4 +-
 drivers/usb/typec/ucsi/ucsi.c                |   2 +
 include/linux/usb/pd_vdo.h                   |   2 +
 include/linux/usb/typec.h                    |   1 +
 include/linux/usb/typec_altmode.h            |  12 +
 include/linux/usb/typec_dp.h                 |   2 +
 include/linux/usb/typec_tbt.h                |   3 +
 18 files changed, 775 insertions(+), 17 deletions(-)
 create mode 100644 drivers/usb/typec/mode_selection.c
 create mode 100644 drivers/usb/typec/mode_selection.h

-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* [PATCH 01/10] usb: typec: Add alt_mode_override field to port property
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
@ 2025-06-16 13:31 ` Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 02/10] platform/chrome: cros_ec_typec: Set alt_mode_override flag Andrei Kuchynski
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

This new field in the port properties dictates whether the Platform Policy
Manager (PPM) allows the OS Policy Manager (OPM) to change the currently
active, negotiated alternate mode.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/usb/typec/class.c | 14 +++++++++++---
 drivers/usb/typec/class.h |  2 ++
 include/linux/usb/typec.h |  1 +
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 67a533e35150..a72325ff099a 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -459,9 +459,16 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
 	struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
 
 	if (attr == &dev_attr_active.attr)
-		if (!is_typec_port(adev->dev.parent) &&
-		    (!adev->ops || !adev->ops->activate))
-			return 0444;
+		if (!is_typec_port(adev->dev.parent)) {
+			struct typec_partner *partner =
+				to_typec_partner(adev->dev.parent);
+			struct typec_port *port =
+				to_typec_port(partner->dev.parent);
+
+			if (!port->alt_mode_override || !adev->ops ||
+				!adev->ops->activate)
+				return 0444;
+		}
 
 	return attr->mode;
 }
@@ -2681,6 +2688,7 @@ struct typec_port *typec_register_port(struct device *parent,
 	}
 
 	port->pd = cap->pd;
+	port->alt_mode_override = cap->alt_mode_override;
 
 	ret = device_add(&port->dev);
 	if (ret) {
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index db2fe96c48ff..f05d9201c233 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -80,6 +80,8 @@ struct typec_port {
 	 */
 	struct device			*usb2_dev;
 	struct device			*usb3_dev;
+
+	bool				alt_mode_override;
 };
 
 #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 252af3f77039..6e09e68788dd 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -304,6 +304,7 @@ struct typec_capability {
 	enum typec_accessory	accessory[TYPEC_MAX_ACCESSORY];
 	unsigned int		orientation_aware:1;
 	u8			usb_capability;
+	bool			alt_mode_override;
 
 	struct fwnode_handle	*fwnode;
 	void			*driver_data;
-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* [PATCH 02/10] platform/chrome: cros_ec_typec: Set alt_mode_override flag
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 01/10] usb: typec: Add alt_mode_override field to port property Andrei Kuchynski
@ 2025-06-16 13:31 ` Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 03/10] usb: typec: ucsi: " Andrei Kuchynski
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

This flag specifies that the Embedded Controller (EC) must receive explicit
approval from the Application Processor (AP) before initiating Type-C
alternate modes or USB4 mode.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/platform/chrome/cros_ec_typec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 7678e3d05fd3..3aed429fde03 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -408,6 +408,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
 
 		cap->driver_data = cros_port;
 		cap->ops = &cros_typec_usb_mode_ops;
+		cap->alt_mode_override = typec->ap_driven_altmode;
 
 		cros_port->port = typec_register_port(dev, cap);
 		if (IS_ERR(cros_port->port)) {
-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* [PATCH 03/10] usb: typec: ucsi: Set alt_mode_override flag
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 01/10] usb: typec: Add alt_mode_override field to port property Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 02/10] platform/chrome: cros_ec_typec: Set alt_mode_override flag Andrei Kuchynski
@ 2025-06-16 13:31 ` Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs Andrei Kuchynski
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

This flag indicates that the PPM allows the OPM to change the currently
negotiated alternate mode using the SET_NEW_CAM command.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/usb/typec/ucsi/ucsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 01ce858a1a2b..1372becaec82 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1616,6 +1616,8 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
 
 	cap->driver_data = con;
 	cap->ops = &ucsi_ops;
+	cap->alt_mode_override =
+		!!(con->ucsi->cap.features & UCSI_CAP_ALT_MODE_OVERRIDE);
 
 	if (ucsi->ops->update_connector)
 		ucsi->ops->update_connector(con);
-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (2 preceding siblings ...)
  2025-06-16 13:31 ` [PATCH 03/10] usb: typec: ucsi: " Andrei Kuchynski
@ 2025-06-16 13:31 ` Andrei Kuchynski
  2025-06-17  9:50   ` Heikki Krogerus
  2025-06-16 13:31 ` [PATCH 05/10] usb: typec: Implement automated alternate mode selection Andrei Kuchynski
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

This sysfs attribute specifies the preferred order for enabling
DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 Documentation/ABI/testing/sysfs-class-typec | 17 ++++
 drivers/usb/typec/Makefile                  |  2 +-
 drivers/usb/typec/class.c                   | 26 ++++++
 drivers/usb/typec/class.h                   |  2 +
 drivers/usb/typec/mode_selection.c          | 93 +++++++++++++++++++++
 drivers/usb/typec/mode_selection.h          |  5 ++
 include/linux/usb/typec_altmode.h           |  7 ++
 7 files changed, 151 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/typec/mode_selection.c
 create mode 100644 drivers/usb/typec/mode_selection.h

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index 38e101c17a00..46eee82042ab 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -162,6 +162,23 @@ Description:	Lists the supported USB Modes. The default USB mode that is used
 		- usb3 (USB 3.2)
 		- usb4 (USB4)
 
+What:		/sys/class/typec/<port>/altmode_priorities
+Date:		June 2025
+Contact:	Andrei Kuchynski <akuchynski@chromium.org>
+Description:	Lists the alternate modes supported by the port and their
+		priorities. The priority setting determines the order in which
+		Displayport alt-mode, Thunderbolt alt-mode and USB4 mode will be
+		activated, indicating the preferred selection sequence. A value of -1
+		disables automatic entry into a specific mode, while lower numbers
+		indicate higher priority. The default priorities can be modified by
+		assigning new values. Modes without explicitly set values default to -1,
+		effectively disabling them.
+
+		Example values:
+		- "USB4=0 TBT=1 DP=2"
+		- "USB4=-1 TBT=0"
+		- "DP=-1 USB4=-1 TBT=-1"
+
 USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
 
 What:		/sys/class/typec/<port>-partner/accessory_mode
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 7a368fea61bc..8a6a1c663eb6 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_TYPEC)		+= typec.o
-typec-y				:= class.o mux.o bus.o pd.o retimer.o
+typec-y				:= class.o mux.o bus.o pd.o retimer.o mode_selection.o
 typec-$(CONFIG_ACPI)		+= port-mapper.o
 obj-$(CONFIG_TYPEC)		+= altmodes/
 obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index a72325ff099a..726fc0411c44 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -19,6 +19,7 @@
 #include "bus.h"
 #include "class.h"
 #include "pd.h"
+#include "mode_selection.h"
 
 static DEFINE_IDA(typec_index_ida);
 
@@ -1942,6 +1943,25 @@ static ssize_t orientation_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(orientation);
 
+static ssize_t altmode_priorities_store(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t size)
+{
+	struct typec_port *port = to_typec_port(dev);
+	int ret = typec_mode_priorities_set(port, buf);
+
+	return ret ? : size;
+}
+
+static ssize_t altmode_priorities_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct typec_port *port = to_typec_port(dev);
+
+	return typec_mode_priorities_get(port, buf);
+}
+static DEVICE_ATTR_RW(altmode_priorities);
+
 static struct attribute *typec_attrs[] = {
 	&dev_attr_data_role.attr,
 	&dev_attr_power_operation_mode.attr,
@@ -1954,6 +1974,7 @@ static struct attribute *typec_attrs[] = {
 	&dev_attr_port_type.attr,
 	&dev_attr_orientation.attr,
 	&dev_attr_usb_capability.attr,
+	&dev_attr_altmode_priorities.attr,
 	NULL,
 };
 
@@ -1992,6 +2013,9 @@ static umode_t typec_attr_is_visible(struct kobject *kobj,
 			return 0;
 		if (!port->ops || !port->ops->default_usb_mode_set)
 			return 0444;
+	} else if (attr == &dev_attr_altmode_priorities.attr) {
+		if (!port->alt_mode_override)
+			return 0;
 	}
 
 	return attr->mode;
@@ -2652,6 +2676,8 @@ struct typec_port *typec_register_port(struct device *parent,
 	else if (cap->usb_capability & USB_CAPABILITY_USB2)
 		port->usb_mode = USB_MODE_USB2;
 
+	typec_mode_priorities_set(port, NULL);
+
 	device_initialize(&port->dev);
 	port->dev.class = &typec_class;
 	port->dev.parent = parent;
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index f05d9201c233..dffe5ef03bc6 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -5,6 +5,7 @@
 
 #include <linux/device.h>
 #include <linux/usb/typec.h>
+#include <linux/usb/typec_altmode.h>
 
 struct typec_mux;
 struct typec_switch;
@@ -82,6 +83,7 @@ struct typec_port {
 	struct device			*usb3_dev;
 
 	bool				alt_mode_override;
+	int				altmode_priorities[TYPEC_ALTMODE_MAX];
 };
 
 #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
diff --git a/drivers/usb/typec/mode_selection.c b/drivers/usb/typec/mode_selection.c
new file mode 100644
index 000000000000..d984c79eef45
--- /dev/null
+++ b/drivers/usb/typec/mode_selection.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Google LLC.
+ */
+
+#include <linux/usb/typec_altmode.h>
+#include <linux/vmalloc.h>
+#include "class.h"
+
+#define MODE_PRIORITY_DISABLED -1
+
+static const char * const altmode_names[] = {
+	[TYPEC_ALTMODE_DP] = "DP",
+	[TYPEC_ALTMODE_TBT] = "TBT",
+	[TYPEC_ALTMODE_USB4] = "USB4",
+};
+static const char * const default_priorities = "USB4=0 TBT=1 DP=2";
+
+/* -------------------------------------------------------------------------- */
+/* port 'altmode_priorities' attribute */
+
+int typec_mode_priorities_set(struct typec_port *port,
+		const char *user_priorities)
+{
+	int priorities[TYPEC_ALTMODE_MAX];
+	const char *str_priority = user_priorities ? : default_priorities;
+	char *buf, *buf_free;
+	int ret = -EINVAL;
+	char *str_name;
+	int i;
+
+	buf = vmalloc(strlen(str_priority) + 1);
+	if (!buf)
+		return -ENOMEM;
+	strscpy(buf, str_priority, strlen(str_priority) + 1);
+	buf_free = buf;
+
+	for (i = 0; i < TYPEC_ALTMODE_MAX; i++)
+		priorities[i] = MODE_PRIORITY_DISABLED;
+
+	while ((str_name = strsep(&buf, " "))) {
+		char *str_value = strchr(str_name, '=');
+		int value;
+		int mode;
+
+		ret = -EINVAL;
+		if (!str_value)
+			goto parse_exit;
+		*str_value++ = '\0';
+
+		if (kstrtoint(str_value, 10, &value) ||
+			value < MODE_PRIORITY_DISABLED)
+			goto parse_exit;
+
+		if (value > MODE_PRIORITY_DISABLED) {
+			for (i = 0; i < TYPEC_ALTMODE_MAX; i++)
+				if (value == priorities[i])
+					goto parse_exit;
+		}
+
+		for (mode = 0; mode < TYPEC_ALTMODE_MAX &&
+			strcmp(str_name, altmode_names[mode]);)
+			mode++;
+		if (mode == TYPEC_ALTMODE_MAX ||
+			priorities[mode] != MODE_PRIORITY_DISABLED)
+			goto parse_exit;
+
+		priorities[mode] = value;
+		ret = 0;
+	}
+
+	for (i = 0; i < TYPEC_ALTMODE_MAX; i++)
+		port->altmode_priorities[i] = priorities[i];
+
+parse_exit:
+	vfree(buf_free);
+
+	return ret;
+}
+
+int typec_mode_priorities_get(struct typec_port *port, char *buf)
+{
+	ssize_t count = 0;
+	int i;
+
+	for (i = 0; i < TYPEC_ALTMODE_MAX; i++) {
+		if (i != TYPEC_ALTMODE_USB4 ||
+				port->cap->usb_capability & USB_CAPABILITY_USB4)
+			count += sysfs_emit_at(buf, count, "%s=%d ",
+				altmode_names[i], port->altmode_priorities[i]);
+	}
+	return count + sysfs_emit_at(buf, count, "\n");
+}
diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
new file mode 100644
index 000000000000..643f04f48343
--- /dev/null
+++ b/drivers/usb/typec/mode_selection.h
@@ -0,0 +1,5 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+int typec_mode_priorities_set(struct typec_port *port,
+		const char *user_priorities);
+int typec_mode_priorities_get(struct typec_port *port, char *buf);
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index b3c0866ea70f..7ca2040ee1e4 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -145,6 +145,13 @@ enum {
 
 #define TYPEC_MODAL_STATE(_state_)	((_state_) + TYPEC_STATE_MODAL)
 
+enum {
+	TYPEC_ALTMODE_DP = 0,
+	TYPEC_ALTMODE_TBT,
+	TYPEC_ALTMODE_USB4,
+	TYPEC_ALTMODE_MAX,
+};
+
 struct typec_altmode *typec_altmode_get_plug(struct typec_altmode *altmode,
 					     enum typec_plug_index index);
 void typec_altmode_put_plug(struct typec_altmode *plug);
-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* [PATCH 05/10] usb: typec: Implement automated alternate mode selection
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (3 preceding siblings ...)
  2025-06-16 13:31 ` [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs Andrei Kuchynski
@ 2025-06-16 13:31 ` Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 06/10] Revert "usb: typec: displayport: Receive DP Status Update NAK request exit dp altmode" Andrei Kuchynski
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

This commit introduces mode_selection sysfs attribute to control automated
alternate mode negotiation. Writing "yes" to this file activates the
automated selection process for DisplayPort alt-mode, Thunderbolt alt-mode,
and USB4 mode. Conversely, writing "no" will cancel any ongoing selection
process and exit the currently active alternate mode.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 Documentation/ABI/testing/sysfs-class-typec |  17 +
 drivers/usb/typec/class.c                   |  55 ++-
 drivers/usb/typec/class.h                   |  12 +
 drivers/usb/typec/mode_selection.c          | 412 ++++++++++++++++++++
 drivers/usb/typec/mode_selection.h          |  37 ++
 include/linux/usb/pd_vdo.h                  |   2 +
 include/linux/usb/typec_altmode.h           |   5 +
 7 files changed, 537 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index 46eee82042ab..ec205f49964e 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -264,6 +264,23 @@ Description:	The USB Modes that the partner device supports. The active mode
 		- usb3 (USB 3.2)
 		- usb4 (USB4)
 
+What:		/sys/class/typec/<port>-partner/mode_selection
+Date:		June 2025
+Contact:	Andrei Kuchynski <akuchynski@chromium.org>
+Description:	Lists the partner-supported alternate modes and mode entry
+		results with the currently entered mode bracketed. If a cable doesn't
+		support a mode, it's marked as 'nc'. An ellipsis indicates a mode
+		currently in progress. Automated mode selection is activated by writing
+		"yes" to the file. Conversely, writing "no" will cancel any ongoing
+		selection process and exit the currently active mode, if any.
+
+		Example values:
+		- "DP TBT=... USB4=nc": The cable does not support USB4 mode,
+			The driver is currently attempting to enter Thunderbolt alt-mode.
+		- "[DP] TBT=-EOPNOTSUPP USB4=-ETIME": USB4 mode entry failed due to
+			a timeout, Thunderbolt failed with the result -EOPNOTSUPP,
+			and DisplayPort is the active alt-mode.
+
 USB Type-C cable devices (eg. /sys/class/typec/port0-cable/)
 
 Note: Electronically Marked Cables will have a device also for one cable plug
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 726fc0411c44..e8432070b403 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -541,7 +541,7 @@ static void typec_altmode_release(struct device *dev)
 }
 
 const struct device_type typec_altmode_dev_type = {
-	.name = "typec_alternate_mode",
+	.name = ALTERNATE_MODE_DEVICE_TYPE_NAME,
 	.groups = typec_altmode_groups,
 	.release = typec_altmode_release,
 };
@@ -741,6 +741,34 @@ static ssize_t number_of_alternate_modes_show(struct device *dev, struct device_
 }
 static DEVICE_ATTR_RO(number_of_alternate_modes);
 
+static ssize_t mode_selection_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct typec_partner *partner = to_typec_partner(dev);
+
+	return typec_mode_selection_get(partner, buf);
+}
+
+static ssize_t mode_selection_store(struct device *dev, struct device_attribute *attr,
+			      const char *buf, size_t size)
+{
+	struct typec_partner *partner = to_typec_partner(dev);
+	bool start;
+	int ret = kstrtobool(buf, &start);
+
+	if (ret)
+		return ret;
+
+	if (start)
+		ret = typec_mode_selection_start(partner);
+	else
+		ret = typec_mode_selection_reset(partner);
+
+	return ret ? : size;
+}
+static DEVICE_ATTR_RW(mode_selection);
+
 static struct attribute *typec_partner_attrs[] = {
 	&dev_attr_accessory_mode.attr,
 	&dev_attr_supports_usb_power_delivery.attr,
@@ -748,6 +776,7 @@ static struct attribute *typec_partner_attrs[] = {
 	&dev_attr_type.attr,
 	&dev_attr_usb_mode.attr,
 	&dev_attr_usb_power_delivery_revision.attr,
+	&dev_attr_mode_selection.attr,
 	NULL
 };
 
@@ -772,6 +801,10 @@ static umode_t typec_partner_attr_is_visible(struct kobject *kobj, struct attrib
 		if (!get_pd_product_type(kobj_to_dev(kobj)))
 			return 0;
 
+	if (attr == &dev_attr_mode_selection.attr)
+		if (!port->alt_mode_override)
+			return 0444;
+
 	return attr->mode;
 }
 
@@ -850,8 +883,10 @@ int typec_partner_set_identity(struct typec_partner *partner)
 			usb_capability |= USB_CAPABILITY_USB2;
 		if (devcap & DEV_USB3_CAPABLE)
 			usb_capability |= USB_CAPABILITY_USB3;
-		if (devcap & DEV_USB4_CAPABLE)
+		if (devcap & DEV_USB4_CAPABLE) {
 			usb_capability |= USB_CAPABILITY_USB4;
+			typec_mode_selection_add_mode(partner, TYPEC_ALTMODE_USB4);
+		}
 	} else {
 		usb_capability = PD_VDO_DFP_HOSTCAP(id->vdo[0]);
 	}
@@ -971,7 +1006,12 @@ struct typec_altmode *
 typec_partner_register_altmode(struct typec_partner *partner,
 			       const struct typec_altmode_desc *desc)
 {
-	return typec_register_altmode(&partner->dev, desc);
+	struct typec_altmode *alt = typec_register_altmode(&partner->dev, desc);
+
+	if (alt)
+		typec_mode_selection_add_mode(partner, TYPEC_SVID_TO_ALTMODE(alt->svid));
+
+	return alt;
 }
 EXPORT_SYMBOL_GPL(typec_partner_register_altmode);
 
@@ -1075,6 +1115,8 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
 		typec_partner_link_device(partner, port->usb3_dev);
 	mutex_unlock(&port->partner_link_lock);
 
+	typec_mode_selection_create(partner);
+
 	return partner;
 }
 EXPORT_SYMBOL_GPL(typec_register_partner);
@@ -1092,6 +1134,7 @@ void typec_unregister_partner(struct typec_partner *partner)
 	if (IS_ERR_OR_NULL(partner))
 		return;
 
+	typec_mode_selection_destroy(partner);
 	port = to_typec_port(partner->dev.parent);
 
 	mutex_lock(&port->partner_link_lock);
@@ -1360,6 +1403,7 @@ int typec_cable_set_identity(struct typec_cable *cable)
 }
 EXPORT_SYMBOL_GPL(typec_cable_set_identity);
 
+static struct typec_partner *typec_get_partner(struct typec_port *port);
 /**
  * typec_register_cable - Register a USB Type-C Cable
  * @port: The USB Type-C Port the cable is connected to
@@ -1374,6 +1418,7 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
 					 struct typec_cable_desc *desc)
 {
 	struct typec_cable *cable;
+	struct typec_partner *partner;
 	int ret;
 
 	cable = kzalloc(sizeof(*cable), GFP_KERNEL);
@@ -1405,6 +1450,10 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
 		return ERR_PTR(ret);
 	}
 
+	partner = typec_get_partner(port);
+	typec_mode_selection_add_cable(partner, cable);
+	put_device(&partner->dev);
+
 	return cable;
 }
 EXPORT_SYMBOL_GPL(typec_register_cable);
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index dffe5ef03bc6..30e66d283a10 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -6,6 +6,7 @@
 #include <linux/device.h>
 #include <linux/usb/typec.h>
 #include <linux/usb/typec_altmode.h>
+#include <linux/kfifo.h>
 
 struct typec_mux;
 struct typec_switch;
@@ -27,6 +28,8 @@ struct typec_cable {
 	enum usb_pd_svdm_ver		svdm_version;
 };
 
+struct mode_selection_state;
+
 struct typec_partner {
 	struct device			dev;
 	unsigned int			usb_pd:1;
@@ -41,6 +44,13 @@ struct typec_partner {
 
 	struct usb_power_delivery	*pd;
 
+	struct delayed_work mode_selection_work;
+	DECLARE_KFIFO(mode_sequence, struct mode_selection_state *,
+			roundup_pow_of_two(TYPEC_ALTMODE_MAX));
+	struct mutex mode_sequence_lock;
+	struct mode_selection_state *mode_states;
+	struct mode_selection_state *active_mode;
+
 	void (*attach)(struct typec_partner *partner, struct device *dev);
 	void (*deattach)(struct typec_partner *partner, struct device *dev);
 };
@@ -113,4 +123,6 @@ static inline int typec_link_ports(struct typec_port *connector) { return 0; }
 static inline void typec_unlink_ports(struct typec_port *connector) { }
 #endif
 
+#define ALTERNATE_MODE_DEVICE_TYPE_NAME "typec_alternate_mode"
+
 #endif /* __USB_TYPEC_CLASS__ */
diff --git a/drivers/usb/typec/mode_selection.c b/drivers/usb/typec/mode_selection.c
index d984c79eef45..2bef50226af4 100644
--- a/drivers/usb/typec/mode_selection.c
+++ b/drivers/usb/typec/mode_selection.c
@@ -5,9 +5,22 @@
 
 #include <linux/usb/typec_altmode.h>
 #include <linux/vmalloc.h>
+#include <linux/usb/pd_vdo.h>
+#include <linux/kfifo.h>
+#include "mode_selection.h"
 #include "class.h"
 
 #define MODE_PRIORITY_DISABLED -1
+#define MODE_SELECTION_NO_RESULT 1
+
+static unsigned int mode_selection_timeout = 4000;
+module_param(mode_selection_timeout, uint, 0644);
+MODULE_PARM_DESC(mode_selection_timeout, "The timeout mode entry, ms");
+
+static unsigned int mode_selection_delay = 1000;
+module_param(mode_selection_delay, uint, 0644);
+MODULE_PARM_DESC(mode_selection_delay,
+	"The delay between attempts to enter or exit a mode, ms");
 
 static const char * const altmode_names[] = {
 	[TYPEC_ALTMODE_DP] = "DP",
@@ -16,6 +29,14 @@ static const char * const altmode_names[] = {
 };
 static const char * const default_priorities = "USB4=0 TBT=1 DP=2";
 
+struct mode_selection_state {
+	int mode;
+	bool enable;
+	bool cable_capability;
+	bool enter;
+	int result;
+};
+
 /* -------------------------------------------------------------------------- */
 /* port 'altmode_priorities' attribute */
 
@@ -91,3 +112,394 @@ int typec_mode_priorities_get(struct typec_port *port, char *buf)
 	}
 	return count + sysfs_emit_at(buf, count, "\n");
 }
+
+/* -------------------------------------------------------------------------- */
+/* partner 'mod_selection' attribute */
+
+/**
+ * mode_selection_next() - Process mode selection results and schedule next
+ * action
+ *
+ * This function evaluates the outcome of the previous mode entry or exit
+ * attempt. Based on this result, it determines the next alternate mode to
+ * process and schedules `mode_selection_work()` if further actions are
+ * required.
+ *
+ * If the previous mode entry was successful, the mode selection sequence is
+ * considered complete for the current cycle.
+ *
+ * If the previous mode entry failed, this function schedules
+ * `mode_selection_work()` to attempt exiting the currently active mode.
+ *
+ * If the previous operation was an exit (after a failed entry attempt),
+ * `mode_selection_next()` then advances the internal list of candidate
+ * modes to determine the next mode to enter.
+ */
+static void mode_selection_next(
+	struct typec_partner *partner, struct mode_selection_state *ms)
+{
+	if (!ms->result && ms->enter) {
+		dev_info(&partner->dev, "%s mode entered\n", altmode_names[ms->mode]);
+
+		partner->active_mode = ms;
+		kfifo_reset(&partner->mode_sequence);
+	} else {
+		if (ms->result && ms->enter)
+			dev_err(&partner->dev, "%s mode entry failed: %pe\n",
+				altmode_names[ms->mode], ERR_PTR(ms->result));
+
+		if (ms->result != -EBUSY) {
+			if (ms->enter)
+				ms->enter = false;
+			else
+				kfifo_skip(&partner->mode_sequence);
+		}
+
+		if (!kfifo_is_empty(&partner->mode_sequence)) {
+			cancel_delayed_work(&partner->mode_selection_work);
+			schedule_delayed_work(&partner->mode_selection_work,
+				msecs_to_jiffies(mode_selection_delay));
+		}
+	}
+}
+
+static void mode_selection_complete(struct typec_partner *partner,
+				const int mode, const int result)
+{
+	struct mode_selection_state *ms;
+
+	mutex_lock(&partner->mode_sequence_lock);
+	if (kfifo_peek(&partner->mode_sequence, &ms)) {
+		if (ms->mode == mode) {
+			ms->result = result;
+			mode_selection_next(partner, ms);
+		}
+	}
+	mutex_unlock(&partner->mode_sequence_lock);
+}
+
+void typec_mode_selection_altmode_complete(struct typec_altmode *alt,
+				const int result)
+{
+	mode_selection_complete(to_typec_partner(alt->dev.parent),
+		TYPEC_SVID_TO_ALTMODE(alt->svid), result);
+}
+EXPORT_SYMBOL_GPL(typec_mode_selection_altmode_complete);
+
+void typec_mode_selection_usb4_complete(struct typec_partner *partner,
+				const int result)
+{
+	mode_selection_complete(partner, TYPEC_ALTMODE_USB4, result);
+}
+EXPORT_SYMBOL_GPL(typec_mode_selection_usb4_complete);
+
+static int mode_selection_activate_altmode(struct device *dev, void *data)
+{
+	if (!strcmp(dev->type->name, ALTERNATE_MODE_DEVICE_TYPE_NAME)) {
+		struct typec_altmode *alt = to_typec_altmode(dev);
+		struct mode_selection_state *ms = (struct mode_selection_state *)data;
+
+		if (ms->mode == TYPEC_SVID_TO_ALTMODE(alt->svid)) {
+			int result = -EOPNOTSUPP;
+
+			if (alt->ops && alt->ops->activate)
+				result = alt->ops->activate(alt, ms->enter ? 1 : 0);
+			if (ms->enter)
+				ms->result = result;
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+static void mode_selection_activate_mode(struct typec_partner *partner,
+	struct mode_selection_state *ms)
+{
+	dev_info(&partner->dev, "Attempt to %s %s mode\n",
+		ms->enter ? "enter" : "exit", altmode_names[ms->mode]);
+
+	if (ms->mode == TYPEC_ALTMODE_USB4) {
+		struct typec_port *port = to_typec_port(partner->dev.parent);
+		int result = -EOPNOTSUPP;
+
+		if (port->ops && port->ops->enter_usb_mode)
+			result = port->ops->enter_usb_mode(port,
+				ms->enter ? USB_MODE_USB4 : USB_MODE_NONE);
+
+		if (ms->enter)
+			ms->result = result;
+	} else {
+		const int ret = device_for_each_child(&partner->dev, ms,
+				mode_selection_activate_altmode);
+		if (!ret && ms->enter)
+			ms->result = -ENODEV;
+	}
+}
+
+/**
+ * mode_selection_work() - Activate entry into the upcoming mode
+ *
+ * This function works in conjunction with `mode_selection_next()`.
+ * It attempts to activate the next alternate mode in the selection sequence.
+ *
+ * If the mode activation (`mode_selection_activate_mode()`) fails,
+ * `mode_selection_next()` will be called to initiate a new selection cycle.
+ *
+ * Otherwise, the result is temporarily set to -ETIME, and
+ * `mode_selection_activate_mode()` is scheduled for a subsequent entry after a
+ * timeout period. The alternate mode driver is expected to call back with the
+ * actual mode entry result. Upon this callback, `mode_selection_next()` will
+ * determine the subsequent mode and re-schedule mode_selection_work().
+ */
+static void mode_selection_work(struct work_struct *work)
+{
+	struct typec_partner *partner = container_of(work, struct typec_partner,
+						  mode_selection_work.work);
+	struct mode_selection_state *ms;
+
+	mutex_lock(&partner->mode_sequence_lock);
+	if (kfifo_peek(&partner->mode_sequence, &ms)) {
+		if (ms->enter && ms->result == -ETIME) {
+			mode_selection_next(partner, ms);
+		} else {
+			mode_selection_activate_mode(partner, ms);
+
+			if (!ms->enter || ms->result)
+				mode_selection_next(partner, ms);
+			else {
+				ms->result = -ETIME;
+				schedule_delayed_work(&partner->mode_selection_work,
+					msecs_to_jiffies(mode_selection_timeout));
+			}
+		}
+	}
+	mutex_unlock(&partner->mode_sequence_lock);
+}
+
+static void mode_selection_init(struct typec_partner *partner)
+{
+	int i;
+
+	for (i = 0; i < TYPEC_ALTMODE_MAX; i++) {
+		partner->mode_states[i].mode = i;
+		partner->mode_states[i].result = MODE_SELECTION_NO_RESULT;
+	}
+
+	kfifo_reset(&partner->mode_sequence);
+	partner->active_mode = NULL;
+}
+
+int typec_mode_selection_create(struct typec_partner *partner)
+{
+	partner->mode_states = vmalloc(
+		sizeof(struct mode_selection_state) * TYPEC_ALTMODE_MAX);
+	if (!partner->mode_states)
+		return -ENOMEM;
+
+	INIT_KFIFO(partner->mode_sequence);
+	mutex_init(&partner->mode_sequence_lock);
+	mode_selection_init(partner);
+	INIT_DELAYED_WORK(&partner->mode_selection_work, mode_selection_work);
+
+	return 0;
+}
+
+void typec_mode_selection_add_mode(struct typec_partner *partner,
+		const int mode)
+{
+	struct typec_port *port = to_typec_port(partner->dev.parent);
+
+	if (!partner->mode_states)
+		return;
+
+	if (mode < TYPEC_ALTMODE_MAX) {
+		if (mode == TYPEC_ALTMODE_USB4) {
+			if (!(port->cap->usb_capability & USB_CAPABILITY_USB4))
+				return;
+		}
+		partner->mode_states[mode].enable = true;
+	}
+}
+
+void typec_mode_selection_add_cable(struct typec_partner *partner,
+		struct typec_cable *cable)
+{
+	const u32 id_header = cable->identity->id_header;
+	const u32 vdo0 = cable->identity->vdo[0];
+	const u32 vdo1 = cable->identity->vdo[1];
+	const u32 type = PD_IDH_PTYPE(id_header);
+	const u32 speed = VDO_TYPEC_CABLE_SPEED(vdo0);
+	bool capable_dp = true;
+	bool capable_tbt = false;
+	bool capable_usb4 = false;
+
+	if (!partner->mode_states)
+		return;
+
+	if (type == IDH_PTYPE_PCABLE) {
+		capable_dp = (speed > CABLE_USB2_ONLY);
+		capable_tbt = capable_dp;
+		capable_usb4 = capable_dp;
+	} else if (type == IDH_PTYPE_ACABLE) {
+		const u32 version = VDO_TYPEC_CABLE_VERSION(vdo0);
+		const bool usb4_support = VDO_TYPEC_CABLE_USB4_SUPP(vdo1);
+		const bool modal_support = PD_IDH_MODAL_SUPP(id_header);
+
+		capable_dp = modal_support;
+		capable_tbt = true;
+		capable_usb4 = (version == 3) ? usb4_support : modal_support;
+	}
+
+	if (capable_dp || capable_tbt || capable_usb4)
+		dev_info(&partner->dev, "cable capabilities: %s %s %s\n",
+			capable_dp ? altmode_names[TYPEC_ALTMODE_DP] : "",
+			capable_tbt ? altmode_names[TYPEC_ALTMODE_TBT] : "",
+			capable_usb4 ? altmode_names[TYPEC_ALTMODE_USB4] : "");
+	partner->mode_states[TYPEC_ALTMODE_DP].cable_capability = capable_dp;
+	partner->mode_states[TYPEC_ALTMODE_TBT].cable_capability = capable_tbt;
+	partner->mode_states[TYPEC_ALTMODE_USB4].cable_capability = capable_usb4;
+}
+
+static void mode_selection_cancel_work(struct typec_partner *partner)
+{
+	/*
+	 * mode_sequence_lock provides exclusive access to `mode_sequence` FIFO
+	 * If the FIFO is empty, no further mode selection activities are expected
+	 */
+	mutex_lock(&partner->mode_sequence_lock);
+	kfifo_reset(&partner->mode_sequence);
+	mutex_unlock(&partner->mode_sequence_lock);
+
+	cancel_delayed_work_sync(&partner->mode_selection_work);
+}
+
+void typec_mode_selection_destroy(struct typec_partner *partner)
+{
+	if (!partner->mode_states)
+		return;
+
+	mode_selection_cancel_work(partner);
+	mutex_destroy(&partner->mode_sequence_lock);
+	vfree(partner->mode_states);
+	partner->mode_states = NULL;
+}
+
+/**
+ * typec_mode_selection_start() - Starts the alternate mode selection process.
+ *
+ * This function populates a 'mode_sequence' FIFO with pointers to
+ * `struct mode_selection_state` instances. The sequence is generated based on
+ * partner/cable capabilities and prioritized according to the port's settings.
+ */
+int typec_mode_selection_start(struct typec_partner *partner)
+{
+	struct typec_port *port = to_typec_port(partner->dev.parent);
+	int priorities[TYPEC_ALTMODE_MAX];
+	bool pending_mode = true;
+	int i;
+
+	if (!partner->mode_states)
+		return -ENOMEM;
+
+	mutex_lock(&partner->mode_sequence_lock);
+	if (!kfifo_is_empty(&partner->mode_sequence)) {
+		mutex_unlock(&partner->mode_sequence_lock);
+		return -EINPROGRESS;
+	}
+	if (partner->active_mode) {
+		mutex_unlock(&partner->mode_sequence_lock);
+		return -EALREADY;
+	}
+
+	mode_selection_init(partner);
+
+	for (i = 0; i < TYPEC_ALTMODE_MAX; i++) {
+		if (partner->mode_states[i].enable &&
+			partner->mode_states[i].cable_capability)
+			priorities[i] = port->altmode_priorities[i];
+		else
+			priorities[i] = MODE_PRIORITY_DISABLED;
+	}
+
+	while (pending_mode) {
+		int mode = TYPEC_ALTMODE_MAX;
+		int max_priority = INT_MAX;
+
+		for (i = 0; i < TYPEC_ALTMODE_MAX; i++) {
+			if (priorities[i] != MODE_PRIORITY_DISABLED &&
+				priorities[i] < max_priority) {
+				max_priority = priorities[i];
+				mode = i;
+			}
+		}
+
+		if (mode == TYPEC_ALTMODE_MAX)
+			pending_mode = false;
+		else {
+			partner->mode_states[mode].enter = true;
+			kfifo_put(&partner->mode_sequence, &partner->mode_states[mode]);
+			priorities[mode] = MODE_PRIORITY_DISABLED;
+		}
+	}
+
+	if (!kfifo_is_empty(&partner->mode_sequence))
+		schedule_delayed_work(&partner->mode_selection_work, 0);
+	mutex_unlock(&partner->mode_sequence_lock);
+
+	return 0;
+}
+
+int typec_mode_selection_reset(struct typec_partner *partner)
+{
+	if (!partner->mode_states)
+		return -ENOMEM;
+
+	mode_selection_cancel_work(partner);
+
+	if (partner->active_mode) {
+		partner->active_mode->enter = false;
+		mode_selection_activate_mode(partner, partner->active_mode);
+	}
+	mode_selection_init(partner);
+
+	return 0;
+}
+
+int typec_mode_selection_get(struct typec_partner *partner, char *buf)
+{
+	ssize_t count = 0;
+	int i;
+	struct mode_selection_state *running_ms;
+
+	if (!partner->mode_states)
+		return -ENOMEM;
+
+	mutex_lock(&partner->mode_sequence_lock);
+	if (!kfifo_peek(&partner->mode_sequence, &running_ms))
+		running_ms = NULL;
+
+	for (i = 0; i < TYPEC_ALTMODE_MAX; i++) {
+		struct mode_selection_state *ms = &partner->mode_states[i];
+
+		if (ms->enable) {
+			if (!ms->cable_capability)
+				count += sysfs_emit_at(buf, count, "%s=nc ", altmode_names[i]);
+			else if (ms == running_ms)
+				count += sysfs_emit_at(buf, count, "%s=... ", altmode_names[i]);
+			else if (ms->result == MODE_SELECTION_NO_RESULT)
+				count += sysfs_emit_at(buf, count, "%s ", altmode_names[i]);
+			else if (ms->result == 0)
+				count += sysfs_emit_at(buf, count, "[%s] ", altmode_names[i]);
+			else
+				count += sysfs_emit_at(buf, count, "%s=%pe ", altmode_names[i],
+					ERR_PTR(ms->result));
+		}
+	}
+	mutex_unlock(&partner->mode_sequence_lock);
+
+	if (count)
+		count += sysfs_emit_at(buf, count, "\n");
+
+	return count;
+}
diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
index 643f04f48343..62e92aed76c7 100644
--- a/drivers/usb/typec/mode_selection.h
+++ b/drivers/usb/typec/mode_selection.h
@@ -1,5 +1,42 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_tbt.h>
+
+#define TYPEC_SVID_TO_ALTMODE(svid) \
+	(((svid) == USB_TYPEC_DP_SID) ? TYPEC_ALTMODE_DP : \
+	 ((svid) == USB_TYPEC_TBT_SID) ? TYPEC_ALTMODE_TBT : TYPEC_ALTMODE_MAX)
+
 int typec_mode_priorities_set(struct typec_port *port,
 		const char *user_priorities);
 int typec_mode_priorities_get(struct typec_port *port, char *buf);
+
+/**
+ * The mode selection process follows a lifecycle tied to the USB-C partner
+ * device. The API is designed to first build a set of desired modes and then
+ * trigger the selection process. The expected sequence of calls is as follows:
+ *
+ * Creation and Configuration:
+ * call typec_mode_selection_create() when the partner device is being set
+ * up. This allocates resources for the mode selection.
+ * After creation, call typec_mode_selection_add_mode() and
+ * typec_mode_selection_add_cable() to define the parameters for the
+ * selection process.
+ *
+ * Execution:
+ * Call typec_mode_selection_start() to trigger the mode selection.
+ * Call typec_mode_selection_reset() to prematurely stop the selection
+ * process and clear any stored results.
+ *
+ * Destruction:
+ * Before destroying a partner, call typec_mode_selection_destroy()
+ */
+int typec_mode_selection_create(struct typec_partner *partner);
+void typec_mode_selection_destroy(struct typec_partner *partner);
+int typec_mode_selection_start(struct typec_partner *partner);
+int typec_mode_selection_reset(struct typec_partner *partner);
+void typec_mode_selection_add_mode(struct typec_partner *partner,
+		const int mode);
+void typec_mode_selection_add_cable(struct typec_partner *partner,
+		struct typec_cable *cable);
+int typec_mode_selection_get(struct typec_partner *partner, char *buf);
diff --git a/include/linux/usb/pd_vdo.h b/include/linux/usb/pd_vdo.h
index 5c48e8a81403..20bcf37ad634 100644
--- a/include/linux/usb/pd_vdo.h
+++ b/include/linux/usb/pd_vdo.h
@@ -439,6 +439,8 @@
 	 | (trans) << 11 | (phy) << 10 | (ele) << 9 | (u4) << 8			\
 	 | ((hops) & 0x3) << 6 | (u2) << 5 | (u32) << 4 | (lane) << 3		\
 	 | (iso) << 2 | (gen))
+#define VDO_TYPEC_CABLE_VERSION(vdo) (((vdo) >> 21) & 0x7)
+#define VDO_TYPEC_CABLE_USB4_SUPP(vdo) (((vdo) & BIT(8)) == ACAB2_USB4_SUPP)
 
 /*
  * AMA VDO (PD Rev2.0)
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index 7ca2040ee1e4..46b0ab2b8f5d 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -225,4 +225,9 @@ void typec_altmode_unregister_driver(struct typec_altmode_driver *drv);
 	module_driver(__typec_altmode_driver, typec_altmode_register_driver, \
 		      typec_altmode_unregister_driver)
 
+void typec_mode_selection_altmode_complete(struct typec_altmode *alt,
+				const int result);
+void typec_mode_selection_usb4_complete(struct typec_partner *partner,
+				const int result);
+
 #endif /* __USB_TYPEC_ALTMODE_H */
-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* [PATCH 06/10] Revert "usb: typec: displayport: Receive DP Status Update NAK request exit dp altmode"
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (4 preceding siblings ...)
  2025-06-16 13:31 ` [PATCH 05/10] usb: typec: Implement automated alternate mode selection Andrei Kuchynski
@ 2025-06-16 13:31 ` Andrei Kuchynski
  2025-06-16 14:15   ` Greg Kroah-Hartman
  2025-06-17  8:54   ` Heikki Krogerus
  2025-06-16 13:31 ` [PATCH 07/10] usb: typec: Report altmode entry status via callback Andrei Kuchynski
                   ` (4 subsequent siblings)
  10 siblings, 2 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

This reverts commit b4b38ffb38c91afd4dc387608db26f6fc34ed40b.

The commit introduced a deadlock with the cros_ec_typec driver.
The deadlock occurs due to a recursive lock acquisition of
`cros_typec_altmode_work::mutex`.
The call chain is as follows: 
1. cros_typec_altmode_work() acquires the mutex
2. typec_altmode_vdm() -> dp_altmode_vdm() ->
3. typec_altmode_exit() -> cros_typec_altmode_exit()
4. cros_typec_altmode_exit() attempts to acquire the mutex again

This revert is considered safe as no other known driver sends back
DP_CMD_STATUS_UPDATE command with the NAK flag.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/usb/typec/altmodes/displayport.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index b09b58d7311d..ac84a6d64c2f 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -393,10 +393,6 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
 		break;
 	case CMDT_RSP_NAK:
 		switch (cmd) {
-		case DP_CMD_STATUS_UPDATE:
-			if (typec_altmode_exit(alt))
-				dev_err(&dp->alt->dev, "Exit Mode Failed!\n");
-			break;
 		case DP_CMD_CONFIGURE:
 			dp->data.conf = 0;
 			ret = dp_altmode_configured(dp);
-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* [PATCH 07/10] usb: typec: Report altmode entry status via callback
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (5 preceding siblings ...)
  2025-06-16 13:31 ` [PATCH 06/10] Revert "usb: typec: displayport: Receive DP Status Update NAK request exit dp altmode" Andrei Kuchynski
@ 2025-06-16 13:31 ` Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 08/10] usb: typec: ucsi: displayport: Propagate DP altmode entry result Andrei Kuchynski
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

The Type-C mode selection logic requires feedback on the result of an
alternate mode entry attempt.
Call the `typec_mode_selection_altmode_complete()` callback to provide
this final success or failure status.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/usb/typec/altmodes/displayport.c | 13 +++++++++++--
 drivers/usb/typec/altmodes/thunderbolt.c |  6 ++++++
 include/linux/usb/typec_tbt.h            |  3 +++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index ac84a6d64c2f..946f61b57fa0 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -274,16 +274,20 @@ static void dp_altmode_work(struct work_struct *work)
 		header = DP_HEADER(dp, svdm_version, DP_CMD_STATUS_UPDATE);
 		vdo = 1;
 		ret = typec_altmode_vdm(dp->alt, header, &vdo, 2);
-		if (ret)
+		if (ret) {
 			dev_err(&dp->alt->dev,
 				"unable to send Status Update command (%d)\n",
 				ret);
+			typec_mode_selection_altmode_complete(dp->alt, ret);
+		}
 		break;
 	case DP_STATE_CONFIGURE:
 		ret = dp_altmode_configure_vdm(dp, dp->data.conf);
-		if (ret)
+		if (ret) {
 			dev_err(&dp->alt->dev,
 				"unable to send Configure command (%d)\n", ret);
+			typec_mode_selection_altmode_complete(dp->alt, ret);
+		}
 		break;
 	case DP_STATE_CONFIGURE_PRIME:
 		ret = dp_altmode_configure_vdm_cable(dp, dp->data_prime.conf);
@@ -397,6 +401,9 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
 			dp->data.conf = 0;
 			ret = dp_altmode_configured(dp);
 			break;
+		case DP_CMD_STATUS_UPDATE:
+			ret = *(int *)vdo;
+			break;
 		default:
 			break;
 		}
@@ -407,6 +414,8 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
 
 	if (dp->state != DP_STATE_IDLE)
 		schedule_work(&dp->work);
+	else if (ret || cmd == DP_CMD_CONFIGURE)
+		typec_mode_selection_altmode_complete(dp->alt, ret);
 
 err_unlock:
 	mutex_unlock(&dp->lock);
diff --git a/drivers/usb/typec/altmodes/thunderbolt.c b/drivers/usb/typec/altmodes/thunderbolt.c
index 6eadf7835f8f..697ab6060652 100644
--- a/drivers/usb/typec/altmodes/thunderbolt.c
+++ b/drivers/usb/typec/altmodes/thunderbolt.c
@@ -242,6 +242,9 @@ static int tbt_altmode_vdm(struct typec_altmode *alt,
 			else if (tbt->plug[TYPEC_PLUG_SOP_P])
 				tbt->state = TBT_STATE_SOP_P_EXIT;
 			break;
+		case TBT_CMD_STATUS_UPDATE:
+			typec_mode_selection_altmode_complete(alt, 0);
+			break;
 		}
 		break;
 	case CMDT_RSP_NAK:
@@ -249,6 +252,9 @@ static int tbt_altmode_vdm(struct typec_altmode *alt,
 		case CMD_ENTER_MODE:
 			dev_warn(&alt->dev, "Enter Mode refused\n");
 			break;
+		case TBT_CMD_STATUS_UPDATE:
+			typec_mode_selection_altmode_complete(alt, *(int *)vdo);
+			break;
 		default:
 			break;
 		}
diff --git a/include/linux/usb/typec_tbt.h b/include/linux/usb/typec_tbt.h
index 55dcea12082c..57cbda5292bb 100644
--- a/include/linux/usb/typec_tbt.h
+++ b/include/linux/usb/typec_tbt.h
@@ -24,6 +24,9 @@ struct typec_thunderbolt_data {
 	u32 enter_vdo;
 };
 
+/* TBT3 alt mode specific commands */
+#define TBT_CMD_STATUS_UPDATE		VDO_CMD_VENDOR(0)
+
 /* TBT3 Device Discover Mode VDO bits */
 #define TBT_MODE			BIT(0)
 #define TBT_ADAPTER(_vdo_)		FIELD_GET(BIT(16), _vdo_)
-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* [PATCH 08/10] usb: typec: ucsi: displayport: Propagate DP altmode entry result
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (6 preceding siblings ...)
  2025-06-16 13:31 ` [PATCH 07/10] usb: typec: Report altmode entry status via callback Andrei Kuchynski
@ 2025-06-16 13:31 ` Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 09/10] platform/chrome: cros_ec_typec: Propagate " Andrei Kuchynski
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

The `DP_CMD_CONFIGURE` VDM is the final step in the DisplayPort alternate
mode entry sequence. Reporting the error code from this command back to
the Type-C mode selection logic allows the detailed result to be propagated
to user space.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/usb/typec/ucsi/displayport.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index 8aae80b457d7..6f754e696d93 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -241,7 +241,9 @@ static int ucsi_displayport_vdm(struct typec_altmode *alt,
 			break;
 		case DP_CMD_CONFIGURE:
 			dp->data.conf = *data;
-			if (ucsi_displayport_configure(dp)) {
+			dp->data.error = ucsi_displayport_configure(dp);
+			if (dp->data.error) {
+				dp->vdo_data = &dp->data.error;
 				dp->header |= VDO_CMDT(CMDT_RSP_NAK);
 			} else {
 				dp->header |= VDO_CMDT(CMDT_RSP_ACK);
-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* [PATCH 09/10] platform/chrome: cros_ec_typec: Propagate altmode entry result
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (7 preceding siblings ...)
  2025-06-16 13:31 ` [PATCH 08/10] usb: typec: ucsi: displayport: Propagate DP altmode entry result Andrei Kuchynski
@ 2025-06-16 13:31 ` Andrei Kuchynski
  2025-06-16 13:31 ` [PATCH 10/10] platform/chrome: cros_ec_typec: Report USB4 mode entry status via callback Andrei Kuchynski
  2025-06-16 13:34 ` [PATCH 00/10] USB Type-C mode selection Dmitry Baryshkov
  10 siblings, 0 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

In the `cros_typec_configure_mux` function, which concludes the DP/TBT
alternate mode entry, the error code should be reported back to the Type-C
mode selection logic. This ensures a detailed result is conveyed to user
space.
To inform partner drivers about the result, the VDM mechanism is used. For
DP altmode, the DP_CMD_STATUS_UPDATE message is utilized, as it is the
final one in the mode entry sequence. For TBT mode, the
TBT_CMD_STATUS_UPDATE message is sent.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/platform/chrome/cros_ec_typec.c      |  9 ++++++
 drivers/platform/chrome/cros_typec_altmode.c | 32 ++++++++++++++++++--
 drivers/platform/chrome/cros_typec_altmode.h |  6 ++++
 include/linux/usb/typec_dp.h                 |  2 ++
 4 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 3aed429fde03..a4f338771094 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -610,6 +610,7 @@ static int cros_typec_enable_dp(struct cros_typec_data *typec,
 	if (!ret)
 		ret = typec_mux_set(port->mux, &port->state);
 
+	dp_data.error = 0;
 	if (!ret)
 		ret = cros_typec_displayport_status_update(port->state.alt,
 							   port->state.data);
@@ -699,8 +700,16 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
 		ret = cros_typec_enable_usb4(typec, port_num, pd_ctrl);
 	} else if (port->mux_flags & USB_PD_MUX_TBT_COMPAT_ENABLED) {
 		ret = cros_typec_enable_tbt(typec, port_num, pd_ctrl);
+		cros_typec_tbt_status_update(
+			port->port_altmode[CROS_EC_ALTMODE_TBT], ret);
 	} else if (port->mux_flags & USB_PD_MUX_DP_ENABLED) {
 		ret = cros_typec_enable_dp(typec, port_num, pd_ctrl);
+		if (ret) {
+			struct typec_displayport_data dp_data = {.error = ret};
+
+			cros_typec_displayport_status_update(
+				port->port_altmode[CROS_EC_ALTMODE_DP], &dp_data);
+		}
 	} else if (port->mux_flags & USB_PD_MUX_SAFE_MODE) {
 		ret = cros_typec_usb_safe_state(port);
 	} else if (port->mux_flags & USB_PD_MUX_USB_ENABLED) {
diff --git a/drivers/platform/chrome/cros_typec_altmode.c b/drivers/platform/chrome/cros_typec_altmode.c
index 557340b53af0..7ee295cf0c02 100644
--- a/drivers/platform/chrome/cros_typec_altmode.c
+++ b/drivers/platform/chrome/cros_typec_altmode.c
@@ -28,6 +28,7 @@ struct cros_typec_altmode_data {
 
 	u16 sid;
 	u8 mode;
+	int error;
 };
 
 struct cros_typec_dp_data {
@@ -295,9 +296,16 @@ int cros_typec_displayport_status_update(struct typec_altmode *altmode,
 
 	dp_data->data = *data;
 	dp_data->pending_status_update = false;
-	adata->header |= VDO_CMDT(CMDT_RSP_ACK);
-	adata->vdo_data = &dp_data->data.status;
-	adata->vdo_size = 2;
+	if (data->error) {
+		adata->header |= VDO_CMDT(CMDT_RSP_NAK);
+		adata->error = dp_data->data.error;
+		adata->vdo_data = &adata->error;
+		adata->vdo_size = 1;
+	} else {
+		adata->header |= VDO_CMDT(CMDT_RSP_ACK);
+		adata->vdo_data = &dp_data->data.status;
+		adata->vdo_size = 2;
+	}
 	schedule_work(&adata->work);
 
 	mutex_unlock(&adata->lock);
@@ -370,4 +378,22 @@ cros_typec_register_thunderbolt(struct cros_typec_port *port,
 
 	return alt;
 }
+
+int cros_typec_tbt_status_update(struct typec_altmode *alt, int error)
+{
+	struct cros_typec_altmode_data *adata = typec_altmode_get_drvdata(alt);
+
+	mutex_lock(&adata->lock);
+
+	adata->header = VDO(adata->sid, 1, SVDM_VER_2_0, TBT_CMD_STATUS_UPDATE);
+	adata->header |= VDO_CMDT(error ? CMDT_RSP_NAK : CMDT_RSP_ACK);
+	adata->error = error;
+	adata->vdo_data = &adata->error;
+	adata->vdo_size = 1;
+	schedule_work(&adata->work);
+
+	mutex_unlock(&adata->lock);
+
+	return 0;
+}
 #endif
diff --git a/drivers/platform/chrome/cros_typec_altmode.h b/drivers/platform/chrome/cros_typec_altmode.h
index 3f2aa95d065a..848a2b194b34 100644
--- a/drivers/platform/chrome/cros_typec_altmode.h
+++ b/drivers/platform/chrome/cros_typec_altmode.h
@@ -39,6 +39,7 @@ static inline int cros_typec_displayport_status_update(struct typec_altmode *alt
 struct typec_altmode *
 cros_typec_register_thunderbolt(struct cros_typec_port *port,
 				struct typec_altmode_desc *desc);
+int cros_typec_tbt_status_update(struct typec_altmode *alt, int error);
 #else
 static inline struct typec_altmode *
 cros_typec_register_thunderbolt(struct cros_typec_port *port,
@@ -46,6 +47,11 @@ cros_typec_register_thunderbolt(struct cros_typec_port *port,
 {
 	return typec_port_register_altmode(port->port, desc);
 }
+static inline int cros_typec_tbt_status_update(struct typec_altmode *alt,
+					int error)
+{
+	return 0;
+}
 #endif
 
 #endif /* __CROS_TYPEC_ALTMODE_H__ */
diff --git a/include/linux/usb/typec_dp.h b/include/linux/usb/typec_dp.h
index f2da264d9c14..1679f7bb0c75 100644
--- a/include/linux/usb/typec_dp.h
+++ b/include/linux/usb/typec_dp.h
@@ -44,10 +44,12 @@ enum {
  * commands: Status Update and Configure.
  *
  * @status will show for example the status of the HPD signal.
+ * @error will contain the error code, if applicable.
  */
 struct typec_displayport_data {
 	u32 status;
 	u32 conf;
+	int error;
 };
 
 enum {
-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* [PATCH 10/10] platform/chrome: cros_ec_typec: Report USB4 mode entry status via callback
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (8 preceding siblings ...)
  2025-06-16 13:31 ` [PATCH 09/10] platform/chrome: cros_ec_typec: Propagate " Andrei Kuchynski
@ 2025-06-16 13:31 ` Andrei Kuchynski
  2025-06-16 13:34 ` [PATCH 00/10] USB Type-C mode selection Dmitry Baryshkov
  10 siblings, 0 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 13:31 UTC (permalink / raw)
  To: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform, Andrei Kuchynski

The Type-C mode selection logic requires feedback on the result of USB4
mode entry attempt. Call the `typec_mode_selection_usb4_complete()`
callback to provide this final success or failure status.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/platform/chrome/cros_ec_typec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index a4f338771094..c5a7f42ffb5a 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -698,6 +698,7 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
 
 	if (port->mux_flags & USB_PD_MUX_USB4_ENABLED) {
 		ret = cros_typec_enable_usb4(typec, port_num, pd_ctrl);
+		typec_mode_selection_usb4_complete(port->partner, ret);
 	} else if (port->mux_flags & USB_PD_MUX_TBT_COMPAT_ENABLED) {
 		ret = cros_typec_enable_tbt(typec, port_num, pd_ctrl);
 		cros_typec_tbt_status_update(
-- 
2.50.0.rc1.591.g9c95f17f64-goog


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

* Re: [PATCH 00/10] USB Type-C mode selection
  2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (9 preceding siblings ...)
  2025-06-16 13:31 ` [PATCH 10/10] platform/chrome: cros_ec_typec: Report USB4 mode entry status via callback Andrei Kuchynski
@ 2025-06-16 13:34 ` Dmitry Baryshkov
  10 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2025-06-16 13:34 UTC (permalink / raw)
  To: Andrei Kuchynski, Heikki Krogerus, Dmitry Baryshkov,
	Abhishek Pandit-Subedi, Jameson Thies, Benson Leung,
	Tzung-Bi Shih
  Cc: Greg Kroah-Hartman, Guenter Roeck, Pooja Katiyar,
	Badhri Jagan Sridharan, RD Babiera, linux-usb, linux-kernel,
	chrome-platform

On 16/06/2025 16:31, Andrei Kuchynski wrote:
> This patch series introduces a flexible mechanism for USB Type-C mode
> selection, enabling into USB4 mode, Thunderbolt alternate mode, or
> DisplayPort alternate mode.
> Two new sysfs attributes are exposed to provide user control over mode
> selection:
> `altmode_priorities`: Allows users to define their preferred order for
> attempting mode entry.
> `mode_selection`: Initiates an automatic mode entry process based on the
> configured priorities and reports the outcome.
> The mode selection logic attempts to enter prioritized modes sequentially.
> A mode is considered successfully negotiated only when its alternate mode
> driver explicitly reports a positive status. Alternate mode drivers are
> required to report their mode entry status (either successful or failed).
> If an alt-mode driver does not report its status within a defined timeout
> period, the system automatically proceeds to attempt entry into the next
> preferred mode.
> 
> This series was tested on a ChromeOS Brya device running kernel 6.6, and on
> an Android OS device with kernel 6.12.

Both kernels are terribly old. Please run the tests on top of linux-next 
or usb-next.

> 
> Andrei Kuchynski (10):
>    usb: typec: Add alt_mode_override field to port property
>    platform/chrome: cros_ec_typec: Set alt_mode_override flag
>    usb: typec: ucsi: Set alt_mode_override flag
>    usb: typec: Expose alternate mode priorities via sysfs
>    usb: typec: Implement automated alternate mode selection
>    Revert "usb: typec: displayport: Receive DP Status Update NAK request
>      exit dp altmode"
>    usb: typec: Report altmode entry status via callback
>    usb: typec: ucsi: displayport: Propagate DP altmode entry result
>    platform/chrome: cros_ec_typec: Propagate altmode entry result
>    platform/chrome: cros_ec_typec: Report USB4 mode entry status via
>      callback
> 
>   Documentation/ABI/testing/sysfs-class-typec  |  34 ++
>   drivers/platform/chrome/cros_ec_typec.c      |  11 +
>   drivers/platform/chrome/cros_typec_altmode.c |  32 +-
>   drivers/platform/chrome/cros_typec_altmode.h |   6 +
>   drivers/usb/typec/Makefile                   |   2 +-
>   drivers/usb/typec/altmodes/displayport.c     |  17 +-
>   drivers/usb/typec/altmodes/thunderbolt.c     |   6 +
>   drivers/usb/typec/class.c                    |  95 +++-
>   drivers/usb/typec/class.h                    |  16 +
>   drivers/usb/typec/mode_selection.c           | 505 +++++++++++++++++++
>   drivers/usb/typec/mode_selection.h           |  42 ++
>   drivers/usb/typec/ucsi/displayport.c         |   4 +-
>   drivers/usb/typec/ucsi/ucsi.c                |   2 +
>   include/linux/usb/pd_vdo.h                   |   2 +
>   include/linux/usb/typec.h                    |   1 +
>   include/linux/usb/typec_altmode.h            |  12 +
>   include/linux/usb/typec_dp.h                 |   2 +
>   include/linux/usb/typec_tbt.h                |   3 +
>   18 files changed, 775 insertions(+), 17 deletions(-)
>   create mode 100644 drivers/usb/typec/mode_selection.c
>   create mode 100644 drivers/usb/typec/mode_selection.h
> 


-- 
With best wishes
Dmitry

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

* Re: [PATCH 06/10] Revert "usb: typec: displayport: Receive DP Status Update NAK request exit dp altmode"
  2025-06-16 13:31 ` [PATCH 06/10] Revert "usb: typec: displayport: Receive DP Status Update NAK request exit dp altmode" Andrei Kuchynski
@ 2025-06-16 14:15   ` Greg Kroah-Hartman
  2025-06-16 19:42     ` Andrei Kuchynski
  2025-06-17  8:54   ` Heikki Krogerus
  1 sibling, 1 reply; 22+ messages in thread
From: Greg Kroah-Hartman @ 2025-06-16 14:15 UTC (permalink / raw)
  To: Andrei Kuchynski
  Cc: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih, Guenter Roeck,
	Pooja Katiyar, Badhri Jagan Sridharan, RD Babiera, linux-usb,
	linux-kernel, chrome-platform

On Mon, Jun 16, 2025 at 01:31:43PM +0000, Andrei Kuchynski wrote:
> This reverts commit b4b38ffb38c91afd4dc387608db26f6fc34ed40b.
> 
> The commit introduced a deadlock with the cros_ec_typec driver.
> The deadlock occurs due to a recursive lock acquisition of
> `cros_typec_altmode_work::mutex`.
> The call chain is as follows: 
> 1. cros_typec_altmode_work() acquires the mutex
> 2. typec_altmode_vdm() -> dp_altmode_vdm() ->
> 3. typec_altmode_exit() -> cros_typec_altmode_exit()
> 4. cros_typec_altmode_exit() attempts to acquire the mutex again
> 
> This revert is considered safe as no other known driver sends back
> DP_CMD_STATUS_UPDATE command with the NAK flag.
> 
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> ---
>  drivers/usb/typec/altmodes/displayport.c | 4 ----
>  1 file changed, 4 deletions(-)

Why isn't this being sent as a separate patch for 6.16-final?  And why
not put a fixes: line?

thanks,

greg k-h

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

* Re: [PATCH 06/10] Revert "usb: typec: displayport: Receive DP Status Update NAK request exit dp altmode"
  2025-06-16 14:15   ` Greg Kroah-Hartman
@ 2025-06-16 19:42     ` Andrei Kuchynski
  0 siblings, 0 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-16 19:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih, Guenter Roeck,
	Pooja Katiyar, Badhri Jagan Sridharan, RD Babiera, linux-usb,
	linux-kernel, chrome-platform

On Mon, Jun 16, 2025 at 4:15 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jun 16, 2025 at 01:31:43PM +0000, Andrei Kuchynski wrote:
> > This reverts commit b4b38ffb38c91afd4dc387608db26f6fc34ed40b.
> >
> > The commit introduced a deadlock with the cros_ec_typec driver.
> > The deadlock occurs due to a recursive lock acquisition of
> > `cros_typec_altmode_work::mutex`.
> > The call chain is as follows:
> > 1. cros_typec_altmode_work() acquires the mutex
> > 2. typec_altmode_vdm() -> dp_altmode_vdm() ->
> > 3. typec_altmode_exit() -> cros_typec_altmode_exit()
> > 4. cros_typec_altmode_exit() attempts to acquire the mutex again
> >
> > This revert is considered safe as no other known driver sends back
> > DP_CMD_STATUS_UPDATE command with the NAK flag.
> >
> > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > ---
> >  drivers/usb/typec/altmodes/displayport.c | 4 ----
> >  1 file changed, 4 deletions(-)
>
> Why isn't this being sent as a separate patch for 6.16-final?  And why
> not put a fixes: line?
>

Hi Greg,

The issue will emerge only after this series is applied, so 6.16
remains secure as this code is not executable.
I will submit it as a separate patch, with the relevant tags included.

Thanks,
Andrei

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

* Re: [PATCH 06/10] Revert "usb: typec: displayport: Receive DP Status Update NAK request exit dp altmode"
  2025-06-16 13:31 ` [PATCH 06/10] Revert "usb: typec: displayport: Receive DP Status Update NAK request exit dp altmode" Andrei Kuchynski
  2025-06-16 14:15   ` Greg Kroah-Hartman
@ 2025-06-17  8:54   ` Heikki Krogerus
  2025-06-17 12:54     ` Andrei Kuchynski
  1 sibling, 1 reply; 22+ messages in thread
From: Heikki Krogerus @ 2025-06-17  8:54 UTC (permalink / raw)
  To: Andrei Kuchynski
  Cc: Dmitry Baryshkov, Abhishek Pandit-Subedi, Jameson Thies,
	Benson Leung, Tzung-Bi Shih, Greg Kroah-Hartman, Guenter Roeck,
	Pooja Katiyar, Badhri Jagan Sridharan, RD Babiera, linux-usb,
	linux-kernel, chrome-platform

Hi Andrei,

On Mon, Jun 16, 2025 at 01:31:43PM +0000, Andrei Kuchynski wrote:
> This reverts commit b4b38ffb38c91afd4dc387608db26f6fc34ed40b.
> 
> The commit introduced a deadlock with the cros_ec_typec driver.
> The deadlock occurs due to a recursive lock acquisition of
> `cros_typec_altmode_work::mutex`.
> The call chain is as follows: 
> 1. cros_typec_altmode_work() acquires the mutex
> 2. typec_altmode_vdm() -> dp_altmode_vdm() ->
> 3. typec_altmode_exit() -> cros_typec_altmode_exit()
> 4. cros_typec_altmode_exit() attempts to acquire the mutex again
> 
> This revert is considered safe as no other known driver sends back
> DP_CMD_STATUS_UPDATE command with the NAK flag.
> 
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> ---
>  drivers/usb/typec/altmodes/displayport.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index b09b58d7311d..ac84a6d64c2f 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -393,10 +393,6 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
>  		break;
>  	case CMDT_RSP_NAK:
>  		switch (cmd) {
> -		case DP_CMD_STATUS_UPDATE:
> -			if (typec_altmode_exit(alt))
> -				dev_err(&dp->alt->dev, "Exit Mode Failed!\n");
> -			break;

Commit b4b38ffb38c9 ("usb: typec: displayport: Receive DP Status
Update NAK request exit dp altmode") addressed a very real problem
with failure to execute data role swap. You are not really offering
anything else for that issue here.

Is it not an option to just schedule the mode exit here instead to
solve the problem?

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index b09b58d7311d..2abbe4de3216 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -394,8 +394,7 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
        case CMDT_RSP_NAK:
                switch (cmd) {
                case DP_CMD_STATUS_UPDATE:
-                       if (typec_altmode_exit(alt))
-                               dev_err(&dp->alt->dev, "Exit Mode Failed!\n");
+                       dp->state = DP_STATE_EXIT;
                        break;
                case DP_CMD_CONFIGURE:
                        dp->data.conf = 0;


-- 
heikki

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

* Re: [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs
  2025-06-16 13:31 ` [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs Andrei Kuchynski
@ 2025-06-17  9:50   ` Heikki Krogerus
  2025-06-17 12:38     ` Andrei Kuchynski
  0 siblings, 1 reply; 22+ messages in thread
From: Heikki Krogerus @ 2025-06-17  9:50 UTC (permalink / raw)
  To: Andrei Kuchynski
  Cc: Dmitry Baryshkov, Abhishek Pandit-Subedi, Jameson Thies,
	Benson Leung, Tzung-Bi Shih, Greg Kroah-Hartman, Guenter Roeck,
	Pooja Katiyar, Badhri Jagan Sridharan, RD Babiera, linux-usb,
	linux-kernel, chrome-platform

On Mon, Jun 16, 2025 at 01:31:41PM +0000, Andrei Kuchynski wrote:
> This sysfs attribute specifies the preferred order for enabling
> DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> 
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> ---
>  Documentation/ABI/testing/sysfs-class-typec | 17 ++++
>  drivers/usb/typec/Makefile                  |  2 +-
>  drivers/usb/typec/class.c                   | 26 ++++++
>  drivers/usb/typec/class.h                   |  2 +
>  drivers/usb/typec/mode_selection.c          | 93 +++++++++++++++++++++
>  drivers/usb/typec/mode_selection.h          |  5 ++
>  include/linux/usb/typec_altmode.h           |  7 ++
>  7 files changed, 151 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/usb/typec/mode_selection.c
>  create mode 100644 drivers/usb/typec/mode_selection.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index 38e101c17a00..46eee82042ab 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -162,6 +162,23 @@ Description:	Lists the supported USB Modes. The default USB mode that is used
>  		- usb3 (USB 3.2)
>  		- usb4 (USB4)
>  
> +What:		/sys/class/typec/<port>/altmode_priorities
> +Date:		June 2025
> +Contact:	Andrei Kuchynski <akuchynski@chromium.org>
> +Description:	Lists the alternate modes supported by the port and their
> +		priorities. The priority setting determines the order in which
> +		Displayport alt-mode, Thunderbolt alt-mode and USB4 mode will be
> +		activated, indicating the preferred selection sequence. A value of -1
> +		disables automatic entry into a specific mode, while lower numbers
> +		indicate higher priority. The default priorities can be modified by
> +		assigning new values. Modes without explicitly set values default to -1,
> +		effectively disabling them.
> +
> +		Example values:
> +		- "USB4=0 TBT=1 DP=2"
> +		- "USB4=-1 TBT=0"
> +		- "DP=-1 USB4=-1 TBT=-1"

No. If you want to disable entry to a mode by default, then you
deactivate that mode, so -1 is not needed. USB4 is also not an alt
mode, so this at the very least should be named differently.

But I'm not sure this is the correct way to handle the modes in
general. Can you please explain what exactly is the use case you are
thinking?

If you just need to prevent for example USB4 entry by default, then
you write usb3 (or usb2) to the usb_capability. The alt modes you can
deactivate by default, no?

I can appreciate the convenience that you get from a single file like
that, but it does not really give much room to move if for example the
user space needs to behave differently in case of failures to enter a
specific mode compared to successful entries.

Br,

-- 
heikki

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

* Re: [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs
  2025-06-17  9:50   ` Heikki Krogerus
@ 2025-06-17 12:38     ` Andrei Kuchynski
  2025-06-17 13:28       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-17 12:38 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Dmitry Baryshkov, Abhishek Pandit-Subedi, Jameson Thies,
	Benson Leung, Tzung-Bi Shih, Greg Kroah-Hartman, Guenter Roeck,
	Pooja Katiyar, Badhri Jagan Sridharan, RD Babiera, linux-usb,
	linux-kernel, chrome-platform

On Tue, Jun 17, 2025 at 11:50 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Mon, Jun 16, 2025 at 01:31:41PM +0000, Andrei Kuchynski wrote:
> > This sysfs attribute specifies the preferred order for enabling
> > DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> >
> > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > ---
> >  Documentation/ABI/testing/sysfs-class-typec | 17 ++++
> >  drivers/usb/typec/Makefile                  |  2 +-
> >  drivers/usb/typec/class.c                   | 26 ++++++
> >  drivers/usb/typec/class.h                   |  2 +
> >  drivers/usb/typec/mode_selection.c          | 93 +++++++++++++++++++++
> >  drivers/usb/typec/mode_selection.h          |  5 ++
> >  include/linux/usb/typec_altmode.h           |  7 ++
> >  7 files changed, 151 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/usb/typec/mode_selection.c
> >  create mode 100644 drivers/usb/typec/mode_selection.h
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index 38e101c17a00..46eee82042ab 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -162,6 +162,23 @@ Description:     Lists the supported USB Modes. The default USB mode that is used
> >               - usb3 (USB 3.2)
> >               - usb4 (USB4)
> >
> > +What:                /sys/class/typec/<port>/altmode_priorities
> > +Date:                June 2025
> > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > +Description: Lists the alternate modes supported by the port and their
> > +             priorities. The priority setting determines the order in which
> > +             Displayport alt-mode, Thunderbolt alt-mode and USB4 mode will be
> > +             activated, indicating the preferred selection sequence. A value of -1
> > +             disables automatic entry into a specific mode, while lower numbers
> > +             indicate higher priority. The default priorities can be modified by
> > +             assigning new values. Modes without explicitly set values default to -1,
> > +             effectively disabling them.
> > +
> > +             Example values:
> > +             - "USB4=0 TBT=1 DP=2"
> > +             - "USB4=-1 TBT=0"
> > +             - "DP=-1 USB4=-1 TBT=-1"
>
> No. If you want to disable entry to a mode by default, then you
> deactivate that mode, so -1 is not needed. USB4 is also not an alt
> mode, so this at the very least should be named differently.
>
> But I'm not sure this is the correct way to handle the modes in
> general. Can you please explain what exactly is the use case you are
> thinking?

Hi Heikki,

This implements the mode selection logic within the kernel, replacing
its userspace counterpart. Implementing this in the kernel offers
advantages, making the process both more robust and far easier to
manage.
This eliminates the need for user space to verify port, partner, or
cable capabilities, and to control the mode entry process. User space
doesn't even need to know if USB4 mode is supported by the port or
partner; unsupported modes are automatically skipped.
User space's role is now limited to high-level tasks like security,
with the kernel managing technical implementation. Mode selection and
activation can occur without user space intervention at all if no
special rules apply and the default policy (USB4 -> TBT -> DP) is
acceptable.

>
> If you just need to prevent for example USB4 entry by default, then
> you write usb3 (or usb2) to the usb_capability. The alt modes you can
> deactivate by default, no?
>
> I can appreciate the convenience that you get from a single file like
> that, but it does not really give much room to move if for example the
> user space needs to behave differently in case of failures to enter a
> specific mode compared to successful entries.
>

You are right, this patch series is proposed for its convenience. It
offers a more convenient method for users to enable or activate any
mode, without altering existing methods. Users can decide whether they
prioritize more control or an easier mode selection method.

Thanks,
Andrei

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

* Re: [PATCH 06/10] Revert "usb: typec: displayport: Receive DP Status Update NAK request exit dp altmode"
  2025-06-17  8:54   ` Heikki Krogerus
@ 2025-06-17 12:54     ` Andrei Kuchynski
  0 siblings, 0 replies; 22+ messages in thread
From: Andrei Kuchynski @ 2025-06-17 12:54 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Dmitry Baryshkov, Abhishek Pandit-Subedi, Jameson Thies,
	Benson Leung, Tzung-Bi Shih, Greg Kroah-Hartman, Guenter Roeck,
	Pooja Katiyar, Badhri Jagan Sridharan, RD Babiera, linux-usb,
	linux-kernel, chrome-platform

On Tue, Jun 17, 2025 at 10:54 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi Andrei,
>
> On Mon, Jun 16, 2025 at 01:31:43PM +0000, Andrei Kuchynski wrote:
> > This reverts commit b4b38ffb38c91afd4dc387608db26f6fc34ed40b.
> >
> > The commit introduced a deadlock with the cros_ec_typec driver.
> > The deadlock occurs due to a recursive lock acquisition of
> > `cros_typec_altmode_work::mutex`.
> > The call chain is as follows:
> > 1. cros_typec_altmode_work() acquires the mutex
> > 2. typec_altmode_vdm() -> dp_altmode_vdm() ->
> > 3. typec_altmode_exit() -> cros_typec_altmode_exit()
> > 4. cros_typec_altmode_exit() attempts to acquire the mutex again
> >
> > This revert is considered safe as no other known driver sends back
> > DP_CMD_STATUS_UPDATE command with the NAK flag.
> >
> > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > ---
> >  drivers/usb/typec/altmodes/displayport.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> > index b09b58d7311d..ac84a6d64c2f 100644
> > --- a/drivers/usb/typec/altmodes/displayport.c
> > +++ b/drivers/usb/typec/altmodes/displayport.c
> > @@ -393,10 +393,6 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
> >               break;
> >       case CMDT_RSP_NAK:
> >               switch (cmd) {
> > -             case DP_CMD_STATUS_UPDATE:
> > -                     if (typec_altmode_exit(alt))
> > -                             dev_err(&dp->alt->dev, "Exit Mode Failed!\n");
> > -                     break;
>
> Commit b4b38ffb38c9 ("usb: typec: displayport: Receive DP Status
> Update NAK request exit dp altmode") addressed a very real problem
> with failure to execute data role swap. You are not really offering
> anything else for that issue here.

Thanks, I see the problem now. Reverting the patch is not feasible.

>
> Is it not an option to just schedule the mode exit here instead to
> solve the problem?

Of course, that's an option. Alternatively, maybe I could resolve the
deadlock within the `cros_ec_typec` driver. Regardless, this seems like
a separate patch.

>
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index b09b58d7311d..2abbe4de3216 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -394,8 +394,7 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
>         case CMDT_RSP_NAK:
>                 switch (cmd) {
>                 case DP_CMD_STATUS_UPDATE:
> -                       if (typec_altmode_exit(alt))
> -                               dev_err(&dp->alt->dev, "Exit Mode Failed!\n");
> +                       dp->state = DP_STATE_EXIT;
>                         break;
>                 case DP_CMD_CONFIGURE:
>                         dp->data.conf = 0;
>
>
> --
> heikki

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

* Re: [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs
  2025-06-17 12:38     ` Andrei Kuchynski
@ 2025-06-17 13:28       ` Greg Kroah-Hartman
  2025-06-18  2:47         ` Abhishek Pandit-Subedi
  0 siblings, 1 reply; 22+ messages in thread
From: Greg Kroah-Hartman @ 2025-06-17 13:28 UTC (permalink / raw)
  To: Andrei Kuchynski
  Cc: Heikki Krogerus, Dmitry Baryshkov, Abhishek Pandit-Subedi,
	Jameson Thies, Benson Leung, Tzung-Bi Shih, Guenter Roeck,
	Pooja Katiyar, Badhri Jagan Sridharan, RD Babiera, linux-usb,
	linux-kernel, chrome-platform

On Tue, Jun 17, 2025 at 02:38:04PM +0200, Andrei Kuchynski wrote:
> On Tue, Jun 17, 2025 at 11:50 AM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Mon, Jun 16, 2025 at 01:31:41PM +0000, Andrei Kuchynski wrote:
> > > This sysfs attribute specifies the preferred order for enabling
> > > DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> > >
> > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > ---
> > >  Documentation/ABI/testing/sysfs-class-typec | 17 ++++
> > >  drivers/usb/typec/Makefile                  |  2 +-
> > >  drivers/usb/typec/class.c                   | 26 ++++++
> > >  drivers/usb/typec/class.h                   |  2 +
> > >  drivers/usb/typec/mode_selection.c          | 93 +++++++++++++++++++++
> > >  drivers/usb/typec/mode_selection.h          |  5 ++
> > >  include/linux/usb/typec_altmode.h           |  7 ++
> > >  7 files changed, 151 insertions(+), 1 deletion(-)
> > >  create mode 100644 drivers/usb/typec/mode_selection.c
> > >  create mode 100644 drivers/usb/typec/mode_selection.h
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > > index 38e101c17a00..46eee82042ab 100644
> > > --- a/Documentation/ABI/testing/sysfs-class-typec
> > > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > > @@ -162,6 +162,23 @@ Description:     Lists the supported USB Modes. The default USB mode that is used
> > >               - usb3 (USB 3.2)
> > >               - usb4 (USB4)
> > >
> > > +What:                /sys/class/typec/<port>/altmode_priorities
> > > +Date:                June 2025
> > > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > > +Description: Lists the alternate modes supported by the port and their
> > > +             priorities. The priority setting determines the order in which
> > > +             Displayport alt-mode, Thunderbolt alt-mode and USB4 mode will be
> > > +             activated, indicating the preferred selection sequence. A value of -1
> > > +             disables automatic entry into a specific mode, while lower numbers
> > > +             indicate higher priority. The default priorities can be modified by
> > > +             assigning new values. Modes without explicitly set values default to -1,
> > > +             effectively disabling them.
> > > +
> > > +             Example values:
> > > +             - "USB4=0 TBT=1 DP=2"
> > > +             - "USB4=-1 TBT=0"
> > > +             - "DP=-1 USB4=-1 TBT=-1"
> >
> > No. If you want to disable entry to a mode by default, then you
> > deactivate that mode, so -1 is not needed. USB4 is also not an alt
> > mode, so this at the very least should be named differently.
> >
> > But I'm not sure this is the correct way to handle the modes in
> > general. Can you please explain what exactly is the use case you are
> > thinking?
> 
> Hi Heikki,
> 
> This implements the mode selection logic within the kernel, replacing
> its userspace counterpart. Implementing this in the kernel offers
> advantages, making the process both more robust and far easier to
> manage.
> This eliminates the need for user space to verify port, partner, or
> cable capabilities, and to control the mode entry process. User space
> doesn't even need to know if USB4 mode is supported by the port or
> partner; unsupported modes are automatically skipped.

But that's all things that userspace can do, so it should be doing it.
You need to justify why userspace can NOT do something in order to move
that logic into the kernel.

> User space's role is now limited to high-level tasks like security,
> with the kernel managing technical implementation. Mode selection and
> activation can occur without user space intervention at all if no
> special rules apply and the default policy (USB4 -> TBT -> DP) is
> acceptable.

What is wrong with "userspace intervention"?  Is the current api broken
somehow?

> > If you just need to prevent for example USB4 entry by default, then
> > you write usb3 (or usb2) to the usb_capability. The alt modes you can
> > deactivate by default, no?
> >
> > I can appreciate the convenience that you get from a single file like
> > that, but it does not really give much room to move if for example the
> > user space needs to behave differently in case of failures to enter a
> > specific mode compared to successful entries.
> >
> 
> You are right, this patch series is proposed for its convenience. It
> offers a more convenient method for users to enable or activate any
> mode, without altering existing methods. Users can decide whether they
> prioritize more control or an easier mode selection method.

So now you are going to maintain 2 different ways to do this for the
next 40+ years?  How are you going to test this over time to make sure
nothing breaks/changes?

thanks,

greg k-h

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

* Re: [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs
  2025-06-17 13:28       ` Greg Kroah-Hartman
@ 2025-06-18  2:47         ` Abhishek Pandit-Subedi
  2025-06-18 13:42           ` Heikki Krogerus
  0 siblings, 1 reply; 22+ messages in thread
From: Abhishek Pandit-Subedi @ 2025-06-18  2:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Andrei Kuchynski, Heikki Krogerus, Dmitry Baryshkov,
	Jameson Thies, Benson Leung, Tzung-Bi Shih, Guenter Roeck,
	Pooja Katiyar, Badhri Jagan Sridharan, RD Babiera, linux-usb,
	linux-kernel, chrome-platform

On Tue, Jun 17, 2025 at 6:28 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jun 17, 2025 at 02:38:04PM +0200, Andrei Kuchynski wrote:
> > On Tue, Jun 17, 2025 at 11:50 AM Heikki Krogerus
> > <heikki.krogerus@linux.intel.com> wrote:
> > >
> > > On Mon, Jun 16, 2025 at 01:31:41PM +0000, Andrei Kuchynski wrote:
> > > > This sysfs attribute specifies the preferred order for enabling
> > > > DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> > > >
> > > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > > ---
> > > >  Documentation/ABI/testing/sysfs-class-typec | 17 ++++
> > > >  drivers/usb/typec/Makefile                  |  2 +-
> > > >  drivers/usb/typec/class.c                   | 26 ++++++
> > > >  drivers/usb/typec/class.h                   |  2 +
> > > >  drivers/usb/typec/mode_selection.c          | 93 +++++++++++++++++++++
> > > >  drivers/usb/typec/mode_selection.h          |  5 ++
> > > >  include/linux/usb/typec_altmode.h           |  7 ++
> > > >  7 files changed, 151 insertions(+), 1 deletion(-)
> > > >  create mode 100644 drivers/usb/typec/mode_selection.c
> > > >  create mode 100644 drivers/usb/typec/mode_selection.h
> > > >
> > > > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > > > index 38e101c17a00..46eee82042ab 100644
> > > > --- a/Documentation/ABI/testing/sysfs-class-typec
> > > > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > > > @@ -162,6 +162,23 @@ Description:     Lists the supported USB Modes. The default USB mode that is used
> > > >               - usb3 (USB 3.2)
> > > >               - usb4 (USB4)
> > > >
> > > > +What:                /sys/class/typec/<port>/altmode_priorities
> > > > +Date:                June 2025
> > > > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > > > +Description: Lists the alternate modes supported by the port and their
> > > > +             priorities. The priority setting determines the order in which
> > > > +             Displayport alt-mode, Thunderbolt alt-mode and USB4 mode will be
> > > > +             activated, indicating the preferred selection sequence. A value of -1
> > > > +             disables automatic entry into a specific mode, while lower numbers
> > > > +             indicate higher priority. The default priorities can be modified by
> > > > +             assigning new values. Modes without explicitly set values default to -1,
> > > > +             effectively disabling them.
> > > > +
> > > > +             Example values:
> > > > +             - "USB4=0 TBT=1 DP=2"
> > > > +             - "USB4=-1 TBT=0"
> > > > +             - "DP=-1 USB4=-1 TBT=-1"
> > >
> > > No. If you want to disable entry to a mode by default, then you
> > > deactivate that mode, so -1 is not needed. USB4 is also not an alt
> > > mode, so this at the very least should be named differently.

USB4 should be called an alt-mode though based on its behavior and I
think the spec makes a mistake not doing so. It's mutually exclusive
with alternate modes, it modifies the functionality of the data lanes
and it's not a new data mode (just a tunneling protocol). But that's
an aside for this patch...

> > >
> > > But I'm not sure this is the correct way to handle the modes in
> > > general. Can you please explain what exactly is the use case you are
> > > thinking?
> >
> > Hi Heikki,
> >
> > This implements the mode selection logic within the kernel, replacing
> > its userspace counterpart. Implementing this in the kernel offers
> > advantages, making the process both more robust and far easier to
> > manage.
> > This eliminates the need for user space to verify port, partner, or
> > cable capabilities, and to control the mode entry process. User space
> > doesn't even need to know if USB4 mode is supported by the port or
> > partner; unsupported modes are automatically skipped.
>
> But that's all things that userspace can do, so it should be doing it.
> You need to justify why userspace can NOT do something in order to move
> that logic into the kernel.

Userspace "can" do it but it is not doing it today. There is no
userspace alt-mode manager (that I'm aware of) that's writing to
/sys/class/typec (typecd in ChromeOS only reads and does mode
entry/exit via a sideband mechanism) and boltd (userspace thunderbolt
manager; https://github.com/gicmo/bolt) only deals with tunnels and
not mode entry directly.

I think the inclusion of TBT and USB4 in the kernel makes this change
necessary and the existing way of doing it is not widely used yet.

>
> > User space's role is now limited to high-level tasks like security,
> > with the kernel managing technical implementation. Mode selection and
> > activation can occur without user space intervention at all if no
> > special rules apply and the default policy (USB4 -> TBT -> DP) is
> > acceptable.
>
> What is wrong with "userspace intervention"?  Is the current api broken
> somehow?

There were some previous discussions on this:
https://lore.kernel.org/linux-usb/ZyjW0CMXgGIt-usC@kuha.fi.intel.com/
https://lore.kernel.org/chrome-platform/CAA8EJpqiF_0bgYT8boFa4UPJWcxgw89mmfbdMVKeAP-xnFOP4g@mail.gmail.com/

Some background:
* The Type-C port driver will add a partner alt-mode to the altmode bus.
* An altmode driver will probe and attach if available on the system.
    * Currently, the probe function of these altmode drivers always
enter the given mode. The expectation is that if a system allows mode
override, it should always be done by the alt mode driver. This
doesn't scale beyond one alt-mode driver – between DP and TBT, which
altmode driver should win entry? Prior to TBT driver being added in
December and USB4 support circa similar time frame, DP was always the
default choice if override was supported.
    * Userspace could make this decision but then what about USB4? The
current APIs don't expose USB4 information in the same way and the
auto-enter behavior is different (it doesn't enter).

In a lot of existing designs, mode entry is entirely handled by the
firmware and no mode selection is possible from the kernel. This is
covered by patch 3 of this series (alt_mode_override) flag. Windows
designs usually fall into this category as they use PD controllers
implementing UCSI (and usually a very old UCSI version 1.2).

A few designs allow overriding the current mode:
* Chromebooks with cros_ec_ucsi which support Thunderbolt will allow
override of the alternate mode (via UCSI set_new_cam) and USB mode
(via UCSI Set_USB). They will enter DPAM instead of TBT by default if
possible.
* Chromebooks with the cros_ec_typec driver which support Thunderbolt
will require the OS to make the mode entry decision (no mode entry by
default on attach).
* Pixel phone TCPM currently requires mode entry from the kernel as
there's no userspace mode selection component.
* Potentially some Windows designs that implement AltMode override via
UCSI (ucsi_acpi). These should behave similarly to cros_ec_ucsi but
will likely automatically enter TBT mode.

In these cases, the kernel is a better place to make alternate mode decisions:
* Quicker than having a userspace implementation (and more consistent
during boot).
* Works consistently across drivers and accounts for both overridable
and non-overridable systems.

The main thing userspace should really be doing with mode entry is
setting policy around what modes they want to enter instead of doing
low level checking of compatibility and sequencing of mode entry
operations. And the main policy we're trying to enforce on ChromeOS is
to not enter TBT by default (based on our security posture described
at https://www.chromium.org/chromium-os/developer-library/reference/security/usb4/).

Given that Thunderbolt altmode driver support and USB4 support was
only added around December of last year, I don't imagine there are
many userspace usages yet and we should fix these API issues before
they become too widely used. I think mode sequencing belongs in the
kernel and not userspace.


>
> > > If you just need to prevent for example USB4 entry by default, then
> > > you write usb3 (or usb2) to the usb_capability. The alt modes you can
> > > deactivate by default, no?
> > >
> > > I can appreciate the convenience that you get from a single file like
> > > that, but it does not really give much room to move if for example the
> > > user space needs to behave differently in case of failures to enter a
> > > specific mode compared to successful entries.
> > >
> >
> > You are right, this patch series is proposed for its convenience. It
> > offers a more convenient method for users to enable or activate any
> > mode, without altering existing methods. Users can decide whether they
> > prioritize more control or an easier mode selection method.
>
> So now you are going to maintain 2 different ways to do this for the
> next 40+ years?  How are you going to test this over time to make sure
> nothing breaks/changes?

While the current sysfs apis for altmode + usb are useful for testing,
they aren't as useful for normal user use-cases. Deprecating or moving
this control to debugfs and only exposing the
mode_selection/mode_priority fields would be my choice going forward.
The mode selection work currently does go through the same typec APIs
as the sysfs entries so we gain test coverage of driver behavior using
either API. If we need to retain the old APIs, I think we'll need to
add some Kunit tests to make sure behavior is consistent across the
two ways of doing things.

>
> thanks,
>
> greg k-h

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

* Re: [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs
  2025-06-18  2:47         ` Abhishek Pandit-Subedi
@ 2025-06-18 13:42           ` Heikki Krogerus
  2025-06-18 19:00             ` Abhishek Pandit-Subedi
  0 siblings, 1 reply; 22+ messages in thread
From: Heikki Krogerus @ 2025-06-18 13:42 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: Greg Kroah-Hartman, Andrei Kuchynski, Dmitry Baryshkov,
	Jameson Thies, Benson Leung, Tzung-Bi Shih, Guenter Roeck,
	Pooja Katiyar, Badhri Jagan Sridharan, RD Babiera, linux-usb,
	linux-kernel, chrome-platform

Hi Abhishek,

On Tue, Jun 17, 2025 at 07:47:18PM -0700, Abhishek Pandit-Subedi wrote:
> On Tue, Jun 17, 2025 at 6:28 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Jun 17, 2025 at 02:38:04PM +0200, Andrei Kuchynski wrote:
> > > On Tue, Jun 17, 2025 at 11:50 AM Heikki Krogerus
> > > <heikki.krogerus@linux.intel.com> wrote:
> > > >
> > > > On Mon, Jun 16, 2025 at 01:31:41PM +0000, Andrei Kuchynski wrote:
> > > > > This sysfs attribute specifies the preferred order for enabling
> > > > > DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> > > > >
> > > > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > > > ---
> > > > >  Documentation/ABI/testing/sysfs-class-typec | 17 ++++
> > > > >  drivers/usb/typec/Makefile                  |  2 +-
> > > > >  drivers/usb/typec/class.c                   | 26 ++++++
> > > > >  drivers/usb/typec/class.h                   |  2 +
> > > > >  drivers/usb/typec/mode_selection.c          | 93 +++++++++++++++++++++
> > > > >  drivers/usb/typec/mode_selection.h          |  5 ++
> > > > >  include/linux/usb/typec_altmode.h           |  7 ++
> > > > >  7 files changed, 151 insertions(+), 1 deletion(-)
> > > > >  create mode 100644 drivers/usb/typec/mode_selection.c
> > > > >  create mode 100644 drivers/usb/typec/mode_selection.h
> > > > >
> > > > > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > > > > index 38e101c17a00..46eee82042ab 100644
> > > > > --- a/Documentation/ABI/testing/sysfs-class-typec
> > > > > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > > > > @@ -162,6 +162,23 @@ Description:     Lists the supported USB Modes. The default USB mode that is used
> > > > >               - usb3 (USB 3.2)
> > > > >               - usb4 (USB4)
> > > > >
> > > > > +What:                /sys/class/typec/<port>/altmode_priorities
> > > > > +Date:                June 2025
> > > > > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > > > > +Description: Lists the alternate modes supported by the port and their
> > > > > +             priorities. The priority setting determines the order in which
> > > > > +             Displayport alt-mode, Thunderbolt alt-mode and USB4 mode will be
> > > > > +             activated, indicating the preferred selection sequence. A value of -1
> > > > > +             disables automatic entry into a specific mode, while lower numbers
> > > > > +             indicate higher priority. The default priorities can be modified by
> > > > > +             assigning new values. Modes without explicitly set values default to -1,
> > > > > +             effectively disabling them.
> > > > > +
> > > > > +             Example values:
> > > > > +             - "USB4=0 TBT=1 DP=2"
> > > > > +             - "USB4=-1 TBT=0"
> > > > > +             - "DP=-1 USB4=-1 TBT=-1"
> > > >
> > > > No. If you want to disable entry to a mode by default, then you
> > > > deactivate that mode, so -1 is not needed. USB4 is also not an alt
> > > > mode, so this at the very least should be named differently.
> 
> USB4 should be called an alt-mode though based on its behavior and I
> think the spec makes a mistake not doing so. It's mutually exclusive
> with alternate modes, it modifies the functionality of the data lanes
> and it's not a new data mode (just a tunneling protocol). But that's
> an aside for this patch...

It's still a big no from me for this one as it is, sorry. You are
trying to achieve too many things with these files.

The first thing here is that you want to offer the user a way to
define the order in which the modes are entered, and that's actually
something that we do *not* have any support for at the moment. I'm not
against this part. As long as you justify the need for this, and
ideally offer a way to setup the priorities also in tcpm.c, I'm okay
with it. But you do need to justify the need for this IMO. The specs
actually dictate the order in which these modes should be entered does
it not

But the second part is that you want to also use this file to disable
the modes. For that we do have the mechanisms already in place, so
there is no point in trying to include that here. It does not even
sound like the primary goal for you - your main goal is to just be
able to define the order, right?

So this is my suggestion: Change this so that you can only supply the
preferred order for the modes - the priorities. Get rid of the numeric
values - by simply listing the modes in the file becomes more machine
readable while still being perfectly human readable.

And the name. I agree that USB4 should be an alt mode, but it's not.
This should really be a no-brainer. Just s/altmode/mode/. That way
the naming is aligned with what you are proposing in patch 5/6.

> > > > But I'm not sure this is the correct way to handle the modes in
> > > > general. Can you please explain what exactly is the use case you are
> > > > thinking?
> > >
> > > Hi Heikki,
> > >
> > > This implements the mode selection logic within the kernel, replacing
> > > its userspace counterpart. Implementing this in the kernel offers
> > > advantages, making the process both more robust and far easier to
> > > manage.
> > > This eliminates the need for user space to verify port, partner, or
> > > cable capabilities, and to control the mode entry process. User space
> > > doesn't even need to know if USB4 mode is supported by the port or
> > > partner; unsupported modes are automatically skipped.
> >
> > But that's all things that userspace can do, so it should be doing it.
> > You need to justify why userspace can NOT do something in order to move
> > that logic into the kernel.
> 
> Userspace "can" do it but it is not doing it today. There is no
> userspace alt-mode manager (that I'm aware of) that's writing to
> /sys/class/typec (typecd in ChromeOS only reads and does mode
> entry/exit via a sideband mechanism) and boltd (userspace thunderbolt
> manager; https://github.com/gicmo/bolt) only deals with tunnels and
> not mode entry directly.
> 
> I think the inclusion of TBT and USB4 in the kernel makes this change
> necessary and the existing way of doing it is not widely used yet.
> 
> >
> > > User space's role is now limited to high-level tasks like security,
> > > with the kernel managing technical implementation. Mode selection and
> > > activation can occur without user space intervention at all if no
> > > special rules apply and the default policy (USB4 -> TBT -> DP) is
> > > acceptable.
> >
> > What is wrong with "userspace intervention"?  Is the current api broken
> > somehow?
> 
> There were some previous discussions on this:
> https://lore.kernel.org/linux-usb/ZyjW0CMXgGIt-usC@kuha.fi.intel.com/
> https://lore.kernel.org/chrome-platform/CAA8EJpqiF_0bgYT8boFa4UPJWcxgw89mmfbdMVKeAP-xnFOP4g@mail.gmail.com/
> 
> Some background:
> * The Type-C port driver will add a partner alt-mode to the altmode bus.
> * An altmode driver will probe and attach if available on the system.
>     * Currently, the probe function of these altmode drivers always
> enter the given mode. The expectation is that if a system allows mode
> override, it should always be done by the alt mode driver. This
> doesn't scale beyond one alt-mode driver – between DP and TBT, which
> altmode driver should win entry? Prior to TBT driver being added in
> December and USB4 support circa similar time frame, DP was always the
> default choice if override was supported.
>     * Userspace could make this decision but then what about USB4? The
> current APIs don't expose USB4 information in the same way and the
> auto-enter behavior is different (it doesn't enter).
> 
> In a lot of existing designs, mode entry is entirely handled by the
> firmware and no mode selection is possible from the kernel. This is
> covered by patch 3 of this series (alt_mode_override) flag. Windows
> designs usually fall into this category as they use PD controllers
> implementing UCSI (and usually a very old UCSI version 1.2).
> 
> A few designs allow overriding the current mode:
> * Chromebooks with cros_ec_ucsi which support Thunderbolt will allow
> override of the alternate mode (via UCSI set_new_cam) and USB mode
> (via UCSI Set_USB). They will enter DPAM instead of TBT by default if
> possible.
> * Chromebooks with the cros_ec_typec driver which support Thunderbolt
> will require the OS to make the mode entry decision (no mode entry by
> default on attach).
> * Pixel phone TCPM currently requires mode entry from the kernel as
> there's no userspace mode selection component.
> * Potentially some Windows designs that implement AltMode override via
> UCSI (ucsi_acpi). These should behave similarly to cros_ec_ucsi but
> will likely automatically enter TBT mode.
> 
> In these cases, the kernel is a better place to make alternate mode decisions:
> * Quicker than having a userspace implementation (and more consistent
> during boot).
> * Works consistently across drivers and accounts for both overridable
> and non-overridable systems.
> 
> The main thing userspace should really be doing with mode entry is
> setting policy around what modes they want to enter instead of doing
> low level checking of compatibility and sequencing of mode entry
> operations. And the main policy we're trying to enforce on ChromeOS is
> to not enter TBT by default (based on our security posture described
> at https://www.chromium.org/chromium-os/developer-library/reference/security/usb4/).
> 
> Given that Thunderbolt altmode driver support and USB4 support was
> only added around December of last year, I don't imagine there are
> many userspace usages yet and we should fix these API issues before
> they become too widely used. I think mode sequencing belongs in the
> kernel and not userspace.

My two cents - I'm sure this is clear to most of you, but not to
everybody.

The problem with the USB Type-C framework that I wrote is that it was
primary designed to support tcpm.c, but unfortunately it does not work
very well with the PD controllers when it comes to the alternate modes.

With tcpm.c we need to handle the alternate mode communication inside
kernel, but with PD controller we of course can't do that. The altmode
bus and altmode drivers are not needed with PD controllers, in fact
they only complicate things with the PD controllers.

So there are basically two separate ways of handling the same thing.
That's why I think we may need something like these files that you
guys are suggesting. We need a way to support the controlling of
alternate modes with PD controllers, or perhaps I should say
with interfaces that support it.

But that said, this series is not acceptable as it is quite yet. You
are trying to push a lot of information and control to these couple of
files which I'm not convinced is always necessary, and possibly even
prone to errors. I'm also really concerned that you want to only deal
with a predefined list of modes. It will probable not be a problem as
long as we can fallback to ABI that we already have. But during my
short career I've already seen several cases where assumptions have
been made about how a specific technology will be used, and it has in
practice always backfired.

thanks,

-- 
heikki

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

* Re: [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs
  2025-06-18 13:42           ` Heikki Krogerus
@ 2025-06-18 19:00             ` Abhishek Pandit-Subedi
  0 siblings, 0 replies; 22+ messages in thread
From: Abhishek Pandit-Subedi @ 2025-06-18 19:00 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Andrei Kuchynski, Dmitry Baryshkov,
	Jameson Thies, Benson Leung, Tzung-Bi Shih, Guenter Roeck,
	Pooja Katiyar, Badhri Jagan Sridharan, RD Babiera, linux-usb,
	linux-kernel, chrome-platform

On Wed, Jun 18, 2025 at 6:42 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi Abhishek,
>
> On Tue, Jun 17, 2025 at 07:47:18PM -0700, Abhishek Pandit-Subedi wrote:
> > On Tue, Jun 17, 2025 at 6:28 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, Jun 17, 2025 at 02:38:04PM +0200, Andrei Kuchynski wrote:
> > > > On Tue, Jun 17, 2025 at 11:50 AM Heikki Krogerus
> > > > <heikki.krogerus@linux.intel.com> wrote:
> > > > >
> > > > > On Mon, Jun 16, 2025 at 01:31:41PM +0000, Andrei Kuchynski wrote:
> > > > > > This sysfs attribute specifies the preferred order for enabling
> > > > > > DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> > > > > >
> > > > > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > > > > ---
> > > > > >  Documentation/ABI/testing/sysfs-class-typec | 17 ++++
> > > > > >  drivers/usb/typec/Makefile                  |  2 +-
> > > > > >  drivers/usb/typec/class.c                   | 26 ++++++
> > > > > >  drivers/usb/typec/class.h                   |  2 +
> > > > > >  drivers/usb/typec/mode_selection.c          | 93 +++++++++++++++++++++
> > > > > >  drivers/usb/typec/mode_selection.h          |  5 ++
> > > > > >  include/linux/usb/typec_altmode.h           |  7 ++
> > > > > >  7 files changed, 151 insertions(+), 1 deletion(-)
> > > > > >  create mode 100644 drivers/usb/typec/mode_selection.c
> > > > > >  create mode 100644 drivers/usb/typec/mode_selection.h
> > > > > >
> > > > > > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > > > > > index 38e101c17a00..46eee82042ab 100644
> > > > > > --- a/Documentation/ABI/testing/sysfs-class-typec
> > > > > > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > > > > > @@ -162,6 +162,23 @@ Description:     Lists the supported USB Modes. The default USB mode that is used
> > > > > >               - usb3 (USB 3.2)
> > > > > >               - usb4 (USB4)
> > > > > >
> > > > > > +What:                /sys/class/typec/<port>/altmode_priorities
> > > > > > +Date:                June 2025
> > > > > > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > > > > > +Description: Lists the alternate modes supported by the port and their
> > > > > > +             priorities. The priority setting determines the order in which
> > > > > > +             Displayport alt-mode, Thunderbolt alt-mode and USB4 mode will be
> > > > > > +             activated, indicating the preferred selection sequence. A value of -1
> > > > > > +             disables automatic entry into a specific mode, while lower numbers
> > > > > > +             indicate higher priority. The default priorities can be modified by
> > > > > > +             assigning new values. Modes without explicitly set values default to -1,
> > > > > > +             effectively disabling them.
> > > > > > +
> > > > > > +             Example values:
> > > > > > +             - "USB4=0 TBT=1 DP=2"
> > > > > > +             - "USB4=-1 TBT=0"
> > > > > > +             - "DP=-1 USB4=-1 TBT=-1"
> > > > >
> > > > > No. If you want to disable entry to a mode by default, then you
> > > > > deactivate that mode, so -1 is not needed. USB4 is also not an alt
> > > > > mode, so this at the very least should be named differently.
> >
> > USB4 should be called an alt-mode though based on its behavior and I
> > think the spec makes a mistake not doing so. It's mutually exclusive
> > with alternate modes, it modifies the functionality of the data lanes
> > and it's not a new data mode (just a tunneling protocol). But that's
> > an aside for this patch...
>
> It's still a big no from me for this one as it is, sorry. You are
> trying to achieve too many things with these files.
>
> The first thing here is that you want to offer the user a way to
> define the order in which the modes are entered, and that's actually
> something that we do *not* have any support for at the moment. I'm not
> against this part. As long as you justify the need for this, and
> ideally offer a way to setup the priorities also in tcpm.c, I'm okay
> with it. But you do need to justify the need for this IMO. The specs
> actually dictate the order in which these modes should be entered does
> it not

The spec lists an example mode entry flow with cable warnings (USB
Type-C Spec 2.3, Section E.5) but it's not dictating the order there.
bleung@ and jthies@ contributed this to the spec and this somewhat
matches what ChromeOS does.

The main reason for having a priorities list in sysfs (vs statically
defined) is to reflect our security policy around PCI tunnels: we do
not allow PCI tunnels unless the user is logged in and has toggled the
flag to enable PCI tunnels (which is controllable via the UI or
Enterprise policy). What this results in is a preference to enter DPAM
when in the login screen even if the attached device supports
Thunderbolt and then a mode switch to Thunderbolt after login. This
was done on older ChromeOS platforms by bypassing sysfs and talking
directly to the EC. With our adoption of PDC and UCSI, we want a
common way to do this without having mode sequencing occuring in
userspace.

The eventual operational goal being:
* A PDC based system will automatically enter an alt-mode in the PDC
by default but allow override later based on the defined mode
priority.
* Our older TCPM based systems that do AP driven mode entry will use
the in-kernel mode selection implementation to automatically enter
modes.
* In-kernel TCPM system will also automatically enter an alt-mode
using the in-kernel mode selection implementation.
* Userspace can change the mode priorities when a user logs in and
triggers mode selection. A new connection while logged in would have
the kernel apply the preferred mode.
* Userspace can change mode priorities when the user locks the screen
but does not trigger mode selection. Only new connections would get
the changed priority and existing connections are left alone.

Our preferred alt-mode ordering:
* Default => USB4, DPAM, TBT3
* Logged-in, Tunneling allowed => USB4, TBT3, DPAM

>
> But the second part is that you want to also use this file to disable
> the modes. For that we do have the mechanisms already in place, so
> there is no point in trying to include that here. It does not even
> sound like the primary goal for you - your main goal is to just be
> able to define the order, right?

Limiting to just priority in this file sounds good and simplifies this
change. It does seem to duplicate checks already existing in
`altmode->activate`.

>
> So this is my suggestion: Change this so that you can only supply the
> preferred order for the modes - the priorities. Get rid of the numeric
> values - by simply listing the modes in the file becomes more machine
> readable while still being perfectly human readable.
>
> And the name. I agree that USB4 should be an alt mode, but it's not.
> This should really be a no-brainer. Just s/altmode/mode/. That way
> the naming is aligned with what you are proposing in patch 5/6.

`mode_priorities` and `mode_selection` sounds good.

>
> > > > > But I'm not sure this is the correct way to handle the modes in
> > > > > general. Can you please explain what exactly is the use case you are
> > > > > thinking?
> > > >
> > > > Hi Heikki,
> > > >
> > > > This implements the mode selection logic within the kernel, replacing
> > > > its userspace counterpart. Implementing this in the kernel offers
> > > > advantages, making the process both more robust and far easier to
> > > > manage.
> > > > This eliminates the need for user space to verify port, partner, or
> > > > cable capabilities, and to control the mode entry process. User space
> > > > doesn't even need to know if USB4 mode is supported by the port or
> > > > partner; unsupported modes are automatically skipped.
> > >
> > > But that's all things that userspace can do, so it should be doing it.
> > > You need to justify why userspace can NOT do something in order to move
> > > that logic into the kernel.
> >
> > Userspace "can" do it but it is not doing it today. There is no
> > userspace alt-mode manager (that I'm aware of) that's writing to
> > /sys/class/typec (typecd in ChromeOS only reads and does mode
> > entry/exit via a sideband mechanism) and boltd (userspace thunderbolt
> > manager; https://github.com/gicmo/bolt) only deals with tunnels and
> > not mode entry directly.
> >
> > I think the inclusion of TBT and USB4 in the kernel makes this change
> > necessary and the existing way of doing it is not widely used yet.
> >
> > >
> > > > User space's role is now limited to high-level tasks like security,
> > > > with the kernel managing technical implementation. Mode selection and
> > > > activation can occur without user space intervention at all if no
> > > > special rules apply and the default policy (USB4 -> TBT -> DP) is
> > > > acceptable.
> > >
> > > What is wrong with "userspace intervention"?  Is the current api broken
> > > somehow?
> >
> > There were some previous discussions on this:
> > https://lore.kernel.org/linux-usb/ZyjW0CMXgGIt-usC@kuha.fi.intel.com/
> > https://lore.kernel.org/chrome-platform/CAA8EJpqiF_0bgYT8boFa4UPJWcxgw89mmfbdMVKeAP-xnFOP4g@mail.gmail.com/
> >
> > Some background:
> > * The Type-C port driver will add a partner alt-mode to the altmode bus.
> > * An altmode driver will probe and attach if available on the system.
> >     * Currently, the probe function of these altmode drivers always
> > enter the given mode. The expectation is that if a system allows mode
> > override, it should always be done by the alt mode driver. This
> > doesn't scale beyond one alt-mode driver – between DP and TBT, which
> > altmode driver should win entry? Prior to TBT driver being added in
> > December and USB4 support circa similar time frame, DP was always the
> > default choice if override was supported.
> >     * Userspace could make this decision but then what about USB4? The
> > current APIs don't expose USB4 information in the same way and the
> > auto-enter behavior is different (it doesn't enter).
> >
> > In a lot of existing designs, mode entry is entirely handled by the
> > firmware and no mode selection is possible from the kernel. This is
> > covered by patch 3 of this series (alt_mode_override) flag. Windows
> > designs usually fall into this category as they use PD controllers
> > implementing UCSI (and usually a very old UCSI version 1.2).
> >
> > A few designs allow overriding the current mode:
> > * Chromebooks with cros_ec_ucsi which support Thunderbolt will allow
> > override of the alternate mode (via UCSI set_new_cam) and USB mode
> > (via UCSI Set_USB). They will enter DPAM instead of TBT by default if
> > possible.
> > * Chromebooks with the cros_ec_typec driver which support Thunderbolt
> > will require the OS to make the mode entry decision (no mode entry by
> > default on attach).
> > * Pixel phone TCPM currently requires mode entry from the kernel as
> > there's no userspace mode selection component.
> > * Potentially some Windows designs that implement AltMode override via
> > UCSI (ucsi_acpi). These should behave similarly to cros_ec_ucsi but
> > will likely automatically enter TBT mode.
> >
> > In these cases, the kernel is a better place to make alternate mode decisions:
> > * Quicker than having a userspace implementation (and more consistent
> > during boot).
> > * Works consistently across drivers and accounts for both overridable
> > and non-overridable systems.
> >
> > The main thing userspace should really be doing with mode entry is
> > setting policy around what modes they want to enter instead of doing
> > low level checking of compatibility and sequencing of mode entry
> > operations. And the main policy we're trying to enforce on ChromeOS is
> > to not enter TBT by default (based on our security posture described
> > at https://www.chromium.org/chromium-os/developer-library/reference/security/usb4/).
> >
> > Given that Thunderbolt altmode driver support and USB4 support was
> > only added around December of last year, I don't imagine there are
> > many userspace usages yet and we should fix these API issues before
> > they become too widely used. I think mode sequencing belongs in the
> > kernel and not userspace.
>
> My two cents - I'm sure this is clear to most of you, but not to
> everybody.
>
> The problem with the USB Type-C framework that I wrote is that it was
> primary designed to support tcpm.c, but unfortunately it does not work
> very well with the PD controllers when it comes to the alternate modes.
>
> With tcpm.c we need to handle the alternate mode communication inside
> kernel, but with PD controller we of course can't do that. The altmode
> bus and altmode drivers are not needed with PD controllers, in fact
> they only complicate things with the PD controllers.
>
> So there are basically two separate ways of handling the same thing.
> That's why I think we may need something like these files that you
> guys are suggesting. We need a way to support the controlling of
> alternate modes with PD controllers, or perhaps I should say
> with interfaces that support it.

Making this sysfs only visible on drivers that support in-kernel mode
selection is probably a good idea to reduce confusion.
typec_attr_is_visible should be updated for the `mode_priorities`
attribute to reflect that.

Patch 1 in this series makes the `active` sysfs field reflect the
`read_only` nature of the altmode as well, which should help make the
API clearer for PD controller based systems.

>
> But that said, this series is not acceptable as it is quite yet. You
> are trying to push a lot of information and control to these couple of
> files which I'm not convinced is always necessary, and possibly even
> prone to errors. I'm also really concerned that you want to only deal
> with a predefined list of modes. It will probable not be a problem as
> long as we can fallback to ABI that we already have. But during my
> short career I've already seen several cases where assumptions have
> been made about how a specific technology will be used, and it has in
> practice always backfired.

That's fair. With respect to the `predefined list of modes`, these are
all the spec defined alternate-modes (and USB4).

I understand your reservations about the predefined list though. I
think I'd want to see this file have a superset of all kernel
supported modes and have port drivers enable support accordingly.
Right now, the nvidia driver is the only one un-accounted for but that
looks like it's just remapping DP to another SVID. Converting the
TYPEC_SVID_TO_ALTMODE macro to an inline function with a switch
statement would cover that.

>
> thanks,
>
> --
> heikki

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

end of thread, other threads:[~2025-06-18 19:01 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-16 13:31 [PATCH 00/10] USB Type-C mode selection Andrei Kuchynski
2025-06-16 13:31 ` [PATCH 01/10] usb: typec: Add alt_mode_override field to port property Andrei Kuchynski
2025-06-16 13:31 ` [PATCH 02/10] platform/chrome: cros_ec_typec: Set alt_mode_override flag Andrei Kuchynski
2025-06-16 13:31 ` [PATCH 03/10] usb: typec: ucsi: " Andrei Kuchynski
2025-06-16 13:31 ` [PATCH 04/10] usb: typec: Expose alternate mode priorities via sysfs Andrei Kuchynski
2025-06-17  9:50   ` Heikki Krogerus
2025-06-17 12:38     ` Andrei Kuchynski
2025-06-17 13:28       ` Greg Kroah-Hartman
2025-06-18  2:47         ` Abhishek Pandit-Subedi
2025-06-18 13:42           ` Heikki Krogerus
2025-06-18 19:00             ` Abhishek Pandit-Subedi
2025-06-16 13:31 ` [PATCH 05/10] usb: typec: Implement automated alternate mode selection Andrei Kuchynski
2025-06-16 13:31 ` [PATCH 06/10] Revert "usb: typec: displayport: Receive DP Status Update NAK request exit dp altmode" Andrei Kuchynski
2025-06-16 14:15   ` Greg Kroah-Hartman
2025-06-16 19:42     ` Andrei Kuchynski
2025-06-17  8:54   ` Heikki Krogerus
2025-06-17 12:54     ` Andrei Kuchynski
2025-06-16 13:31 ` [PATCH 07/10] usb: typec: Report altmode entry status via callback Andrei Kuchynski
2025-06-16 13:31 ` [PATCH 08/10] usb: typec: ucsi: displayport: Propagate DP altmode entry result Andrei Kuchynski
2025-06-16 13:31 ` [PATCH 09/10] platform/chrome: cros_ec_typec: Propagate " Andrei Kuchynski
2025-06-16 13:31 ` [PATCH 10/10] platform/chrome: cros_ec_typec: Report USB4 mode entry status via callback Andrei Kuchynski
2025-06-16 13:34 ` [PATCH 00/10] USB Type-C mode selection Dmitry Baryshkov

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