linux-kernel - Re: [PATCH v3 04/10] usb: typec: Expose mode priorities via sysfs

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: <CANFp7mW92PgjSWyJq7Bz6ZLJ8ZgnsCRw2kAYAjKX3yymKW9hBA@mail.gmail.com>
Date: Tue, 12 Aug 2025 13:33:58 -0700
From: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
To: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc: Andrei Kuchynski <akuchynski@...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>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.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 04/10] usb: typec: Expose mode priorities via sysfs

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

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

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

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

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

>
> thanks,
>
> --
> heikki