| Message ID | 20241011124402.3306994-2-heikki.krogerus@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: typec: USB Modes | expand |
On Fri, Oct 11, 2024 at 03:43:59PM +0300, Heikki Krogerus wrote: > +What: /sys/class/typec/<port>/usb_capability > +Date: May 2024 It's a bit past May 2024 :) > +static ssize_t > +usb_capability_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + enum usb_mode mode = to_typec_port(dev)->usb_mode; > + u8 cap = to_typec_port(dev)->cap->usb_capability; > + int len = 0; > + int i; > + > + for (i = USB_MODE_USB2; i < USB_MODE_USB4 + 1; i++) { > + if (!(BIT(i - 1) & cap)) > + continue; > + > + if (i == mode) > + len += sysfs_emit_at(buf, len, "[%s] ", usb_modes[i]); > + else > + len += sysfs_emit_at(buf, len, "%s ", usb_modes[i]); > + } > + > + buf[len - 1] = '\n'; Nit, shouldn't this be another call to sysfs_emit_at()? > @@ -240,6 +251,7 @@ struct typec_partner_desc { > * @port_type_set: Set port type > * @pd_get: Get available USB Power Delivery Capabilities. > * @pd_set: Set USB Power Delivery Capabilities. > + * @default_usb_mode_set: USB Mode to be used by default with Enter_USB Message > */ > struct typec_operations { > int (*try_role)(struct typec_port *port, int role); > @@ -250,6 +262,7 @@ struct typec_operations { > enum typec_port_type type); > struct usb_power_delivery **(*pd_get)(struct typec_port *port); > int (*pd_set)(struct typec_port *port, struct usb_power_delivery *pd); > + int (*default_usb_mode_set)(struct typec_port *port, enum usb_mode mode); Naming is hard, but usually it's "noun_verb" so wouldn't this be just "mode_set_default"? We know it's usb :) But why default, why not just "mode_set"? or "set_mode" given you have "try_role" here, but then you have "pd_set". Ick, I don't know, it's your code, so your call, nevermind... thanks, greg k-h
On Fri, Oct 11, 2024 at 03:07:21PM +0200, Greg Kroah-Hartman wrote: > On Fri, Oct 11, 2024 at 03:43:59PM +0300, Heikki Krogerus wrote: > > +What: /sys/class/typec/<port>/usb_capability > > +Date: May 2024 > > It's a bit past May 2024 :) Yes. I'm sorry about that. > > +static ssize_t > > +usb_capability_show(struct device *dev, struct device_attribute *attr, char *buf) > > +{ > > + enum usb_mode mode = to_typec_port(dev)->usb_mode; > > + u8 cap = to_typec_port(dev)->cap->usb_capability; > > + int len = 0; > > + int i; > > + > > + for (i = USB_MODE_USB2; i < USB_MODE_USB4 + 1; i++) { > > + if (!(BIT(i - 1) & cap)) > > + continue; > > + > > + if (i == mode) > > + len += sysfs_emit_at(buf, len, "[%s] ", usb_modes[i]); > > + else > > + len += sysfs_emit_at(buf, len, "%s ", usb_modes[i]); > > + } > > + > > + buf[len - 1] = '\n'; > > Nit, shouldn't this be another call to sysfs_emit_at()? Yes. > > @@ -240,6 +251,7 @@ struct typec_partner_desc { > > * @port_type_set: Set port type > > * @pd_get: Get available USB Power Delivery Capabilities. > > * @pd_set: Set USB Power Delivery Capabilities. > > + * @default_usb_mode_set: USB Mode to be used by default with Enter_USB Message > > */ > > struct typec_operations { > > int (*try_role)(struct typec_port *port, int role); > > @@ -250,6 +262,7 @@ struct typec_operations { > > enum typec_port_type type); > > struct usb_power_delivery **(*pd_get)(struct typec_port *port); > > int (*pd_set)(struct typec_port *port, struct usb_power_delivery *pd); > > + int (*default_usb_mode_set)(struct typec_port *port, enum usb_mode mode); > > Naming is hard, but usually it's "noun_verb" so wouldn't this be just > "mode_set_default"? We know it's usb :) > > But why default, why not just "mode_set"? or "set_mode" given you have > "try_role" here, but then you have "pd_set". Ick, I don't know, it's > your code, so your call, nevermind... I think I'll just change it this back to the way it was in the last version, and drop "default" from the name. thanks,
On Fri, Oct 11, 2024 at 6:59 AM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > On Fri, Oct 11, 2024 at 03:07:21PM +0200, Greg Kroah-Hartman wrote: > > On Fri, Oct 11, 2024 at 03:43:59PM +0300, Heikki Krogerus wrote: > > > +What: /sys/class/typec/<port>/usb_capability > > > +Date: May 2024 > > > > It's a bit past May 2024 :) > > Yes. I'm sorry about that. > > > > +static ssize_t > > > +usb_capability_show(struct device *dev, struct device_attribute *attr, char *buf) > > > +{ > > > + enum usb_mode mode = to_typec_port(dev)->usb_mode; > > > + u8 cap = to_typec_port(dev)->cap->usb_capability; > > > + int len = 0; > > > + int i; > > > + > > > + for (i = USB_MODE_USB2; i < USB_MODE_USB4 + 1; i++) { > > > + if (!(BIT(i - 1) & cap)) > > > + continue; > > > + > > > + if (i == mode) > > > + len += sysfs_emit_at(buf, len, "[%s] ", usb_modes[i]); > > > + else > > > + len += sysfs_emit_at(buf, len, "%s ", usb_modes[i]); > > > + } > > > + > > > + buf[len - 1] = '\n'; > > > > Nit, shouldn't this be another call to sysfs_emit_at()? > > Yes. > > > > @@ -240,6 +251,7 @@ struct typec_partner_desc { > > > * @port_type_set: Set port type > > > * @pd_get: Get available USB Power Delivery Capabilities. > > > * @pd_set: Set USB Power Delivery Capabilities. > > > + * @default_usb_mode_set: USB Mode to be used by default with Enter_USB Message > > > */ > > > struct typec_operations { > > > int (*try_role)(struct typec_port *port, int role); > > > @@ -250,6 +262,7 @@ struct typec_operations { > > > enum typec_port_type type); > > > struct usb_power_delivery **(*pd_get)(struct typec_port *port); > > > int (*pd_set)(struct typec_port *port, struct usb_power_delivery *pd); > > > + int (*default_usb_mode_set)(struct typec_port *port, enum usb_mode mode); > > > > Naming is hard, but usually it's "noun_verb" so wouldn't this be just > > "mode_set_default"? We know it's usb :) > > > > But why default, why not just "mode_set"? or "set_mode" given you have > > "try_role" here, but then you have "pd_set". Ick, I don't know, it's > > your code, so your call, nevermind... > > I think I'll just change it this back to the way it was in the last > version, and drop "default" from the name. What's being set underneath is what USB mode to enter on the next reset or attach -- i.e. the default USB mode to enter. So appropriate naming here may be one of usb_set_default, usb_set_next. Dropping "usb" makes less sense vs dropping "mode", which could also refer to alternate modes so I'd prefer we don't call it mode_set. > > thanks, > > -- > heikki
> > > > @@ -240,6 +251,7 @@ struct typec_partner_desc { > > > > * @port_type_set: Set port type > > > > * @pd_get: Get available USB Power Delivery Capabilities. > > > > * @pd_set: Set USB Power Delivery Capabilities. > > > > + * @default_usb_mode_set: USB Mode to be used by default with Enter_USB Message > > > > */ > > > > struct typec_operations { > > > > int (*try_role)(struct typec_port *port, int role); > > > > @@ -250,6 +262,7 @@ struct typec_operations { > > > > enum typec_port_type type); > > > > struct usb_power_delivery **(*pd_get)(struct typec_port *port); > > > > int (*pd_set)(struct typec_port *port, struct usb_power_delivery *pd); > > > > + int (*default_usb_mode_set)(struct typec_port *port, enum usb_mode mode); > > > > > > Naming is hard, but usually it's "noun_verb" so wouldn't this be just > > > "mode_set_default"? We know it's usb :) > > > > > > But why default, why not just "mode_set"? or "set_mode" given you have > > > "try_role" here, but then you have "pd_set". Ick, I don't know, it's > > > your code, so your call, nevermind... > > > > I think I'll just change it this back to the way it was in the last > > version, and drop "default" from the name. > > What's being set underneath is what USB mode to enter on the next > reset or attach -- i.e. the default USB mode to enter. So appropriate > naming here may be one of usb_set_default, usb_set_next. Dropping > "usb" makes less sense vs dropping "mode", which could also refer to > alternate modes so I'd prefer we don't call it mode_set. I'll keep it the way it is now. This is kernel internal stuff, so we can always change it later. thanks,
diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec index 281b995beb05..7c307f02d99e 100644 --- a/Documentation/ABI/testing/sysfs-class-typec +++ b/Documentation/ABI/testing/sysfs-class-typec @@ -149,6 +149,19 @@ Description: advertise to the partner. The currently used capabilities are in brackets. Selection happens by writing to the file. +What: /sys/class/typec/<port>/usb_capability +Date: May 2024 +Contact: Heikki Krogerus <heikki.krogerus@linux.intel.com> +Description: Lists the supported USB Modes. The default USB mode that is used + next time with the Enter_USB Message is in brackets. The default + mode can be changed by writing to the file when supported by the + driver. + + Valid values: + - usb2 (USB 2.0) + - usb3 (USB 3.2) + - usb4 (USB4) + 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 9262fcd4144f..ea9ee47bb246 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -219,6 +219,13 @@ static ssize_t usb_power_delivery_revision_show(struct device *dev, char *buf); static DEVICE_ATTR_RO(usb_power_delivery_revision); +static const char * const usb_modes[] = { + [USB_MODE_NONE] = "none", + [USB_MODE_USB2] = "usb2", + [USB_MODE_USB3] = "usb3", + [USB_MODE_USB4] = "usb4" +}; + /* ------------------------------------------------------------------------- */ /* Alternate Modes */ @@ -1289,6 +1296,67 @@ EXPORT_SYMBOL_GPL(typec_unregister_cable); /* ------------------------------------------------------------------------- */ /* USB Type-C ports */ +/** + * typec_port_set_usb_mode - Set the operational USB mode for the port + * @port: USB Type-C port + * @mode: USB Mode (USB2, USB3 or USB4) + * + * @mode will be used with the next Enter_USB message. Existing connections are + * not affected. + */ +void typec_port_set_usb_mode(struct typec_port *port, enum usb_mode mode) +{ + port->usb_mode = mode; +} +EXPORT_SYMBOL_GPL(typec_port_set_usb_mode); + +static ssize_t +usb_capability_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + enum usb_mode mode = to_typec_port(dev)->usb_mode; + u8 cap = to_typec_port(dev)->cap->usb_capability; + int len = 0; + int i; + + for (i = USB_MODE_USB2; i < USB_MODE_USB4 + 1; i++) { + if (!(BIT(i - 1) & cap)) + continue; + + if (i == mode) + len += sysfs_emit_at(buf, len, "[%s] ", usb_modes[i]); + else + len += sysfs_emit_at(buf, len, "%s ", usb_modes[i]); + } + + buf[len - 1] = '\n'; + return len; +} + +static ssize_t +usb_capability_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t size) +{ + struct typec_port *port = to_typec_port(dev); + int ret = 0; + int mode; + + if (!port->ops || !port->ops->default_usb_mode_set) + return -EOPNOTSUPP; + + mode = sysfs_match_string(usb_modes, buf); + if (mode < 0) + return mode; + + ret = port->ops->default_usb_mode_set(port, mode); + if (ret) + return ret; + + port->usb_mode = mode; + + return size; +} +static DEVICE_ATTR_RW(usb_capability); + /** * typec_port_set_usb_power_delivery - Assign USB PD for port. * @port: USB Type-C port. @@ -1757,6 +1825,7 @@ static struct attribute *typec_attrs[] = { &dev_attr_vconn_source.attr, &dev_attr_port_type.attr, &dev_attr_orientation.attr, + &dev_attr_usb_capability.attr, NULL, }; @@ -1790,6 +1859,11 @@ static umode_t typec_attr_is_visible(struct kobject *kobj, if (port->cap->orientation_aware) return 0444; return 0; + } else if (attr == &dev_attr_usb_capability.attr) { + if (!port->cap->usb_capability) + return 0; + if (!port->ops || !port->ops->default_usb_mode_set) + return 0444; } return attr->mode; @@ -2428,6 +2502,13 @@ struct typec_port *typec_register_port(struct device *parent, port->con.attach = typec_partner_attach; port->con.deattach = typec_partner_deattach; + if (cap->usb_capability & USB_CAPABILITY_USB4) + port->usb_mode = USB_MODE_USB4; + else if (cap->usb_capability & USB_CAPABILITY_USB3) + port->usb_mode = USB_MODE_USB3; + else if (cap->usb_capability & USB_CAPABILITY_USB2) + port->usb_mode = USB_MODE_USB2; + device_initialize(&port->dev); port->dev.class = &typec_class; port->dev.parent = parent; diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h index 7485cdb9dd20..85bc50aa54f7 100644 --- a/drivers/usb/typec/class.h +++ b/drivers/usb/typec/class.h @@ -55,6 +55,7 @@ struct typec_port { enum typec_role vconn_role; enum typec_pwr_opmode pwr_opmode; enum typec_port_type port_type; + enum usb_mode usb_mode; struct mutex port_type_lock; enum typec_orientation orientation; diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h index 549275f8ac1b..f7edced5b10b 100644 --- a/include/linux/usb/typec.h +++ b/include/linux/usb/typec.h @@ -87,6 +87,17 @@ enum typec_orientation { TYPEC_ORIENTATION_REVERSE, }; +enum usb_mode { + USB_MODE_NONE, + USB_MODE_USB2, + USB_MODE_USB3, + USB_MODE_USB4 +}; + +#define USB_CAPABILITY_USB2 BIT(0) +#define USB_CAPABILITY_USB3 BIT(1) +#define USB_CAPABILITY_USB4 BIT(2) + /* * struct enter_usb_data - Enter_USB Message details * @eudo: Enter_USB Data Object @@ -240,6 +251,7 @@ struct typec_partner_desc { * @port_type_set: Set port type * @pd_get: Get available USB Power Delivery Capabilities. * @pd_set: Set USB Power Delivery Capabilities. + * @default_usb_mode_set: USB Mode to be used by default with Enter_USB Message */ struct typec_operations { int (*try_role)(struct typec_port *port, int role); @@ -250,6 +262,7 @@ struct typec_operations { enum typec_port_type type); struct usb_power_delivery **(*pd_get)(struct typec_port *port); int (*pd_set)(struct typec_port *port, struct usb_power_delivery *pd); + int (*default_usb_mode_set)(struct typec_port *port, enum usb_mode mode); }; enum usb_pd_svdm_ver { @@ -267,6 +280,7 @@ enum usb_pd_svdm_ver { * @svdm_version: USB PD Structured VDM version if supported * @prefer_role: Initial role preference (DRP ports). * @accessory: Supported Accessory Modes + * @usb_capability: Supported USB Modes * @fwnode: Optional fwnode of the port * @driver_data: Private pointer for driver specific info * @pd: Optional USB Power Delivery Support @@ -283,6 +297,7 @@ struct typec_capability { int prefer_role; enum typec_accessory accessory[TYPEC_MAX_ACCESSORY]; unsigned int orientation_aware:1; + u8 usb_capability; struct fwnode_handle *fwnode; void *driver_data; @@ -350,6 +365,8 @@ int typec_port_set_usb_power_delivery(struct typec_port *port, struct usb_power_ int typec_partner_set_usb_power_delivery(struct typec_partner *partner, struct usb_power_delivery *pd); +void typec_port_set_usb_mode(struct typec_port *port, enum usb_mode mode); + /** * struct typec_connector - Representation of Type-C port for external drivers * @attach: notification about device removal
This attribute file, named "usb_capability", will show the supported USB modes, which are USB 2.0, USB 3.2 and USB4. These modes are defined in the USB Type-C (R2.0) and USB Power Delivery (R3.0 V2.0) Specifications. Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> --- Documentation/ABI/testing/sysfs-class-typec | 13 ++++ drivers/usb/typec/class.c | 81 +++++++++++++++++++++ drivers/usb/typec/class.h | 1 + include/linux/usb/typec.h | 17 +++++ 4 files changed, 112 insertions(+)