Message-ID: <2025061727-crinkle-drew-4a0d@gregkh>
Date: Tue, 17 Jun 2025 15:28:07 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Andrei Kuchynski <akuchynski@...omium.org>
Cc: Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Dmitry Baryshkov <lumag@...nel.org>,
	Abhishek Pandit-Subedi <abhishekpandit@...omium.org>,
	Jameson Thies <jthies@...gle.com>,
	Benson Leung <bleung@...omium.org>,
	Tzung-Bi Shih <tzungbi@...nel.org>,
	Guenter Roeck <groeck@...omium.org>,
	Pooja Katiyar <pooja.katiyar@...el.com>,
	Badhri Jagan Sridharan <badhri@...gle.com>,
	RD Babiera <rdbabiera@...gle.com>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, chrome-platform@...ts.linux.dev
Subject: Re: [PATCH 04/10] usb: typec: Expose alternate mode priorities via
 sysfs

On Tue, Jun 17, 2025 at 02:38:04PM +0200, Andrei Kuchynski wrote:
> On Tue, Jun 17, 2025 at 11:50 AM Heikki Krogerus
> <heikki.krogerus@...ux.intel.com> wrote:
> >
> > On Mon, Jun 16, 2025 at 01:31:41PM +0000, Andrei Kuchynski wrote:
> > > This sysfs attribute specifies the preferred order for enabling
> > > DisplayPort alt-mode, Thunderbolt alt-mode, and USB4 mode.
> > >
> > > Signed-off-by: Andrei Kuchynski <akuchynski@...omium.org>
> > > ---
> > >  Documentation/ABI/testing/sysfs-class-typec | 17 ++++
> > >  drivers/usb/typec/Makefile                  |  2 +-
> > >  drivers/usb/typec/class.c                   | 26 ++++++
> > >  drivers/usb/typec/class.h                   |  2 +
> > >  drivers/usb/typec/mode_selection.c          | 93 +++++++++++++++++++++
> > >  drivers/usb/typec/mode_selection.h          |  5 ++
> > >  include/linux/usb/typec_altmode.h           |  7 ++
> > >  7 files changed, 151 insertions(+), 1 deletion(-)
> > >  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..46eee82042ab 100644
> > > --- a/Documentation/ABI/testing/sysfs-class-typec
> > > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > > @@ -162,6 +162,23 @@ Description:     Lists the supported USB Modes. The default USB mode that is used
> > >               - usb3 (USB 3.2)
> > >               - usb4 (USB4)
> > >
> > > +What:                /sys/class/typec/<port>/altmode_priorities
> > > +Date:                June 2025
> > > +Contact:     Andrei Kuchynski <akuchynski@...omium.org>
> > > +Description: Lists the alternate modes supported by the port and their
> > > +             priorities. The priority setting determines the order in which
> > > +             Displayport alt-mode, Thunderbolt alt-mode and USB4 mode will be
> > > +             activated, indicating the preferred selection sequence. A value of -1
> > > +             disables automatic entry into a specific mode, while lower numbers
> > > +             indicate higher priority. The default priorities can be modified by
> > > +             assigning new values. Modes without explicitly set values default to -1,
> > > +             effectively disabling them.
> > > +
> > > +             Example values:
> > > +             - "USB4=0 TBT=1 DP=2"
> > > +             - "USB4=-1 TBT=0"
> > > +             - "DP=-1 USB4=-1 TBT=-1"
> >
> > No. If you want to disable entry to a mode by default, then you
> > deactivate that mode, so -1 is not needed. USB4 is also not an alt
> > mode, so this at the very least should be named differently.
> >
> > But I'm not sure this is the correct way to handle the modes in
> > general. Can you please explain what exactly is the use case you are
> > thinking?
> 
> Hi Heikki,
> 
> This implements the mode selection logic within the kernel, replacing
> its userspace counterpart. Implementing this in the kernel offers
> advantages, making the process both more robust and far easier to
> manage.
> This eliminates the need for user space to verify port, partner, or
> cable capabilities, and to control the mode entry process. User space
> doesn't even need to know if USB4 mode is supported by the port or
> partner; unsupported modes are automatically skipped.

But that's all things that userspace can do, so it should be doing it.
You need to justify why userspace can NOT do something in order to move
that logic into the kernel.

> User space's role is now limited to high-level tasks like security,
> with the kernel managing technical implementation. Mode selection and
> activation can occur without user space intervention at all if no
> special rules apply and the default policy (USB4 -> TBT -> DP) is
> acceptable.

What is wrong with "userspace intervention"?  Is the current api broken
somehow?

> > If you just need to prevent for example USB4 entry by default, then
> > you write usb3 (or usb2) to the usb_capability. The alt modes you can
> > deactivate by default, no?
> >
> > I can appreciate the convenience that you get from a single file like
> > that, but it does not really give much room to move if for example the
> > user space needs to behave differently in case of failures to enter a
> > specific mode compared to successful entries.
> >
> 
> You are right, this patch series is proposed for its convenience. It
> offers a more convenient method for users to enable or activate any
> mode, without altering existing methods. Users can decide whether they
> prioritize more control or an easier mode selection method.

So now you are going to maintain 2 different ways to do this for the
next 40+ years?  How are you going to test this over time to make sure
nothing breaks/changes?

thanks,

greg k-h

Powered by blists - more mailing lists