linux-kernel - Re: [PATCH v3 4/5] usb: typec: Implement alternate mode priority handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMMMRMdOiGQdmgQ7COa1Nf9Rh9BQLD79TJ77VW4ejadx_bNyeg@mail.gmail.com>
Date: Wed, 10 Sep 2025 19:35:42 +0000
From: Andrei Kuchynski <akuchynski@...omium.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@...ux.intel.com>, 
	Abhishek Pandit-Subedi <abhishekpandit@...omium.org>, Benson Leung <bleung@...omium.org>, 
	Jameson Thies <jthies@...gle.com>, Tzung-Bi Shih <tzungbi@...nel.org>, linux-usb@...r.kernel.org, 
	chrome-platform@...ts.linux.dev, Guenter Roeck <groeck@...omium.org>, 
	Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>, "Christian A. Ehrhardt" <lk@...e.de>, 
	Venkat Jayaraman <venkat.jayaraman@...el.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/5] usb: typec: Implement alternate mode priority handling

On Wed, Sep 10, 2025 at 1:31 PM Greg Kroah-Hartman
<gregkh@...uxfoundation.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@...omium.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