Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1403714 > unrolled thread

[RFC PATCHv2] usb: USB Type-C Connector Class

Started byHeikki Krogerus <heikki.krogerus@linux.intel.com>
First post2016-05-19 14:50 +0200
Last post2016-05-31 10:40 +0200
Articles 10 on this page of 50 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-19 14:50 +0200
    Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-19 17:00 +0200
      Re: [RFC PATCHv2] usb: USB Type-C Connector Class Greg KH <gregkh@linuxfoundation.org> - 2016-05-19 17:50 +0200
        Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-20 13:00 +0200
    Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-19 17:00 +0200
      Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-20 13:30 +0200
        Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-20 15:50 +0200
          Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-21 08:00 +0200
            Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-21 08:50 +0200
              Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-22 18:00 +0200
                Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-23 07:40 +0200
                  Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-23 15:30 +0200
                    Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-23 16:10 +0200
                      Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-23 16:50 +0200
                        Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-23 18:00 +0200
                          Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-23 19:00 +0200
                            Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-24 12:10 +0200
                              Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-24 12:30 +0200
                                Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-24 13:10 +0200
    Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-19 20:00 +0200
      Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-20 12:50 +0200
        Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-20 19:10 +0200
          Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-23 11:30 +0200
    Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-20 16:30 +0200
      Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-23 12:00 +0200
        Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-23 13:30 +0200
          Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-23 19:10 +0200
            Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-24 11:10 +0200
          Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-24 11:40 +0200
    Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-24 15:00 +0200
      Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-25 13:30 +0200
        Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-25 17:30 +0200
          Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-27 09:40 +0200
    Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-24 15:50 +0200
      Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-25 13:40 +0200
        Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-25 15:20 +0200
    Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-24 21:30 +0200
      Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-25 14:00 +0200
        Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-25 15:30 +0200
          Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-25 16:10 +0200
            Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-25 16:30 +0200
              Re: [RFC PATCHv2] usb: USB Type-C Connector Class Guenter Roeck <linux@roeck-us.net> - 2016-05-25 17:10 +0200
                Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-27 09:30 +0200
    [RFC PATCH] usb: typec: Various API updates and fixes Guenter Roeck <linux@roeck-us.net> - 2016-05-25 20:40 +0200
      Re: [RFC PATCH] usb: typec: Various API updates and fixes Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-27 10:00 +0200
        Re: [RFC PATCH] usb: typec: Various API updates and fixes Guenter Roeck <linux@roeck-us.net> - 2016-05-27 16:10 +0200
          Re: [RFC PATCH] usb: typec: Various API updates and fixes Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-30 14:50 +0200
    Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-30 15:30 +0200
      Re: [RFC PATCHv2] usb: USB Type-C Connector Class Oliver Neukum <oneukum@suse.com> - 2016-05-30 16:10 +0200
        Re: [RFC PATCHv2] usb: USB Type-C Connector Class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-31 10:40 +0200

Page 3 of 3 — ← Prev page 1 2 [3]

#1406946

FromOliver Neukum <oneukum@suse.com>
Date2016-05-25 16:30 +0200
Message-ID<rCHYS-22l-23@gated-at.bofh.it>
In reply to#1406928
On Wed, 2016-05-25 at 17:04 +0300, Heikki Krogerus wrote:

> I'm not against leaving the responsibility of registering the alternate
> modes to the drivers. I'm a little bit worried about relying then on
> the drivers to also handle the unregistering accordingly, but I can
> live with that. But we just shouldn't share the responsibility of
> un/registering them between the class and the drivers, so the driver
> should then handle the registration always.
> 
> Oliver, what do you think?

Either will do for me. Registration by the drivers is a bit better.
But it has to be the one or the other. Mixing is indeed bad.

	Regards
		Oliver

[toc] | [prev] | [next] | [standalone]

#1406996

FromGuenter Roeck <linux@roeck-us.net>
Date2016-05-25 17:10 +0200
Message-ID<rCIBz-2uj-23@gated-at.bofh.it>
In reply to#1406946
On Wed, May 25, 2016 at 04:20:56PM +0200, Oliver Neukum wrote:
> On Wed, 2016-05-25 at 17:04 +0300, Heikki Krogerus wrote:
> 
> > I'm not against leaving the responsibility of registering the alternate
> > modes to the drivers. I'm a little bit worried about relying then on
> > the drivers to also handle the unregistering accordingly, but I can
> > live with that. But we just shouldn't share the responsibility of
> > un/registering them between the class and the drivers, so the driver
> > should then handle the registration always.
> > 
> > Oliver, what do you think?
> 
> Either will do for me. Registration by the drivers is a bit better.
> But it has to be the one or the other. Mixing is indeed bad.
> 
Same here. I don't have any problems handling unregistering
from the driver. I just have to keep track of the state and call
typec_unregister_altmodes() before calling typec_disconnect().

Having to wait for mode discovery to complete before calling
typec_connect() is much more complicated, at least with my current
code.

Guenter

[toc] | [prev] | [next] | [standalone]

#1407868

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-05-27 09:30 +0200
Message-ID<rDknw-rU-9@gated-at.bofh.it>
In reply to#1406996
On Wed, May 25, 2016 at 07:59:57AM -0700, Guenter Roeck wrote:
> On Wed, May 25, 2016 at 04:20:56PM +0200, Oliver Neukum wrote:
> > On Wed, 2016-05-25 at 17:04 +0300, Heikki Krogerus wrote:
> > 
> > > I'm not against leaving the responsibility of registering the alternate
> > > modes to the drivers. I'm a little bit worried about relying then on
> > > the drivers to also handle the unregistering accordingly, but I can
> > > live with that. But we just shouldn't share the responsibility of
> > > un/registering them between the class and the drivers, so the driver
> > > should then handle the registration always.
> > > 
> > > Oliver, what do you think?
> > 
> > Either will do for me. Registration by the drivers is a bit better.
> > But it has to be the one or the other. Mixing is indeed bad.
> > 
> Same here. I don't have any problems handling unregistering
> from the driver. I just have to keep track of the state and call
> typec_unregister_altmodes() before calling typec_disconnect().
> 
> Having to wait for mode discovery to complete before calling
> typec_connect() is much more complicated, at least with my current
> code.

OK, so we'll change this and make the driver take care of the
registration.


-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1407158 — [RFC PATCH] usb: typec: Various API updates and fixes

FromGuenter Roeck <linux@roeck-us.net>
Date2016-05-25 20:40 +0200
Subject[RFC PATCH] usb: typec: Various API updates and fixes
Message-ID<rCLSN-4jE-1@gated-at.bofh.it>
In reply to#1403714
From: Guenter Roeck <groeck@chromium.org>

New API functions (calls into class code)
	typec_set_usb_role()
	typec_set_pwr_role()
	typec_set_vconn_role()
	typec_set_pwr_opmode()

Modified API functions (calls into class code):
	typec_register_port(dev, cap) ->
			typec_register_port(dev, cap, driver_data)

Modified callback functions:
	dr_swap(port) -> dr_set(port, driver_data, role);
	pr_swap(port) -> pr_set(port, driver_data, role);
	vconn_swap(port) -> vconn_set(port, driver_data, role);
	fix_role(port) -> fix_role(port, driver_data, role);
	activate_mode(...) -> activate_mode(..., driver_data, ...);

New sysfs attribute:
	current_vconn_role

Other:
- Extract role initialization to new function typec_init_roles()
- Call driver code unconditionally on role changes
- Add NULL check in typec_unregister_altmodes()
- If an alternate mode description pointer is NULL, display
  an empty string.

Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
This patch applies on top of '[RFC PATCHv2] usb: USB Type-C Connector Class'
from Heikki Krogerus. It provided the changes I made to get the code
operational.

 drivers/usb/type-c/typec.c | 134 ++++++++++++++++++++++++++++++++++++---------
 include/linux/usb/typec.h  |  26 ++++++---
 2 files changed, 125 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/type-c/typec.c b/drivers/usb/type-c/typec.c
index 8028b7df0951..6836e972b681 100644
--- a/drivers/usb/type-c/typec.c
+++ b/drivers/usb/type-c/typec.c
@@ -27,6 +27,8 @@ struct typec_port {
 	struct typec_partner	*partner;
 	struct typec_cable	*cable;
 
+	void			*driver_data;
+
 	unsigned int		connected:1;
 
 	int			n_altmode;
@@ -324,6 +326,20 @@ static void typec_remove_cable(struct typec_port *port)
 	device_unregister(&port->cable->dev);
 }
 
+static void typec_init_roles(struct typec_port *port)
+{
+	if (port->fixed_role == TYPEC_PORT_DFP) {
+		port->usb_role = TYPEC_HOST;
+		port->pwr_role = TYPEC_PWR_SOURCE;
+		port->vconn_role = TYPEC_PWR_SOURCE;
+	} else {
+		/* Device mode as default also with DRP ports */
+		port->usb_role = TYPEC_DEVICE;
+		port->pwr_role = TYPEC_PWR_SINK;
+		port->vconn_role = TYPEC_PWR_SINK;
+	}
+}
+
 /* -------------------------------- */
 
 int typec_connect(struct typec_port *port, struct typec_connection *con)
@@ -378,16 +394,7 @@ void typec_disconnect(struct typec_port *port)
 
 	port->pwr_opmode = TYPEC_PWR_MODE_USB;
 
-	if (port->fixed_role == TYPEC_PORT_DFP) {
-		port->usb_role = TYPEC_HOST;
-		port->pwr_role = TYPEC_PWR_SOURCE;
-		port->vconn_role = TYPEC_PWR_SOURCE;
-	} else {
-		/* Device mode as default also with DRP ports */
-		port->usb_role = TYPEC_DEVICE;
-		port->pwr_role = TYPEC_PWR_SINK;
-		port->vconn_role = TYPEC_PWR_SINK;
-	}
+	typec_init_roles(port);
 
 	kobject_uevent(&port->dev.kobj, KOBJ_CHANGE);
 }
@@ -405,6 +412,34 @@ struct typec_port *typec_dev2port(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(typec_dev2port);
 
+/* --------------------------------------- */
+/* Driver callbacks to report role updates */
+
+void typec_set_usb_role(struct typec_port *port, enum typec_usb_role role)
+{
+	port->usb_role = role;
+}
+EXPORT_SYMBOL(typec_set_usb_role);
+
+void typec_set_pwr_role(struct typec_port *port, enum typec_pwr_role role)
+{
+	port->pwr_role = role;
+}
+EXPORT_SYMBOL(typec_set_pwr_role);
+
+void typec_set_vconn_role(struct typec_port *port, enum typec_pwr_role role)
+{
+	port->vconn_role = role;
+}
+EXPORT_SYMBOL(typec_set_vconn_role);
+
+void typec_set_pwr_opmode(struct typec_port *port,
+			  enum typec_pwr_opmode opmode)
+{
+	port->pwr_opmode = opmode;
+}
+EXPORT_SYMBOL(typec_set_pwr_opmode);
+
 /* -------------------------------- */
 /* Alternate Modes */
 
@@ -451,7 +486,7 @@ typec_altmode_desc_show(struct device *dev, struct device_attribute *attr,
 	struct typec_mode *mode = container_of(attr, struct typec_mode,
 					       desc_attr);
 
-	return sprintf(buf, "%s\n", mode->desc);
+	return sprintf(buf, "%s\n", mode->desc ? mode->desc : "");
 }
 
 static ssize_t
@@ -561,6 +596,9 @@ void typec_unregister_altmodes(struct typec_altmode *alt_modes)
 {
 	struct typec_altmode *alt;
 
+	if (!alt_modes)
+		return;
+
 	for (alt = alt_modes; alt->svid; alt++)
 		device_unregister(&alt->dev);
 }
@@ -581,7 +619,7 @@ current_usb_data_role_store(struct device *dev, struct device_attribute *attr,
 		return -EOPNOTSUPP;
 	}
 
-	if (!port->cap->dr_swap) {
+	if (!port->cap->dr_set) {
 		dev_warn(dev, "data role swapping not supported\n");
 		return -EOPNOTSUPP;
 	}
@@ -593,10 +631,7 @@ current_usb_data_role_store(struct device *dev, struct device_attribute *attr,
 	else
 		return -EINVAL;
 
-	if (port->usb_role == role || !port->partner)
-		return size;
-
-	ret = port->cap->dr_swap(port);
+	ret = port->cap->dr_set(port, port->driver_data, role);
 	if (ret)
 		return ret;
 
@@ -655,10 +690,7 @@ current_data_role_store(struct device *dev, struct device_attribute *attr,
 	else
 		return -EINVAL;
 
-	if (port->fixed_role == role)
-		return size;
-
-	ret = port->cap->fix_role(port, role);
+	ret = port->cap->fix_role(port, port->driver_data, role);
 	if (ret)
 		return ret;
 
@@ -688,7 +720,7 @@ static ssize_t current_power_role_store(struct device *dev,
 		return -EOPNOTSUPP;
 	}
 
-	if (!port->cap->pr_swap) {
+	if (!port->cap->pr_set) {
 		dev_warn(dev, "power role swapping not supported\n");
 		return -EOPNOTSUPP;
 	}
@@ -705,10 +737,7 @@ static ssize_t current_power_role_store(struct device *dev,
 	else
 		return -EINVAL;
 
-	if (port->pwr_role == role || !port->partner)
-		return size;
-
-	ret = port->cap->pr_swap(port);
+	ret = port->cap->pr_set(port, port->driver_data, role);
 	if (ret)
 		return ret;
 
@@ -762,6 +791,54 @@ static ssize_t power_operation_mode_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(power_operation_mode);
 
+static ssize_t current_vconn_role_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t size)
+{
+	struct typec_port *port = to_typec_port(dev);
+	enum typec_pwr_role role;
+	int ret;
+
+	if (!port->cap->usb_pd) {
+		dev_dbg(dev, "vconn swap only supported with USB PD\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (!port->cap->vconn_set) {
+		dev_warn(dev, "vconn swapping not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (!strncmp(buf, "source", 6))
+		role = TYPEC_PWR_SOURCE;
+	else if (!strncmp(buf, "sink", 4))
+		role = TYPEC_PWR_SINK;
+	else
+		return -EINVAL;
+
+	ret = port->cap->vconn_set(port, port->driver_data, role);
+	if (ret)
+		return ret;
+
+	return size;
+}
+
+static ssize_t current_vconn_role_show(struct device *dev,
+				       struct device_attribute *attr, char *buf)
+{
+	struct typec_port *port = to_typec_port(dev);
+
+	switch (port->vconn_role) {
+	case TYPEC_PWR_SOURCE:
+		return sprintf(buf, "source\n");
+	case TYPEC_PWR_SINK:
+		return sprintf(buf, "sink\n");
+	default:
+		return sprintf(buf, "unknown\n");
+	};
+}
+static DEVICE_ATTR_RW(current_vconn_role);
+
 static ssize_t supports_audio_accessory_show(struct device *dev,
 					     struct device_attribute *attr,
 					     char *buf)
@@ -795,6 +872,7 @@ static DEVICE_ATTR_RO(supports_usb_power_delivery);
 static struct attribute *typec_attrs[] = {
 	&dev_attr_current_data_role.attr,
 	&dev_attr_current_power_role.attr,
+	&dev_attr_current_vconn_role.attr,
 	&dev_attr_current_usb_data_role.attr,
 	&dev_attr_power_operation_mode.attr,
 	&dev_attr_supported_data_roles.attr,
@@ -862,7 +940,8 @@ static struct device_type typec_port_dev_type = {
 };
 
 struct typec_port *typec_register_port(struct device *dev,
-				       struct typec_capability *cap)
+				       struct typec_capability *cap,
+				       void *driver_data)
 {
 	struct typec_port *port;
 	int ret;
@@ -880,6 +959,7 @@ struct typec_port *typec_register_port(struct device *dev,
 
 	port->id = id;
 	port->cap = cap;
+	port->driver_data = driver_data;
 	port->dev.type = &typec_port_dev_type;
 	port->dev.class = &typec_class;
 	port->dev.parent = dev;
@@ -888,6 +968,8 @@ struct typec_port *typec_register_port(struct device *dev,
 
 	port->fixed_role = port->cap->role;
 
+	typec_init_roles(port);
+
 	ret = device_register(&port->dev);
 	if (ret) {
 		ida_simple_remove(&typec_index_ida, id);
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 86e5c867800b..d16a38de57ac 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -168,9 +168,9 @@ struct typec_partner {
  * @audio_accessory: Audio Accessory Adapter Mode support
  * @debug_accessory: Debug Accessory Mode support
  * @fix_role: Set a fixed data role for DRP port
- * @dr_swap: Data Role Swap support
- * @pr_swap: Power Role Swap support
- * @vconn_swap: VCONN Swap support
+ * @dr_set: Set Data Role
+ * @pr_set: Set Power Role
+ * @vconn_set: Set VCONN Role
  * @activate_mode: Enter/exit given Alternate Mode
  *
  * Static capabilities of a single USB Type-C port.
@@ -182,14 +182,14 @@ struct typec_capability {
 	unsigned int		audio_accessory:1;
 	unsigned int		debug_accessory:1;
 
-	int			(*fix_role)(struct typec_port *,
+	int			(*fix_role)(struct typec_port *, void *,
 					    enum typec_data_role);
 
-	int			(*dr_swap)(struct typec_port *);
-	int			(*pr_swap)(struct typec_port *);
-	int			(*vconn_swap)(struct typec_port *);
+	int			(*dr_set)(struct typec_port *, void *, enum typec_usb_role);
+	int			(*pr_set)(struct typec_port *, void *, enum typec_pwr_role);
+	int			(*vconn_set)(struct typec_port *, void *, enum typec_pwr_role);
 
-	int			(*activate_mode)(struct typec_altmode *,
+	int			(*activate_mode)(struct typec_altmode *, void *,
 						 int mode, int activate);
 };
 
@@ -217,7 +217,8 @@ struct typec_connection {
 };
 
 struct typec_port *typec_register_port(struct device *dev,
-				       struct typec_capability *cap);
+				       struct typec_capability *cap,
+				       void *driver_data);
 void typec_unregister_port(struct typec_port *port);
 
 int typec_connect(struct typec_port *port, struct typec_connection *con);
@@ -227,4 +228,11 @@ void typec_disconnect(struct typec_port *port);
 struct device *typec_port2dev(struct typec_port *port);
 struct typec_port *typec_dev2port(struct device *dev);
 
+/* Callbacks from driver */
+
+void typec_set_usb_role(struct typec_port *, enum typec_usb_role);
+void typec_set_pwr_role(struct typec_port *, enum typec_pwr_role);
+void typec_set_vconn_role(struct typec_port *, enum typec_pwr_role);
+void typec_set_pwr_opmode(struct typec_port *, enum typec_pwr_opmode);
+
 #endif /* __LINUX_USB_TYPEC_H */
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1407891 — Re: [RFC PATCH] usb: typec: Various API updates and fixes

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-05-27 10:00 +0200
SubjectRe: [RFC PATCH] usb: typec: Various API updates and fixes
Message-ID<rDkQy-BQ-25@gated-at.bofh.it>
In reply to#1407158
Hi,

On Wed, May 25, 2016 at 11:35:07AM -0700, Guenter Roeck wrote:
> From: Guenter Roeck <groeck@chromium.org>
> 
> New API functions (calls into class code)
> 	typec_set_usb_role()
> 	typec_set_pwr_role()
> 	typec_set_vconn_role()
> 	typec_set_pwr_opmode()
> 
> Modified API functions (calls into class code):
> 	typec_register_port(dev, cap) ->
> 			typec_register_port(dev, cap, driver_data)
> 
> Modified callback functions:
> 	dr_swap(port) -> dr_set(port, driver_data, role);
> 	pr_swap(port) -> pr_set(port, driver_data, role);
> 	vconn_swap(port) -> vconn_set(port, driver_data, role);
> 	fix_role(port) -> fix_role(port, driver_data, role);
> 	activate_mode(...) -> activate_mode(..., driver_data, ...);
> 
> New sysfs attribute:
> 	current_vconn_role
> 
> Other:
> - Extract role initialization to new function typec_init_roles()
> - Call driver code unconditionally on role changes
> - Add NULL check in typec_unregister_altmodes()
> - If an alternate mode description pointer is NULL, display
>   an empty string.
> 
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> This patch applies on top of '[RFC PATCHv2] usb: USB Type-C Connector Class'
> from Heikki Krogerus. It provided the changes I made to get the code
> operational.
> 
>  drivers/usb/type-c/typec.c | 134 ++++++++++++++++++++++++++++++++++++---------
>  include/linux/usb/typec.h  |  26 ++++++---
>  2 files changed, 125 insertions(+), 35 deletions(-)

Thanks Guenter. After a quick glance looks good to me.

I think I'll create a development branch to my github tree for this
class and add this on top of the original v2 patch as is. Just to have
some history for as long we develop this thing. Though I'm not sure
how useful it is in this case. I think we need to put this together
fast. I'm thinking we need to target v4.8 with this.

I'll merge this into any case to v3, and I'll send on Monday.


> diff --git a/drivers/usb/type-c/typec.c b/drivers/usb/type-c/typec.c
> index 8028b7df0951..6836e972b681 100644
> --- a/drivers/usb/type-c/typec.c
> +++ b/drivers/usb/type-c/typec.c
> @@ -27,6 +27,8 @@ struct typec_port {
>  	struct typec_partner	*partner;
>  	struct typec_cable	*cable;
>  
> +	void			*driver_data;
> +
>  	unsigned int		connected:1;
>  
>  	int			n_altmode;
> @@ -324,6 +326,20 @@ static void typec_remove_cable(struct typec_port *port)
>  	device_unregister(&port->cable->dev);
>  }
>  
> +static void typec_init_roles(struct typec_port *port)
> +{
> +	if (port->fixed_role == TYPEC_PORT_DFP) {
> +		port->usb_role = TYPEC_HOST;
> +		port->pwr_role = TYPEC_PWR_SOURCE;
> +		port->vconn_role = TYPEC_PWR_SOURCE;
> +	} else {
> +		/* Device mode as default also with DRP ports */
> +		port->usb_role = TYPEC_DEVICE;
> +		port->pwr_role = TYPEC_PWR_SINK;
> +		port->vconn_role = TYPEC_PWR_SINK;
> +	}
> +}
> +
>  /* -------------------------------- */
>  
>  int typec_connect(struct typec_port *port, struct typec_connection *con)
> @@ -378,16 +394,7 @@ void typec_disconnect(struct typec_port *port)
>  
>  	port->pwr_opmode = TYPEC_PWR_MODE_USB;
>  
> -	if (port->fixed_role == TYPEC_PORT_DFP) {
> -		port->usb_role = TYPEC_HOST;
> -		port->pwr_role = TYPEC_PWR_SOURCE;
> -		port->vconn_role = TYPEC_PWR_SOURCE;
> -	} else {
> -		/* Device mode as default also with DRP ports */
> -		port->usb_role = TYPEC_DEVICE;
> -		port->pwr_role = TYPEC_PWR_SINK;
> -		port->vconn_role = TYPEC_PWR_SINK;
> -	}
> +	typec_init_roles(port);
>  
>  	kobject_uevent(&port->dev.kobj, KOBJ_CHANGE);
>  }
> @@ -405,6 +412,34 @@ struct typec_port *typec_dev2port(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(typec_dev2port);
>  
> +/* --------------------------------------- */
> +/* Driver callbacks to report role updates */
> +
> +void typec_set_usb_role(struct typec_port *port, enum typec_usb_role role)
> +{
> +	port->usb_role = role;
> +}
> +EXPORT_SYMBOL(typec_set_usb_role);
> +
> +void typec_set_pwr_role(struct typec_port *port, enum typec_pwr_role role)
> +{
> +	port->pwr_role = role;
> +}
> +EXPORT_SYMBOL(typec_set_pwr_role);
> +
> +void typec_set_vconn_role(struct typec_port *port, enum typec_pwr_role role)
> +{
> +	port->vconn_role = role;
> +}
> +EXPORT_SYMBOL(typec_set_vconn_role);
> +
> +void typec_set_pwr_opmode(struct typec_port *port,
> +			  enum typec_pwr_opmode opmode)
> +{
> +	port->pwr_opmode = opmode;
> +}
> +EXPORT_SYMBOL(typec_set_pwr_opmode);
> +
>  /* -------------------------------- */
>  /* Alternate Modes */
>  
> @@ -451,7 +486,7 @@ typec_altmode_desc_show(struct device *dev, struct device_attribute *attr,
>  	struct typec_mode *mode = container_of(attr, struct typec_mode,
>  					       desc_attr);
>  
> -	return sprintf(buf, "%s\n", mode->desc);
> +	return sprintf(buf, "%s\n", mode->desc ? mode->desc : "");
>  }
>  
>  static ssize_t
> @@ -561,6 +596,9 @@ void typec_unregister_altmodes(struct typec_altmode *alt_modes)
>  {
>  	struct typec_altmode *alt;
>  
> +	if (!alt_modes)
> +		return;
> +
>  	for (alt = alt_modes; alt->svid; alt++)
>  		device_unregister(&alt->dev);
>  }
> @@ -581,7 +619,7 @@ current_usb_data_role_store(struct device *dev, struct device_attribute *attr,
>  		return -EOPNOTSUPP;
>  	}
>  
> -	if (!port->cap->dr_swap) {
> +	if (!port->cap->dr_set) {
>  		dev_warn(dev, "data role swapping not supported\n");
>  		return -EOPNOTSUPP;
>  	}
> @@ -593,10 +631,7 @@ current_usb_data_role_store(struct device *dev, struct device_attribute *attr,
>  	else
>  		return -EINVAL;
>  
> -	if (port->usb_role == role || !port->partner)
> -		return size;
> -
> -	ret = port->cap->dr_swap(port);
> +	ret = port->cap->dr_set(port, port->driver_data, role);
>  	if (ret)
>  		return ret;
>  
> @@ -655,10 +690,7 @@ current_data_role_store(struct device *dev, struct device_attribute *attr,
>  	else
>  		return -EINVAL;
>  
> -	if (port->fixed_role == role)
> -		return size;
> -
> -	ret = port->cap->fix_role(port, role);
> +	ret = port->cap->fix_role(port, port->driver_data, role);
>  	if (ret)
>  		return ret;
>  
> @@ -688,7 +720,7 @@ static ssize_t current_power_role_store(struct device *dev,
>  		return -EOPNOTSUPP;
>  	}
>  
> -	if (!port->cap->pr_swap) {
> +	if (!port->cap->pr_set) {
>  		dev_warn(dev, "power role swapping not supported\n");
>  		return -EOPNOTSUPP;
>  	}
> @@ -705,10 +737,7 @@ static ssize_t current_power_role_store(struct device *dev,
>  	else
>  		return -EINVAL;
>  
> -	if (port->pwr_role == role || !port->partner)
> -		return size;
> -
> -	ret = port->cap->pr_swap(port);
> +	ret = port->cap->pr_set(port, port->driver_data, role);
>  	if (ret)
>  		return ret;
>  
> @@ -762,6 +791,54 @@ static ssize_t power_operation_mode_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(power_operation_mode);
>  
> +static ssize_t current_vconn_role_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t size)
> +{
> +	struct typec_port *port = to_typec_port(dev);
> +	enum typec_pwr_role role;
> +	int ret;
> +
> +	if (!port->cap->usb_pd) {
> +		dev_dbg(dev, "vconn swap only supported with USB PD\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (!port->cap->vconn_set) {
> +		dev_warn(dev, "vconn swapping not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (!strncmp(buf, "source", 6))
> +		role = TYPEC_PWR_SOURCE;
> +	else if (!strncmp(buf, "sink", 4))
> +		role = TYPEC_PWR_SINK;
> +	else
> +		return -EINVAL;
> +
> +	ret = port->cap->vconn_set(port, port->driver_data, role);
> +	if (ret)
> +		return ret;
> +
> +	return size;
> +}
> +
> +static ssize_t current_vconn_role_show(struct device *dev,
> +				       struct device_attribute *attr, char *buf)
> +{
> +	struct typec_port *port = to_typec_port(dev);
> +
> +	switch (port->vconn_role) {
> +	case TYPEC_PWR_SOURCE:
> +		return sprintf(buf, "source\n");
> +	case TYPEC_PWR_SINK:
> +		return sprintf(buf, "sink\n");
> +	default:
> +		return sprintf(buf, "unknown\n");
> +	};
> +}
> +static DEVICE_ATTR_RW(current_vconn_role);
> +
>  static ssize_t supports_audio_accessory_show(struct device *dev,
>  					     struct device_attribute *attr,
>  					     char *buf)
> @@ -795,6 +872,7 @@ static DEVICE_ATTR_RO(supports_usb_power_delivery);
>  static struct attribute *typec_attrs[] = {
>  	&dev_attr_current_data_role.attr,
>  	&dev_attr_current_power_role.attr,
> +	&dev_attr_current_vconn_role.attr,
>  	&dev_attr_current_usb_data_role.attr,
>  	&dev_attr_power_operation_mode.attr,
>  	&dev_attr_supported_data_roles.attr,
> @@ -862,7 +940,8 @@ static struct device_type typec_port_dev_type = {
>  };
>  
>  struct typec_port *typec_register_port(struct device *dev,
> -				       struct typec_capability *cap)
> +				       struct typec_capability *cap,
> +				       void *driver_data)
>  {
>  	struct typec_port *port;
>  	int ret;
> @@ -880,6 +959,7 @@ struct typec_port *typec_register_port(struct device *dev,
>  
>  	port->id = id;
>  	port->cap = cap;
> +	port->driver_data = driver_data;
>  	port->dev.type = &typec_port_dev_type;
>  	port->dev.class = &typec_class;
>  	port->dev.parent = dev;
> @@ -888,6 +968,8 @@ struct typec_port *typec_register_port(struct device *dev,
>  
>  	port->fixed_role = port->cap->role;
>  
> +	typec_init_roles(port);
> +
>  	ret = device_register(&port->dev);
>  	if (ret) {
>  		ida_simple_remove(&typec_index_ida, id);
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 86e5c867800b..d16a38de57ac 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -168,9 +168,9 @@ struct typec_partner {
>   * @audio_accessory: Audio Accessory Adapter Mode support
>   * @debug_accessory: Debug Accessory Mode support
>   * @fix_role: Set a fixed data role for DRP port
> - * @dr_swap: Data Role Swap support
> - * @pr_swap: Power Role Swap support
> - * @vconn_swap: VCONN Swap support
> + * @dr_set: Set Data Role
> + * @pr_set: Set Power Role
> + * @vconn_set: Set VCONN Role
>   * @activate_mode: Enter/exit given Alternate Mode
>   *
>   * Static capabilities of a single USB Type-C port.
> @@ -182,14 +182,14 @@ struct typec_capability {
>  	unsigned int		audio_accessory:1;
>  	unsigned int		debug_accessory:1;
>  
> -	int			(*fix_role)(struct typec_port *,
> +	int			(*fix_role)(struct typec_port *, void *,
>  					    enum typec_data_role);
>  
> -	int			(*dr_swap)(struct typec_port *);
> -	int			(*pr_swap)(struct typec_port *);
> -	int			(*vconn_swap)(struct typec_port *);
> +	int			(*dr_set)(struct typec_port *, void *, enum typec_usb_role);
> +	int			(*pr_set)(struct typec_port *, void *, enum typec_pwr_role);
> +	int			(*vconn_set)(struct typec_port *, void *, enum typec_pwr_role);
>  
> -	int			(*activate_mode)(struct typec_altmode *,
> +	int			(*activate_mode)(struct typec_altmode *, void *,
>  						 int mode, int activate);
>  };
>  
> @@ -217,7 +217,8 @@ struct typec_connection {
>  };
>  
>  struct typec_port *typec_register_port(struct device *dev,
> -				       struct typec_capability *cap);
> +				       struct typec_capability *cap,
> +				       void *driver_data);
>  void typec_unregister_port(struct typec_port *port);
>  
>  int typec_connect(struct typec_port *port, struct typec_connection *con);
> @@ -227,4 +228,11 @@ void typec_disconnect(struct typec_port *port);
>  struct device *typec_port2dev(struct typec_port *port);
>  struct typec_port *typec_dev2port(struct device *dev);
>  
> +/* Callbacks from driver */
> +
> +void typec_set_usb_role(struct typec_port *, enum typec_usb_role);
> +void typec_set_pwr_role(struct typec_port *, enum typec_pwr_role);
> +void typec_set_vconn_role(struct typec_port *, enum typec_pwr_role);
> +void typec_set_pwr_opmode(struct typec_port *, enum typec_pwr_opmode);
> +
>  #endif /* __LINUX_USB_TYPEC_H */
> -- 
> 2.5.0

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1408095 — Re: [RFC PATCH] usb: typec: Various API updates and fixes

FromGuenter Roeck <linux@roeck-us.net>
Date2016-05-27 16:10 +0200
SubjectRe: [RFC PATCH] usb: typec: Various API updates and fixes
Message-ID<rDqCC-4tT-17@gated-at.bofh.it>
In reply to#1407891
On 05/27/2016 12:55 AM, Heikki Krogerus wrote:
> Hi,
>

[ ... ]

>> ---
>> This patch applies on top of '[RFC PATCHv2] usb: USB Type-C Connector Class'
>> from Heikki Krogerus. It provided the changes I made to get the code
>> operational.
>>
>>   drivers/usb/type-c/typec.c | 134 ++++++++++++++++++++++++++++++++++++---------
>>   include/linux/usb/typec.h  |  26 ++++++---
>>   2 files changed, 125 insertions(+), 35 deletions(-)
>
> Thanks Guenter. After a quick glance looks good to me.
>
My pleasure.

> I think I'll create a development branch to my github tree for this
> class and add this on top of the original v2 patch as is. Just to have
> some history for as long we develop this thing. Though I'm not sure
> how useful it is in this case. I think we need to put this together
> fast. I'm thinking we need to target v4.8 with this.
>

Yes, agreed.

> I'll merge this into any case to v3, and I'll send on Monday.
>
Sounds good.

Couple of additional comments.

I don't really know what to do with the 'desc' field in struct typec_mode
for modes received from the partner. I wonder if it makes sense to display
it for partner modes.

Also, there is currently no attribute to show the partner identity (data
received from the Discover Identity command). Would it make sense to add
an 'identity' attribute to the partner device (plus an associated API
function to set it) ?

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

#1409074 — Re: [RFC PATCH] usb: typec: Various API updates and fixes

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-05-30 14:50 +0200
SubjectRe: [RFC PATCH] usb: typec: Various API updates and fixes
Message-ID<rEuNQ-4g8-9@gated-at.bofh.it>
In reply to#1408095
Hi guys,

On Fri, May 27, 2016 at 07:06:41AM -0700, Guenter Roeck wrote:
> On 05/27/2016 12:55 AM, Heikki Krogerus wrote:
> > I'll merge this into any case to v3, and I'll send on Monday.
> > 
> Sounds good.
> 
> Couple of additional comments.
> 
> I don't really know what to do with the 'desc' field in struct typec_mode
> for modes received from the partner. I wonder if it makes sense to display
> it for partner modes.

The idea comes straight from Billboard class where every mode has an
index to an _optional_ string. Those desc should ultimately come from
the altmode bus driver, and I think never from the port drivers.

> Also, there is currently no attribute to show the partner identity (data
> received from the Discover Identity command). Would it make sense to add
> an 'identity' attribute to the partner device (plus an associated API
> function to set it) ?

The problem is that we can only use it when the partner and our port
are both PD capable. Details about PDUSB devices that are attached are
out side the scope of this interface IMO, and need to be in any case
handled in USB core by using the PD Class specific descriptors etc.,
and possible with the help of the future PD stack that you are working
on. It's an other task. So this interface IMO should expose only the
Type-C specific details about the ports and partners. How about if we
just add an attribute "usb_communication_capable" for the partners?
That is something that we should be able to always determine, even
without USB PD.


Thanks,

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1409108

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-05-30 15:30 +0200
Message-ID<rEvqx-4K0-11@gated-at.bofh.it>
In reply to#1403714

[Multipart message — attachments visible in raw view] — view raw

Hi guys,

I'm attaching a diff instead of full v3. I'm not yet adding attributes
for the reset and cable_reset. I still don't understand what is the
case where the userspace would need to be able to tricker reset? Why
isn't it enough for the userspace to be able to enter/exit modes?
Oliver! Can you please comment?

Guenter, I removed the driver_data you proposed and changed the API
so that the struct typec_capability is passed to the function pointers
instead of struct typec_port. The driver_data pointer felt a bit
clumsy to me, as it was something extra that always had to be passed
to every function. Let me know if it's not OK for you.

I also made a change to the partner devices so that they always have
the port as their parent. That will have an effect on the location
where the partner device is exposed in sysfs (so now always under the
port). And because of that, I would like to get an OK from you guys.

I'm a bit concerned about the current API for adding the alternate
modes. Since we are passing the parent for an alternate mode as
struct device, it makes it possible for the caller to place it into
some wrong place. But I guess we can change the API even later.

We also need to decide how the alternate modes a port support are
exposed to the userspace. Do we just assume the port drivers will
create them as devices under the port device itself, just like
alternate modes of partners and cable plugs are exposed under the
partners and cable plugs? That works for me, but again, the class does
not have any effect on that, and it will be just a guideline. Maybe
we can add some kind of helpers and force the port drivers to use
them.

And finally, mostly as a reminder for myself. I didn't yet add
attributes for Try.SRC/SNK. So can we make it something like
"preferred_role" like I think you proposed Guenter? The
"current_data_role" I'm dropping.

So to summarize. There are still open questions regarding the required
attributes and attribute names and locations. Let's agree on those
quickly and then we can polish the patch.


Thanks,

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1409146

FromOliver Neukum <oneukum@suse.com>
Date2016-05-30 16:10 +0200
Message-ID<rEw3f-5dn-5@gated-at.bofh.it>
In reply to#1409108
On Mon, 2016-05-30 at 16:19 +0300, Heikki Krogerus wrote:
> Hi guys,
> 
> I'm attaching a diff instead of full v3. I'm not yet adding attributes
> for the reset and cable_reset. I still don't understand what is the
> case where the userspace would need to be able to tricker reset? Why
> isn't it enough for the userspace to be able to enter/exit modes?
> Oliver! Can you please comment?

1. Because we need error handling.
   Devices crash. Cables will crash. We will get out of sync.
   You never put yourself in a place where you cannot handle an
   IO error.
2. Because it is in the spec. We do not second guess the spec.
   We implement it.

> I also made a change to the partner devices so that they always have
> the port as their parent. That will have an effect on the location
> where the partner device is exposed in sysfs (so now always under the
> port). And because of that, I would like to get an OK from you guys.

It is not very important. i am fine with it.

> I'm a bit concerned about the current API for adding the alternate
> modes. Since we are passing the parent for an alternate mode as
> struct device, it makes it possible for the caller to place it into
> some wrong place. But I guess we can change the API even later.
> 
> We also need to decide how the alternate modes a port support are
> exposed to the userspace. Do we just assume the port drivers will
> create them as devices under the port device itself, just like
> alternate modes of partners and cable plugs are exposed under the
> partners and cable plugs? That works for me, but again, the class does
> not have any effect on that, and it will be just a guideline. Maybe
> we can add some kind of helpers and force the port drivers to use
> them.

What are the alternatives?

> And finally, mostly as a reminder for myself. I didn't yet add
> attributes for Try.SRC/SNK. So can we make it something like
> "preferred_role" like I think you proposed Guenter? The
> "current_data_role" I'm dropping.

That sounds good.

	Regards
		Oliver

[toc] | [prev] | [next] | [standalone]

#1409990

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-05-31 10:40 +0200
Message-ID<rENnr-iy-19@gated-at.bofh.it>
In reply to#1409146
Hi Oliver,

On Mon, May 30, 2016 at 03:59:27PM +0200, Oliver Neukum wrote:
> On Mon, 2016-05-30 at 16:19 +0300, Heikki Krogerus wrote:
> > Hi guys,
> > 
> > I'm attaching a diff instead of full v3. I'm not yet adding attributes
> > for the reset and cable_reset. I still don't understand what is the
> > case where the userspace would need to be able to tricker reset? Why
> > isn't it enough for the userspace to be able to enter/exit modes?
> > Oliver! Can you please comment?
> 
> 1. Because we need error handling.
>    Devices crash. Cables will crash. We will get out of sync.
>    You never put yourself in a place where you cannot handle an
>    IO error.
> 2. Because it is in the spec. We do not second guess the spec.
>    We implement it.

Error conditions and crashes are the responsibility of the USB PD
stack, not userspace. In those cases the stack can not wait for a
command from the userspace. So for example if a timer like
NoResponseTimer times out, the stack an its state machines will have
to take care of the reset quite independently.

If you get out of sync with an alternate mode, you reset that specific
alternate mode by exiting and re-entering it, and you do not reset the
entire PD connection, port, partner or cable.

The resets from userspace would be purely unsolicited. What would the
cases where we would need to tricker a reset like that?

I want to be careful with exposing reset to userspace. Reset in USB PD
is not just an IO related thing. When you tricker a reset with USB PD,
even if it's a soft reset, it may lead into hard reset, which may
potentially lead into sudden voltage and current drop, which may lead
into the entire system crashing. We really need to understand the
cases where it would be necessary to tricker a reset from userspace.
Right now I don't see any.

> > I also made a change to the partner devices so that they always have
> > the port as their parent. That will have an effect on the location
> > where the partner device is exposed in sysfs (so now always under the
> > port). And because of that, I would like to get an OK from you guys.
> 
> It is not very important. i am fine with it.
> 
> > I'm a bit concerned about the current API for adding the alternate
> > modes. Since we are passing the parent for an alternate mode as
> > struct device, it makes it possible for the caller to place it into
> > some wrong place. But I guess we can change the API even later.
> > 
> > We also need to decide how the alternate modes a port support are
> > exposed to the userspace. Do we just assume the port drivers will
> > create them as devices under the port device itself, just like
> > alternate modes of partners and cable plugs are exposed under the
> > partners and cable plugs? That works for me, but again, the class does
> > not have any effect on that, and it will be just a guideline. Maybe
> > we can add some kind of helpers and force the port drivers to use
> > them.
> 
> What are the alternatives?

Can we make a group for them under the port device somehow? Like the
supported_alternate_modes I proposed. I guess it's not possible to add
devices to a specific group in sysfs. And would it even be useful.


Thanks,

-- 
heikki

[toc] | [prev] | [standalone]

Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel

csiph-web