diff mbox series

[v3,1/4] usb: typec: Add attribute file showing the supported USB modes of the port

Message ID 20241011124402.3306994-2-heikki.krogerus@linux.intel.com
State Superseded
Headers show
Series usb: typec: USB Modes | expand

Commit Message

Heikki Krogerus Oct. 11, 2024, 12:43 p.m. UTC
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(+)

Comments

Heikki Krogerus Oct. 11, 2024, 1:59 p.m. UTC | #1
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,
Abhishek Pandit-Subedi Oct. 15, 2024, 2:38 a.m. UTC | #2
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
Heikki Krogerus Oct. 15, 2024, 12:12 p.m. UTC | #3
> > > > @@ -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 mbox series

Patch

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