linux-kernel - Re: [PATCH v1 5/5] usb: typec: Expose alternate mode priority via sysfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMMMRMd4Gw+yC+4wB=_ZRChuE1=TEZXePYZa5PoqmLSiGDJCRQ@mail.gmail.com>
Date: Thu, 21 Aug 2025 16:44:45 +0200
From: Andrei Kuchynski <akuchynski@...omium.org>
To: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc: Abhishek Pandit-Subedi <abhishekpandit@...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>, 
	Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>, "Christian A. Ehrhardt" <lk@...e.de>, 
	Venkat Jayaraman <venkat.jayaraman@...el.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 5/5] usb: typec: Expose alternate mode priority via sysfs

On Thu, Aug 21, 2025 at 9:36 AM Heikki Krogerus
<heikki.krogerus@...ux.intel.com> wrote:
>
> Hi Andrei,
>
> On Thu, Aug 14, 2025 at 06:44:55PM +0000, Andrei Kuchynski wrote:
> > This patch introduces a priority sysfs attribute to the USB Type-C
> > alternate mode port interface. This new attribute allows user-space to
> > configure the numeric priority of alternate modes managing their preferred
> > order of operation.
> >
> > Signed-off-by: Andrei Kuchynski <akuchynski@...omium.org>
> > ---
> >  Documentation/ABI/testing/sysfs-class-typec | 12 ++++++
> >  drivers/usb/typec/class.c                   | 47 ++++++++++++++++++++-
> >  2 files changed, 58 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index 38e101c17a00..001202d651fa 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -162,6 +162,18 @@ 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@...omium.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.
>
> I was expecting this to be already used in this series.
>
> IMO this file should be the only thing the user space needs to use by
> default at least.
>

No more entries for mode priorities.
What about the trigger of the process?
I'm going to introduce a mode_selection binary sysfs attribute in
the next series. Writing 1 to this attribute will activate the
mode selection process. Writing 0 will cancel the process and
exit the currently active mode.

> > @@ -469,6 +500,15 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
> >                               !adev->ops->activate)
> >                               return 0444;
> >               }
> > +     } else if (attr == &dev_attr_priority.attr) {
> > +             if (is_typec_port(adev->dev.parent))  {
> > +                     struct typec_port *port = to_typec_port(adev->dev.parent);
> > +
> > +                     if (!port->alt_mode_override)
> > +                             return 0;
> > +             } else
> > +                     return 0;
> > +     }
>
> If we have the local port variable, this should be enough:
>
>                 if (!is_typec_port(adev->dev.parent) || !port->alt_mode_override)
>                         return 0;
>

typec_altmode2port is really powerful. Thank you again for this hint!

> >       return attr->mode;
> >  }
> > @@ -2029,6 +2069,7 @@ static void typec_release(struct device *dev)
> >       typec_mux_put(port->mux);
> >       typec_retimer_put(port->retimer);
> >       kfree(port->cap);
> > +     typec_mode_selection_destroy(port);
> >       kfree(port);
> >  }
> >
> > @@ -2496,6 +2537,8 @@ typec_port_register_altmode(struct typec_port *port,
> >               to_altmode(adev)->retimer = retimer;
> >       }
> >
> > +     typec_mode_set_priority(adev, -1);
>
> This really should not be necessary. Why can't we set the priority
> based on the order the drives registers the altmodes for the port?
>

Setting priorities based on the order of registration is a good
idea. This simplifies the logic by making the default_priorities
unnecessary.
I will modify the initialization to call

          typec_mode_set_priority(adev, 0);

With this change, the later a mode is registered, the higher its
priority will be. For the cros_ec_typec driver, this produces the
same default sequence: USB4 TBT DP.

Thanks,

Andrei



> >       return adev;
> >  }
> >  EXPORT_SYMBOL_GPL(typec_port_register_altmode);
> > @@ -2645,6 +2688,8 @@ struct typec_port *typec_register_port(struct device *parent,
> >       port->con.attach = typec_partner_attach;
> >       port->con.deattach = typec_partner_deattach;
> >
> > +     INIT_LIST_HEAD(&port->mode_list);
> > +
> >       if (cap->usb_capability & USB_CAPABILITY_USB4)
> >               port->usb_mode = USB_MODE_USB4;
> >       else if (cap->usb_capability & USB_CAPABILITY_USB3)
>
> thanks,
>
> --
> heikki