| 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
| ||
|
Message-ID: <aKbMeSCHf-ZhbcvT@kuha.fi.intel.com>
Date: Thu, 21 Aug 2025 10:36:25 +0300
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: Andrei Kuchynski <akuchynski@...omium.org>
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
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.
> 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 a72325ff099a..708f3487222a 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -19,6 +19,7 @@
> #include "bus.h"
> #include "class.h"
> #include "pd.h"
> +#include "mode_selection.h"
>
> static DEFINE_IDA(typec_index_ida);
>
> @@ -445,11 +446,41 @@ svid_show(struct device *dev, struct device_attribute *attr, char *buf)
> }
> static DEVICE_ATTR_RO(svid);
>
> +static ssize_t priority_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + unsigned int val;
> + int err = kstrtouint(buf, 10, &val);
> +
> + if (!err) {
> + err = typec_mode_set_priority(to_typec_altmode(dev), val);
> + if (!err)
> + return size;
> + }
> +
> + return err;
> +}
> +
> +static ssize_t priority_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int val;
> + const int err = typec_mode_get_priority(to_typec_altmode(dev), &val);
> +
> + if (err)
> + return err;
> +
> + return sprintf(buf, "%d\n", val);
> +}
> +static DEVICE_ATTR_RW(priority);
> +
> static struct attribute *typec_altmode_attrs[] = {
> &dev_attr_active.attr,
> &dev_attr_mode.attr,
> &dev_attr_svid.attr,
> &dev_attr_vdo.attr,
> + &dev_attr_priority.attr,
> NULL
> };
>
> @@ -458,7 +489,7 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
> {
> struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
>
> - if (attr == &dev_attr_active.attr)
> + if (attr == &dev_attr_active.attr) {
> if (!is_typec_port(adev->dev.parent)) {
> struct typec_partner *partner =
> to_typec_partner(adev->dev.parent);
> @@ -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;
> 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?
> 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
Powered by blists - more mailing lists