Message-ID: <CANFp7mVwZksBYBOgPLDDYiynjFDh3jJqY1PFvvBWxtFd8MMYjw@mail.gmail.com>
Date: Wed, 17 Sep 2025 17:09:44 -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 10:51 AM Abhishek Pandit-Subedi
<abhishekpandit@...omium.org> wrote:
>
> 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.

After actually trying to do this today, I think a better approach
might be to just add a MODULE_SOFTDEP for all the typec altmodes. That
way, all the modules get loaded together and there's less of a chance
of waiting for the altmode driver to load when enumerating partners on
boot.

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