linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/10] USB Type-C mode selection
@ 2025-08-04  9:03 Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 01/10] usb: typec: Add alt_mode_override field to port property Andrei Kuchynski
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, 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 attributes are exposed to provide user control over mode
selection:
`priority`, `usb4_priority`: Allows users to define their preferred order
for attempting mode entry.
`mode_priorities`: Lists the modes supported by the port, ordered by
priority.
`mode_selection`: Lists modes supported by the partner and triggers an
automatic mode negotiation.
`entry_result`, `usb4_entry_result`: Reports the status of the last mode
selection attempt.

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 a ChromeOS Brya device running kernel 6.6, and on
an Android OS device with kernel 6.16.

Changes in v3:
- The mode_priorities sysfs attribute has been made read-only.
- The mode_selection attribute now exclusively lists partner-supported
modes, with mode entry results reported via separate attributes.
- The driver returns mode entry results instead of error codes.
- Constant values are used in place of module parameters.

Andrei Kuchynski (10):
  usb: typec: Add alt_mode_override field to port property
  platform/chrome: cros_ec_typec: Set alt_mode_override flag
  usb: typec: ucsi: Set alt_mode_override flag
  usb: typec: Expose mode priorities via sysfs
  usb: typec: Implement automated mode selection
  usb: typec: Report altmode entry status via callback
  usb: typec: ucsi: displayport: Propagate DP altmode entry result
  platform/chrome: cros_ec_typec: Propagate altmode entry result
  platform/chrome: cros_ec_typec: Report USB4 entry status via callback
  platform/chrome: cros_ec_typec: Add default_usb_mode_set support

 Documentation/ABI/testing/sysfs-class-typec  |  72 +++
 drivers/platform/chrome/cros_ec_typec.c      |  17 +
 drivers/platform/chrome/cros_typec_altmode.c |  32 +-
 drivers/platform/chrome/cros_typec_altmode.h |   6 +
 drivers/usb/typec/Makefile                   |   2 +-
 drivers/usb/typec/altmodes/displayport.c     |  17 +-
 drivers/usb/typec/altmodes/thunderbolt.c     |   8 +
 drivers/usb/typec/class.c                    | 212 ++++++-
 drivers/usb/typec/class.h                    |  15 +
 drivers/usb/typec/mode_selection.c           | 575 +++++++++++++++++++
 drivers/usb/typec/mode_selection.h           |  54 ++
 drivers/usb/typec/ucsi/displayport.c         |  10 +-
 drivers/usb/typec/ucsi/ucsi.c                |   2 +
 include/linux/usb/pd_vdo.h                   |   2 +
 include/linux/usb/typec.h                    |   1 +
 include/linux/usb/typec_altmode.h            |  12 +
 include/linux/usb/typec_dp.h                 |   2 +
 include/linux/usb/typec_tbt.h                |   3 +
 18 files changed, 1024 insertions(+), 18 deletions(-)
 create mode 100644 drivers/usb/typec/mode_selection.c
 create mode 100644 drivers/usb/typec/mode_selection.h

-- 
2.50.1.565.gc32cd1483b-goog


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

* [PATCH v3 01/10] usb: typec: Add alt_mode_override field to port property
  2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
@ 2025-08-04  9:03 ` Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 02/10] platform/chrome: cros_ec_typec: Set alt_mode_override flag Andrei Kuchynski
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, linux-kernel,
	Andrei Kuchynski

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

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

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


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

* [PATCH v3 02/10] platform/chrome: cros_ec_typec: Set alt_mode_override flag
  2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 01/10] usb: typec: Add alt_mode_override field to port property Andrei Kuchynski
@ 2025-08-04  9:03 ` Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 03/10] usb: typec: ucsi: " Andrei Kuchynski
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, linux-kernel,
	Andrei Kuchynski

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

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

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index b712bcff6fb2..99f549263c37 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -491,6 +491,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
 
 		cap->driver_data = cros_port;
 		cap->ops = &cros_typec_usb_mode_ops;
+		cap->alt_mode_override = typec->ap_driven_altmode;
 
 		cros_port->port = typec_register_port(dev, cap);
 		if (IS_ERR(cros_port->port)) {
-- 
2.50.1.565.gc32cd1483b-goog


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

* [PATCH v3 03/10] usb: typec: ucsi: Set alt_mode_override flag
  2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 01/10] usb: typec: Add alt_mode_override field to port property Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 02/10] platform/chrome: cros_ec_typec: Set alt_mode_override flag Andrei Kuchynski
@ 2025-08-04  9:03 ` Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs Andrei Kuchynski
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, linux-kernel,
	Andrei Kuchynski

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

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

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 5739ea2abdd1..5ba8b1bc874b 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1616,6 +1616,8 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
 
 	cap->driver_data = con;
 	cap->ops = &ucsi_ops;
+	cap->alt_mode_override =
+		!!(con->ucsi->cap.features & UCSI_CAP_ALT_MODE_OVERRIDE);
 
 	if (ucsi->ops->update_connector)
 		ucsi->ops->update_connector(con);
-- 
2.50.1.565.gc32cd1483b-goog


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

* [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs
  2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (2 preceding siblings ...)
  2025-08-04  9:03 ` [PATCH v3 03/10] usb: typec: ucsi: " Andrei Kuchynski
@ 2025-08-04  9:03 ` Andrei Kuchynski
  2025-08-11 14:25   ` Heikki Krogerus
  2025-08-04  9:03 ` [PATCH v3 05/10] usb: typec: Implement automated mode selection Andrei Kuchynski
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, linux-kernel,
	Andrei Kuchynski

This patch introduces new sysfs attributes to allow users to configure
and view Type-C mode priorities.

`priority`, `usb4_priority` attributes allow users to assign a numeric
priority to DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.

`mode_priorities` - read-only attribute that displays an ordered list
of all modes based on their configured priorities.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 Documentation/ABI/testing/sysfs-class-typec |  33 +++++
 drivers/usb/typec/Makefile                  |   2 +-
 drivers/usb/typec/class.c                   | 103 +++++++++++++++-
 drivers/usb/typec/class.h                   |   1 +
 drivers/usb/typec/mode_selection.c          | 130 ++++++++++++++++++++
 drivers/usb/typec/mode_selection.h          |  23 ++++
 include/linux/usb/typec_altmode.h           |   7 ++
 7 files changed, 295 insertions(+), 4 deletions(-)
 create mode 100644 drivers/usb/typec/mode_selection.c
 create mode 100644 drivers/usb/typec/mode_selection.h

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index 38e101c17a00..575dd94f33ab 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -162,6 +162,39 @@ Description:	Lists the supported USB Modes. The default USB mode that is used
 		- usb3 (USB 3.2)
 		- usb4 (USB4)
 
+		What:		/sys/class/typec/<port>/<alt-mode>/priority
+Date:		July 2025
+Contact:	Andrei Kuchynski <akuchynski@chromium.org>
+Description:
+		Displays and allows setting the priority for a specific alt-mode.
+		When read, it shows the current integer priority value. Lower numerical
+		values indicate higher priority (0 is the highest priority).
+		If the new value is already in use by another mode, the priority of the
+		conflicting mode and any subsequent modes will be incremented until they
+		are all unique.
+		This attribute is visible only if the kernel supports mode selection.
+
+		What:		/sys/class/typec/<port>/usb4_priority
+Date:		July 2025
+Contact:	Andrei Kuchynski <akuchynski@chromium.org>
+Description:
+		Displays and allows setting the priority for USB4 mode. Its behavior and
+		priority numbering scheme are identical to the general alt-mode
+		"priority" attributes.
+
+What:		/sys/class/typec/<port>/mode_priorities
+Date:		July 2025
+Contact:	Andrei Kuchynski <akuchynski@chromium.org>
+Description:	This read-only file lists the modes supported by the port,
+		ordered by their activation priority. It reflects the preferred sequence
+		the kernel will attempt to activate modes (DisplayPort alt-mode,
+		Thunderbolt alt-mode, USB4 mode).
+		This attribute is visible only if the kernel supports mode selection.
+
+		Example values:
+		- "USB4 Thunderbolt3 DisplayPort"
+		- "DisplayPort": the port only supports Displayport alt-mode
+
 USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
 
 What:		/sys/class/typec/<port>-partner/accessory_mode
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 7a368fea61bc..8a6a1c663eb6 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_TYPEC)		+= typec.o
-typec-y				:= class.o mux.o bus.o pd.o retimer.o
+typec-y				:= class.o mux.o bus.o pd.o retimer.o mode_selection.o
 typec-$(CONFIG_ACPI)		+= port-mapper.o
 obj-$(CONFIG_TYPEC)		+= altmodes/
 obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index a72325ff099a..414d94c45ab9 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -19,6 +19,7 @@
 #include "bus.h"
 #include "class.h"
 #include "pd.h"
+#include "mode_selection.h"
 
 static DEFINE_IDA(typec_index_ida);
 
@@ -445,11 +446,45 @@ svid_show(struct device *dev, struct device_attribute *attr, char *buf)
 }
 static DEVICE_ATTR_RO(svid);
 
+static ssize_t priority_store(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t size)
+{
+	struct typec_altmode *adev = to_typec_altmode(dev);
+	unsigned int val;
+	int err = kstrtouint(buf, 10, &val);
+
+	if (!err) {
+		err = typec_mode_set_priority(to_typec_port(adev->dev.parent),
+			typec_svid_to_altmode(adev->svid), val);
+		if (!err)
+			return size;
+	}
+
+	return err;
+}
+
+static ssize_t priority_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct typec_altmode *adev = to_typec_altmode(dev);
+	int val;
+	const int err = typec_mode_get_priority(to_typec_port(adev->dev.parent),
+			typec_svid_to_altmode(adev->svid), &val);
+
+	if (err)
+		return err;
+
+	return sprintf(buf, "%d\n", val);
+}
+static DEVICE_ATTR_RW(priority);
+
 static struct attribute *typec_altmode_attrs[] = {
 	&dev_attr_active.attr,
 	&dev_attr_mode.attr,
 	&dev_attr_svid.attr,
 	&dev_attr_vdo.attr,
+	&dev_attr_priority.attr,
 	NULL
 };
 
@@ -458,7 +493,7 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
 {
 	struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
 
-	if (attr == &dev_attr_active.attr)
+	if (attr == &dev_attr_active.attr) {
 		if (!is_typec_port(adev->dev.parent)) {
 			struct typec_partner *partner =
 				to_typec_partner(adev->dev.parent);
@@ -469,6 +504,15 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
 				!adev->ops->activate)
 				return 0444;
 		}
+	} else if (attr == &dev_attr_priority.attr) {
+		if (is_typec_port(adev->dev.parent))  {
+			struct typec_port *port = to_typec_port(adev->dev.parent);
+
+			if (!port->alt_mode_override)
+				return 0;
+		} else
+			return 0;
+	}
 
 	return attr->mode;
 }
@@ -1942,6 +1986,44 @@ static ssize_t orientation_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(orientation);
 
+static ssize_t mode_priorities_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	return typec_mode_get_priority_list(to_typec_port(dev), buf);
+}
+static DEVICE_ATTR_RO(mode_priorities);
+
+static ssize_t usb4_priority_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct typec_port *port = to_typec_port(dev);
+	int val;
+	const int err = typec_mode_get_priority(port, TYPEC_USB4_MODE, &val);
+
+	if (err)
+		return err;
+
+	return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t usb4_priority_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t size)
+{
+	struct typec_port *port = to_typec_port(dev);
+	unsigned int val;
+	int err = kstrtouint(buf, 10, &val);
+
+	if (!err) {
+		err = typec_mode_set_priority(port, TYPEC_USB4_MODE, val);
+		if (!err)
+			return size;
+	}
+
+	return err;
+}
+static DEVICE_ATTR_RW(usb4_priority);
+
 static struct attribute *typec_attrs[] = {
 	&dev_attr_data_role.attr,
 	&dev_attr_power_operation_mode.attr,
@@ -1954,6 +2036,8 @@ static struct attribute *typec_attrs[] = {
 	&dev_attr_port_type.attr,
 	&dev_attr_orientation.attr,
 	&dev_attr_usb_capability.attr,
+	&dev_attr_mode_priorities.attr,
+	&dev_attr_usb4_priority.attr,
 	NULL,
 };
 
@@ -1992,6 +2076,13 @@ static umode_t typec_attr_is_visible(struct kobject *kobj,
 			return 0;
 		if (!port->ops || !port->ops->default_usb_mode_set)
 			return 0444;
+	} else if (attr == &dev_attr_mode_priorities.attr) {
+		if (!port->alt_mode_override)
+			return 0;
+	} else if (attr == &dev_attr_usb4_priority.attr) {
+		if (!port->alt_mode_override ||
+			!(port->cap->usb_capability & USB_CAPABILITY_USB4))
+			return 0;
 	}
 
 	return attr->mode;
@@ -2029,6 +2120,7 @@ static void typec_release(struct device *dev)
 	typec_mux_put(port->mux);
 	typec_retimer_put(port->retimer);
 	kfree(port->cap);
+	typec_mode_selection_destroy(port);
 	kfree(port);
 }
 
@@ -2496,6 +2588,8 @@ typec_port_register_altmode(struct typec_port *port,
 		to_altmode(adev)->retimer = retimer;
 	}
 
+	typec_mode_set_priority(port, typec_svid_to_altmode(adev->svid), -1);
+
 	return adev;
 }
 EXPORT_SYMBOL_GPL(typec_port_register_altmode);
@@ -2645,9 +2739,12 @@ struct typec_port *typec_register_port(struct device *parent,
 	port->con.attach = typec_partner_attach;
 	port->con.deattach = typec_partner_deattach;
 
-	if (cap->usb_capability & USB_CAPABILITY_USB4)
+	typec_mode_selection_init(port);
+
+	if (cap->usb_capability & USB_CAPABILITY_USB4) {
 		port->usb_mode = USB_MODE_USB4;
-	else if (cap->usb_capability & USB_CAPABILITY_USB3)
+		typec_mode_set_priority(port, TYPEC_USB4_MODE, -1);
+	} else if (cap->usb_capability & USB_CAPABILITY_USB3)
 		port->usb_mode = USB_MODE_USB3;
 	else if (cap->usb_capability & USB_CAPABILITY_USB2)
 		port->usb_mode = USB_MODE_USB2;
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index f05d9201c233..c6467e576569 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -82,6 +82,7 @@ struct typec_port {
 	struct device			*usb3_dev;
 
 	bool				alt_mode_override;
+	struct list_head		mode_list;
 };
 
 #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
diff --git a/drivers/usb/typec/mode_selection.c b/drivers/usb/typec/mode_selection.c
new file mode 100644
index 000000000000..9a7185c07d0c
--- /dev/null
+++ b/drivers/usb/typec/mode_selection.c
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Google LLC.
+ */
+
+#include <linux/usb/typec_altmode.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+#include "mode_selection.h"
+#include "class.h"
+
+static const char * const mode_names[TYPEC_MODE_MAX] = {
+	[TYPEC_DP_ALTMODE] = "DisplayPort",
+	[TYPEC_TBT_ALTMODE] = "Thunderbolt3",
+	[TYPEC_USB4_MODE] = "USB4",
+};
+
+static const int default_priorities[TYPEC_MODE_MAX] = {
+	[TYPEC_DP_ALTMODE] = 2,
+	[TYPEC_TBT_ALTMODE] = 1,
+	[TYPEC_USB4_MODE] = 0,
+};
+
+/**
+ * struct mode_selection_state - State tracking for a specific Type-C mode
+ * @mode: The type of mode this instance represents
+ * @name: Name string pointer
+ * @priority: The mode priority. Higher values indicate a more preferred mode.
+ * @list: List head to link this mode state into a prioritized list.
+ */
+struct mode_selection_state {
+	enum typec_mode_type mode;
+	const char *name;
+	int priority;
+	struct list_head list;
+};
+
+/* -------------------------------------------------------------------------- */
+/* port 'mode_priorities' attribute */
+void typec_mode_selection_init(struct typec_port *port)
+{
+	INIT_LIST_HEAD(&port->mode_list);
+}
+
+void typec_mode_selection_destroy(struct typec_port *port)
+{
+	struct mode_selection_state *ms, *tmp;
+
+	list_for_each_entry_safe(ms, tmp, &port->mode_list, list) {
+		list_del(&ms->list);
+		kfree(ms);
+	}
+}
+
+int typec_mode_set_priority(struct typec_port *port,
+		const enum typec_mode_type mode, const int priority)
+{
+	struct mode_selection_state *ms_target = NULL;
+	struct mode_selection_state *ms, *tmp;
+
+	if (mode >= TYPEC_MODE_MAX || !mode_names[mode])
+		return -EOPNOTSUPP;
+
+	list_for_each_entry_safe(ms, tmp, &port->mode_list, list) {
+		if (ms->mode == mode) {
+			ms_target = ms;
+			list_del(&ms->list);
+			break;
+		}
+	}
+
+	if (!ms_target) {
+		ms_target = kzalloc(sizeof(struct mode_selection_state), GFP_KERNEL);
+		if (!ms_target)
+			return -ENOMEM;
+		ms_target->mode = mode;
+		ms_target->name = mode_names[mode];
+		INIT_LIST_HEAD(&ms_target->list);
+	}
+
+	if (priority >= 0)
+		ms_target->priority = priority;
+	else
+		ms_target->priority = default_priorities[mode];
+
+	while (ms_target) {
+		struct mode_selection_state *ms_peer = NULL;
+
+		list_for_each_entry(ms, &port->mode_list, list)
+			if (ms->priority >= ms_target->priority) {
+				if (ms->priority == ms_target->priority)
+					ms_peer = ms;
+				break;
+			}
+
+		list_add_tail(&ms_target->list, &ms->list);
+		ms_target = ms_peer;
+		if (ms_target) {
+			ms_target->priority++;
+			list_del(&ms_target->list);
+		}
+	}
+
+	return 0;
+}
+
+int typec_mode_get_priority(struct typec_port *port,
+		const enum typec_mode_type mode, int *priority)
+{
+	struct mode_selection_state *ms;
+
+	list_for_each_entry(ms, &port->mode_list, list)
+		if (ms->mode == mode) {
+			*priority = ms->priority;
+			return 0;
+		}
+
+	return -EOPNOTSUPP;
+}
+
+ssize_t typec_mode_get_priority_list(struct typec_port *port, char *buf)
+{
+	struct mode_selection_state *ms;
+	ssize_t count = 0;
+
+	list_for_each_entry(ms, &port->mode_list, list)
+		count += sysfs_emit_at(buf, count, "%s ", ms->name);
+
+	return count + sysfs_emit_at(buf, count, "\n");
+}
diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
new file mode 100644
index 000000000000..151f0f8b6632
--- /dev/null
+++ b/drivers/usb/typec/mode_selection.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_tbt.h>
+
+static inline enum typec_mode_type typec_svid_to_altmode(const u16 svid)
+{
+	switch (svid) {
+	case USB_TYPEC_DP_SID:
+		return TYPEC_DP_ALTMODE;
+	case USB_TYPEC_TBT_SID:
+		return TYPEC_TBT_ALTMODE;
+	}
+	return TYPEC_MODE_MAX;
+}
+
+void typec_mode_selection_init(struct typec_port *port);
+void typec_mode_selection_destroy(struct typec_port *port);
+int typec_mode_set_priority(struct typec_port *port,
+		const enum typec_mode_type mode, const int priority);
+int typec_mode_get_priority(struct typec_port *port,
+		const enum typec_mode_type mode, int *priority);
+ssize_t typec_mode_get_priority_list(struct typec_port *port, char *buf);
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index b3c0866ea70f..5d14363e02eb 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -145,6 +145,13 @@ enum {
 
 #define TYPEC_MODAL_STATE(_state_)	((_state_) + TYPEC_STATE_MODAL)
 
+enum typec_mode_type {
+	TYPEC_DP_ALTMODE = 0,
+	TYPEC_TBT_ALTMODE,
+	TYPEC_USB4_MODE,
+	TYPEC_MODE_MAX,
+};
+
 struct typec_altmode *typec_altmode_get_plug(struct typec_altmode *altmode,
 					     enum typec_plug_index index);
 void typec_altmode_put_plug(struct typec_altmode *plug);
-- 
2.50.1.565.gc32cd1483b-goog


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

* [PATCH v3 05/10] usb: typec: Implement automated mode selection
  2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (3 preceding siblings ...)
  2025-08-04  9:03 ` [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs Andrei Kuchynski
@ 2025-08-04  9:03 ` Andrei Kuchynski
  2025-08-04 13:00   ` kernel test robot
  2025-08-11 15:31   ` Heikki Krogerus
  2025-08-04  9:03 ` [PATCH v3 06/10] usb: typec: Report altmode entry status via callback Andrei Kuchynski
                   ` (4 subsequent siblings)
  9 siblings, 2 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, linux-kernel,
	Andrei Kuchynski

This patch introduces new sysfs attributes to enable user control over
Type-C automated mode selection and provide negotiation feedback.

`mode_selection` attribute shows a prioritized list of supported modes
with the currently entered mode bracketed. Writing boolean 0 or 1 to
this attribute starts or stops the mode selection process,
respectively.

`entry_result`, `usb4_entry_result` read-only attributes show the
result of the last mode selection attempt for a specific mode.

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

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index 575dd94f33ab..ed89b9880085 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -280,6 +280,45 @@ 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:		July 2025
+Contact:	Andrei Kuchynski <akuchynski@chromium.org>
+Description:	Displays a prioritized list of modes that both the port and the
+		partner support with the currently entered mode bracketed. Parentheses
+		indicates a mode currently in progress. Modes listed before the active
+		or in-progress mode have failed.
+		Automated 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.
+		This attribute is only present if the kernel supports AP driven mode
+		entry, where the Application Processor manages USB Type-C alt-modes.
+
+		Example values:
+		- "USB4 (TBT) DP": USB4 mode entry failed, Thunderbolt alt-mode is in
+			progress, DisplayPort alt-mode is next.
+		- "[USB4] TBT DP": USB4 mode is currently active.
+
+What:		/sys/class/typec/<port>-partner/<alt-mode>/entry_result
+Date:		July 2025
+Contact:	Andrei Kuchynski <akuchynski@chromium.org>
+Description:	This read-only file represents the status for a specific
+		alt-mode after the last mode selection process.
+		This attribute is visible only if the kernel supports mode selection.
+
+		Example values:
+		- "none": No mode selection attempt has occurred for this alt-mode.
+		- "in progress": The mode entry process is currently underway.
+		- "active": The alt-mode is currently active.
+		- "cable failed": The connected cable doesn't support the mode.
+		- "timeout": Mode entry failed due to a timeout.
+		- "failed": The attempt to activate the mode failed.
+
+What:		/sys/class/typec/<port>-partner/usb4_entry_result
+Date:		July 2025
+Contact:	Andrei Kuchynski <akuchynski@chromium.org>
+Description:	Represents a status for USB4 mode. Its values are identical to
+		the general <alt-mode>/entry_result attributes.
+
 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 414d94c45ab9..f9515fc594f8 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -479,12 +479,24 @@ static ssize_t priority_show(struct device *dev,
 }
 static DEVICE_ATTR_RW(priority);
 
+static ssize_t entry_result_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct typec_altmode *adev = to_typec_altmode(dev);
+	struct typec_partner *partner = to_typec_partner(adev->dev.parent);
+
+	return typec_mode_selection_get_result(partner,
+		typec_svid_to_altmode(adev->svid), buf);
+}
+static DEVICE_ATTR_RO(entry_result);
+
 static struct attribute *typec_altmode_attrs[] = {
 	&dev_attr_active.attr,
 	&dev_attr_mode.attr,
 	&dev_attr_svid.attr,
 	&dev_attr_vdo.attr,
 	&dev_attr_priority.attr,
+	&dev_attr_entry_result.attr,
 	NULL
 };
 
@@ -508,6 +520,17 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
 		if (is_typec_port(adev->dev.parent))  {
 			struct typec_port *port = to_typec_port(adev->dev.parent);
 
+			if (!port->alt_mode_override)
+				return 0;
+		} else
+			return 0;
+	} else if (attr == &dev_attr_entry_result.attr) {
+		if (is_typec_partner(adev->dev.parent))  {
+			struct typec_partner *partner =
+				to_typec_partner(adev->dev.parent);
+			struct typec_port *port =
+				to_typec_port(partner->dev.parent);
+
 			if (!port->alt_mode_override)
 				return 0;
 		} else
@@ -584,7 +607,7 @@ static void typec_altmode_release(struct device *dev)
 }
 
 const struct device_type typec_altmode_dev_type = {
-	.name = "typec_alternate_mode",
+	.name = ALTERNATE_MODE_DEVICE_TYPE_NAME,
 	.groups = typec_altmode_groups,
 	.release = typec_altmode_release,
 };
@@ -784,6 +807,44 @@ static ssize_t number_of_alternate_modes_show(struct device *dev, struct device_
 }
 static DEVICE_ATTR_RO(number_of_alternate_modes);
 
+static ssize_t mode_selection_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct typec_partner *partner = to_typec_partner(dev);
+
+	return typec_mode_selection_get_active(partner, buf);
+}
+
+static ssize_t mode_selection_store(struct device *dev, struct device_attribute *attr,
+			      const char *buf, size_t size)
+{
+	struct typec_partner *partner = to_typec_partner(dev);
+	bool start;
+	int ret = kstrtobool(buf, &start);
+
+	if (!ret) {
+		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 ssize_t usb4_entry_result_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	return typec_mode_selection_get_result(to_typec_partner(dev),
+		TYPEC_USB4_MODE, buf);
+}
+static DEVICE_ATTR_RO(usb4_entry_result);
+
 static struct attribute *typec_partner_attrs[] = {
 	&dev_attr_accessory_mode.attr,
 	&dev_attr_supports_usb_power_delivery.attr,
@@ -791,6 +852,8 @@ 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,
+	&dev_attr_usb4_entry_result.attr,
 	NULL
 };
 
@@ -815,6 +878,16 @@ static umode_t typec_partner_attr_is_visible(struct kobject *kobj, struct attrib
 		if (!get_pd_product_type(kobj_to_dev(kobj)))
 			return 0;
 
+	if (attr == &dev_attr_mode_selection.attr)
+		if (!port->alt_mode_override)
+			return 0;
+
+	if (attr == &dev_attr_usb4_entry_result.attr) {
+		if (!port->alt_mode_override ||
+			!(partner->usb_capability & USB_CAPABILITY_USB4))
+			return 0;
+	}
+
 	return attr->mode;
 }
 
@@ -893,8 +966,10 @@ int typec_partner_set_identity(struct typec_partner *partner)
 			usb_capability |= USB_CAPABILITY_USB2;
 		if (devcap & DEV_USB3_CAPABLE)
 			usb_capability |= USB_CAPABILITY_USB3;
-		if (devcap & DEV_USB4_CAPABLE)
+		if (devcap & DEV_USB4_CAPABLE) {
 			usb_capability |= USB_CAPABILITY_USB4;
+			typec_mode_selection_add_mode(partner, TYPEC_USB4_MODE);
+		}
 	} else {
 		usb_capability = PD_VDO_DFP_HOSTCAP(id->vdo[0]);
 	}
@@ -1014,7 +1089,12 @@ struct typec_altmode *
 typec_partner_register_altmode(struct typec_partner *partner,
 			       const struct typec_altmode_desc *desc)
 {
-	return typec_register_altmode(&partner->dev, desc);
+	struct typec_altmode *alt = typec_register_altmode(&partner->dev, desc);
+
+	if (alt)
+		typec_mode_selection_add_mode(partner, typec_svid_to_altmode(alt->svid));
+
+	return alt;
 }
 EXPORT_SYMBOL_GPL(typec_partner_register_altmode);
 
@@ -1118,6 +1198,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);
@@ -1135,6 +1217,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);
@@ -1403,6 +1486,7 @@ int typec_cable_set_identity(struct typec_cable *cable)
 }
 EXPORT_SYMBOL_GPL(typec_cable_set_identity);
 
+static struct typec_partner *typec_get_partner(struct typec_port *port);
 /**
  * typec_register_cable - Register a USB Type-C Cable
  * @port: The USB Type-C Port the cable is connected to
@@ -1417,6 +1501,7 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
 					 struct typec_cable_desc *desc)
 {
 	struct typec_cable *cable;
+	struct typec_partner *partner;
 	int ret;
 
 	cable = kzalloc(sizeof(*cable), GFP_KERNEL);
@@ -1448,6 +1533,10 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
 		return ERR_PTR(ret);
 	}
 
+	partner = typec_get_partner(port);
+	typec_mode_selection_add_cable(partner, cable);
+	put_device(&partner->dev);
+
 	return cable;
 }
 EXPORT_SYMBOL_GPL(typec_register_cable);
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index c6467e576569..281dcb6d675c 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -5,6 +5,8 @@
 
 #include <linux/device.h>
 #include <linux/usb/typec.h>
+#include <linux/usb/typec_altmode.h>
+#include <linux/kfifo.h>
 
 struct typec_mux;
 struct typec_switch;
@@ -26,6 +28,8 @@ struct typec_cable {
 	enum usb_pd_svdm_ver		svdm_version;
 };
 
+struct mode_selection_state;
+
 struct typec_partner {
 	struct device			dev;
 	unsigned int			usb_pd:1;
@@ -40,6 +44,12 @@ struct typec_partner {
 
 	struct usb_power_delivery	*pd;
 
+	struct delayed_work mode_selection_work;
+	DECLARE_KFIFO(mode_sequence, struct mode_selection_state *,
+			roundup_pow_of_two(TYPEC_MODE_MAX));
+	struct mutex mode_sequence_lock;
+	struct mode_selection_state *active_mode;
+
 	void (*attach)(struct typec_partner *partner, struct device *dev);
 	void (*deattach)(struct typec_partner *partner, struct device *dev);
 };
@@ -112,4 +122,6 @@ static inline int typec_link_ports(struct typec_port *connector) { return 0; }
 static inline void typec_unlink_ports(struct typec_port *connector) { }
 #endif
 
+#define ALTERNATE_MODE_DEVICE_TYPE_NAME "typec_alternate_mode"
+
 #endif /* __USB_TYPEC_CLASS__ */
diff --git a/drivers/usb/typec/mode_selection.c b/drivers/usb/typec/mode_selection.c
index 9a7185c07d0c..c7d164478787 100644
--- a/drivers/usb/typec/mode_selection.c
+++ b/drivers/usb/typec/mode_selection.c
@@ -5,10 +5,19 @@
 
 #include <linux/usb/typec_altmode.h>
 #include <linux/slab.h>
+#include <linux/usb/pd_vdo.h>
+#include <linux/kfifo.h>
 #include <linux/list.h>
 #include "mode_selection.h"
 #include "class.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;
+
 static const char * const mode_names[TYPEC_MODE_MAX] = {
 	[TYPEC_DP_ALTMODE] = "DisplayPort",
 	[TYPEC_TBT_ALTMODE] = "Thunderbolt3",
@@ -21,18 +30,59 @@ static const int default_priorities[TYPEC_MODE_MAX] = {
 	[TYPEC_USB4_MODE] = 0,
 };
 
+/**
+ * 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_CABLE_FAILED: The connected cable doesn't support the mode
+ * @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_CABLE_FAILED,
+	MS_STATE_TIMEOUT,
+	MS_STATE_FAILED,
+	MS_STATE_MAX
+};
+static const char * const ms_state_strings[MS_STATE_MAX] = {
+	[MS_STATE_IDLE] = "none",
+	[MS_STATE_INPROGRESS] = "in progress",
+	[MS_STATE_ACTIVE] = "active",
+	[MS_STATE_CABLE_FAILED] = "cable failed",
+	[MS_STATE_TIMEOUT] = "timeout",
+	[MS_STATE_FAILED] = "failed",
+};
+
 /**
  * struct mode_selection_state - State tracking for a specific Type-C mode
  * @mode: The type of mode this instance represents
  * @name: Name string pointer
  * @priority: The mode priority. Higher values indicate a more preferred mode.
  * @list: List head to link this mode state into a prioritized list.
+ * @partner_supported: Flag indicating if this mode is supported by the partner
+ * @cable_supported: Flag indicating if this mode is supported by the cable
+ * @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
  */
 struct mode_selection_state {
 	enum typec_mode_type mode;
 	const char *name;
 	int priority;
 	struct list_head list;
+
+	bool partner_supported;
+	bool cable_supported;
+	bool enter;
+	int attempt_count;
+	enum ms_state state;
+	int error;
 };
 
 /* -------------------------------------------------------------------------- */
@@ -128,3 +178,398 @@ ssize_t typec_mode_get_priority_list(struct typec_port *port, char *buf)
 
 	return count + sysfs_emit_at(buf, count, "\n");
 }
+
+/* -------------------------------------------------------------------------- */
+/* partner 'mod_selection' attribute */
+
+/**
+ * mode_selection_next() - Process mode selection results and schedule next
+ * action
+ * @partner: pointer to the partner structure
+ * @ms: pointer to active mode_selection_state object that is on top in
+ * mode_sequence FIFO.
+ *
+ * The mutex protecting the mode_sequence FIFO 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()` if further actions are required.
+ *
+ * If the previous mode entry was successful, the mode selection sequence is
+ * considered complete for the current cycle.
+ *
+ * If the previous mode entry failed, this function schedules
+ * `mode_selection_work()` to attempt exiting the mode that was partially
+ * activated but not fully entered.
+ *
+ * If the previous operation was an exit (after a failed entry attempt),
+ * `mode_selection_next()` then advances the internal list of candidate
+ * modes to determine the next mode to enter.
+ */
+static void mode_selection_next(
+	struct typec_partner *partner, struct mode_selection_state *ms)
+
+	__must_hold(&partner->mode_sequence_lock)
+{
+	if (!ms->enter) {
+		kfifo_skip(&partner->mode_sequence);
+	} else if (ms->state == MS_STATE_INPROGRESS && !ms->error) {
+		ms->state = MS_STATE_ACTIVE;
+		partner->active_mode = ms;
+		kfifo_reset(&partner->mode_sequence);
+	} 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;
+	}
+
+	if (!kfifo_is_empty(&partner->mode_sequence))
+		schedule_delayed_work(&partner->mode_selection_work,
+			msecs_to_jiffies(mode_selection_delay));
+}
+
+static void mode_selection_complete(struct typec_partner *partner,
+			const enum typec_mode_type mode, const int error)
+{
+	struct mode_selection_state *ms;
+
+	mutex_lock(&partner->mode_sequence_lock);
+	if (kfifo_peek(&partner->mode_sequence, &ms)) {
+		if (ms->mode == mode && ms->state == MS_STATE_INPROGRESS) {
+			ms->error = error;
+			cancel_delayed_work(&partner->mode_selection_work);
+			mode_selection_next(partner, ms);
+		}
+	}
+	mutex_unlock(&partner->mode_sequence_lock);
+}
+
+void typec_mode_selection_altmode_complete(struct typec_altmode *alt,
+				const int error)
+{
+	mode_selection_complete(to_typec_partner(alt->dev.parent),
+		typec_svid_to_altmode(alt->svid), error);
+}
+EXPORT_SYMBOL_GPL(typec_mode_selection_altmode_complete);
+
+void typec_mode_selection_usb4_complete(struct typec_partner *partner,
+				const int error)
+{
+	mode_selection_complete(partner, TYPEC_USB4_MODE, error);
+}
+EXPORT_SYMBOL_GPL(typec_mode_selection_usb4_complete);
+
+static void mode_selection_activate_usb4_mode(struct typec_partner *partner,
+	struct mode_selection_state *ms)
+{
+	struct typec_port *port = to_typec_port(partner->dev.parent);
+	int error = -EOPNOTSUPP;
+
+	if (port->ops && port->ops->enter_usb_mode) {
+		if (ms->enter && port->usb_mode != USB_MODE_USB4)
+			error = -EPERM;
+		else
+			error = port->ops->enter_usb_mode(port,
+				ms->enter ? USB_MODE_USB4 : USB_MODE_USB3);
+	}
+
+	if (ms->enter)
+		ms->error = error;
+}
+
+static int mode_selection_activate_altmode(struct device *dev, void *data)
+{
+	struct typec_altmode *alt = to_typec_altmode(dev);
+	struct mode_selection_state *ms = (struct mode_selection_state *)data;
+	int error = -ENODEV;
+	int ret = 0;
+
+	if (!strcmp(dev->type->name, ALTERNATE_MODE_DEVICE_TYPE_NAME)) {
+		if (ms->mode == typec_svid_to_altmode(alt->svid)) {
+			if (alt->ops && alt->ops->activate)
+				error = alt->ops->activate(alt, ms->enter);
+			else
+				error = -EOPNOTSUPP;
+			ret = 1;
+		}
+	}
+
+	if (ms->enter)
+		ms->error = error;
+
+	return ret;
+}
+
+static void mode_selection_activate_mode(struct typec_partner *partner,
+	struct mode_selection_state *ms)
+{
+	if (ms->enter)
+		ms->attempt_count++;
+
+	if (ms->mode == TYPEC_USB4_MODE)
+		mode_selection_activate_usb4_mode(partner, ms);
+	else
+		device_for_each_child(&partner->dev, ms,
+			mode_selection_activate_altmode);
+}
+
+/**
+ * mode_selection_work() - 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_mode()`) 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()` is scheduled for a subsequent entry after a timeout
+ * period. The alternate mode driver is expected to call back with the actual
+ * mode entry result. Upon this callback, `mode_selection_next()` will determine
+ * the subsequent mode and re-schedule `mode_selection_work()`.
+ */
+static void mode_selection_work(struct work_struct *work)
+{
+	struct typec_partner *partner = container_of(work, struct typec_partner,
+						  mode_selection_work.work);
+	struct mode_selection_state *ms;
+
+	mutex_lock(&partner->mode_sequence_lock);
+	if (kfifo_peek(&partner->mode_sequence, &ms)) {
+		if (ms->state == MS_STATE_INPROGRESS) {
+			ms->state = MS_STATE_TIMEOUT;
+			mode_selection_next(partner, ms);
+		} else {
+			mode_selection_activate_mode(partner, ms);
+
+			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_sequence_lock);
+}
+
+static void mode_selection_clear_results(struct typec_partner *partner)
+{
+	struct typec_port *port = to_typec_port(partner->dev.parent);
+	struct mode_selection_state *ms;
+
+	list_for_each_entry(ms, &port->mode_list, list) {
+		ms->enter = true;
+		ms->state = MS_STATE_IDLE;
+		ms->error = 0;
+		ms->attempt_count = 0;
+	}
+
+	kfifo_reset(&partner->mode_sequence);
+	partner->active_mode = NULL;
+}
+
+void typec_mode_selection_add_partner(struct typec_partner *partner)
+{
+	struct typec_port *port = to_typec_port(partner->dev.parent);
+	struct mode_selection_state *ms;
+
+	list_for_each_entry(ms, &port->mode_list, list) {
+		ms->partner_supported = false;
+		ms->cable_supported = false;
+	}
+
+	INIT_KFIFO(partner->mode_sequence);
+	mutex_init(&partner->mode_sequence_lock);
+	mode_selection_clear_results(partner);
+	INIT_DELAYED_WORK(&partner->mode_selection_work, mode_selection_work);
+}
+
+void typec_mode_selection_add_mode(struct typec_partner *partner,
+		const enum typec_mode_type mode)
+{
+	struct typec_port *port = to_typec_port(partner->dev.parent);
+	struct mode_selection_state *ms;
+
+	list_for_each_entry(ms, &port->mode_list, list) {
+		if (ms->mode == mode) {
+			ms->partner_supported = true;
+			break;
+		}
+	}
+}
+
+void typec_mode_selection_add_cable(struct typec_partner *partner,
+		struct typec_cable *cable)
+{
+	struct typec_port *port = to_typec_port(partner->dev.parent);
+	struct mode_selection_state *ms;
+	const u32 id_header = cable->identity->id_header;
+	const u32 vdo1 = cable->identity->vdo[0];
+	const u32 type = PD_IDH_PTYPE(id_header);
+	const u32 speed = VDO_TYPEC_CABLE_SPEED(vdo1);
+	/*
+	 * Some USB devices supporting DisplayPort lack valid cable VDO.
+	 * Allow only DP mode in this case.
+	 */
+	bool capability[TYPEC_MODE_MAX] = {
+		[TYPEC_DP_ALTMODE] = true,
+		[TYPEC_TBT_ALTMODE] = false,
+		[TYPEC_USB4_MODE] = false,
+	};
+
+	if (type == IDH_PTYPE_PCABLE) {
+		capability[TYPEC_DP_ALTMODE] = (speed > CABLE_USB2_ONLY);
+		capability[TYPEC_TBT_ALTMODE] = (speed > CABLE_USB2_ONLY);
+		capability[TYPEC_USB4_MODE] = (speed > CABLE_USB2_ONLY);
+	} else if (type == IDH_PTYPE_ACABLE) {
+		const u32 vdo2 = cable->identity->vdo[1];
+		const u32 version = VDO_TYPEC_CABLE_VERSION(vdo1);
+		const bool usb4_support = VDO_TYPEC_CABLE_USB4_SUPP(vdo2);
+		const bool modal_support = PD_IDH_MODAL_SUPP(id_header);
+
+		capability[TYPEC_DP_ALTMODE] = modal_support;
+		capability[TYPEC_TBT_ALTMODE] = true;
+		if (version == CABLE_VDO_VER1_3)
+			capability[TYPEC_USB4_MODE] = usb4_support;
+		else
+			capability[TYPEC_USB4_MODE] = modal_support;
+	}
+
+	list_for_each_entry(ms, &port->mode_list, list)
+		ms->cable_supported = capability[ms->mode];
+}
+
+void typec_mode_selection_remove_partner(struct typec_partner *partner)
+{
+	mutex_lock(&partner->mode_sequence_lock);
+	kfifo_reset(&partner->mode_sequence);
+	mutex_unlock(&partner->mode_sequence_lock);
+
+	cancel_delayed_work_sync(&partner->mode_selection_work);
+	mutex_destroy(&partner->mode_sequence_lock);
+}
+
+/**
+ * typec_mode_selection_start() - Starts the mode selection process.
+ * @partner: pointer to the partner structure
+ *
+ * This function populates a 'mode_sequence' FIFO with pointers to
+ * `struct mode_selection_state` instances. The sequence is generated based on
+ * partner/cable capabilities and prioritized according to the port's settings.
+ */
+int typec_mode_selection_start(struct typec_partner *partner)
+{
+	struct typec_port *port = to_typec_port(partner->dev.parent);
+	struct mode_selection_state *ms;
+	int ret = 0;
+
+	mutex_lock(&partner->mode_sequence_lock);
+
+	if (!kfifo_is_empty(&partner->mode_sequence))
+		ret = -EINPROGRESS;
+	else if (partner->active_mode)
+		ret = -EALREADY;
+	else {
+		mode_selection_clear_results(partner);
+
+		list_for_each_entry(ms, &port->mode_list, list) {
+			if (ms->partner_supported) {
+				if (ms->cable_supported)
+					kfifo_put(&partner->mode_sequence, ms);
+				else
+					ms->state = MS_STATE_CABLE_FAILED;
+			}
+		}
+
+		if (kfifo_peek(&partner->mode_sequence, &ms))
+			schedule_delayed_work(&partner->mode_selection_work, 0);
+	}
+
+	mutex_unlock(&partner->mode_sequence_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 exit is already scheduled, or 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;
+
+	mutex_lock(&partner->mode_sequence_lock);
+	if (kfifo_peek(&partner->mode_sequence, &ms)) {
+		kfifo_reset(&partner->mode_sequence);
+
+		if (!ms->enter || ms->state != MS_STATE_IDLE) {
+			ms->attempt_count = mode_entry_attempts;
+			kfifo_put(&partner->mode_sequence, ms);
+			mutex_unlock(&partner->mode_sequence_lock);
+
+			return -EINPROGRESS;
+		}
+	}
+
+	if (partner->active_mode) {
+		partner->active_mode->enter = false;
+		mode_selection_activate_mode(partner, partner->active_mode);
+	}
+	mode_selection_clear_results(partner);
+	mutex_unlock(&partner->mode_sequence_lock);
+
+	return 0;
+}
+
+int typec_mode_selection_get_active(struct typec_partner *partner, char *buf)
+{
+	struct typec_port *port = to_typec_port(partner->dev.parent);
+	struct mode_selection_state *ms, *running_ms;
+	ssize_t count = 0;
+
+	mutex_lock(&partner->mode_sequence_lock);
+	if (!kfifo_peek(&partner->mode_sequence, &running_ms))
+		running_ms = NULL;
+
+	list_for_each_entry(ms, &port->mode_list, list) {
+		if (ms->partner_supported) {
+			if (ms->state == MS_STATE_ACTIVE)
+				count += sysfs_emit_at(buf, count, "[%s] ", ms->name);
+			else if (ms == running_ms)
+				count += sysfs_emit_at(buf, count, "(%s) ", ms->name);
+			else
+				count += sysfs_emit_at(buf, count, "%s ", ms->name);
+		}
+	}
+	mutex_unlock(&partner->mode_sequence_lock);
+
+	if (count)
+		count += sysfs_emit_at(buf, count, "\n");
+
+	return count;
+}
+
+int typec_mode_selection_get_result(struct typec_partner *partner,
+		const enum typec_mode_type mode, char *buf)
+{
+	struct typec_port *port = to_typec_port(partner->dev.parent);
+	struct mode_selection_state *ms;
+
+	list_for_each_entry(ms, &port->mode_list, list)
+		if (ms->mode == mode)
+			return sysfs_emit(buf, "%s\n", ms_state_strings[ms->state]);
+
+	return -EOPNOTSUPP;
+}
diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
index 151f0f8b6632..2238a7200eae 100644
--- a/drivers/usb/typec/mode_selection.h
+++ b/drivers/usb/typec/mode_selection.h
@@ -21,3 +21,34 @@ int typec_mode_set_priority(struct typec_port *port,
 int typec_mode_get_priority(struct typec_port *port,
 		const enum typec_mode_type mode, int *priority);
 ssize_t typec_mode_get_priority_list(struct typec_port *port, char *buf);
+
+/**
+ * The mode selection process follows a lifecycle tied to the USB-C partner
+ * device. The API is designed to first build a set of desired modes and then
+ * trigger the selection process. The expected sequence of calls is as follows:
+ *
+ * Creation and Configuration:
+ * call typec_mode_selection_add_partner() when the partner device is being set
+ * up. After creation, call typec_mode_selection_add_mode() and
+ * typec_mode_selection_add_cable() to define the parameters for the
+ * selection process.
+ *
+ * Execution:
+ * Call typec_mode_selection_start() to trigger the mode selection.
+ * Call typec_mode_selection_reset() to prematurely stop the selection
+ * process and clear any stored results.
+ *
+ * Destruction:
+ * Before destroying a partner, call typec_mode_selection_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);
+int typec_mode_selection_reset(struct typec_partner *partner);
+void typec_mode_selection_add_mode(struct typec_partner *partner,
+		const enum typec_mode_type mode);
+void typec_mode_selection_add_cable(struct typec_partner *partner,
+		struct typec_cable *cable);
+int typec_mode_selection_get_active(struct typec_partner *partner, char *buf);
+int typec_mode_selection_get_result(struct typec_partner *partner,
+		const enum typec_mode_type mode, char *buf);
diff --git a/include/linux/usb/pd_vdo.h b/include/linux/usb/pd_vdo.h
index 5c48e8a81403..20bcf37ad634 100644
--- a/include/linux/usb/pd_vdo.h
+++ b/include/linux/usb/pd_vdo.h
@@ -439,6 +439,8 @@
 	 | (trans) << 11 | (phy) << 10 | (ele) << 9 | (u4) << 8			\
 	 | ((hops) & 0x3) << 6 | (u2) << 5 | (u32) << 4 | (lane) << 3		\
 	 | (iso) << 2 | (gen))
+#define VDO_TYPEC_CABLE_VERSION(vdo) (((vdo) >> 21) & 0x7)
+#define VDO_TYPEC_CABLE_USB4_SUPP(vdo) (((vdo) & BIT(8)) == ACAB2_USB4_SUPP)
 
 /*
  * AMA VDO (PD Rev2.0)
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index 5d14363e02eb..f7fd51b4c23e 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -225,4 +225,9 @@ void typec_altmode_unregister_driver(struct typec_altmode_driver *drv);
 	module_driver(__typec_altmode_driver, typec_altmode_register_driver, \
 		      typec_altmode_unregister_driver)
 
+void typec_mode_selection_altmode_complete(struct typec_altmode *alt,
+				const int result);
+void typec_mode_selection_usb4_complete(struct typec_partner *partner,
+				const int result);
+
 #endif /* __USB_TYPEC_ALTMODE_H */
-- 
2.50.1.565.gc32cd1483b-goog


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

* [PATCH v3 06/10] usb: typec: Report altmode entry status via callback
  2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (4 preceding siblings ...)
  2025-08-04  9:03 ` [PATCH v3 05/10] usb: typec: Implement automated mode selection Andrei Kuchynski
@ 2025-08-04  9:03 ` Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 07/10] usb: typec: ucsi: displayport: Propagate DP altmode entry result Andrei Kuchynski
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, 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 | 17 +++++++++++++++--
 drivers/usb/typec/altmodes/thunderbolt.c |  8 ++++++++
 include/linux/usb/typec_tbt.h            |  3 +++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 1dcb77faf85d..cac78d995047 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_mode_selection_altmode_complete(dp->alt, ret);
+		}
 		break;
 	case DP_STATE_CONFIGURE:
 		ret = dp_altmode_configure_vdm(dp, dp->data.conf);
-		if (ret)
+		if (ret) {
 			dev_err(&dp->alt->dev,
 				"unable to send Configure command (%d)\n", ret);
+			typec_mode_selection_altmode_complete(dp->alt, ret);
+		}
 		break;
 	case DP_STATE_CONFIGURE_PRIME:
 		ret = dp_altmode_configure_vdm_cable(dp, dp->data_prime.conf);
@@ -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,12 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
 		switch (cmd) {
 		case DP_CMD_STATUS_UPDATE:
 			dp->state = DP_STATE_EXIT;
+			entry_result = *(int *)vdo;
 			break;
 		case DP_CMD_CONFIGURE:
 			dp->data.conf = 0;
 			ret = dp_altmode_configured(dp);
+			entry_result = *(int *)vdo;
 			break;
 		default:
 			break;
@@ -432,6 +439,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_mode_selection_altmode_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..bbba3c6bc8b8 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,10 @@ static int tbt_altmode_vdm(struct typec_altmode *alt,
 		switch (cmd) {
 		case CMD_ENTER_MODE:
 			dev_warn(&alt->dev, "Enter Mode refused\n");
+			entry_result = *(int *)vdo;
+			break;
+		case TBT_CMD_STATUS_UPDATE:
+			entry_result = *(int *)vdo;
 			break;
 		default:
 			break;
@@ -262,6 +267,9 @@ static int tbt_altmode_vdm(struct typec_altmode *alt,
 
 	mutex_unlock(&tbt->lock);
 
+	if (entry_result || cmd == TBT_CMD_STATUS_UPDATE)
+		typec_mode_selection_altmode_complete(alt, entry_result);
+
 	return 0;
 }
 
diff --git a/include/linux/usb/typec_tbt.h b/include/linux/usb/typec_tbt.h
index 55dcea12082c..57cbda5292bb 100644
--- a/include/linux/usb/typec_tbt.h
+++ b/include/linux/usb/typec_tbt.h
@@ -24,6 +24,9 @@ struct typec_thunderbolt_data {
 	u32 enter_vdo;
 };
 
+/* TBT3 alt mode specific commands */
+#define TBT_CMD_STATUS_UPDATE		VDO_CMD_VENDOR(0)
+
 /* TBT3 Device Discover Mode VDO bits */
 #define TBT_MODE			BIT(0)
 #define TBT_ADAPTER(_vdo_)		FIELD_GET(BIT(16), _vdo_)
-- 
2.50.1.565.gc32cd1483b-goog


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

* [PATCH v3 07/10] usb: typec: ucsi: displayport: Propagate DP altmode entry result
  2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (5 preceding siblings ...)
  2025-08-04  9:03 ` [PATCH v3 06/10] usb: typec: Report altmode entry status via callback Andrei Kuchynski
@ 2025-08-04  9:03 ` Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 08/10] platform/chrome: cros_ec_typec: Propagate " Andrei Kuchynski
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, 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.50.1.565.gc32cd1483b-goog


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

* [PATCH v3 08/10] platform/chrome: cros_ec_typec: Propagate altmode entry result
  2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (6 preceding siblings ...)
  2025-08-04  9:03 ` [PATCH v3 07/10] usb: typec: ucsi: displayport: Propagate DP altmode entry result Andrei Kuchynski
@ 2025-08-04  9:03 ` Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 09/10] platform/chrome: cros_ec_typec: Report USB4 entry status via callback Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 10/10] platform/chrome: cros_ec_typec: Add default_usb_mode_set support Andrei Kuchynski
  9 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, 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 ++++
 include/linux/usb/typec_dp.h                 |  2 ++
 4 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 99f549263c37..73aa25433a50 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..7ee295cf0c02 100644
--- a/drivers/platform/chrome/cros_typec_altmode.c
+++ b/drivers/platform/chrome/cros_typec_altmode.c
@@ -28,6 +28,7 @@ struct cros_typec_altmode_data {
 
 	u16 sid;
 	u8 mode;
+	int error;
 };
 
 struct cros_typec_dp_data {
@@ -295,9 +296,16 @@ int cros_typec_displayport_status_update(struct typec_altmode *altmode,
 
 	dp_data->data = *data;
 	dp_data->pending_status_update = false;
-	adata->header |= VDO_CMDT(CMDT_RSP_ACK);
-	adata->vdo_data = &dp_data->data.status;
-	adata->vdo_size = 2;
+	if (data->error) {
+		adata->header |= VDO_CMDT(CMDT_RSP_NAK);
+		adata->error = dp_data->data.error;
+		adata->vdo_data = &adata->error;
+		adata->vdo_size = 1;
+	} else {
+		adata->header |= VDO_CMDT(CMDT_RSP_ACK);
+		adata->vdo_data = &dp_data->data.status;
+		adata->vdo_size = 2;
+	}
 	schedule_work(&adata->work);
 
 	mutex_unlock(&adata->lock);
@@ -370,4 +378,22 @@ cros_typec_register_thunderbolt(struct cros_typec_port *port,
 
 	return alt;
 }
+
+int cros_typec_tbt_status_update(struct typec_altmode *alt, int error)
+{
+	struct cros_typec_altmode_data *adata = typec_altmode_get_drvdata(alt);
+
+	mutex_lock(&adata->lock);
+
+	adata->header = VDO(adata->sid, 1, SVDM_VER_2_0, TBT_CMD_STATUS_UPDATE);
+	adata->header |= VDO_CMDT(error ? CMDT_RSP_NAK : CMDT_RSP_ACK);
+	adata->error = error;
+	adata->vdo_data = &adata->error;
+	adata->vdo_size = 1;
+	schedule_work(&adata->work);
+
+	mutex_unlock(&adata->lock);
+
+	return 0;
+}
 #endif
diff --git a/drivers/platform/chrome/cros_typec_altmode.h b/drivers/platform/chrome/cros_typec_altmode.h
index 3f2aa95d065a..848a2b194b34 100644
--- a/drivers/platform/chrome/cros_typec_altmode.h
+++ b/drivers/platform/chrome/cros_typec_altmode.h
@@ -39,6 +39,7 @@ static inline int cros_typec_displayport_status_update(struct typec_altmode *alt
 struct typec_altmode *
 cros_typec_register_thunderbolt(struct cros_typec_port *port,
 				struct typec_altmode_desc *desc);
+int cros_typec_tbt_status_update(struct typec_altmode *alt, int error);
 #else
 static inline struct typec_altmode *
 cros_typec_register_thunderbolt(struct cros_typec_port *port,
@@ -46,6 +47,11 @@ cros_typec_register_thunderbolt(struct cros_typec_port *port,
 {
 	return typec_port_register_altmode(port->port, desc);
 }
+static inline int cros_typec_tbt_status_update(struct typec_altmode *alt,
+					int error)
+{
+	return 0;
+}
 #endif
 
 #endif /* __CROS_TYPEC_ALTMODE_H__ */
diff --git a/include/linux/usb/typec_dp.h b/include/linux/usb/typec_dp.h
index 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 {
-- 
2.50.1.565.gc32cd1483b-goog


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

* [PATCH v3 09/10] platform/chrome: cros_ec_typec: Report USB4 entry status via callback
  2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (7 preceding siblings ...)
  2025-08-04  9:03 ` [PATCH v3 08/10] platform/chrome: cros_ec_typec: Propagate " Andrei Kuchynski
@ 2025-08-04  9:03 ` Andrei Kuchynski
  2025-08-04  9:03 ` [PATCH v3 10/10] platform/chrome: cros_ec_typec: Add default_usb_mode_set support Andrei Kuchynski
  9 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, linux-kernel,
	Andrei Kuchynski

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

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

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 73aa25433a50..5a3d393c26ee 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -781,6 +781,7 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
 
 	if (port->mux_flags & USB_PD_MUX_USB4_ENABLED) {
 		ret = cros_typec_enable_usb4(typec, port_num, pd_ctrl);
+		typec_mode_selection_usb4_complete(port->partner, ret);
 	} else if (port->mux_flags & USB_PD_MUX_TBT_COMPAT_ENABLED) {
 		ret = cros_typec_enable_tbt(typec, port_num, pd_ctrl);
 		cros_typec_tbt_status_update(
-- 
2.50.1.565.gc32cd1483b-goog


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

* [PATCH v3 10/10] platform/chrome: cros_ec_typec: Add default_usb_mode_set support
  2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
                   ` (8 preceding siblings ...)
  2025-08-04  9:03 ` [PATCH v3 09/10] platform/chrome: cros_ec_typec: Report USB4 entry status via callback Andrei Kuchynski
@ 2025-08-04  9:03 ` Andrei Kuchynski
  9 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-04  9:03 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, Dmitry Baryshkov,
	Christian A. Ehrhardt, Venkat Jayaraman, linux-kernel,
	Andrei Kuchynski

The `cros_ec_typec` driver currently doesn't directly consume a default
USB mode value. This commit adds the `default_usb_mode_set` function,
enabling the `usb_capability` sysfs attribute to be writable.
This functionality allows users to dynamically activate or deactivate
USB4 mode on a given port.

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

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 5a3d393c26ee..9bfe78e315fc 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -44,6 +44,11 @@ static void cros_typec_role_switch_quirk(struct fwnode_handle *fwnode)
 #endif
 }
 
+static int cros_typec_default_usb_mode_set(struct typec_port *port, enum usb_mode mode)
+{
+	return 0;
+}
+
 static int cros_typec_enter_usb_mode(struct typec_port *tc_port, enum usb_mode mode)
 {
 	struct cros_typec_port *port = typec_get_drvdata(tc_port);
@@ -140,6 +145,7 @@ static int cros_typec_pr_swap(struct typec_port *port, enum typec_role role)
 }
 
 static const struct typec_operations cros_typec_usb_mode_ops = {
+	.default_usb_mode_set = cros_typec_default_usb_mode_set,
 	.enter_usb_mode = cros_typec_enter_usb_mode,
 	.dr_set = cros_typec_dr_swap,
 	.pr_set = cros_typec_pr_swap,
-- 
2.50.1.565.gc32cd1483b-goog


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

* Re: [PATCH v3 05/10] usb: typec: Implement automated mode selection
  2025-08-04  9:03 ` [PATCH v3 05/10] usb: typec: Implement automated mode selection Andrei Kuchynski
@ 2025-08-04 13:00   ` kernel test robot
  2025-08-11 15:31   ` Heikki Krogerus
  1 sibling, 0 replies; 18+ messages in thread
From: kernel test robot @ 2025-08-04 13:00 UTC (permalink / raw)
  To: Andrei Kuchynski, Heikki Krogerus, Abhishek Pandit-Subedi,
	Benson Leung, Jameson Thies, Tzung-Bi Shih, linux-usb,
	chrome-platform
  Cc: oe-kbuild-all, Guenter Roeck, Greg Kroah-Hartman,
	Dmitry Baryshkov, Christian A. Ehrhardt, Venkat Jayaraman,
	linux-kernel, Andrei Kuchynski

Hi Andrei,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus chrome-platform/for-next chrome-platform/for-firmware-next linus/master v6.16 next-20250804]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andrei-Kuchynski/usb-typec-Add-alt_mode_override-field-to-port-property/20250804-170745
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20250804090340.3062182-6-akuchynski%40chromium.org
patch subject: [PATCH v3 05/10] usb: typec: Implement automated mode selection
config: i386-buildonly-randconfig-002-20250804 (https://download.01.org/0day-ci/archive/20250804/202508042044.JDdEBQcS-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250804/202508042044.JDdEBQcS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508042044.JDdEBQcS-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/usb/typec/mode_selection.c:49 Enum value 'MS_STATE_MAX' not described in enum 'ms_state'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs
  2025-08-04  9:03 ` [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs Andrei Kuchynski
@ 2025-08-11 14:25   ` Heikki Krogerus
  2025-08-11 18:22     ` Abhishek Pandit-Subedi
  0 siblings, 1 reply; 18+ messages in thread
From: Heikki Krogerus @ 2025-08-11 14:25 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, Dmitry Baryshkov, Christian A. Ehrhardt,
	Venkat Jayaraman, linux-kernel

Hi Andrei,

On Mon, Aug 04, 2025 at 09:03:33AM +0000, Andrei Kuchynski wrote:
> This patch introduces new sysfs attributes to allow users to configure
> and view Type-C mode priorities.
> 
> `priority`, `usb4_priority` attributes allow users to assign a numeric
> priority to DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> 
> `mode_priorities` - read-only attribute that displays an ordered list
> of all modes based on their configured priorities.
> 
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> ---
>  Documentation/ABI/testing/sysfs-class-typec |  33 +++++
>  drivers/usb/typec/Makefile                  |   2 +-
>  drivers/usb/typec/class.c                   | 103 +++++++++++++++-
>  drivers/usb/typec/class.h                   |   1 +
>  drivers/usb/typec/mode_selection.c          | 130 ++++++++++++++++++++
>  drivers/usb/typec/mode_selection.h          |  23 ++++
>  include/linux/usb/typec_altmode.h           |   7 ++
>  7 files changed, 295 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/usb/typec/mode_selection.c
>  create mode 100644 drivers/usb/typec/mode_selection.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index 38e101c17a00..575dd94f33ab 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -162,6 +162,39 @@ Description:	Lists the supported USB Modes. The default USB mode that is used
>  		- usb3 (USB 3.2)
>  		- usb4 (USB4)
>  
> +		What:		/sys/class/typec/<port>/<alt-mode>/priority
> +Date:		July 2025
> +Contact:	Andrei Kuchynski <akuchynski@chromium.org>
> +Description:
> +		Displays and allows setting the priority for a specific alt-mode.
> +		When read, it shows the current integer priority value. Lower numerical
> +		values indicate higher priority (0 is the highest priority).
> +		If the new value is already in use by another mode, the priority of the
> +		conflicting mode and any subsequent modes will be incremented until they
> +		are all unique.
> +		This attribute is visible only if the kernel supports mode selection.
> +
> +		What:		/sys/class/typec/<port>/usb4_priority
> +Date:		July 2025
> +Contact:	Andrei Kuchynski <akuchynski@chromium.org>
> +Description:
> +		Displays and allows setting the priority for USB4 mode. Its behavior and
> +		priority numbering scheme are identical to the general alt-mode
> +		"priority" attributes.

I'm not sure those above two file make any sense.

> +What:		/sys/class/typec/<port>/mode_priorities
> +Date:		July 2025
> +Contact:	Andrei Kuchynski <akuchynski@chromium.org>
> +Description:	This read-only file lists the modes supported by the port,
> +		ordered by their activation priority. It reflects the preferred sequence
> +		the kernel will attempt to activate modes (DisplayPort alt-mode,
> +		Thunderbolt alt-mode, USB4 mode).
> +		This attribute is visible only if the kernel supports mode selection.
> +
> +		Example values:
> +		- "USB4 Thunderbolt3 DisplayPort"
> +		- "DisplayPort": the port only supports Displayport alt-mode

Why not just use this one instead so that you write the highest
priority mode to it?

>  USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
>  
>  What:		/sys/class/typec/<port>-partner/accessory_mode
> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> index 7a368fea61bc..8a6a1c663eb6 100644
> --- a/drivers/usb/typec/Makefile
> +++ b/drivers/usb/typec/Makefile
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_TYPEC)		+= typec.o
> -typec-y				:= class.o mux.o bus.o pd.o retimer.o
> +typec-y				:= class.o mux.o bus.o pd.o retimer.o mode_selection.o
>  typec-$(CONFIG_ACPI)		+= port-mapper.o
>  obj-$(CONFIG_TYPEC)		+= altmodes/
>  obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index a72325ff099a..414d94c45ab9 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -19,6 +19,7 @@
>  #include "bus.h"
>  #include "class.h"
>  #include "pd.h"
> +#include "mode_selection.h"
>  
>  static DEFINE_IDA(typec_index_ida);
>  
> @@ -445,11 +446,45 @@ svid_show(struct device *dev, struct device_attribute *attr, char *buf)
>  }
>  static DEVICE_ATTR_RO(svid);
>  
> +static ssize_t priority_store(struct device *dev,
> +			       struct device_attribute *attr,
> +			       const char *buf, size_t size)
> +{
> +	struct typec_altmode *adev = to_typec_altmode(dev);
> +	unsigned int val;
> +	int err = kstrtouint(buf, 10, &val);
> +
> +	if (!err) {
> +		err = typec_mode_set_priority(to_typec_port(adev->dev.parent),
> +			typec_svid_to_altmode(adev->svid), val);
> +		if (!err)
> +			return size;
> +	}
> +
> +	return err;
> +}
> +
> +static ssize_t priority_show(struct device *dev,
> +			      struct device_attribute *attr, char *buf)
> +{
> +	struct typec_altmode *adev = to_typec_altmode(dev);
> +	int val;
> +	const int err = typec_mode_get_priority(to_typec_port(adev->dev.parent),
> +			typec_svid_to_altmode(adev->svid), &val);
> +
> +	if (err)
> +		return err;
> +
> +	return sprintf(buf, "%d\n", val);
> +}
> +static DEVICE_ATTR_RW(priority);
> +
>  static struct attribute *typec_altmode_attrs[] = {
>  	&dev_attr_active.attr,
>  	&dev_attr_mode.attr,
>  	&dev_attr_svid.attr,
>  	&dev_attr_vdo.attr,
> +	&dev_attr_priority.attr,
>  	NULL
>  };
>  
> @@ -458,7 +493,7 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
>  {
>  	struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
>  
> -	if (attr == &dev_attr_active.attr)
> +	if (attr == &dev_attr_active.attr) {
>  		if (!is_typec_port(adev->dev.parent)) {
>  			struct typec_partner *partner =
>  				to_typec_partner(adev->dev.parent);
> @@ -469,6 +504,15 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
>  				!adev->ops->activate)
>  				return 0444;
>  		}
> +	} else if (attr == &dev_attr_priority.attr) {
> +		if (is_typec_port(adev->dev.parent))  {
> +			struct typec_port *port = to_typec_port(adev->dev.parent);
> +
> +			if (!port->alt_mode_override)
> +				return 0;
> +		} else
> +			return 0;
> +	}
>  
>  	return attr->mode;
>  }
> @@ -1942,6 +1986,44 @@ static ssize_t orientation_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(orientation);
>  
> +static ssize_t mode_priorities_show(struct device *dev,
> +			      struct device_attribute *attr, char *buf)
> +{
> +	return typec_mode_get_priority_list(to_typec_port(dev), buf);
> +}
> +static DEVICE_ATTR_RO(mode_priorities);
> +
> +static ssize_t usb4_priority_show(struct device *dev,
> +			      struct device_attribute *attr, char *buf)
> +{
> +	struct typec_port *port = to_typec_port(dev);
> +	int val;
> +	const int err = typec_mode_get_priority(port, TYPEC_USB4_MODE, &val);
> +
> +	if (err)
> +		return err;
> +
> +	return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t usb4_priority_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t size)
> +{
> +	struct typec_port *port = to_typec_port(dev);
> +	unsigned int val;
> +	int err = kstrtouint(buf, 10, &val);
> +
> +	if (!err) {
> +		err = typec_mode_set_priority(port, TYPEC_USB4_MODE, val);
> +		if (!err)
> +			return size;
> +	}
> +
> +	return err;
> +}
> +static DEVICE_ATTR_RW(usb4_priority);
> +
>  static struct attribute *typec_attrs[] = {
>  	&dev_attr_data_role.attr,
>  	&dev_attr_power_operation_mode.attr,
> @@ -1954,6 +2036,8 @@ static struct attribute *typec_attrs[] = {
>  	&dev_attr_port_type.attr,
>  	&dev_attr_orientation.attr,
>  	&dev_attr_usb_capability.attr,
> +	&dev_attr_mode_priorities.attr,
> +	&dev_attr_usb4_priority.attr,
>  	NULL,
>  };
>  
> @@ -1992,6 +2076,13 @@ static umode_t typec_attr_is_visible(struct kobject *kobj,
>  			return 0;
>  		if (!port->ops || !port->ops->default_usb_mode_set)
>  			return 0444;
> +	} else if (attr == &dev_attr_mode_priorities.attr) {
> +		if (!port->alt_mode_override)
> +			return 0;

I think the mode order could be visible even when it's read only.

> +	} else if (attr == &dev_attr_usb4_priority.attr) {
> +		if (!port->alt_mode_override ||
> +			!(port->cap->usb_capability & USB_CAPABILITY_USB4))
> +			return 0;
>  	}
>  
>  	return attr->mode;
> @@ -2029,6 +2120,7 @@ static void typec_release(struct device *dev)
>  	typec_mux_put(port->mux);
>  	typec_retimer_put(port->retimer);
>  	kfree(port->cap);
> +	typec_mode_selection_destroy(port);
>  	kfree(port);
>  }
>  
> @@ -2496,6 +2588,8 @@ typec_port_register_altmode(struct typec_port *port,
>  		to_altmode(adev)->retimer = retimer;
>  	}
>  
> +	typec_mode_set_priority(port, typec_svid_to_altmode(adev->svid), -1);
> +
>  	return adev;
>  }
>  EXPORT_SYMBOL_GPL(typec_port_register_altmode);
> @@ -2645,9 +2739,12 @@ struct typec_port *typec_register_port(struct device *parent,
>  	port->con.attach = typec_partner_attach;
>  	port->con.deattach = typec_partner_deattach;
>  
> -	if (cap->usb_capability & USB_CAPABILITY_USB4)
> +	typec_mode_selection_init(port);
> +
> +	if (cap->usb_capability & USB_CAPABILITY_USB4) {
>  		port->usb_mode = USB_MODE_USB4;
> -	else if (cap->usb_capability & USB_CAPABILITY_USB3)
> +		typec_mode_set_priority(port, TYPEC_USB4_MODE, -1);
> +	} else if (cap->usb_capability & USB_CAPABILITY_USB3)
>  		port->usb_mode = USB_MODE_USB3;
>  	else if (cap->usb_capability & USB_CAPABILITY_USB2)
>  		port->usb_mode = USB_MODE_USB2;
> diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> index f05d9201c233..c6467e576569 100644
> --- a/drivers/usb/typec/class.h
> +++ b/drivers/usb/typec/class.h
> @@ -82,6 +82,7 @@ struct typec_port {
>  	struct device			*usb3_dev;
>  
>  	bool				alt_mode_override;
> +	struct list_head		mode_list;
>  };
>  
>  #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> diff --git a/drivers/usb/typec/mode_selection.c b/drivers/usb/typec/mode_selection.c
> new file mode 100644
> index 000000000000..9a7185c07d0c
> --- /dev/null
> +++ b/drivers/usb/typec/mode_selection.c
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2025 Google LLC.
> + */
> +
> +#include <linux/usb/typec_altmode.h>
> +#include <linux/slab.h>
> +#include <linux/list.h>
> +#include "mode_selection.h"
> +#include "class.h"
> +
> +static const char * const mode_names[TYPEC_MODE_MAX] = {
> +	[TYPEC_DP_ALTMODE] = "DisplayPort",
> +	[TYPEC_TBT_ALTMODE] = "Thunderbolt3",
> +	[TYPEC_USB4_MODE] = "USB4",
> +};
> +
> +static const int default_priorities[TYPEC_MODE_MAX] = {
> +	[TYPEC_DP_ALTMODE] = 2,
> +	[TYPEC_TBT_ALTMODE] = 1,
> +	[TYPEC_USB4_MODE] = 0,
> +};
> +
> +/**
> + * struct mode_selection_state - State tracking for a specific Type-C mode
> + * @mode: The type of mode this instance represents
> + * @name: Name string pointer
> + * @priority: The mode priority. Higher values indicate a more preferred mode.
> + * @list: List head to link this mode state into a prioritized list.
> + */
> +struct mode_selection_state {
> +	enum typec_mode_type mode;
> +	const char *name;
> +	int priority;
> +	struct list_head list;
> +};

The name member looks unnecessary, but maybe you use it out side of
this file in the following patches.

> +/* -------------------------------------------------------------------------- */
> +/* port 'mode_priorities' attribute */
> +void typec_mode_selection_init(struct typec_port *port)
> +{
> +	INIT_LIST_HEAD(&port->mode_list);
> +}

Useless function.

> +void typec_mode_selection_destroy(struct typec_port *port)
> +{
> +	struct mode_selection_state *ms, *tmp;
> +
> +	list_for_each_entry_safe(ms, tmp, &port->mode_list, list) {
> +		list_del(&ms->list);
> +		kfree(ms);
> +	}
> +}
> +
> +int typec_mode_set_priority(struct typec_port *port,
> +		const enum typec_mode_type mode, const int priority)
> +{
> +	struct mode_selection_state *ms_target = NULL;
> +	struct mode_selection_state *ms, *tmp;
> +
> +	if (mode >= TYPEC_MODE_MAX || !mode_names[mode])
> +		return -EOPNOTSUPP;
> +
> +	list_for_each_entry_safe(ms, tmp, &port->mode_list, list) {
> +		if (ms->mode == mode) {
> +			ms_target = ms;
> +			list_del(&ms->list);
> +			break;
> +		}
> +	}
> +
> +	if (!ms_target) {
> +		ms_target = kzalloc(sizeof(struct mode_selection_state), GFP_KERNEL);
> +		if (!ms_target)
> +			return -ENOMEM;
> +		ms_target->mode = mode;
> +		ms_target->name = mode_names[mode];
> +		INIT_LIST_HEAD(&ms_target->list);
> +	}
> +
> +	if (priority >= 0)
> +		ms_target->priority = priority;
> +	else
> +		ms_target->priority = default_priorities[mode];
> +
> +	while (ms_target) {
> +		struct mode_selection_state *ms_peer = NULL;
> +
> +		list_for_each_entry(ms, &port->mode_list, list)
> +			if (ms->priority >= ms_target->priority) {
> +				if (ms->priority == ms_target->priority)
> +					ms_peer = ms;
> +				break;
> +			}
> +
> +		list_add_tail(&ms_target->list, &ms->list);
> +		ms_target = ms_peer;
> +		if (ms_target) {
> +			ms_target->priority++;
> +			list_del(&ms_target->list);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +int typec_mode_get_priority(struct typec_port *port,
> +		const enum typec_mode_type mode, int *priority)
> +{
> +	struct mode_selection_state *ms;
> +
> +	list_for_each_entry(ms, &port->mode_list, list)
> +		if (ms->mode == mode) {
> +			*priority = ms->priority;
> +			return 0;
> +		}
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +ssize_t typec_mode_get_priority_list(struct typec_port *port, char *buf)
> +{
> +	struct mode_selection_state *ms;
> +	ssize_t count = 0;
> +
> +	list_for_each_entry(ms, &port->mode_list, list)
> +		count += sysfs_emit_at(buf, count, "%s ", ms->name);
> +
> +	return count + sysfs_emit_at(buf, count, "\n");
> +}
> diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
> new file mode 100644
> index 000000000000..151f0f8b6632
> --- /dev/null
> +++ b/drivers/usb/typec/mode_selection.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <linux/usb/typec_dp.h>
> +#include <linux/usb/typec_tbt.h>
> +
> +static inline enum typec_mode_type typec_svid_to_altmode(const u16 svid)
> +{
> +	switch (svid) {
> +	case USB_TYPEC_DP_SID:
> +		return TYPEC_DP_ALTMODE;
> +	case USB_TYPEC_TBT_SID:
> +		return TYPEC_TBT_ALTMODE;
> +	}
> +	return TYPEC_MODE_MAX;
> +}
> +
> +void typec_mode_selection_init(struct typec_port *port);
> +void typec_mode_selection_destroy(struct typec_port *port);
> +int typec_mode_set_priority(struct typec_port *port,
> +		const enum typec_mode_type mode, const int priority);
> +int typec_mode_get_priority(struct typec_port *port,
> +		const enum typec_mode_type mode, int *priority);
> +ssize_t typec_mode_get_priority_list(struct typec_port *port, char *buf);
> diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> index b3c0866ea70f..5d14363e02eb 100644
> --- a/include/linux/usb/typec_altmode.h
> +++ b/include/linux/usb/typec_altmode.h
> @@ -145,6 +145,13 @@ enum {
>  
>  #define TYPEC_MODAL_STATE(_state_)	((_state_) + TYPEC_STATE_MODAL)
>  
> +enum typec_mode_type {
> +	TYPEC_DP_ALTMODE = 0,
> +	TYPEC_TBT_ALTMODE,
> +	TYPEC_USB4_MODE,
> +	TYPEC_MODE_MAX,
> +};
> +
>  struct typec_altmode *typec_altmode_get_plug(struct typec_altmode *altmode,
>  					     enum typec_plug_index index);
>  void typec_altmode_put_plug(struct typec_altmode *plug);

It looks like this patch would allow the user space to write the mode
priority order without it taking effect. You need to re-organise this
series.

Please introduce the kernel APIs first followed by the user space ABI
changes. That should also make these a bit easier to review.

thanks,

-- 
heikki

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

* Re: [PATCH v3 05/10] usb: typec: Implement automated mode selection
  2025-08-04  9:03 ` [PATCH v3 05/10] usb: typec: Implement automated mode selection Andrei Kuchynski
  2025-08-04 13:00   ` kernel test robot
@ 2025-08-11 15:31   ` Heikki Krogerus
  1 sibling, 0 replies; 18+ messages in thread
From: Heikki Krogerus @ 2025-08-11 15:31 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, Dmitry Baryshkov, Christian A. Ehrhardt,
	Venkat Jayaraman, linux-kernel

Hi Andrei,

On Mon, Aug 04, 2025 at 09:03:34AM +0000, Andrei Kuchynski wrote:
> This patch introduces new sysfs attributes to enable user control over
> Type-C automated mode selection and provide negotiation feedback.
> 
> `mode_selection` attribute shows a prioritized list of supported modes
> with the currently entered mode bracketed. Writing boolean 0 or 1 to
> this attribute starts or stops the mode selection process,
> respectively.
> 
> `entry_result`, `usb4_entry_result` read-only attributes show the
> result of the last mode selection attempt for a specific mode.
> 
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> ---
>  Documentation/ABI/testing/sysfs-class-typec |  39 ++
>  drivers/usb/typec/class.c                   |  95 ++++-
>  drivers/usb/typec/class.h                   |  12 +
>  drivers/usb/typec/mode_selection.c          | 445 ++++++++++++++++++++
>  drivers/usb/typec/mode_selection.h          |  31 ++
>  include/linux/usb/pd_vdo.h                  |   2 +
>  include/linux/usb/typec_altmode.h           |   5 +
>  7 files changed, 626 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index 575dd94f33ab..ed89b9880085 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -280,6 +280,45 @@ 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:		July 2025
> +Contact:	Andrei Kuchynski <akuchynski@chromium.org>
> +Description:	Displays a prioritized list of modes that both the port and the
> +		partner support with the currently entered mode bracketed. Parentheses
> +		indicates a mode currently in progress. Modes listed before the active
> +		or in-progress mode have failed.
> +		Automated 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.
> +		This attribute is only present if the kernel supports AP driven mode
> +		entry, where the Application Processor manages USB Type-C alt-modes.
> +
> +		Example values:
> +		- "USB4 (TBT) DP": USB4 mode entry failed, Thunderbolt alt-mode is in
> +			progress, DisplayPort alt-mode is next.
> +		- "[USB4] TBT DP": USB4 mode is currently active.

There seems to be at least two or three different functions for this
one file (listing of the modes, showing the state and enabling the
"automated mode selection"), so it's probable not going to work like
that.

I'm actually not completely sure from that what do you mean by
"automated mode selection", but is the idea that the "automated mode
selection" is newer enabled by default?

Perhaps the "automated mode selection" enabling should be handled with
its own file that that you can write and that also returns 0 or 1.

> +What:		/sys/class/typec/<port>-partner/<alt-mode>/entry_result
> +Date:		July 2025
> +Contact:	Andrei Kuchynski <akuchynski@chromium.org>
> +Description:	This read-only file represents the status for a specific
> +		alt-mode after the last mode selection process.
> +		This attribute is visible only if the kernel supports mode selection.
> +
> +		Example values:
> +		- "none": No mode selection attempt has occurred for this alt-mode.
> +		- "in progress": The mode entry process is currently underway.
> +		- "active": The alt-mode is currently active.
> +		- "cable failed": The connected cable doesn't support the mode.
> +		- "timeout": Mode entry failed due to a timeout.
> +		- "failed": The attempt to activate the mode failed.

That looks like just debugging information. Where are those states
coming from - they are not defined in any public specification, or are
they?

I'm not sure if you can have a sysfs file for that, but maybe it would
still be okay to inform the user space about a state like that with an
uevent (or maybe not)? Somebody else needs to comment on this.

        char *envp[2] = { };
        ...
        envp[0] = kasprintf(GFP_KERNEL, "STATE=%s", entry_state);
        ...
        kobject_uevent_env(kobj, KOBJ_CHANGE, envp);
        kfree(envp[0]);

I think this patch needs to be split into two or more patches in any
case, but I would suggest that you propose these sysfs files only
after we the mode priority order for the port figured out and
accepted.

thanks,

-- 
heikki

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

* Re: [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs
  2025-08-11 14:25   ` Heikki Krogerus
@ 2025-08-11 18:22     ` Abhishek Pandit-Subedi
  2025-08-12 11:40       ` Heikki Krogerus
  0 siblings, 1 reply; 18+ messages in thread
From: Abhishek Pandit-Subedi @ 2025-08-11 18:22 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,
	Dmitry Baryshkov, Christian A. Ehrhardt, Venkat Jayaraman,
	linux-kernel

On Mon, Aug 11, 2025 at 7:25 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi Andrei,
>
> On Mon, Aug 04, 2025 at 09:03:33AM +0000, Andrei Kuchynski wrote:
> > This patch introduces new sysfs attributes to allow users to configure
> > and view Type-C mode priorities.
> >
> > `priority`, `usb4_priority` attributes allow users to assign a numeric
> > priority to DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> >
> > `mode_priorities` - read-only attribute that displays an ordered list
> > of all modes based on their configured priorities.
> >
> > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > ---
> >  Documentation/ABI/testing/sysfs-class-typec |  33 +++++
> >  drivers/usb/typec/Makefile                  |   2 +-
> >  drivers/usb/typec/class.c                   | 103 +++++++++++++++-
> >  drivers/usb/typec/class.h                   |   1 +
> >  drivers/usb/typec/mode_selection.c          | 130 ++++++++++++++++++++
> >  drivers/usb/typec/mode_selection.h          |  23 ++++
> >  include/linux/usb/typec_altmode.h           |   7 ++
> >  7 files changed, 295 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/usb/typec/mode_selection.c
> >  create mode 100644 drivers/usb/typec/mode_selection.h
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index 38e101c17a00..575dd94f33ab 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -162,6 +162,39 @@ Description:     Lists the supported USB Modes. The default USB mode that is used
> >               - usb3 (USB 3.2)
> >               - usb4 (USB4)
> >
> > +             What:           /sys/class/typec/<port>/<alt-mode>/priority
> > +Date:                July 2025
> > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > +Description:
> > +             Displays and allows setting the priority for a specific alt-mode.
> > +             When read, it shows the current integer priority value. Lower numerical
> > +             values indicate higher priority (0 is the highest priority).
> > +             If the new value is already in use by another mode, the priority of the
> > +             conflicting mode and any subsequent modes will be incremented until they
> > +             are all unique.
> > +             This attribute is visible only if the kernel supports mode selection.
> > +
> > +             What:           /sys/class/typec/<port>/usb4_priority
> > +Date:                July 2025
> > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > +Description:
> > +             Displays and allows setting the priority for USB4 mode. Its behavior and
> > +             priority numbering scheme are identical to the general alt-mode
> > +             "priority" attributes.
>
> I'm not sure those above two file make any sense.
>
> > +What:                /sys/class/typec/<port>/mode_priorities
> > +Date:                July 2025
> > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > +Description: This read-only file lists the modes supported by the port,
> > +             ordered by their activation priority. It reflects the preferred sequence
> > +             the kernel will attempt to activate modes (DisplayPort alt-mode,
> > +             Thunderbolt alt-mode, USB4 mode).
> > +             This attribute is visible only if the kernel supports mode selection.
> > +
> > +             Example values:
> > +             - "USB4 Thunderbolt3 DisplayPort"
> > +             - "DisplayPort": the port only supports Displayport alt-mode
>
> Why not just use this one instead so that you write the highest
> priority mode to it?

Feedback from Greg on
https://lore.kernel.org/linux-usb/2025070159-judgingly-baggage-042a@gregkh/:

"quote":
Multiple value sysfs files are generally frowned apon.  sysfs files that
also have to be manually parsed in the kernel are also frowned apon.
Are you _SURE_ there is no other way that you could possibly do this?

The reason we originally suggested a single "mode priorities" was
because we weren't sure what to do about USB4. Otherwise, it makes
sense to push a priority field to each alt_mode sysfs group and keep
it internally ordered. This is where I really wish we just treated
USB4 as an alternate mode :)

As such, our current API recommendation looks like the following:

* On each port, we lay out priorities for all supported alternate modes + USB4.
* We expose a file to trigger the mode selection task. Reading from it
gives you the current status of mode selection (single value).
* Detailed results from mode entry are pushed to the mode sysfs group
(via entry_results). Converting these to UEVENT is fine but a more
persistent value in debugfs would be useful for debugging.

>
> >  USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
> >
> >  What:                /sys/class/typec/<port>-partner/accessory_mode
> > diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> > index 7a368fea61bc..8a6a1c663eb6 100644
> > --- a/drivers/usb/typec/Makefile
> > +++ b/drivers/usb/typec/Makefile
> > @@ -1,6 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  obj-$(CONFIG_TYPEC)          += typec.o
> > -typec-y                              := class.o mux.o bus.o pd.o retimer.o
> > +typec-y                              := class.o mux.o bus.o pd.o retimer.o mode_selection.o
> >  typec-$(CONFIG_ACPI)         += port-mapper.o
> >  obj-$(CONFIG_TYPEC)          += altmodes/
> >  obj-$(CONFIG_TYPEC_TCPM)     += tcpm/
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index a72325ff099a..414d94c45ab9 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -19,6 +19,7 @@
> >  #include "bus.h"
> >  #include "class.h"
> >  #include "pd.h"
> > +#include "mode_selection.h"
> >
> >  static DEFINE_IDA(typec_index_ida);
> >
> > @@ -445,11 +446,45 @@ svid_show(struct device *dev, struct device_attribute *attr, char *buf)
> >  }
> >  static DEVICE_ATTR_RO(svid);
> >
> > +static ssize_t priority_store(struct device *dev,
> > +                            struct device_attribute *attr,
> > +                            const char *buf, size_t size)
> > +{
> > +     struct typec_altmode *adev = to_typec_altmode(dev);
> > +     unsigned int val;
> > +     int err = kstrtouint(buf, 10, &val);
> > +
> > +     if (!err) {
> > +             err = typec_mode_set_priority(to_typec_port(adev->dev.parent),
> > +                     typec_svid_to_altmode(adev->svid), val);
> > +             if (!err)
> > +                     return size;
> > +     }
> > +
> > +     return err;
> > +}
> > +
> > +static ssize_t priority_show(struct device *dev,
> > +                           struct device_attribute *attr, char *buf)
> > +{
> > +     struct typec_altmode *adev = to_typec_altmode(dev);
> > +     int val;
> > +     const int err = typec_mode_get_priority(to_typec_port(adev->dev.parent),
> > +                     typec_svid_to_altmode(adev->svid), &val);
> > +
> > +     if (err)
> > +             return err;
> > +
> > +     return sprintf(buf, "%d\n", val);
> > +}
> > +static DEVICE_ATTR_RW(priority);
> > +
> >  static struct attribute *typec_altmode_attrs[] = {
> >       &dev_attr_active.attr,
> >       &dev_attr_mode.attr,
> >       &dev_attr_svid.attr,
> >       &dev_attr_vdo.attr,
> > +     &dev_attr_priority.attr,
> >       NULL
> >  };
> >
> > @@ -458,7 +493,7 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
> >  {
> >       struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
> >
> > -     if (attr == &dev_attr_active.attr)
> > +     if (attr == &dev_attr_active.attr) {
> >               if (!is_typec_port(adev->dev.parent)) {
> >                       struct typec_partner *partner =
> >                               to_typec_partner(adev->dev.parent);
> > @@ -469,6 +504,15 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
> >                               !adev->ops->activate)
> >                               return 0444;
> >               }
> > +     } else if (attr == &dev_attr_priority.attr) {
> > +             if (is_typec_port(adev->dev.parent))  {
> > +                     struct typec_port *port = to_typec_port(adev->dev.parent);
> > +
> > +                     if (!port->alt_mode_override)
> > +                             return 0;
> > +             } else
> > +                     return 0;
> > +     }
> >
> >       return attr->mode;
> >  }
> > @@ -1942,6 +1986,44 @@ static ssize_t orientation_show(struct device *dev,
> >  }
> >  static DEVICE_ATTR_RO(orientation);
> >
> > +static ssize_t mode_priorities_show(struct device *dev,
> > +                           struct device_attribute *attr, char *buf)
> > +{
> > +     return typec_mode_get_priority_list(to_typec_port(dev), buf);
> > +}
> > +static DEVICE_ATTR_RO(mode_priorities);
> > +
> > +static ssize_t usb4_priority_show(struct device *dev,
> > +                           struct device_attribute *attr, char *buf)
> > +{
> > +     struct typec_port *port = to_typec_port(dev);
> > +     int val;
> > +     const int err = typec_mode_get_priority(port, TYPEC_USB4_MODE, &val);
> > +
> > +     if (err)
> > +             return err;
> > +
> > +     return sprintf(buf, "%d\n", val);
> > +}
> > +
> > +static ssize_t usb4_priority_store(struct device *dev,
> > +                               struct device_attribute *attr,
> > +                               const char *buf, size_t size)
> > +{
> > +     struct typec_port *port = to_typec_port(dev);
> > +     unsigned int val;
> > +     int err = kstrtouint(buf, 10, &val);
> > +
> > +     if (!err) {
> > +             err = typec_mode_set_priority(port, TYPEC_USB4_MODE, val);
> > +             if (!err)
> > +                     return size;
> > +     }
> > +
> > +     return err;
> > +}
> > +static DEVICE_ATTR_RW(usb4_priority);
> > +
> >  static struct attribute *typec_attrs[] = {
> >       &dev_attr_data_role.attr,
> >       &dev_attr_power_operation_mode.attr,
> > @@ -1954,6 +2036,8 @@ static struct attribute *typec_attrs[] = {
> >       &dev_attr_port_type.attr,
> >       &dev_attr_orientation.attr,
> >       &dev_attr_usb_capability.attr,
> > +     &dev_attr_mode_priorities.attr,
> > +     &dev_attr_usb4_priority.attr,
> >       NULL,
> >  };
> >
> > @@ -1992,6 +2076,13 @@ static umode_t typec_attr_is_visible(struct kobject *kobj,
> >                       return 0;
> >               if (!port->ops || !port->ops->default_usb_mode_set)
> >                       return 0444;
> > +     } else if (attr == &dev_attr_mode_priorities.attr) {
> > +             if (!port->alt_mode_override)
> > +                     return 0;
>
> I think the mode order could be visible even when it's read only.
>
> > +     } else if (attr == &dev_attr_usb4_priority.attr) {
> > +             if (!port->alt_mode_override ||
> > +                     !(port->cap->usb_capability & USB_CAPABILITY_USB4))
> > +                     return 0;
> >       }
> >
> >       return attr->mode;
> > @@ -2029,6 +2120,7 @@ static void typec_release(struct device *dev)
> >       typec_mux_put(port->mux);
> >       typec_retimer_put(port->retimer);
> >       kfree(port->cap);
> > +     typec_mode_selection_destroy(port);
> >       kfree(port);
> >  }
> >
> > @@ -2496,6 +2588,8 @@ typec_port_register_altmode(struct typec_port *port,
> >               to_altmode(adev)->retimer = retimer;
> >       }
> >
> > +     typec_mode_set_priority(port, typec_svid_to_altmode(adev->svid), -1);
> > +
> >       return adev;
> >  }
> >  EXPORT_SYMBOL_GPL(typec_port_register_altmode);
> > @@ -2645,9 +2739,12 @@ struct typec_port *typec_register_port(struct device *parent,
> >       port->con.attach = typec_partner_attach;
> >       port->con.deattach = typec_partner_deattach;
> >
> > -     if (cap->usb_capability & USB_CAPABILITY_USB4)
> > +     typec_mode_selection_init(port);
> > +
> > +     if (cap->usb_capability & USB_CAPABILITY_USB4) {
> >               port->usb_mode = USB_MODE_USB4;
> > -     else if (cap->usb_capability & USB_CAPABILITY_USB3)
> > +             typec_mode_set_priority(port, TYPEC_USB4_MODE, -1);
> > +     } else if (cap->usb_capability & USB_CAPABILITY_USB3)
> >               port->usb_mode = USB_MODE_USB3;
> >       else if (cap->usb_capability & USB_CAPABILITY_USB2)
> >               port->usb_mode = USB_MODE_USB2;
> > diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> > index f05d9201c233..c6467e576569 100644
> > --- a/drivers/usb/typec/class.h
> > +++ b/drivers/usb/typec/class.h
> > @@ -82,6 +82,7 @@ struct typec_port {
> >       struct device                   *usb3_dev;
> >
> >       bool                            alt_mode_override;
> > +     struct list_head                mode_list;
> >  };
> >
> >  #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> > diff --git a/drivers/usb/typec/mode_selection.c b/drivers/usb/typec/mode_selection.c
> > new file mode 100644
> > index 000000000000..9a7185c07d0c
> > --- /dev/null
> > +++ b/drivers/usb/typec/mode_selection.c
> > @@ -0,0 +1,130 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright 2025 Google LLC.
> > + */
> > +
> > +#include <linux/usb/typec_altmode.h>
> > +#include <linux/slab.h>
> > +#include <linux/list.h>
> > +#include "mode_selection.h"
> > +#include "class.h"
> > +
> > +static const char * const mode_names[TYPEC_MODE_MAX] = {
> > +     [TYPEC_DP_ALTMODE] = "DisplayPort",
> > +     [TYPEC_TBT_ALTMODE] = "Thunderbolt3",
> > +     [TYPEC_USB4_MODE] = "USB4",
> > +};
> > +
> > +static const int default_priorities[TYPEC_MODE_MAX] = {
> > +     [TYPEC_DP_ALTMODE] = 2,
> > +     [TYPEC_TBT_ALTMODE] = 1,
> > +     [TYPEC_USB4_MODE] = 0,
> > +};
> > +
> > +/**
> > + * struct mode_selection_state - State tracking for a specific Type-C mode
> > + * @mode: The type of mode this instance represents
> > + * @name: Name string pointer
> > + * @priority: The mode priority. Higher values indicate a more preferred mode.
> > + * @list: List head to link this mode state into a prioritized list.
> > + */
> > +struct mode_selection_state {
> > +     enum typec_mode_type mode;
> > +     const char *name;
> > +     int priority;
> > +     struct list_head list;
> > +};
>
> The name member looks unnecessary, but maybe you use it out side of
> this file in the following patches.
>
> > +/* -------------------------------------------------------------------------- */
> > +/* port 'mode_priorities' attribute */
> > +void typec_mode_selection_init(struct typec_port *port)
> > +{
> > +     INIT_LIST_HEAD(&port->mode_list);
> > +}
>
> Useless function.
>
> > +void typec_mode_selection_destroy(struct typec_port *port)
> > +{
> > +     struct mode_selection_state *ms, *tmp;
> > +
> > +     list_for_each_entry_safe(ms, tmp, &port->mode_list, list) {
> > +             list_del(&ms->list);
> > +             kfree(ms);
> > +     }
> > +}
> > +
> > +int typec_mode_set_priority(struct typec_port *port,
> > +             const enum typec_mode_type mode, const int priority)
> > +{
> > +     struct mode_selection_state *ms_target = NULL;
> > +     struct mode_selection_state *ms, *tmp;
> > +
> > +     if (mode >= TYPEC_MODE_MAX || !mode_names[mode])
> > +             return -EOPNOTSUPP;
> > +
> > +     list_for_each_entry_safe(ms, tmp, &port->mode_list, list) {
> > +             if (ms->mode == mode) {
> > +                     ms_target = ms;
> > +                     list_del(&ms->list);
> > +                     break;
> > +             }
> > +     }
> > +
> > +     if (!ms_target) {
> > +             ms_target = kzalloc(sizeof(struct mode_selection_state), GFP_KERNEL);
> > +             if (!ms_target)
> > +                     return -ENOMEM;
> > +             ms_target->mode = mode;
> > +             ms_target->name = mode_names[mode];
> > +             INIT_LIST_HEAD(&ms_target->list);
> > +     }
> > +
> > +     if (priority >= 0)
> > +             ms_target->priority = priority;
> > +     else
> > +             ms_target->priority = default_priorities[mode];
> > +
> > +     while (ms_target) {
> > +             struct mode_selection_state *ms_peer = NULL;
> > +
> > +             list_for_each_entry(ms, &port->mode_list, list)
> > +                     if (ms->priority >= ms_target->priority) {
> > +                             if (ms->priority == ms_target->priority)
> > +                                     ms_peer = ms;
> > +                             break;
> > +                     }
> > +
> > +             list_add_tail(&ms_target->list, &ms->list);
> > +             ms_target = ms_peer;
> > +             if (ms_target) {
> > +                     ms_target->priority++;
> > +                     list_del(&ms_target->list);
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +int typec_mode_get_priority(struct typec_port *port,
> > +             const enum typec_mode_type mode, int *priority)
> > +{
> > +     struct mode_selection_state *ms;
> > +
> > +     list_for_each_entry(ms, &port->mode_list, list)
> > +             if (ms->mode == mode) {
> > +                     *priority = ms->priority;
> > +                     return 0;
> > +             }
> > +
> > +     return -EOPNOTSUPP;
> > +}
> > +
> > +ssize_t typec_mode_get_priority_list(struct typec_port *port, char *buf)
> > +{
> > +     struct mode_selection_state *ms;
> > +     ssize_t count = 0;
> > +
> > +     list_for_each_entry(ms, &port->mode_list, list)
> > +             count += sysfs_emit_at(buf, count, "%s ", ms->name);
> > +
> > +     return count + sysfs_emit_at(buf, count, "\n");
> > +}
> > diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
> > new file mode 100644
> > index 000000000000..151f0f8b6632
> > --- /dev/null
> > +++ b/drivers/usb/typec/mode_selection.h
> > @@ -0,0 +1,23 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#include <linux/usb/typec_dp.h>
> > +#include <linux/usb/typec_tbt.h>
> > +
> > +static inline enum typec_mode_type typec_svid_to_altmode(const u16 svid)
> > +{
> > +     switch (svid) {
> > +     case USB_TYPEC_DP_SID:
> > +             return TYPEC_DP_ALTMODE;
> > +     case USB_TYPEC_TBT_SID:
> > +             return TYPEC_TBT_ALTMODE;
> > +     }
> > +     return TYPEC_MODE_MAX;
> > +}
> > +
> > +void typec_mode_selection_init(struct typec_port *port);
> > +void typec_mode_selection_destroy(struct typec_port *port);
> > +int typec_mode_set_priority(struct typec_port *port,
> > +             const enum typec_mode_type mode, const int priority);
> > +int typec_mode_get_priority(struct typec_port *port,
> > +             const enum typec_mode_type mode, int *priority);
> > +ssize_t typec_mode_get_priority_list(struct typec_port *port, char *buf);
> > diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> > index b3c0866ea70f..5d14363e02eb 100644
> > --- a/include/linux/usb/typec_altmode.h
> > +++ b/include/linux/usb/typec_altmode.h
> > @@ -145,6 +145,13 @@ enum {
> >
> >  #define TYPEC_MODAL_STATE(_state_)   ((_state_) + TYPEC_STATE_MODAL)
> >
> > +enum typec_mode_type {
> > +     TYPEC_DP_ALTMODE = 0,
> > +     TYPEC_TBT_ALTMODE,
> > +     TYPEC_USB4_MODE,
> > +     TYPEC_MODE_MAX,
> > +};
> > +
> >  struct typec_altmode *typec_altmode_get_plug(struct typec_altmode *altmode,
> >                                            enum typec_plug_index index);
> >  void typec_altmode_put_plug(struct typec_altmode *plug);
>
> It looks like this patch would allow the user space to write the mode
> priority order without it taking effect. You need to re-organise this
> series.
>
> Please introduce the kernel APIs first followed by the user space ABI
> changes. That should also make these a bit easier to review.
>
> thanks,
>
> --
> heikki

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

* Re: [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs
  2025-08-11 18:22     ` Abhishek Pandit-Subedi
@ 2025-08-12 11:40       ` Heikki Krogerus
  2025-08-12 20:33         ` Abhishek Pandit-Subedi
  0 siblings, 1 reply; 18+ messages in thread
From: Heikki Krogerus @ 2025-08-12 11:40 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,
	Dmitry Baryshkov, Christian A. Ehrhardt, Venkat Jayaraman,
	linux-kernel

Hi Abhishek,

On Mon, Aug 11, 2025 at 11:22:38AM -0700, Abhishek Pandit-Subedi wrote:
> On Mon, Aug 11, 2025 at 7:25 AM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > Hi Andrei,
> >
> > On Mon, Aug 04, 2025 at 09:03:33AM +0000, Andrei Kuchynski wrote:
> > > This patch introduces new sysfs attributes to allow users to configure
> > > and view Type-C mode priorities.
> > >
> > > `priority`, `usb4_priority` attributes allow users to assign a numeric
> > > priority to DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> > >
> > > `mode_priorities` - read-only attribute that displays an ordered list
> > > of all modes based on their configured priorities.
> > >
> > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > ---
> > >  Documentation/ABI/testing/sysfs-class-typec |  33 +++++
> > >  drivers/usb/typec/Makefile                  |   2 +-
> > >  drivers/usb/typec/class.c                   | 103 +++++++++++++++-
> > >  drivers/usb/typec/class.h                   |   1 +
> > >  drivers/usb/typec/mode_selection.c          | 130 ++++++++++++++++++++
> > >  drivers/usb/typec/mode_selection.h          |  23 ++++
> > >  include/linux/usb/typec_altmode.h           |   7 ++
> > >  7 files changed, 295 insertions(+), 4 deletions(-)
> > >  create mode 100644 drivers/usb/typec/mode_selection.c
> > >  create mode 100644 drivers/usb/typec/mode_selection.h
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > > index 38e101c17a00..575dd94f33ab 100644
> > > --- a/Documentation/ABI/testing/sysfs-class-typec
> > > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > > @@ -162,6 +162,39 @@ Description:     Lists the supported USB Modes. The default USB mode that is used
> > >               - usb3 (USB 3.2)
> > >               - usb4 (USB4)
> > >
> > > +             What:           /sys/class/typec/<port>/<alt-mode>/priority
> > > +Date:                July 2025
> > > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > > +Description:
> > > +             Displays and allows setting the priority for a specific alt-mode.
> > > +             When read, it shows the current integer priority value. Lower numerical
> > > +             values indicate higher priority (0 is the highest priority).
> > > +             If the new value is already in use by another mode, the priority of the
> > > +             conflicting mode and any subsequent modes will be incremented until they
> > > +             are all unique.
> > > +             This attribute is visible only if the kernel supports mode selection.
> > > +
> > > +             What:           /sys/class/typec/<port>/usb4_priority
> > > +Date:                July 2025
> > > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > > +Description:
> > > +             Displays and allows setting the priority for USB4 mode. Its behavior and
> > > +             priority numbering scheme are identical to the general alt-mode
> > > +             "priority" attributes.
> >
> > I'm not sure those above two file make any sense.
> >
> > > +What:                /sys/class/typec/<port>/mode_priorities
> > > +Date:                July 2025
> > > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > > +Description: This read-only file lists the modes supported by the port,
> > > +             ordered by their activation priority. It reflects the preferred sequence
> > > +             the kernel will attempt to activate modes (DisplayPort alt-mode,
> > > +             Thunderbolt alt-mode, USB4 mode).
> > > +             This attribute is visible only if the kernel supports mode selection.
> > > +
> > > +             Example values:
> > > +             - "USB4 Thunderbolt3 DisplayPort"
> > > +             - "DisplayPort": the port only supports Displayport alt-mode
> >
> > Why not just use this one instead so that you write the highest
> > priority mode to it?
> 
> Feedback from Greg on
> https://lore.kernel.org/linux-usb/2025070159-judgingly-baggage-042a@gregkh/:
> 
> "quote":
> Multiple value sysfs files are generally frowned apon.  sysfs files that
> also have to be manually parsed in the kernel are also frowned apon.
> Are you _SURE_ there is no other way that you could possibly do this?
> 
> The reason we originally suggested a single "mode priorities" was
> because we weren't sure what to do about USB4. Otherwise, it makes
> sense to push a priority field to each alt_mode sysfs group and keep
> it internally ordered. This is where I really wish we just treated
> USB4 as an alternate mode :)

I'm probable forgetting something, but I'm pretty sure I already told
you guys that I agree, it should be an alt mode. So why not just
register a special alt mode for it?

Sorry if I missed something.

> As such, our current API recommendation looks like the following:
> 
> * On each port, we lay out priorities for all supported alternate modes + USB4.

This first part I understand.

> * We expose a file to trigger the mode selection task. Reading from it
> gives you the current status of mode selection (single value).
> * Detailed results from mode entry are pushed to the mode sysfs group
> (via entry_results). Converting these to UEVENT is fine but a more
> persistent value in debugfs would be useful for debugging.

This second part I would really like to handle separately, after we
have a solution for the first part.

thanks,

-- 
heikki

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

* Re: [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs
  2025-08-12 11:40       ` Heikki Krogerus
@ 2025-08-12 20:33         ` Abhishek Pandit-Subedi
  2025-08-14  8:22           ` Andrei Kuchynski
  0 siblings, 1 reply; 18+ messages in thread
From: Abhishek Pandit-Subedi @ 2025-08-12 20:33 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,
	Dmitry Baryshkov, Christian A. Ehrhardt, Venkat Jayaraman,
	linux-kernel

On Tue, Aug 12, 2025 at 4:41 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi Abhishek,
>
> On Mon, Aug 11, 2025 at 11:22:38AM -0700, Abhishek Pandit-Subedi wrote:
> > On Mon, Aug 11, 2025 at 7:25 AM Heikki Krogerus
> > <heikki.krogerus@linux.intel.com> wrote:
> > >
> > > Hi Andrei,
> > >
> > > On Mon, Aug 04, 2025 at 09:03:33AM +0000, Andrei Kuchynski wrote:
> > > > This patch introduces new sysfs attributes to allow users to configure
> > > > and view Type-C mode priorities.
> > > >
> > > > `priority`, `usb4_priority` attributes allow users to assign a numeric
> > > > priority to DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> > > >
> > > > `mode_priorities` - read-only attribute that displays an ordered list
> > > > of all modes based on their configured priorities.
> > > >
> > > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > > ---
> > > >  Documentation/ABI/testing/sysfs-class-typec |  33 +++++
> > > >  drivers/usb/typec/Makefile                  |   2 +-
> > > >  drivers/usb/typec/class.c                   | 103 +++++++++++++++-
> > > >  drivers/usb/typec/class.h                   |   1 +
> > > >  drivers/usb/typec/mode_selection.c          | 130 ++++++++++++++++++++
> > > >  drivers/usb/typec/mode_selection.h          |  23 ++++
> > > >  include/linux/usb/typec_altmode.h           |   7 ++
> > > >  7 files changed, 295 insertions(+), 4 deletions(-)
> > > >  create mode 100644 drivers/usb/typec/mode_selection.c
> > > >  create mode 100644 drivers/usb/typec/mode_selection.h
> > > >
> > > > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > > > index 38e101c17a00..575dd94f33ab 100644
> > > > --- a/Documentation/ABI/testing/sysfs-class-typec
> > > > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > > > @@ -162,6 +162,39 @@ Description:     Lists the supported USB Modes. The default USB mode that is used
> > > >               - usb3 (USB 3.2)
> > > >               - usb4 (USB4)
> > > >
> > > > +             What:           /sys/class/typec/<port>/<alt-mode>/priority
> > > > +Date:                July 2025
> > > > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > > > +Description:
> > > > +             Displays and allows setting the priority for a specific alt-mode.
> > > > +             When read, it shows the current integer priority value. Lower numerical
> > > > +             values indicate higher priority (0 is the highest priority).
> > > > +             If the new value is already in use by another mode, the priority of the
> > > > +             conflicting mode and any subsequent modes will be incremented until they
> > > > +             are all unique.
> > > > +             This attribute is visible only if the kernel supports mode selection.
> > > > +
> > > > +             What:           /sys/class/typec/<port>/usb4_priority
> > > > +Date:                July 2025
> > > > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > > > +Description:
> > > > +             Displays and allows setting the priority for USB4 mode. Its behavior and
> > > > +             priority numbering scheme are identical to the general alt-mode
> > > > +             "priority" attributes.
> > >
> > > I'm not sure those above two file make any sense.
> > >
> > > > +What:                /sys/class/typec/<port>/mode_priorities
> > > > +Date:                July 2025
> > > > +Contact:     Andrei Kuchynski <akuchynski@chromium.org>
> > > > +Description: This read-only file lists the modes supported by the port,
> > > > +             ordered by their activation priority. It reflects the preferred sequence
> > > > +             the kernel will attempt to activate modes (DisplayPort alt-mode,
> > > > +             Thunderbolt alt-mode, USB4 mode).
> > > > +             This attribute is visible only if the kernel supports mode selection.
> > > > +
> > > > +             Example values:
> > > > +             - "USB4 Thunderbolt3 DisplayPort"
> > > > +             - "DisplayPort": the port only supports Displayport alt-mode
> > >
> > > Why not just use this one instead so that you write the highest
> > > priority mode to it?
> >
> > Feedback from Greg on
> > https://lore.kernel.org/linux-usb/2025070159-judgingly-baggage-042a@gregkh/:
> >
> > "quote":
> > Multiple value sysfs files are generally frowned apon.  sysfs files that
> > also have to be manually parsed in the kernel are also frowned apon.
> > Are you _SURE_ there is no other way that you could possibly do this?
> >
> > The reason we originally suggested a single "mode priorities" was
> > because we weren't sure what to do about USB4. Otherwise, it makes
> > sense to push a priority field to each alt_mode sysfs group and keep
> > it internally ordered. This is where I really wish we just treated
> > USB4 as an alternate mode :)
>
> I'm probable forgetting something, but I'm pretty sure I already told
> you guys that I agree, it should be an alt mode. So why not just
> register a special alt mode for it?

We interpreted this a bit differently (as just rename it):
https://patchwork.kernel.org/project/linux-usb/patch/20250616133147.1835939-5-akuchynski@chromium.org/#26431992

Thanks for the clarification here. In that case, we'll get rid of
`usb_priorities` and `usb_results` and just add a new alternate mode
for USB4. The vendor ids list on usb.org
(https://www.usb.org/sites/default/files/vendor_ids072325_1.pdf) shows
0xff00 for USB4 so that's what we'll use. So the attributes should be:
.active (similar to other modes), .mode = 1 (unused really), .svid =
0xff00, .vdo = <usb eudo> (if supported).

>
> Sorry if I missed something.
>
> > As such, our current API recommendation looks like the following:
> >
> > * On each port, we lay out priorities for all supported alternate modes + USB4.
>
> This first part I understand.
>
> > * We expose a file to trigger the mode selection task. Reading from it
> > gives you the current status of mode selection (single value).
> > * Detailed results from mode entry are pushed to the mode sysfs group
> > (via entry_results). Converting these to UEVENT is fine but a more
> > persistent value in debugfs would be useful for debugging.
>
> This second part I would really like to handle separately, after we
> have a solution for the first part.

Ack. We'll reduce the series so it's easier to review for mode_priorities first.

>
> thanks,
>
> --
> heikki

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

* Re: [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs
  2025-08-12 20:33         ` Abhishek Pandit-Subedi
@ 2025-08-14  8:22           ` Andrei Kuchynski
  0 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-08-14  8:22 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi, Heikki Krogerus
  Cc: Benson Leung, Jameson Thies, Tzung-Bi Shih, linux-usb,
	chrome-platform, Guenter Roeck, Greg Kroah-Hartman,
	Dmitry Baryshkov, Christian A. Ehrhardt, Venkat Jayaraman,
	linux-kernel

On Tue, Aug 12, 2025 at 10:34 PM Abhishek Pandit-Subedi
<abhishekpandit@chromium.org> wrote:
>
>
> We interpreted this a bit differently (as just rename it):
> https://patchwork.kernel.org/project/linux-usb/patch/20250616133147.1835939-5-akuchynski@chromium.org/#26431992
>
> Thanks for the clarification here. In that case, we'll get rid of
> `usb_priorities` and `usb_results` and just add a new alternate mode
> for USB4. The vendor ids list on usb.org
> (https://www.usb.org/sites/default/files/vendor_ids072325_1.pdf) shows
> 0xff00 for USB4 so that's what we'll use. So the attributes should be:
> .active (similar to other modes), .mode = 1 (unused really), .svid =
> 0xff00, .vdo = <usb eudo> (if supported).
>
> >
> > > As such, our current API recommendation looks like the following:
> > >
> > > * On each port, we lay out priorities for all supported alternate modes + USB4.
> >
> > This first part I understand.
> >
> > > * We expose a file to trigger the mode selection task. Reading from it
> > > gives you the current status of mode selection (single value).
> > > * Detailed results from mode entry are pushed to the mode sysfs group
> > > (via entry_results). Converting these to UEVENT is fine but a more
> > > persistent value in debugfs would be useful for debugging.
> >
> > This second part I would really like to handle separately, after we
> > have a solution for the first part.
>
> Ack. We'll reduce the series so it's easier to review for mode_priorities first.
>

Hi Heikki, Abhishek,

Thank you for your review. I have addressed the feedback and plan to
resubmit the series.

Here are the changes I will make:
- The `typec_mode_selection_init` function and the `name` member
from the mode_selection_state struct will be removed.
- Patch 4 will be split into API and ABI parts.
- The entire series will be divided into `mode priority` (patches 1-4)
and then `mode selection`.
- The mode selection logic will be standardized to function
identically for all modes, including USB4.

Thanks again for your guidance.

Andrei

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

end of thread, other threads:[~2025-08-14  8:28 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-04  9:03 [PATCH v3 00/10] USB Type-C mode selection Andrei Kuchynski
2025-08-04  9:03 ` [PATCH v3 01/10] usb: typec: Add alt_mode_override field to port property Andrei Kuchynski
2025-08-04  9:03 ` [PATCH v3 02/10] platform/chrome: cros_ec_typec: Set alt_mode_override flag Andrei Kuchynski
2025-08-04  9:03 ` [PATCH v3 03/10] usb: typec: ucsi: " Andrei Kuchynski
2025-08-04  9:03 ` [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs Andrei Kuchynski
2025-08-11 14:25   ` Heikki Krogerus
2025-08-11 18:22     ` Abhishek Pandit-Subedi
2025-08-12 11:40       ` Heikki Krogerus
2025-08-12 20:33         ` Abhishek Pandit-Subedi
2025-08-14  8:22           ` Andrei Kuchynski
2025-08-04  9:03 ` [PATCH v3 05/10] usb: typec: Implement automated mode selection Andrei Kuchynski
2025-08-04 13:00   ` kernel test robot
2025-08-11 15:31   ` Heikki Krogerus
2025-08-04  9:03 ` [PATCH v3 06/10] usb: typec: Report altmode entry status via callback Andrei Kuchynski
2025-08-04  9:03 ` [PATCH v3 07/10] usb: typec: ucsi: displayport: Propagate DP altmode entry result Andrei Kuchynski
2025-08-04  9:03 ` [PATCH v3 08/10] platform/chrome: cros_ec_typec: Propagate " Andrei Kuchynski
2025-08-04  9:03 ` [PATCH v3 09/10] platform/chrome: cros_ec_typec: Report USB4 entry status via callback Andrei Kuchynski
2025-08-04  9:03 ` [PATCH v3 10/10] platform/chrome: cros_ec_typec: Add default_usb_mode_set support Andrei Kuchynski

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