[PATCH 00/15] USB Type-C changes

[PATCH 00/15] USB Type-C changes

[Heikki Krogerus]

Hi,

This series adds a number of smaller improvements to the USB Type-C
Connector class code, including new helpers for cables, new
definitions for the latest standard versions, support for the Enter
Mode VDO, etc.

The series also includes patches that are meant to (partially) prepare
the subsystem for USB4 support, and for Thundebolt3 alternate mode.
Especially the new Enter_Mode Message that USB4 requires is considered
with the definitions for the "USB mode" (which can be usb2, usb3 or
usb4), which is one of the fields in that message. Later we will need
to allow the user to control the USB mode, but I'm not including the
patches that add the attributes for that in this series. I'll send
them separately as RFC following this series.

thanks,

Heikki Krogerus (15):
  usb: typec: Block mode entry if the port has the mode disabled
  usb: typec: Add parameter for the VDO to typec_altmode_enter()
  usb: typec: More API for cable handling
  usb: typec: Make the attributes read-only when writing is not possible
  usb: typec: Hide the port_type attribute when it's not supported
  usb: typec: Allow power role swapping even without USB PD
  usb: typec: Fix the description of struct typec_capability
  usb: pd: Add definitions for the Enter_USB message
  usb: pd: Add definition for DFP and UFP1 VDOs
  usb: typec: Add the Product Type VDOs to struct usb_pd_identity
  usb: typec: Add definitions for the latest specification releases
  usb: typec: Give the mux drivers all the details regarding the port
    state
  usb: typec: Provide definitions for the USB modes
  usb: typec: Add member for the supported USB modes to struct
    typec_capability
  usb: typec: ucsi: Store the supported USB modes

 Documentation/ABI/testing/sysfs-class-typec |  14 +-
 drivers/usb/typec/altmodes/displayport.c    |   5 +-
 drivers/usb/typec/bus.c                     |  40 +++--
 drivers/usb/typec/class.c                   | 174 +++++++++++++-------
 drivers/usb/typec/mux/pi3usb30532.c         |   5 +-
 drivers/usb/typec/tcpm/tcpm.c               |   6 +-
 drivers/usb/typec/ucsi/displayport.c        |   2 +-
 drivers/usb/typec/ucsi/ucsi.c               |   5 +
 include/linux/usb/pd.h                      |  33 +++-
 include/linux/usb/pd_vdo.h                  |  32 ++++
 include/linux/usb/typec.h                   |  17 +-
 include/linux/usb/typec_altmode.h           |  20 ++-
 include/linux/usb/typec_mux.h               |  10 +-
 13 files changed, 273 insertions(+), 90 deletions(-)

-- 
2.24.1

[PATCH 01/15] usb: typec: Block mode entry if the port has the mode disabled

[Heikki Krogerus]

Originally the port drivers were expected to check does the
connector have the mode enabled or disabled when the alt
mode drivers attempted to enter the mode, but since
typec_altmode_enter() puts the connector into USB Safe
State before calling the port driver, it really has to do
the check on its own, and before changing the state.
Otherwise the connector may be left in USB Safe State if the
port driver does not move it back to normal USB operation
when the mode is disabled.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/bus.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index 74cb3c2ecb34..4116379fbd68 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -101,6 +101,9 @@ int typec_altmode_enter(struct typec_altmode *adev)
 	if (!pdev->ops || !pdev->ops->enter)
 		return -EOPNOTSUPP;
 
+	if (is_typec_port(pdev->dev.parent) && !pdev->active)
+		return -EPERM;
+
 	/* Moving to USB Safe State */
 	ret = typec_altmode_set_state(adev, TYPEC_STATE_SAFE);
 	if (ret)
-- 
2.24.1

[PATCH 02/15] usb: typec: Add parameter for the VDO to typec_altmode_enter()

[Heikki Krogerus]

Enter Mode Command may contain one VDO.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/altmodes/displayport.c | 5 +++--
 drivers/usb/typec/bus.c                  | 8 +++++---
 drivers/usb/typec/tcpm/tcpm.c            | 6 +++---
 drivers/usb/typec/ucsi/displayport.c     | 2 +-
 include/linux/usb/typec_altmode.h        | 4 ++--
 5 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 4092248a5936..0edfb89e04a8 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -188,7 +188,7 @@ static void dp_altmode_work(struct work_struct *work)
 
 	switch (dp->state) {
 	case DP_STATE_ENTER:
-		ret = typec_altmode_enter(dp->alt);
+		ret = typec_altmode_enter(dp->alt, NULL);
 		if (ret)
 			dev_err(&dp->alt->dev, "failed to enter mode\n");
 		break;
@@ -306,7 +306,8 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
 
 static int dp_altmode_activate(struct typec_altmode *alt, int activate)
 {
-	return activate ? typec_altmode_enter(alt) : typec_altmode_exit(alt);
+	return activate ? typec_altmode_enter(alt, NULL) :
+			  typec_altmode_exit(alt);
 }
 
 static const struct typec_altmode_ops dp_altmode_ops = {
diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index 4116379fbd68..76e024be2502 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -84,12 +84,14 @@ EXPORT_SYMBOL_GPL(typec_altmode_notify);
 /**
  * typec_altmode_enter - Enter Mode
  * @adev: The alternate mode
+ * @vdo: VDO for the Enter Mode command
  *
  * The alternate mode drivers use this function to enter mode. The port drivers
  * use this to inform the alternate mode drivers that the partner has initiated
- * Enter Mode command.
+ * Enter Mode command. If the alternate mode does not require VDO, @vdo must be
+ * NULL.
  */
-int typec_altmode_enter(struct typec_altmode *adev)
+int typec_altmode_enter(struct typec_altmode *adev, u32 *vdo)
 {
 	struct altmode *partner = to_altmode(adev)->partner;
 	struct typec_altmode *pdev = &partner->adev;
@@ -110,7 +112,7 @@ int typec_altmode_enter(struct typec_altmode *adev)
 		return ret;
 
 	/* Enter Mode */
-	return pdev->ops->enter(pdev);
+	return pdev->ops->enter(pdev, vdo);
 }
 EXPORT_SYMBOL_GPL(typec_altmode_enter);
 
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 56fc356bc55c..f3087ef8265c 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -1475,16 +1475,16 @@ static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
 	return 0;
 }
 
-static int tcpm_altmode_enter(struct typec_altmode *altmode)
+static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
 {
 	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
 	u32 header;
 
 	mutex_lock(&port->lock);
-	header = VDO(altmode->svid, 1, CMD_ENTER_MODE);
+	header = VDO(altmode->svid, vdo ? 2 : 1, CMD_ENTER_MODE);
 	header |= VDO_OPOS(altmode->mode);
 
-	tcpm_queue_vdm(port, header, NULL, 0);
+	tcpm_queue_vdm(port, header, vdo, vdo ? 1 : 0);
 	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
 	mutex_unlock(&port->lock);
 
diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index d4d5189edfb8..0f1273ae086c 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -45,7 +45,7 @@ struct ucsi_dp {
  * -EOPNOTSUPP.
  */
 
-static int ucsi_displayport_enter(struct typec_altmode *alt)
+static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
 {
 	struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
 	struct ucsi *ucsi = dp->con->ucsi;
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index 9a88c74a1d0d..fc57fd88004f 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -55,7 +55,7 @@ static inline void *typec_altmode_get_drvdata(struct typec_altmode *altmode)
  * @activate: User callback for Enter/Exit Mode
  */
 struct typec_altmode_ops {
-	int (*enter)(struct typec_altmode *altmode);
+	int (*enter)(struct typec_altmode *altmode, u32 *vdo);
 	int (*exit)(struct typec_altmode *altmode);
 	void (*attention)(struct typec_altmode *altmode, u32 vdo);
 	int (*vdm)(struct typec_altmode *altmode, const u32 hdr,
@@ -65,7 +65,7 @@ struct typec_altmode_ops {
 	int (*activate)(struct typec_altmode *altmode, int activate);
 };
 
-int typec_altmode_enter(struct typec_altmode *altmode);
+int typec_altmode_enter(struct typec_altmode *altmode, u32 *vdo);
 int typec_altmode_exit(struct typec_altmode *altmode);
 void typec_altmode_attention(struct typec_altmode *altmode, u32 vdo);
 int typec_altmode_vdm(struct typec_altmode *altmode,
-- 
2.24.1

[PATCH 03/15] usb: typec: More API for cable handling

[Heikki Krogerus]

Thunderbolt 3, and probable USB4 too, will need to be able
to get details about the cables. Adding typec_cable_get()
function that the alternate mode drivers can use to gain
access to gain access to the cable.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/class.c | 46 +++++++++++++++++++++++++++++++++++++++
 include/linux/usb/typec.h |  4 ++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 91d62276b56f..08923637cd88 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -834,6 +834,52 @@ static const struct device_type typec_cable_dev_type = {
 	.release = typec_cable_release,
 };
 
+static int cable_match(struct device *dev, void *data)
+{
+	return is_typec_cable(dev);
+}
+
+/**
+ * typec_cable_get - Get a reference to the USB Type-C cable
+ * @port: The USB Type-C Port the cable is connected to
+ *
+ * The caller must decrement the reference count with typec_cable_put() after
+ * use.
+ */
+struct typec_cable *typec_cable_get(struct typec_port *port)
+{
+	struct device *dev;
+
+	dev = device_find_child(&port->dev, NULL, cable_match);
+	if (!dev)
+		return NULL;
+
+	return to_typec_cable(dev);
+}
+EXPORT_SYMBOL_GPL(typec_cable_get);
+
+/**
+ * typec_cable_get - Decrement the reference count on USB Type-C cable
+ * @cable: The USB Type-C cable
+ */
+void typec_cable_put(struct typec_cable *cable)
+{
+	put_device(&cable->dev);
+}
+EXPORT_SYMBOL_GPL(typec_cable_put);
+
+/**
+ * typec_cable_is_active - Check is the USB Type-C cable active or passive
+ * @cable: The USB Type-C Cable
+ *
+ * Return 1 if the cable is active or 0 if it's passive.
+ */
+int typec_cable_is_active(struct typec_cable *cable)
+{
+	return cable->active;
+}
+EXPORT_SYMBOL_GPL(typec_cable_is_active);
+
 /**
  * typec_cable_set_identity - Report result from Discover Identity command
  * @cable: The cable updated identity values
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 0f52723a11bd..d95ea0d398b8 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -230,6 +230,10 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
 					 struct typec_cable_desc *desc);
 void typec_unregister_cable(struct typec_cable *cable);
 
+struct typec_cable *typec_cable_get(struct typec_port *port);
+void typec_cable_put(struct typec_cable *cable);
+int typec_cable_is_active(struct typec_cable *cable);
+
 struct typec_plug *typec_register_plug(struct typec_cable *cable,
 				       struct typec_plug_desc *desc);
 void typec_unregister_plug(struct typec_plug *plug);
-- 
2.24.1

[PATCH 04/15] usb: typec: Make the attributes read-only when writing is not possible

[Heikki Krogerus]

For now the read-writable attribute files have made a check
in their store callback function to see does the underlying
port interface support changing the value or not, and when
it didn't, the callbacks returned -EOPNOTSUPP. From user
perspective that is not ideal, as there is no way to know is
changing the value possible beforehand.

Instead of returning -EOPNOTSUPP, making the attribute file
read-only when the operation is not supported.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/class.c | 122 +++++++++++++++++++++-----------------
 1 file changed, 66 insertions(+), 56 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 08923637cd88..3abcfa09ecdf 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -373,12 +373,9 @@ static ssize_t active_store(struct device *dev, struct device_attribute *attr,
 		}
 	}
 
-	/* Note: If there is no driver, the mode will not be entered */
-	if (adev->ops && adev->ops->activate) {
-		ret = adev->ops->activate(adev, enter);
-		if (ret)
-			return ret;
-	}
+	ret = adev->ops->activate(adev, enter);
+	if (ret)
+		return ret;
 
 	return size;
 }
@@ -432,7 +429,28 @@ static struct attribute *typec_altmode_attrs[] = {
 	&dev_attr_vdo.attr,
 	NULL
 };
-ATTRIBUTE_GROUPS(typec_altmode);
+
+static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
+					     struct attribute *attr, int n)
+{
+	struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
+
+	if (attr == &dev_attr_active.attr)
+		if (!adev->ops || !adev->ops->activate)
+			return 0444;
+
+	return attr->mode;
+}
+
+static struct attribute_group typec_altmode_group = {
+	.is_visible = typec_altmode_attr_is_visible,
+	.attrs = typec_altmode_attrs
+};
+
+static const struct attribute_group *typec_altmode_groups[] = {
+	&typec_altmode_group,
+	NULL
+};
 
 static int altmode_id_get(struct device *dev)
 {
@@ -997,16 +1015,6 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
 	int role;
 	int ret;
 
-	if (port->cap->type != TYPEC_PORT_DRP) {
-		dev_dbg(dev, "Preferred role only supported with DRP ports\n");
-		return -EOPNOTSUPP;
-	}
-
-	if (!port->ops || !port->ops->try_role) {
-		dev_dbg(dev, "Setting preferred role not supported\n");
-		return -EOPNOTSUPP;
-	}
-
 	role = sysfs_match_string(typec_roles, buf);
 	if (role < 0) {
 		if (sysfs_streq(buf, "none"))
@@ -1029,9 +1037,6 @@ preferred_role_show(struct device *dev, struct device_attribute *attr,
 {
 	struct typec_port *port = to_typec_port(dev);
 
-	if (port->cap->type != TYPEC_PORT_DRP)
-		return 0;
-
 	if (port->prefer_role < 0)
 		return 0;
 
@@ -1046,20 +1051,11 @@ static ssize_t data_role_store(struct device *dev,
 	struct typec_port *port = to_typec_port(dev);
 	int ret;
 
-	if (!port->ops || !port->ops->dr_set) {
-		dev_dbg(dev, "data role swapping not supported\n");
-		return -EOPNOTSUPP;
-	}
-
 	ret = sysfs_match_string(typec_data_roles, buf);
 	if (ret < 0)
 		return ret;
 
 	mutex_lock(&port->port_type_lock);
-	if (port->cap->data != TYPEC_PORT_DRD) {
-		ret = -EOPNOTSUPP;
-		goto unlock_and_ret;
-	}
 
 	ret = port->ops->dr_set(port, ret);
 	if (ret)
@@ -1091,16 +1087,6 @@ static ssize_t power_role_store(struct device *dev,
 	struct typec_port *port = to_typec_port(dev);
 	int ret;
 
-	if (!port->cap->pd_revision) {
-		dev_dbg(dev, "USB Power Delivery not supported\n");
-		return -EOPNOTSUPP;
-	}
-
-	if (!port->ops || !port->ops->pr_set) {
-		dev_dbg(dev, "power role swapping not supported\n");
-		return -EOPNOTSUPP;
-	}
-
 	if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
 		dev_dbg(dev, "partner unable to swap power role\n");
 		return -EIO;
@@ -1149,12 +1135,6 @@ port_type_store(struct device *dev, struct device_attribute *attr,
 	int ret;
 	enum typec_port_type type;
 
-	if (port->cap->type != TYPEC_PORT_DRP ||
-	    !port->ops || !port->ops->port_type_set) {
-		dev_dbg(dev, "changing port type not supported\n");
-		return -EOPNOTSUPP;
-	}
-
 	ret = sysfs_match_string(typec_port_power_roles, buf);
 	if (ret < 0)
 		return ret;
@@ -1218,16 +1198,6 @@ static ssize_t vconn_source_store(struct device *dev,
 	bool source;
 	int ret;
 
-	if (!port->cap->pd_revision) {
-		dev_dbg(dev, "VCONN swap depends on USB Power Delivery\n");
-		return -EOPNOTSUPP;
-	}
-
-	if (!port->ops || !port->ops->vconn_set) {
-		dev_dbg(dev, "VCONN swapping not supported\n");
-		return -EOPNOTSUPP;
-	}
-
 	ret = kstrtobool(buf, &source);
 	if (ret)
 		return ret;
@@ -1305,7 +1275,47 @@ static struct attribute *typec_attrs[] = {
 	&dev_attr_port_type.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(typec);
+
+static umode_t typec_attr_is_visible(struct kobject *kobj,
+				     struct attribute *attr, int n)
+{
+	struct typec_port *port = to_typec_port(kobj_to_dev(kobj));
+
+	if (attr == &dev_attr_data_role.attr) {
+		if (port->cap->data != TYPEC_PORT_DRD ||
+		    !port->ops || !port->ops->dr_set)
+			return 0444;
+	} else if (attr == &dev_attr_power_role.attr) {
+		if (port->cap->type != TYPEC_PORT_DRP ||
+		    !port->cap->pd_revision ||
+		    !port->ops || !port->ops->pr_set)
+			return 0444;
+	} else if (attr == &dev_attr_vconn_source.attr) {
+		if (!port->cap->pd_revision ||
+		    !port->ops || !port->ops->vconn_set)
+			return 0444;
+	} else if (attr == &dev_attr_preferred_role.attr) {
+		if (port->cap->type != TYPEC_PORT_DRP ||
+		    !port->ops || !port->ops->try_role)
+			return 0444;
+	} else if (attr == &dev_attr_port_type.attr) {
+		if (port->cap->type != TYPEC_PORT_DRP ||
+		    !port->ops || !port->ops->port_type_set)
+			return 0444;
+	}
+
+	return attr->mode;
+}
+
+static struct attribute_group typec_group = {
+	.is_visible = typec_attr_is_visible,
+	.attrs = typec_attrs
+};
+
+static const struct attribute_group *typec_groups[] = {
+	&typec_group,
+	NULL
+};
 
 static int typec_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
-- 
2.24.1

[PATCH 05/15] usb: typec: Hide the port_type attribute when it's not supported

[Heikki Krogerus]

The port_type attribute is special. It is meant to allow
changing the capability of the port in runtime. It is purely
Linux kernel specific feature, i.e. the feature is not
described in any of the USB specifications.

Because of the special nature of this attribute, handling it
differently compared to the other writable attributes, and
hiding it when the underlying port interface (or just the
driver) does not support the feature.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/class.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 3abcfa09ecdf..5d2d97e9fa2e 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1299,8 +1299,9 @@ static umode_t typec_attr_is_visible(struct kobject *kobj,
 		    !port->ops || !port->ops->try_role)
 			return 0444;
 	} else if (attr == &dev_attr_port_type.attr) {
-		if (port->cap->type != TYPEC_PORT_DRP ||
-		    !port->ops || !port->ops->port_type_set)
+		if (!port->ops || !port->ops->port_type_set)
+			return 0;
+		if (port->cap->type != TYPEC_PORT_DRP)
 			return 0444;
 	}
 
-- 
2.24.1

[PATCH 06/15] usb: typec: Allow power role swapping even without USB PD

[Heikki Krogerus]

Even though originally the USB Type-C Specification did not
describe the steps for power role swapping without USB PD
contract in place, it did not actually mean power role swap
without USB PD was not allowed. The USB Type-C Specification
did not clearly separate the data and power roles until in
the release 1.2 which is why there also were no clear steps
for the scenario where only the power role was swapped
without USB PD contract before that.

Since in the latest version of the specification the power
role swap without USB PD is now clearly mentioned as allowed
operation, removing the check that prevented power role swap
without USB PD support.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 Documentation/ABI/testing/sysfs-class-typec | 14 +++++++-------
 drivers/usb/typec/class.c                   |  1 -
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index d7647b258c3c..0c2eb26fdc06 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -20,13 +20,13 @@ Date:		April 2017
 Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
 Description:
 		The supported power roles. This attribute can be used to request
-		power role swap on the port when the port supports 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. Change on the role will
-		also generate uevent KOBJ_CHANGE. The current role is show in
-		brackets, for example "[source] sink" when in source mode.
+		power role swap on the 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. Change on the role will also generate uevent
+		KOBJ_CHANGE. The current role is show in brackets, for example
+		"[source] sink" when in source mode.
 
 		Valid values: source, sink
 
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 5d2d97e9fa2e..c744928d6525 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1287,7 +1287,6 @@ static umode_t typec_attr_is_visible(struct kobject *kobj,
 			return 0444;
 	} else if (attr == &dev_attr_power_role.attr) {
 		if (port->cap->type != TYPEC_PORT_DRP ||
-		    !port->cap->pd_revision ||
 		    !port->ops || !port->ops->pr_set)
 			return 0444;
 	} else if (attr == &dev_attr_vconn_source.attr) {
-- 
2.24.1

[PATCH 07/15] usb: typec: Fix the description of struct typec_capability

[Heikki Krogerus]

Removing descriptions for mux and sw members. They are no
longer part of the structure.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/usb/typec.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index d95ea0d398b8..a6615d488c95 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -193,8 +193,6 @@ struct typec_operations {
  * @pd_revision: USB Power Delivery Specification revision if supported
  * @prefer_role: Initial role preference (DRP ports).
  * @accessory: Supported Accessory Modes
- * @sw: Cable plug orientation switch
- * @mux: Multiplexer switch for Alternate/Accessory Modes
  * @fwnode: Optional fwnode of the port
  * @driver_data: Private pointer for driver specific info
  * @ops: Port operations vector
-- 
2.24.1

[PATCH 08/15] usb: pd: Add definitions for the Enter_USB message

[Heikki Krogerus]

Version 2.0 of the USB Power Delivery Specification R3.0
defines a new message called Enter_USB, which is made with
USB4 in mind. Enter_USB message is in practice the same as
the Enter Mode command (used when entering alternate modes)
that just needs to be used when entering USB4 mode.

The message does also support entering USB 2.0 or USB 3.2
mode instead of USB4 mode, but it is only required with
USB4. I.e. with USB2 and USB3 Enter_USB message is optional.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/usb/pd.h | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 145c38e351c2..a665d7f21142 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -45,7 +45,8 @@ enum pd_data_msg_type {
 	PD_DATA_BATT_STATUS = 5,
 	PD_DATA_ALERT = 6,
 	PD_DATA_GET_COUNTRY_INFO = 7,
-	/* 8-14 Reserved */
+	PD_DATA_ENTER_USB = 8,
+	/* 9-14 Reserved */
 	PD_DATA_VENDOR_DEF = 15,
 	/* 16-31 Reserved */
 };
@@ -418,6 +419,36 @@ static inline unsigned int rdo_max_power(u32 rdo)
 	return ((rdo >> RDO_BATT_MAX_PWR_SHIFT) & RDO_PWR_MASK) * 250;
 }
 
+/* Enter_USB Data Object */
+#define EUDO_USB_MODE_MASK		GENMASK(30, 28)
+#define EUDO_USB_MODE_SHIFT		28
+#define   EUDO_USB_MODE_USB2		0
+#define   EUDO_USB_MODE_USB3		1
+#define   EUDO_USB_MODE_USB4		2
+#define EUDO_USB4_DRD			BIT(26)
+#define EUDO_USB3_DRD			BIT(25)
+#define EUDO_CABLE_SPEED_MASK		GENMASK(23, 21)
+#define EUDO_CABLE_SPEED_SHIFT		21
+#define   EUDO_CABLE_SPEED_USB2		0
+#define   EUDO_CABLE_SPEED_USB3_GEN1	1
+#define   EUDO_CABLE_SPEED_USB4_GEN2	2
+#define   EUDO_CABLE_SPEED_USB4_GEN3	3
+#define EUDO_CABLE_TYPE_MASK		GENMASK(20, 19)
+#define EUDO_CABLE_TYPE_SHIFT		19
+#define   EUDO_CABLE_TYPE_PASSIVE	0
+#define   EUDO_CABLE_TYPE_RE_TIMER	1
+#define   EUDO_CABLE_TYPE_RE_DRIVER	2
+#define   EUDO_CABLE_TYPE_OPTICAL	3
+#define EUDO_CABLE_CURRENT_MASK		GENMASK(18, 17)
+#define EUDO_CABLE_CURRENT_SHIFT	17
+#define   EUDO_CABLE_CURRENT_NOTSUPP	0
+#define   EUDO_CABLE_CURRENT_3A		2
+#define   EUDO_CABLE_CURRENT_5A		3
+#define EUDO_PCIE_SUPPORT		BIT(16)
+#define EUDO_DP_SUPPORT			BIT(15)
+#define EUDO_TBT_SUPPORT		BIT(14)
+#define EUDO_HOST_PRESENT		BIT(13)
+
 /* USB PD timers and counters */
 #define PD_T_NO_RESPONSE	5000	/* 4.5 - 5.5 seconds */
 #define PD_T_DB_DETECT		10000	/* 10 - 15 seconds */
-- 
2.24.1

[PATCH 09/15] usb: pd: Add definition for DFP and UFP1 VDOs

[Heikki Krogerus]

The latest version of the USB Power Delivery Specification
R3.0 added UFP and DFP product types for the Discover
Identity message. Both types can be used for example for
checking the USB capability of the partner, which means the
USB modes (USB 2.0, USB 3.0 and USB4) that the partner
device supports.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/usb/pd_vdo.h | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/include/linux/usb/pd_vdo.h b/include/linux/usb/pd_vdo.h
index 781f4e98dd23..35b8e15efaa0 100644
--- a/include/linux/usb/pd_vdo.h
+++ b/include/linux/usb/pd_vdo.h
@@ -139,6 +139,38 @@
 #define VDO_PRODUCT(pid, bcd)	(((pid) & 0xffff) << 16 | ((bcd) & 0xffff))
 #define PD_PRODUCT_PID(vdo)	(((vdo) >> 16) & 0xffff)
 
+/*
+ * UFP VDO1
+ * --------
+ * <31:29> :: UFP VDO version
+ * <28>    :: Reserved
+ * <27:24> :: Device capability
+ * <23:6>  :: Reserved
+ * <5:3>   :: Alternate modes
+ * <2:0>   :: USB highest speed
+ */
+#define PD_VDO1_UFP_DEVCAP(vdo)	(((vdo) & GENMASK(27, 24)) >> 24)
+
+#define DEV_USB2_CAPABLE	BIT(0)
+#define DEV_USB2_BILLBOARD	BIT(1)
+#define DEV_USB3_CAPABLE	BIT(2)
+#define DEV_USB4_CAPABLE	BIT(3)
+
+/*
+ * DFP VDO
+ * --------
+ * <31:29> :: DFP VDO version
+ * <28:27> :: Reserved
+ * <26:24> :: Host capability
+ * <23:5>  :: Reserved
+ * <4:0>   :: Port number
+ */
+#define PD_VDO_DFP_HOSTCAP(vdo)	(((vdo) & GENMASK(26, 24)) >> 24)
+
+#define HOST_USB2_CAPABLE	BIT(0)
+#define HOST_USB3_CAPABLE	BIT(1)
+#define HOST_USB4_CAPABLE	BIT(2)
+
 /*
  * Cable VDO
  * ---------
-- 
2.24.1

[PATCH 10/15] usb: typec: Add the Product Type VDOs to struct usb_pd_identity

[Heikki Krogerus]

Discover Identity command response has also 3 product type
specific VDOs on top of ID Header VDO, Cert Stat VDO and
Product VDO.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/usb/typec.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index a6615d488c95..269a76a4f9b6 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -74,6 +74,7 @@ enum typec_orientation {
  * @id_header: ID Header VDO
  * @cert_stat: Cert Stat VDO
  * @product: Product VDO
+ * @vdo: Product Type Specific VDOs
  *
  * USB power delivery Discover Identity command response data.
  *
@@ -84,6 +85,7 @@ struct usb_pd_identity {
 	u32			id_header;
 	u32			cert_stat;
 	u32			product;
+	u32			vdo[3];
 };
 
 int typec_partner_set_identity(struct typec_partner *partner);
-- 
2.24.1

[PATCH 11/15] usb: typec: Add definitions for the latest specification releases

[Heikki Krogerus]

Adding definitions for USB Type-C Specification Release 1.3,
1.4 and 2.0.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/usb/typec.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 269a76a4f9b6..44d28387ced4 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -9,6 +9,9 @@
 #define USB_TYPEC_REV_1_0	0x100 /* 1.0 */
 #define USB_TYPEC_REV_1_1	0x110 /* 1.1 */
 #define USB_TYPEC_REV_1_2	0x120 /* 1.2 */
+#define USB_TYPEC_REV_1_3	0x130 /* 1.3 */
+#define USB_TYPEC_REV_1_4	0x140 /* 1.4 */
+#define USB_TYPEC_REV_2_0	0x200 /* 2.0 */
 
 struct typec_partner;
 struct typec_cable;
-- 
2.24.1

[PATCH 12/15] usb: typec: Give the mux drivers all the details regarding the port state

[Heikki Krogerus]

Passing all the details that the alternate mode drivers
provide to the mux drivers during mode changes.

The mux drivers will in practice need to be able to make
decisions on their own. It is not enough that they get only
the requested port state. With the Thunderbolt 3 alternate
mode for example the mux driver will need to consider also
the capabilities of the cable before configuring the mux.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/bus.c             | 29 ++++++++++++++++++++---------
 drivers/usb/typec/class.c           |  6 +++++-
 drivers/usb/typec/mux/pi3usb30532.c |  5 +++--
 include/linux/usb/typec_mux.h       | 10 +++++++++-
 4 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index 76e024be2502..fab0794d1302 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -10,12 +10,23 @@
 
 #include "bus.h"
 
-static inline int typec_altmode_set_mux(struct altmode *alt, u8 state)
+static inline int
+typec_altmode_set_mux(struct altmode *alt, unsigned long conf, void *data)
 {
-	return alt->mux ? alt->mux->set(alt->mux, state) : 0;
+	struct typec_mux_state state;
+
+	if (!alt->mux)
+		return 0;
+
+	state.alt = &alt->adev;
+	state.mode = conf;
+	state.data = data;
+
+	return alt->mux->set(alt->mux, &state);
 }
 
-static int typec_altmode_set_state(struct typec_altmode *adev, int state)
+static int typec_altmode_set_state(struct typec_altmode *adev,
+				   unsigned long conf, void *data)
 {
 	bool is_port = is_typec_port(adev->dev.parent);
 	struct altmode *port_altmode;
@@ -23,11 +34,11 @@ static int typec_altmode_set_state(struct typec_altmode *adev, int state)
 
 	port_altmode = is_port ? to_altmode(adev) : to_altmode(adev)->partner;
 
-	ret = typec_altmode_set_mux(port_altmode, state);
+	ret = typec_altmode_set_mux(port_altmode, conf, data);
 	if (ret)
 		return ret;
 
-	blocking_notifier_call_chain(&port_altmode->nh, state, NULL);
+	blocking_notifier_call_chain(&port_altmode->nh, conf, NULL);
 
 	return 0;
 }
@@ -67,7 +78,7 @@ int typec_altmode_notify(struct typec_altmode *adev,
 	is_port = is_typec_port(adev->dev.parent);
 	partner = altmode->partner;
 
-	ret = typec_altmode_set_mux(is_port ? altmode : partner, (u8)conf);
+	ret = typec_altmode_set_mux(is_port ? altmode : partner, conf, data);
 	if (ret)
 		return ret;
 
@@ -107,7 +118,7 @@ int typec_altmode_enter(struct typec_altmode *adev, u32 *vdo)
 		return -EPERM;
 
 	/* Moving to USB Safe State */
-	ret = typec_altmode_set_state(adev, TYPEC_STATE_SAFE);
+	ret = typec_altmode_set_state(adev, TYPEC_STATE_SAFE, NULL);
 	if (ret)
 		return ret;
 
@@ -135,7 +146,7 @@ int typec_altmode_exit(struct typec_altmode *adev)
 		return -EOPNOTSUPP;
 
 	/* Moving to USB Safe State */
-	ret = typec_altmode_set_state(adev, TYPEC_STATE_SAFE);
+	ret = typec_altmode_set_state(adev, TYPEC_STATE_SAFE, NULL);
 	if (ret)
 		return ret;
 
@@ -388,7 +399,7 @@ static int typec_remove(struct device *dev)
 		drv->remove(to_typec_altmode(dev));
 
 	if (adev->active) {
-		WARN_ON(typec_altmode_set_state(adev, TYPEC_STATE_SAFE));
+		WARN_ON(typec_altmode_set_state(adev, TYPEC_STATE_SAFE, NULL));
 		typec_altmode_update_active(adev, false);
 	}
 
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index c744928d6525..70f3c5e9eb0c 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1539,7 +1539,11 @@ EXPORT_SYMBOL_GPL(typec_get_orientation);
  */
 int typec_set_mode(struct typec_port *port, int mode)
 {
-	return port->mux ? port->mux->set(port->mux, mode) : 0;
+	struct typec_mux_state state = { };
+
+	state.mode = mode;
+
+	return port->mux ? port->mux->set(port->mux, &state) : 0;
 }
 EXPORT_SYMBOL_GPL(typec_set_mode);
 
diff --git a/drivers/usb/typec/mux/pi3usb30532.c b/drivers/usb/typec/mux/pi3usb30532.c
index 5585b109095b..46457c133d2b 100644
--- a/drivers/usb/typec/mux/pi3usb30532.c
+++ b/drivers/usb/typec/mux/pi3usb30532.c
@@ -73,7 +73,8 @@ static int pi3usb30532_sw_set(struct typec_switch *sw,
 	return ret;
 }
 
-static int pi3usb30532_mux_set(struct typec_mux *mux, int state)
+static int
+pi3usb30532_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
 {
 	struct pi3usb30532 *pi = typec_mux_get_drvdata(mux);
 	u8 new_conf;
@@ -82,7 +83,7 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, int state)
 	mutex_lock(&pi->lock);
 	new_conf = pi->conf;
 
-	switch (state) {
+	switch (state->mode) {
 	case TYPEC_STATE_SAFE:
 		new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
 			   PI3USB30532_CONF_OPEN;
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
index 873ace5b0cf8..be7292c0be5e 100644
--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h
@@ -8,6 +8,7 @@
 struct device;
 struct typec_mux;
 struct typec_switch;
+struct typec_altmode;
 struct fwnode_handle;
 
 typedef int (*typec_switch_set_fn_t)(struct typec_switch *sw,
@@ -29,7 +30,14 @@ void typec_switch_unregister(struct typec_switch *sw);
 void typec_switch_set_drvdata(struct typec_switch *sw, void *data);
 void *typec_switch_get_drvdata(struct typec_switch *sw);
 
-typedef int (*typec_mux_set_fn_t)(struct typec_mux *mux, int state);
+struct typec_mux_state {
+	struct typec_altmode *alt;
+	unsigned long mode;
+	void *data;
+};
+
+typedef int (*typec_mux_set_fn_t)(struct typec_mux *mux,
+				  struct typec_mux_state *state);
 
 struct typec_mux_desc {
 	struct fwnode_handle *fwnode;
-- 
2.24.1

[PATCH 13/15] usb: typec: Provide definitions for the USB modes

[Heikki Krogerus]

Defining the USB modes from the latest USB Power Delivery
Specification - USB 2.0, USB 3.2 and USB4 - as special modal
states just like the Accessory Modes.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/usb/typec_altmode.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index fc57fd88004f..923ff3af0628 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -101,6 +101,22 @@ enum {
 	TYPEC_MODE_DEBUG,			/* Debug Accessory */
 };
 
+/*
+ * USB4 also requires that the pins on the connector are repurposed, just like
+ * Alternate Modes. USB4 mode is however not entered with the Enter Mode Command
+ * like the Alternate Modes are, but instead with a special Enter_USB Message.
+ * The Enter_USB Message can also be used for setting to connector to operate in
+ * USB 3.2 or in USB 2.0 mode instead of USB4.
+ *
+ * The Enter_USB specific "USB Modes" are also supplied here as special modal
+ * state values, just like the Accessory Modes.
+ */
+enum {
+	TYPEC_MODE_USB2 = TYPEC_MODE_DEBUG,	/* USB 2.0 mode */
+	TYPEC_MODE_USB3,			/* USB 3.2 mode */
+	TYPEC_MODE_USB4				/* USB4 mode */
+};
+
 #define TYPEC_MODAL_STATE(_state_)	((_state_) + TYPEC_STATE_MODAL)
 
 struct typec_altmode *typec_altmode_get_plug(struct typec_altmode *altmode,
-- 
2.24.1

[PATCH 14/15] usb: typec: Add member for the supported USB modes to struct typec_capability

[Heikki Krogerus]

The ports can inform which USB modes (USB2, USB3 and USB4)
they support with this member.

This information will be needed once we start using the new
Enter_USB message.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/usb/typec.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 44d28387ced4..0acfbcd8bf04 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -72,6 +72,10 @@ enum typec_orientation {
 	TYPEC_ORIENTATION_REVERSE,
 };
 
+#define USB_CAPABILITY_USB2	BIT(0)
+#define USB_CAPABILITY_USB3	BIT(1)
+#define USB_CAPABILITY_USB4	BIT(2)
+
 /*
  * struct usb_pd_identity - USB Power Delivery identity data
  * @id_header: ID Header VDO
@@ -198,6 +202,7 @@ struct typec_operations {
  * @pd_revision: USB Power Delivery Specification revision if supported
  * @prefer_role: Initial role preference (DRP ports).
  * @accessory: Supported Accessory Modes
+ * @usb: Supported USB Modes
  * @fwnode: Optional fwnode of the port
  * @driver_data: Private pointer for driver specific info
  * @ops: Port operations vector
@@ -211,6 +216,7 @@ struct typec_capability {
 	u16			pd_revision; /* 0300H = "3.0" */
 	int			prefer_role;
 	enum typec_accessory	accessory[TYPEC_MAX_ACCESSORY];
+	u8			usb;
 
 	struct fwnode_handle	*fwnode;
 	void			*driver_data;
-- 
2.24.1

[PATCH 15/15] usb: typec: ucsi: Store the supported USB modes

[Heikki Krogerus]

UCSI can tell does the port support USB2 and USB3 modes, but
it can not tell is USB4 supported even when it is.
Nevertheless, registering the ports with the USB2 and USB3
capability set correctly for now.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/ucsi/ucsi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 4459bc68aa33..4b23d271fed7 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -809,6 +809,11 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 	if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DEBUG_ACCESSORY)
 		*accessory = TYPEC_ACCESSORY_DEBUG;
 
+	if (con->cap.op_mode & UCSI_CONCAP_OPMODE_USB2)
+		cap->usb |= USB_CAPABILITY_USB2;
+	if (con->cap.op_mode & UCSI_CONCAP_OPMODE_USB3)
+		cap->usb |= USB_CAPABILITY_USB3;
+
 	cap->fwnode = ucsi_find_fwnode(con);
 	cap->driver_data = con;
 	cap->ops = &ucsi_ops;
-- 
2.24.1

Re: [PATCH 04/15] usb: typec: Make the attributes read-only when writing is not possible

[Greg Kroah-Hartman]

On Mon, Dec 30, 2019 at 05:26:00PM +0300, Heikki Krogerus wrote:
> For now the read-writable attribute files have made a check
> in their store callback function to see does the underlying
> port interface support changing the value or not, and when
> it didn't, the callbacks returned -EOPNOTSUPP. From user
> perspective that is not ideal, as there is no way to know is
> changing the value possible beforehand.
> 
> Instead of returning -EOPNOTSUPP, making the attribute file
> read-only when the operation is not supported.

As fun as this is, if someone then changes the permission on the sysfs
file, trying to write to them will now crash the kernel, right?

So I think you need to leave this check in:

> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 08923637cd88..3abcfa09ecdf 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -373,12 +373,9 @@ static ssize_t active_store(struct device *dev, struct device_attribute *attr,
>  		}
>  	}
>  
> -	/* Note: If there is no driver, the mode will not be entered */
> -	if (adev->ops && adev->ops->activate) {
> -		ret = adev->ops->activate(adev, enter);
> -		if (ret)
> -			return ret;
> -	}
> +	ret = adev->ops->activate(adev, enter);
> +	if (ret)
> +		return ret;
>  
>  	return size;
>  }

We had to go add this type of check to some subsystems recently that had
this same problem, I don't want to have to go and add it back for this
one as well :)

thanks,

greg k-h

Re: [PATCH 14/15] usb: typec: Add member for the supported USB modes to struct typec_capability

[Greg Kroah-Hartman]

On Mon, Dec 30, 2019 at 05:26:10PM +0300, Heikki Krogerus wrote:
> The ports can inform which USB modes (USB2, USB3 and USB4)
> they support with this member.
> 
> This information will be needed once we start using the new
> Enter_USB message.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  include/linux/usb/typec.h | 6 ++++++
>  1 file changed, 6 insertions(+)

I dropped a few other patches in this series, and stopped here.  If you
could fix up the attribute issue I pointed out and rebase and resend the
reset of these, I'll be glad to take them.

thanks,

greg k-h

Re: [PATCH 04/15] usb: typec: Make the attributes read-only when writing is not possible

[Heikki Krogerus]

On Thu, Jan 09, 2020 at 10:46:25AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Dec 30, 2019 at 05:26:00PM +0300, Heikki Krogerus wrote:
> > For now the read-writable attribute files have made a check
> > in their store callback function to see does the underlying
> > port interface support changing the value or not, and when
> > it didn't, the callbacks returned -EOPNOTSUPP. From user
> > perspective that is not ideal, as there is no way to know is
> > changing the value possible beforehand.
> > 
> > Instead of returning -EOPNOTSUPP, making the attribute file
> > read-only when the operation is not supported.
> 
> As fun as this is, if someone then changes the permission on the sysfs
> file, trying to write to them will now crash the kernel, right?
> 
> So I think you need to leave this check in:
> 
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 08923637cd88..3abcfa09ecdf 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -373,12 +373,9 @@ static ssize_t active_store(struct device *dev, struct device_attribute *attr,
> >  		}
> >  	}
> >  
> > -	/* Note: If there is no driver, the mode will not be entered */
> > -	if (adev->ops && adev->ops->activate) {
> > -		ret = adev->ops->activate(adev, enter);
> > -		if (ret)
> > -			return ret;
> > -	}
> > +	ret = adev->ops->activate(adev, enter);
> > +	if (ret)
> > +		return ret;
> >  
> >  	return size;
> >  }
> 
> We had to go add this type of check to some subsystems recently that had
> this same problem, I don't want to have to go and add it back for this
> one as well :)

You are correct. I'll leave the check in.

thanks,

-- 
heikki

Re: [PATCH 14/15] usb: typec: Add member for the supported USB modes to struct typec_capability

[Heikki Krogerus]

On Thu, Jan 09, 2020 at 10:52:05AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Dec 30, 2019 at 05:26:10PM +0300, Heikki Krogerus wrote:
> > The ports can inform which USB modes (USB2, USB3 and USB4)
> > they support with this member.
> > 
> > This information will be needed once we start using the new
> > Enter_USB message.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  include/linux/usb/typec.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> 
> I dropped a few other patches in this series, and stopped here.  If you
> could fix up the attribute issue I pointed out and rebase and resend the
> reset of these, I'll be glad to take them.

OK. Thanks for going over these.

Br,

-- 
heikki