Hi guys,
On Tue, Aug 30, 2016 at 06:47:41AM -0700, Guenter Roeck wrote:
> On 08/30/2016 06:11 AM, Heikki Krogerus wrote:
> > Hi,
> >
> > On Tue, Aug 30, 2016 at 02:49:50PM +0300, Heikki Krogerus wrote:
> > > On Tue, Aug 30, 2016 at 01:16:46PM +0200, Oliver Neukum wrote:
> > > > Error reporting does not require a synchronous operation. Reporting
> > > > it in the next read() or write() and making it pollable is perfectly
> > > > viable. It just must not be silently dropped.
> > >
> > > OK, I think I got it. I need to document that. I'll also add
> > > get_pr/dr/vconn hooks to the API for getting the status.
> >
> > Would the attached diff do the trick? It also includes the other
> > suggestions from Guenter.
> >
>
> It is not at all what I meant or asked for :-(. I'll have a closer
> look into the latest patch set later today.
An other attempt. This one has just the suggestions from you Guenter
(including the supports_usb_power_delivery attibute for cables as
well), and update to the ABI document.
Let me know if it's OK.
Thanks,
--
heikki
diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index 53fdd11..6b60dd4 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -6,10 +6,10 @@ Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
Description:
The current USB data role the port is operating in. This
attribute can be used for requesting data role swapping on the
- port. Swapping is only supported as an asynchronous operation
- and requires polling of the attribute in order to know the
- result, so successful write operation does not mean successful
- swap.
+ port. Swapping is supported as synchronous operation, so
+ write(2) to the attribute will not return until the operation
+ has finished. The attribute is notified about role changes so
+ that poll(2) on the attribute wakes up.
Valid values:
- host
@@ -21,10 +21,10 @@ Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
Description:
The current power role of the port. This attribute can be used
to request power role swap on the port when the port supports
- USB Power Delivery. Swapping is only supported as an
- asynchronous operation and requires polling of the attribute in
- order to know the result, so successful write operation does not
- mean successful swap.
+ USB Power Delivery. Swapping is supported as synchronous
+ operation, so write(2) to the attribute will not return until
+ the operation has finished. The attribute is notified about role
+ changes so that poll(2) on the attribute wakes up.
Valid values:
- source
@@ -37,6 +37,10 @@ Description:
Shows is the port VCONN Source. This attribute can be used to
request VCONN swap to change the VCONN Source during connection
when both the port and the partner support USB Power Delivery.
+ Swapping is supported as synchronous operation, so write(2) to
+ the attribute will not return until the operation has finished.
+ The attribute is notified about VCONN source changes so that
+ poll(2) on the attribute wakes up.
Valid values are:
- 0 when the port is not the VCONN Source
@@ -101,6 +105,7 @@ Description:
Valid values:
- source
- sink
+ - source, sink (DRP as defined in USB Type-C specification v1.2)
What: /sys/class/typec/<port>/supports_usb_power_delivery
Date: June 2016
@@ -113,28 +118,21 @@ Description:
USB Type-C partner devices (eg. /sys/class/typec/usbc0-partner/)
-What: /sys/class/typec/<port>-partner/accessory
+What: /sys/class/typec/<port>-partner/accessory_mode
Date: June 2016
Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
Description:
- The attribute is visible only when the partner's type is
- "Accessory". The type can be read from its own attribute.
+ Shows the Accessory Mode name, or "no" when the partner does not
+ support Accesory Modes. The Accessory Modes are defined in USB
+ Type-C Specification.
- Shows the name of the Accessory Mode. The Accessory Modes are
- defined in USB Type-C Specification.
-
-What: /sys/class/typec/<port>-partner/type
+What: /sys/class/typec/<port>-partner/supports_usb_power_delivery
Date: June 2016
Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
Description:
- Shows the type of the partner. Can be one of the following:
- - USB - When the partner is normal USB host/peripheral.
- - Charger - When the partner has been identified as dedicated
- charger.
- - Alternate Mode - When the partner supports Alternate Modes.
- - Accessory - When the partner is one of the accessories with
- specific Accessory Mode defined in USB Type-C
- specification.
+ Shows if the partner supports USB Power Delivery:
+ - 0 when USB Power Delivery is not supported
+ - 1 when USB Power Delivery is supported
USB Type-C cable devices (eg. /sys/class/typec/usbc0-cable/)
@@ -165,6 +163,14 @@ Description:
- Type-C - USB Type-C
- Captive - Non-standard
+What: /sys/class/typec/<port>-cable/supports_usb_power_delivery
+Date: June 2016
+Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
+Description:
+ Shows if the cable supports USB Power Delivery:
+ - 0 when USB Power Delivery is not supported
+ - 1 when USB Power Delivery is supported
+
Alternate Mode devices (For example,
/sys/class/typec/usbc0-partner/usbc0-partner.svid:xxxx/). The ports, partners
@@ -177,7 +183,15 @@ Description:
Shows if the mode is active or not. The attribute can be used
for entering/exiting the mode with partners and cable plugs, and
with the port alternate modes it can be used for disabling
- support for specific alternate modes.
+ support for specific alternate modes. Entering/exiting modes is
+ supported as synchronous operation so write(2) to the attribute
+ does not return until the enter/exit mode operation has
+ finished. The attribute is notified when the mode is
+ entered/exited so poll(2) on the attribute wakes up.
+
+ Valid values:
+ - 0 when the mode is deactive
+ - 1 when the mode is active
What: /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/description
Date: June 2016
diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 366a04c..ef2dbb8 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -42,10 +42,9 @@ static struct class typec_class = {
};
static const char * const typec_accessory_modes[] = {
- [TYPEC_ACCESSORY_NONE] = "none",
- [TYPEC_ACCESSORY_AUDIO] = "Audio",
- [TYPEC_ACCESSORY_DEBUG] = "Debug",
- [TYPEC_ACCESSORY_DAUDIO] = "Digital Audio",
+ [TYPEC_ACCESSORY_NONE] = "no",
+ [TYPEC_ACCESSORY_AUDIO] = "Audio Adapter Accessory Mode",
+ [TYPEC_ACCESSORY_DEBUG] = "Debug Accessory Mode",
};
/* ------------------------------------------------------------------------- */
@@ -55,43 +54,34 @@ static void typec_dev_release(struct device *dev)
{
}
-static const char * const typec_partner_types[] = {
- [TYPEC_PARTNER_USB] = "USB",
- [TYPEC_PARTNER_CHARGER] = "Charger",
- [TYPEC_PARTNER_ALTMODE] = "Alternate Mode",
- [TYPEC_PARTNER_ACCESSORY] = "Accessory",
-};
-
-static ssize_t
-partner_type_show(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t partner_usb_pd_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
- struct typec_partner *partner = container_of(dev, struct typec_partner,
- dev);
+ struct typec_partner *p = container_of(dev, struct typec_partner, dev);
- return sprintf(buf, "%s\n", typec_partner_types[partner->type]);
+ return sprintf(buf, "%d\n", p->usb_pd);
}
-static struct device_attribute dev_attr_partner_type = {
+static struct device_attribute dev_attr_partner_usb_pd = {
.attr = {
- .name = "type",
+ .name = "supports_usb_power_delivery",
.mode = S_IRUGO,
},
- .show = partner_type_show,
+ .show = partner_usb_pd_show,
};
static ssize_t
partner_accessory_mode_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- struct typec_partner *partner = container_of(dev, struct typec_partner,
- dev);
+ struct typec_partner *p = container_of(dev, struct typec_partner, dev);
- return sprintf(buf, "%s\n", typec_accessory_modes[partner->accessory]);
+ return sprintf(buf, "%s\n", typec_accessory_modes[p->accessory]);
}
static struct device_attribute dev_attr_partner_accessory = {
.attr = {
- .name = "accessory",
+ .name = "accessory_mode",
.mode = S_IRUGO,
},
.show = partner_accessory_mode_show,
@@ -99,27 +89,12 @@ static struct device_attribute dev_attr_partner_accessory = {
static struct attribute *typec_partner_attrs[] = {
&dev_attr_partner_accessory.attr,
- &dev_attr_partner_type.attr,
+ &dev_attr_partner_usb_pd.attr,
NULL
};
-static umode_t
-partner_is_visible(struct kobject *kobj, struct attribute *attr, int n)
-{
- struct device *dev = container_of(kobj, struct device, kobj);
- struct typec_partner *partner = container_of(dev, struct typec_partner,
- dev);
-
- if (attr == &dev_attr_partner_accessory.attr &&
- partner->type != TYPEC_PARTNER_ACCESSORY)
- return 0;
-
- return attr->mode;
-}
-
static struct attribute_group typec_partner_group = {
.attrs = typec_partner_attrs,
- .is_visible = partner_is_visible,
};
static const struct attribute_group *typec_partner_groups[] = {
@@ -215,6 +190,23 @@ static struct device_attribute dev_attr_cable_active = {
.show = cable_active_show,
};
+static ssize_t
+cable_usb_pd_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct typec_cable *cable = container_of(dev, struct typec_cable, dev);
+
+ return sprintf(buf, "%d\n", cable->usb_pd);
+}
+
+static struct device_attribute dev_attr_cable_usb_pd = {
+ .attr = {
+ .name = "supports_usb_power_delivery",
+ .mode = S_IRUGO,
+ },
+ .show = cable_usb_pd_show,
+};
+
static const char * const typec_plug_types[] = {
[USB_PLUG_NONE] = "unknown",
[USB_PLUG_TYPE_A] = "Type-A",
@@ -242,6 +234,7 @@ static struct device_attribute dev_attr_plug_type = {
static struct attribute *typec_cable_attrs[] = {
&dev_attr_cable_active.attr,
+ &dev_attr_cable_usb_pd.attr,
&dev_attr_plug_type.attr,
NULL
};
@@ -925,15 +918,15 @@ static ssize_t supported_accessory_modes_show(struct device *dev,
char *buf)
{
struct typec_port *port = to_typec_port(dev);
- enum typec_accessory *accessory;
ssize_t ret = 0;
int i;
- if (port->cap->accessory)
- for (accessory = port->cap->accessory, i = 0;
- i < port->cap->num_accessory; accessory++, i++)
- ret += sprintf(buf, "%s\n",
- typec_accessory_modes[*accessory]);
+ if (!port->cap->accessory[0])
+ return 0;
+
+ for (i = 0; port->cap->accessory[i]; i++)
+ ret += sprintf(buf + ret, "%s\n",
+ typec_accessory_modes[port->cap->accessory[i]]);
return ret;
}
static DEVICE_ATTR_RO(supported_accessory_modes);
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index eda6747..1270ca1 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -13,13 +13,6 @@ enum typec_port_type {
TYPEC_PORT_DRP,
};
-enum typec_partner_type {
- TYPEC_PARTNER_USB,
- TYPEC_PARTNER_CHARGER,
- TYPEC_PARTNER_ALTMODE,
- TYPEC_PARTNER_ACCESSORY,
-};
-
enum typec_plug_type {
USB_PLUG_NONE,
USB_PLUG_TYPE_A,
@@ -49,7 +42,6 @@ enum typec_accessory {
TYPEC_ACCESSORY_NONE,
TYPEC_ACCESSORY_AUDIO,
TYPEC_ACCESSORY_DEBUG,
- TYPEC_ACCESSORY_DAUDIO,
};
/*
@@ -177,7 +169,6 @@ struct typec_cable {
*/
struct typec_partner {
struct device dev;
- enum typec_partner_type type;
unsigned int usb_pd:1;
u32 vdo;
enum typec_accessory accessory;
@@ -188,8 +179,7 @@ struct typec_partner {
* struct typec_capability - USB Type-C Port Capabilities
* @role: DFP (Host-only), UFP (Device-only) or DRP (Dual Role)
* @usb_pd: USB Power Delivery support
- * @accessory: Supported Accessory Modes
- * @num_accessory: Number of supported Accessory Modes
+ * @accessory: Supported Accessory Modes (NULL terminated array)
* @alt_modes: Alternate Modes the connector supports (null terminated)
* @try_role: Set a fixed data role for DRP port
* @dr_set: Set Data Role
@@ -203,7 +193,6 @@ struct typec_capability {
enum typec_port_type type;
unsigned int usb_pd:1;
enum typec_accessory *accessory;
- unsigned int num_accessory;
struct typec_altmode *alt_modes;
int (*try_role)(const struct typec_capability *,
