Message-ID: <CANFp7mWk_TuA6Gxbtc8OmB7eq_vT8wUg=xkPJsxLCBTrQwOd6A@mail.gmail.com>
Date: Wed, 17 Sep 2025 10:51:11 -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>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC 0/5] USB Type-C alternate mode selection

On Wed, Sep 17, 2025 at 5:28 AM Heikki Krogerus
<heikki.krogerus@...ux.intel.com> wrote:
>
> On Tue, Sep 16, 2025 at 09:47:44AM -0700, Abhishek Pandit-Subedi wrote:
> > On Tue, Sep 16, 2025 at 6:12 AM Heikki Krogerus
> > <heikki.krogerus@...ux.intel.com> wrote:
> > >
> > > On Tue, Sep 09, 2025 at 12:30:23PM +0000, Andrei Kuchynski wrote:
> > > > This patch series introduces a flexible mechanism for USB Type-C mode
> > > > selection, enabling into USB4 mode, Thunderbolt alternate mode, or
> > > > DisplayPort alternate mode.
> > > >
> > > > New sysfs `mode_selection` attribute is exposed to provide user control
> > > > over mode selection. It triggers an alternate mode negotiation.
> > > > The mode selection logic attempts to enter prioritized modes sequentially.
> > > > A mode is considered successfully negotiated only when its alternate mode
> > > > driver explicitly reports a positive status. Alternate mode drivers are
> > > > required to report their mode entry status (either successful or failed).
> > > > If the driver does not report its status within a defined timeout period,
> > > > the system automatically proceeds to attempt entry into the next preferred
> > > > mode.
> > >
> > > I'm still struggling to understand what is the benefit from this - why
> > > would you want the user space to explicitly start the entry process
> > > like this? Instead why would you not just take full control over the
> > > alt modes in user space by enabling the them one by one in what ever
> > > order you want?
> >
> > I think after the many patch iterations we went through upstreaming,
> > we may have lost the point a little bit wrt/ the mode selection task.
> > We talked about this on the very first iteration of this patchset
> > here: https://lore.kernel.org/linux-usb/CANFp7mVWo4GhiYqfLcD_wFV34WMkmXncMTOnmMfnKH4vm2X8Hg@mail.gmail.com/
> >
> > The motivation behind it was to allow the kernel driver to own mode
> > selection entirely and not need user space intervention. The current
> > alt-mode drivers attempt to own the mode entry process and this fails
> > when you have two or more altmode drivers loaded (i.e. displayport,
> > thunderbolt). The original goal of the mode selection task was to move
> > the mode entry decision away from the alt-mode driver and to the port
> > driver instead.
> >
> > What's missing in the current patch series to show this is probably
> > actually calling mode_selection once all partner modes are added :)
> > Andrei should be adding that to this patch series in the next patch
> > version.
> >
> > Adding the mode_selection sysfs trigger is for another reason: to
> > re-run mode selection after priorities have been changed in userspace
> > and there is no partner hotplug. We specifically have some security
> > policies around PCI tunnels that result in the following need:
> > * When we enable pci tunneling, we PREFER tbt over dp and would like
> > to select the preferred mode. When we disable it, we PREFER dp over
> > TBT and would like to select the preferred mode.
> > * When users are logged out, we always prefer DP over TBT.
> > * When the system is locked, we prefer DP over TBT for new connections
> > (but existing connections can be left as TBT). When we unlock, we want
> > to enter the most preferred mode (TBT > DP).
> >
> > While this is do-able with the alt-mode active sysfs field, we would
> > basically be re-creating the priority selection done in the kernel in
> > user space again. Hence why we want to expose the mode selection
> > trigger as done here.
>
> But this would be a step backwards. You want to keep the kernel in
> control of the mode selection, which is fine, but then you have these
> special cases where you have to give some of the control to the user
> space. So instead of taking complete control of the mode selection in
> user space, you want to create this partial control method of
> supporting your special cases while still leaving "most" of the
> control to kernel.
>
> I don't believe this will work in all cases. I'm fine with the
> priority as a way to tell the kernel the preferred entry order, but if
> the user space needs to take control of the actual mode selection, it
> has to take full control of it instead of like this, partially. This
> just looks incredibly fragile.
>
> So I'm still not convinced that there is any use for this. Either the
> user space takes over the mode selection completely with the active
> attribute files, or just leaves the selection completely to the kernel.
>

That's a fair stance to take. We CAN do our special cases via the
"active" sysfs node. I've had a bit more time to think about the
problem we are solving and I'd like to elaborate a little.

When we designed this mode selection task, there were two motivating factors:
* The existing typec_displayport and typec_thunderbolt modules will
both automatically try to enter a mode when probing and that does not
work well. You want a deterministic entry order.
* There is no generic typec daemon for userspace on Linux and there
isn't always a need for one (i.e. UCSI systems). The kernel has the
most information about what any given system needs and should be able
to handle mode entry timing better.

For those motivating factors, I think an in-kernel mode selection as
designed in this series still makes sense. Let the kernel do the mode
selection, inform userspace when it is completed and userspace can
simply set priorities + report success/failure/errors.
One other change we will probably want to make is to turn the partner
altmode Kconfig options to boolean and roll it into the typec module.
Alt-mode module loading breaks mode selection ordering because you
can't be guaranteed the partner altmodes are loaded on the system when
you do partner altmode enumeration.

Heikki, can you confirm we are on the same page up till this point at
least? The net effect here is we are moving partner altmodes
individually entering modes to centralizing mode entry in the typec
class itself.

Also, with respect to dropping the mode_selection sysfs node and
simply using the `active` fields to override:
* How can we ensure that user space does not race with the kernel mode entry?
* Should we delay exposing "number_of_alternate_modes" until after
mode selection is done? Should we keep the mode_selection sysfs (or a
similarly named file) as a read-only indicator of current status?

Thanks,
Abhishek

> Br,
>
> > > I don't believe you can make this approach scale much if and when in
> > > the future the use cases change. Right now I don't feel comfortable
> > > with this at all.
> > >
> > > thanks,
> > >
> > > > This series was tested on an Android OS device with kernel 6.16.
> > > > This series depends on the 'USB Type-C alternate mode priorities' series:
> > > > https://lore.kernel.org/all/20250905142206.4105351-1-akuchynski@chromium.org/
> > > >
> > > > Andrei Kuchynski (5):
> > > >   usb: typec: Implement mode selection
> > > >   usb: typec: Expose mode_selection attribute via sysfs
> > > >   usb: typec: Report altmode entry status via callback
> > > >   usb: typec: ucsi: displayport: Propagate DP altmode entry result
> > > >   platform/chrome: cros_ec_typec: Propagate altmode entry result
> > > >
> > > >  Documentation/ABI/testing/sysfs-class-typec  |  11 +
> > > >  drivers/platform/chrome/cros_ec_typec.c      |   9 +
> > > >  drivers/platform/chrome/cros_typec_altmode.c |  32 +-
> > > >  drivers/platform/chrome/cros_typec_altmode.h |   6 +
> > > >  drivers/usb/typec/altmodes/displayport.c     |  19 +-
> > > >  drivers/usb/typec/altmodes/thunderbolt.c     |  10 +
> > > >  drivers/usb/typec/class.c                    |  37 ++
> > > >  drivers/usb/typec/class.h                    |   4 +
> > > >  drivers/usb/typec/mode_selection.c           | 345 +++++++++++++++++++
> > > >  drivers/usb/typec/mode_selection.h           |  25 ++
> > > >  drivers/usb/typec/ucsi/displayport.c         |  10 +-
> > > >  include/linux/usb/typec_altmode.h            |  11 +
> > > >  include/linux/usb/typec_dp.h                 |   2 +
> > > >  include/linux/usb/typec_tbt.h                |   3 +
> > > >  14 files changed, 516 insertions(+), 8 deletions(-)
> > > >
> > > > --
> > > > 2.51.0.384.g4c02a37b29-goog
>
> --
> heikki

Powered by blists - more mailing lists