Message-ID: <CANFp7mVWo4GhiYqfLcD_wFV34WMkmXncMTOnmMfnKH4vm2X8Hg@mail.gmail.com>
Date: Tue, 17 Jun 2025 19:47:18 -0700
From: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Andrei Kuchynski <akuchynski@...omium.org>, 
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Dmitry Baryshkov <lumag@...nel.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 6:28 AM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> 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.

USB4 should be called an alt-mode though based on its behavior and I
think the spec makes a mistake not doing so. It's mutually exclusive
with alternate modes, it modifies the functionality of the data lanes
and it's not a new data mode (just a tunneling protocol). But that's
an aside for this patch...

> > >
> > > 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.

Userspace "can" do it but it is not doing it today. There is no
userspace alt-mode manager (that I'm aware of) that's writing to
/sys/class/typec (typecd in ChromeOS only reads and does mode
entry/exit via a sideband mechanism) and boltd (userspace thunderbolt
manager; https://github.com/gicmo/bolt) only deals with tunnels and
not mode entry directly.

I think the inclusion of TBT and USB4 in the kernel makes this change
necessary and the existing way of doing it is not widely used yet.

>
> > 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?

There were some previous discussions on this:
https://lore.kernel.org/linux-usb/ZyjW0CMXgGIt-usC@kuha.fi.intel.com/
https://lore.kernel.org/chrome-platform/CAA8EJpqiF_0bgYT8boFa4UPJWcxgw89mmfbdMVKeAP-xnFOP4g@mail.gmail.com/

Some background:
* The Type-C port driver will add a partner alt-mode to the altmode bus.
* An altmode driver will probe and attach if available on the system.
    * Currently, the probe function of these altmode drivers always
enter the given mode. The expectation is that if a system allows mode
override, it should always be done by the alt mode driver. This
doesn't scale beyond one alt-mode driver – between DP and TBT, which
altmode driver should win entry? Prior to TBT driver being added in
December and USB4 support circa similar time frame, DP was always the
default choice if override was supported.
    * Userspace could make this decision but then what about USB4? The
current APIs don't expose USB4 information in the same way and the
auto-enter behavior is different (it doesn't enter).

In a lot of existing designs, mode entry is entirely handled by the
firmware and no mode selection is possible from the kernel. This is
covered by patch 3 of this series (alt_mode_override) flag. Windows
designs usually fall into this category as they use PD controllers
implementing UCSI (and usually a very old UCSI version 1.2).

A few designs allow overriding the current mode:
* Chromebooks with cros_ec_ucsi which support Thunderbolt will allow
override of the alternate mode (via UCSI set_new_cam) and USB mode
(via UCSI Set_USB). They will enter DPAM instead of TBT by default if
possible.
* Chromebooks with the cros_ec_typec driver which support Thunderbolt
will require the OS to make the mode entry decision (no mode entry by
default on attach).
* Pixel phone TCPM currently requires mode entry from the kernel as
there's no userspace mode selection component.
* Potentially some Windows designs that implement AltMode override via
UCSI (ucsi_acpi). These should behave similarly to cros_ec_ucsi but
will likely automatically enter TBT mode.

In these cases, the kernel is a better place to make alternate mode decisions:
* Quicker than having a userspace implementation (and more consistent
during boot).
* Works consistently across drivers and accounts for both overridable
and non-overridable systems.

The main thing userspace should really be doing with mode entry is
setting policy around what modes they want to enter instead of doing
low level checking of compatibility and sequencing of mode entry
operations. And the main policy we're trying to enforce on ChromeOS is
to not enter TBT by default (based on our security posture described
at https://www.chromium.org/chromium-os/developer-library/reference/security/usb4/).

Given that Thunderbolt altmode driver support and USB4 support was
only added around December of last year, I don't imagine there are
many userspace usages yet and we should fix these API issues before
they become too widely used. I think mode sequencing belongs in the
kernel and not userspace.


>
> > > 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?

While the current sysfs apis for altmode + usb are useful for testing,
they aren't as useful for normal user use-cases. Deprecating or moving
this control to debugfs and only exposing the
mode_selection/mode_priority fields would be my choice going forward.
The mode selection work currently does go through the same typec APIs
as the sysfs entries so we gain test coverage of driver behavior using
either API. If we need to retain the old APIs, I think we'll need to
add some Kunit tests to make sure behavior is consistent across the
two ways of doing things.

>
> thanks,
>
> greg k-h

Powered by blists - more mailing lists