[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMMMRMc2t=0CNR4VPURNuKu0YGRdXqWhXAppgR8jNLoJ_TCYVw@mail.gmail.com>
Date: Sat, 17 Jan 2026 20:25:44 +0100
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>, linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
chrome-platform@...ts.linux.dev, Tzung-Bi Shih <tzungbi@...nel.org>,
Guenter Roeck <groeck@...omium.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
Łukasz Bartosik <ukaszb@...omium.org>,
Abel Vesa <abel.vesa@...aro.org>, Pooja Katiyar <pooja.katiyar@...el.com>,
Johan Hovold <johan@...nel.org>, Hsin-Te Yuan <yuanhsinte@...omium.org>, Madhu M <madhu.m@...el.com>,
Venkat Jayaraman <venkat.jayaraman@...el.com>
Subject: Re: [PATCH v4 4/8] usb: typec: Expose alternate mode priority via sysfs
On Thu, Jan 15, 2026 at 3:34 PM Heikki Krogerus
<heikki.krogerus@...ux.intel.com> wrote:
>
> Hi Andrei,
>
> Tue, Jan 13, 2026 at 01:05:32PM +0000, Andrei Kuchynski kirjoitti:
> > 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. If a new priority value conflicts with an existing
> > mode's priority, the priorities of the conflicting mode and all subsequent
> > modes are automatically incremented to ensure uniqueness.
> >
> > Signed-off-by: Andrei Kuchynski <akuchynski@...omium.org>
> > Reviewed-by: Benson Leung <bleung@...omium.org>
> > ---
> > Documentation/ABI/testing/sysfs-class-typec | 11 +++
> > drivers/usb/typec/class.c | 90 ++++++++++++++++++++-
> > include/linux/usb/typec_altmode.h | 1 +
> > 3 files changed, 101 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index 38e101c17a004..737b76828b509 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -162,6 +162,17 @@ 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 alternate mode.
> > + The priority is an integer in the range 0-255. A lower numerical value
> > + indicates a higher priority (0 is the highest).
> > + 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.
>
> Greg already told you to show the range somehow with this, so the
> total number of (so far known?) alternate modes. Maybe something like
> <index> / <total_num_altmodes> ?
>
Thank you for the review, Heikki.
Greg asked me to define a range and document it. The range is 0 to 255.
I'm not sure if this range needs to be shown.
In our current userspace application, we strictly read/write 0/1 here.
> > USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
> >
> > What: /sys/class/typec/<port>-partner/accessory_mode
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 0f12d6120511b..1fb5450c0a2f2 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -445,11 +445,88 @@ svid_show(struct device *dev, struct device_attribute *attr, char *buf)
> > }
> > static DEVICE_ATTR_RO(svid);
> >
> > +static int increment_duplicated_priority(struct device *dev, void *data)
> > +{
> > + if (is_typec_port_altmode(dev)) {
> > + struct typec_altmode **alt_target = (struct typec_altmode **)data;
> > + struct typec_altmode *alt = to_typec_altmode(dev);
> > +
> > + if (alt != *alt_target && alt->priority == (*alt_target)->priority) {
> > + alt->priority++;
> > + *alt_target = alt;
> > + return 1;
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > +static int find_duplicated_priority(struct device *dev, void *data)
> > +{
> > + if (is_typec_port_altmode(dev)) {
> > + struct typec_altmode **alt_target = (struct typec_altmode **)data;
> > + struct typec_altmode *alt = to_typec_altmode(dev);
> > +
> > + if (alt != *alt_target && alt->priority == (*alt_target)->priority)
> > + return 1;
> > + }
> > + return 0;
> > +}
> > +
> > +static int typec_mode_set_priority(struct typec_altmode *alt, const u8 priority)
> > +{
> > + struct typec_port *port = to_typec_port(alt->dev.parent);
> > + const u8 old_priority = alt->priority;
> > + int res = 1;
> > +
> > + alt->priority = priority;
> > + while (res) {
> > + res = device_for_each_child(&port->dev, &alt, find_duplicated_priority);
> > + if (res) {
> > + alt->priority++;
> > + if (alt->priority == 0) {
> > + alt->priority = old_priority;
> > + return -EOVERFLOW;
> > + }
> > + }
> > + }
> > +
> > + res = 1;
> > + alt->priority = priority;
> > + while (res)
> > + res = device_for_each_child(&port->dev, &alt,
> > + increment_duplicated_priority);
>
> Please align the code properly.
>
I will. Thanks for mentioning that.
> > + return 0;
> > +}
> > +
> > +static ssize_t priority_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t size)
> > +{
> > + u8 val;
> > + int err = kstrtou8(buf, 10, &val);
> > +
> > + if (!err)
> > + err = typec_mode_set_priority(to_typec_altmode(dev), val);
> > +
> > + if (!err)
> > + return size;
> > + return err;
>
> I know not everybody likes the ternary operator, but I would just
>
> return err ?: size;
>
I like the ternary operator. But I might keep the current implementation.
Thanks,
Andrei
Powered by blists - more mailing lists