* [PATCH v2 01/10] usb: typec: Add alt_mode_override field to port property
2025-06-30 14:12 [PATCH v2 00/10] USB Type-C mode selection Andrei Kuchynski
@ 2025-06-30 14:12 ` Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 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-06-30 14:12 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, 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.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 02/10] platform/chrome: cros_ec_typec: Set alt_mode_override flag
2025-06-30 14:12 [PATCH v2 00/10] USB Type-C mode selection Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 01/10] usb: typec: Add alt_mode_override field to port property Andrei Kuchynski
@ 2025-06-30 14:12 ` Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 03/10] usb: typec: ucsi: " Andrei Kuchynski
` (7 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-06-30 14:12 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, 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 7678e3d05fd3..3aed429fde03 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -408,6 +408,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
cap->driver_data = cros_port;
cap->ops = &cros_typec_usb_mode_ops;
+ cap->alt_mode_override = typec->ap_driven_altmode;
cros_port->port = typec_register_port(dev, cap);
if (IS_ERR(cros_port->port)) {
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 03/10] usb: typec: ucsi: Set alt_mode_override flag
2025-06-30 14:12 [PATCH v2 00/10] USB Type-C mode selection Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 01/10] usb: typec: Add alt_mode_override field to port property Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 02/10] platform/chrome: cros_ec_typec: Set alt_mode_override flag Andrei Kuchynski
@ 2025-06-30 14:12 ` Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 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-06-30 14:12 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, 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 01ce858a1a2b..1372becaec82 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1616,6 +1616,8 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
cap->driver_data = con;
cap->ops = &ucsi_ops;
+ cap->alt_mode_override =
+ !!(con->ucsi->cap.features & UCSI_CAP_ALT_MODE_OVERRIDE);
if (ucsi->ops->update_connector)
ucsi->ops->update_connector(con);
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 04/10] usb: typec: Expose mode priorities via sysfs
2025-06-30 14:12 [PATCH v2 00/10] USB Type-C mode selection Andrei Kuchynski
` (2 preceding siblings ...)
2025-06-30 14:12 ` [PATCH v2 03/10] usb: typec: ucsi: " Andrei Kuchynski
@ 2025-06-30 14:12 ` Andrei Kuchynski
2025-07-01 8:32 ` Greg Kroah-Hartman
2025-06-30 14:12 ` [PATCH v2 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-06-30 14:12 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, linux-kernel, Andrei Kuchynski
This sysfs attribute specifies the preferred order for enabling
DisplayPort, Thunderbolt alternate modes, and USB4 mode.
Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
Documentation/ABI/testing/sysfs-class-typec | 16 +++
drivers/usb/typec/Makefile | 2 +-
drivers/usb/typec/class.c | 28 ++++-
drivers/usb/typec/class.h | 4 +
drivers/usb/typec/mode_selection.c | 116 ++++++++++++++++++++
drivers/usb/typec/mode_selection.h | 19 ++++
include/linux/usb/typec_altmode.h | 7 ++
7 files changed, 190 insertions(+), 2 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..ff3296ee8e1c 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -162,6 +162,22 @@ Description: Lists the supported USB Modes. The default USB mode that is used
- usb3 (USB 3.2)
- usb4 (USB4)
+What: /sys/class/typec/<port>/mode_priorities
+Date: June 2025
+Contact: Andrei Kuchynski <akuchynski@chromium.org>
+Description: Lists the modes supported by the port, ordered by their
+ activation priority. It defines the preferred sequence for activating
+ modes such as Displayport alt-mode, Thunderbolt alt-mode and USB4 mode.
+ The default order can be modified by writing a new sequence to this
+ attribute. Any modes omitted from a user-provided list will be
+ automatically placed at the end of the list.
+
+ Example values:
+ - "USB4 TBT DP": default priority order
+ - "USB4 DP TBT": modified priority order after writing "USB4 DP TBT" or
+ "USB4 DP"
+ - "DP": 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..93eadbcdd4c0 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);
@@ -540,7 +541,7 @@ static void typec_altmode_release(struct device *dev)
}
const struct device_type typec_altmode_dev_type = {
- .name = "typec_alternate_mode",
+ .name = ALTERNATE_MODE_DEVICE_TYPE_NAME,
.groups = typec_altmode_groups,
.release = typec_altmode_release,
};
@@ -1942,6 +1943,25 @@ static ssize_t orientation_show(struct device *dev,
}
static DEVICE_ATTR_RO(orientation);
+static ssize_t mode_priorities_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct typec_port *port = to_typec_port(dev);
+ int ret = typec_mode_priorities_set(port, buf);
+
+ return ret ? : size;
+}
+
+static ssize_t mode_priorities_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct typec_port *port = to_typec_port(dev);
+
+ return typec_mode_priorities_get(port, buf);
+}
+static DEVICE_ATTR_RW(mode_priorities);
+
static struct attribute *typec_attrs[] = {
&dev_attr_data_role.attr,
&dev_attr_power_operation_mode.attr,
@@ -1954,6 +1974,7 @@ static struct attribute *typec_attrs[] = {
&dev_attr_port_type.attr,
&dev_attr_orientation.attr,
&dev_attr_usb_capability.attr,
+ &dev_attr_mode_priorities.attr,
NULL,
};
@@ -1992,6 +2013,9 @@ static umode_t typec_attr_is_visible(struct kobject *kobj,
return 0;
if (!port->ops || !port->ops->default_usb_mode_set)
return 0444;
+ } else if (attr == &dev_attr_mode_priorities.attr) {
+ if (!port->alt_mode_override)
+ return 0;
}
return attr->mode;
@@ -2652,6 +2676,8 @@ struct typec_port *typec_register_port(struct device *parent,
else if (cap->usb_capability & USB_CAPABILITY_USB2)
port->usb_mode = USB_MODE_USB2;
+ typec_mode_priorities_set(port, "");
+
device_initialize(&port->dev);
port->dev.class = &typec_class;
port->dev.parent = parent;
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index f05d9201c233..28b3c19a0632 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -5,6 +5,7 @@
#include <linux/device.h>
#include <linux/usb/typec.h>
+#include <linux/usb/typec_altmode.h>
struct typec_mux;
struct typec_switch;
@@ -82,6 +83,7 @@ struct typec_port {
struct device *usb3_dev;
bool alt_mode_override;
+ int mode_priority_list[TYPEC_MODE_MAX];
};
#define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
@@ -111,4 +113,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
new file mode 100644
index 000000000000..cb7ddf679037
--- /dev/null
+++ b/drivers/usb/typec/mode_selection.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Google LLC.
+ */
+
+#include <linux/usb/typec_altmode.h>
+#include <linux/vmalloc.h>
+#include "mode_selection.h"
+#include "class.h"
+
+static const char * const mode_names[] = {
+ [TYPEC_DP_ALTMODE] = "DP",
+ [TYPEC_TBT_ALTMODE] = "TBT",
+ [TYPEC_USB4_MODE] = "USB4",
+};
+static const char * const default_priorities = "USB4 TBT DP";
+
+/* -------------------------------------------------------------------------- */
+/* port 'mode_priorities' attribute */
+static int typec_mode_parse_priority_string(const char *str, int *list)
+{
+ const bool user_settings = list[0] == TYPEC_MODE_MAX;
+ char *buf, *ptr;
+ char *token;
+ int ret = 0;
+
+ buf = vmalloc(strlen(str) + 1);
+ if (!buf)
+ return -ENOMEM;
+ for (int i = 0; i <= strlen(str); i++)
+ buf[i] = (str[i] == '\n') ? '\0' : str[i];
+ ptr = buf;
+
+ while ((token = strsep(&ptr, " ")) && !ret) {
+ if (strlen(token)) {
+ int mode = 0;
+
+ while ((mode < TYPEC_MODE_MAX) &&
+ strcmp(token, mode_names[mode]))
+ mode++;
+ if (mode == TYPEC_MODE_MAX) {
+ ret = -EINVAL;
+ continue;
+ }
+
+ for (int i = 0; i < TYPEC_MODE_MAX; i++) {
+ if (list[i] == TYPEC_MODE_MAX) {
+ list[i] = mode;
+ break;
+ }
+ if (list[i] == mode) {
+ if (user_settings)
+ ret = -EINVAL;
+ break;
+ }
+ }
+ }
+ }
+ vfree(buf);
+
+ return ret;
+}
+
+int typec_mode_priorities_set(struct typec_port *port,
+ const char *user_priorities)
+{
+ int list[TYPEC_MODE_MAX];
+ int ret;
+
+ for (int i = 0; i < TYPEC_MODE_MAX; i++)
+ list[i] = TYPEC_MODE_MAX;
+
+ ret = typec_mode_parse_priority_string(user_priorities, list);
+ if (!ret)
+ ret = typec_mode_parse_priority_string(default_priorities, list);
+
+ if (!ret)
+ for (int i = 0; i < TYPEC_MODE_MAX; i++)
+ port->mode_priority_list[i] = list[i];
+
+ return ret;
+}
+
+static int port_altmode_supported(struct device *dev, void *data)
+{
+ if (!strcmp(dev->type->name, ALTERNATE_MODE_DEVICE_TYPE_NAME)) {
+ struct typec_altmode *alt = to_typec_altmode(dev);
+
+ if (*(int *)data == typec_svid_to_altmode(alt->svid))
+ return 1;
+ }
+ return 0;
+}
+
+static bool port_mode_supported(struct typec_port *port, int mode)
+{
+ if (mode >= TYPEC_MODE_MAX)
+ return false;
+ if (mode == TYPEC_USB4_MODE)
+ return !!(port->cap->usb_capability & USB_CAPABILITY_USB4);
+ return device_for_each_child(&port->dev, &mode, port_altmode_supported);
+}
+
+int typec_mode_priorities_get(struct typec_port *port, char *buf)
+{
+ ssize_t count = 0;
+
+ for (int i = 0; i < TYPEC_MODE_MAX; i++) {
+ int mode = port->mode_priority_list[i];
+
+ if (port_mode_supported(port, mode))
+ count += sysfs_emit_at(buf, count, "%s ", mode_names[mode]);
+ }
+
+ 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..c595c84e26a4
--- /dev/null
+++ b/drivers/usb/typec/mode_selection.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_tbt.h>
+
+static inline int 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;
+}
+
+int typec_mode_priorities_set(struct typec_port *port,
+ const char *user_priorities);
+int typec_mode_priorities_get(struct typec_port *port, char *buf);
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index b3c0866ea70f..4f05c5f5c91d 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_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.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 04/10] usb: typec: Expose mode priorities via sysfs
2025-06-30 14:12 ` [PATCH v2 04/10] usb: typec: Expose mode priorities via sysfs Andrei Kuchynski
@ 2025-07-01 8:32 ` Greg Kroah-Hartman
2025-07-22 15:07 ` Andrei Kuchynski
0 siblings, 1 reply; 18+ messages in thread
From: Greg Kroah-Hartman @ 2025-07-01 8:32 UTC (permalink / raw)
To: Andrei Kuchynski
Cc: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform,
Guenter Roeck, Dmitry Baryshkov, Christian A. Ehrhardt,
linux-kernel
On Mon, Jun 30, 2025 at 02:12:33PM +0000, Andrei Kuchynski wrote:
> This sysfs attribute specifies the preferred order for enabling
> DisplayPort, Thunderbolt alternate modes, and USB4 mode.
>
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> ---
> Documentation/ABI/testing/sysfs-class-typec | 16 +++
> drivers/usb/typec/Makefile | 2 +-
> drivers/usb/typec/class.c | 28 ++++-
> drivers/usb/typec/class.h | 4 +
> drivers/usb/typec/mode_selection.c | 116 ++++++++++++++++++++
> drivers/usb/typec/mode_selection.h | 19 ++++
> include/linux/usb/typec_altmode.h | 7 ++
> 7 files changed, 190 insertions(+), 2 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..ff3296ee8e1c 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -162,6 +162,22 @@ Description: Lists the supported USB Modes. The default USB mode that is used
> - usb3 (USB 3.2)
> - usb4 (USB4)
>
> +What: /sys/class/typec/<port>/mode_priorities
> +Date: June 2025
> +Contact: Andrei Kuchynski <akuchynski@chromium.org>
> +Description: Lists the modes supported by the port, ordered by their
> + activation priority. It defines the preferred sequence for activating
> + modes such as Displayport alt-mode, Thunderbolt alt-mode and USB4 mode.
> + The default order can be modified by writing a new sequence to this
> + attribute. Any modes omitted from a user-provided list will be
> + automatically placed at the end of the list.
> +
> + Example values:
> + - "USB4 TBT DP": default priority order
> + - "USB4 DP TBT": modified priority order after writing "USB4 DP TBT" or
> + "USB4 DP"
> + - "DP": the port only supports Displayport alt-mode
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?
> +
> 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..93eadbcdd4c0 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);
>
> @@ -540,7 +541,7 @@ static void typec_altmode_release(struct device *dev)
> }
>
> const struct device_type typec_altmode_dev_type = {
> - .name = "typec_alternate_mode",
> + .name = ALTERNATE_MODE_DEVICE_TYPE_NAME,
> .groups = typec_altmode_groups,
> .release = typec_altmode_release,
> };
> @@ -1942,6 +1943,25 @@ static ssize_t orientation_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(orientation);
>
> +static ssize_t mode_priorities_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct typec_port *port = to_typec_port(dev);
> + int ret = typec_mode_priorities_set(port, buf);
You don't pass in size here, what could go wrong...
> +
> + return ret ? : size;
Please do not use ? : unless you have to. Spell it out, it makes code
easier to maintain. Remember, we write code for people first, compilers
second.
> +}
> +
> +static ssize_t mode_priorities_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct typec_port *port = to_typec_port(dev);
> +
> + return typec_mode_priorities_get(port, buf);
> +}
> +static DEVICE_ATTR_RW(mode_priorities);
> +
> static struct attribute *typec_attrs[] = {
> &dev_attr_data_role.attr,
> &dev_attr_power_operation_mode.attr,
> @@ -1954,6 +1974,7 @@ static struct attribute *typec_attrs[] = {
> &dev_attr_port_type.attr,
> &dev_attr_orientation.attr,
> &dev_attr_usb_capability.attr,
> + &dev_attr_mode_priorities.attr,
> NULL,
> };
>
> @@ -1992,6 +2013,9 @@ static umode_t typec_attr_is_visible(struct kobject *kobj,
> return 0;
> if (!port->ops || !port->ops->default_usb_mode_set)
> return 0444;
> + } else if (attr == &dev_attr_mode_priorities.attr) {
> + if (!port->alt_mode_override)
> + return 0;
> }
>
> return attr->mode;
> @@ -2652,6 +2676,8 @@ struct typec_port *typec_register_port(struct device *parent,
> else if (cap->usb_capability & USB_CAPABILITY_USB2)
> port->usb_mode = USB_MODE_USB2;
>
> + typec_mode_priorities_set(port, "");
> +
> device_initialize(&port->dev);
> port->dev.class = &typec_class;
> port->dev.parent = parent;
> diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> index f05d9201c233..28b3c19a0632 100644
> --- a/drivers/usb/typec/class.h
> +++ b/drivers/usb/typec/class.h
> @@ -5,6 +5,7 @@
>
> #include <linux/device.h>
> #include <linux/usb/typec.h>
> +#include <linux/usb/typec_altmode.h>
>
> struct typec_mux;
> struct typec_switch;
> @@ -82,6 +83,7 @@ struct typec_port {
> struct device *usb3_dev;
>
> bool alt_mode_override;
> + int mode_priority_list[TYPEC_MODE_MAX];
> };
>
> #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> @@ -111,4 +113,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
> new file mode 100644
> index 000000000000..cb7ddf679037
> --- /dev/null
> +++ b/drivers/usb/typec/mode_selection.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2025 Google LLC.
> + */
> +
> +#include <linux/usb/typec_altmode.h>
> +#include <linux/vmalloc.h>
> +#include "mode_selection.h"
> +#include "class.h"
> +
> +static const char * const mode_names[] = {
> + [TYPEC_DP_ALTMODE] = "DP",
> + [TYPEC_TBT_ALTMODE] = "TBT",
> + [TYPEC_USB4_MODE] = "USB4",
No TYPEC_MODE_MAX entry? Why not? This is going to get out of sync,
see below for my comment about that.
> +};
> +static const char * const default_priorities = "USB4 TBT DP";
A comment here about what this is for?
> +
> +/* -------------------------------------------------------------------------- */
> +/* port 'mode_priorities' attribute */
> +static int typec_mode_parse_priority_string(const char *str, int *list)
> +{
> + const bool user_settings = list[0] == TYPEC_MODE_MAX;
> + char *buf, *ptr;
> + char *token;
> + int ret = 0;
> +
> + buf = vmalloc(strlen(str) + 1);
Why vmalloc for such a small chunk of memory?
> + if (!buf)
> + return -ENOMEM;
> + for (int i = 0; i <= strlen(str); i++)
> + buf[i] = (str[i] == '\n') ? '\0' : str[i];
Please spell out if statements, especially ones that do assignements in
them. This is going to be a pain to maintain over time, right? Make it
obvious what is happening please.
> + ptr = buf;
> +
> + while ((token = strsep(&ptr, " ")) && !ret) {
> + if (strlen(token)) {
> + int mode = 0;
> +
> + while ((mode < TYPEC_MODE_MAX) &&
> + strcmp(token, mode_names[mode]))
> + mode++;
> + if (mode == TYPEC_MODE_MAX) {
> + ret = -EINVAL;
> + continue;
> + }
> +
> + for (int i = 0; i < TYPEC_MODE_MAX; i++) {
> + if (list[i] == TYPEC_MODE_MAX) {
> + list[i] = mode;
> + break;
> + }
> + if (list[i] == mode) {
> + if (user_settings)
> + ret = -EINVAL;
> + break;
> + }
> + }
> + }
> + }
> + vfree(buf);
Why not just use a free() type model and that way your error paths above
are much simpler?
> +
> + return ret;
> +}
> +
> +int typec_mode_priorities_set(struct typec_port *port,
> + const char *user_priorities)
> +{
> + int list[TYPEC_MODE_MAX];
> + int ret;
> +
> + for (int i = 0; i < TYPEC_MODE_MAX; i++)
> + list[i] = TYPEC_MODE_MAX;
> +
> + ret = typec_mode_parse_priority_string(user_priorities, list);
> + if (!ret)
> + ret = typec_mode_parse_priority_string(default_priorities, list);
> +
> + if (!ret)
> + for (int i = 0; i < TYPEC_MODE_MAX; i++)
> + port->mode_priority_list[i] = list[i];
> +
> + return ret;
> +}
> +
> +static int port_altmode_supported(struct device *dev, void *data)
> +{
> + if (!strcmp(dev->type->name, ALTERNATE_MODE_DEVICE_TYPE_NAME)) {
> + struct typec_altmode *alt = to_typec_altmode(dev);
> +
> + if (*(int *)data == typec_svid_to_altmode(alt->svid))
> + return 1;
> + }
> + return 0;
> +}
> +
> +static bool port_mode_supported(struct typec_port *port, int mode)
> +{
> + if (mode >= TYPEC_MODE_MAX)
> + return false;
> + if (mode == TYPEC_USB4_MODE)
> + return !!(port->cap->usb_capability & USB_CAPABILITY_USB4);
> + return device_for_each_child(&port->dev, &mode, port_altmode_supported);
> +}
> +
> +int typec_mode_priorities_get(struct typec_port *port, char *buf)
> +{
> + ssize_t count = 0;
> +
> + for (int i = 0; i < TYPEC_MODE_MAX; i++) {
> + int mode = port->mode_priority_list[i];
> +
> + if (port_mode_supported(port, mode))
> + count += sysfs_emit_at(buf, count, "%s ", mode_names[mode]);
> + }
> +
> + 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..c595c84e26a4
> --- /dev/null
> +++ b/drivers/usb/typec/mode_selection.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <linux/usb/typec_dp.h>
> +#include <linux/usb/typec_tbt.h>
> +
> +static inline int 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;
> +}
> +
> +int typec_mode_priorities_set(struct typec_port *port,
> + const char *user_priorities);
> +int typec_mode_priorities_get(struct typec_port *port, char *buf);
> diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> index b3c0866ea70f..4f05c5f5c91d 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_DP_ALTMODE = 0,
> + TYPEC_TBT_ALTMODE,
> + TYPEC_USB4_MODE,
> + TYPEC_MODE_MAX,
This list is going to get out of order and sync with your string list
elsewhere in the other .c file. What is going to ensure that this does
not happen?
Again, I'm really not happy with this api, it feels fragile and tricky
and will get out of sync very easily over time. We need loads of
justification for why this really is the only possible way this can be
done, and some type of proof that this actually has been tested (and
maybe fuzzed?)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 04/10] usb: typec: Expose mode priorities via sysfs
2025-07-01 8:32 ` Greg Kroah-Hartman
@ 2025-07-22 15:07 ` Andrei Kuchynski
0 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-07-22 15:07 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform,
Guenter Roeck, Dmitry Baryshkov, Christian A. Ehrhardt,
linux-kernel
On Tue, Jul 1, 2025 at 10:32 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jun 30, 2025 at 02:12:33PM +0000, Andrei Kuchynski wrote:
> > This sysfs attribute specifies the preferred order for enabling
> > DisplayPort, Thunderbolt alternate modes, and USB4 mode.
> >
> > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > ---
> > Documentation/ABI/testing/sysfs-class-typec | 16 +++
> > drivers/usb/typec/Makefile | 2 +-
> > drivers/usb/typec/class.c | 28 ++++-
> > drivers/usb/typec/class.h | 4 +
> > drivers/usb/typec/mode_selection.c | 116 ++++++++++++++++++++
> > drivers/usb/typec/mode_selection.h | 19 ++++
> > include/linux/usb/typec_altmode.h | 7 ++
> > 7 files changed, 190 insertions(+), 2 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..ff3296ee8e1c 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -162,6 +162,22 @@ Description: Lists the supported USB Modes. The default USB mode that is used
> > - usb3 (USB 3.2)
> > - usb4 (USB4)
> >
> > +What: /sys/class/typec/<port>/mode_priorities
> > +Date: June 2025
> > +Contact: Andrei Kuchynski <akuchynski@chromium.org>
> > +Description: Lists the modes supported by the port, ordered by their
> > + activation priority. It defines the preferred sequence for activating
> > + modes such as Displayport alt-mode, Thunderbolt alt-mode and USB4 mode.
> > + The default order can be modified by writing a new sequence to this
> > + attribute. Any modes omitted from a user-provided list will be
> > + automatically placed at the end of the list.
> > +
> > + Example values:
> > + - "USB4 TBT DP": default priority order
> > + - "USB4 DP TBT": modified priority order after writing "USB4 DP TBT" or
> > + "USB4 DP"
> > + - "DP": the port only supports Displayport alt-mode
>
> 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 string parser will be removed. See my comment below.
> > +
> > 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..93eadbcdd4c0 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);
> >
> > @@ -540,7 +541,7 @@ static void typec_altmode_release(struct device *dev)
> > }
> >
> > const struct device_type typec_altmode_dev_type = {
> > - .name = "typec_alternate_mode",
> > + .name = ALTERNATE_MODE_DEVICE_TYPE_NAME,
> > .groups = typec_altmode_groups,
> > .release = typec_altmode_release,
> > };
> > @@ -1942,6 +1943,25 @@ static ssize_t orientation_show(struct device *dev,
> > }
> > static DEVICE_ATTR_RO(orientation);
> >
> > +static ssize_t mode_priorities_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t size)
> > +{
> > + struct typec_port *port = to_typec_port(dev);
> > + int ret = typec_mode_priorities_set(port, buf);
>
> You don't pass in size here, what could go wrong...
>
Yes, the buffer size really needs to be checked. Thank you.
> > +
> > + return ret ? : size;
>
> Please do not use ? : unless you have to. Spell it out, it makes code
> easier to maintain. Remember, we write code for people first, compilers
> second.
>
Ternary operators will be changed to if/else.
> > +}
> > +
> > +static ssize_t mode_priorities_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct typec_port *port = to_typec_port(dev);
> > +
> > + return typec_mode_priorities_get(port, buf);
> > +}
> > +static DEVICE_ATTR_RW(mode_priorities);
> > +
> > static struct attribute *typec_attrs[] = {
> > &dev_attr_data_role.attr,
> > &dev_attr_power_operation_mode.attr,
> > @@ -1954,6 +1974,7 @@ static struct attribute *typec_attrs[] = {
> > &dev_attr_port_type.attr,
> > &dev_attr_orientation.attr,
> > &dev_attr_usb_capability.attr,
> > + &dev_attr_mode_priorities.attr,
> > NULL,
> > };
> >
> > @@ -1992,6 +2013,9 @@ static umode_t typec_attr_is_visible(struct kobject *kobj,
> > return 0;
> > if (!port->ops || !port->ops->default_usb_mode_set)
> > return 0444;
> > + } else if (attr == &dev_attr_mode_priorities.attr) {
> > + if (!port->alt_mode_override)
> > + return 0;
> > }
> >
> > return attr->mode;
> > @@ -2652,6 +2676,8 @@ struct typec_port *typec_register_port(struct device *parent,
> > else if (cap->usb_capability & USB_CAPABILITY_USB2)
> > port->usb_mode = USB_MODE_USB2;
> >
> > + typec_mode_priorities_set(port, "");
> > +
> > device_initialize(&port->dev);
> > port->dev.class = &typec_class;
> > port->dev.parent = parent;
> > diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> > index f05d9201c233..28b3c19a0632 100644
> > --- a/drivers/usb/typec/class.h
> > +++ b/drivers/usb/typec/class.h
> > @@ -5,6 +5,7 @@
> >
> > #include <linux/device.h>
> > #include <linux/usb/typec.h>
> > +#include <linux/usb/typec_altmode.h>
> >
> > struct typec_mux;
> > struct typec_switch;
> > @@ -82,6 +83,7 @@ struct typec_port {
> > struct device *usb3_dev;
> >
> > bool alt_mode_override;
> > + int mode_priority_list[TYPEC_MODE_MAX];
> > };
> >
> > #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> > @@ -111,4 +113,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
> > new file mode 100644
> > index 000000000000..cb7ddf679037
> > --- /dev/null
> > +++ b/drivers/usb/typec/mode_selection.c
> > @@ -0,0 +1,116 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright 2025 Google LLC.
> > + */
> > +
> > +#include <linux/usb/typec_altmode.h>
> > +#include <linux/vmalloc.h>
> > +#include "mode_selection.h"
> > +#include "class.h"
> > +
> > +static const char * const mode_names[] = {
> > + [TYPEC_DP_ALTMODE] = "DP",
> > + [TYPEC_TBT_ALTMODE] = "TBT",
> > + [TYPEC_USB4_MODE] = "USB4",
>
> No TYPEC_MODE_MAX entry? Why not? This is going to get out of sync,
> see below for my comment about that.
>
The mode_selection mechanism will strictly support and work only with
modes explicitly described in mode_names array. Elements in this array
will be checked for NULL. Thanks for raising it.
> > +};
> > +static const char * const default_priorities = "USB4 TBT DP";
>
> A comment here about what this is for?
>
Thanks, I’ll add it
> > +
> > +/* -------------------------------------------------------------------------- */
> > +/* port 'mode_priorities' attribute */
> > +static int typec_mode_parse_priority_string(const char *str, int *list)
> > +{
> > + const bool user_settings = list[0] == TYPEC_MODE_MAX;
> > + char *buf, *ptr;
> > + char *token;
> > + int ret = 0;
> > +
> > + buf = vmalloc(strlen(str) + 1);
>
> Why vmalloc for such a small chunk of memory?
>
Good point. kstrndup should be used here.
> > + if (!buf)
> > + return -ENOMEM;
> > + for (int i = 0; i <= strlen(str); i++)
> > + buf[i] = (str[i] == '\n') ? '\0' : str[i];
>
> Please spell out if statements, especially ones that do assignements in
> them. This is going to be a pain to maintain over time, right? Make it
> obvious what is happening please.
>
>
Yes, I see it now. `strreplace` would be helpful here.
> > + ptr = buf;
> > +
> > + while ((token = strsep(&ptr, " ")) && !ret) {
> > + if (strlen(token)) {
> > + int mode = 0;
> > +
> > + while ((mode < TYPEC_MODE_MAX) &&
> > + strcmp(token, mode_names[mode]))
> > + mode++;
> > + if (mode == TYPEC_MODE_MAX) {
> > + ret = -EINVAL;
> > + continue;
> > + }
> > +
> > + for (int i = 0; i < TYPEC_MODE_MAX; i++) {
> > + if (list[i] == TYPEC_MODE_MAX) {
> > + list[i] = mode;
> > + break;
> > + }
> > + if (list[i] == mode) {
> > + if (user_settings)
> > + ret = -EINVAL;
> > + break;
> > + }
> > + }
> > + }
> > + }
> > + vfree(buf);
>
> Why not just use a free() type model and that way your error paths above
> are much simpler?
>
>
Acknowledged.
> > +
> > + return ret;
> > +}
> > +
> > +int typec_mode_priorities_set(struct typec_port *port,
> > + const char *user_priorities)
> > +{
> > + int list[TYPEC_MODE_MAX];
> > + int ret;
> > +
> > + for (int i = 0; i < TYPEC_MODE_MAX; i++)
> > + list[i] = TYPEC_MODE_MAX;
> > +
> > + ret = typec_mode_parse_priority_string(user_priorities, list);
> > + if (!ret)
> > + ret = typec_mode_parse_priority_string(default_priorities, list);
> > +
> > + if (!ret)
> > + for (int i = 0; i < TYPEC_MODE_MAX; i++)
> > + port->mode_priority_list[i] = list[i];
> > +
> > + return ret;
> > +}
> > +
> > +static int port_altmode_supported(struct device *dev, void *data)
> > +{
> > + if (!strcmp(dev->type->name, ALTERNATE_MODE_DEVICE_TYPE_NAME)) {
> > + struct typec_altmode *alt = to_typec_altmode(dev);
> > +
> > + if (*(int *)data == typec_svid_to_altmode(alt->svid))
> > + return 1;
> > + }
> > + return 0;
> > +}
> > +
> > +static bool port_mode_supported(struct typec_port *port, int mode)
> > +{
> > + if (mode >= TYPEC_MODE_MAX)
> > + return false;
> > + if (mode == TYPEC_USB4_MODE)
> > + return !!(port->cap->usb_capability & USB_CAPABILITY_USB4);
> > + return device_for_each_child(&port->dev, &mode, port_altmode_supported);
> > +}
> > +
> > +int typec_mode_priorities_get(struct typec_port *port, char *buf)
> > +{
> > + ssize_t count = 0;
> > +
> > + for (int i = 0; i < TYPEC_MODE_MAX; i++) {
> > + int mode = port->mode_priority_list[i];
> > +
> > + if (port_mode_supported(port, mode))
> > + count += sysfs_emit_at(buf, count, "%s ", mode_names[mode]);
> > + }
> > +
> > + 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..c595c84e26a4
> > --- /dev/null
> > +++ b/drivers/usb/typec/mode_selection.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#include <linux/usb/typec_dp.h>
> > +#include <linux/usb/typec_tbt.h>
> > +
> > +static inline int 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;
> > +}
> > +
> > +int typec_mode_priorities_set(struct typec_port *port,
> > + const char *user_priorities);
> > +int typec_mode_priorities_get(struct typec_port *port, char *buf);
> > diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> > index b3c0866ea70f..4f05c5f5c91d 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_DP_ALTMODE = 0,
> > + TYPEC_TBT_ALTMODE,
> > + TYPEC_USB4_MODE,
> > + TYPEC_MODE_MAX,
>
> This list is going to get out of order and sync with your string list
> elsewhere in the other .c file. What is going to ensure that this does
> not happen?
>
> Again, I'm really not happy with this api, it feels fragile and tricky
> and will get out of sync very easily over time. We need loads of
> justification for why this really is the only possible way this can be
> done, and some type of proof that this actually has been tested (and
> maybe fuzzed?)
>
The current mode priority string parser will be removed in the next
patch series. This will enable individual, per-mode priority
configuration. For the USB4 mode, priority will then be set via a
dedicated usb4_priority sysfs port attribute, or within a new
USB4-specific attribute group.
A read-only mode_priorities attribute will then be used to provide the
resulting, validated priority order to userspace.
Thank you,
Andrei
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 05/10] usb: typec: Implement automated mode selection
2025-06-30 14:12 [PATCH v2 00/10] USB Type-C mode selection Andrei Kuchynski
` (3 preceding siblings ...)
2025-06-30 14:12 ` [PATCH v2 04/10] usb: typec: Expose mode priorities via sysfs Andrei Kuchynski
@ 2025-06-30 14:12 ` Andrei Kuchynski
2025-07-01 3:59 ` kernel test robot
2025-07-01 8:40 ` Greg Kroah-Hartman
2025-06-30 14:12 ` [PATCH v2 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-06-30 14:12 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, linux-kernel, Andrei Kuchynski
This commit introduces mode_selection sysfs attribute to control automated
mode negotiation. Writing "yes" to this file activates the automated
selection process for DisplayPort, Thunderbolt alternate modes, and USB4
mode. Conversely, writing "no" will cancel any ongoing selection process
and exit the currently active mode.
Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
Documentation/ABI/testing/sysfs-class-typec | 17 +
drivers/usb/typec/class.c | 52 ++-
drivers/usb/typec/class.h | 10 +
drivers/usb/typec/mode_selection.c | 413 ++++++++++++++++++++
drivers/usb/typec/mode_selection.h | 30 ++
include/linux/usb/pd_vdo.h | 2 +
include/linux/usb/typec_altmode.h | 5 +
7 files changed, 527 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index ff3296ee8e1c..0ffc71a7c374 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -263,6 +263,23 @@ Description: The USB Modes that the partner device supports. The active mode
- usb3 (USB 3.2)
- usb4 (USB4)
+What: /sys/class/typec/<port>-partner/mode_selection
+Date: June 2025
+Contact: Andrei Kuchynski <akuchynski@chromium.org>
+Description: Lists the partner-supported alternate modes and mode entry
+ results with the currently entered mode bracketed. If a cable doesn't
+ support a mode, it's marked as 'nc'. An ellipsis indicates a mode
+ currently in progress. Automated mode selection is activated by writing
+ "yes" to the file. Conversely, writing "no" will cancel any ongoing
+ selection process and exit the currently active mode, if any.
+
+ Example values:
+ - "DP TBT=... USB4=nc": The cable does not support USB4 mode,
+ The driver is currently attempting to enter Thunderbolt alt-mode.
+ - "[DP] TBT=-EOPNOTSUPP USB4=-ETIME": USB4 mode entry failed due to
+ a timeout, Thunderbolt failed with the result -EOPNOTSUPP,
+ and DisplayPort is the active alt-mode.
+
USB Type-C cable devices (eg. /sys/class/typec/port0-cable/)
Note: Electronically Marked Cables will have a device also for one cable plug
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 93eadbcdd4c0..8455e07a9934 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -741,6 +741,33 @@ static ssize_t number_of_alternate_modes_show(struct device *dev, struct device_
}
static DEVICE_ATTR_RO(number_of_alternate_modes);
+static ssize_t mode_selection_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct typec_partner *partner = to_typec_partner(dev);
+
+ return typec_mode_selection_get(partner, buf);
+}
+
+static ssize_t mode_selection_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct typec_partner *partner = to_typec_partner(dev);
+ bool start;
+ int ret = kstrtobool(buf, &start);
+
+ if (!ret) {
+ if (start)
+ ret = typec_mode_selection_start(partner);
+ else
+ ret = typec_mode_selection_reset(partner);
+ }
+
+ return ret ? : size;
+}
+static DEVICE_ATTR_RW(mode_selection);
+
static struct attribute *typec_partner_attrs[] = {
&dev_attr_accessory_mode.attr,
&dev_attr_supports_usb_power_delivery.attr,
@@ -748,6 +775,7 @@ static struct attribute *typec_partner_attrs[] = {
&dev_attr_type.attr,
&dev_attr_usb_mode.attr,
&dev_attr_usb_power_delivery_revision.attr,
+ &dev_attr_mode_selection.attr,
NULL
};
@@ -772,6 +800,10 @@ static umode_t typec_partner_attr_is_visible(struct kobject *kobj, struct attrib
if (!get_pd_product_type(kobj_to_dev(kobj)))
return 0;
+ if (attr == &dev_attr_mode_selection.attr)
+ if (!port->alt_mode_override)
+ return 0;
+
return attr->mode;
}
@@ -850,8 +882,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]);
}
@@ -971,7 +1005,12 @@ struct typec_altmode *
typec_partner_register_altmode(struct typec_partner *partner,
const struct typec_altmode_desc *desc)
{
- return typec_register_altmode(&partner->dev, desc);
+ struct typec_altmode *alt = typec_register_altmode(&partner->dev, desc);
+
+ if (alt)
+ typec_mode_selection_add_mode(partner, typec_svid_to_altmode(alt->svid));
+
+ return alt;
}
EXPORT_SYMBOL_GPL(typec_partner_register_altmode);
@@ -1075,6 +1114,8 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
typec_partner_link_device(partner, port->usb3_dev);
mutex_unlock(&port->partner_link_lock);
+ typec_mode_selection_create(partner);
+
return partner;
}
EXPORT_SYMBOL_GPL(typec_register_partner);
@@ -1092,6 +1133,7 @@ void typec_unregister_partner(struct typec_partner *partner)
if (IS_ERR_OR_NULL(partner))
return;
+ typec_mode_selection_destroy(partner);
port = to_typec_port(partner->dev.parent);
mutex_lock(&port->partner_link_lock);
@@ -1360,6 +1402,7 @@ int typec_cable_set_identity(struct typec_cable *cable)
}
EXPORT_SYMBOL_GPL(typec_cable_set_identity);
+static struct typec_partner *typec_get_partner(struct typec_port *port);
/**
* typec_register_cable - Register a USB Type-C Cable
* @port: The USB Type-C Port the cable is connected to
@@ -1374,6 +1417,7 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
struct typec_cable_desc *desc)
{
struct typec_cable *cable;
+ struct typec_partner *partner;
int ret;
cable = kzalloc(sizeof(*cable), GFP_KERNEL);
@@ -1405,6 +1449,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 28b3c19a0632..17efaacc2b8f 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -6,6 +6,7 @@
#include <linux/device.h>
#include <linux/usb/typec.h>
#include <linux/usb/typec_altmode.h>
+#include <linux/kfifo.h>
struct typec_mux;
struct typec_switch;
@@ -27,6 +28,8 @@ struct typec_cable {
enum usb_pd_svdm_ver svdm_version;
};
+struct mode_selection_state;
+
struct typec_partner {
struct device dev;
unsigned int usb_pd:1;
@@ -41,6 +44,13 @@ struct typec_partner {
struct usb_power_delivery *pd;
+ struct delayed_work mode_selection_work;
+ DECLARE_KFIFO(mode_sequence, struct mode_selection_state *,
+ roundup_pow_of_two(TYPEC_MODE_MAX));
+ struct mutex mode_sequence_lock;
+ struct mode_selection_state *mode_states;
+ struct mode_selection_state *active_mode;
+
void (*attach)(struct typec_partner *partner, struct device *dev);
void (*deattach)(struct typec_partner *partner, struct device *dev);
};
diff --git a/drivers/usb/typec/mode_selection.c b/drivers/usb/typec/mode_selection.c
index cb7ddf679037..5b29ca0201ca 100644
--- a/drivers/usb/typec/mode_selection.c
+++ b/drivers/usb/typec/mode_selection.c
@@ -5,9 +5,25 @@
#include <linux/usb/typec_altmode.h>
#include <linux/vmalloc.h>
+#include <linux/usb/pd_vdo.h>
+#include <linux/kfifo.h>
#include "mode_selection.h"
#include "class.h"
+static unsigned int mode_selection_timeout = 4000;
+module_param(mode_selection_timeout, uint, 0644);
+MODULE_PARM_DESC(mode_selection_timeout, "The timeout mode entry, ms");
+
+static unsigned int mode_selection_delay = 1000;
+module_param(mode_selection_delay, uint, 0644);
+MODULE_PARM_DESC(mode_selection_delay,
+ "The delay between attempts to enter or exit a mode, ms");
+
+static unsigned int mode_selection_entry_attempts = 4;
+module_param(mode_selection_entry_attempts, uint, 0644);
+MODULE_PARM_DESC(mode_selection_entry_attempts,
+ "Max attempts to enter mode on BUSY result");
+
static const char * const mode_names[] = {
[TYPEC_DP_ALTMODE] = "DP",
[TYPEC_TBT_ALTMODE] = "TBT",
@@ -15,6 +31,15 @@ static const char * const mode_names[] = {
};
static const char * const default_priorities = "USB4 TBT DP";
+struct mode_selection_state {
+ int mode;
+ bool enable;
+ bool cable_capability;
+ bool enter;
+ int attempt_count;
+ int result;
+};
+
/* -------------------------------------------------------------------------- */
/* port 'mode_priorities' attribute */
static int typec_mode_parse_priority_string(const char *str, int *list)
@@ -114,3 +139,391 @@ int typec_mode_priorities_get(struct typec_port *port, char *buf)
return count + sysfs_emit_at(buf, count, "\n");
}
+
+/* -------------------------------------------------------------------------- */
+/* partner 'mod_selection' attribute */
+
+/**
+ * mode_selection_next() - Process mode selection results and schedule next
+ * action
+ *
+ * This function evaluates the outcome of the previous mode entry or exit
+ * attempt. Based on this result, it determines the next 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)
+{
+ if (!ms->enter) {
+ kfifo_skip(&partner->mode_sequence);
+ } else if (!ms->result) {
+ dev_info(&partner->dev, "%s mode entered\n", mode_names[ms->mode]);
+
+ partner->active_mode = ms;
+ kfifo_reset(&partner->mode_sequence);
+ } else {
+ dev_err(&partner->dev, "%s mode entry failed: %pe\n",
+ mode_names[ms->mode], ERR_PTR(ms->result));
+
+ if (ms->result != -EBUSY ||
+ ms->attempt_count >= mode_selection_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 int mode, const int result)
+{
+ struct mode_selection_state *ms;
+
+ mutex_lock(&partner->mode_sequence_lock);
+ if (kfifo_peek(&partner->mode_sequence, &ms)) {
+ if (ms->mode == mode) {
+ ms->result = result;
+ 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 result)
+{
+ mode_selection_complete(to_typec_partner(alt->dev.parent),
+ typec_svid_to_altmode(alt->svid), result);
+}
+EXPORT_SYMBOL_GPL(typec_mode_selection_altmode_complete);
+
+void typec_mode_selection_usb4_complete(struct typec_partner *partner,
+ const int result)
+{
+ mode_selection_complete(partner, TYPEC_USB4_MODE, result);
+}
+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 result = -EOPNOTSUPP;
+
+ if (port->ops && port->ops->enter_usb_mode) {
+ if (ms->enter && port->usb_mode != USB_MODE_USB4)
+ result = -EPERM;
+ else
+ result = port->ops->enter_usb_mode(port,
+ ms->enter ? USB_MODE_USB4 : USB_MODE_USB3);
+ }
+
+ if (ms->enter)
+ ms->result = result;
+}
+
+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 result = -ENODEV;
+
+ 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)
+ result = alt->ops->activate(alt, ms->enter ? 1 : 0);
+ else
+ result = -EOPNOTSUPP;
+ }
+ }
+
+ if (ms->enter)
+ ms->result = result;
+
+ return result == -ENODEV ? 0 : 1;
+}
+
+static void mode_selection_activate_mode(struct typec_partner *partner,
+ struct mode_selection_state *ms)
+{
+ dev_info(&partner->dev, "%s %s mode\n",
+ ms->enter ? "Enter" : "Exit", mode_names[ms->mode]);
+
+ 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);
+
+ if (ms->enter && ms->result)
+ dev_err(&partner->dev, "%s mode activation failed: %pe\n",
+ mode_names[ms->mode], ERR_PTR(ms->result));
+}
+
+/**
+ * mode_selection_work() - Activate entry into the upcoming mode
+ *
+ * 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 result is temporarily set to -ETIME, and
+ * `mode_selection_activate_mode()` is scheduled for a subsequent entry after a
+ * timeout period. The alternate mode driver is expected to call back with the
+ * actual mode entry result. Upon this callback, `mode_selection_next()` will
+ * determine the subsequent mode and re-schedule mode_selection_work().
+ */
+static void mode_selection_work(struct work_struct *work)
+{
+ struct typec_partner *partner = container_of(work, struct typec_partner,
+ mode_selection_work.work);
+ struct mode_selection_state *ms;
+
+ mutex_lock(&partner->mode_sequence_lock);
+ if (kfifo_peek(&partner->mode_sequence, &ms)) {
+ if (ms->enter && ms->result == -ETIME) {
+ mode_selection_next(partner, ms);
+ } else {
+ mode_selection_activate_mode(partner, ms);
+
+ if (ms->enter) {
+ if (!ms->result) {
+ ms->result = -ETIME;
+ schedule_delayed_work(&partner->mode_selection_work,
+ msecs_to_jiffies(mode_selection_timeout));
+ } else {
+ ms->enter = ms->result == -EBUSY;
+ mode_selection_next(partner, ms);
+ }
+ } else
+ mode_selection_next(partner, ms);
+ }
+ }
+ mutex_unlock(&partner->mode_sequence_lock);
+}
+
+static void mode_selection_init(struct typec_partner *partner)
+{
+ for (int i = 0; i < TYPEC_MODE_MAX; i++) {
+ partner->mode_states[i].mode = i;
+ partner->mode_states[i].enter = true;
+ partner->mode_states[i].result = 0;
+ partner->mode_states[i].attempt_count = 0;
+ }
+
+ kfifo_reset(&partner->mode_sequence);
+ partner->active_mode = NULL;
+}
+
+int typec_mode_selection_create(struct typec_partner *partner)
+{
+ partner->mode_states = vmalloc(
+ sizeof(struct mode_selection_state) * TYPEC_MODE_MAX);
+ if (!partner->mode_states)
+ return -ENOMEM;
+
+ INIT_KFIFO(partner->mode_sequence);
+ mutex_init(&partner->mode_sequence_lock);
+ mode_selection_init(partner);
+ INIT_DELAYED_WORK(&partner->mode_selection_work, mode_selection_work);
+
+ return 0;
+}
+
+void typec_mode_selection_add_mode(struct typec_partner *partner,
+ const int mode)
+{
+ if (partner->mode_states)
+ partner->mode_states[mode].enable =
+ port_mode_supported(to_typec_port(partner->dev.parent), mode);
+}
+
+void typec_mode_selection_add_cable(struct typec_partner *partner,
+ struct typec_cable *cable)
+{
+ 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);
+ bool capability[] = {
+ [TYPEC_DP_ALTMODE] = true,
+ [TYPEC_TBT_ALTMODE] = false,
+ [TYPEC_USB4_MODE] = false,
+ };
+
+ if (!partner->mode_states)
+ return;
+
+ 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;
+ }
+
+ for (int i = 0; i < TYPEC_MODE_MAX; i++)
+ partner->mode_states[i].cable_capability = capability[i];
+}
+
+void typec_mode_selection_destroy(struct typec_partner *partner)
+{
+ if (!partner->mode_states)
+ return;
+
+ 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);
+ vfree(partner->mode_states);
+ partner->mode_states = NULL;
+}
+
+/**
+ * typec_mode_selection_start() - Starts the mode selection process.
+ *
+ * This function populates a 'mode_sequence' FIFO with pointers to
+ * `struct mode_selection_state` instances. The sequence is generated based on
+ * partner/cable capabilities and prioritized according to the port's settings.
+ */
+int typec_mode_selection_start(struct typec_partner *partner)
+{
+ struct typec_port *port = to_typec_port(partner->dev.parent);
+ int ret = 0;
+
+ if (!partner->mode_states)
+ return -ENOMEM;
+
+ 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_init(partner);
+
+ for (int i = 0; i < TYPEC_MODE_MAX; i++) {
+ const int mode = port->mode_priority_list[i];
+ struct mode_selection_state *ms;
+
+ if (mode < TYPEC_MODE_MAX) {
+ ms = &partner->mode_states[mode];
+ if (ms->enable && ms->cable_capability)
+ kfifo_put(&partner->mode_sequence, ms);
+ }
+ }
+
+ if (!kfifo_is_empty(&partner->mode_sequence))
+ 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.
+ *
+ * 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;
+
+ if (!partner->mode_states)
+ return -ENOMEM;
+
+ mutex_lock(&partner->mode_sequence_lock);
+ if (kfifo_peek(&partner->mode_sequence, &ms)) {
+ kfifo_reset(&partner->mode_sequence);
+
+ if (!ms->enter || ms->result) {
+ ms->attempt_count = mode_selection_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_init(partner);
+ mutex_unlock(&partner->mode_sequence_lock);
+
+ return 0;
+}
+
+int typec_mode_selection_get(struct typec_partner *partner, char *buf)
+{
+ ssize_t count = 0;
+ struct mode_selection_state *running_ms;
+
+ if (!partner->mode_states)
+ return -ENOMEM;
+
+ mutex_lock(&partner->mode_sequence_lock);
+ if (!kfifo_peek(&partner->mode_sequence, &running_ms))
+ running_ms = NULL;
+
+ for (int i = 0; i < TYPEC_MODE_MAX; i++) {
+ struct mode_selection_state *ms = &partner->mode_states[i];
+
+ if (ms->enable) {
+ if (!ms->cable_capability)
+ count += sysfs_emit_at(buf, count, "%s=nc ", mode_names[i]);
+ else if (ms == running_ms)
+ count += sysfs_emit_at(buf, count, "%s=... ", mode_names[i]);
+ else if (ms->attempt_count == 0)
+ count += sysfs_emit_at(buf, count, "%s ", mode_names[i]);
+ else if (ms->result == 0)
+ count += sysfs_emit_at(buf, count, "[%s] ", mode_names[i]);
+ else
+ count += sysfs_emit_at(buf, count, "%s=%pe ", mode_names[i],
+ ERR_PTR(ms->result));
+ }
+ }
+ mutex_unlock(&partner->mode_sequence_lock);
+
+ if (count)
+ count += sysfs_emit_at(buf, count, "\n");
+
+ return count;
+}
diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
index c595c84e26a4..6a54792a20db 100644
--- a/drivers/usb/typec/mode_selection.h
+++ b/drivers/usb/typec/mode_selection.h
@@ -17,3 +17,33 @@ static inline int typec_svid_to_altmode(const u16 svid)
int typec_mode_priorities_set(struct typec_port *port,
const char *user_priorities);
int typec_mode_priorities_get(struct typec_port *port, char *buf);
+
+/**
+ * The mode selection process follows a lifecycle tied to the USB-C partner
+ * device. The API is designed to first build a set of desired modes and then
+ * trigger the selection process. The expected sequence of calls is as follows:
+ *
+ * Creation and Configuration:
+ * call typec_mode_selection_create() when the partner device is being set
+ * up. This allocates resources for the mode selection.
+ * After creation, call typec_mode_selection_add_mode() and
+ * typec_mode_selection_add_cable() to define the parameters for the
+ * selection process.
+ *
+ * Execution:
+ * Call typec_mode_selection_start() to trigger the mode selection.
+ * Call typec_mode_selection_reset() to prematurely stop the selection
+ * process and clear any stored results.
+ *
+ * Destruction:
+ * Before destroying a partner, call typec_mode_selection_destroy()
+ */
+int typec_mode_selection_create(struct typec_partner *partner);
+void typec_mode_selection_destroy(struct typec_partner *partner);
+int typec_mode_selection_start(struct typec_partner *partner);
+int typec_mode_selection_reset(struct typec_partner *partner);
+void typec_mode_selection_add_mode(struct typec_partner *partner,
+ const int mode);
+void typec_mode_selection_add_cable(struct typec_partner *partner,
+ struct typec_cable *cable);
+int typec_mode_selection_get(struct typec_partner *partner, char *buf);
diff --git a/include/linux/usb/pd_vdo.h b/include/linux/usb/pd_vdo.h
index 5c48e8a81403..20bcf37ad634 100644
--- a/include/linux/usb/pd_vdo.h
+++ b/include/linux/usb/pd_vdo.h
@@ -439,6 +439,8 @@
| (trans) << 11 | (phy) << 10 | (ele) << 9 | (u4) << 8 \
| ((hops) & 0x3) << 6 | (u2) << 5 | (u32) << 4 | (lane) << 3 \
| (iso) << 2 | (gen))
+#define VDO_TYPEC_CABLE_VERSION(vdo) (((vdo) >> 21) & 0x7)
+#define VDO_TYPEC_CABLE_USB4_SUPP(vdo) (((vdo) & BIT(8)) == ACAB2_USB4_SUPP)
/*
* AMA VDO (PD Rev2.0)
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index 4f05c5f5c91d..49213cf27565 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -225,4 +225,9 @@ void typec_altmode_unregister_driver(struct typec_altmode_driver *drv);
module_driver(__typec_altmode_driver, typec_altmode_register_driver, \
typec_altmode_unregister_driver)
+void typec_mode_selection_altmode_complete(struct typec_altmode *alt,
+ const int result);
+void typec_mode_selection_usb4_complete(struct typec_partner *partner,
+ const int result);
+
#endif /* __USB_TYPEC_ALTMODE_H */
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 05/10] usb: typec: Implement automated mode selection
2025-06-30 14:12 ` [PATCH v2 05/10] usb: typec: Implement automated mode selection Andrei Kuchynski
@ 2025-07-01 3:59 ` kernel test robot
2025-07-01 8:40 ` Greg Kroah-Hartman
1 sibling, 0 replies; 18+ messages in thread
From: kernel test robot @ 2025-07-01 3:59 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, linux-kernel,
Andrei Kuchynski
Hi Andrei,
kernel test robot noticed the following build warnings:
[auto build test WARNING on usb/usb-linus]
[also build test WARNING on chrome-platform/for-next chrome-platform/for-firmware-next linus/master v6.16-rc4 next-20250630]
[cannot apply to usb/usb-testing usb/usb-next]
[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/20250630-221822
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus
patch link: https://lore.kernel.org/r/20250630141239.3174390-6-akuchynski%40chromium.org
patch subject: [PATCH v2 05/10] usb: typec: Implement automated mode selection
config: arm-randconfig-r072-20250701 (https://download.01.org/0day-ci/archive/20250701/202507011101.LAOe7nS8-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250701/202507011101.LAOe7nS8-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/202507011101.LAOe7nS8-lkp@intel.com/
All warnings (new ones prefixed by >>):
Warning: drivers/usb/typec/mode_selection.c:166 function parameter 'partner' not described in 'mode_selection_next'
Warning: drivers/usb/typec/mode_selection.c:166 function parameter 'ms' not described in 'mode_selection_next'
Warning: drivers/usb/typec/mode_selection.c:294 function parameter 'work' not described in 'mode_selection_work'
Warning: drivers/usb/typec/mode_selection.c:419 function parameter 'partner' not described in 'typec_mode_selection_start'
>> Warning: drivers/usb/typec/mode_selection.c:464 function parameter 'partner' not described in 'typec_mode_selection_reset'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 05/10] usb: typec: Implement automated mode selection
2025-06-30 14:12 ` [PATCH v2 05/10] usb: typec: Implement automated mode selection Andrei Kuchynski
2025-07-01 3:59 ` kernel test robot
@ 2025-07-01 8:40 ` Greg Kroah-Hartman
2025-07-22 15:08 ` Andrei Kuchynski
1 sibling, 1 reply; 18+ messages in thread
From: Greg Kroah-Hartman @ 2025-07-01 8:40 UTC (permalink / raw)
To: Andrei Kuchynski
Cc: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform,
Guenter Roeck, Dmitry Baryshkov, Christian A. Ehrhardt,
linux-kernel
On Mon, Jun 30, 2025 at 02:12:34PM +0000, Andrei Kuchynski wrote:
> This commit introduces mode_selection sysfs attribute to control automated
> mode negotiation. Writing "yes" to this file activates the automated
> selection process for DisplayPort, Thunderbolt alternate modes, and USB4
> mode. Conversely, writing "no" will cancel any ongoing selection process
> and exit the currently active mode.
>
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> ---
> Documentation/ABI/testing/sysfs-class-typec | 17 +
> drivers/usb/typec/class.c | 52 ++-
> drivers/usb/typec/class.h | 10 +
> drivers/usb/typec/mode_selection.c | 413 ++++++++++++++++++++
> drivers/usb/typec/mode_selection.h | 30 ++
> include/linux/usb/pd_vdo.h | 2 +
> include/linux/usb/typec_altmode.h | 5 +
> 7 files changed, 527 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index ff3296ee8e1c..0ffc71a7c374 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -263,6 +263,23 @@ Description: The USB Modes that the partner device supports. The active mode
> - usb3 (USB 3.2)
> - usb4 (USB4)
>
> +What: /sys/class/typec/<port>-partner/mode_selection
> +Date: June 2025
> +Contact: Andrei Kuchynski <akuchynski@chromium.org>
> +Description: Lists the partner-supported alternate modes and mode entry
> + results with the currently entered mode bracketed. If a cable doesn't
> + support a mode, it's marked as 'nc'. An ellipsis indicates a mode
> + currently in progress. Automated mode selection is activated by writing
> + "yes" to the file. Conversely, writing "no" will cancel any ongoing
> + selection process and exit the currently active mode, if any.
> +
> + Example values:
> + - "DP TBT=... USB4=nc": The cable does not support USB4 mode,
> + The driver is currently attempting to enter Thunderbolt alt-mode.
> + - "[DP] TBT=-EOPNOTSUPP USB4=-ETIME": USB4 mode entry failed due to
> + a timeout, Thunderbolt failed with the result -EOPNOTSUPP,
> + and DisplayPort is the active alt-mode.
We don't print error codes to userspace like this :(
And "yes" and "no" are not the traditional sysfs apis for on/off, please
use the in-kernel function for that instead that takes many more types
of values.
And again, multiple values in a sysfs file are not usually a good idea
at all as now userspace has to parse them properly. What userspace tool
is going to do that?
> +
> 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 93eadbcdd4c0..8455e07a9934 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -741,6 +741,33 @@ static ssize_t number_of_alternate_modes_show(struct device *dev, struct device_
> }
> static DEVICE_ATTR_RO(number_of_alternate_modes);
>
> +static ssize_t mode_selection_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct typec_partner *partner = to_typec_partner(dev);
> +
> + return typec_mode_selection_get(partner, buf);
> +}
> +
> +static ssize_t mode_selection_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct typec_partner *partner = to_typec_partner(dev);
> + bool start;
> + int ret = kstrtobool(buf, &start);
> +
> + if (!ret) {
> + if (start)
> + ret = typec_mode_selection_start(partner);
> + else
> + ret = typec_mode_selection_reset(partner);
> + }
> +
> + return ret ? : size;
Again, only use ? : if you have to (hint, you don't have to here.)
> +static unsigned int mode_selection_timeout = 4000;
> +module_param(mode_selection_timeout, uint, 0644);
> +MODULE_PARM_DESC(mode_selection_timeout, "The timeout mode entry, ms");
> +
> +static unsigned int mode_selection_delay = 1000;
> +module_param(mode_selection_delay, uint, 0644);
> +MODULE_PARM_DESC(mode_selection_delay,
> + "The delay between attempts to enter or exit a mode, ms");
> +
> +static unsigned int mode_selection_entry_attempts = 4;
> +module_param(mode_selection_entry_attempts, uint, 0644);
> +MODULE_PARM_DESC(mode_selection_entry_attempts,
> + "Max attempts to enter mode on BUSY result");
This is not the 1990's, please NEVER add new module parameters
(especially ones that you never even documented in the changelog!)
This just will not work, attributes for functionality either need to
"just work properly" or you need to make them on a per-controller type
basis as remember, systems have multiple controllers in them...
> +
> static const char * const mode_names[] = {
> [TYPEC_DP_ALTMODE] = "DP",
> [TYPEC_TBT_ALTMODE] = "TBT",
> @@ -15,6 +31,15 @@ static const char * const mode_names[] = {
> };
> static const char * const default_priorities = "USB4 TBT DP";
>
> +struct mode_selection_state {
> + int mode;
Shouldn't this be an enum?
> + bool enable;
> + bool cable_capability;
> + bool enter;
> + int attempt_count;
> + int result;
> +};
No documentation for what this structure is for?
> +
> /* -------------------------------------------------------------------------- */
> /* port 'mode_priorities' attribute */
> static int typec_mode_parse_priority_string(const char *str, int *list)
> @@ -114,3 +139,391 @@ int typec_mode_priorities_get(struct typec_port *port, char *buf)
>
> return count + sysfs_emit_at(buf, count, "\n");
> }
> +
> +/* -------------------------------------------------------------------------- */
> +/* partner 'mod_selection' attribute */
> +
> +/**
> + * mode_selection_next() - Process mode selection results and schedule next
> + * action
> + *
> + * This function evaluates the outcome of the previous mode entry or exit
> + * attempt. Based on this result, it determines the next 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)
> +{
> + if (!ms->enter) {
> + kfifo_skip(&partner->mode_sequence);
> + } else if (!ms->result) {
> + dev_info(&partner->dev, "%s mode entered\n", mode_names[ms->mode]);
Please remove debugging code.
> +
> + partner->active_mode = ms;
> + kfifo_reset(&partner->mode_sequence);
> + } else {
> + dev_err(&partner->dev, "%s mode entry failed: %pe\n",
> + mode_names[ms->mode], ERR_PTR(ms->result));
What can a user do with this error message?
> +
> + if (ms->result != -EBUSY ||
> + ms->attempt_count >= mode_selection_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 int mode, const int result)
> +{
> + struct mode_selection_state *ms;
> +
> + mutex_lock(&partner->mode_sequence_lock);
You use a lock here, but not in the function above? Why?
> + if (kfifo_peek(&partner->mode_sequence, &ms)) {
> + if (ms->mode == mode) {
> + ms->result = result;
> + cancel_delayed_work(&partner->mode_selection_work);
> + mode_selection_next(partner, ms);
Ah, you need to have the lock held, you didn't document that in text or
in a way the compiler can verify/check it :(
> + }
> + }
> + mutex_unlock(&partner->mode_sequence_lock);
> +}
> +
> +void typec_mode_selection_altmode_complete(struct typec_altmode *alt,
> + const int result)
> +{
> + mode_selection_complete(to_typec_partner(alt->dev.parent),
> + typec_svid_to_altmode(alt->svid), result);
> +}
> +EXPORT_SYMBOL_GPL(typec_mode_selection_altmode_complete);
> +
> +void typec_mode_selection_usb4_complete(struct typec_partner *partner,
> + const int result)
> +{
> + mode_selection_complete(partner, TYPEC_USB4_MODE, result);
> +}
> +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 result = -EOPNOTSUPP;
> +
> + if (port->ops && port->ops->enter_usb_mode) {
> + if (ms->enter && port->usb_mode != USB_MODE_USB4)
> + result = -EPERM;
> + else
> + result = port->ops->enter_usb_mode(port,
> + ms->enter ? USB_MODE_USB4 : USB_MODE_USB3);
> + }
> +
> + if (ms->enter)
> + ms->result = result;
> +}
> +
> +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 result = -ENODEV;
> +
> + 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)
> + result = alt->ops->activate(alt, ms->enter ? 1 : 0);
> + else
> + result = -EOPNOTSUPP;
> + }
> + }
> +
> + if (ms->enter)
> + ms->result = result;
> +
> + return result == -ENODEV ? 0 : 1;
> +}
> +
> +static void mode_selection_activate_mode(struct typec_partner *partner,
> + struct mode_selection_state *ms)
> +{
> + dev_info(&partner->dev, "%s %s mode\n",
> + ms->enter ? "Enter" : "Exit", mode_names[ms->mode]);
Again, please remove debugging code.
When drivers work properly, they are quiet.
And this really is the only valid use for ? : around, so that's ok here :)
> +void typec_mode_selection_add_cable(struct typec_partner *partner,
> + struct typec_cable *cable)
> +{
> + 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);
> + bool capability[] = {
> + [TYPEC_DP_ALTMODE] = true,
> + [TYPEC_TBT_ALTMODE] = false,
> + [TYPEC_USB4_MODE] = false,
> + };
Why are these the default capabilities? Where is that documented? Why
these specific values to start with?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 05/10] usb: typec: Implement automated mode selection
2025-07-01 8:40 ` Greg Kroah-Hartman
@ 2025-07-22 15:08 ` Andrei Kuchynski
2025-07-24 8:39 ` Andrei Kuchynski
0 siblings, 1 reply; 18+ messages in thread
From: Andrei Kuchynski @ 2025-07-22 15:08 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform,
Guenter Roeck, Dmitry Baryshkov, Christian A. Ehrhardt,
linux-kernel
On Tue, Jul 1, 2025 at 10:41 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jun 30, 2025 at 02:12:34PM +0000, Andrei Kuchynski wrote:
> > This commit introduces mode_selection sysfs attribute to control automated
> > mode negotiation. Writing "yes" to this file activates the automated
> > selection process for DisplayPort, Thunderbolt alternate modes, and USB4
> > mode. Conversely, writing "no" will cancel any ongoing selection process
> > and exit the currently active mode.
> >
> > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > ---
> > Documentation/ABI/testing/sysfs-class-typec | 17 +
> > drivers/usb/typec/class.c | 52 ++-
> > drivers/usb/typec/class.h | 10 +
> > drivers/usb/typec/mode_selection.c | 413 ++++++++++++++++++++
> > drivers/usb/typec/mode_selection.h | 30 ++
> > include/linux/usb/pd_vdo.h | 2 +
> > include/linux/usb/typec_altmode.h | 5 +
> > 7 files changed, 527 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index ff3296ee8e1c..0ffc71a7c374 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -263,6 +263,23 @@ Description: The USB Modes that the partner device supports. The active mode
> > - usb3 (USB 3.2)
> > - usb4 (USB4)
> >
> > +What: /sys/class/typec/<port>-partner/mode_selection
> > +Date: June 2025
> > +Contact: Andrei Kuchynski <akuchynski@chromium.org>
> > +Description: Lists the partner-supported alternate modes and mode entry
> > + results with the currently entered mode bracketed. If a cable doesn't
> > + support a mode, it's marked as 'nc'. An ellipsis indicates a mode
> > + currently in progress. Automated mode selection is activated by writing
> > + "yes" to the file. Conversely, writing "no" will cancel any ongoing
> > + selection process and exit the currently active mode, if any.
> > +
> > + Example values:
> > + - "DP TBT=... USB4=nc": The cable does not support USB4 mode,
> > + The driver is currently attempting to enter Thunderbolt alt-mode.
> > + - "[DP] TBT=-EOPNOTSUPP USB4=-ETIME": USB4 mode entry failed due to
> > + a timeout, Thunderbolt failed with the result -EOPNOTSUPP,
> > + and DisplayPort is the active alt-mode.
>
> We don't print error codes to userspace like this :(
>
The intention is to provide detailed status updates regarding failures.
I will revise this to simplify string parsing in user-space and omit
any error codes.
> And "yes" and "no" are not the traditional sysfs apis for on/off, please
> use the in-kernel function for that instead that takes many more types
> of values.
>
kstrtobool() is used. It is just not documented here. I will update the
documentation
> And again, multiple values in a sysfs file are not usually a good idea
> at all as now userspace has to parse them properly. What userspace tool
> is going to do that?
>
By combining results into a single, complex status string, the user can
receive the information atomically. This is why type_mode_selection_get
function is mutex-protected. It prevents the user from encountering
inconsistent states when reading different files.
> > +
> > 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 93eadbcdd4c0..8455e07a9934 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -741,6 +741,33 @@ static ssize_t number_of_alternate_modes_show(struct device *dev, struct device_
> > }
> > static DEVICE_ATTR_RO(number_of_alternate_modes);
> >
> > +static ssize_t mode_selection_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct typec_partner *partner = to_typec_partner(dev);
> > +
> > + return typec_mode_selection_get(partner, buf);
> > +}
> > +
> > +static ssize_t mode_selection_store(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t size)
> > +{
> > + struct typec_partner *partner = to_typec_partner(dev);
> > + bool start;
> > + int ret = kstrtobool(buf, &start);
> > +
> > + if (!ret) {
> > + if (start)
> > + ret = typec_mode_selection_start(partner);
> > + else
> > + ret = typec_mode_selection_reset(partner);
> > + }
> > +
> > + return ret ? : size;
>
> Again, only use ? : if you have to (hint, you don't have to here.)
>
Will be replaced with an if/else statement.
> > +static unsigned int mode_selection_timeout = 4000;
> > +module_param(mode_selection_timeout, uint, 0644);
> > +MODULE_PARM_DESC(mode_selection_timeout, "The timeout mode entry, ms");
> > +
> > +static unsigned int mode_selection_delay = 1000;
> > +module_param(mode_selection_delay, uint, 0644);
> > +MODULE_PARM_DESC(mode_selection_delay,
> > + "The delay between attempts to enter or exit a mode, ms");
> > +
> > +static unsigned int mode_selection_entry_attempts = 4;
> > +module_param(mode_selection_entry_attempts, uint, 0644);
> > +MODULE_PARM_DESC(mode_selection_entry_attempts,
> > + "Max attempts to enter mode on BUSY result");
>
> This is not the 1990's, please NEVER add new module parameters
> (especially ones that you never even documented in the changelog!)
>
> This just will not work, attributes for functionality either need to
> "just work properly" or you need to make them on a per-controller type
> basis as remember, systems have multiple controllers in them...
>
The current values are suitable for all controllers we work with,
rendering per-controller adjustments unnecessary. Therefore, I will
proceed with removing these module parameters.
> > +
> > static const char * const mode_names[] = {
> > [TYPEC_DP_ALTMODE] = "DP",
> > [TYPEC_TBT_ALTMODE] = "TBT",
> > @@ -15,6 +31,15 @@ static const char * const mode_names[] = {
> > };
> > static const char * const default_priorities = "USB4 TBT DP";
> >
> > +struct mode_selection_state {
> > + int mode;
>
> Shouldn't this be an enum?
>
enum is much better, thanks
> > + bool enable;
> > + bool cable_capability;
> > + bool enter;
> > + int attempt_count;
> > + int result;
> > +};
>
> No documentation for what this structure is for?
>
This, of course, needs to be described.
> > +
> > /* -------------------------------------------------------------------------- */
> > /* port 'mode_priorities' attribute */
> > static int typec_mode_parse_priority_string(const char *str, int *list)
> > @@ -114,3 +139,391 @@ int typec_mode_priorities_get(struct typec_port *port, char *buf)
> >
> > return count + sysfs_emit_at(buf, count, "\n");
> > }
> > +
> > +/* -------------------------------------------------------------------------- */
> > +/* partner 'mod_selection' attribute */
> > +
> > +/**
> > + * mode_selection_next() - Process mode selection results and schedule next
> > + * action
> > + *
> > + * This function evaluates the outcome of the previous mode entry or exit
> > + * attempt. Based on this result, it determines the next 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)
> > +{
> > + if (!ms->enter) {
> > + kfifo_skip(&partner->mode_sequence);
> > + } else if (!ms->result) {
> > + dev_info(&partner->dev, "%s mode entered\n", mode_names[ms->mode]);
>
> Please remove debugging code.
>
> > +
> > + partner->active_mode = ms;
> > + kfifo_reset(&partner->mode_sequence);
> > + } else {
> > + dev_err(&partner->dev, "%s mode entry failed: %pe\n",
> > + mode_names[ms->mode], ERR_PTR(ms->result));
>
> What can a user do with this error message?
>
All prints will be removed.
> > +
> > + if (ms->result != -EBUSY ||
> > + ms->attempt_count >= mode_selection_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 int mode, const int result)
> > +{
> > + struct mode_selection_state *ms;
> > +
> > + mutex_lock(&partner->mode_sequence_lock);
>
> You use a lock here, but not in the function above? Why?
>
`mode_sequence_lock` mutex protects `mode_sequence` kfifo state. It
should be locked before kfifo_peek()
> > + if (kfifo_peek(&partner->mode_sequence, &ms)) {
> > + if (ms->mode == mode) {
> > + ms->result = result;
> > + cancel_delayed_work(&partner->mode_selection_work);
> > + mode_selection_next(partner, ms);
>
> Ah, you need to have the lock held, you didn't document that in text or
> in a way the compiler can verify/check it :(
>
>
It is a good case for __must_hold macro. Thanks
> > + }
> > + }
> > + mutex_unlock(&partner->mode_sequence_lock);
> > +}
> > +
> > +void typec_mode_selection_altmode_complete(struct typec_altmode *alt,
> > + const int result)
> > +{
> > + mode_selection_complete(to_typec_partner(alt->dev.parent),
> > + typec_svid_to_altmode(alt->svid), result);
> > +}
> > +EXPORT_SYMBOL_GPL(typec_mode_selection_altmode_complete);
> > +
> > +void typec_mode_selection_usb4_complete(struct typec_partner *partner,
> > + const int result)
> > +{
> > + mode_selection_complete(partner, TYPEC_USB4_MODE, result);
> > +}
> > +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 result = -EOPNOTSUPP;
> > +
> > + if (port->ops && port->ops->enter_usb_mode) {
> > + if (ms->enter && port->usb_mode != USB_MODE_USB4)
> > + result = -EPERM;
> > + else
> > + result = port->ops->enter_usb_mode(port,
> > + ms->enter ? USB_MODE_USB4 : USB_MODE_USB3);
> > + }
> > +
> > + if (ms->enter)
> > + ms->result = result;
> > +}
> > +
> > +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 result = -ENODEV;
> > +
> > + 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)
> > + result = alt->ops->activate(alt, ms->enter ? 1 : 0);
> > + else
> > + result = -EOPNOTSUPP;
> > + }
> > + }
> > +
> > + if (ms->enter)
> > + ms->result = result;
> > +
> > + return result == -ENODEV ? 0 : 1;
> > +}
> > +
> > +static void mode_selection_activate_mode(struct typec_partner *partner,
> > + struct mode_selection_state *ms)
> > +{
> > + dev_info(&partner->dev, "%s %s mode\n",
> > + ms->enter ? "Enter" : "Exit", mode_names[ms->mode]);
>
> Again, please remove debugging code.
>
> When drivers work properly, they are quiet.
>
> And this really is the only valid use for ? : around, so that's ok here :)
>
Sadly, the only good case will be removed :)
> > +void typec_mode_selection_add_cable(struct typec_partner *partner,
> > + struct typec_cable *cable)
> > +{
> > + 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);
> > + bool capability[] = {
> > + [TYPEC_DP_ALTMODE] = true,
> > + [TYPEC_TBT_ALTMODE] = false,
> > + [TYPEC_USB4_MODE] = false,
> > + };
>
> Why are these the default capabilities? Where is that documented? Why
> these specific values to start with?
>
These values are currently utilized in ChromeOS when VDO is 0 and the
cable is neither passive nor active. This unfortunately includes some USB
dongles with DisplayPort support. I will document this behavior.
Thanks for the review!
Andrei
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 05/10] usb: typec: Implement automated mode selection
2025-07-22 15:08 ` Andrei Kuchynski
@ 2025-07-24 8:39 ` Andrei Kuchynski
2025-07-31 16:09 ` Andrei Kuchynski
0 siblings, 1 reply; 18+ messages in thread
From: Andrei Kuchynski @ 2025-07-24 8:39 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform,
Guenter Roeck, Dmitry Baryshkov, Christian A. Ehrhardt,
linux-kernel
Proposed sysfs entries for V3:
- portN/portN.M/priority, RW.
This attribute assigns a unique priority to each mode. If a user
attempts to input a value that is already in use, the existing mode at
that priority will have its priority incremented by one to accommodate
the new input. Users cannot disable a mode via this entry; disabling
is handled by `active` for altmodes and `usb_capability` for USB4 mode
- portN/mode_priorities, RO.
Provides a prioritized list of all available modes for the port,
formatted as a space-separated string (e.g., "USB4 TBT DP").
- portN-partner/mode_selection, RW.
Write: 1/0 to trigger or cancel mode selection.
Read: Provides a prioritized list of all available modes for the
partner. Modes currently in progress are indicated by parentheses
(e.g., "USB4 (TBT) DP"). Active modes are enclosed in brackets
(e.g., "USB4 [TBT] DP").
- portN-partner.M/entry_result, RO.
Represents a mode state for this altmode, e.g. "none", "active",
"in progress", "cable error", "timeout".
- portN/usb4_priority, RW.
- portN-partner/usb4_entry_result, RO.
USB4 mode, not being part of `typec_altmode_group`, introduces
additional attributes with the same meaning as alternate modes
attributes.
Please let me know if you have any questions, require further
clarification on these proposed sysfs entries, or have objections to
them.
Thanks,
Andrei
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 05/10] usb: typec: Implement automated mode selection
2025-07-24 8:39 ` Andrei Kuchynski
@ 2025-07-31 16:09 ` Andrei Kuchynski
0 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-07-31 16:09 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Heikki Krogerus, Abhishek Pandit-Subedi, Benson Leung,
Jameson Thies, Tzung-Bi Shih, linux-usb, chrome-platform,
Guenter Roeck, Dmitry Baryshkov, Christian A. Ehrhardt,
linux-kernel
On Thu, Jul 24, 2025 at 10:39 AM Andrei Kuchynski
<akuchynski@chromium.org> wrote:
>
> Proposed sysfs entries for V3:
>
> - portN/portN.M/priority, RW.
> This attribute assigns a unique priority to each mode. If a user
> attempts to input a value that is already in use, the existing mode at
> that priority will have its priority incremented by one to accommodate
> the new input. Users cannot disable a mode via this entry; disabling
> is handled by `active` for altmodes and `usb_capability` for USB4 mode
>
> - portN/mode_priorities, RO.
> Provides a prioritized list of all available modes for the port,
> formatted as a space-separated string (e.g., "USB4 TBT DP").
>
> - portN-partner/mode_selection, RW.
> Write: 1/0 to trigger or cancel mode selection.
> Read: Provides a prioritized list of all available modes for the
> partner. Modes currently in progress are indicated by parentheses
> (e.g., "USB4 (TBT) DP"). Active modes are enclosed in brackets
> (e.g., "USB4 [TBT] DP").
>
> - portN-partner.M/entry_result, RO.
> Represents a mode state for this altmode, e.g. "none", "active",
> "in progress", "cable error", "timeout".
>
> - portN/usb4_priority, RW.
> - portN-partner/usb4_entry_result, RO.
> USB4 mode, not being part of `typec_altmode_group`, introduces
> additional attributes with the same meaning as alternate modes
> attributes.
>
> Please let me know if you have any questions, require further
> clarification on these proposed sysfs entries, or have objections to
> them.
Regarding the sysfs attributes, Heikki, do you have any suggestions or
disagreements? Please let me know your thoughts.
Additionally, for consistency, it would be beneficial to use names
"DisplayPort" and "Thunderbolt3" since they are already recognized
within the kernel. Using these full names rather than "DP" and "TBT"
would be preferable
Thanks,
Andrei
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 06/10] usb: typec: Report altmode entry status via callback
2025-06-30 14:12 [PATCH v2 00/10] USB Type-C mode selection Andrei Kuchynski
` (4 preceding siblings ...)
2025-06-30 14:12 ` [PATCH v2 05/10] usb: typec: Implement automated mode selection Andrei Kuchynski
@ 2025-06-30 14:12 ` Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 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-06-30 14:12 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, 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 d8b906ec4d1c..f0c8395a6652 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -274,16 +274,20 @@ static void dp_altmode_work(struct work_struct *work)
header = DP_HEADER(dp, svdm_version, DP_CMD_STATUS_UPDATE);
vdo = 1;
ret = typec_altmode_vdm(dp->alt, header, &vdo, 2);
- if (ret)
+ if (ret) {
dev_err(&dp->alt->dev,
"unable to send Status Update command (%d)\n",
ret);
+ typec_mode_selection_altmode_complete(dp->alt, ret);
+ }
break;
case DP_STATE_CONFIGURE:
ret = dp_altmode_configure_vdm(dp, dp->data.conf);
- if (ret)
+ if (ret) {
dev_err(&dp->alt->dev,
"unable to send Configure command (%d)\n", ret);
+ typec_mode_selection_altmode_complete(dp->alt, ret);
+ }
break;
case DP_STATE_CONFIGURE_PRIME:
ret = dp_altmode_configure_vdm_cable(dp, dp->data_prime.conf);
@@ -352,6 +356,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);
@@ -395,10 +400,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;
@@ -413,6 +420,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.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 07/10] usb: typec: ucsi: displayport: Propagate DP altmode entry result
2025-06-30 14:12 [PATCH v2 00/10] USB Type-C mode selection Andrei Kuchynski
` (5 preceding siblings ...)
2025-06-30 14:12 ` [PATCH v2 06/10] usb: typec: Report altmode entry status via callback Andrei Kuchynski
@ 2025-06-30 14:12 ` Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 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-06-30 14:12 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, 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.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 08/10] platform/chrome: cros_ec_typec: Propagate altmode entry result
2025-06-30 14:12 [PATCH v2 00/10] USB Type-C mode selection Andrei Kuchynski
` (6 preceding siblings ...)
2025-06-30 14:12 ` [PATCH v2 07/10] usb: typec: ucsi: displayport: Propagate DP altmode entry result Andrei Kuchynski
@ 2025-06-30 14:12 ` Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 09/10] platform/chrome: cros_ec_typec: Report USB4 entry status via callback Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 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-06-30 14:12 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, 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 3aed429fde03..a4f338771094 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -610,6 +610,7 @@ static int cros_typec_enable_dp(struct cros_typec_data *typec,
if (!ret)
ret = typec_mux_set(port->mux, &port->state);
+ dp_data.error = 0;
if (!ret)
ret = cros_typec_displayport_status_update(port->state.alt,
port->state.data);
@@ -699,8 +700,16 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
ret = cros_typec_enable_usb4(typec, port_num, pd_ctrl);
} else if (port->mux_flags & USB_PD_MUX_TBT_COMPAT_ENABLED) {
ret = cros_typec_enable_tbt(typec, port_num, pd_ctrl);
+ cros_typec_tbt_status_update(
+ port->port_altmode[CROS_EC_ALTMODE_TBT], ret);
} else if (port->mux_flags & USB_PD_MUX_DP_ENABLED) {
ret = cros_typec_enable_dp(typec, port_num, pd_ctrl);
+ if (ret) {
+ struct typec_displayport_data dp_data = {.error = ret};
+
+ cros_typec_displayport_status_update(
+ port->port_altmode[CROS_EC_ALTMODE_DP], &dp_data);
+ }
} else if (port->mux_flags & USB_PD_MUX_SAFE_MODE) {
ret = cros_typec_usb_safe_state(port);
} else if (port->mux_flags & USB_PD_MUX_USB_ENABLED) {
diff --git a/drivers/platform/chrome/cros_typec_altmode.c b/drivers/platform/chrome/cros_typec_altmode.c
index 557340b53af0..7ee295cf0c02 100644
--- a/drivers/platform/chrome/cros_typec_altmode.c
+++ b/drivers/platform/chrome/cros_typec_altmode.c
@@ -28,6 +28,7 @@ struct cros_typec_altmode_data {
u16 sid;
u8 mode;
+ int error;
};
struct cros_typec_dp_data {
@@ -295,9 +296,16 @@ int cros_typec_displayport_status_update(struct typec_altmode *altmode,
dp_data->data = *data;
dp_data->pending_status_update = false;
- adata->header |= VDO_CMDT(CMDT_RSP_ACK);
- adata->vdo_data = &dp_data->data.status;
- adata->vdo_size = 2;
+ if (data->error) {
+ adata->header |= VDO_CMDT(CMDT_RSP_NAK);
+ adata->error = dp_data->data.error;
+ adata->vdo_data = &adata->error;
+ adata->vdo_size = 1;
+ } else {
+ adata->header |= VDO_CMDT(CMDT_RSP_ACK);
+ adata->vdo_data = &dp_data->data.status;
+ adata->vdo_size = 2;
+ }
schedule_work(&adata->work);
mutex_unlock(&adata->lock);
@@ -370,4 +378,22 @@ cros_typec_register_thunderbolt(struct cros_typec_port *port,
return alt;
}
+
+int cros_typec_tbt_status_update(struct typec_altmode *alt, int error)
+{
+ struct cros_typec_altmode_data *adata = typec_altmode_get_drvdata(alt);
+
+ mutex_lock(&adata->lock);
+
+ adata->header = VDO(adata->sid, 1, SVDM_VER_2_0, TBT_CMD_STATUS_UPDATE);
+ adata->header |= VDO_CMDT(error ? CMDT_RSP_NAK : CMDT_RSP_ACK);
+ adata->error = error;
+ adata->vdo_data = &adata->error;
+ adata->vdo_size = 1;
+ schedule_work(&adata->work);
+
+ mutex_unlock(&adata->lock);
+
+ return 0;
+}
#endif
diff --git a/drivers/platform/chrome/cros_typec_altmode.h b/drivers/platform/chrome/cros_typec_altmode.h
index 3f2aa95d065a..848a2b194b34 100644
--- a/drivers/platform/chrome/cros_typec_altmode.h
+++ b/drivers/platform/chrome/cros_typec_altmode.h
@@ -39,6 +39,7 @@ static inline int cros_typec_displayport_status_update(struct typec_altmode *alt
struct typec_altmode *
cros_typec_register_thunderbolt(struct cros_typec_port *port,
struct typec_altmode_desc *desc);
+int cros_typec_tbt_status_update(struct typec_altmode *alt, int error);
#else
static inline struct typec_altmode *
cros_typec_register_thunderbolt(struct cros_typec_port *port,
@@ -46,6 +47,11 @@ cros_typec_register_thunderbolt(struct cros_typec_port *port,
{
return typec_port_register_altmode(port->port, desc);
}
+static inline int cros_typec_tbt_status_update(struct typec_altmode *alt,
+ int error)
+{
+ return 0;
+}
#endif
#endif /* __CROS_TYPEC_ALTMODE_H__ */
diff --git a/include/linux/usb/typec_dp.h b/include/linux/usb/typec_dp.h
index 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.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 09/10] platform/chrome: cros_ec_typec: Report USB4 entry status via callback
2025-06-30 14:12 [PATCH v2 00/10] USB Type-C mode selection Andrei Kuchynski
` (7 preceding siblings ...)
2025-06-30 14:12 ` [PATCH v2 08/10] platform/chrome: cros_ec_typec: Propagate " Andrei Kuchynski
@ 2025-06-30 14:12 ` Andrei Kuchynski
2025-06-30 14:12 ` [PATCH v2 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-06-30 14:12 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, 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 a4f338771094..c5a7f42ffb5a 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -698,6 +698,7 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
if (port->mux_flags & USB_PD_MUX_USB4_ENABLED) {
ret = cros_typec_enable_usb4(typec, port_num, pd_ctrl);
+ typec_mode_selection_usb4_complete(port->partner, ret);
} else if (port->mux_flags & USB_PD_MUX_TBT_COMPAT_ENABLED) {
ret = cros_typec_enable_tbt(typec, port_num, pd_ctrl);
cros_typec_tbt_status_update(
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 10/10] platform/chrome: cros_ec_typec: Add default_usb_mode_set support
2025-06-30 14:12 [PATCH v2 00/10] USB Type-C mode selection Andrei Kuchynski
` (8 preceding siblings ...)
2025-06-30 14:12 ` [PATCH v2 09/10] platform/chrome: cros_ec_typec: Report USB4 entry status via callback Andrei Kuchynski
@ 2025-06-30 14:12 ` Andrei Kuchynski
9 siblings, 0 replies; 18+ messages in thread
From: Andrei Kuchynski @ 2025-06-30 14:12 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, 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 c5a7f42ffb5a..3de6b819906e 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);
@@ -59,6 +64,7 @@ static int cros_typec_enter_usb_mode(struct typec_port *tc_port, enum usb_mode m
}
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
};
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread