Re: [PATCH] usb: typec: Add a sysfs node to manage port type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/23/2017 06:39 AM, Heikki Krogerus wrote:
On Tue, May 23, 2017 at 06:16:28AM -0700, Guenter Roeck wrote:
On 05/23/2017 03:46 AM, Heikki Krogerus wrote:
Hi,

On Mon, May 22, 2017 at 01:05:42PM -0700, Badhri Jagan Sridharan wrote:
User space applications in some cases have the need to enforce a
specific port type(DFP/UFP/DRP). This change allows userspace to
attempt setting the desired port type. Low level drivers can
however reject the request if the specific port type is not supported.

Signed-off-by: Badhri Jagan Sridharan <Badhri@xxxxxxxxxx>
---
   Documentation/ABI/testing/sysfs-class-typec | 13 ++++++++++
   drivers/usb/typec/typec.c                   | 40 +++++++++++++++++++++++++++++
   include/linux/usb/typec.h                   |  4 +++
   3 files changed, 57 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index d4a3d23eb09c..853b2ef73641 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -73,6 +73,19 @@ Description:
   		Valid values: source, sink, none (to remove preference)
+What:           /sys/class/typec/<port>/port_type
+Date:           May 2017
+Description:
+		Indicates the type of the port. This attribute can be used for
+		requesting a change in the port type. Port type change is
+		supported as a synchronous operation, so write(2) to the
+		attribute will not return until the operation has finished.
+
+		Valid values:
+		- DRP
+		- DFP
+		- UFP

   What:		/sys/class/typec/<port>/supported_accessory_modes
   Date:		April 2017
   Contact:	Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 89e540bb7ff3..684a13bb744d 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -789,6 +789,12 @@ static const char * const typec_data_roles[] = {
   	[TYPEC_HOST]	= "host",
   };
+static const char * const typec_port_types[] = {
+	[TYPEC_PORT_DFP] = "dfp",
+	[TYPEC_PORT_UFP] = "ufp",
+	[TYPEC_PORT_DRP] = "drp",
+};

The meaning of those abbreviations has changed in every version of the
spec since v1.0 which makes me a bit uncomfortable using them with the
attributes. In USB Type-C specification v1.2, DRP now means
Dual-Role-Power, but DFP and UFP are used with USB data operation.

I would prefer "source, "sink" and "drp". Actually, I don't even like
"drp". How about "dual" instead?

   static ssize_t
   preferred_role_store(struct device *dev, struct device_attribute *attr,
   		     const char *buf, size_t size)
@@ -926,6 +932,39 @@ static ssize_t power_role_show(struct device *dev,
   }
   static DEVICE_ATTR_RW(power_role);
+static ssize_t
+port_type_store(struct device *dev, struct device_attribute *attr,
+			const char *buf, size_t size)
+{
+	struct typec_port *port = to_typec_port(dev);
+	int ret;
+
+	if (!port->cap->port_type_set) {
+		dev_dbg(dev, "changing port type not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	ret = sysfs_match_string(typec_port_types, buf);
+	if (ret < 0)
+		return ret;
+
+	ret = port->cap->port_type_set(port->cap, ret);
+	if (ret)
+		return ret;
+
+	return size;
+}
+
+static ssize_t
+port_type_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct typec_port *port = to_typec_port(dev);
+
+	return sprintf(buf, "%s\n", typec_port_types[port->cap->type]);
+}
+static DEVICE_ATTR_RW(port_type);

This doesn't tell the user the capabilities of the port. All the
supported roles should be listed here like with the other attributes,
the active one in brackets. This probable means we need a small
addition/change to the API too.

Sorry, not the API..

typec_capability already lists the port type. Presumably it can be
restricted to TYPEC_PORT_DFP or TYPEC_PORT_UFP only if it is reported
as TYPEC_PORT_DRP. Am I missing something ?

I mean, we should not overwrite the type member in typec_capability.
DRP capable port is still DRP capable even if we fix it to DFP or UFP.
So the active, fixed role, should be stored to struct typec_port.

Yes, you are right. Makes sense.

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux