This patch introduces APIs to manage the priority of USB Type-C alternate
modes. These APIs allow for setting and retrieving a priority number for
each mode. If a new priority value conflicts with an existing mode's
priority, the priorities of the conflicting mode and all subsequent modes
are automatically incremented to ensure uniqueness.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/usb/typec/Makefile         |  2 +-
 drivers/usb/typec/mode_selection.c | 38 ++++++++++++++++++++++++++++++
 drivers/usb/typec/mode_selection.h |  6 +++++
 include/linux/usb/typec_altmode.h  |  1 +
 4 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/typec/mode_selection.c
 create mode 100644 drivers/usb/typec/mode_selection.h

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/mode_selection.c b/drivers/usb/typec/mode_selection.c
new file mode 100644
index 000000000000..2179bf25f5d4
--- /dev/null
+++ b/drivers/usb/typec/mode_selection.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Google LLC.
+ */
+
+#include "mode_selection.h"
+#include "class.h"
+#include "bus.h"
+
+static int increment_duplicated_priority(struct device *dev, void *data)
+{
+	struct typec_altmode **alt_target = (struct typec_altmode **)data;
+
+	if (is_typec_altmode(dev)) {
+		struct typec_altmode *alt = to_typec_altmode(dev);
+
+		if (alt != *alt_target && alt->priority == (*alt_target)->priority) {
+			alt->priority++;
+			*alt_target = alt;
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+void typec_mode_set_priority(struct typec_altmode *alt,
+		const unsigned int priority)
+{
+	struct typec_port *port = to_typec_port(alt->dev.parent);
+	int res = 1;
+
+	alt->priority = priority;
+
+	while (res)
+		res = device_for_each_child(&port->dev, &alt,
+				increment_duplicated_priority);
+}
diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
new file mode 100644
index 000000000000..cbf5a37e6404
--- /dev/null
+++ b/drivers/usb/typec/mode_selection.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/usb/typec_altmode.h>
+
+void typec_mode_set_priority(struct typec_altmode *alt,
+		const unsigned int priority);
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index b3c0866ea70f..571c6e00b54f 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -28,6 +28,7 @@ struct typec_altmode {
 	int				mode;
 	u32				vdo;
 	unsigned int			active:1;
+	unsigned int			priority;
 
 	char				*desc;
 	const struct typec_altmode_ops	*ops;
-- 
2.51.0.355.g5224444f11-goog

On Fri, Sep 05, 2025 at 02:22:05PM +0000, Andrei Kuchynski wrote:
> This patch introduces APIs to manage the priority of USB Type-C alternate
> modes. These APIs allow for setting and retrieving a priority number for
> each mode. If a new priority value conflicts with an existing mode's
> priority, the priorities of the conflicting mode and all subsequent modes
> are automatically incremented to ensure uniqueness.
> 
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> ---
>  drivers/usb/typec/Makefile         |  2 +-
>  drivers/usb/typec/mode_selection.c | 38 ++++++++++++++++++++++++++++++
>  drivers/usb/typec/mode_selection.h |  6 +++++
>  include/linux/usb/typec_altmode.h  |  1 +
>  4 files changed, 46 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/usb/typec/mode_selection.c
>  create mode 100644 drivers/usb/typec/mode_selection.h
> 
> 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/mode_selection.c b/drivers/usb/typec/mode_selection.c
> new file mode 100644
> index 000000000000..2179bf25f5d4
> --- /dev/null
> +++ b/drivers/usb/typec/mode_selection.c
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2025 Google LLC.
> + */
> +
> +#include "mode_selection.h"
> +#include "class.h"
> +#include "bus.h"
> +
> +static int increment_duplicated_priority(struct device *dev, void *data)
> +{
> +	struct typec_altmode **alt_target = (struct typec_altmode **)data;
> +
> +	if (is_typec_altmode(dev)) {
> +		struct typec_altmode *alt = to_typec_altmode(dev);
> +
> +		if (alt != *alt_target && alt->priority == (*alt_target)->priority) {
> +			alt->priority++;
> +			*alt_target = alt;
> +			return 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +void typec_mode_set_priority(struct typec_altmode *alt,
> +		const unsigned int priority)
> +{
> +	struct typec_port *port = to_typec_port(alt->dev.parent);
> +	int res = 1;
> +
> +	alt->priority = priority;
> +
> +	while (res)
> +		res = device_for_each_child(&port->dev, &alt,
> +				increment_duplicated_priority);
> +}
> diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
> new file mode 100644
> index 000000000000..cbf5a37e6404
> --- /dev/null
> +++ b/drivers/usb/typec/mode_selection.h
> @@ -0,0 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <linux/usb/typec_altmode.h>
> +
> +void typec_mode_set_priority(struct typec_altmode *alt,
> +		const unsigned int priority);
> diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> index b3c0866ea70f..571c6e00b54f 100644
> --- a/include/linux/usb/typec_altmode.h
> +++ b/include/linux/usb/typec_altmode.h
> @@ -28,6 +28,7 @@ struct typec_altmode {
>  	int				mode;
>  	u32				vdo;
>  	unsigned int			active:1;
> +	unsigned int			priority;

What is the range of this?  And this value is only incremented, never
decremented?

thanks,

greg k-h

On Wed, Sep 10, 2025 at 1:31 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Sep 05, 2025 at 02:22:05PM +0000, Andrei Kuchynski wrote:
> > This patch introduces APIs to manage the priority of USB Type-C alternate
> > modes. These APIs allow for setting and retrieving a priority number for
> > each mode. If a new priority value conflicts with an existing mode's
> > priority, the priorities of the conflicting mode and all subsequent modes
> > are automatically incremented to ensure uniqueness.
> >
> > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > ---
> >  drivers/usb/typec/Makefile         |  2 +-
> >  drivers/usb/typec/mode_selection.c | 38 ++++++++++++++++++++++++++++++
> >  drivers/usb/typec/mode_selection.h |  6 +++++
> >  include/linux/usb/typec_altmode.h  |  1 +
> >  4 files changed, 46 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/usb/typec/mode_selection.c
> >  create mode 100644 drivers/usb/typec/mode_selection.h
> >
> > 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/mode_selection.c b/drivers/usb/typec/mode_selection.c
> > new file mode 100644
> > index 000000000000..2179bf25f5d4
> > --- /dev/null
> > +++ b/drivers/usb/typec/mode_selection.c
> > @@ -0,0 +1,38 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright 2025 Google LLC.
> > + */
> > +
> > +#include "mode_selection.h"
> > +#include "class.h"
> > +#include "bus.h"
> > +
> > +static int increment_duplicated_priority(struct device *dev, void *data)
> > +{
> > +     struct typec_altmode **alt_target = (struct typec_altmode **)data;
> > +
> > +     if (is_typec_altmode(dev)) {
> > +             struct typec_altmode *alt = to_typec_altmode(dev);
> > +
> > +             if (alt != *alt_target && alt->priority == (*alt_target)->priority) {
> > +                     alt->priority++;
> > +                     *alt_target = alt;
> > +                     return 1;
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +void typec_mode_set_priority(struct typec_altmode *alt,
> > +             const unsigned int priority)
> > +{
> > +     struct typec_port *port = to_typec_port(alt->dev.parent);
> > +     int res = 1;
> > +
> > +     alt->priority = priority;
> > +
> > +     while (res)
> > +             res = device_for_each_child(&port->dev, &alt,
> > +                             increment_duplicated_priority);
> > +}
> > diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
> > new file mode 100644
> > index 000000000000..cbf5a37e6404
> > --- /dev/null
> > +++ b/drivers/usb/typec/mode_selection.h
> > @@ -0,0 +1,6 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#include <linux/usb/typec_altmode.h>
> > +
> > +void typec_mode_set_priority(struct typec_altmode *alt,
> > +             const unsigned int priority);
> > diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> > index b3c0866ea70f..571c6e00b54f 100644
> > --- a/include/linux/usb/typec_altmode.h
> > +++ b/include/linux/usb/typec_altmode.h
> > @@ -28,6 +28,7 @@ struct typec_altmode {
> >       int                             mode;
> >       u32                             vdo;
> >       unsigned int                    active:1;
> > +     unsigned int                    priority;
>
> What is the range of this?  And this value is only incremented, never
> decremented?
>

The range extends from 0 to UINT_MAX. The value is only incremented.
The only exception is that If the user sets UINT_MAX for two alternate
modes in turn, the priority of the first mode becomes 0. This does not
break the algorithm, and the user can check all priorities via
‘priority’ attributes.
I am unsure if a check for this specific case is necessary, as it
would require examining priorities across all modes, not just a simple
'if' statement.
There are a few ideas in this algorithm:
- all priorities must always be valid and unique
- no unnecessary restrictions for the user
- as simple as possible

Thanks,
Andrei

On Wed, Sep 10, 2025 at 07:35:42PM +0000, Andrei Kuchynski wrote:
> On Wed, Sep 10, 2025 at 1:31 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Sep 05, 2025 at 02:22:05PM +0000, Andrei Kuchynski wrote:
> > > This patch introduces APIs to manage the priority of USB Type-C alternate
> > > modes. These APIs allow for setting and retrieving a priority number for
> > > each mode. If a new priority value conflicts with an existing mode's
> > > priority, the priorities of the conflicting mode and all subsequent modes
> > > are automatically incremented to ensure uniqueness.
> > >
> > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > ---
> > >  drivers/usb/typec/Makefile         |  2 +-
> > >  drivers/usb/typec/mode_selection.c | 38 ++++++++++++++++++++++++++++++
> > >  drivers/usb/typec/mode_selection.h |  6 +++++
> > >  include/linux/usb/typec_altmode.h  |  1 +
> > >  4 files changed, 46 insertions(+), 1 deletion(-)
> > >  create mode 100644 drivers/usb/typec/mode_selection.c
> > >  create mode 100644 drivers/usb/typec/mode_selection.h
> > >
> > > 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/mode_selection.c b/drivers/usb/typec/mode_selection.c
> > > new file mode 100644
> > > index 000000000000..2179bf25f5d4
> > > --- /dev/null
> > > +++ b/drivers/usb/typec/mode_selection.c
> > > @@ -0,0 +1,38 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright 2025 Google LLC.
> > > + */
> > > +
> > > +#include "mode_selection.h"
> > > +#include "class.h"
> > > +#include "bus.h"
> > > +
> > > +static int increment_duplicated_priority(struct device *dev, void *data)
> > > +{
> > > +     struct typec_altmode **alt_target = (struct typec_altmode **)data;
> > > +
> > > +     if (is_typec_altmode(dev)) {
> > > +             struct typec_altmode *alt = to_typec_altmode(dev);
> > > +
> > > +             if (alt != *alt_target && alt->priority == (*alt_target)->priority) {
> > > +                     alt->priority++;
> > > +                     *alt_target = alt;
> > > +                     return 1;
> > > +             }
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +void typec_mode_set_priority(struct typec_altmode *alt,
> > > +             const unsigned int priority)
> > > +{
> > > +     struct typec_port *port = to_typec_port(alt->dev.parent);
> > > +     int res = 1;
> > > +
> > > +     alt->priority = priority;
> > > +
> > > +     while (res)
> > > +             res = device_for_each_child(&port->dev, &alt,
> > > +                             increment_duplicated_priority);
> > > +}
> > > diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
> > > new file mode 100644
> > > index 000000000000..cbf5a37e6404
> > > --- /dev/null
> > > +++ b/drivers/usb/typec/mode_selection.h
> > > @@ -0,0 +1,6 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +
> > > +#include <linux/usb/typec_altmode.h>
> > > +
> > > +void typec_mode_set_priority(struct typec_altmode *alt,
> > > +             const unsigned int priority);
> > > diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> > > index b3c0866ea70f..571c6e00b54f 100644
> > > --- a/include/linux/usb/typec_altmode.h
> > > +++ b/include/linux/usb/typec_altmode.h
> > > @@ -28,6 +28,7 @@ struct typec_altmode {
> > >       int                             mode;
> > >       u32                             vdo;
> > >       unsigned int                    active:1;
> > > +     unsigned int                    priority;
> >
> > What is the range of this?  And this value is only incremented, never
> > decremented?
> >
> 
> The range extends from 0 to UINT_MAX. The value is only incremented.
> The only exception is that If the user sets UINT_MAX for two alternate
> modes in turn, the priority of the first mode becomes 0. This does not
> break the algorithm, and the user can check all priorities via
> ‘priority’ attributes.

Why not use u32 to define a sane range?  Or u8?  How many different
priorities will actually be used in the real world?

And what happens when it wraps?

> I am unsure if a check for this specific case is necessary, as it
> would require examining priorities across all modes, not just a simple
> 'if' statement.
> There are a few ideas in this algorithm:
> - all priorities must always be valid and unique

Is that true?  Where is that validated?

> - no unnecessary restrictions for the user

What does the user actually want to do here?

> - as simple as possible

But not too simple :)

thanks,

greg k-h

On Thu, Sep 11, 2025 at 7:07 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Sep 10, 2025 at 07:35:42PM +0000, Andrei Kuchynski wrote:
> > On Wed, Sep 10, 2025 at 1:31 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Fri, Sep 05, 2025 at 02:22:05PM +0000, Andrei Kuchynski wrote:
> > > > This patch introduces APIs to manage the priority of USB Type-C alternate
> > > > modes. These APIs allow for setting and retrieving a priority number for
> > > > each mode. If a new priority value conflicts with an existing mode's
> > > > priority, the priorities of the conflicting mode and all subsequent modes
> > > > are automatically incremented to ensure uniqueness.
> > > >
> > > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > > ---
> > > >  drivers/usb/typec/Makefile         |  2 +-
> > > >  drivers/usb/typec/mode_selection.c | 38 ++++++++++++++++++++++++++++++
> > > >  drivers/usb/typec/mode_selection.h |  6 +++++
> > > >  include/linux/usb/typec_altmode.h  |  1 +
> > > >  4 files changed, 46 insertions(+), 1 deletion(-)
> > > >  create mode 100644 drivers/usb/typec/mode_selection.c
> > > >  create mode 100644 drivers/usb/typec/mode_selection.h
> > > >
> > > > 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/mode_selection.c b/drivers/usb/typec/mode_selection.c
> > > > new file mode 100644
> > > > index 000000000000..2179bf25f5d4
> > > > --- /dev/null
> > > > +++ b/drivers/usb/typec/mode_selection.c
> > > > @@ -0,0 +1,38 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +/*
> > > > + * Copyright 2025 Google LLC.
> > > > + */
> > > > +
> > > > +#include "mode_selection.h"
> > > > +#include "class.h"
> > > > +#include "bus.h"
> > > > +
> > > > +static int increment_duplicated_priority(struct device *dev, void *data)
> > > > +{
> > > > +     struct typec_altmode **alt_target = (struct typec_altmode **)data;
> > > > +
> > > > +     if (is_typec_altmode(dev)) {
> > > > +             struct typec_altmode *alt = to_typec_altmode(dev);
> > > > +
> > > > +             if (alt != *alt_target && alt->priority == (*alt_target)->priority) {
> > > > +                     alt->priority++;
> > > > +                     *alt_target = alt;
> > > > +                     return 1;
> > > > +             }
> > > > +     }
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +void typec_mode_set_priority(struct typec_altmode *alt,
> > > > +             const unsigned int priority)
> > > > +{
> > > > +     struct typec_port *port = to_typec_port(alt->dev.parent);
> > > > +     int res = 1;
> > > > +
> > > > +     alt->priority = priority;
> > > > +
> > > > +     while (res)
> > > > +             res = device_for_each_child(&port->dev, &alt,
> > > > +                             increment_duplicated_priority);
> > > > +}
> > > > diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
> > > > new file mode 100644
> > > > index 000000000000..cbf5a37e6404
> > > > --- /dev/null
> > > > +++ b/drivers/usb/typec/mode_selection.h
> > > > @@ -0,0 +1,6 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +
> > > > +#include <linux/usb/typec_altmode.h>
> > > > +
> > > > +void typec_mode_set_priority(struct typec_altmode *alt,
> > > > +             const unsigned int priority);
> > > > diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> > > > index b3c0866ea70f..571c6e00b54f 100644
> > > > --- a/include/linux/usb/typec_altmode.h
> > > > +++ b/include/linux/usb/typec_altmode.h
> > > > @@ -28,6 +28,7 @@ struct typec_altmode {
> > > >       int                             mode;
> > > >       u32                             vdo;
> > > >       unsigned int                    active:1;
> > > > +     unsigned int                    priority;
> > >
> > > What is the range of this?  And this value is only incremented, never
> > > decremented?
> > >
> >
> > The range extends from 0 to UINT_MAX. The value is only incremented.
> > The only exception is that If the user sets UINT_MAX for two alternate
> > modes in turn, the priority of the first mode becomes 0. This does not
> > break the algorithm, and the user can check all priorities via
> > ‘priority’ attributes.
>
> Why not use u32 to define a sane range?  Or u8?  How many different
> priorities will actually be used in the real world?
>

The priority can be u32 or u8, but not bool. I use unsigned int as the
precise bit count is not important here.

Three different priorities are enough for DisplayPort, Thunderbolt,
USB4, at least for now. The algorithm is designed to accommodate any
number of modes, as it does not rely on predefined MAX_ALTMODE or
MAX_PRIORITY values.

Priority numbers are used only with the '<' operator.
In fact, we need a list of modes to determine the order of entry.
Previously, a list was used in the 'mode selection' series, but
parsing it is more complex than using kstrtouint().

> And what happens when it wraps?
>

If a user sets all priorities (either 2 or 3) to UINT_MAX, the
resulting mode sequence will not be as expected due to overflow.

> > I am unsure if a check for this specific case is necessary, as it
> > would require examining priorities across all modes, not just a simple
> > 'if' statement.
> > There are a few ideas in this algorithm:
> > - all priorities must always be valid and unique
>
> Is that true?  Where is that validated?
>

typec_mode_set_priority() ensures all priorities remain unique. Any
numerical value is valid as long as it can be compared to another,
kstrtouint() will return this value.

> > - no unnecessary restrictions for the user
>
> What does the user actually want to do here?
>

User wants to set the highest priority mode. For this, a single '0'
write operation is required. Also user can prioritize all modes by
assigning '0' to the 'priority' attribute for each mode, beginning
with the lowest prioritized mode and moving upwards.

For example, USB4 is the preferred mode, with priority 0. If a partner
(or port) device does not support USB4, the user can set the next
preferred mode (TBT or DP) based on security policy by assigning
priority 1 for only one mode. This mode is TBT when a user is logged
in, and DP otherwise.

> > - as simple as possible
>
> But not too simple :)
>
> thanks,
>
> greg k-h

Thanks,
Andrei

On Thu, Sep 11, 2025 at 10:46:52AM +0200, Andrei Kuchynski wrote:
> On Thu, Sep 11, 2025 at 7:07 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Sep 10, 2025 at 07:35:42PM +0000, Andrei Kuchynski wrote:
> > > On Wed, Sep 10, 2025 at 1:31 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Fri, Sep 05, 2025 at 02:22:05PM +0000, Andrei Kuchynski wrote:
> > > > > This patch introduces APIs to manage the priority of USB Type-C alternate
> > > > > modes. These APIs allow for setting and retrieving a priority number for
> > > > > each mode. If a new priority value conflicts with an existing mode's
> > > > > priority, the priorities of the conflicting mode and all subsequent modes
> > > > > are automatically incremented to ensure uniqueness.
> > > > >
> > > > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > > > ---
> > > > >  drivers/usb/typec/Makefile         |  2 +-
> > > > >  drivers/usb/typec/mode_selection.c | 38 ++++++++++++++++++++++++++++++
> > > > >  drivers/usb/typec/mode_selection.h |  6 +++++
> > > > >  include/linux/usb/typec_altmode.h  |  1 +
> > > > >  4 files changed, 46 insertions(+), 1 deletion(-)
> > > > >  create mode 100644 drivers/usb/typec/mode_selection.c
> > > > >  create mode 100644 drivers/usb/typec/mode_selection.h
> > > > >
> > > > > 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/mode_selection.c b/drivers/usb/typec/mode_selection.c
> > > > > new file mode 100644
> > > > > index 000000000000..2179bf25f5d4
> > > > > --- /dev/null
> > > > > +++ b/drivers/usb/typec/mode_selection.c
> > > > > @@ -0,0 +1,38 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > +/*
> > > > > + * Copyright 2025 Google LLC.
> > > > > + */
> > > > > +
> > > > > +#include "mode_selection.h"
> > > > > +#include "class.h"
> > > > > +#include "bus.h"
> > > > > +
> > > > > +static int increment_duplicated_priority(struct device *dev, void *data)
> > > > > +{
> > > > > +     struct typec_altmode **alt_target = (struct typec_altmode **)data;
> > > > > +
> > > > > +     if (is_typec_altmode(dev)) {
> > > > > +             struct typec_altmode *alt = to_typec_altmode(dev);
> > > > > +
> > > > > +             if (alt != *alt_target && alt->priority == (*alt_target)->priority) {
> > > > > +                     alt->priority++;
> > > > > +                     *alt_target = alt;
> > > > > +                     return 1;
> > > > > +             }
> > > > > +     }
> > > > > +
> > > > > +     return 0;
> > > > > +}
> > > > > +
> > > > > +void typec_mode_set_priority(struct typec_altmode *alt,
> > > > > +             const unsigned int priority)
> > > > > +{
> > > > > +     struct typec_port *port = to_typec_port(alt->dev.parent);
> > > > > +     int res = 1;
> > > > > +
> > > > > +     alt->priority = priority;
> > > > > +
> > > > > +     while (res)
> > > > > +             res = device_for_each_child(&port->dev, &alt,
> > > > > +                             increment_duplicated_priority);
> > > > > +}
> > > > > diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
> > > > > new file mode 100644
> > > > > index 000000000000..cbf5a37e6404
> > > > > --- /dev/null
> > > > > +++ b/drivers/usb/typec/mode_selection.h
> > > > > @@ -0,0 +1,6 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > > +
> > > > > +#include <linux/usb/typec_altmode.h>
> > > > > +
> > > > > +void typec_mode_set_priority(struct typec_altmode *alt,
> > > > > +             const unsigned int priority);
> > > > > diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> > > > > index b3c0866ea70f..571c6e00b54f 100644
> > > > > --- a/include/linux/usb/typec_altmode.h
> > > > > +++ b/include/linux/usb/typec_altmode.h
> > > > > @@ -28,6 +28,7 @@ struct typec_altmode {
> > > > >       int                             mode;
> > > > >       u32                             vdo;
> > > > >       unsigned int                    active:1;
> > > > > +     unsigned int                    priority;
> > > >
> > > > What is the range of this?  And this value is only incremented, never
> > > > decremented?
> > > >
> > >
> > > The range extends from 0 to UINT_MAX. The value is only incremented.
> > > The only exception is that If the user sets UINT_MAX for two alternate
> > > modes in turn, the priority of the first mode becomes 0. This does not
> > > break the algorithm, and the user can check all priorities via
> > > ‘priority’ attributes.
> >
> > Why not use u32 to define a sane range?  Or u8?  How many different
> > priorities will actually be used in the real world?
> >
> 
> The priority can be u32 or u8, but not bool. I use unsigned int as the
> precise bit count is not important here.

But the range is, that needs to be documented.

> Three different priorities are enough for DisplayPort, Thunderbolt,
> USB4, at least for now. The algorithm is designed to accommodate any
> number of modes, as it does not rely on predefined MAX_ALTMODE or
> MAX_PRIORITY values.

Great, then let's use u8 for now.

> > And what happens when it wraps?
> >
> 
> If a user sets all priorities (either 2 or 3) to UINT_MAX, the
> resulting mode sequence will not be as expected due to overflow.

That sounds like a bug :(

thanks,

greg k-h

On Thu, Sep 11, 2025 at 12:41 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Sep 11, 2025 at 10:46:52AM +0200, Andrei Kuchynski wrote:
> > On Thu, Sep 11, 2025 at 7:07 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Wed, Sep 10, 2025 at 07:35:42PM +0000, Andrei Kuchynski wrote:
> > > > On Wed, Sep 10, 2025 at 1:31 PM Greg Kroah-Hartman
> > > > <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > On Fri, Sep 05, 2025 at 02:22:05PM +0000, Andrei Kuchynski wrote:
> > > > > > This patch introduces APIs to manage the priority of USB Type-C alternate
> > > > > > modes. These APIs allow for setting and retrieving a priority number for
> > > > > > each mode. If a new priority value conflicts with an existing mode's
> > > > > > priority, the priorities of the conflicting mode and all subsequent modes
> > > > > > are automatically incremented to ensure uniqueness.
> > > > > >
> > > > > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > > > > ---
> > > > > >  drivers/usb/typec/Makefile         |  2 +-
> > > > > >  drivers/usb/typec/mode_selection.c | 38 ++++++++++++++++++++++++++++++
> > > > > >  drivers/usb/typec/mode_selection.h |  6 +++++
> > > > > >  include/linux/usb/typec_altmode.h  |  1 +
> > > > > >  4 files changed, 46 insertions(+), 1 deletion(-)
> > > > > >  create mode 100644 drivers/usb/typec/mode_selection.c
> > > > > >  create mode 100644 drivers/usb/typec/mode_selection.h
> > > > > >
> > > > > > 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/mode_selection.c b/drivers/usb/typec/mode_selection.c
> > > > > > new file mode 100644
> > > > > > index 000000000000..2179bf25f5d4
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/usb/typec/mode_selection.c
> > > > > > @@ -0,0 +1,38 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > > +/*
> > > > > > + * Copyright 2025 Google LLC.
> > > > > > + */
> > > > > > +
> > > > > > +#include "mode_selection.h"
> > > > > > +#include "class.h"
> > > > > > +#include "bus.h"
> > > > > > +
> > > > > > +static int increment_duplicated_priority(struct device *dev, void *data)
> > > > > > +{
> > > > > > +     struct typec_altmode **alt_target = (struct typec_altmode **)data;
> > > > > > +
> > > > > > +     if (is_typec_altmode(dev)) {
> > > > > > +             struct typec_altmode *alt = to_typec_altmode(dev);
> > > > > > +
> > > > > > +             if (alt != *alt_target && alt->priority == (*alt_target)->priority) {
> > > > > > +                     alt->priority++;
> > > > > > +                     *alt_target = alt;
> > > > > > +                     return 1;
> > > > > > +             }
> > > > > > +     }
> > > > > > +
> > > > > > +     return 0;
> > > > > > +}
> > > > > > +
> > > > > > +void typec_mode_set_priority(struct typec_altmode *alt,
> > > > > > +             const unsigned int priority)
> > > > > > +{
> > > > > > +     struct typec_port *port = to_typec_port(alt->dev.parent);
> > > > > > +     int res = 1;
> > > > > > +
> > > > > > +     alt->priority = priority;
> > > > > > +
> > > > > > +     while (res)
> > > > > > +             res = device_for_each_child(&port->dev, &alt,
> > > > > > +                             increment_duplicated_priority);
> > > > > > +}
> > > > > > diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
> > > > > > new file mode 100644
> > > > > > index 000000000000..cbf5a37e6404
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/usb/typec/mode_selection.h
> > > > > > @@ -0,0 +1,6 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > > > +
> > > > > > +#include <linux/usb/typec_altmode.h>
> > > > > > +
> > > > > > +void typec_mode_set_priority(struct typec_altmode *alt,
> > > > > > +             const unsigned int priority);
> > > > > > diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> > > > > > index b3c0866ea70f..571c6e00b54f 100644
> > > > > > --- a/include/linux/usb/typec_altmode.h
> > > > > > +++ b/include/linux/usb/typec_altmode.h
> > > > > > @@ -28,6 +28,7 @@ struct typec_altmode {
> > > > > >       int                             mode;
> > > > > >       u32                             vdo;
> > > > > >       unsigned int                    active:1;
> > > > > > +     unsigned int                    priority;
> > > > >
> > > > > What is the range of this?  And this value is only incremented, never
> > > > > decremented?
> > > > >
> > > >
> > > > The range extends from 0 to UINT_MAX. The value is only incremented.
> > > > The only exception is that If the user sets UINT_MAX for two alternate
> > > > modes in turn, the priority of the first mode becomes 0. This does not
> > > > break the algorithm, and the user can check all priorities via
> > > > ‘priority’ attributes.
> > >
> > > Why not use u32 to define a sane range?  Or u8?  How many different
> > > priorities will actually be used in the real world?
> > >
> >
> > The priority can be u32 or u8, but not bool. I use unsigned int as the
> > precise bit count is not important here.
>
> But the range is, that needs to be documented.
>

The priority range will be documented in the ABI

> > Three different priorities are enough for DisplayPort, Thunderbolt,
> > USB4, at least for now. The algorithm is designed to accommodate any
> > number of modes, as it does not rely on predefined MAX_ALTMODE or
> > MAX_PRIORITY values.
>
> Great, then let's use u8 for now.
>

I will change the priority type from unsigned int to u8.

> > > And what happens when it wraps?
> > >
> >
> > If a user sets all priorities (either 2 or 3) to UINT_MAX, the
> > resulting mode sequence will not be as expected due to overflow.
>
> That sounds like a bug :(
>

I will add a check to the function and return -EOVERFLOW if the input
causes the priority values to wrap.

Greg, thank you for the review!

Andrei

On Fri, Sep 05, 2025 at 02:22:05PM +0000, Andrei Kuchynski wrote:
> This patch introduces APIs to manage the priority of USB Type-C alternate
> modes. These APIs allow for setting and retrieving a priority number for
> each mode. If a new priority value conflicts with an existing mode's
> priority, the priorities of the conflicting mode and all subsequent modes
> are automatically incremented to ensure uniqueness.
> 
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/Makefile         |  2 +-
>  drivers/usb/typec/mode_selection.c | 38 ++++++++++++++++++++++++++++++
>  drivers/usb/typec/mode_selection.h |  6 +++++
>  include/linux/usb/typec_altmode.h  |  1 +
>  4 files changed, 46 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/usb/typec/mode_selection.c
>  create mode 100644 drivers/usb/typec/mode_selection.h
> 
> 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/mode_selection.c b/drivers/usb/typec/mode_selection.c
> new file mode 100644
> index 000000000000..2179bf25f5d4
> --- /dev/null
> +++ b/drivers/usb/typec/mode_selection.c
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2025 Google LLC.
> + */
> +
> +#include "mode_selection.h"
> +#include "class.h"
> +#include "bus.h"
> +
> +static int increment_duplicated_priority(struct device *dev, void *data)
> +{
> +	struct typec_altmode **alt_target = (struct typec_altmode **)data;
> +
> +	if (is_typec_altmode(dev)) {
> +		struct typec_altmode *alt = to_typec_altmode(dev);
> +
> +		if (alt != *alt_target && alt->priority == (*alt_target)->priority) {
> +			alt->priority++;
> +			*alt_target = alt;
> +			return 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +void typec_mode_set_priority(struct typec_altmode *alt,
> +		const unsigned int priority)
> +{
> +	struct typec_port *port = to_typec_port(alt->dev.parent);
> +	int res = 1;
> +
> +	alt->priority = priority;
> +
> +	while (res)
> +		res = device_for_each_child(&port->dev, &alt,
> +				increment_duplicated_priority);
> +}
> diff --git a/drivers/usb/typec/mode_selection.h b/drivers/usb/typec/mode_selection.h
> new file mode 100644
> index 000000000000..cbf5a37e6404
> --- /dev/null
> +++ b/drivers/usb/typec/mode_selection.h
> @@ -0,0 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <linux/usb/typec_altmode.h>
> +
> +void typec_mode_set_priority(struct typec_altmode *alt,
> +		const unsigned int priority);
> diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> index b3c0866ea70f..571c6e00b54f 100644
> --- a/include/linux/usb/typec_altmode.h
> +++ b/include/linux/usb/typec_altmode.h
> @@ -28,6 +28,7 @@ struct typec_altmode {
>  	int				mode;
>  	u32				vdo;
>  	unsigned int			active:1;
> +	unsigned int			priority;
>  
>  	char				*desc;
>  	const struct typec_altmode_ops	*ops;
> -- 
> 2.51.0.355.g5224444f11-goog

-- 
heikki