[PATCH 0/3] platform/chrome: cros_ec_typec: Add plug and plug altmodes

[PATCH 0/3] platform/chrome: cros_ec_typec: Add plug and plug altmodes

[Prashant Malani]

This patch series add plug registration support to the cros-ec-typec
driver. It also adds support for registering alternate modes for the
registered plug. These features utilize the API provided by the Type C
connector class framework.

The first patch adds support to the connector class framework for the
number_of_alternate_modes attribute (along with the relevant ABI
documentation).

The next two patches add plug registration, and then altmode
registration for the plugs. The latter of these two patches utilizes the
new function for plug number_of_alternate_modes introduced in the first patch.

This series is based on top of the following branch and other patch
series (applied in the order specified):
- Branch: chrome-platform for-next [1], which is currently set to the
  "Linux 5.10-rc1" tag.
- cros-ec-typec: Patch series to register PD identity information + partner altmodes[2]
- cros-ec-typec: Patch series to register cable[3]
- cros-ec-typec: Patch series to add partner number_of_altmodes[4]

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=for-next
[2]: https://lore.kernel.org/lkml/20201029222738.482366-1-pmalani@chromium.org/
[3]: https://lore.kernel.org/lkml/20201106184104.939284-1-pmalani@chromium.org/
[4]: https://lore.kernel.org/lkml/20201110061535.2163599-1-pmalani@chromium.org/

Prashant Malani (3):
  usb: typec: Add plug num_altmodes sysfs attr
  platform/chrome: cros_ec_typec: Register SOP' cable plug
  platform/chrome: cros_ec_typec: Register plug altmodes

 Documentation/ABI/testing/sysfs-class-typec |  9 +++
 drivers/platform/chrome/cros_ec_typec.c     | 85 ++++++++++++++++-----
 drivers/usb/typec/class.c                   | 77 ++++++++++++++++++-
 include/linux/usb/typec.h                   |  1 +
 4 files changed, 151 insertions(+), 21 deletions(-)

-- 
2.29.2.222.g5d2a92d10f8-goog

[PATCH 1/3] usb: typec: Add plug num_altmodes sysfs attr

[Prashant Malani]

Add a field to the typec_plug struct to record the number of available
altmodes as well as the corresponding sysfs attribute to expose this to
userspace.

This allows userspace to determine whether there are any
remaining alternate modes left to be registered by the kernel driver. It
can begin executing any policy state machine after all available
alternate modes have been registered with the connector class framework.

This value is set to "-1" initially, signifying that a valid number of
alternate modes haven't been set for the plug. The sysfs file remains
hidden as long as the attribute value is -1.

We re-use the partner attribute for number_of_alternate_modes since the
usage and name is similar, and update the corresponding *_show() command
to support both partner and plugs.

Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
 Documentation/ABI/testing/sysfs-class-typec |  9 +++
 drivers/usb/typec/class.c                   | 77 ++++++++++++++++++++-
 include/linux/usb/typec.h                   |  1 +
 3 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index 73ac7b461ae5..29eccf5fb8ed 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -204,6 +204,15 @@ Description:
 		- type-c
 		- captive
 
+What:		/sys/class/typec/<port>-<plug>/number_of_alternate_modes
+Date:		November 2020
+Contact:	Prashant Malani <pmalani@chromium.org>
+Description:
+		Shows the number of alternate modes which are advertised by the plug
+		associated with a particular cable during Power Delivery discovery.
+		This file remains hidden until a value greater than or equal to 0
+		is set by Type C port driver.
+
 What:		/sys/class/typec/<port>-cable/identity/
 Date:		April 2017
 Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index c7412ddbd311..e68798599ca8 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -18,6 +18,7 @@ struct typec_plug {
 	struct device			dev;
 	enum typec_plug_index		index;
 	struct ida			mode_ids;
+	int				num_altmodes;
 };
 
 struct typec_cable {
@@ -536,9 +537,21 @@ static DEVICE_ATTR_RO(supports_usb_power_delivery);
 static ssize_t number_of_alternate_modes_show(struct device *dev, struct device_attribute *attr,
 					      char *buf)
 {
-	struct typec_partner *p = to_typec_partner(dev);
+	struct typec_partner *partner;
+	struct typec_plug *plug;
+	int num_altmodes;
+
+	if (is_typec_partner(dev)) {
+		partner = to_typec_partner(dev);
+		num_altmodes = partner->num_altmodes;
+	} else if (is_typec_plug(dev)) {
+		plug = to_typec_plug(dev);
+		num_altmodes = plug->num_altmodes;
+	} else {
+		return 0;
+	}
 
-	return sysfs_emit(buf, "%d\n", p->num_altmodes);
+	return sysfs_emit(buf, "%d\n", num_altmodes);
 }
 static DEVICE_ATTR_RO(number_of_alternate_modes);
 
@@ -726,11 +739,70 @@ static void typec_plug_release(struct device *dev)
 	kfree(plug);
 }
 
+static struct attribute *typec_plug_attrs[] = {
+	&dev_attr_number_of_alternate_modes.attr,
+	NULL
+};
+
+static umode_t typec_plug_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+	struct typec_plug *plug = to_typec_plug(kobj_to_dev(kobj));
+
+	if (attr == &dev_attr_number_of_alternate_modes.attr) {
+		if (plug->num_altmodes < 0)
+			return 0;
+	}
+
+	return attr->mode;
+}
+
+static struct attribute_group typec_plug_group = {
+	.is_visible = typec_plug_attr_is_visible,
+	.attrs = typec_plug_attrs
+};
+
+static const struct attribute_group *typec_plug_groups[] = {
+	&typec_plug_group,
+	NULL
+};
+
 static const struct device_type typec_plug_dev_type = {
 	.name = "typec_plug",
+	.groups = typec_plug_groups,
 	.release = typec_plug_release,
 };
 
+/**
+ * typec_plug_set_num_altmodes - Set the number of available plug altmodes
+ * @plug: The plug to be updated.
+ * @num_altmodes: The number of altmodes we want to specify as available.
+ *
+ * This routine is used to report the number of alternate modes supported by the
+ * plug. This value is *not* enforced in alternate mode registration routines.
+ *
+ * @plug.num_altmodes is set to -1 on plug registration, denoting that
+ * a valid value has not been set for it yet.
+ *
+ * Returns 0 on success or negative error number on failure.
+ */
+int typec_plug_set_num_altmodes(struct typec_plug *plug, int num_altmodes)
+{
+	int ret;
+
+	if (num_altmodes < 0)
+		return -EINVAL;
+
+	plug->num_altmodes = num_altmodes;
+	ret = sysfs_update_group(&plug->dev.kobj, &typec_plug_group);
+	if (ret < 0)
+		return ret;
+
+	sysfs_notify(&plug->dev.kobj, NULL, "number_of_alternate_modes");
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(typec_plug_set_num_altmodes);
+
 /**
  * typec_plug_register_altmode - Register USB Type-C Cable Plug Alternate Mode
  * @plug: USB Type-C Cable Plug that supports the alternate mode
@@ -776,6 +848,7 @@ struct typec_plug *typec_register_plug(struct typec_cable *cable,
 	sprintf(name, "plug%d", desc->index);
 
 	ida_init(&plug->mode_ids);
+	plug->num_altmodes = -1;
 	plug->index = desc->index;
 	plug->dev.class = typec_class;
 	plug->dev.parent = &cable->dev;
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index bc6b1a71cb8a..54475323f83b 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -130,6 +130,7 @@ int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmod
 struct typec_altmode
 *typec_partner_register_altmode(struct typec_partner *partner,
 				const struct typec_altmode_desc *desc);
+int typec_plug_set_num_altmodes(struct typec_plug *plug, int num_altmodes);
 struct typec_altmode
 *typec_plug_register_altmode(struct typec_plug *plug,
 			     const struct typec_altmode_desc *desc);
-- 
2.29.2.222.g5d2a92d10f8-goog

[PATCH 2/3] platform/chrome: cros_ec_typec: Register SOP' cable plug

[Prashant Malani]

In order to register cable alternate modes, we need to first register a
plug object. Use the Type C connector class framework to register a SOP'
plug for this purpose.

Since a cable and plug go hand in hand, we can handle the registration
and removal together.

Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
 drivers/platform/chrome/cros_ec_typec.c | 35 ++++++++++++++++++-------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index ad5e37bfd45d..d2e154ae2362 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -45,6 +45,8 @@ struct cros_typec_port {
 	struct typec_capability caps;
 	struct typec_partner *partner;
 	struct typec_cable *cable;
+	/* SOP' plug. */
+	struct typec_plug *plug;
 	/* Port partner PD identity info. */
 	struct usb_pd_identity p_identity;
 	/* Port cable PD identity info. */
@@ -222,6 +224,8 @@ static void cros_typec_remove_cable(struct cros_typec_data *typec,
 {
 	struct cros_typec_port *port = typec->ports[port_num];
 
+	typec_unregister_plug(port->plug);
+	port->plug = NULL;
 	typec_unregister_cable(port->cable);
 	port->cable = NULL;
 	memset(&port->c_identity, 0, sizeof(port->c_identity));
@@ -712,7 +716,8 @@ static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec, int p
 {
 	struct cros_typec_port *port = typec->ports[port_num];
 	struct ec_response_typec_discovery *disc = port->disc_data;
-	struct typec_cable_desc desc = {};
+	struct typec_cable_desc c_desc = {};
+	struct typec_plug_desc p_desc;
 	struct ec_params_typec_discovery req = {
 		.port = port_num,
 		.partner_type = TYPEC_PARTNER_SOP_PRIME,
@@ -735,32 +740,44 @@ static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec, int p
 		cable_plug_type = VDO_TYPEC_CABLE_TYPE(port->c_identity.vdo[0]);
 		switch (cable_plug_type) {
 		case CABLE_ATYPE:
-			desc.type = USB_PLUG_TYPE_A;
+			c_desc.type = USB_PLUG_TYPE_A;
 			break;
 		case CABLE_BTYPE:
-			desc.type = USB_PLUG_TYPE_B;
+			c_desc.type = USB_PLUG_TYPE_B;
 			break;
 		case CABLE_CTYPE:
-			desc.type = USB_PLUG_TYPE_C;
+			c_desc.type = USB_PLUG_TYPE_C;
 			break;
 		case CABLE_CAPTIVE:
-			desc.type = USB_PLUG_CAPTIVE;
+			c_desc.type = USB_PLUG_CAPTIVE;
 			break;
 		default:
-			desc.type = USB_PLUG_NONE;
+			c_desc.type = USB_PLUG_NONE;
 		}
-		desc.active = PD_IDH_PTYPE(port->c_identity.id_header) == IDH_PTYPE_ACABLE;
+		c_desc.active = PD_IDH_PTYPE(port->c_identity.id_header) == IDH_PTYPE_ACABLE;
 	}
 
-	desc.identity = &port->c_identity;
+	c_desc.identity = &port->c_identity;
 
-	port->cable = typec_register_cable(port->port, &desc);
+	port->cable = typec_register_cable(port->port, &c_desc);
 	if (IS_ERR(port->cable)) {
 		ret = PTR_ERR(port->cable);
 		port->cable = NULL;
+		goto sop_prime_disc_exit;
+	}
+
+	p_desc.index = TYPEC_PLUG_SOP_P;
+	port->plug = typec_register_plug(port->cable, &p_desc);
+	if (IS_ERR(port->plug)) {
+		ret = PTR_ERR(port->plug);
+		port->plug = NULL;
+		goto sop_prime_disc_exit;
 	}
 
+	return 0;
+
 sop_prime_disc_exit:
+	cros_typec_remove_cable(typec, port_num);
 	return ret;
 }
 
-- 
2.29.2.222.g5d2a92d10f8-goog

[PATCH 3/3] platform/chrome: cros_ec_typec: Register plug altmodes

[Prashant Malani]

Modify the altmode registration (and unregistration) code so that it
can be used by both partners and plugs.

Then, add code to register plug altmodes using the newly parameterized
function. Also set the number of alternate modes for the plug using the
associated Type C connector class function
typec_plug_set_num_altmodes().

Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
 drivers/platform/chrome/cros_ec_typec.c | 50 ++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index d2e154ae2362..65c5d0090ccd 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -67,6 +67,7 @@ struct cros_typec_port {
 	bool sop_prime_disc_done;
 	struct ec_response_typec_discovery *disc_data;
 	struct list_head partner_mode_list;
+	struct list_head plug_mode_list;
 };
 
 /* Platform-specific data for the Chrome OS EC Type C controller. */
@@ -186,12 +187,15 @@ static int cros_typec_add_partner(struct cros_typec_data *typec, int port_num,
 	return ret;
 }
 
-static void cros_typec_unregister_altmodes(struct cros_typec_data *typec, int port_num)
+static void cros_typec_unregister_altmodes(struct cros_typec_data *typec, int port_num,
+					   bool is_partner)
 {
 	struct cros_typec_port *port = typec->ports[port_num];
 	struct cros_typec_altmode_node *node, *tmp;
+	struct list_head *head;
 
-	list_for_each_entry_safe(node, tmp, &port->partner_mode_list, list) {
+	head = is_partner ? &port->partner_mode_list : &port->plug_mode_list;
+	list_for_each_entry_safe(node, tmp, head, list) {
 		list_del(&node->list);
 		typec_unregister_altmode(node->amode);
 		devm_kfree(typec->dev, node);
@@ -203,7 +207,7 @@ static void cros_typec_remove_partner(struct cros_typec_data *typec,
 {
 	struct cros_typec_port *port = typec->ports[port_num];
 
-	cros_typec_unregister_altmodes(typec, port_num);
+	cros_typec_unregister_altmodes(typec, port_num, true);
 
 	port->state.alt = NULL;
 	port->state.mode = TYPEC_STATE_USB;
@@ -224,6 +228,8 @@ static void cros_typec_remove_cable(struct cros_typec_data *typec,
 {
 	struct cros_typec_port *port = typec->ports[port_num];
 
+	cros_typec_unregister_altmodes(typec, port_num, false);
+
 	typec_unregister_plug(port->plug);
 	port->plug = NULL;
 	typec_unregister_cable(port->cable);
@@ -352,6 +358,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
 		}
 
 		INIT_LIST_HEAD(&cros_port->partner_mode_list);
+		INIT_LIST_HEAD(&cros_port->plug_mode_list);
 	}
 
 	return 0;
@@ -639,7 +646,11 @@ static int cros_typec_get_mux_info(struct cros_typec_data *typec, int port_num,
 				     sizeof(req), resp, sizeof(*resp));
 }
 
-static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_num)
+/*
+ * Helper function to register partner/plug altmodes.
+ */
+static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_num,
+					bool is_partner)
 {
 	struct cros_typec_port *port = typec->ports[port_num];
 	struct ec_response_typec_discovery *sop_disc = port->disc_data;
@@ -657,7 +668,11 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
 			desc.mode = j;
 			desc.vdo = sop_disc->svids[i].mode_vdo[j];
 
-			amode = typec_partner_register_altmode(port->partner, &desc);
+			if (is_partner)
+				amode = typec_partner_register_altmode(port->partner, &desc);
+			else
+				amode = typec_plug_register_altmode(port->plug, &desc);
+
 			if (IS_ERR(amode)) {
 				ret = PTR_ERR(amode);
 				goto err_cleanup;
@@ -672,21 +687,30 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
 			}
 
 			node->amode = amode;
-			list_add_tail(&node->list, &port->partner_mode_list);
+
+			if (is_partner)
+				list_add_tail(&node->list, &port->partner_mode_list);
+			else
+				list_add_tail(&node->list, &port->plug_mode_list);
 			num_altmodes++;
 		}
 	}
 
-	ret = typec_partner_set_num_altmodes(port->partner, num_altmodes);
+	if (is_partner)
+		ret = typec_partner_set_num_altmodes(port->partner, num_altmodes);
+	else
+		ret = typec_plug_set_num_altmodes(port->plug, num_altmodes);
+
 	if (ret < 0) {
-		dev_err(typec->dev, "Unable to set partner num_altmodes for port: %d\n", port_num);
+		dev_err(typec->dev, "Unable to set %s num_altmodes for port: %d\n",
+			is_partner ? "partner" : "plug", port_num);
 		goto err_cleanup;
 	}
 
 	return 0;
 
 err_cleanup:
-	cros_typec_unregister_altmodes(typec, port_num);
+	cros_typec_unregister_altmodes(typec, port_num, is_partner);
 	return ret;
 }
 
@@ -774,6 +798,12 @@ static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec, int p
 		goto sop_prime_disc_exit;
 	}
 
+	ret = cros_typec_register_altmodes(typec, port_num, false);
+	if (ret < 0) {
+		dev_err(typec->dev, "Failed to register plug altmodes, port: %d\n", port_num);
+		goto sop_prime_disc_exit;
+	}
+
 	return 0;
 
 sop_prime_disc_exit:
@@ -815,7 +845,7 @@ static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_nu
 		goto disc_exit;
 	}
 
-	ret = cros_typec_register_altmodes(typec, port_num);
+	ret = cros_typec_register_altmodes(typec, port_num, true);
 	if (ret < 0) {
 		dev_err(typec->dev, "Failed to register partner altmodes, port: %d\n", port_num);
 		goto disc_exit;
-- 
2.29.2.222.g5d2a92d10f8-goog

Re: [PATCH 0/3] platform/chrome: cros_ec_typec: Add plug and plug altmodes

[Greg KH]

On Wed, Nov 11, 2020 at 05:23:25PM -0800, Prashant Malani wrote:
> This patch series add plug registration support to the cros-ec-typec
> driver. It also adds support for registering alternate modes for the
> registered plug. These features utilize the API provided by the Type C
> connector class framework.
> 
> The first patch adds support to the connector class framework for the
> number_of_alternate_modes attribute (along with the relevant ABI
> documentation).
> 
> The next two patches add plug registration, and then altmode
> registration for the plugs. The latter of these two patches utilizes the
> new function for plug number_of_alternate_modes introduced in the first patch.
> 
> This series is based on top of the following branch and other patch
> series (applied in the order specified):
> - Branch: chrome-platform for-next [1], which is currently set to the
>   "Linux 5.10-rc1" tag.
> - cros-ec-typec: Patch series to register PD identity information + partner altmodes[2]
> - cros-ec-typec: Patch series to register cable[3]
> - cros-ec-typec: Patch series to add partner number_of_altmodes[4]
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=for-next
> [2]: https://lore.kernel.org/lkml/20201029222738.482366-1-pmalani@chromium.org/
> [3]: https://lore.kernel.org/lkml/20201106184104.939284-1-pmalani@chromium.org/
> [4]: https://lore.kernel.org/lkml/20201110061535.2163599-1-pmalani@chromium.org/

Ok, I'm confused.  This is not the first submission of this series, as
you sent out a v2 a few days before this one.

And am I supposed to suck in the chrome-platform branch into the
usb-next tree?

What should I do here, ignore these?  Merge them?

I see the USB change lost the reviewer's ack as well, why?

I'm going to delete all of these patches from my review queue now and
wait for a resend with some clarity as to what I should do with it :)

thanks,

greg k-h

Re: [PATCH 0/3] platform/chrome: cros_ec_typec: Add plug and plug altmodes

[Prashant Malani]

Hi Greg,

On Fri, Nov 13, 2020 at 6:13 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Nov 11, 2020 at 05:23:25PM -0800, Prashant Malani wrote:
> > This patch series add plug registration support to the cros-ec-typec
> > driver. It also adds support for registering alternate modes for the
> > registered plug. These features utilize the API provided by the Type C
> > connector class framework.
> >
> > The first patch adds support to the connector class framework for the
> > number_of_alternate_modes attribute (along with the relevant ABI
> > documentation).
> >
> > The next two patches add plug registration, and then altmode
> > registration for the plugs. The latter of these two patches utilizes the
> > new function for plug number_of_alternate_modes introduced in the first patch.
> >
> > This series is based on top of the following branch and other patch
> > series (applied in the order specified):
> > - Branch: chrome-platform for-next [1], which is currently set to the
> >   "Linux 5.10-rc1" tag.
> > - cros-ec-typec: Patch series to register PD identity information + partner altmodes[2]
> > - cros-ec-typec: Patch series to register cable[3]
> > - cros-ec-typec: Patch series to add partner number_of_altmodes[4]
> >
> > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=for-next
> > [2]: https://lore.kernel.org/lkml/20201029222738.482366-1-pmalani@chromium.org/
> > [3]: https://lore.kernel.org/lkml/20201106184104.939284-1-pmalani@chromium.org/
> > [4]: https://lore.kernel.org/lkml/20201110061535.2163599-1-pmalani@chromium.org/
>
> Ok, I'm confused.  This is not the first submission of this series, as
> you sent out a v2 a few days before this one.

Sorry for confusing you. To clarify, this is the first version of this
series particular set
of patches. A few more related series of patches have been sent
regarding this earlier
(see [2], [3] & [4]) and I've not done a resend of those.

This series depends on those earlier series.

>
> And am I supposed to suck in the chrome-platform branch into the
> usb-next tree?
>
> What should I do here, ignore these?  Merge them?

TBH I'm a little confused about how these get in myself. Across all
these patches, there are probably
3 patches (1 USB PD VDO header, two drivers/usb/typec/class.c patches)
which belong to usb-next
whereas everything else is chrome-platform.

>
> I see the USB change lost the reviewer's ack as well, why?

I'm sorry but I didn't follow. As I mentioned above this is the first
version of this
series (which deals with cable plug and it's altmodes) and AFAIK it
hasn't received any acks till now.

All the previous series have not had any re-uploads, so they should
still have all their review tags.

>
> I'm going to delete all of these patches from my review queue now and
> wait for a resend with some clarity as to what I should do with it :)

Enric, could you kindly help suggest a way I can upload things?
Should I just merge everything into one big set of patches (RIght from
[2] to this series) ?
Or just let all the series go through chrome-platform?

BR,

-Prashant