[PATCH v2 0/6] platform/chrome: cros_ec_typec: Add cable

[PATCH v2 0/6] platform/chrome: cros_ec_typec: Add cable

[Prashant Malani]

The following series adds Type C cable registration to the cros-ec-typec
port driver using the Type C connector class framework. The first few
patches perform a few minor re-organizations to prepare for the cable
registration patch.

The last couple of CLs update the USB PD VDO header file to add a
captive cable connector for the Type C cable plug field, and then use
the added macro to add the corresponding field of the Type C cable
descriptor in the cros-ec-typec driver.

v1:
https://lore.kernel.org/lkml/20201106012758.525472-1-pmalani@chromium.org/

Changes since v2:
- Changed local variable uint32_t to u32 in patch 6/6.

Prashant Malani (6):
  platform/chrome: cros_ec_typec: Make disc_done flag partner-only
  platform/chrome: cros_ec_typec: Factor out PD identity parsing
  platform/chrome: cros_ec_typec: Rename discovery struct
  platform/chrome: cros_ec_typec: Register cable
  usb: pd: Add captive Type C cable type
  platform/chrome: cros_ec_typec: Store cable plug type

 drivers/platform/chrome/cros_ec_typec.c | 148 +++++++++++++++++++-----
 include/linux/usb/pd_vdo.h              |   4 +-
 2 files changed, 123 insertions(+), 29 deletions(-)

-- 
2.29.1.341.ge80a0c044ae-goog

[PATCH v2 1/6] platform/chrome: cros_ec_typec: Make disc_done flag partner-only

[Prashant Malani]

Change the disc_done flag, which indicates whether PD discovery is
complete, to sop_disc_done instead, since we will process SOP and SOP'
discovery data separately.

Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v2:
- No changes.

 drivers/platform/chrome/cros_ec_typec.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index ce031a10eb1b..801c3d2c1fbd 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -57,8 +57,8 @@ struct cros_typec_port {
 	/* Port alt modes. */
 	struct typec_altmode p_altmode[CROS_EC_ALTMODE_MAX];
 
-	/* Flag indicating that PD discovery data parsing is completed. */
-	bool disc_done;
+	/* Flag indicating that PD partner discovery data parsing is completed. */
+	bool sop_disc_done;
 	struct ec_response_typec_discovery *sop_disc;
 	struct list_head partner_mode_list;
 };
@@ -210,7 +210,7 @@ static void cros_typec_remove_partner(struct cros_typec_data *typec,
 	typec_unregister_partner(port->partner);
 	port->partner = NULL;
 	memset(&port->p_identity, 0, sizeof(port->p_identity));
-	port->disc_done = false;
+	port->sop_disc_done = false;
 }
 
 static void cros_unregister_ports(struct cros_typec_data *typec)
@@ -727,18 +727,13 @@ static void cros_typec_handle_status(struct cros_typec_data *typec, int port_num
 		return;
 	}
 
-	if (typec->ports[port_num]->disc_done)
-		return;
-
 	/* Handle any events appropriately. */
-	if (resp.events & PD_STATUS_EVENT_SOP_DISC_DONE) {
+	if (resp.events & PD_STATUS_EVENT_SOP_DISC_DONE && !typec->ports[port_num]->sop_disc_done) {
 		ret = cros_typec_handle_sop_disc(typec, port_num);
-		if (ret < 0) {
+		if (ret < 0)
 			dev_err(typec->dev, "Couldn't parse SOP Disc data, port: %d\n", port_num);
-			return;
-		}
-
-		typec->ports[port_num]->disc_done = true;
+		else
+			typec->ports[port_num]->sop_disc_done = true;
 	}
 }
 
-- 
2.29.1.341.ge80a0c044ae-goog

[PATCH v2 2/6] platform/chrome: cros_ec_typec: Factor out PD identity parsing

[Prashant Malani]

Factor out the PD identity parsing code into a separate function. This
way it can be re-used for Cable PD identity parsing in future patches.

No functional changes are introduced by this patch.

Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v2:
- No changes.

 drivers/platform/chrome/cros_ec_typec.c | 35 ++++++++++++++++---------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 801c3d2c1fbd..f6d3c37c2c27 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -657,6 +657,28 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
 	return ret;
 }
 
+/*
+ * Parse the PD identity data from the EC PD discovery responses and copy that to the supplied
+ * PD identity struct.
+ */
+static void cros_typec_parse_pd_identity(struct usb_pd_identity *id,
+					 struct ec_response_typec_discovery *disc)
+{
+	int i;
+
+	/* First, update the PD identity VDOs for the partner. */
+	if (disc->identity_count > 0)
+		id->id_header = disc->discovery_vdo[0];
+	if (disc->identity_count > 1)
+		id->cert_stat = disc->discovery_vdo[1];
+	if (disc->identity_count > 2)
+		id->product = disc->discovery_vdo[2];
+
+	/* Copy the remaining identity VDOs till a maximum of 6. */
+	for (i = 3; i < disc->identity_count && i < VDO_MAX_OBJECTS; i++)
+		id->vdo[i - 3] = disc->discovery_vdo[i];
+}
+
 static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_num)
 {
 	struct cros_typec_port *port = typec->ports[port_num];
@@ -666,7 +688,6 @@ static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_nu
 		.partner_type = TYPEC_PARTNER_SOP,
 	};
 	int ret = 0;
-	int i;
 
 	if (!port->partner) {
 		dev_err(typec->dev,
@@ -684,17 +705,7 @@ static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_nu
 		goto disc_exit;
 	}
 
-	/* First, update the PD identity VDOs for the partner. */
-	if (sop_disc->identity_count > 0)
-		port->p_identity.id_header = sop_disc->discovery_vdo[0];
-	if (sop_disc->identity_count > 1)
-		port->p_identity.cert_stat = sop_disc->discovery_vdo[1];
-	if (sop_disc->identity_count > 2)
-		port->p_identity.product = sop_disc->discovery_vdo[2];
-
-	/* Copy the remaining identity VDOs till a maximum of 6. */
-	for (i = 3; i < sop_disc->identity_count && i < VDO_MAX_OBJECTS; i++)
-		port->p_identity.vdo[i - 3] = sop_disc->discovery_vdo[i];
+	cros_typec_parse_pd_identity(&port->p_identity, sop_disc);
 
 	ret = typec_partner_set_identity(port->partner);
 	if (ret < 0) {
-- 
2.29.1.341.ge80a0c044ae-goog

[PATCH v2 3/6] platform/chrome: cros_ec_typec: Rename discovery struct

[Prashant Malani]

Rename the sop_disc data struct which is used to store PD discovery data
to the more generic name of disc_data. It can then be re-used to store
and process cable discovery data.

Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v2:
- No changes.

 drivers/platform/chrome/cros_ec_typec.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index f6d3c37c2c27..3c8ff07c8803 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -59,7 +59,7 @@ struct cros_typec_port {
 
 	/* Flag indicating that PD partner discovery data parsing is completed. */
 	bool sop_disc_done;
-	struct ec_response_typec_discovery *sop_disc;
+	struct ec_response_typec_discovery *disc_data;
 	struct list_head partner_mode_list;
 };
 
@@ -323,8 +323,8 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
 
 		cros_typec_register_port_altmodes(typec, port_num);
 
-		cros_port->sop_disc = devm_kzalloc(dev, EC_PROTO2_MAX_RESPONSE_SIZE, GFP_KERNEL);
-		if (!cros_port->sop_disc) {
+		cros_port->disc_data = devm_kzalloc(dev, EC_PROTO2_MAX_RESPONSE_SIZE, GFP_KERNEL);
+		if (!cros_port->disc_data) {
 			ret = -ENOMEM;
 			goto unregister_ports;
 		}
@@ -617,7 +617,7 @@ static int cros_typec_get_mux_info(struct cros_typec_data *typec, int port_num,
 static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_num)
 {
 	struct cros_typec_port *port = typec->ports[port_num];
-	struct ec_response_typec_discovery *sop_disc = port->sop_disc;
+	struct ec_response_typec_discovery *sop_disc = port->disc_data;
 	struct cros_typec_altmode_node *node;
 	struct typec_altmode_desc desc;
 	struct typec_altmode *amode;
@@ -682,7 +682,7 @@ static void cros_typec_parse_pd_identity(struct usb_pd_identity *id,
 static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_num)
 {
 	struct cros_typec_port *port = typec->ports[port_num];
-	struct ec_response_typec_discovery *sop_disc = port->sop_disc;
+	struct ec_response_typec_discovery *sop_disc = port->disc_data;
 	struct ec_params_typec_discovery req = {
 		.port = port_num,
 		.partner_type = TYPEC_PARTNER_SOP,
-- 
2.29.1.341.ge80a0c044ae-goog

[PATCH v2 4/6] platform/chrome: cros_ec_typec: Register cable

[Prashant Malani]

When the Chrome Embedded Controller notifies the driver that SOP'
discovery is complete, retrieve the PD discovery data and register a
cable object with the Type C connector class framework.

Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v2:
- No changes.

 drivers/platform/chrome/cros_ec_typec.c | 67 +++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 3c8ff07c8803..5e7f0b4ebbec 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -44,8 +44,11 @@ struct cros_typec_port {
 	/* Initial capabilities for the port. */
 	struct typec_capability caps;
 	struct typec_partner *partner;
+	struct typec_cable *cable;
 	/* Port partner PD identity info. */
 	struct usb_pd_identity p_identity;
+	/* Port cable PD identity info. */
+	struct usb_pd_identity c_identity;
 	struct typec_switch *ori_sw;
 	struct typec_mux *mux;
 	struct usb_role_switch *role_sw;
@@ -59,6 +62,7 @@ struct cros_typec_port {
 
 	/* Flag indicating that PD partner discovery data parsing is completed. */
 	bool sop_disc_done;
+	bool sop_prime_disc_done;
 	struct ec_response_typec_discovery *disc_data;
 	struct list_head partner_mode_list;
 };
@@ -213,6 +217,17 @@ static void cros_typec_remove_partner(struct cros_typec_data *typec,
 	port->sop_disc_done = false;
 }
 
+static void cros_typec_remove_cable(struct cros_typec_data *typec,
+				    int port_num)
+{
+	struct cros_typec_port *port = typec->ports[port_num];
+
+	typec_unregister_cable(port->cable);
+	port->cable = NULL;
+	memset(&port->c_identity, 0, sizeof(port->c_identity));
+	port->sop_prime_disc_done = false;
+}
+
 static void cros_unregister_ports(struct cros_typec_data *typec)
 {
 	int i;
@@ -224,6 +239,9 @@ static void cros_unregister_ports(struct cros_typec_data *typec)
 		if (typec->ports[i]->partner)
 			cros_typec_remove_partner(typec, i);
 
+		if (typec->ports[i]->cable)
+			cros_typec_remove_cable(typec, i);
+
 		usb_role_switch_put(typec->ports[i]->role_sw);
 		typec_switch_put(typec->ports[i]->ori_sw);
 		typec_mux_put(typec->ports[i]->mux);
@@ -600,6 +618,9 @@ static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
 		if (!typec->ports[port_num]->partner)
 			return;
 		cros_typec_remove_partner(typec, port_num);
+
+		if (typec->ports[port_num]->cable)
+			cros_typec_remove_cable(typec, port_num);
 	}
 }
 
@@ -679,6 +700,43 @@ static void cros_typec_parse_pd_identity(struct usb_pd_identity *id,
 		id->vdo[i - 3] = disc->discovery_vdo[i];
 }
 
+static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec, int port_num)
+{
+	struct cros_typec_port *port = typec->ports[port_num];
+	struct ec_response_typec_discovery *disc = port->disc_data;
+	struct typec_cable_desc desc = {};
+	struct ec_params_typec_discovery req = {
+		.port = port_num,
+		.partner_type = TYPEC_PARTNER_SOP_PRIME,
+	};
+	int ret = 0;
+
+	memset(disc, 0, EC_PROTO2_MAX_RESPONSE_SIZE);
+	ret = cros_typec_ec_command(typec, 0, EC_CMD_TYPEC_DISCOVERY, &req, sizeof(req),
+				    disc, EC_PROTO2_MAX_RESPONSE_SIZE);
+	if (ret < 0) {
+		dev_err(typec->dev, "Failed to get SOP' discovery data for port: %d\n", port_num);
+		goto sop_prime_disc_exit;
+	}
+
+	/* Parse the PD identity data, even if only 0s were returned. */
+	cros_typec_parse_pd_identity(&port->c_identity, disc);
+
+	if (disc->identity_count != 0)
+		desc.active = PD_IDH_PTYPE(port->c_identity.id_header) == IDH_PTYPE_ACABLE;
+
+	desc.identity = &port->c_identity;
+
+	port->cable = typec_register_cable(port->port, &desc);
+	if (IS_ERR(port->cable)) {
+		ret = PTR_ERR(port->cable);
+		port->cable = NULL;
+	}
+
+sop_prime_disc_exit:
+	return ret;
+}
+
 static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_num)
 {
 	struct cros_typec_port *port = typec->ports[port_num];
@@ -746,6 +804,15 @@ static void cros_typec_handle_status(struct cros_typec_data *typec, int port_num
 		else
 			typec->ports[port_num]->sop_disc_done = true;
 	}
+
+	if (resp.events & PD_STATUS_EVENT_SOP_PRIME_DISC_DONE &&
+	    !typec->ports[port_num]->sop_prime_disc_done) {
+		ret = cros_typec_handle_sop_prime_disc(typec, port_num);
+		if (ret < 0)
+			dev_err(typec->dev, "Couldn't parse SOP' Disc data, port: %d\n", port_num);
+		else
+			typec->ports[port_num]->sop_prime_disc_done = true;
+	}
 }
 
 static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
-- 
2.29.1.341.ge80a0c044ae-goog

[PATCH v2 5/6] usb: pd: Add captive Type C cable type

[Prashant Malani]

The USB Power Delivery Specification R3.0 adds a captive cable type
to the "USB Type-C plug to USB Type-C/Captive" field (Bits 19-18,
Passive/Active Cable VDO, Table 6-38 & 6-39).

Add the corresponding definition to the Cable VDO header. Also add a
helper macro to get the Type C cable connector type, when provided
the cable VDO.

Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v2:
- No changes.

 include/linux/usb/pd_vdo.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/usb/pd_vdo.h b/include/linux/usb/pd_vdo.h
index 68bdc4e2f5a9..8c5cb5830754 100644
--- a/include/linux/usb/pd_vdo.h
+++ b/include/linux/usb/pd_vdo.h
@@ -177,7 +177,7 @@
  * <31:28> :: Cable HW version
  * <27:24> :: Cable FW version
  * <23:20> :: Reserved, Shall be set to zero
- * <19:18> :: type-C to Type-A/B/C (00b == A, 01 == B, 10 == C)
+ * <19:18> :: type-C to Type-A/B/C/Captive (00b == A, 01 == B, 10 == C, 11 == Captive)
  * <17>    :: Type-C to Plug/Receptacle (0b == plug, 1b == receptacle)
  * <16:13> :: cable latency (0001 == <10ns(~1m length))
  * <12:11> :: cable termination type (11b == both ends active VCONN req)
@@ -193,6 +193,7 @@
 #define CABLE_ATYPE		0
 #define CABLE_BTYPE		1
 #define CABLE_CTYPE		2
+#define CABLE_CAPTIVE		3
 #define CABLE_PLUG		0
 #define CABLE_RECEPTACLE	1
 #define CABLE_CURR_1A5		0
@@ -208,6 +209,7 @@
 	 | (tx1d) << 10 | (tx2d) << 9 | (rx1d) << 8 | (rx2d) << 7	\
 	 | ((cur) & 0x3) << 5 | (vps) << 4 | (sopp) << 3		\
 	 | ((usbss) & 0x7))
+#define VDO_TYPEC_CABLE_TYPE(vdo)	(((vdo) >> 18) & 0x3)
 
 /*
  * AMA VDO
-- 
2.29.1.341.ge80a0c044ae-goog

[PATCH v2 6/6] platform/chrome: cros_ec_typec: Store cable plug type

[Prashant Malani]

Use the PD VDO Type C cable plug type macro to retrieve and store the
cable plug type in the cable descriptor.

Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v2:
- Changed local variable from uint32_to u32.

 drivers/platform/chrome/cros_ec_typec.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 5e7f0b4ebbec..cf609aa10567 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -709,6 +709,7 @@ static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec, int p
 		.port = port_num,
 		.partner_type = TYPEC_PARTNER_SOP_PRIME,
 	};
+	u32 cable_plug_type;
 	int ret = 0;
 
 	memset(disc, 0, EC_PROTO2_MAX_RESPONSE_SIZE);
@@ -722,8 +723,26 @@ static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec, int p
 	/* Parse the PD identity data, even if only 0s were returned. */
 	cros_typec_parse_pd_identity(&port->c_identity, disc);
 
-	if (disc->identity_count != 0)
+	if (disc->identity_count != 0) {
+		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;
+			break;
+		case CABLE_BTYPE:
+			desc.type = USB_PLUG_TYPE_B;
+			break;
+		case CABLE_CTYPE:
+			desc.type = USB_PLUG_TYPE_C;
+			break;
+		case CABLE_CAPTIVE:
+			desc.type = USB_PLUG_CAPTIVE;
+			break;
+		default:
+			desc.type = USB_PLUG_NONE;
+		}
 		desc.active = PD_IDH_PTYPE(port->c_identity.id_header) == IDH_PTYPE_ACABLE;
+	}
 
 	desc.identity = &port->c_identity;
 
-- 
2.29.1.341.ge80a0c044ae-goog

Re: [PATCH v2 5/6] usb: pd: Add captive Type C cable type

[Benson Leung]

[-- Attachment #1: Type: text/plain, Size: 2495 bytes --]

Hi Prashant,


On Fri, Nov 06, 2020 at 10:41:08AM -0800, Prashant Malani wrote:
> The USB Power Delivery Specification R3.0 adds a captive cable type
> to the "USB Type-C plug to USB Type-C/Captive" field (Bits 19-18,
> Passive/Active Cable VDO, Table 6-38 & 6-39).
> 
> Add the corresponding definition to the Cable VDO header. Also add a
> helper macro to get the Type C cable connector type, when provided
> the cable VDO.
> 
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

Reviewed-by: Benson Leung <bleung@chromium.org>

Looks good, to modify the Cable VDO decoding to bring it up to date to PD 3.0.

I should also note that the ID Header VDO also has a new field (that I added to
the PD spec) that marks whether or not SOP/SOP' is a captive plug versus a
receptacle as well. Nothing depends on it yet, but we should keep this in
mind for future changes to the connector class.

Thanks,
Benson

> ---
> 
> Changes in v2:
> - No changes.
> 
>  include/linux/usb/pd_vdo.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/usb/pd_vdo.h b/include/linux/usb/pd_vdo.h
> index 68bdc4e2f5a9..8c5cb5830754 100644
> --- a/include/linux/usb/pd_vdo.h
> +++ b/include/linux/usb/pd_vdo.h
> @@ -177,7 +177,7 @@
>   * <31:28> :: Cable HW version
>   * <27:24> :: Cable FW version
>   * <23:20> :: Reserved, Shall be set to zero
> - * <19:18> :: type-C to Type-A/B/C (00b == A, 01 == B, 10 == C)
> + * <19:18> :: type-C to Type-A/B/C/Captive (00b == A, 01 == B, 10 == C, 11 == Captive)
>   * <17>    :: Type-C to Plug/Receptacle (0b == plug, 1b == receptacle)
>   * <16:13> :: cable latency (0001 == <10ns(~1m length))
>   * <12:11> :: cable termination type (11b == both ends active VCONN req)
> @@ -193,6 +193,7 @@
>  #define CABLE_ATYPE		0
>  #define CABLE_BTYPE		1
>  #define CABLE_CTYPE		2
> +#define CABLE_CAPTIVE		3
>  #define CABLE_PLUG		0
>  #define CABLE_RECEPTACLE	1
>  #define CABLE_CURR_1A5		0
> @@ -208,6 +209,7 @@
>  	 | (tx1d) << 10 | (tx2d) << 9 | (rx1d) << 8 | (rx2d) << 7	\
>  	 | ((cur) & 0x3) << 5 | (vps) << 4 | (sopp) << 3		\
>  	 | ((usbss) & 0x7))
> +#define VDO_TYPEC_CABLE_TYPE(vdo)	(((vdo) >> 18) & 0x3)
>  
>  /*
>   * AMA VDO
> -- 
> 2.29.1.341.ge80a0c044ae-goog
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 0/6] platform/chrome: cros_ec_typec: Add cable

[Greg KH]

On Fri, Nov 06, 2020 at 10:40:57AM -0800, Prashant Malani wrote:
> The following series adds Type C cable registration to the cros-ec-typec
> port driver using the Type C connector class framework. The first few
> patches perform a few minor re-organizations to prepare for the cable
> registration patch.
> 
> The last couple of CLs update the USB PD VDO header file to add a
> captive cable connector for the Type C cable plug field, and then use
> the added macro to add the corresponding field of the Type C cable
> descriptor in the cros-ec-typec driver.
> 
> v1:
> https://lore.kernel.org/lkml/20201106012758.525472-1-pmalani@chromium.org/
> 
> Changes since v2:
> - Changed local variable uint32_t to u32 in patch 6/6.

Looks good to me, for all of these:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v2 1/6] platform/chrome: cros_ec_typec: Make disc_done flag partner-only

[Heikki Krogerus]

On Fri, Nov 06, 2020 at 10:40:59AM -0800, Prashant Malani wrote:
> Change the disc_done flag, which indicates whether PD discovery is
> complete, to sop_disc_done instead, since we will process SOP and SOP'
> discovery data separately.
> 
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Changes in v2:
> - No changes.
> 
>  drivers/platform/chrome/cros_ec_typec.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index ce031a10eb1b..801c3d2c1fbd 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -57,8 +57,8 @@ struct cros_typec_port {
>  	/* Port alt modes. */
>  	struct typec_altmode p_altmode[CROS_EC_ALTMODE_MAX];
>  
> -	/* Flag indicating that PD discovery data parsing is completed. */
> -	bool disc_done;
> +	/* Flag indicating that PD partner discovery data parsing is completed. */
> +	bool sop_disc_done;
>  	struct ec_response_typec_discovery *sop_disc;
>  	struct list_head partner_mode_list;
>  };
> @@ -210,7 +210,7 @@ static void cros_typec_remove_partner(struct cros_typec_data *typec,
>  	typec_unregister_partner(port->partner);
>  	port->partner = NULL;
>  	memset(&port->p_identity, 0, sizeof(port->p_identity));
> -	port->disc_done = false;
> +	port->sop_disc_done = false;
>  }
>  
>  static void cros_unregister_ports(struct cros_typec_data *typec)
> @@ -727,18 +727,13 @@ static void cros_typec_handle_status(struct cros_typec_data *typec, int port_num
>  		return;
>  	}
>  
> -	if (typec->ports[port_num]->disc_done)
> -		return;
> -
>  	/* Handle any events appropriately. */
> -	if (resp.events & PD_STATUS_EVENT_SOP_DISC_DONE) {
> +	if (resp.events & PD_STATUS_EVENT_SOP_DISC_DONE && !typec->ports[port_num]->sop_disc_done) {
>  		ret = cros_typec_handle_sop_disc(typec, port_num);
> -		if (ret < 0) {
> +		if (ret < 0)
>  			dev_err(typec->dev, "Couldn't parse SOP Disc data, port: %d\n", port_num);
> -			return;
> -		}
> -
> -		typec->ports[port_num]->disc_done = true;
> +		else
> +			typec->ports[port_num]->sop_disc_done = true;
>  	}
>  }
>  

thanks,

-- 
heikki

Re: [PATCH v2 2/6] platform/chrome: cros_ec_typec: Factor out PD identity parsing

[Heikki Krogerus]

On Fri, Nov 06, 2020 at 10:41:01AM -0800, Prashant Malani wrote:
> Factor out the PD identity parsing code into a separate function. This
> way it can be re-used for Cable PD identity parsing in future patches.
> 
> No functional changes are introduced by this patch.
> 
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Changes in v2:
> - No changes.
> 
>  drivers/platform/chrome/cros_ec_typec.c | 35 ++++++++++++++++---------
>  1 file changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 801c3d2c1fbd..f6d3c37c2c27 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -657,6 +657,28 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
>  	return ret;
>  }
>  
> +/*
> + * Parse the PD identity data from the EC PD discovery responses and copy that to the supplied
> + * PD identity struct.
> + */
> +static void cros_typec_parse_pd_identity(struct usb_pd_identity *id,
> +					 struct ec_response_typec_discovery *disc)
> +{
> +	int i;
> +
> +	/* First, update the PD identity VDOs for the partner. */
> +	if (disc->identity_count > 0)
> +		id->id_header = disc->discovery_vdo[0];
> +	if (disc->identity_count > 1)
> +		id->cert_stat = disc->discovery_vdo[1];
> +	if (disc->identity_count > 2)
> +		id->product = disc->discovery_vdo[2];
> +
> +	/* Copy the remaining identity VDOs till a maximum of 6. */
> +	for (i = 3; i < disc->identity_count && i < VDO_MAX_OBJECTS; i++)
> +		id->vdo[i - 3] = disc->discovery_vdo[i];
> +}
> +
>  static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_num)
>  {
>  	struct cros_typec_port *port = typec->ports[port_num];
> @@ -666,7 +688,6 @@ static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_nu
>  		.partner_type = TYPEC_PARTNER_SOP,
>  	};
>  	int ret = 0;
> -	int i;
>  
>  	if (!port->partner) {
>  		dev_err(typec->dev,
> @@ -684,17 +705,7 @@ static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_nu
>  		goto disc_exit;
>  	}
>  
> -	/* First, update the PD identity VDOs for the partner. */
> -	if (sop_disc->identity_count > 0)
> -		port->p_identity.id_header = sop_disc->discovery_vdo[0];
> -	if (sop_disc->identity_count > 1)
> -		port->p_identity.cert_stat = sop_disc->discovery_vdo[1];
> -	if (sop_disc->identity_count > 2)
> -		port->p_identity.product = sop_disc->discovery_vdo[2];
> -
> -	/* Copy the remaining identity VDOs till a maximum of 6. */
> -	for (i = 3; i < sop_disc->identity_count && i < VDO_MAX_OBJECTS; i++)
> -		port->p_identity.vdo[i - 3] = sop_disc->discovery_vdo[i];
> +	cros_typec_parse_pd_identity(&port->p_identity, sop_disc);
>  
>  	ret = typec_partner_set_identity(port->partner);
>  	if (ret < 0) {
> -- 

thanks,

-- 
heikki

Re: [PATCH v2 3/6] platform/chrome: cros_ec_typec: Rename discovery struct

[Heikki Krogerus]

On Fri, Nov 06, 2020 at 10:41:04AM -0800, Prashant Malani wrote:
> Rename the sop_disc data struct which is used to store PD discovery data
> to the more generic name of disc_data. It can then be re-used to store
> and process cable discovery data.
> 
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Changes in v2:
> - No changes.
> 
>  drivers/platform/chrome/cros_ec_typec.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index f6d3c37c2c27..3c8ff07c8803 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -59,7 +59,7 @@ struct cros_typec_port {
>  
>  	/* Flag indicating that PD partner discovery data parsing is completed. */
>  	bool sop_disc_done;
> -	struct ec_response_typec_discovery *sop_disc;
> +	struct ec_response_typec_discovery *disc_data;
>  	struct list_head partner_mode_list;
>  };
>  
> @@ -323,8 +323,8 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
>  
>  		cros_typec_register_port_altmodes(typec, port_num);
>  
> -		cros_port->sop_disc = devm_kzalloc(dev, EC_PROTO2_MAX_RESPONSE_SIZE, GFP_KERNEL);
> -		if (!cros_port->sop_disc) {
> +		cros_port->disc_data = devm_kzalloc(dev, EC_PROTO2_MAX_RESPONSE_SIZE, GFP_KERNEL);
> +		if (!cros_port->disc_data) {
>  			ret = -ENOMEM;
>  			goto unregister_ports;
>  		}
> @@ -617,7 +617,7 @@ static int cros_typec_get_mux_info(struct cros_typec_data *typec, int port_num,
>  static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_num)
>  {
>  	struct cros_typec_port *port = typec->ports[port_num];
> -	struct ec_response_typec_discovery *sop_disc = port->sop_disc;
> +	struct ec_response_typec_discovery *sop_disc = port->disc_data;
>  	struct cros_typec_altmode_node *node;
>  	struct typec_altmode_desc desc;
>  	struct typec_altmode *amode;
> @@ -682,7 +682,7 @@ static void cros_typec_parse_pd_identity(struct usb_pd_identity *id,
>  static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_num)
>  {
>  	struct cros_typec_port *port = typec->ports[port_num];
> -	struct ec_response_typec_discovery *sop_disc = port->sop_disc;
> +	struct ec_response_typec_discovery *sop_disc = port->disc_data;
>  	struct ec_params_typec_discovery req = {
>  		.port = port_num,
>  		.partner_type = TYPEC_PARTNER_SOP,
> -- 
> 2.29.1.341.ge80a0c044ae-goog

thanks,

-- 
heikki

Re: [PATCH v2 4/6] platform/chrome: cros_ec_typec: Register cable

[Heikki Krogerus]

On Fri, Nov 06, 2020 at 10:41:06AM -0800, Prashant Malani wrote:
> When the Chrome Embedded Controller notifies the driver that SOP'
> discovery is complete, retrieve the PD discovery data and register a
> cable object with the Type C connector class framework.
> 
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Changes in v2:
> - No changes.
> 
>  drivers/platform/chrome/cros_ec_typec.c | 67 +++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 3c8ff07c8803..5e7f0b4ebbec 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -44,8 +44,11 @@ struct cros_typec_port {
>  	/* Initial capabilities for the port. */
>  	struct typec_capability caps;
>  	struct typec_partner *partner;
> +	struct typec_cable *cable;
>  	/* Port partner PD identity info. */
>  	struct usb_pd_identity p_identity;
> +	/* Port cable PD identity info. */
> +	struct usb_pd_identity c_identity;
>  	struct typec_switch *ori_sw;
>  	struct typec_mux *mux;
>  	struct usb_role_switch *role_sw;
> @@ -59,6 +62,7 @@ struct cros_typec_port {
>  
>  	/* Flag indicating that PD partner discovery data parsing is completed. */
>  	bool sop_disc_done;
> +	bool sop_prime_disc_done;
>  	struct ec_response_typec_discovery *disc_data;
>  	struct list_head partner_mode_list;
>  };
> @@ -213,6 +217,17 @@ static void cros_typec_remove_partner(struct cros_typec_data *typec,
>  	port->sop_disc_done = false;
>  }
>  
> +static void cros_typec_remove_cable(struct cros_typec_data *typec,
> +				    int port_num)
> +{
> +	struct cros_typec_port *port = typec->ports[port_num];
> +
> +	typec_unregister_cable(port->cable);
> +	port->cable = NULL;
> +	memset(&port->c_identity, 0, sizeof(port->c_identity));
> +	port->sop_prime_disc_done = false;
> +}
> +
>  static void cros_unregister_ports(struct cros_typec_data *typec)
>  {
>  	int i;
> @@ -224,6 +239,9 @@ static void cros_unregister_ports(struct cros_typec_data *typec)
>  		if (typec->ports[i]->partner)
>  			cros_typec_remove_partner(typec, i);
>  
> +		if (typec->ports[i]->cable)
> +			cros_typec_remove_cable(typec, i);
> +
>  		usb_role_switch_put(typec->ports[i]->role_sw);
>  		typec_switch_put(typec->ports[i]->ori_sw);
>  		typec_mux_put(typec->ports[i]->mux);
> @@ -600,6 +618,9 @@ static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
>  		if (!typec->ports[port_num]->partner)
>  			return;
>  		cros_typec_remove_partner(typec, port_num);
> +
> +		if (typec->ports[port_num]->cable)
> +			cros_typec_remove_cable(typec, port_num);
>  	}
>  }
>  
> @@ -679,6 +700,43 @@ static void cros_typec_parse_pd_identity(struct usb_pd_identity *id,
>  		id->vdo[i - 3] = disc->discovery_vdo[i];
>  }
>  
> +static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec, int port_num)
> +{
> +	struct cros_typec_port *port = typec->ports[port_num];
> +	struct ec_response_typec_discovery *disc = port->disc_data;
> +	struct typec_cable_desc desc = {};
> +	struct ec_params_typec_discovery req = {
> +		.port = port_num,
> +		.partner_type = TYPEC_PARTNER_SOP_PRIME,
> +	};
> +	int ret = 0;
> +
> +	memset(disc, 0, EC_PROTO2_MAX_RESPONSE_SIZE);
> +	ret = cros_typec_ec_command(typec, 0, EC_CMD_TYPEC_DISCOVERY, &req, sizeof(req),
> +				    disc, EC_PROTO2_MAX_RESPONSE_SIZE);
> +	if (ret < 0) {
> +		dev_err(typec->dev, "Failed to get SOP' discovery data for port: %d\n", port_num);
> +		goto sop_prime_disc_exit;
> +	}
> +
> +	/* Parse the PD identity data, even if only 0s were returned. */
> +	cros_typec_parse_pd_identity(&port->c_identity, disc);
> +
> +	if (disc->identity_count != 0)
> +		desc.active = PD_IDH_PTYPE(port->c_identity.id_header) == IDH_PTYPE_ACABLE;
> +
> +	desc.identity = &port->c_identity;
> +
> +	port->cable = typec_register_cable(port->port, &desc);
> +	if (IS_ERR(port->cable)) {
> +		ret = PTR_ERR(port->cable);
> +		port->cable = NULL;
> +	}
> +
> +sop_prime_disc_exit:
> +	return ret;
> +}
> +
>  static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_num)
>  {
>  	struct cros_typec_port *port = typec->ports[port_num];
> @@ -746,6 +804,15 @@ static void cros_typec_handle_status(struct cros_typec_data *typec, int port_num
>  		else
>  			typec->ports[port_num]->sop_disc_done = true;
>  	}
> +
> +	if (resp.events & PD_STATUS_EVENT_SOP_PRIME_DISC_DONE &&
> +	    !typec->ports[port_num]->sop_prime_disc_done) {
> +		ret = cros_typec_handle_sop_prime_disc(typec, port_num);
> +		if (ret < 0)
> +			dev_err(typec->dev, "Couldn't parse SOP' Disc data, port: %d\n", port_num);
> +		else
> +			typec->ports[port_num]->sop_prime_disc_done = true;
> +	}
>  }
>  
>  static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
> -- 
> 2.29.1.341.ge80a0c044ae-goog

thanks,

-- 
heikki

Re: [PATCH v2 5/6] usb: pd: Add captive Type C cable type

[Heikki Krogerus]

On Fri, Nov 06, 2020 at 10:41:08AM -0800, Prashant Malani wrote:
> The USB Power Delivery Specification R3.0 adds a captive cable type
> to the "USB Type-C plug to USB Type-C/Captive" field (Bits 19-18,
> Passive/Active Cable VDO, Table 6-38 & 6-39).
> 
> Add the corresponding definition to the Cable VDO header. Also add a
> helper macro to get the Type C cable connector type, when provided
> the cable VDO.
> 
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Changes in v2:
> - No changes.
> 
>  include/linux/usb/pd_vdo.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/usb/pd_vdo.h b/include/linux/usb/pd_vdo.h
> index 68bdc4e2f5a9..8c5cb5830754 100644
> --- a/include/linux/usb/pd_vdo.h
> +++ b/include/linux/usb/pd_vdo.h
> @@ -177,7 +177,7 @@
>   * <31:28> :: Cable HW version
>   * <27:24> :: Cable FW version
>   * <23:20> :: Reserved, Shall be set to zero
> - * <19:18> :: type-C to Type-A/B/C (00b == A, 01 == B, 10 == C)
> + * <19:18> :: type-C to Type-A/B/C/Captive (00b == A, 01 == B, 10 == C, 11 == Captive)
>   * <17>    :: Type-C to Plug/Receptacle (0b == plug, 1b == receptacle)
>   * <16:13> :: cable latency (0001 == <10ns(~1m length))
>   * <12:11> :: cable termination type (11b == both ends active VCONN req)
> @@ -193,6 +193,7 @@
>  #define CABLE_ATYPE		0
>  #define CABLE_BTYPE		1
>  #define CABLE_CTYPE		2
> +#define CABLE_CAPTIVE		3
>  #define CABLE_PLUG		0
>  #define CABLE_RECEPTACLE	1
>  #define CABLE_CURR_1A5		0
> @@ -208,6 +209,7 @@
>  	 | (tx1d) << 10 | (tx2d) << 9 | (rx1d) << 8 | (rx2d) << 7	\
>  	 | ((cur) & 0x3) << 5 | (vps) << 4 | (sopp) << 3		\
>  	 | ((usbss) & 0x7))
> +#define VDO_TYPEC_CABLE_TYPE(vdo)	(((vdo) >> 18) & 0x3)
>  
>  /*
>   * AMA VDO
> -- 
> 2.29.1.341.ge80a0c044ae-goog

thanks,

-- 
heikki

Re: [PATCH v2 6/6] platform/chrome: cros_ec_typec: Store cable plug type

[Heikki Krogerus]

On Fri, Nov 06, 2020 at 10:41:10AM -0800, Prashant Malani wrote:
> Use the PD VDO Type C cable plug type macro to retrieve and store the
> cable plug type in the cable descriptor.
> 
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Changes in v2:
> - Changed local variable from uint32_to u32.
> 
>  drivers/platform/chrome/cros_ec_typec.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 5e7f0b4ebbec..cf609aa10567 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -709,6 +709,7 @@ static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec, int p
>  		.port = port_num,
>  		.partner_type = TYPEC_PARTNER_SOP_PRIME,
>  	};
> +	u32 cable_plug_type;
>  	int ret = 0;
>  
>  	memset(disc, 0, EC_PROTO2_MAX_RESPONSE_SIZE);
> @@ -722,8 +723,26 @@ static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec, int p
>  	/* Parse the PD identity data, even if only 0s were returned. */
>  	cros_typec_parse_pd_identity(&port->c_identity, disc);
>  
> -	if (disc->identity_count != 0)
> +	if (disc->identity_count != 0) {
> +		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;
> +			break;
> +		case CABLE_BTYPE:
> +			desc.type = USB_PLUG_TYPE_B;
> +			break;
> +		case CABLE_CTYPE:
> +			desc.type = USB_PLUG_TYPE_C;
> +			break;
> +		case CABLE_CAPTIVE:
> +			desc.type = USB_PLUG_CAPTIVE;
> +			break;
> +		default:
> +			desc.type = USB_PLUG_NONE;
> +		}
>  		desc.active = PD_IDH_PTYPE(port->c_identity.id_header) == IDH_PTYPE_ACABLE;
> +	}
>  
>  	desc.identity = &port->c_identity;
>  
> -- 
> 2.29.1.341.ge80a0c044ae-goog

thanks,

-- 
heikki