2020-11-06 18:43:12

by Prashant Malani

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

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/[email protected]/

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


2020-11-06 18:43:17

by Prashant Malani

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

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 <[email protected]>
---

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

2020-11-06 18:43:51

by Prashant Malani

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

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 <[email protected]>
---

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

2020-11-06 18:44:13

by Prashant Malani

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

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 <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---

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

2020-11-06 18:45:26

by Prashant Malani

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

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 <[email protected]>
---

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

2020-11-06 18:46:34

by Prashant Malani

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

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 <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---

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

2020-11-06 18:47:01

by Prashant Malani

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

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 <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---

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

2020-11-06 18:55:52

by Benson Leung

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

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 <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>

Reviewed-by: Benson Leung <[email protected]>

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.
[email protected]
Chromium OS Project
[email protected]


Attachments:
(No filename) (2.44 kB)
signature.asc (235.00 B)
Download all attachments

2020-11-07 08:39:07

by Greg Kroah-Hartman

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

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/[email protected]/
>
> 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 <[email protected]>

2020-11-09 08:23:33

by Heikki Krogerus

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

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 <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
>
> 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

2020-11-09 08:25:15

by Heikki Krogerus

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

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 <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
>
> 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

2020-11-09 08:28:28

by Heikki Krogerus

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

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 <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
>
> 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

2020-11-09 08:30:20

by Heikki Krogerus

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

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 <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
>
> 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

2020-11-09 08:33:32

by Heikki Krogerus

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

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 <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
>
> 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

2020-11-09 08:35:24

by Heikki Krogerus

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

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 <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
>
> 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