public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/5] USB Type-C alternate mode selection
@ 2025-09-09 12:30 Andrei Kuchynski
  2025-09-09 12:30 ` [PATCH RFC 1/5] usb: typec: Implement " Andrei Kuchynski
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Andrei Kuchynski @ 2025-09-09 12:30 UTC (permalink / raw)
  To: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
	Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform
  Cc: Guenter Roeck, Greg Kroah-Hartman, linux-kernel, 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.

New sysfs `mode_selection` attribute is exposed to provide user control
over mode selection. It triggers an alternate mode negotiation.
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 the 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 an Android OS device with kernel 6.16.
This series depends on the 'USB Type-C alternate mode priorities' series:
https://lore.kernel.org/all/20250905142206.4105351-1-akuchynski@chromium.org/ 

Andrei Kuchynski (5):
  usb: typec: Implement mode selection
  usb: typec: Expose mode_selection attribute via sysfs
  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

 Documentation/ABI/testing/sysfs-class-typec  |  11 +
 drivers/platform/chrome/cros_ec_typec.c      |   9 +
 drivers/platform/chrome/cros_typec_altmode.c |  32 +-
 drivers/platform/chrome/cros_typec_altmode.h |   6 +
 drivers/usb/typec/altmodes/displayport.c     |  19 +-
 drivers/usb/typec/altmodes/thunderbolt.c     |  10 +
 drivers/usb/typec/class.c                    |  37 ++
 drivers/usb/typec/class.h                    |   4 +
 drivers/usb/typec/mode_selection.c           | 345 +++++++++++++++++++
 drivers/usb/typec/mode_selection.h           |  25 ++
 drivers/usb/typec/ucsi/displayport.c         |  10 +-
 include/linux/usb/typec_altmode.h            |  11 +
 include/linux/usb/typec_dp.h                 |   2 +
 include/linux/usb/typec_tbt.h                |   3 +
 14 files changed, 516 insertions(+), 8 deletions(-)

-- 
2.51.0.384.g4c02a37b29-goog


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

* [PATCH RFC 1/5] usb: typec: Implement mode selection
  2025-09-09 12:30 [PATCH RFC 0/5] USB Type-C alternate mode selection Andrei Kuchynski
@ 2025-09-09 12:30 ` Andrei Kuchynski
  2025-09-09 12:30 ` [PATCH RFC 2/5] usb: typec: Expose mode_selection attribute via sysfs Andrei Kuchynski
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Andrei Kuchynski @ 2025-09-09 12:30 UTC (permalink / raw)
  To: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
	Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform
  Cc: Guenter Roeck, Greg Kroah-Hartman, linux-kernel, Andrei Kuchynski

This patch adds APIs for managing the alternate mode selection process,
enabling the initiation and termination of mode entry based on each
mode's priority.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/usb/typec/class.h          |   4 +
 drivers/usb/typec/mode_selection.c | 345 +++++++++++++++++++++++++++++
 drivers/usb/typec/mode_selection.h |  25 +++
 include/linux/usb/typec_altmode.h  |  11 +
 4 files changed, 385 insertions(+)

diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index c53a04b9dc75..f3e731616f41 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -40,6 +40,10 @@ struct typec_partner {
 
 	struct usb_power_delivery	*pd;
 
+	struct list_head		mode_list;
+	struct mutex			mode_list_lock;
+	struct delayed_work		mode_selection_work;
+
 	void (*attach)(struct typec_partner *partner, struct device *dev);
 	void (*deattach)(struct typec_partner *partner, struct device *dev);
 };
diff --git a/drivers/usb/typec/mode_selection.c b/drivers/usb/typec/mode_selection.c
index 2179bf25f5d4..dbeb94f7f3e6 100644
--- a/drivers/usb/typec/mode_selection.c
+++ b/drivers/usb/typec/mode_selection.c
@@ -3,10 +3,58 @@
  * Copyright 2025 Google LLC.
  */
 
+#include <linux/list_sort.h>
+
 #include "mode_selection.h"
 #include "class.h"
 #include "bus.h"
 
+/* Timeout for a mode entry attempt, ms */
+static const unsigned int mode_selection_timeout = 4000;
+/* Delay between mode entry/exit attempts, ms */
+static const unsigned int mode_selection_delay = 1000;
+/* Maximum retries for mode entry on busy status */
+static const unsigned int mode_entry_attempts = 4;
+
+/**
+ * enum ms_state - Specific mode selection states
+ * @MS_STATE_IDLE: The mode entry process has not started
+ * @MS_STATE_INPROGRESS: The mode entry process is currently underway
+ * @MS_STATE_ACTIVE: The mode has been successfully entered
+ * @MS_STATE_TIMEOUT: Mode entry failed due to a timeout
+ * @MS_STATE_FAILED: The mode driver reported the error
+ */
+enum ms_state {
+	MS_STATE_IDLE = 0,
+	MS_STATE_INPROGRESS,
+	MS_STATE_ACTIVE,
+	MS_STATE_TIMEOUT,
+	MS_STATE_FAILED,
+};
+
+/**
+ * struct mode_selection_state - State tracking for a specific Type-C mode
+ * @svid: The Standard or Vendor ID (SVID) for this alternate mode
+ * @name: Name of the alternate mode
+ * @priority: The mode priority. Lower values indicate a more preferred mode
+ * @enter: Flag indicating if the driver is currently attempting to enter or
+ * exit the mode
+ * @attempt_count: Number of times the driver has attempted to enter the mode
+ * @state: The current mode selection state
+ * @error: The outcome of the last attempt to enter the mode
+ * @list: List head to link this mode state into a prioritized list
+ */
+struct mode_selection_state {
+	u16 svid;
+	const char *name;
+	unsigned int priority;
+	bool enter;
+	int attempt_count;
+	enum ms_state state;
+	int error;
+	struct list_head list;
+};
+
 static int increment_duplicated_priority(struct device *dev, void *data)
 {
 	struct typec_altmode **alt_target = (struct typec_altmode **)data;
@@ -36,3 +84,300 @@ void typec_mode_set_priority(struct typec_altmode *alt,
 		res = device_for_each_child(&port->dev, &alt,
 				increment_duplicated_priority);
 }
+
+static void mode_list_clean(struct typec_partner *partner)
+{
+	struct mode_selection_state *ms, *tmp;
+
+	list_for_each_entry_safe(ms, tmp, &partner->mode_list, list) {
+		list_del(&ms->list);
+		kfree(ms);
+	}
+}
+
+/**
+ * mode_selection_next() - Process mode selection results and schedule next
+ * action
+ * @partner: pointer to the partner structure
+ * @ms: pointer to active mode_selection_state object that is on top in
+ * mode_list.
+ *
+ * The mutex protecting mode_list must be held by the caller when invoking this
+ * function.
+ *
+ * This function evaluates the outcome of the previous mode entry or exit
+ * attempt. Based on this result, it determines the next mode to process and
+ * schedules `mode_selection_work_fn()` 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_fn()` to attempt exiting the mode that was partially
+ * activated but not fully entered.
+ *
+ * If the previous operation was an exit (after a failed entry attempt),
+ * the internal list of candidate modes is advanced to determine the next mode
+ * to enter.
+ */
+static void mode_selection_next(
+	struct typec_partner *partner, struct mode_selection_state *ms)
+
+	__must_hold(&partner->mode_list_lock)
+{
+	if (!ms->enter) {
+		list_del(&ms->list);
+		kfree(ms);
+	} else if (ms->state == MS_STATE_INPROGRESS && !ms->error) {
+		list_del(&ms->list);
+		mode_list_clean(partner);
+
+		ms->state = MS_STATE_ACTIVE;
+		list_add_tail(&ms->list, &partner->mode_list);
+	} else {
+		if (ms->error) {
+			ms->state = MS_STATE_FAILED;
+			dev_dbg(&partner->dev, "%s: entry mode error %pe\n",
+				ms->name, ERR_PTR(ms->error));
+		}
+		if (ms->error != -EBUSY || ms->attempt_count >= mode_entry_attempts)
+			ms->enter = false;
+	}
+
+	ms = list_first_entry_or_null(
+		&partner->mode_list, struct mode_selection_state, list);
+	if (ms && ms->state != MS_STATE_ACTIVE)
+		schedule_delayed_work(&partner->mode_selection_work,
+			msecs_to_jiffies(mode_selection_delay));
+}
+
+void typec_altmode_entry_complete(struct typec_altmode *altmode,
+				const int error)
+{
+	struct typec_partner *partner = to_typec_partner(altmode->dev.parent);
+	struct mode_selection_state *ms;
+
+	mutex_lock(&partner->mode_list_lock);
+
+	ms = list_first_entry_or_null(
+		&partner->mode_list, struct mode_selection_state, list);
+	if (ms) {
+		if (ms->svid == altmode->svid && ms->state == MS_STATE_INPROGRESS) {
+			ms->error = error;
+			cancel_delayed_work(&partner->mode_selection_work);
+			mode_selection_next(partner, ms);
+		}
+	}
+
+	mutex_unlock(&partner->mode_list_lock);
+}
+EXPORT_SYMBOL_GPL(typec_altmode_entry_complete);
+
+static int mode_selection_activate_altmode(struct device *dev, void *data)
+{
+	struct mode_selection_state *ms = (struct mode_selection_state *)data;
+	int error = -ENODEV;
+	int ret = 0;
+
+	if (is_typec_altmode(dev)) {
+		struct typec_altmode *altmode = to_typec_altmode(dev);
+
+		if (ms->svid == altmode->svid) {
+			if (altmode->ops && altmode->ops->activate)
+				error = altmode->ops->activate(altmode, ms->enter);
+			else
+				error = -EOPNOTSUPP;
+			ret = 1;
+		}
+	}
+
+	if (ms->enter) {
+		ms->attempt_count++;
+		ms->error = error;
+	}
+
+	return ret;
+}
+
+/**
+ * mode_selection_work_fn() - Activate entry into the upcoming mode
+ * @work: work structure
+ *
+ * This function works in conjunction with `mode_selection_next()`.
+ * It attempts to activate the next mode in the selection sequence.
+ *
+ * If the mode activation (`mode_selection_activate_altmode()`) fails,
+ * `mode_selection_next()` will be called to initiate a new selection cycle.
+ *
+ * Otherwise, the state is set to MS_STATE_INPROGRESS, and
+ * `mode_selection_work_fn()` 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_fn()`.
+ */
+static void mode_selection_work_fn(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_list_lock);
+
+	ms = list_first_entry_or_null(
+		&partner->mode_list, struct mode_selection_state, list);
+	if (ms) {
+		if (ms->state == MS_STATE_INPROGRESS) {
+			ms->state = MS_STATE_TIMEOUT;
+			mode_selection_next(partner, ms);
+		} else {
+			device_for_each_child(&partner->dev, ms,
+				mode_selection_activate_altmode);
+
+			if (ms->enter && !ms->error) {
+				ms->state = MS_STATE_INPROGRESS;
+				schedule_delayed_work(&partner->mode_selection_work,
+					msecs_to_jiffies(mode_selection_timeout));
+			} else
+				mode_selection_next(partner, ms);
+		}
+	}
+
+	mutex_unlock(&partner->mode_list_lock);
+}
+
+void typec_mode_selection_add_partner(struct typec_partner *partner)
+{
+	INIT_LIST_HEAD(&partner->mode_list);
+	mutex_init(&partner->mode_list_lock);
+	INIT_DELAYED_WORK(&partner->mode_selection_work, mode_selection_work_fn);
+}
+
+void typec_mode_selection_remove_partner(struct typec_partner *partner)
+{
+	mutex_lock(&partner->mode_list_lock);
+	mode_list_clean(partner);
+	mutex_unlock(&partner->mode_list_lock);
+
+	cancel_delayed_work_sync(&partner->mode_selection_work);
+	mutex_destroy(&partner->mode_list_lock);
+}
+
+bool typec_mode_selection_is_pending(struct typec_partner *partner)
+{
+	struct mode_selection_state *ms = list_first_entry_or_null(
+			&partner->mode_list, struct mode_selection_state, list);
+
+	return ms != NULL;
+}
+
+static int compare_priorities(void *priv,
+	const struct list_head *a, const struct list_head *b)
+{
+	struct mode_selection_state *msa =
+			container_of(a, struct mode_selection_state, list);
+	struct mode_selection_state *msb =
+			container_of(b, struct mode_selection_state, list);
+
+	if (msa->priority < msb->priority)
+		return -1;
+	return 1;
+}
+
+static int mode_add_to_list(struct device *dev, void *data)
+{
+	struct list_head *list = (struct list_head *)data;
+	struct mode_selection_state *ms;
+
+	if (is_typec_altmode(dev)) {
+		struct typec_altmode *altmode = to_typec_altmode(dev);
+		const struct typec_altmode *pdev = typec_altmode_get_partner(altmode);
+
+		if (pdev) {
+			ms = kzalloc(sizeof(struct mode_selection_state), GFP_KERNEL);
+			if (!ms)
+				return -ENOMEM;
+
+			ms->svid = altmode->svid;
+			ms->name = altmode->desc;
+			ms->priority = pdev->priority;
+			ms->enter = true;
+			INIT_LIST_HEAD(&ms->list);
+			list_add_tail(&ms->list, list);
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * typec_mode_selection_start() - Starts the mode selection process.
+ * @partner: pointer to the partner structure
+ *
+ * This function populates mode_list with pointers to
+ * `struct mode_selection_state` instances. The sequence is generated based on
+ * partner capabilities and prioritized according to the port's settings.
+ */
+int typec_mode_selection_start(struct typec_partner *partner)
+{
+	int ret;
+
+	mutex_lock(&partner->mode_list_lock);
+
+	if (typec_mode_selection_is_pending(partner))
+		ret = -EALREADY;
+	else {
+		ret = device_for_each_child(
+			&partner->dev, &partner->mode_list, mode_add_to_list);
+
+		if (ret)
+			mode_list_clean(partner);
+		else if (!list_empty(&partner->mode_list)) {
+			list_sort(NULL, &partner->mode_list, compare_priorities);
+			schedule_delayed_work(&partner->mode_selection_work, 0);
+		}
+	}
+
+	mutex_unlock(&partner->mode_list_lock);
+	return ret;
+}
+
+/**
+ * typec_mode_selection_reset() - Reset the mode selection process.
+ * @partner: pointer to the partner structure
+ *
+ * This function cancels ongoing mode selection and exits the currently active
+ * mode, if present.
+ * It returns -EINPROGRESS when a mode entry is ongoing, indicating that the
+ * reset cannot immediately complete.
+ */
+int typec_mode_selection_reset(struct typec_partner *partner)
+{
+	struct mode_selection_state *ms;
+	int ret = 0;
+
+	mutex_lock(&partner->mode_list_lock);
+
+	ms = list_first_entry_or_null(
+		&partner->mode_list, struct mode_selection_state, list);
+	if (ms) {
+		if (ms->state == MS_STATE_ACTIVE) {
+			ms->enter = false;
+			device_for_each_child(&partner->dev, ms,
+					mode_selection_activate_altmode);
+		} else if (ms->state != MS_STATE_IDLE) {
+			list_del(&ms->list);
+			mode_list_clean(partner);
+
+			ms->attempt_count = mode_entry_attempts;
+			list_add(&ms->list, &partner->mode_list);
+
+			ret = -EINPROGRESS;
+		}
+
+		if (!ret)
+			mode_list_clean(partner);
+	}
+
+	mutex_unlock(&partner->mode_list_lock);
+	return ret;
+}
diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
index cbf5a37e6404..9049b5a25d63 100644
--- a/drivers/usb/typec/mode_selection.h
+++ b/drivers/usb/typec/mode_selection.h
@@ -4,3 +4,28 @@
 
 void typec_mode_set_priority(struct typec_altmode *alt,
 		const unsigned int priority);
+
+/**
+ * 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_add_partner() when the partner device is being set
+ * up.
+ *
+ * Execution:
+ * Call typec_mode_selection_start() to trigger the mode selection.
+ * typec_mode_selection_is_pending() returns true if the process is in progress
+ * or complete.
+ * Call typec_mode_selection_reset() to stop the selection process and exit
+ * the currently active mode.
+ *
+ * Destruction:
+ * Before destroying a partner, call typec_mode_selection_remove_partner()
+ */
+void typec_mode_selection_add_partner(struct typec_partner *partner);
+void typec_mode_selection_remove_partner(struct typec_partner *partner);
+int typec_mode_selection_start(struct typec_partner *partner);
+bool typec_mode_selection_is_pending(struct typec_partner *partner);
+int typec_mode_selection_reset(struct typec_partner *partner);
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index 571c6e00b54f..fd9ee3ef8de3 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -219,4 +219,15 @@ void typec_altmode_unregister_driver(struct typec_altmode_driver *drv);
 	module_driver(__typec_altmode_driver, typec_altmode_register_driver, \
 		      typec_altmode_unregister_driver)
 
+/**
+ * typec_altmode_entry_complete - Complete an alternate mode entry request
+ * @altmode: Handle to the alternate mode.
+ * @result: Result of the entry operation.
+ *
+ * This function should be called by a driver to report the final result of
+ * an asynchronous alternate mode entry request.
+ */
+void typec_altmode_entry_complete(struct typec_altmode *altmode,
+				const int result);
+
 #endif /* __USB_TYPEC_ALTMODE_H */
-- 
2.51.0.384.g4c02a37b29-goog


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

* [PATCH RFC 2/5] usb: typec: Expose mode_selection attribute via sysfs
  2025-09-09 12:30 [PATCH RFC 0/5] USB Type-C alternate mode selection Andrei Kuchynski
  2025-09-09 12:30 ` [PATCH RFC 1/5] usb: typec: Implement " Andrei Kuchynski
@ 2025-09-09 12:30 ` Andrei Kuchynski
  2025-09-09 12:30 ` [PATCH RFC 3/5] usb: typec: Report altmode entry status via callback Andrei Kuchynski
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Andrei Kuchynski @ 2025-09-09 12:30 UTC (permalink / raw)
  To: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
	Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform
  Cc: Guenter Roeck, Greg Kroah-Hartman, linux-kernel, Andrei Kuchynski

This patch introduces new sysfs attribute to enable user control over
Type-C mode selection. Writing a boolean '1' to this attribute starts the
mode selection process, while writing '0' stops it.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 Documentation/ABI/testing/sysfs-class-typec | 11 ++++++
 drivers/usb/typec/class.c                   | 37 +++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index dab3e4e727b6..7addf0e69c5c 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -258,6 +258,17 @@ 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:		August 2025
+Contact:	Andrei Kuchynski <akuchynski@chromium.org>
+Description:	Mode selection is activated by writing boolean 1 to the
+		file. Conversely, writing boolean 0 will cancel any ongoing selection
+		process and exit the currently active mode, if any. The attribute
+		returns "on" when mode selection is either in progress or complete,
+		and "off" otherwise.
+		This attribute is only present if the kernel supports AP driven mode
+		entry, where the Application Processor manages USB Type-C alt-modes.
+
 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 aaab2e1e98b4..b66fe62f282d 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -763,6 +763,35 @@ 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)
+{
+	const bool ret = typec_mode_selection_is_pending(to_typec_partner(dev));
+
+	return sprintf(buf, "%s\n", str_on_off(ret));
+}
+
+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) {
+		if (start)
+			ret = typec_mode_selection_start(partner);
+		else
+			ret = typec_mode_selection_reset(partner);
+	}
+
+	if (ret)
+		return ret;
+
+	return 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,
@@ -770,6 +799,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
 };
 
@@ -794,6 +824,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->mode_control)
+			return 0;
+
 	return attr->mode;
 }
 
@@ -1097,6 +1131,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_add_partner(partner);
+
 	return partner;
 }
 EXPORT_SYMBOL_GPL(typec_register_partner);
@@ -1114,6 +1150,7 @@ void typec_unregister_partner(struct typec_partner *partner)
 	if (IS_ERR_OR_NULL(partner))
 		return;
 
+	typec_mode_selection_remove_partner(partner);
 	port = to_typec_port(partner->dev.parent);
 
 	mutex_lock(&port->partner_link_lock);
-- 
2.51.0.384.g4c02a37b29-goog


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

* [PATCH RFC 3/5] usb: typec: Report altmode entry status via callback
  2025-09-09 12:30 [PATCH RFC 0/5] USB Type-C alternate mode selection Andrei Kuchynski
  2025-09-09 12:30 ` [PATCH RFC 1/5] usb: typec: Implement " Andrei Kuchynski
  2025-09-09 12:30 ` [PATCH RFC 2/5] usb: typec: Expose mode_selection attribute via sysfs Andrei Kuchynski
@ 2025-09-09 12:30 ` Andrei Kuchynski
  2025-09-09 12:30 ` [PATCH RFC 4/5] usb: typec: ucsi: displayport: Propagate DP altmode entry result Andrei Kuchynski
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Andrei Kuchynski @ 2025-09-09 12:30 UTC (permalink / raw)
  To: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
	Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform
  Cc: Guenter Roeck, Greg Kroah-Hartman, linux-kernel, 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 | 19 +++++++++++++++++--
 drivers/usb/typec/altmodes/thunderbolt.c | 10 ++++++++++
 include/linux/usb/typec_dp.h             |  2 ++
 include/linux/usb/typec_tbt.h            |  3 +++
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 1dcb77faf85d..052d18b54f0a 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -293,16 +293,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_altmode_entry_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_altmode_entry_complete(dp->alt, ret);
+		}
 		break;
 	case DP_STATE_CONFIGURE_PRIME:
 		ret = dp_altmode_configure_vdm_cable(dp, dp->data_prime.conf);
@@ -371,6 +375,7 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
 	int cmd_type = PD_VDO_CMDT(hdr);
 	int cmd = PD_VDO_CMD(hdr);
 	int ret = 0;
+	int entry_result = 0;
 
 	mutex_lock(&dp->lock);
 
@@ -414,10 +419,14 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
 		switch (cmd) {
 		case DP_CMD_STATUS_UPDATE:
 			dp->state = DP_STATE_EXIT;
+			if (vdo)
+				entry_result = *(int *)vdo;
 			break;
 		case DP_CMD_CONFIGURE:
 			dp->data.conf = 0;
 			ret = dp_altmode_configured(dp);
+			if (vdo)
+				entry_result = *(int *)vdo;
 			break;
 		default:
 			break;
@@ -432,6 +441,12 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
 
 err_unlock:
 	mutex_unlock(&dp->lock);
+
+	if (!entry_result)
+		entry_result = ret;
+	if (entry_result || cmd == DP_CMD_CONFIGURE)
+		typec_altmode_entry_complete(dp->alt, entry_result);
+
 	return ret;
 }
 
diff --git a/drivers/usb/typec/altmodes/thunderbolt.c b/drivers/usb/typec/altmodes/thunderbolt.c
index 6eadf7835f8f..765ba7348ac4 100644
--- a/drivers/usb/typec/altmodes/thunderbolt.c
+++ b/drivers/usb/typec/altmodes/thunderbolt.c
@@ -214,6 +214,7 @@ static int tbt_altmode_vdm(struct typec_altmode *alt,
 	struct typec_thunderbolt_data data;
 	int cmd_type = PD_VDO_CMDT(hdr);
 	int cmd = PD_VDO_CMD(hdr);
+	int entry_result = 0;
 
 	mutex_lock(&tbt->lock);
 
@@ -248,6 +249,12 @@ static int tbt_altmode_vdm(struct typec_altmode *alt,
 		switch (cmd) {
 		case CMD_ENTER_MODE:
 			dev_warn(&alt->dev, "Enter Mode refused\n");
+			if (vdo)
+				entry_result = *(int *)vdo;
+			break;
+		case TBT_CMD_STATUS_UPDATE:
+			if (vdo)
+				entry_result = *(int *)vdo;
 			break;
 		default:
 			break;
@@ -262,6 +269,9 @@ static int tbt_altmode_vdm(struct typec_altmode *alt,
 
 	mutex_unlock(&tbt->lock);
 
+	if (entry_result || cmd == TBT_CMD_STATUS_UPDATE)
+		typec_altmode_entry_complete(alt, entry_result);
+
 	return 0;
 }
 
diff --git a/include/linux/usb/typec_dp.h b/include/linux/usb/typec_dp.h
index acb0ad03bdac..c9fa68cd1265 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 {
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.51.0.384.g4c02a37b29-goog


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

* [PATCH RFC 4/5] usb: typec: ucsi: displayport: Propagate DP altmode entry result
  2025-09-09 12:30 [PATCH RFC 0/5] USB Type-C alternate mode selection Andrei Kuchynski
                   ` (2 preceding siblings ...)
  2025-09-09 12:30 ` [PATCH RFC 3/5] usb: typec: Report altmode entry status via callback Andrei Kuchynski
@ 2025-09-09 12:30 ` Andrei Kuchynski
  2025-09-09 12:30 ` [PATCH RFC 5/5] platform/chrome: cros_ec_typec: Propagate " Andrei Kuchynski
  2025-09-16 13:12 ` [PATCH RFC 0/5] USB Type-C alternate mode selection Heikki Krogerus
  5 siblings, 0 replies; 16+ messages in thread
From: Andrei Kuchynski @ 2025-09-09 12:30 UTC (permalink / raw)
  To: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
	Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform
  Cc: Guenter Roeck, Greg Kroah-Hartman, linux-kernel, Andrei Kuchynski

Reporting the error code via VDM 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 | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index 8aae80b457d7..47c28646cfa9 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -234,14 +234,18 @@ static int ucsi_displayport_vdm(struct typec_altmode *alt,
 
 		switch (cmd) {
 		case DP_CMD_STATUS_UPDATE:
-			if (ucsi_displayport_status_update(dp))
+			dp->data.error = ucsi_displayport_status_update(dp);
+			if (dp->data.error) {
+				dp->vdo_data = &dp->data.error;
 				dp->header |= VDO_CMDT(CMDT_RSP_NAK);
-			else
+			} else
 				dp->header |= VDO_CMDT(CMDT_RSP_ACK);
 			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.51.0.384.g4c02a37b29-goog


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

* [PATCH RFC 5/5] platform/chrome: cros_ec_typec: Propagate altmode entry result
  2025-09-09 12:30 [PATCH RFC 0/5] USB Type-C alternate mode selection Andrei Kuchynski
                   ` (3 preceding siblings ...)
  2025-09-09 12:30 ` [PATCH RFC 4/5] usb: typec: ucsi: displayport: Propagate DP altmode entry result Andrei Kuchynski
@ 2025-09-09 12:30 ` Andrei Kuchynski
  2025-09-16 13:12 ` [PATCH RFC 0/5] USB Type-C alternate mode selection Heikki Krogerus
  5 siblings, 0 replies; 16+ messages in thread
From: Andrei Kuchynski @ 2025-09-09 12:30 UTC (permalink / raw)
  To: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
	Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform
  Cc: Guenter Roeck, Greg Kroah-Hartman, linux-kernel, 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: DP_CMD_STATUS_UPDATE for DP altmode and TBT_CMD_STATUS_UPDATE for
TBT altmode.

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 ++++
 3 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index c0806c562bb9..18e627c9fc22 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -693,6 +693,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);
@@ -782,8 +783,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..75c7e6af1320 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 = 2;
+	} 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 = 2;
+	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__ */
-- 
2.51.0.384.g4c02a37b29-goog


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

* Re: [PATCH RFC 0/5] USB Type-C alternate mode selection
  2025-09-09 12:30 [PATCH RFC 0/5] USB Type-C alternate mode selection Andrei Kuchynski
                   ` (4 preceding siblings ...)
  2025-09-09 12:30 ` [PATCH RFC 5/5] platform/chrome: cros_ec_typec: Propagate " Andrei Kuchynski
@ 2025-09-16 13:12 ` Heikki Krogerus
  2025-09-16 16:47   ` Abhishek Pandit-Subedi
  5 siblings, 1 reply; 16+ messages in thread
From: Heikki Krogerus @ 2025-09-16 13:12 UTC (permalink / raw)
  To: Andrei Kuchynski
  Cc: Abhishek Pandit-Subedi, Benson Leung, Jameson Thies,
	Tzung-Bi Shih, linux-usb, chrome-platform, Guenter Roeck,
	Greg Kroah-Hartman, linux-kernel

On Tue, Sep 09, 2025 at 12:30:23PM +0000, 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.
> 
> New sysfs `mode_selection` attribute is exposed to provide user control
> over mode selection. It triggers an alternate mode negotiation.
> 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 the driver does not report its status within a defined timeout period,
> the system automatically proceeds to attempt entry into the next preferred
> mode.

I'm still struggling to understand what is the benefit from this - why
would you want the user space to explicitly start the entry process
like this? Instead why would you not just take full control over the
alt modes in user space by enabling the them one by one in what ever
order you want?

I don't believe you can make this approach scale much if and when in
the future the use cases change. Right now I don't feel comfortable
with this at all.

thanks,

> This series was tested on an Android OS device with kernel 6.16.
> This series depends on the 'USB Type-C alternate mode priorities' series:
> https://lore.kernel.org/all/20250905142206.4105351-1-akuchynski@chromium.org/ 
> 
> Andrei Kuchynski (5):
>   usb: typec: Implement mode selection
>   usb: typec: Expose mode_selection attribute via sysfs
>   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
> 
>  Documentation/ABI/testing/sysfs-class-typec  |  11 +
>  drivers/platform/chrome/cros_ec_typec.c      |   9 +
>  drivers/platform/chrome/cros_typec_altmode.c |  32 +-
>  drivers/platform/chrome/cros_typec_altmode.h |   6 +
>  drivers/usb/typec/altmodes/displayport.c     |  19 +-
>  drivers/usb/typec/altmodes/thunderbolt.c     |  10 +
>  drivers/usb/typec/class.c                    |  37 ++
>  drivers/usb/typec/class.h                    |   4 +
>  drivers/usb/typec/mode_selection.c           | 345 +++++++++++++++++++
>  drivers/usb/typec/mode_selection.h           |  25 ++
>  drivers/usb/typec/ucsi/displayport.c         |  10 +-
>  include/linux/usb/typec_altmode.h            |  11 +
>  include/linux/usb/typec_dp.h                 |   2 +
>  include/linux/usb/typec_tbt.h                |   3 +
>  14 files changed, 516 insertions(+), 8 deletions(-)
> 
> -- 
> 2.51.0.384.g4c02a37b29-goog

-- 
heikki

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

* Re: [PATCH RFC 0/5] USB Type-C alternate mode selection
  2025-09-16 13:12 ` [PATCH RFC 0/5] USB Type-C alternate mode selection Heikki Krogerus
@ 2025-09-16 16:47   ` Abhishek Pandit-Subedi
  2025-09-17 12:28     ` Heikki Krogerus
  0 siblings, 1 reply; 16+ messages in thread
From: Abhishek Pandit-Subedi @ 2025-09-16 16:47 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Andrei Kuchynski, Benson Leung, Jameson Thies, Tzung-Bi Shih,
	linux-usb, chrome-platform, Guenter Roeck, Greg Kroah-Hartman,
	linux-kernel

On Tue, Sep 16, 2025 at 6:12 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Tue, Sep 09, 2025 at 12:30:23PM +0000, 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.
> >
> > New sysfs `mode_selection` attribute is exposed to provide user control
> > over mode selection. It triggers an alternate mode negotiation.
> > 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 the driver does not report its status within a defined timeout period,
> > the system automatically proceeds to attempt entry into the next preferred
> > mode.
>
> I'm still struggling to understand what is the benefit from this - why
> would you want the user space to explicitly start the entry process
> like this? Instead why would you not just take full control over the
> alt modes in user space by enabling the them one by one in what ever
> order you want?

I think after the many patch iterations we went through upstreaming,
we may have lost the point a little bit wrt/ the mode selection task.
We talked about this on the very first iteration of this patchset
here: https://lore.kernel.org/linux-usb/CANFp7mVWo4GhiYqfLcD_wFV34WMkmXncMTOnmMfnKH4vm2X8Hg@mail.gmail.com/

The motivation behind it was to allow the kernel driver to own mode
selection entirely and not need user space intervention. The current
alt-mode drivers attempt to own the mode entry process and this fails
when you have two or more altmode drivers loaded (i.e. displayport,
thunderbolt). The original goal of the mode selection task was to move
the mode entry decision away from the alt-mode driver and to the port
driver instead.

What's missing in the current patch series to show this is probably
actually calling mode_selection once all partner modes are added :)
Andrei should be adding that to this patch series in the next patch
version.

Adding the mode_selection sysfs trigger is for another reason: to
re-run mode selection after priorities have been changed in userspace
and there is no partner hotplug. We specifically have some security
policies around PCI tunnels that result in the following need:
* When we enable pci tunneling, we PREFER tbt over dp and would like
to select the preferred mode. When we disable it, we PREFER dp over
TBT and would like to select the preferred mode.
* When users are logged out, we always prefer DP over TBT.
* When the system is locked, we prefer DP over TBT for new connections
(but existing connections can be left as TBT). When we unlock, we want
to enter the most preferred mode (TBT > DP).

While this is do-able with the alt-mode active sysfs field, we would
basically be re-creating the priority selection done in the kernel in
user space again. Hence why we want to expose the mode selection
trigger as done here.

>
> I don't believe you can make this approach scale much if and when in
> the future the use cases change. Right now I don't feel comfortable
> with this at all.
>
> thanks,
>
> > This series was tested on an Android OS device with kernel 6.16.
> > This series depends on the 'USB Type-C alternate mode priorities' series:
> > https://lore.kernel.org/all/20250905142206.4105351-1-akuchynski@chromium.org/
> >
> > Andrei Kuchynski (5):
> >   usb: typec: Implement mode selection
> >   usb: typec: Expose mode_selection attribute via sysfs
> >   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
> >
> >  Documentation/ABI/testing/sysfs-class-typec  |  11 +
> >  drivers/platform/chrome/cros_ec_typec.c      |   9 +
> >  drivers/platform/chrome/cros_typec_altmode.c |  32 +-
> >  drivers/platform/chrome/cros_typec_altmode.h |   6 +
> >  drivers/usb/typec/altmodes/displayport.c     |  19 +-
> >  drivers/usb/typec/altmodes/thunderbolt.c     |  10 +
> >  drivers/usb/typec/class.c                    |  37 ++
> >  drivers/usb/typec/class.h                    |   4 +
> >  drivers/usb/typec/mode_selection.c           | 345 +++++++++++++++++++
> >  drivers/usb/typec/mode_selection.h           |  25 ++
> >  drivers/usb/typec/ucsi/displayport.c         |  10 +-
> >  include/linux/usb/typec_altmode.h            |  11 +
> >  include/linux/usb/typec_dp.h                 |   2 +
> >  include/linux/usb/typec_tbt.h                |   3 +
> >  14 files changed, 516 insertions(+), 8 deletions(-)
> >
> > --
> > 2.51.0.384.g4c02a37b29-goog
>
> --
> heikki

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

* Re: [PATCH RFC 0/5] USB Type-C alternate mode selection
  2025-09-16 16:47   ` Abhishek Pandit-Subedi
@ 2025-09-17 12:28     ` Heikki Krogerus
  2025-09-17 17:51       ` Abhishek Pandit-Subedi
  0 siblings, 1 reply; 16+ messages in thread
From: Heikki Krogerus @ 2025-09-17 12:28 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: Andrei Kuchynski, Benson Leung, Jameson Thies, Tzung-Bi Shih,
	linux-usb, chrome-platform, Guenter Roeck, Greg Kroah-Hartman,
	linux-kernel

On Tue, Sep 16, 2025 at 09:47:44AM -0700, Abhishek Pandit-Subedi wrote:
> On Tue, Sep 16, 2025 at 6:12 AM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Tue, Sep 09, 2025 at 12:30:23PM +0000, 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.
> > >
> > > New sysfs `mode_selection` attribute is exposed to provide user control
> > > over mode selection. It triggers an alternate mode negotiation.
> > > 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 the driver does not report its status within a defined timeout period,
> > > the system automatically proceeds to attempt entry into the next preferred
> > > mode.
> >
> > I'm still struggling to understand what is the benefit from this - why
> > would you want the user space to explicitly start the entry process
> > like this? Instead why would you not just take full control over the
> > alt modes in user space by enabling the them one by one in what ever
> > order you want?
> 
> I think after the many patch iterations we went through upstreaming,
> we may have lost the point a little bit wrt/ the mode selection task.
> We talked about this on the very first iteration of this patchset
> here: https://lore.kernel.org/linux-usb/CANFp7mVWo4GhiYqfLcD_wFV34WMkmXncMTOnmMfnKH4vm2X8Hg@mail.gmail.com/
> 
> The motivation behind it was to allow the kernel driver to own mode
> selection entirely and not need user space intervention. The current
> alt-mode drivers attempt to own the mode entry process and this fails
> when you have two or more altmode drivers loaded (i.e. displayport,
> thunderbolt). The original goal of the mode selection task was to move
> the mode entry decision away from the alt-mode driver and to the port
> driver instead.
> 
> What's missing in the current patch series to show this is probably
> actually calling mode_selection once all partner modes are added :)
> Andrei should be adding that to this patch series in the next patch
> version.
> 
> Adding the mode_selection sysfs trigger is for another reason: to
> re-run mode selection after priorities have been changed in userspace
> and there is no partner hotplug. We specifically have some security
> policies around PCI tunnels that result in the following need:
> * When we enable pci tunneling, we PREFER tbt over dp and would like
> to select the preferred mode. When we disable it, we PREFER dp over
> TBT and would like to select the preferred mode.
> * When users are logged out, we always prefer DP over TBT.
> * When the system is locked, we prefer DP over TBT for new connections
> (but existing connections can be left as TBT). When we unlock, we want
> to enter the most preferred mode (TBT > DP).
> 
> While this is do-able with the alt-mode active sysfs field, we would
> basically be re-creating the priority selection done in the kernel in
> user space again. Hence why we want to expose the mode selection
> trigger as done here.

But this would be a step backwards. You want to keep the kernel in
control of the mode selection, which is fine, but then you have these
special cases where you have to give some of the control to the user
space. So instead of taking complete control of the mode selection in
user space, you want to create this partial control method of
supporting your special cases while still leaving "most" of the
control to kernel.

I don't believe this will work in all cases. I'm fine with the
priority as a way to tell the kernel the preferred entry order, but if
the user space needs to take control of the actual mode selection, it
has to take full control of it instead of like this, partially. This
just looks incredibly fragile.

So I'm still not convinced that there is any use for this. Either the
user space takes over the mode selection completely with the active
attribute files, or just leaves the selection completely to the kernel.

Br,

> > I don't believe you can make this approach scale much if and when in
> > the future the use cases change. Right now I don't feel comfortable
> > with this at all.
> >
> > thanks,
> >
> > > This series was tested on an Android OS device with kernel 6.16.
> > > This series depends on the 'USB Type-C alternate mode priorities' series:
> > > https://lore.kernel.org/all/20250905142206.4105351-1-akuchynski@chromium.org/
> > >
> > > Andrei Kuchynski (5):
> > >   usb: typec: Implement mode selection
> > >   usb: typec: Expose mode_selection attribute via sysfs
> > >   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
> > >
> > >  Documentation/ABI/testing/sysfs-class-typec  |  11 +
> > >  drivers/platform/chrome/cros_ec_typec.c      |   9 +
> > >  drivers/platform/chrome/cros_typec_altmode.c |  32 +-
> > >  drivers/platform/chrome/cros_typec_altmode.h |   6 +
> > >  drivers/usb/typec/altmodes/displayport.c     |  19 +-
> > >  drivers/usb/typec/altmodes/thunderbolt.c     |  10 +
> > >  drivers/usb/typec/class.c                    |  37 ++
> > >  drivers/usb/typec/class.h                    |   4 +
> > >  drivers/usb/typec/mode_selection.c           | 345 +++++++++++++++++++
> > >  drivers/usb/typec/mode_selection.h           |  25 ++
> > >  drivers/usb/typec/ucsi/displayport.c         |  10 +-
> > >  include/linux/usb/typec_altmode.h            |  11 +
> > >  include/linux/usb/typec_dp.h                 |   2 +
> > >  include/linux/usb/typec_tbt.h                |   3 +
> > >  14 files changed, 516 insertions(+), 8 deletions(-)
> > >
> > > --
> > > 2.51.0.384.g4c02a37b29-goog

-- 
heikki

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

* Re: [PATCH RFC 0/5] USB Type-C alternate mode selection
  2025-09-17 12:28     ` Heikki Krogerus
@ 2025-09-17 17:51       ` Abhishek Pandit-Subedi
  2025-09-18  0:09         ` Abhishek Pandit-Subedi
  2025-09-19 11:50         ` Heikki Krogerus
  0 siblings, 2 replies; 16+ messages in thread
From: Abhishek Pandit-Subedi @ 2025-09-17 17:51 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Andrei Kuchynski, Benson Leung, Jameson Thies, Tzung-Bi Shih,
	linux-usb, chrome-platform, Guenter Roeck, Greg Kroah-Hartman,
	linux-kernel

On Wed, Sep 17, 2025 at 5:28 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Tue, Sep 16, 2025 at 09:47:44AM -0700, Abhishek Pandit-Subedi wrote:
> > On Tue, Sep 16, 2025 at 6:12 AM Heikki Krogerus
> > <heikki.krogerus@linux.intel.com> wrote:
> > >
> > > On Tue, Sep 09, 2025 at 12:30:23PM +0000, 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.
> > > >
> > > > New sysfs `mode_selection` attribute is exposed to provide user control
> > > > over mode selection. It triggers an alternate mode negotiation.
> > > > 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 the driver does not report its status within a defined timeout period,
> > > > the system automatically proceeds to attempt entry into the next preferred
> > > > mode.
> > >
> > > I'm still struggling to understand what is the benefit from this - why
> > > would you want the user space to explicitly start the entry process
> > > like this? Instead why would you not just take full control over the
> > > alt modes in user space by enabling the them one by one in what ever
> > > order you want?
> >
> > I think after the many patch iterations we went through upstreaming,
> > we may have lost the point a little bit wrt/ the mode selection task.
> > We talked about this on the very first iteration of this patchset
> > here: https://lore.kernel.org/linux-usb/CANFp7mVWo4GhiYqfLcD_wFV34WMkmXncMTOnmMfnKH4vm2X8Hg@mail.gmail.com/
> >
> > The motivation behind it was to allow the kernel driver to own mode
> > selection entirely and not need user space intervention. The current
> > alt-mode drivers attempt to own the mode entry process and this fails
> > when you have two or more altmode drivers loaded (i.e. displayport,
> > thunderbolt). The original goal of the mode selection task was to move
> > the mode entry decision away from the alt-mode driver and to the port
> > driver instead.
> >
> > What's missing in the current patch series to show this is probably
> > actually calling mode_selection once all partner modes are added :)
> > Andrei should be adding that to this patch series in the next patch
> > version.
> >
> > Adding the mode_selection sysfs trigger is for another reason: to
> > re-run mode selection after priorities have been changed in userspace
> > and there is no partner hotplug. We specifically have some security
> > policies around PCI tunnels that result in the following need:
> > * When we enable pci tunneling, we PREFER tbt over dp and would like
> > to select the preferred mode. When we disable it, we PREFER dp over
> > TBT and would like to select the preferred mode.
> > * When users are logged out, we always prefer DP over TBT.
> > * When the system is locked, we prefer DP over TBT for new connections
> > (but existing connections can be left as TBT). When we unlock, we want
> > to enter the most preferred mode (TBT > DP).
> >
> > While this is do-able with the alt-mode active sysfs field, we would
> > basically be re-creating the priority selection done in the kernel in
> > user space again. Hence why we want to expose the mode selection
> > trigger as done here.
>
> But this would be a step backwards. You want to keep the kernel in
> control of the mode selection, which is fine, but then you have these
> special cases where you have to give some of the control to the user
> space. So instead of taking complete control of the mode selection in
> user space, you want to create this partial control method of
> supporting your special cases while still leaving "most" of the
> control to kernel.
>
> I don't believe this will work in all cases. I'm fine with the
> priority as a way to tell the kernel the preferred entry order, but if
> the user space needs to take control of the actual mode selection, it
> has to take full control of it instead of like this, partially. This
> just looks incredibly fragile.
>
> So I'm still not convinced that there is any use for this. Either the
> user space takes over the mode selection completely with the active
> attribute files, or just leaves the selection completely to the kernel.
>

That's a fair stance to take. We CAN do our special cases via the
"active" sysfs node. I've had a bit more time to think about the
problem we are solving and I'd like to elaborate a little.

When we designed this mode selection task, there were two motivating factors:
* The existing typec_displayport and typec_thunderbolt modules will
both automatically try to enter a mode when probing and that does not
work well. You want a deterministic entry order.
* There is no generic typec daemon for userspace on Linux and there
isn't always a need for one (i.e. UCSI systems). The kernel has the
most information about what any given system needs and should be able
to handle mode entry timing better.

For those motivating factors, I think an in-kernel mode selection as
designed in this series still makes sense. Let the kernel do the mode
selection, inform userspace when it is completed and userspace can
simply set priorities + report success/failure/errors.
One other change we will probably want to make is to turn the partner
altmode Kconfig options to boolean and roll it into the typec module.
Alt-mode module loading breaks mode selection ordering because you
can't be guaranteed the partner altmodes are loaded on the system when
you do partner altmode enumeration.

Heikki, can you confirm we are on the same page up till this point at
least? The net effect here is we are moving partner altmodes
individually entering modes to centralizing mode entry in the typec
class itself.

Also, with respect to dropping the mode_selection sysfs node and
simply using the `active` fields to override:
* How can we ensure that user space does not race with the kernel mode entry?
* Should we delay exposing "number_of_alternate_modes" until after
mode selection is done? Should we keep the mode_selection sysfs (or a
similarly named file) as a read-only indicator of current status?

Thanks,
Abhishek

> Br,
>
> > > I don't believe you can make this approach scale much if and when in
> > > the future the use cases change. Right now I don't feel comfortable
> > > with this at all.
> > >
> > > thanks,
> > >
> > > > This series was tested on an Android OS device with kernel 6.16.
> > > > This series depends on the 'USB Type-C alternate mode priorities' series:
> > > > https://lore.kernel.org/all/20250905142206.4105351-1-akuchynski@chromium.org/
> > > >
> > > > Andrei Kuchynski (5):
> > > >   usb: typec: Implement mode selection
> > > >   usb: typec: Expose mode_selection attribute via sysfs
> > > >   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
> > > >
> > > >  Documentation/ABI/testing/sysfs-class-typec  |  11 +
> > > >  drivers/platform/chrome/cros_ec_typec.c      |   9 +
> > > >  drivers/platform/chrome/cros_typec_altmode.c |  32 +-
> > > >  drivers/platform/chrome/cros_typec_altmode.h |   6 +
> > > >  drivers/usb/typec/altmodes/displayport.c     |  19 +-
> > > >  drivers/usb/typec/altmodes/thunderbolt.c     |  10 +
> > > >  drivers/usb/typec/class.c                    |  37 ++
> > > >  drivers/usb/typec/class.h                    |   4 +
> > > >  drivers/usb/typec/mode_selection.c           | 345 +++++++++++++++++++
> > > >  drivers/usb/typec/mode_selection.h           |  25 ++
> > > >  drivers/usb/typec/ucsi/displayport.c         |  10 +-
> > > >  include/linux/usb/typec_altmode.h            |  11 +
> > > >  include/linux/usb/typec_dp.h                 |   2 +
> > > >  include/linux/usb/typec_tbt.h                |   3 +
> > > >  14 files changed, 516 insertions(+), 8 deletions(-)
> > > >
> > > > --
> > > > 2.51.0.384.g4c02a37b29-goog
>
> --
> heikki

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

* Re: [PATCH RFC 0/5] USB Type-C alternate mode selection
  2025-09-17 17:51       ` Abhishek Pandit-Subedi
@ 2025-09-18  0:09         ` Abhishek Pandit-Subedi
  2025-09-18  7:50           ` Greg Kroah-Hartman
  2025-09-19 11:50         ` Heikki Krogerus
  1 sibling, 1 reply; 16+ messages in thread
From: Abhishek Pandit-Subedi @ 2025-09-18  0:09 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Andrei Kuchynski, Benson Leung, Jameson Thies, Tzung-Bi Shih,
	linux-usb, chrome-platform, Guenter Roeck, Greg Kroah-Hartman,
	linux-kernel

On Wed, Sep 17, 2025 at 10:51 AM Abhishek Pandit-Subedi
<abhishekpandit@chromium.org> wrote:
>
> On Wed, Sep 17, 2025 at 5:28 AM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Tue, Sep 16, 2025 at 09:47:44AM -0700, Abhishek Pandit-Subedi wrote:
> > > On Tue, Sep 16, 2025 at 6:12 AM Heikki Krogerus
> > > <heikki.krogerus@linux.intel.com> wrote:
> > > >
> > > > On Tue, Sep 09, 2025 at 12:30:23PM +0000, 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.
> > > > >
> > > > > New sysfs `mode_selection` attribute is exposed to provide user control
> > > > > over mode selection. It triggers an alternate mode negotiation.
> > > > > 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 the driver does not report its status within a defined timeout period,
> > > > > the system automatically proceeds to attempt entry into the next preferred
> > > > > mode.
> > > >
> > > > I'm still struggling to understand what is the benefit from this - why
> > > > would you want the user space to explicitly start the entry process
> > > > like this? Instead why would you not just take full control over the
> > > > alt modes in user space by enabling the them one by one in what ever
> > > > order you want?
> > >
> > > I think after the many patch iterations we went through upstreaming,
> > > we may have lost the point a little bit wrt/ the mode selection task.
> > > We talked about this on the very first iteration of this patchset
> > > here: https://lore.kernel.org/linux-usb/CANFp7mVWo4GhiYqfLcD_wFV34WMkmXncMTOnmMfnKH4vm2X8Hg@mail.gmail.com/
> > >
> > > The motivation behind it was to allow the kernel driver to own mode
> > > selection entirely and not need user space intervention. The current
> > > alt-mode drivers attempt to own the mode entry process and this fails
> > > when you have two or more altmode drivers loaded (i.e. displayport,
> > > thunderbolt). The original goal of the mode selection task was to move
> > > the mode entry decision away from the alt-mode driver and to the port
> > > driver instead.
> > >
> > > What's missing in the current patch series to show this is probably
> > > actually calling mode_selection once all partner modes are added :)
> > > Andrei should be adding that to this patch series in the next patch
> > > version.
> > >
> > > Adding the mode_selection sysfs trigger is for another reason: to
> > > re-run mode selection after priorities have been changed in userspace
> > > and there is no partner hotplug. We specifically have some security
> > > policies around PCI tunnels that result in the following need:
> > > * When we enable pci tunneling, we PREFER tbt over dp and would like
> > > to select the preferred mode. When we disable it, we PREFER dp over
> > > TBT and would like to select the preferred mode.
> > > * When users are logged out, we always prefer DP over TBT.
> > > * When the system is locked, we prefer DP over TBT for new connections
> > > (but existing connections can be left as TBT). When we unlock, we want
> > > to enter the most preferred mode (TBT > DP).
> > >
> > > While this is do-able with the alt-mode active sysfs field, we would
> > > basically be re-creating the priority selection done in the kernel in
> > > user space again. Hence why we want to expose the mode selection
> > > trigger as done here.
> >
> > But this would be a step backwards. You want to keep the kernel in
> > control of the mode selection, which is fine, but then you have these
> > special cases where you have to give some of the control to the user
> > space. So instead of taking complete control of the mode selection in
> > user space, you want to create this partial control method of
> > supporting your special cases while still leaving "most" of the
> > control to kernel.
> >
> > I don't believe this will work in all cases. I'm fine with the
> > priority as a way to tell the kernel the preferred entry order, but if
> > the user space needs to take control of the actual mode selection, it
> > has to take full control of it instead of like this, partially. This
> > just looks incredibly fragile.
> >
> > So I'm still not convinced that there is any use for this. Either the
> > user space takes over the mode selection completely with the active
> > attribute files, or just leaves the selection completely to the kernel.
> >
>
> That's a fair stance to take. We CAN do our special cases via the
> "active" sysfs node. I've had a bit more time to think about the
> problem we are solving and I'd like to elaborate a little.
>
> When we designed this mode selection task, there were two motivating factors:
> * The existing typec_displayport and typec_thunderbolt modules will
> both automatically try to enter a mode when probing and that does not
> work well. You want a deterministic entry order.
> * There is no generic typec daemon for userspace on Linux and there
> isn't always a need for one (i.e. UCSI systems). The kernel has the
> most information about what any given system needs and should be able
> to handle mode entry timing better.
>
> For those motivating factors, I think an in-kernel mode selection as
> designed in this series still makes sense. Let the kernel do the mode
> selection, inform userspace when it is completed and userspace can
> simply set priorities + report success/failure/errors.
> One other change we will probably want to make is to turn the partner
> altmode Kconfig options to boolean and roll it into the typec module.
> Alt-mode module loading breaks mode selection ordering because you
> can't be guaranteed the partner altmodes are loaded on the system when
> you do partner altmode enumeration.

After actually trying to do this today, I think a better approach
might be to just add a MODULE_SOFTDEP for all the typec altmodes. That
way, all the modules get loaded together and there's less of a chance
of waiting for the altmode driver to load when enumerating partners on
boot.

>
> Heikki, can you confirm we are on the same page up till this point at
> least? The net effect here is we are moving partner altmodes
> individually entering modes to centralizing mode entry in the typec
> class itself.
>
> Also, with respect to dropping the mode_selection sysfs node and
> simply using the `active` fields to override:
> * How can we ensure that user space does not race with the kernel mode entry?
> * Should we delay exposing "number_of_alternate_modes" until after
> mode selection is done? Should we keep the mode_selection sysfs (or a
> similarly named file) as a read-only indicator of current status?
>
> Thanks,
> Abhishek
>
> > Br,
> >
> > > > I don't believe you can make this approach scale much if and when in
> > > > the future the use cases change. Right now I don't feel comfortable
> > > > with this at all.
> > > >
> > > > thanks,
> > > >
> > > > > This series was tested on an Android OS device with kernel 6.16.
> > > > > This series depends on the 'USB Type-C alternate mode priorities' series:
> > > > > https://lore.kernel.org/all/20250905142206.4105351-1-akuchynski@chromium.org/
> > > > >
> > > > > Andrei Kuchynski (5):
> > > > >   usb: typec: Implement mode selection
> > > > >   usb: typec: Expose mode_selection attribute via sysfs
> > > > >   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
> > > > >
> > > > >  Documentation/ABI/testing/sysfs-class-typec  |  11 +
> > > > >  drivers/platform/chrome/cros_ec_typec.c      |   9 +
> > > > >  drivers/platform/chrome/cros_typec_altmode.c |  32 +-
> > > > >  drivers/platform/chrome/cros_typec_altmode.h |   6 +
> > > > >  drivers/usb/typec/altmodes/displayport.c     |  19 +-
> > > > >  drivers/usb/typec/altmodes/thunderbolt.c     |  10 +
> > > > >  drivers/usb/typec/class.c                    |  37 ++
> > > > >  drivers/usb/typec/class.h                    |   4 +
> > > > >  drivers/usb/typec/mode_selection.c           | 345 +++++++++++++++++++
> > > > >  drivers/usb/typec/mode_selection.h           |  25 ++
> > > > >  drivers/usb/typec/ucsi/displayport.c         |  10 +-
> > > > >  include/linux/usb/typec_altmode.h            |  11 +
> > > > >  include/linux/usb/typec_dp.h                 |   2 +
> > > > >  include/linux/usb/typec_tbt.h                |   3 +
> > > > >  14 files changed, 516 insertions(+), 8 deletions(-)
> > > > >
> > > > > --
> > > > > 2.51.0.384.g4c02a37b29-goog
> >
> > --
> > heikki

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

* Re: [PATCH RFC 0/5] USB Type-C alternate mode selection
  2025-09-18  0:09         ` Abhishek Pandit-Subedi
@ 2025-09-18  7:50           ` Greg Kroah-Hartman
  0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-18  7:50 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: Heikki Krogerus, Andrei Kuchynski, Benson Leung, Jameson Thies,
	Tzung-Bi Shih, linux-usb, chrome-platform, Guenter Roeck,
	linux-kernel

On Wed, Sep 17, 2025 at 05:09:44PM -0700, Abhishek Pandit-Subedi wrote:
> After actually trying to do this today, I think a better approach
> might be to just add a MODULE_SOFTDEP for all the typec altmodes. That
> way, all the modules get loaded together and there's less of a chance
> of waiting for the altmode driver to load when enumerating partners on
> boot.

MODULE_SOFTDEP is going to be nothing but problems going forward, I
wouldn't recommend it at all.  Try it out yourself on your hardware and
see :)

thanks,

greg k-h

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

* Re: [PATCH RFC 0/5] USB Type-C alternate mode selection
  2025-09-17 17:51       ` Abhishek Pandit-Subedi
  2025-09-18  0:09         ` Abhishek Pandit-Subedi
@ 2025-09-19 11:50         ` Heikki Krogerus
  2025-09-19 16:58           ` Abhishek Pandit-Subedi
  1 sibling, 1 reply; 16+ messages in thread
From: Heikki Krogerus @ 2025-09-19 11:50 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: Andrei Kuchynski, Benson Leung, Jameson Thies, Tzung-Bi Shih,
	linux-usb, chrome-platform, Guenter Roeck, Greg Kroah-Hartman,
	linux-kernel

On Wed, Sep 17, 2025 at 10:51:11AM -0700, Abhishek Pandit-Subedi wrote:
> On Wed, Sep 17, 2025 at 5:28 AM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Tue, Sep 16, 2025 at 09:47:44AM -0700, Abhishek Pandit-Subedi wrote:
> > > On Tue, Sep 16, 2025 at 6:12 AM Heikki Krogerus
> > > <heikki.krogerus@linux.intel.com> wrote:
> > > >
> > > > On Tue, Sep 09, 2025 at 12:30:23PM +0000, 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.
> > > > >
> > > > > New sysfs `mode_selection` attribute is exposed to provide user control
> > > > > over mode selection. It triggers an alternate mode negotiation.
> > > > > 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 the driver does not report its status within a defined timeout period,
> > > > > the system automatically proceeds to attempt entry into the next preferred
> > > > > mode.
> > > >
> > > > I'm still struggling to understand what is the benefit from this - why
> > > > would you want the user space to explicitly start the entry process
> > > > like this? Instead why would you not just take full control over the
> > > > alt modes in user space by enabling the them one by one in what ever
> > > > order you want?
> > >
> > > I think after the many patch iterations we went through upstreaming,
> > > we may have lost the point a little bit wrt/ the mode selection task.
> > > We talked about this on the very first iteration of this patchset
> > > here: https://lore.kernel.org/linux-usb/CANFp7mVWo4GhiYqfLcD_wFV34WMkmXncMTOnmMfnKH4vm2X8Hg@mail.gmail.com/
> > >
> > > The motivation behind it was to allow the kernel driver to own mode
> > > selection entirely and not need user space intervention. The current
> > > alt-mode drivers attempt to own the mode entry process and this fails
> > > when you have two or more altmode drivers loaded (i.e. displayport,
> > > thunderbolt). The original goal of the mode selection task was to move
> > > the mode entry decision away from the alt-mode driver and to the port
> > > driver instead.
> > >
> > > What's missing in the current patch series to show this is probably
> > > actually calling mode_selection once all partner modes are added :)
> > > Andrei should be adding that to this patch series in the next patch
> > > version.
> > >
> > > Adding the mode_selection sysfs trigger is for another reason: to
> > > re-run mode selection after priorities have been changed in userspace
> > > and there is no partner hotplug. We specifically have some security
> > > policies around PCI tunnels that result in the following need:
> > > * When we enable pci tunneling, we PREFER tbt over dp and would like
> > > to select the preferred mode. When we disable it, we PREFER dp over
> > > TBT and would like to select the preferred mode.
> > > * When users are logged out, we always prefer DP over TBT.
> > > * When the system is locked, we prefer DP over TBT for new connections
> > > (but existing connections can be left as TBT). When we unlock, we want
> > > to enter the most preferred mode (TBT > DP).
> > >
> > > While this is do-able with the alt-mode active sysfs field, we would
> > > basically be re-creating the priority selection done in the kernel in
> > > user space again. Hence why we want to expose the mode selection
> > > trigger as done here.
> >
> > But this would be a step backwards. You want to keep the kernel in
> > control of the mode selection, which is fine, but then you have these
> > special cases where you have to give some of the control to the user
> > space. So instead of taking complete control of the mode selection in
> > user space, you want to create this partial control method of
> > supporting your special cases while still leaving "most" of the
> > control to kernel.
> >
> > I don't believe this will work in all cases. I'm fine with the
> > priority as a way to tell the kernel the preferred entry order, but if
> > the user space needs to take control of the actual mode selection, it
> > has to take full control of it instead of like this, partially. This
> > just looks incredibly fragile.
> >
> > So I'm still not convinced that there is any use for this. Either the
> > user space takes over the mode selection completely with the active
> > attribute files, or just leaves the selection completely to the kernel.
> >
> 
> That's a fair stance to take. We CAN do our special cases via the
> "active" sysfs node. I've had a bit more time to think about the
> problem we are solving and I'd like to elaborate a little.
> 
> When we designed this mode selection task, there were two motivating factors:
> * The existing typec_displayport and typec_thunderbolt modules will
> both automatically try to enter a mode when probing and that does not
> work well. You want a deterministic entry order.
> * There is no generic typec daemon for userspace on Linux and there
> isn't always a need for one (i.e. UCSI systems). The kernel has the
> most information about what any given system needs and should be able
> to handle mode entry timing better.
> 
> For those motivating factors, I think an in-kernel mode selection as
> designed in this series still makes sense. Let the kernel do the mode
> selection, inform userspace when it is completed and userspace can
> simply set priorities + report success/failure/errors.
> One other change we will probably want to make is to turn the partner
> altmode Kconfig options to boolean and roll it into the typec module.
> Alt-mode module loading breaks mode selection ordering because you
> can't be guaranteed the partner altmodes are loaded on the system when
> you do partner altmode enumeration.
> 
> Heikki, can you confirm we are on the same page up till this point at
> least? The net effect here is we are moving partner altmodes
> individually entering modes to centralizing mode entry in the typec
> class itself.

I think we are on the same page, but I don't think you can incorporate
the entry of the modes into the class itself (you should not do that).
You can't even make it a problem for the altmodes - I don't believe we
can make it work in the long term.

Here multiple modes need to be handled as a set, so the set itself
needs to have an object that represents it. The altmodes will be part
of these "sets", but they remain independent of any particular set.
So the "set" defines the order in which the modes are entered instead
of the class, or any individual altmode.

I don't think there is any other way to make sure that the altmodes
continue to work as well independently as part of these "sets".

If you guys think that this makes sense, let me know. There are a
number of ways we could implement this.

> Also, with respect to dropping the mode_selection sysfs node and
> simply using the `active` fields to override:
> * How can we ensure that user space does not race with the kernel mode entry?
> * Should we delay exposing "number_of_alternate_modes" until after
> mode selection is done? Should we keep the mode_selection sysfs (or a
> similarly named file) as a read-only indicator of current status?

Races are definitely a concern. But I don't think that is a problem if
we go ahead with the above idea of separating the mode relationships
and entry into those "mode sets".

Br,

-- 
heikki

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

* Re: [PATCH RFC 0/5] USB Type-C alternate mode selection
  2025-09-19 11:50         ` Heikki Krogerus
@ 2025-09-19 16:58           ` Abhishek Pandit-Subedi
  2025-09-22 13:50             ` Heikki Krogerus
  0 siblings, 1 reply; 16+ messages in thread
From: Abhishek Pandit-Subedi @ 2025-09-19 16:58 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Andrei Kuchynski, Benson Leung, Jameson Thies, Tzung-Bi Shih,
	linux-usb, chrome-platform, Guenter Roeck, Greg Kroah-Hartman,
	linux-kernel

On Fri, Sep 19, 2025 at 4:50 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Wed, Sep 17, 2025 at 10:51:11AM -0700, Abhishek Pandit-Subedi wrote:
> > On Wed, Sep 17, 2025 at 5:28 AM Heikki Krogerus
> > <heikki.krogerus@linux.intel.com> wrote:
> > >
> > > On Tue, Sep 16, 2025 at 09:47:44AM -0700, Abhishek Pandit-Subedi wrote:
> > > > On Tue, Sep 16, 2025 at 6:12 AM Heikki Krogerus
> > > > <heikki.krogerus@linux.intel.com> wrote:
> > > > >
> > > > > On Tue, Sep 09, 2025 at 12:30:23PM +0000, 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.
> > > > > >
> > > > > > New sysfs `mode_selection` attribute is exposed to provide user control
> > > > > > over mode selection. It triggers an alternate mode negotiation.
> > > > > > 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 the driver does not report its status within a defined timeout period,
> > > > > > the system automatically proceeds to attempt entry into the next preferred
> > > > > > mode.
> > > > >
> > > > > I'm still struggling to understand what is the benefit from this - why
> > > > > would you want the user space to explicitly start the entry process
> > > > > like this? Instead why would you not just take full control over the
> > > > > alt modes in user space by enabling the them one by one in what ever
> > > > > order you want?
> > > >
> > > > I think after the many patch iterations we went through upstreaming,
> > > > we may have lost the point a little bit wrt/ the mode selection task.
> > > > We talked about this on the very first iteration of this patchset
> > > > here: https://lore.kernel.org/linux-usb/CANFp7mVWo4GhiYqfLcD_wFV34WMkmXncMTOnmMfnKH4vm2X8Hg@mail.gmail.com/
> > > >
> > > > The motivation behind it was to allow the kernel driver to own mode
> > > > selection entirely and not need user space intervention. The current
> > > > alt-mode drivers attempt to own the mode entry process and this fails
> > > > when you have two or more altmode drivers loaded (i.e. displayport,
> > > > thunderbolt). The original goal of the mode selection task was to move
> > > > the mode entry decision away from the alt-mode driver and to the port
> > > > driver instead.
> > > >
> > > > What's missing in the current patch series to show this is probably
> > > > actually calling mode_selection once all partner modes are added :)
> > > > Andrei should be adding that to this patch series in the next patch
> > > > version.
> > > >
> > > > Adding the mode_selection sysfs trigger is for another reason: to
> > > > re-run mode selection after priorities have been changed in userspace
> > > > and there is no partner hotplug. We specifically have some security
> > > > policies around PCI tunnels that result in the following need:
> > > > * When we enable pci tunneling, we PREFER tbt over dp and would like
> > > > to select the preferred mode. When we disable it, we PREFER dp over
> > > > TBT and would like to select the preferred mode.
> > > > * When users are logged out, we always prefer DP over TBT.
> > > > * When the system is locked, we prefer DP over TBT for new connections
> > > > (but existing connections can be left as TBT). When we unlock, we want
> > > > to enter the most preferred mode (TBT > DP).
> > > >
> > > > While this is do-able with the alt-mode active sysfs field, we would
> > > > basically be re-creating the priority selection done in the kernel in
> > > > user space again. Hence why we want to expose the mode selection
> > > > trigger as done here.
> > >
> > > But this would be a step backwards. You want to keep the kernel in
> > > control of the mode selection, which is fine, but then you have these
> > > special cases where you have to give some of the control to the user
> > > space. So instead of taking complete control of the mode selection in
> > > user space, you want to create this partial control method of
> > > supporting your special cases while still leaving "most" of the
> > > control to kernel.
> > >
> > > I don't believe this will work in all cases. I'm fine with the
> > > priority as a way to tell the kernel the preferred entry order, but if
> > > the user space needs to take control of the actual mode selection, it
> > > has to take full control of it instead of like this, partially. This
> > > just looks incredibly fragile.
> > >
> > > So I'm still not convinced that there is any use for this. Either the
> > > user space takes over the mode selection completely with the active
> > > attribute files, or just leaves the selection completely to the kernel.
> > >
> >
> > That's a fair stance to take. We CAN do our special cases via the
> > "active" sysfs node. I've had a bit more time to think about the
> > problem we are solving and I'd like to elaborate a little.
> >
> > When we designed this mode selection task, there were two motivating factors:
> > * The existing typec_displayport and typec_thunderbolt modules will
> > both automatically try to enter a mode when probing and that does not
> > work well. You want a deterministic entry order.
> > * There is no generic typec daemon for userspace on Linux and there
> > isn't always a need for one (i.e. UCSI systems). The kernel has the
> > most information about what any given system needs and should be able
> > to handle mode entry timing better.
> >
> > For those motivating factors, I think an in-kernel mode selection as
> > designed in this series still makes sense. Let the kernel do the mode
> > selection, inform userspace when it is completed and userspace can
> > simply set priorities + report success/failure/errors.
> > One other change we will probably want to make is to turn the partner
> > altmode Kconfig options to boolean and roll it into the typec module.
> > Alt-mode module loading breaks mode selection ordering because you
> > can't be guaranteed the partner altmodes are loaded on the system when
> > you do partner altmode enumeration.
> >
> > Heikki, can you confirm we are on the same page up till this point at
> > least? The net effect here is we are moving partner altmodes
> > individually entering modes to centralizing mode entry in the typec
> > class itself.
>
> I think we are on the same page, but I don't think you can incorporate
> the entry of the modes into the class itself (you should not do that).
> You can't even make it a problem for the altmodes - I don't believe we
> can make it work in the long term.

It probably needs to be a port driver decision. UCSI based systems
will mostly expect that a mode has already been entered after a
partner connects (and reports the SUPPORTED_CAM_CHANGED bit in
Connector Status Change) but with UCSI 3.0, some systems could
potentially make this decision in kernel (via sending SET_NEW_CAM
<connector> 0xff 0 which would prevent the PPM/LPM from entering any
modes automatically). Even with cros_ec_typec, only devices supporting
TBT/USB4 will need a mode selection decision in the port driver (since
others just auto-enter DP).

>
> Here multiple modes need to be handled as a set, so the set itself
> needs to have an object that represents it. The altmodes will be part
> of these "sets", but they remain independent of any particular set.
> So the "set" defines the order in which the modes are entered instead
> of the class, or any individual altmode.
>
> I don't think there is any other way to make sure that the altmodes
> continue to work as well independently as part of these "sets".
>
> If you guys think that this makes sense, let me know. There are a
> number of ways we could implement this.

The idea you've described here of sets sounds like what this patch
series hopes to implement. The set is just all the partner altmodes
which the port supports and the ordering is the port priority for
those altmodes.

>
> > Also, with respect to dropping the mode_selection sysfs node and
> > simply using the `active` fields to override:
> > * How can we ensure that user space does not race with the kernel mode entry?
> > * Should we delay exposing "number_of_alternate_modes" until after
> > mode selection is done? Should we keep the mode_selection sysfs (or a
> > similarly named file) as a read-only indicator of current status?
>
> Races are definitely a concern. But I don't think that is a problem if
> we go ahead with the above idea of separating the mode relationships
> and entry into those "mode sets".

Is the idea to give userspace the ability to create a set and try to
enter atomically? Who decides what constitutes a set?

>
> Br,
>
> --
> heikki

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

* Re: [PATCH RFC 0/5] USB Type-C alternate mode selection
  2025-09-19 16:58           ` Abhishek Pandit-Subedi
@ 2025-09-22 13:50             ` Heikki Krogerus
  2025-09-23 19:23               ` Abhishek Pandit-Subedi
  0 siblings, 1 reply; 16+ messages in thread
From: Heikki Krogerus @ 2025-09-22 13:50 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: Andrei Kuchynski, Benson Leung, Jameson Thies, Tzung-Bi Shih,
	linux-usb, chrome-platform, Guenter Roeck, Greg Kroah-Hartman,
	linux-kernel

On Fri, Sep 19, 2025 at 09:58:05AM -0700, Abhishek Pandit-Subedi wrote:
> On Fri, Sep 19, 2025 at 4:50 AM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Wed, Sep 17, 2025 at 10:51:11AM -0700, Abhishek Pandit-Subedi wrote:
> > > On Wed, Sep 17, 2025 at 5:28 AM Heikki Krogerus
> > > <heikki.krogerus@linux.intel.com> wrote:
> > > >
> > > > On Tue, Sep 16, 2025 at 09:47:44AM -0700, Abhishek Pandit-Subedi wrote:
> > > > > On Tue, Sep 16, 2025 at 6:12 AM Heikki Krogerus
> > > > > <heikki.krogerus@linux.intel.com> wrote:
> > > > > >
> > > > > > On Tue, Sep 09, 2025 at 12:30:23PM +0000, 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.
> > > > > > >
> > > > > > > New sysfs `mode_selection` attribute is exposed to provide user control
> > > > > > > over mode selection. It triggers an alternate mode negotiation.
> > > > > > > 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 the driver does not report its status within a defined timeout period,
> > > > > > > the system automatically proceeds to attempt entry into the next preferred
> > > > > > > mode.
> > > > > >
> > > > > > I'm still struggling to understand what is the benefit from this - why
> > > > > > would you want the user space to explicitly start the entry process
> > > > > > like this? Instead why would you not just take full control over the
> > > > > > alt modes in user space by enabling the them one by one in what ever
> > > > > > order you want?
> > > > >
> > > > > I think after the many patch iterations we went through upstreaming,
> > > > > we may have lost the point a little bit wrt/ the mode selection task.
> > > > > We talked about this on the very first iteration of this patchset
> > > > > here: https://lore.kernel.org/linux-usb/CANFp7mVWo4GhiYqfLcD_wFV34WMkmXncMTOnmMfnKH4vm2X8Hg@mail.gmail.com/
> > > > >
> > > > > The motivation behind it was to allow the kernel driver to own mode
> > > > > selection entirely and not need user space intervention. The current
> > > > > alt-mode drivers attempt to own the mode entry process and this fails
> > > > > when you have two or more altmode drivers loaded (i.e. displayport,
> > > > > thunderbolt). The original goal of the mode selection task was to move
> > > > > the mode entry decision away from the alt-mode driver and to the port
> > > > > driver instead.
> > > > >
> > > > > What's missing in the current patch series to show this is probably
> > > > > actually calling mode_selection once all partner modes are added :)
> > > > > Andrei should be adding that to this patch series in the next patch
> > > > > version.
> > > > >
> > > > > Adding the mode_selection sysfs trigger is for another reason: to
> > > > > re-run mode selection after priorities have been changed in userspace
> > > > > and there is no partner hotplug. We specifically have some security
> > > > > policies around PCI tunnels that result in the following need:
> > > > > * When we enable pci tunneling, we PREFER tbt over dp and would like
> > > > > to select the preferred mode. When we disable it, we PREFER dp over
> > > > > TBT and would like to select the preferred mode.
> > > > > * When users are logged out, we always prefer DP over TBT.
> > > > > * When the system is locked, we prefer DP over TBT for new connections
> > > > > (but existing connections can be left as TBT). When we unlock, we want
> > > > > to enter the most preferred mode (TBT > DP).
> > > > >
> > > > > While this is do-able with the alt-mode active sysfs field, we would
> > > > > basically be re-creating the priority selection done in the kernel in
> > > > > user space again. Hence why we want to expose the mode selection
> > > > > trigger as done here.
> > > >
> > > > But this would be a step backwards. You want to keep the kernel in
> > > > control of the mode selection, which is fine, but then you have these
> > > > special cases where you have to give some of the control to the user
> > > > space. So instead of taking complete control of the mode selection in
> > > > user space, you want to create this partial control method of
> > > > supporting your special cases while still leaving "most" of the
> > > > control to kernel.
> > > >
> > > > I don't believe this will work in all cases. I'm fine with the
> > > > priority as a way to tell the kernel the preferred entry order, but if
> > > > the user space needs to take control of the actual mode selection, it
> > > > has to take full control of it instead of like this, partially. This
> > > > just looks incredibly fragile.
> > > >
> > > > So I'm still not convinced that there is any use for this. Either the
> > > > user space takes over the mode selection completely with the active
> > > > attribute files, or just leaves the selection completely to the kernel.
> > > >
> > >
> > > That's a fair stance to take. We CAN do our special cases via the
> > > "active" sysfs node. I've had a bit more time to think about the
> > > problem we are solving and I'd like to elaborate a little.
> > >
> > > When we designed this mode selection task, there were two motivating factors:
> > > * The existing typec_displayport and typec_thunderbolt modules will
> > > both automatically try to enter a mode when probing and that does not
> > > work well. You want a deterministic entry order.
> > > * There is no generic typec daemon for userspace on Linux and there
> > > isn't always a need for one (i.e. UCSI systems). The kernel has the
> > > most information about what any given system needs and should be able
> > > to handle mode entry timing better.
> > >
> > > For those motivating factors, I think an in-kernel mode selection as
> > > designed in this series still makes sense. Let the kernel do the mode
> > > selection, inform userspace when it is completed and userspace can
> > > simply set priorities + report success/failure/errors.
> > > One other change we will probably want to make is to turn the partner
> > > altmode Kconfig options to boolean and roll it into the typec module.
> > > Alt-mode module loading breaks mode selection ordering because you
> > > can't be guaranteed the partner altmodes are loaded on the system when
> > > you do partner altmode enumeration.
> > >
> > > Heikki, can you confirm we are on the same page up till this point at
> > > least? The net effect here is we are moving partner altmodes
> > > individually entering modes to centralizing mode entry in the typec
> > > class itself.
> >
> > I think we are on the same page, but I don't think you can incorporate
> > the entry of the modes into the class itself (you should not do that).
> > You can't even make it a problem for the altmodes - I don't believe we
> > can make it work in the long term.
> 
> It probably needs to be a port driver decision. UCSI based systems
> will mostly expect that a mode has already been entered after a
> partner connects (and reports the SUPPORTED_CAM_CHANGED bit in
> Connector Status Change) but with UCSI 3.0, some systems could
> potentially make this decision in kernel (via sending SET_NEW_CAM
> <connector> 0xff 0 which would prevent the PPM/LPM from entering any
> modes automatically). Even with cros_ec_typec, only devices supporting
> TBT/USB4 will need a mode selection decision in the port driver (since
> others just auto-enter DP).
> 
> >
> > Here multiple modes need to be handled as a set, so the set itself
> > needs to have an object that represents it. The altmodes will be part
> > of these "sets", but they remain independent of any particular set.
> > So the "set" defines the order in which the modes are entered instead
> > of the class, or any individual altmode.
> >
> > I don't think there is any other way to make sure that the altmodes
> > continue to work as well independently as part of these "sets".
> >
> > If you guys think that this makes sense, let me know. There are a
> > number of ways we could implement this.
> 
> The idea you've described here of sets sounds like what this patch
> series hopes to implement. The set is just all the partner altmodes
> which the port supports and the ordering is the port priority for
> those altmodes.
> 
> >
> > > Also, with respect to dropping the mode_selection sysfs node and
> > > simply using the `active` fields to override:
> > > * How can we ensure that user space does not race with the kernel mode entry?
> > > * Should we delay exposing "number_of_alternate_modes" until after
> > > mode selection is done? Should we keep the mode_selection sysfs (or a
> > > similarly named file) as a read-only indicator of current status?
> >
> > Races are definitely a concern. But I don't think that is a problem if
> > we go ahead with the above idea of separating the mode relationships
> > and entry into those "mode sets".
> 
> Is the idea to give userspace the ability to create a set and try to
> enter atomically? Who decides what constitutes a set?

It's really up to what we want, and how we you guys want to implement
it. If the mode selection was a device (associated with each port (or
partner) for example), then maybe the "set" could be a driver attached
to it. Maybe :)

I'm just asking that we isolate the feature, because I think it can be
separated in this case. I think isolating it should give some
flexibility on things like where the control is in the end, but more
importantly, that should minimise the impact to the existing bits and
pieces, maybe allow the feature to be selectable, etc. etc.

thanks,

-- 
heikki

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

* Re: [PATCH RFC 0/5] USB Type-C alternate mode selection
  2025-09-22 13:50             ` Heikki Krogerus
@ 2025-09-23 19:23               ` Abhishek Pandit-Subedi
  0 siblings, 0 replies; 16+ messages in thread
From: Abhishek Pandit-Subedi @ 2025-09-23 19:23 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Andrei Kuchynski, Benson Leung, Jameson Thies, Tzung-Bi Shih,
	linux-usb, chrome-platform, Guenter Roeck, Greg Kroah-Hartman,
	linux-kernel

On Mon, Sep 22, 2025 at 6:50 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Fri, Sep 19, 2025 at 09:58:05AM -0700, Abhishek Pandit-Subedi wrote:
> > On Fri, Sep 19, 2025 at 4:50 AM Heikki Krogerus
> > <heikki.krogerus@linux.intel.com> wrote:
> > >
> > > On Wed, Sep 17, 2025 at 10:51:11AM -0700, Abhishek Pandit-Subedi wrote:
> > > > On Wed, Sep 17, 2025 at 5:28 AM Heikki Krogerus
> > > > <heikki.krogerus@linux.intel.com> wrote:
> > > > >
> > > > > On Tue, Sep 16, 2025 at 09:47:44AM -0700, Abhishek Pandit-Subedi wrote:
> > > > > > On Tue, Sep 16, 2025 at 6:12 AM Heikki Krogerus
> > > > > > <heikki.krogerus@linux.intel.com> wrote:
> > > > > > >
> > > > > > > On Tue, Sep 09, 2025 at 12:30:23PM +0000, 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.
> > > > > > > >
> > > > > > > > New sysfs `mode_selection` attribute is exposed to provide user control
> > > > > > > > over mode selection. It triggers an alternate mode negotiation.
> > > > > > > > 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 the driver does not report its status within a defined timeout period,
> > > > > > > > the system automatically proceeds to attempt entry into the next preferred
> > > > > > > > mode.
> > > > > > >
> > > > > > > I'm still struggling to understand what is the benefit from this - why
> > > > > > > would you want the user space to explicitly start the entry process
> > > > > > > like this? Instead why would you not just take full control over the
> > > > > > > alt modes in user space by enabling the them one by one in what ever
> > > > > > > order you want?
> > > > > >
> > > > > > I think after the many patch iterations we went through upstreaming,
> > > > > > we may have lost the point a little bit wrt/ the mode selection task.
> > > > > > We talked about this on the very first iteration of this patchset
> > > > > > here: https://lore.kernel.org/linux-usb/CANFp7mVWo4GhiYqfLcD_wFV34WMkmXncMTOnmMfnKH4vm2X8Hg@mail.gmail.com/
> > > > > >
> > > > > > The motivation behind it was to allow the kernel driver to own mode
> > > > > > selection entirely and not need user space intervention. The current
> > > > > > alt-mode drivers attempt to own the mode entry process and this fails
> > > > > > when you have two or more altmode drivers loaded (i.e. displayport,
> > > > > > thunderbolt). The original goal of the mode selection task was to move
> > > > > > the mode entry decision away from the alt-mode driver and to the port
> > > > > > driver instead.
> > > > > >
> > > > > > What's missing in the current patch series to show this is probably
> > > > > > actually calling mode_selection once all partner modes are added :)
> > > > > > Andrei should be adding that to this patch series in the next patch
> > > > > > version.
> > > > > >
> > > > > > Adding the mode_selection sysfs trigger is for another reason: to
> > > > > > re-run mode selection after priorities have been changed in userspace
> > > > > > and there is no partner hotplug. We specifically have some security
> > > > > > policies around PCI tunnels that result in the following need:
> > > > > > * When we enable pci tunneling, we PREFER tbt over dp and would like
> > > > > > to select the preferred mode. When we disable it, we PREFER dp over
> > > > > > TBT and would like to select the preferred mode.
> > > > > > * When users are logged out, we always prefer DP over TBT.
> > > > > > * When the system is locked, we prefer DP over TBT for new connections
> > > > > > (but existing connections can be left as TBT). When we unlock, we want
> > > > > > to enter the most preferred mode (TBT > DP).
> > > > > >
> > > > > > While this is do-able with the alt-mode active sysfs field, we would
> > > > > > basically be re-creating the priority selection done in the kernel in
> > > > > > user space again. Hence why we want to expose the mode selection
> > > > > > trigger as done here.
> > > > >
> > > > > But this would be a step backwards. You want to keep the kernel in
> > > > > control of the mode selection, which is fine, but then you have these
> > > > > special cases where you have to give some of the control to the user
> > > > > space. So instead of taking complete control of the mode selection in
> > > > > user space, you want to create this partial control method of
> > > > > supporting your special cases while still leaving "most" of the
> > > > > control to kernel.
> > > > >
> > > > > I don't believe this will work in all cases. I'm fine with the
> > > > > priority as a way to tell the kernel the preferred entry order, but if
> > > > > the user space needs to take control of the actual mode selection, it
> > > > > has to take full control of it instead of like this, partially. This
> > > > > just looks incredibly fragile.
> > > > >
> > > > > So I'm still not convinced that there is any use for this. Either the
> > > > > user space takes over the mode selection completely with the active
> > > > > attribute files, or just leaves the selection completely to the kernel.
> > > > >
> > > >
> > > > That's a fair stance to take. We CAN do our special cases via the
> > > > "active" sysfs node. I've had a bit more time to think about the
> > > > problem we are solving and I'd like to elaborate a little.
> > > >
> > > > When we designed this mode selection task, there were two motivating factors:
> > > > * The existing typec_displayport and typec_thunderbolt modules will
> > > > both automatically try to enter a mode when probing and that does not
> > > > work well. You want a deterministic entry order.
> > > > * There is no generic typec daemon for userspace on Linux and there
> > > > isn't always a need for one (i.e. UCSI systems). The kernel has the
> > > > most information about what any given system needs and should be able
> > > > to handle mode entry timing better.
> > > >
> > > > For those motivating factors, I think an in-kernel mode selection as
> > > > designed in this series still makes sense. Let the kernel do the mode
> > > > selection, inform userspace when it is completed and userspace can
> > > > simply set priorities + report success/failure/errors.
> > > > One other change we will probably want to make is to turn the partner
> > > > altmode Kconfig options to boolean and roll it into the typec module.
> > > > Alt-mode module loading breaks mode selection ordering because you
> > > > can't be guaranteed the partner altmodes are loaded on the system when
> > > > you do partner altmode enumeration.
> > > >
> > > > Heikki, can you confirm we are on the same page up till this point at
> > > > least? The net effect here is we are moving partner altmodes
> > > > individually entering modes to centralizing mode entry in the typec
> > > > class itself.
> > >
> > > I think we are on the same page, but I don't think you can incorporate
> > > the entry of the modes into the class itself (you should not do that).
> > > You can't even make it a problem for the altmodes - I don't believe we
> > > can make it work in the long term.
> >
> > It probably needs to be a port driver decision. UCSI based systems
> > will mostly expect that a mode has already been entered after a
> > partner connects (and reports the SUPPORTED_CAM_CHANGED bit in
> > Connector Status Change) but with UCSI 3.0, some systems could
> > potentially make this decision in kernel (via sending SET_NEW_CAM
> > <connector> 0xff 0 which would prevent the PPM/LPM from entering any
> > modes automatically). Even with cros_ec_typec, only devices supporting
> > TBT/USB4 will need a mode selection decision in the port driver (since
> > others just auto-enter DP).
> >
> > >
> > > Here multiple modes need to be handled as a set, so the set itself
> > > needs to have an object that represents it. The altmodes will be part
> > > of these "sets", but they remain independent of any particular set.
> > > So the "set" defines the order in which the modes are entered instead
> > > of the class, or any individual altmode.
> > >
> > > I don't think there is any other way to make sure that the altmodes
> > > continue to work as well independently as part of these "sets".
> > >
> > > If you guys think that this makes sense, let me know. There are a
> > > number of ways we could implement this.
> >
> > The idea you've described here of sets sounds like what this patch
> > series hopes to implement. The set is just all the partner altmodes
> > which the port supports and the ordering is the port priority for
> > those altmodes.
> >
> > >
> > > > Also, with respect to dropping the mode_selection sysfs node and
> > > > simply using the `active` fields to override:
> > > > * How can we ensure that user space does not race with the kernel mode entry?
> > > > * Should we delay exposing "number_of_alternate_modes" until after
> > > > mode selection is done? Should we keep the mode_selection sysfs (or a
> > > > similarly named file) as a read-only indicator of current status?
> > >
> > > Races are definitely a concern. But I don't think that is a problem if
> > > we go ahead with the above idea of separating the mode relationships
> > > and entry into those "mode sets".
> >
> > Is the idea to give userspace the ability to create a set and try to
> > enter atomically? Who decides what constitutes a set?
>
> It's really up to what we want, and how we you guys want to implement
> it. If the mode selection was a device (associated with each port (or
> partner) for example), then maybe the "set" could be a driver attached
> to it. Maybe :)
>
> I'm just asking that we isolate the feature, because I think it can be
> separated in this case. I think isolating it should give some
> flexibility on things like where the control is in the end, but more
> importantly, that should minimise the impact to the existing bits and
> pieces, maybe allow the feature to be selectable, etc. etc.

Mode selection is already opt-in in the current implementation and
needs to be called by the port driver.

When integrating with existing altmode drivers, the major change will
be in the probe functions:
* When mode selection is not in use by the port driver, keep the
current behavior (automatically enter the probing altmode).
* When mode selection is in use by the port driver, do not enter the alt-mode.

Port drivers that enable mode selection could also be required to
select all typec altmodes as =Y so that they are all loaded by the
time the mode selection task runs.
i.e. the way this could work is by introducing CONFIG
TYPEC_MODE_SELECTION_SUPPORT, which selects TYPEC_DISPLAYPORT=y,
TYPEC_THUNDERBOLT=y, TYPEC_NVIDIA=y and have port drivers select them
(i.e. CONFIG CROS_EC_TYPEC selects TYPEC_MODE_SELECTION_SUPPORT).

Does this meet your expectation for isolation?

>
> thanks,

>
> --
> heikki

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

end of thread, other threads:[~2025-09-23 19:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-09 12:30 [PATCH RFC 0/5] USB Type-C alternate mode selection Andrei Kuchynski
2025-09-09 12:30 ` [PATCH RFC 1/5] usb: typec: Implement " Andrei Kuchynski
2025-09-09 12:30 ` [PATCH RFC 2/5] usb: typec: Expose mode_selection attribute via sysfs Andrei Kuchynski
2025-09-09 12:30 ` [PATCH RFC 3/5] usb: typec: Report altmode entry status via callback Andrei Kuchynski
2025-09-09 12:30 ` [PATCH RFC 4/5] usb: typec: ucsi: displayport: Propagate DP altmode entry result Andrei Kuchynski
2025-09-09 12:30 ` [PATCH RFC 5/5] platform/chrome: cros_ec_typec: Propagate " Andrei Kuchynski
2025-09-16 13:12 ` [PATCH RFC 0/5] USB Type-C alternate mode selection Heikki Krogerus
2025-09-16 16:47   ` Abhishek Pandit-Subedi
2025-09-17 12:28     ` Heikki Krogerus
2025-09-17 17:51       ` Abhishek Pandit-Subedi
2025-09-18  0:09         ` Abhishek Pandit-Subedi
2025-09-18  7:50           ` Greg Kroah-Hartman
2025-09-19 11:50         ` Heikki Krogerus
2025-09-19 16:58           ` Abhishek Pandit-Subedi
2025-09-22 13:50             ` Heikki Krogerus
2025-09-23 19:23               ` Abhishek Pandit-Subedi

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