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

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

Hi Abhishek,

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

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

Sorry if I missed something.

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

This first part I understand.

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

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

thanks,

-- 
heikki