[PATCH] platform/chrome: cros_ec_typec: Track port role
Stash the currently reported port role in the port struct and add a
check for that too while determining whether to re-configure on-board
Type C switches (this deals with cases like role swaps where the mux
flags don't change, but the port role does).
Signed-off-by: Prashant Malani
Suggested-by: Nikunj A. Dadhania
Tested-by: Deepti Deshatty
---
drivers/platform/chrome/cros_ec_typec.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index d3df1717a5fd..1a06b8c80f80 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -58,6 +58,7 @@ struct cros_typec_port {
/* Variables keeping track of switch state. */
struct typec_mux_state state;
uint8_t mux_flags;
+ uint8_t role;
/* Port alt modes. */
struct typec_altmode p_altmode[CROS_EC_ALTMODE_MAX];
@@ -995,10 +996,12 @@ static int cros_typec_port_update(struct cros_typec_data
*typec, int port_num)
}
/* No change needs to be made, let's exit early. */
- if (typec->ports[port_num]->mux_flags == mux_resp.flags)
+ if (typec->ports[port_num]->mux_flags == mux_resp.flags &&
+ typec->ports[port_num]->role == resp.role)
return 0;
typec->ports[port_num]->mux_flags = mux_resp.flags;
+ typec->ports[port_num]->role = resp.role;
ret = cros_typec_configure_mux(typec, port_num, mux_resp.flags, &resp);
if (ret)
dev_warn(typec->dev, "Configure muxes failed, err = %d\n", ret);
--
2.31.1.368.gbe11c130af-goog
Re: [PATCH v2 1/2] platform/chrome: cros_ec: Add Type C hard reset
Hi Enric, On Tue, Apr 20, 2021 at 3:00 AM Enric Balletbo i Serra wrote: > > Hi Prashant, > > On 16/4/21 20:27, Prashant Malani wrote: > > Update the EC command header to include the new event bit. This bit > > is included in the latest version of the Chrome EC headers[1]. > > > > [1] > > https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h > > > > Change-Id: I52a36e725d945665814d4e59ddd1e76a3692db9f > > Please remember to remove the ChromeOS specific tags and add properly the > Signed-off Sorry, missed this. Will send another version.
[PATCH v3 1/2] platform/chrome: cros_ec: Add Type C hard reset
Update the EC command header to include the new event bit. This bit
is included in the latest version of the Chrome EC headers[1].
[1]
https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h
Signed-off-by: Prashant Malani
---
Changes in v3:
- Remove Change-Id tag and add Signed-off-by tag to the commit message.
v2 is the first version the patch was introduced.
include/linux/platform_data/cros_ec_commands.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/platform_data/cros_ec_commands.h
b/include/linux/platform_data/cros_ec_commands.h
index 5ff8597ceabd..9156078c6fc6 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -5678,6 +5678,7 @@ enum tcpc_cc_polarity {
#define PD_STATUS_EVENT_SOP_DISC_DONE BIT(0)
#define PD_STATUS_EVENT_SOP_PRIME_DISC_DONEBIT(1)
+#define PD_STATUS_EVENT_HARD_RESET BIT(2)
struct ec_params_typec_status {
uint8_t port;
--
2.31.1.368.gbe11c130af-goog
[PATCH v3 2/2] platform/chrome: cros_ec_typec: Handle hard reset
The Chrome Embedded Controller (EC) generates a hard reset type C event
when a USB Power Delivery (PD) hard reset is encountered. Handle this
event by unregistering the partner and cable on the associated port and
clearing the event flag.
Cc: Benson Leung
Signed-off-by: Prashant Malani
---
Changes in v3:
- No changes.
Changes in v2:
- Split the header change into a separate patch.
- Updated the commit message to reflect the above.
v1:
https://lore.kernel.org/lkml/20210415021456.1805797-1-pmal...@chromium.org/
drivers/platform/chrome/cros_ec_typec.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index d3df1717a5fd..22052f569f2a 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -905,6 +905,19 @@ static void cros_typec_handle_status(struct
cros_typec_data *typec, int port_num
return;
}
+ /* If we got a hard reset, unregister everything and return. */
+ if (resp.events & PD_STATUS_EVENT_HARD_RESET) {
+ cros_typec_remove_partner(typec, port_num);
+ cros_typec_remove_cable(typec, port_num);
+
+ ret = cros_typec_send_clear_event(typec, port_num,
+ PD_STATUS_EVENT_HARD_RESET);
+ if (ret < 0)
+ dev_warn(typec->dev,
+"Failed hard reset event clear, port: %d\n",
port_num);
+ return;
+ }
+
/* Handle any events appropriately. */
if (resp.events & PD_STATUS_EVENT_SOP_DISC_DONE &&
!typec->ports[port_num]->sop_disc_done) {
u16 sop_revision;
--
2.31.1.368.gbe11c130af-goog
[PATCH] platform/chrome: cros_ec_typec: Flush pending work
When a PD notifier event arrives, a new work event won't be enqueued if
the current one hasn't completed. This could lead to dropped events.
So, flush any pending work before scheduling the new instance.
Signed-off-by: Prashant Malani
---
drivers/platform/chrome/cros_ec_typec.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index db83c03ae5cd..2fac95e7a455 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -1031,6 +1031,7 @@ static int cros_ec_typec_event(struct notifier_block *nb,
{
struct cros_typec_data *typec = container_of(nb, struct
cros_typec_data, nb);
+ flush_work(&typec->port_work);
schedule_work(&typec->port_work);
return NOTIFY_OK;
--
2.30.0.478.g8a0d178c01-goog
[PATCH] platform/chrome: cros_ec_typec: Decouple partner removal
Currently, we return if there is no partner present when
!PD_CTRL_RESP_ENABLED_CONNECTED, without proceeding further. This ties
partner removal to cable removal, whereas the two should be independent.
Update the check to remove a partner if one was registered, but continue
after that instead of returning.
Signed-off-by: Prashant Malani
---
drivers/platform/chrome/cros_ec_typec.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index e724a5eaef1c..91b8fc1fd7f3 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -638,9 +638,8 @@ static void cros_typec_set_port_params_v1(struct
cros_typec_data *typec,
"Failed to register partner on port: %d\n",
port_num);
} else {
- if (!typec->ports[port_num]->partner)
- return;
- cros_typec_remove_partner(typec, port_num);
+ if (typec->ports[port_num]->partner)
+ cros_typec_remove_partner(typec, port_num);
if (typec->ports[port_num]->cable)
cros_typec_remove_cable(typec, port_num);
--
2.30.0.365.g02bc693789-goog
[PATCH 1/2] platform/chrome: cros_ec: Import Type C control command
This command is used to communicate with the Chrome Embedded Controller
(EC) regarding USB Type C events and state.
These header updates are included in the latest Chrome OS EC headers [1]
[1]
https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h
Signed-off-by: Prashant Malani
---
.../linux/platform_data/cros_ec_commands.h| 26 +++
1 file changed, 26 insertions(+)
diff --git a/include/linux/platform_data/cros_ec_commands.h
b/include/linux/platform_data/cros_ec_commands.h
index d3c40220b281..a95dc22a5463 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -5578,6 +5578,32 @@ struct ec_response_typec_discovery {
struct svid_mode_info svids[0];
} __ec_align1;
+
+/* USB Type-C commands for AP-controlled device policy. */
+#define EC_CMD_TYPEC_CONTROL 0x0132
+
+enum typec_control_command {
+ TYPEC_CONTROL_COMMAND_EXIT_MODES,
+ TYPEC_CONTROL_COMMAND_CLEAR_EVENTS,
+ TYPEC_CONTROL_COMMAND_ENTER_MODE,
+};
+
+struct ec_params_typec_control {
+ uint8_t port;
+ uint8_t command;/* enum typec_control_command */
+ uint16_t reserved;
+
+ /*
+* This section will be interpreted based on |command|. Define a
+* placeholder structure to avoid having to increase the size and bump
+* the command version when adding new sub-commands.
+*/
+ union {
+ uint32_t clear_events_mask;
+ uint8_t mode_to_enter; /* enum typec_mode */
+ uint8_t placeholder[128];
+ };
+} __ec_align1;
/*
* Gather all status information for a port.
*
--
2.30.0.365.g02bc693789-goog
[PATCH 2/2] platform/chrome: cros_ec_typec: Clear Type C disc events
Clear USB Type C discovery events from the Chrome EC once they've been
successfully handled.
Signed-off-by: Prashant Malani
---
drivers/platform/chrome/cros_ec_typec.c | 28 +++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index e724a5eaef1c..f3bdb87d6dce 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -867,6 +867,18 @@ static int cros_typec_handle_sop_disc(struct
cros_typec_data *typec, int port_nu
return ret;
}
+static int cros_typec_send_clear_event(struct cros_typec_data *typec, int
port_num, u32 events_mask)
+{
+ struct ec_params_typec_control req = {
+ .port = port_num,
+ .command = TYPEC_CONTROL_COMMAND_CLEAR_EVENTS,
+ .clear_events_mask = events_mask,
+ };
+
+ return cros_typec_ec_command(typec, 0, EC_CMD_TYPEC_CONTROL, &req,
+sizeof(req), NULL, 0);
+}
+
static void cros_typec_handle_status(struct cros_typec_data *typec, int
port_num)
{
struct ec_response_typec_status resp;
@@ -887,8 +899,14 @@ static void cros_typec_handle_status(struct
cros_typec_data *typec, int port_num
ret = cros_typec_handle_sop_disc(typec, port_num);
if (ret < 0)
dev_err(typec->dev, "Couldn't parse SOP Disc data,
port: %d\n", port_num);
- else
+ else {
typec->ports[port_num]->sop_disc_done = true;
+ ret = cros_typec_send_clear_event(typec, port_num,
+
PD_STATUS_EVENT_SOP_DISC_DONE);
+ if (ret < 0)
+ dev_warn(typec->dev,
+"Failed SOP Disc event clear, port:
%d\n", port_num);
+ }
}
if (resp.events & PD_STATUS_EVENT_SOP_PRIME_DISC_DONE &&
@@ -896,8 +914,14 @@ static void cros_typec_handle_status(struct
cros_typec_data *typec, int port_num
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
+ else {
typec->ports[port_num]->sop_prime_disc_done = true;
+ ret = cros_typec_send_clear_event(typec, port_num,
+
PD_STATUS_EVENT_SOP_PRIME_DISC_DONE);
+ if (ret < 0)
+ dev_warn(typec->dev,
+"Failed SOP Disc event clear, port:
%d\n", port_num);
+ }
}
}
--
2.30.0.365.g02bc693789-goog
Re: [PATCH 1/2] platform/chrome: cros_ec: Import Type C control command
On Tue, Feb 2, 2021 at 5:49 PM Prashant Malani wrote:
>
> This command is used to communicate with the Chrome Embedded Controller
> (EC) regarding USB Type C events and state.
>
> These header updates are included in the latest Chrome OS EC headers [1]
>
> [1]
> https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h
>
> Signed-off-by: Prashant Malani
> ---
> .../linux/platform_data/cros_ec_commands.h| 26 +++
> 1 file changed, 26 insertions(+)
>
> diff --git a/include/linux/platform_data/cros_ec_commands.h
> b/include/linux/platform_data/cros_ec_commands.h
> index d3c40220b281..a95dc22a5463 100644
> --- a/include/linux/platform_data/cros_ec_commands.h
> +++ b/include/linux/platform_data/cros_ec_commands.h
> @@ -5578,6 +5578,32 @@ struct ec_response_typec_discovery {
> struct svid_mode_info svids[0];
> } __ec_align1;
>
> +
> +/* USB Type-C commands for AP-controlled device policy. */
> +#define EC_CMD_TYPEC_CONTROL 0x0132
> +
> +enum typec_control_command {
> + TYPEC_CONTROL_COMMAND_EXIT_MODES,
> + TYPEC_CONTROL_COMMAND_CLEAR_EVENTS,
> + TYPEC_CONTROL_COMMAND_ENTER_MODE,
> +};
> +
> +struct ec_params_typec_control {
> + uint8_t port;
> + uint8_t command;/* enum typec_control_command */
> + uint16_t reserved;
> +
> + /*
> +* This section will be interpreted based on |command|. Define a
> +* placeholder structure to avoid having to increase the size and bump
> +* the command version when adding new sub-commands.
> +*/
> + union {
> + uint32_t clear_events_mask;
> + uint8_t mode_to_enter; /* enum typec_mode */
> + uint8_t placeholder[128];
> + };
> +} __ec_align1;
Looks like I got the newlines incorrect while porting the structs. I
will send another version.
[PATCH v2 1/2] platform/chrome: cros_ec: Import Type C control command
This command is used to communicate with the Chrome Embedded Controller
(EC) regarding USB Type C events and state.
These header updates are included in the latest Chrome OS EC headers [1]
[1]
https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h
Signed-off-by: Prashant Malani
---
Changes in v2:
- Fixed new line errors.
.../linux/platform_data/cros_ec_commands.h| 26 +++
1 file changed, 26 insertions(+)
diff --git a/include/linux/platform_data/cros_ec_commands.h
b/include/linux/platform_data/cros_ec_commands.h
index d3c40220b281..5a967c9f8aca 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -5578,6 +5578,32 @@ struct ec_response_typec_discovery {
struct svid_mode_info svids[0];
} __ec_align1;
+/* USB Type-C commands for AP-controlled device policy. */
+#define EC_CMD_TYPEC_CONTROL 0x0132
+
+enum typec_control_command {
+ TYPEC_CONTROL_COMMAND_EXIT_MODES,
+ TYPEC_CONTROL_COMMAND_CLEAR_EVENTS,
+ TYPEC_CONTROL_COMMAND_ENTER_MODE,
+};
+
+struct ec_params_typec_control {
+ uint8_t port;
+ uint8_t command;/* enum typec_control_command */
+ uint16_t reserved;
+
+ /*
+* This section will be interpreted based on |command|. Define a
+* placeholder structure to avoid having to increase the size and bump
+* the command version when adding new sub-commands.
+*/
+ union {
+ uint32_t clear_events_mask;
+ uint8_t mode_to_enter; /* enum typec_mode */
+ uint8_t placeholder[128];
+ };
+} __ec_align1;
+
/*
* Gather all status information for a port.
*
--
2.30.0.365.g02bc693789-goog
[PATCH v2 2/2] platform/chrome: cros_ec_typec: Clear Type C disc events
Clear USB Type C discovery events from the Chrome EC once they've been
successfully handled.
Signed-off-by: Prashant Malani
---
Changes in v2:
- No changes.
drivers/platform/chrome/cros_ec_typec.c | 28 +++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index e724a5eaef1c..f3bdb87d6dce 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -867,6 +867,18 @@ static int cros_typec_handle_sop_disc(struct
cros_typec_data *typec, int port_nu
return ret;
}
+static int cros_typec_send_clear_event(struct cros_typec_data *typec, int
port_num, u32 events_mask)
+{
+ struct ec_params_typec_control req = {
+ .port = port_num,
+ .command = TYPEC_CONTROL_COMMAND_CLEAR_EVENTS,
+ .clear_events_mask = events_mask,
+ };
+
+ return cros_typec_ec_command(typec, 0, EC_CMD_TYPEC_CONTROL, &req,
+sizeof(req), NULL, 0);
+}
+
static void cros_typec_handle_status(struct cros_typec_data *typec, int
port_num)
{
struct ec_response_typec_status resp;
@@ -887,8 +899,14 @@ static void cros_typec_handle_status(struct
cros_typec_data *typec, int port_num
ret = cros_typec_handle_sop_disc(typec, port_num);
if (ret < 0)
dev_err(typec->dev, "Couldn't parse SOP Disc data,
port: %d\n", port_num);
- else
+ else {
typec->ports[port_num]->sop_disc_done = true;
+ ret = cros_typec_send_clear_event(typec, port_num,
+
PD_STATUS_EVENT_SOP_DISC_DONE);
+ if (ret < 0)
+ dev_warn(typec->dev,
+"Failed SOP Disc event clear, port:
%d\n", port_num);
+ }
}
if (resp.events & PD_STATUS_EVENT_SOP_PRIME_DISC_DONE &&
@@ -896,8 +914,14 @@ static void cros_typec_handle_status(struct
cros_typec_data *typec, int port_num
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
+ else {
typec->ports[port_num]->sop_prime_disc_done = true;
+ ret = cros_typec_send_clear_event(typec, port_num,
+
PD_STATUS_EVENT_SOP_PRIME_DISC_DONE);
+ if (ret < 0)
+ dev_warn(typec->dev,
+"Failed SOP Disc event clear, port:
%d\n", port_num);
+ }
}
}
--
2.30.0.365.g02bc693789-goog
Re: [PATCH 4/6] platform/chrome: cros_ec_typec: Report SOP' PD revision from status
Hi Benson,
On Thu, Jan 28, 2021 at 10:14 PM Benson Leung wrote:
>
> cros_typec_handle_sop_prime_disc now takes the PD revision provided
> by the EC_CMD_TYPEC_STATUS command response for the SOP'.
>
> Attach the properly formatted pd_revision to the cable desc before
> registering the cable.
>
> Signed-off-by: Benson Leung
Reviewed-by: Prashant Malani
> ---
> drivers/platform/chrome/cros_ec_typec.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c
> b/drivers/platform/chrome/cros_ec_typec.c
> index e724a5eaef1c..30600e9454e1 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -748,7 +748,7 @@ 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)
> +static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec,
> int port_num, u16 pd_revision)
> {
> struct cros_typec_port *port = typec->ports[port_num];
> struct ec_response_typec_discovery *disc = port->disc_data;
> @@ -794,6 +794,7 @@ static int cros_typec_handle_sop_prime_disc(struct
> cros_typec_data *typec, int p
> }
>
> c_desc.identity = &port->c_identity;
> + c_desc.pd_revision = pd_revision;
>
> port->cable = typec_register_cable(port->port, &c_desc);
> if (IS_ERR(port->cable)) {
> @@ -893,7 +894,11 @@ static void cros_typec_handle_status(struct
> cros_typec_data *typec, int port_num
>
> 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);
> + u16 sop_prime_revision;
> +
> + /* Convert BCD to the format preferred by the TypeC framework
> */
> + sop_prime_revision = (le16_to_cpu(resp.sop_prime_revision) &
> 0xff00) >> 4;
> + ret = cros_typec_handle_sop_prime_disc(typec, port_num,
> sop_prime_revision);
> if (ret < 0)
> dev_err(typec->dev, "Couldn't parse SOP' Disc data,
> port: %d\n", port_num);
> else
> --
> 2.30.0.365.g02bc693789-goog
>
Re: [PATCH 5/6] platform/chrome: cros_ec_typec: Set Partner PD revision from status
On Thu, Jan 28, 2021 at 10:14 PM Benson Leung wrote:
>
> Status provides sop_revision. Process it, and set it using the new
> setter in the typec class.
>
> Signed-off-by: Benson Leung
Reviewed-by: Prashant Malani
> ---
> drivers/platform/chrome/cros_ec_typec.c | 14 --
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c
> b/drivers/platform/chrome/cros_ec_typec.c
> index 30600e9454e1..6bc6fafd54a4 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -824,7 +824,7 @@ static int cros_typec_handle_sop_prime_disc(struct
> cros_typec_data *typec, int p
> return ret;
> }
>
> -static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int
> port_num)
> +static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int
> port_num, u16 pd_revision)
> {
> struct cros_typec_port *port = typec->ports[port_num];
> struct ec_response_typec_discovery *sop_disc = port->disc_data;
> @@ -842,6 +842,12 @@ static int cros_typec_handle_sop_disc(struct
> cros_typec_data *typec, int port_nu
> goto disc_exit;
> }
>
> + ret = typec_partner_set_pd_revision(port->partner, pd_revision);
> + if (ret < 0) {
> + dev_err(typec->dev, "Failed to update partner PD revision,
> port: %d\n", port_num);
> + goto disc_exit;
> + }
> +
> memset(sop_disc, 0, EC_PROTO2_MAX_RESPONSE_SIZE);
> ret = cros_typec_ec_command(typec, 0, EC_CMD_TYPEC_DISCOVERY, &req,
> sizeof(req),
> sop_disc, EC_PROTO2_MAX_RESPONSE_SIZE);
> @@ -885,7 +891,11 @@ static void cros_typec_handle_status(struct
> cros_typec_data *typec, int port_num
>
> /* Handle any events appropriately. */
> 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);
> + u16 sop_revision;
> +
> + /* Convert BCD to the format preferred by the TypeC framework
> */
> + sop_revision = (le16_to_cpu(resp.sop_revision) & 0xff00) >> 4;
> + ret = cros_typec_handle_sop_disc(typec, port_num,
> sop_revision);
> if (ret < 0)
> dev_err(typec->dev, "Couldn't parse SOP Disc data,
> port: %d\n", port_num);
> else
> --
> 2.30.0.365.g02bc693789-goog
>
Re: [PATCH 6/6] platform/chrome: cros_ec_typec: Set opmode to PD on SOP connected
On Thu, Jan 28, 2021 at 10:14 PM Benson Leung wrote: > > When SOP Discovery is done, set the opmode to PD if status indicates > SOP is connected. > > SOP connected indicates a PD contract is in place, and is a solid > indication we have transitioned to PD power negotiation, either as > source or sink. > > Signed-off-by: Benson Leung Reviewed-by: Prashant Malani > --- > drivers/platform/chrome/cros_ec_typec.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/platform/chrome/cros_ec_typec.c > b/drivers/platform/chrome/cros_ec_typec.c > index 6bc6fafd54a4..a7778258d0a0 100644 > --- a/drivers/platform/chrome/cros_ec_typec.c > +++ b/drivers/platform/chrome/cros_ec_typec.c > @@ -900,6 +900,9 @@ static void cros_typec_handle_status(struct > cros_typec_data *typec, int port_num > dev_err(typec->dev, "Couldn't parse SOP Disc data, > port: %d\n", port_num); > else > typec->ports[port_num]->sop_disc_done = true; > + > + if (resp.sop_connected) > + typec_set_pwr_opmode(typec->ports[port_num]->port, > TYPEC_PWR_MODE_PD); > } > > if (resp.events & PD_STATUS_EVENT_SOP_PRIME_DISC_DONE && > -- > 2.30.0.365.g02bc693789-goog >
Re: [PATCH v2 1/2] platform/chrome: cros_ec: Import Type C control command
Thanks Benson,
On Wed, Feb 3, 2021 at 11:18 PM Benson Leung wrote:
>
> Hi Prashant,
>
> On Tue, 2 Feb 2021 18:15:37 -0800, Prashant Malani wrote:
> > This command is used to communicate with the Chrome Embedded Controller
> > (EC) regarding USB Type C events and state.
> >
> > These header updates are included in the latest Chrome OS EC headers [1]
> >
> > [1]
> > https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h
>
> Applied, thanks!
>
> [1/2] platform/chrome: cros_ec: Import Type C control command
> commit: 3ec28d3673362076444e290bdb0d94ec6432dc09
> [2/2] platform/chrome: cros_ec_typec: Clear Type C disc events
> commit: d521119c8b0d70b91971d632430ec1143e5d2e26
This (patch 2/2) now looks like it will lead to a conflict with the
following patch which went into Greg's usb tree:
cefc011f8daf ("platform/chrome: cros_ec_typec: Set Partner PD revision
from status")
If we want to submit stuff to for-next, perhaps the immutable branch
should be merged into for-next?
Re: [PATCH 2/2] platform/chrome: cros_ec_types: Support disconnect events without partners
Hi Raj, On Fri, Feb 5, 2021 at 11:52 AM Rajmohan Mani wrote: > > There are certain scenarios, where a disconnect event might > occur on a Type-C port with no port partners. This is required > to enable communication to Burnside Bridge USB4 retimers. > > Signed-off-by: Rajmohan Mani minor commit message nit (apologies for not spotting this earlier): This patch alone doesn't add support for the "without partners" part (that comes in the next patch). This one purely adds support for disconnect events. So might be good to update the commit message if possible. But otherwise LGTM, so: Reviewed-by: Prashant Malani
Re: [PATCH 2/2] platform/chrome: cros_ec_types: Support disconnect events without partners
On Fri, Feb 5, 2021 at 12:05 PM Prashant Malani wrote: > > Hi Raj, > > On Fri, Feb 5, 2021 at 11:52 AM Rajmohan Mani wrote: > > > > There are certain scenarios, where a disconnect event might > > occur on a Type-C port with no port partners. This is required > > to enable communication to Burnside Bridge USB4 retimers. > > > > Signed-off-by: Rajmohan Mani > minor commit message nit (apologies for not spotting this earlier): > > This patch alone doesn't add support for the "without partners" part > (that comes in the next patch). > This one purely adds support for disconnect events. So might be good > to update the commit message if possible. Sorry, I just noticed that I interpreted the ordering of the patches incorrectly, so this commit message is fine as is and I retract my nit. > But otherwise LGTM, so: > > Reviewed-by: Prashant Malani
Re: [PATCH 1/2] platform/chrome: cros_ec_typec: Skip port partner check in configure_mux()
Hi Raj, On Fri, Feb 5, 2021 at 11:52 AM Rajmohan Mani wrote: > > For certain needs like updating the USB4 retimer firmware when no > device are connected, the Type-C ports require mux configuration, > to be able to communicate with the retimer. So removed the above > check to allow for mux configuration of Type-C ports, to enable > retimer communication. > > Signed-off-by: Rajmohan Mani Reviewed-by: Prashant Malani > --- > drivers/platform/chrome/cros_ec_typec.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/platform/chrome/cros_ec_typec.c > b/drivers/platform/chrome/cros_ec_typec.c > index e724a5eaef1c..3d8ff3f8a514 100644 > --- a/drivers/platform/chrome/cros_ec_typec.c > +++ b/drivers/platform/chrome/cros_ec_typec.c > @@ -536,9 +536,6 @@ static int cros_typec_configure_mux(struct > cros_typec_data *typec, int port_num, > enum typec_orientation orientation; > int ret; > > - if (!port->partner) > - return 0; > - > if (mux_flags & USB_PD_MUX_POLARITY_INVERTED) > orientation = TYPEC_ORIENTATION_REVERSE; > else > -- > 2.30.0 >
[PATCH v3 1/2] dt-bindings: chrome: Add cros-ec-typec mux props
Add properties for mode, orientation and USB data role switches for
Type C connectors. When available, these will allow the Type C connector
class port driver to configure the various switches according to USB PD
information (like orientation, alt mode etc.) provided by the Chrome OS
EC controller.
Signed-off-by: Prashant Malani
Acked-by: Benson Leung
---
Changes in v3:
- Fixed Acked-by tag typo.
Changes in v2:
- Added more text to the switch descriptions, explaining their purpose,
and relation to the Type C connector class framework.
.../bindings/chrome/google,cros-ec-typec.yaml | 40 ++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
index 6d7396ab8bee..800c005a0e44 100644
--- a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
+++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
@@ -21,7 +21,34 @@ properties:
const: google,cros-ec-typec
connector:
-$ref: /schemas/connector/usb-connector.yaml#
+allOf:
+ - $ref: /schemas/connector/usb-connector.yaml#
+ - type: object
+properties:
+ mode-switch:
+description: Reference to a DT node for the USB Type C Multiplexer
+ for this connector. This switch controls the data lines routing
+ for this connector for various operation modes, including
+ Alternate Modes. This switch is assumed registered with the Type
C
+ connector class framework by its driver. The Type C connector
+ class framework assumes that the mode switch property uses this
+ name.
+
+ orientation-switch:
+description: Reference to a DT node for the USB Type C orientation
+ switch for this connector. This switch controls routing the
+ correct data pairs depending on the cable plug orientation from
+ this connector to the USB / Alternate Mode controllers. This
+ switch is assumed registered with the Type C connector class
+ framework by its driver. The Type C connector class framework
+ assumes that the orientation switch property uses this name.
+
+ usb-role-switch:
+description: Reference to a DT node for the USB Data role switch
+ for this connector. This switch is assumed registered with the
+ Type C connector class framework by its driver. The Type C
+ connector class framework assumes that the USB role switch
+ property uses this name.
required:
- compatible
@@ -49,6 +76,17 @@ examples:
data-role = "dual";
try-power-role = "source";
};
+
+ connector@1 {
+compatible = "usb-c-connector";
+reg = <1>;
+power-role = "dual";
+data-role = "host";
+try-power-role = "source";
+mode-switch = <&typec_mux>;
+orientation-switch = <&typec_orientation_switch>;
+usb-role-switch = <&typec_mux>;
+ };
};
};
};
--
2.26.2.761.g0e0b3e54be-goog
[PATCH v3 2/2] platform/chrome: typec: Register Type C switches
Register Type C mux and switch handles, when provided via firmware
bindings. These will allow the cros-ec-typec driver, and also alternate
mode drivers to configure connected Muxes correctly, according to PD
information retrieved from the Chrome OS EC.
Signed-off-by: Prashant Malani
Reviewed-by: Heikki Krogerus
Acked-by: Enric Balletbo i Serra
---
Changes in v3:
- Added Acked-by tag.
- Fixed Heikki's email address in Reviewed-by tag.
Changes in v2:
- Changed dev_info prints to dev_dbg.
drivers/platform/chrome/cros_ec_typec.c | 47 +
1 file changed, 47 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index 66b8d21092af..6e79f917314b 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -14,6 +14,8 @@
#include
#include
#include
+#include
+#include
#define DRV_NAME "cros-ec-typec"
@@ -25,6 +27,9 @@ struct cros_typec_port {
struct typec_partner *partner;
/* Port partner PD identity info. */
struct usb_pd_identity p_identity;
+ struct typec_switch *ori_sw;
+ struct typec_mux *mux;
+ struct usb_role_switch *role_sw;
};
/* Platform-specific data for the Chrome OS EC Type C controller. */
@@ -84,6 +89,40 @@ static int cros_typec_parse_port_props(struct
typec_capability *cap,
return 0;
}
+static int cros_typec_get_switch_handles(struct cros_typec_port *port,
+struct fwnode_handle *fwnode,
+struct device *dev)
+{
+ port->mux = fwnode_typec_mux_get(fwnode, NULL);
+ if (IS_ERR(port->mux)) {
+ dev_dbg(dev, "Mux handle not found.\n");
+ goto mux_err;
+ }
+
+ port->ori_sw = fwnode_typec_switch_get(fwnode);
+ if (IS_ERR(port->ori_sw)) {
+ dev_dbg(dev, "Orientation switch handle not found.\n");
+ goto ori_sw_err;
+ }
+
+ port->role_sw = fwnode_usb_role_switch_get(fwnode);
+ if (IS_ERR(port->role_sw)) {
+ dev_dbg(dev, "USB role switch handle not found.\n");
+ goto role_sw_err;
+ }
+
+ return 0;
+
+role_sw_err:
+ usb_role_switch_put(port->role_sw);
+ori_sw_err:
+ typec_switch_put(port->ori_sw);
+mux_err:
+ typec_mux_put(port->mux);
+
+ return -ENODEV;
+}
+
static void cros_unregister_ports(struct cros_typec_data *typec)
{
int i;
@@ -91,6 +130,9 @@ static void cros_unregister_ports(struct cros_typec_data
*typec)
for (i = 0; i < typec->num_ports; i++) {
if (!typec->ports[i])
continue;
+ usb_role_switch_put(typec->ports[i]->role_sw);
+ typec_switch_put(typec->ports[i]->ori_sw);
+ typec_mux_put(typec->ports[i]->mux);
typec_unregister_port(typec->ports[i]->port);
}
}
@@ -153,6 +195,11 @@ static int cros_typec_init_ports(struct cros_typec_data
*typec)
ret = PTR_ERR(cros_port->port);
goto unregister_ports;
}
+
+ ret = cros_typec_get_switch_handles(cros_port, fwnode, dev);
+ if (ret)
+ dev_dbg(dev, "No switch control for port %d\n",
+ port_num);
}
return 0;
--
2.26.2.761.g0e0b3e54be-goog
Re: [PATCH v2 1/2] dt-bindings: chrome: Add cros-ec-typec mux props
Hi Rob,
On Mon, May 18, 2020 at 12:17:04AM -0700, Prashant Malani wrote:
> Add properties for mode, orientation and USB data role switches for
> Type C connectors. When available, these will allow the Type C connector
> class port driver to configure the various switches according to USB PD
> information (like orientation, alt mode etc.) provided by the Chrome OS
> EC controller.
>
> Signed-off-by: Prashant Malani
> Acked-By: Benson Leung
Sorry, looks like I made a typo while adding the "Acked-by" from a
previous version.
I have uploaded v3 with this fixed.
Thanks,
> ---
>
> Changes in v2:
> - Added more text to the switch descriptions, explaining their purpose,
> and relation to the Type C connector class framework.
>
> .../bindings/chrome/google,cros-ec-typec.yaml | 40 ++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git
> a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> index 6d7396ab8bee..800c005a0e44 100644
> --- a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> +++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> @@ -21,7 +21,34 @@ properties:
> const: google,cros-ec-typec
>
>connector:
> -$ref: /schemas/connector/usb-connector.yaml#
> +allOf:
> + - $ref: /schemas/connector/usb-connector.yaml#
> + - type: object
> +properties:
> + mode-switch:
> +description: Reference to a DT node for the USB Type C
> Multiplexer
> + for this connector. This switch controls the data lines routing
> + for this connector for various operation modes, including
> + Alternate Modes. This switch is assumed registered with the
> Type C
> + connector class framework by its driver. The Type C connector
> + class framework assumes that the mode switch property uses this
> + name.
> +
> + orientation-switch:
> +description: Reference to a DT node for the USB Type C
> orientation
> + switch for this connector. This switch controls routing the
> + correct data pairs depending on the cable plug orientation from
> + this connector to the USB / Alternate Mode controllers. This
> + switch is assumed registered with the Type C connector class
> + framework by its driver. The Type C connector class framework
> + assumes that the orientation switch property uses this name.
> +
> + usb-role-switch:
> +description: Reference to a DT node for the USB Data role switch
> + for this connector. This switch is assumed registered with the
> + Type C connector class framework by its driver. The Type C
> + connector class framework assumes that the USB role switch
> + property uses this name.
>
> required:
>- compatible
> @@ -49,6 +76,17 @@ examples:
> data-role = "dual";
> try-power-role = "source";
>};
> +
> + connector@1 {
> +compatible = "usb-c-connector";
> +reg = <1>;
> +power-role = "dual";
> +data-role = "host";
> +try-power-role = "source";
> +mode-switch = <&typec_mux>;
> +orientation-switch = <&typec_orientation_switch>;
> +usb-role-switch = <&typec_mux>;
> + };
> };
>};
> };
> --
> 2.26.2.761.g0e0b3e54be-goog
>
Re: [PATCH v2 1/2] platform/chrome: cros_ec_typec: Send enum values to usb_role_switch_set_role()
Hi Azhar,
On Thu, Jul 30, 2020 at 03:56:08PM -0700, Azhar Shaikh wrote:
> usb_role_switch_set_role() has the second argument as enum for usb_role.
> Currently depending upon the data role i.e. UFP(0) or DFP(1) is sent.
> This eventually translates to USB_ROLE_NONE in case of UFP and
> USB_ROLE_DEVICE in case of DFP. Correct this by sending correct enum
> values as USB_ROLE_DEVICE in case of UFP and USB_ROLE_HOST in case of
> DFP.
>
> Fixes: 7e7def15fa4b ("platform/chrome: cros_ec_typec: Add USB mux control")
>
> Signed-off-by: Azhar Shaikh
> Cc: Prashant Malani
> Reviewed-by: Prashant Malani
> ---
Please add the list of changes in each version after the "---" line.
> drivers/platform/chrome/cros_ec_typec.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c
> b/drivers/platform/chrome/cros_ec_typec.c
> index 3eae01f4c9f7..eb4713b7ae14 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -590,7 +590,8 @@ static int cros_typec_port_update(struct cros_typec_data
> *typec, int port_num)
> dev_warn(typec->dev, "Configure muxes failed, err = %d\n", ret);
>
> return usb_role_switch_set_role(typec->ports[port_num]->role_sw,
> - !!(resp.role & PD_CTRL_RESP_ROLE_DATA));
> +resp.role & PD_CTRL_RESP_ROLE_DATA
> +? USB_ROLE_HOST : USB_ROLE_DEVICE);
> }
>
> static int cros_typec_get_cmd_version(struct cros_typec_data *typec)
> --
> 2.17.1
>
Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Avoid setting usb role during disconnect
Hi Azhar, On Thu, Jul 30, 2020 at 03:56:09PM -0700, Azhar Shaikh wrote: > On disconnect port partner is removed and usb role is set to NONE. > But then in cros_typec_port_update() the role is set again. > Avoid this by moving usb_role_switch_set_role() to > cros_typec_configure_mux(). > > Signed-off-by: Azhar Shaikh > Suggested-by: Prashant Malani > --- > drivers/platform/chrome/cros_ec_typec.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/platform/chrome/cros_ec_typec.c > b/drivers/platform/chrome/cros_ec_typec.c > index eb4713b7ae14..df97431b2275 100644 > --- a/drivers/platform/chrome/cros_ec_typec.c > +++ b/drivers/platform/chrome/cros_ec_typec.c > @@ -515,6 +515,12 @@ static int cros_typec_configure_mux(struct > cros_typec_data *typec, int port_num, > if (ret) > return ret; > > + ret = usb_role_switch_set_role(typec->ports[port_num]->role_sw, > +pd_ctrl->role & PD_CTRL_RESP_ROLE_DATA > +? USB_ROLE_HOST : USB_ROLE_DEVICE); > + if (ret) > + return ret; Since this was the last switch being configured, please maintain the same order and add this at the end of the function, after the if-else if block. Best regards, -Prashant
Re: [PATCH v2 1/2] platform/chrome: cros_ec_typec: Send enum values to usb_role_switch_set_role()
On Thu, Jul 30, 2020 at 11:02:28PM +, Shaikh, Azhar wrote: > Hi Prashant, > > > > > Please add the list of changes in each version after the "---" line. > > I have added those in the cover letter. > > It is good practice to add these to the individual change too, so that the reader doesn't have to go back to the cover letter to determine what has changed in each patch.
Re: [PATCH v2 1/2] platform/chrome: cros_ec_typec: Send enum values to usb_role_switch_set_role()
On Thu, Jul 30, 2020 at 11:14:39PM +, Shaikh, Azhar wrote: > > > > -Original Message- > > From: Prashant Malani > > Sent: Thursday, July 30, 2020 4:05 PM > > To: Shaikh, Azhar > > Cc: ble...@chromium.org; enric.balle...@collabora.com; > > gro...@chromium.org; linux-kernel@vger.kernel.org; > > heikki.kroge...@linux.intel.com; Patel, Utkarsh H > > ; Bowman, Casey G > > ; Mani, Rajmohan > > > > Subject: Re: [PATCH v2 1/2] platform/chrome: cros_ec_typec: Send enum > > values to usb_role_switch_set_role() > > > > > > On Thu, Jul 30, 2020 at 11:02:28PM +, Shaikh, Azhar wrote: > > > Hi Prashant, > > > > > > > > > > > Please add the list of changes in each version after the "---" line. > > > > > > I have added those in the cover letter. > > > > > > It is good practice to add these to the individual change too, so that the > > reader doesn't have to go back to the cover letter to determine what has > > changed in each patch. > > So are you suggesting to move it from cover letter to individual patches? > But then isn't the same thing what cover letter is for? No, I'm suggesting you keep it both places. Each patch should have a change log documenting what has changed in the various versions.
Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Avoid setting usb role during disconnect
Hey Azhar, On Thu, Jul 30, 2020 at 11:06:12PM +, Shaikh, Azhar wrote: > Hi Prashant, > > > > Since this was the last switch being configured, please maintain the same > > order and add this at the end of the function, after the if-else if block. > > > > Please correct if my understanding is not correct here: > Set the orientation , set the role, then configure the mux. Shouldn't this be > the order? Is this documented anywhere? Kindly provide the links to that if so. I wasn't aware of any ordering requirements (but I may be missing something). Please keep in mind that each of these switches (orientation, data-role, mode-switch, or what is referred to here as "mux") can theoretically be different switches, controlled independently by distinct drivers and hardware. We should not change what ordering is already present unless there is a requirement to do so. The existing ordering was orientation switch, "mux" or role switch, then the data-role switch, so let us stick to that. Best regards, > > > > Best regards, > > > > -Prashant
Re: [PATCH v4 3/3] platform/chrome: cros_ec_typec: Re-order connector configuration steps
On Thu, Aug 20, 2020 at 04:38:32PM -0700, Azhar Shaikh wrote:
> As per USB Type-C Spec R2.0 section 4.5.1.2 (Connecting Sources and Sinks)
> and section 4.5.2.2 (Connection State Machine Requirements), the typical
> flow for configuring a device connected to a typeC port is as below:
>
> 1. Source/sink detection
> 2. Orientation
> 3. Data role
> 4. VCONN
> 5. VBUS (USB Type-C currents)
> 6. The connector is now configured. We can start the PD communication
>that should lead into configuration of the mux if we enter a mode.
>
> But in existing code data role was set after the connector and mux are
> already configured. So fix this by following the spec to set the data
> role before the connector and mux are configured.
>
> Signed-off-by: Azhar Shaikh
To be clear, I still maintain that the quoted section pertains to
*detection* of those various properties (which is handled by the TCPM in
the Chrome OS EC), and not any ordering for setting switches etc.
That said, I'm not opposed to the re-ordering, so:
Reviewed-by: Prashant Malani
> ---
> Changes in v4:
> - No change
>
> Changes in v3:
> - New patch added
>
> drivers/platform/chrome/cros_ec_typec.c | 13 +++--
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c
> b/drivers/platform/chrome/cros_ec_typec.c
> index 2b43e1176e73..9e4fad9ca59e 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -516,6 +516,12 @@ static int cros_typec_configure_mux(struct
> cros_typec_data *typec, int port_num,
> if (ret)
> return ret;
>
> + ret = usb_role_switch_set_role(typec->ports[port_num]->role_sw,
> +pd_ctrl->role & PD_CTRL_RESP_ROLE_DATA
> +? USB_ROLE_HOST : USB_ROLE_DEVICE);
> + if (ret)
> + return ret;
> +
> if (mux_flags & USB_PD_MUX_TBT_COMPAT_ENABLED) {
> ret = cros_typec_enable_tbt(typec, port_num, pd_ctrl);
> } else if (mux_flags & USB_PD_MUX_DP_ENABLED) {
> @@ -533,12 +539,7 @@ static int cros_typec_configure_mux(struct
> cros_typec_data *typec, int port_num,
> ret = -ENOTSUPP;
> }
>
> - if (ret)
> - return ret;
> -
> - return usb_role_switch_set_role(typec->ports[port_num]->role_sw,
> -pd_ctrl->role & PD_CTRL_RESP_ROLE_DATA
> -? USB_ROLE_HOST : USB_ROLE_DEVICE);
> + return ret;
> }
>
> static int cros_typec_port_update(struct cros_typec_data *typec, int
> port_num)
> --
> 2.17.1
>
Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Avoid setting usb role during disconnect
Hi Azhar, On Wed, Aug 5, 2020 at 12:22 PM Shaikh, Azhar wrote: > > Hi Prashant, > > > Is this documented anywhere? Kindly provide the links to that if so. I > > wasn't > > aware of any ordering requirements (but I may be missing something). > > The configuration of the connector should always happen in the order defined > in the > USB Type-C specification. Check ch. 2.3 (USB Type-C Spec R2.0). So that will > basically give you: > > 1. orientation > 2. role(s) > 3. the rest. Thanks for the link. Are you referring to Section 2.3 (Configuration Process) ? I couldn't find anything there which implied any required ordering (I'm reading Release 2.0, Aug 2019, so I don't know if something has been added since). Could you kindly point me to the appropriate subsection? Additionally, I think any ordering requirements there are already handled by the TCPM in the Chrome OS EC. > > Also I see in USB Type-C Port Manager driver the above mentioned sequence > being followed > https://elixir.bootlin.com/linux/latest/source/drivers/usb/typec/tcpm/tcpm.c#L664 In addition to the above, note that this is a TCPM implementation (on Chrome OS the TCPM implementation is in the EC), so what is done there doesn't necessarily apply to cros-ec-typec. Best regards, -Prashant > > > > > > -Prashant > > Regards, > Azhar
Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Avoid setting usb role during disconnect
Hi, On Wed, Aug 05, 2020 at 08:28:22PM +, Shaikh, Azhar wrote: > Hi Prashant, > > > > > > > Hi Prashant, > > > > > > > Is this documented anywhere? Kindly provide the links to that if so. > > > > I wasn't aware of any ordering requirements (but I may be missing > > something). > > > > > > The configuration of the connector should always happen in the order > > > defined in the USB Type-C specification. Check ch. 2.3 (USB Type-C Spec > > R2.0). So that will basically give you: > > > > > > 1. orientation > > > 2. role(s) > > > 3. the rest. > > > > Thanks for the link. Are you referring to Section 2.3 (Configuration > > Process) ? I couldn't find anything there which implied any required > > ordering > > (I'm reading Release 2.0, Aug 2019, so I don't know if something has been > > added since). > > Could you kindly point me to the appropriate subsection? > > That is the section I was referring to. (Followed up with Azhar in a side-thread) I don't see anything in that section that enforces an ordering on how these switches should be configured. That said, I may be misinterpreting it so I'm happy to follow up and withdraw my reservations to the change given valid reasoning :) As things stand, it looks like this is being reordered because it is necessary for the particular switch driver (and architecture) in question, i.e intel_pmc_mux.c to fix an edge use case there (correcting the sequencing of PMC IPC messages when handling DP in warm reboots). I'd prefer the ordering be kept the way it was because: 1. We should preserve the existing ordering, and only fix the bug described in the commit message, i.e avoid setting usb_role switch to anything other than "NONE" during disconnect. 2. The CL should do 1 thing or call out why it's doing the re-ordering. Here it is not only fixing the double call to usb_role_switch_set_role (which is addressed in the commit message), but also re-ordering the call (which is not addressed at all). 1.) and 2.) are sort-of related. 3. We should avoid fixing platform specific issues by changing the top level cros-ec-typec driver. Fixing this in the mux driver will make the driver more robust against any other sequencing issues that may crop up later. Also, if for example some other mux driver (driverB) requires a different ordering (e.g mode-switch before role-switch), changing that in cros-ec-typec will end up breaking the mux agent driver. driverB should handle the ordering issues internally, and so should intel_pmc_mux.c BR, -Prashant > > > > > > > > > Regards, > > > Azhar
Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Avoid setting usb role during disconnect
Hi Heikki, On Thu, Aug 6, 2020 at 4:39 AM Heikki Krogerus wrote: > > On Wed, Aug 05, 2020 at 12:37:14PM -0700, Prashant Malani wrote: > > Hi Azhar, > > > > > > On Wed, Aug 5, 2020 at 12:22 PM Shaikh, Azhar > > wrote: > > > > > > Hi Prashant, > > > > > > > Is this documented anywhere? Kindly provide the links to that if so. I > > > > wasn't > > > > aware of any ordering requirements (but I may be missing something). > > > > > > The configuration of the connector should always happen in the order > > > defined in the > > > USB Type-C specification. Check ch. 2.3 (USB Type-C Spec R2.0). So that > > > will basically give you: > > > > > > 1. orientation > > > 2. role(s) > > > 3. the rest. > > > > Thanks for the link. Are you referring to Section 2.3 (Configuration > > Process) ? I couldn't find anything there which > > implied any required ordering (I'm reading Release 2.0, Aug 2019, so I > > don't know if something has been added since). > > Could you kindly point me to the appropriate subsection? > > Please check the section 4.5.1.2 (Connecting Sources and Sinks). Check > the typical flow. You can also check the Connection State Machine > Requirements. The order should be clear from those as well. Thanks for sending the section info. > > 1. Source/sink detection > 2. Orientation > 3. Data role > 4. VCONN > 5. VBUS (USB Type-C currents) > 6. The connector is now configured. We can start the PD communication >that should lead into configuration of the mux if we enter a mode. The cros-ec-typec driver only receives a USB_PD_MUX_INFO [1] host command after we've already entered the mode as far as PD communication is concerned (steps 1-5 and even PD communication to enter the mode is already done by the time cros-ec-typec receives PD_MUX_INFO). There is no further PD communication to be done in this case (for a particular mode), at least nothing that is triggered by the AP. > > The data role, the thing that we are talking about here, really should > be set before the mux is configured. I apologize but I still didn't see anything there enforcing an ordering for those on any AP switches. The state machine you're referring to ((I assume you are referring to Figure 4-12 to Figure Figure 4-18) is already implemented in the TCPM in the Chrome EC for the Chrome OS Platform [2] Perhaps we can take that discussion off-mailing list if necessary ? (I'd like to avoid blasting the large mailing list with more discussion email, but also happy to continue here if that's the preference). To be clear, all these comments are limited to the only Chrome OS platform. > > > Additionally, I think any ordering requirements there are already > > handled by the TCPM in the Chrome OS EC. > > The TCPM does not execute the steps that configure the port on this > platform. The OS is the part that actually executes the steps. My response was w.r.t section 2.3 (the section which was originally quoted) which deals with things like: " - Source-to-Sink attach/detach detection - Plug orientation/cable twist detection - Detect if cable requires Vconn " etc. All these things are performed by the Chrome OS EC (via TCPM or the TCPC). For the items listed in Section 4.5.1.2, AFAIK those steps are performed by the Chrome OS EC (via a combination of the TCPM and TCPC). [1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/tree/include/linux/platform_data/cros_ec_commands.h?h=for-kernelci#n5214 [2]: https://source.corp.google.com/chromeos_public/src/platform/ec/common/usbc/ > > That is one reason (but not the only one) why it is important that > both parts follow the order that is proposed in the spec. Otherwise we > may endup negotiating things with the partner in one order but then > actually executing those steps in some other. I agree with this, but since the role of TCPM is performed by the Chrome EC, I'm not convinced this patch is addressing any spec related ordering requirements. As I mentioned above, the Chrome OS EC is following the state machine as well as the section of the spec you referred to. I would suggest: - Merging Patch 1 (role set correction) and Patch 2 (moving the usb_role_switch_set_role() inside cros_typec_configure_mux() *but* keep it at the end to preserve existing ordering) into 1 patch. - Add another patch which re-orders the calls and which in the commit message lists out all the reasons why this re-ordering needs to be done. Doing the above will help keep better track of why the changes were made. BR, -Prashant > > > thanks, > > -- > heikki
[PATCH v2] usb: typec: Add class for plug alt mode device
Add the Type C class for plug alternate mode devices which are being
registered by the Type C connector class. This ensures that udev events
get generated when the plug alt modes are registered.
Signed-off-by: Prashant Malani
Cc: Heikki Krogerus
---
Changes in v2:
- Changed code to set the class member instead of bus.
- Removed the alteration to typec_bus.rst since it's not longer
required.
- Updated the commit message and subject to reflect the change in code.
v1:
https://lore.kernel.org/linux-usb/20201203030846.51669-1-pmal...@chromium.org/
drivers/usb/typec/class.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 35eec707cb51..29d05b45cc9d 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -482,6 +482,10 @@ typec_register_altmode(struct device *parent,
if (is_typec_partner(parent))
alt->adev.dev.bus = &typec_bus;
+ /* Plug alt modes need a class to generate udev events. */
+ if (is_typec_plug(parent))
+ alt->adev.dev.class = typec_class;
+
ret = device_register(&alt->adev.dev);
if (ret) {
dev_err(parent, "failed to register alternate mode (%d)\n",
--
2.29.2.576.ga3fc446d84-goog
Re: [PATCH 1/2] platform/chrome: cros_ec_typec: Parameterize cros_typec_cmds_supported()
Hi Utkarsh, On Wed, Dec 09, 2020 at 10:09:02PM -0800, Utkarsh Patel wrote: > cros_typec_cmds_supported() is currently being used to check only one > feature flag. > Add a new feature parameter to it so that it can be used to check > multiple feature flags supported in cros_ec. > Rename cros_typec_cmds_supported() to cros_typec_feature_supported(). > > Signed-off-by: Utkarsh Patel Reviewed-by: Prashant Malani Thanks, -Prashant
Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Send mux configuration acknowledgment to EC
Hi Utkarsh, On Wed, Dec 09, 2020 at 10:09:03PM -0800, Utkarsh Patel wrote: > In some corner cases downgrade of the superspeed typec device(e.g. Dell > typec Dock, apple dongle) was seen because before the SOC mux configuration > finishes, EC starts configuring the next mux state. > > With this change, once the SOC mux is configured, kernel will send an > acknowledgment to EC via Host command EC_CMD_USB_PD_MUX_ACK [1]. > After sending the host event EC will wait for the acknowledgment from > kernel before starting the PD negotiation for the next mux state. This > helps to have a framework to build better error handling along with the > synchronization of timing sensitive mux states. > > This change also brings in corresponding EC header updates from the EC code > base [1]. > > [1]: > https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/include/ec_commands.h > > Signed-off-by: Utkarsh Patel I'm not sure what the maintainers' preference is for the header (same patch or separate patch). FWIW: Reviewed-by: Prashant Malani Thanks, -Prashant
Re: [PATCH] usb: typec: Add bus type for plug alt modes
Hi Heikki,
Thanks a lot for looking at the patch.
On Tue, Dec 8, 2020 at 1:37 AM Heikki Krogerus
wrote:
>
> On Wed, Dec 02, 2020 at 07:08:47PM -0800, Prashant Malani wrote:
> > Add the Type C bus for plug alternate modes which are being
> > registered via the Type C connector class. This ensures that udev events
> > get generated when plug alternate modes are registered (and not just for
> > partner/port alternate modes), even though the Type C bus doesn't link
> > plug alternate mode devices to alternate mode drivers.
>
> I still don't understand how is the uevent related to the bus? If you
> check the device_add() function, on line 2917, kobject_uevent() is
> called unconditionally. The device does not need a bus for that event
> to be generated.
My initial thought process was to see what is the difference in the adev device
initialization between partner altmode and plug altmode (the only difference I
saw in
typec_register_altmode() was regarding the bus field).
Yes, kobject_uevent() is called unconditionally, but it's return value isn't
checked,
so we don't know if it succeeded or not.
In the case of cable plug altmode, I see it fail with the following error[1]:
[ 114.431409] kobject: 'port1-plug0.0' (4ad42956): kobject_uevent_env:
filter function caused the event to drop!
I think the filter function which is called is this one: drivers/base/core.c:
dev_uevent_filter() [2]
static int dev_uevent_filter(struct kset *kset, struct kobject *kobj)
{
struct kobj_type *ktype = get_ktype(kobj);
if (ktype == &device_ktype) {
struct device *dev = kobj_to_dev(kobj);
if (dev->bus)
return 1;
if (dev->class)
return 1;
}
return 0;
}
So, both the "if (dev->bus)" and "if (dev->class)" checks are failing here. In
the case of partner alt modes, bus is set by the class.c code
so this check likely returns 1 in that case.
In case the provided fix is not right or acceptable, an alternative I can think
of is:
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index c13779ea3200..ecb4c7546aae 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -517,6 +517,9 @@ typec_register_altmode(struct device *parent,
if (is_typec_partner(parent))
alt->adev.dev.bus = &typec_bus;
+ if (is_typec_plug(parent))
+ alt->adev.dev.class = typec_class;
+
ret = device_register(&alt->adev.dev);
if (ret) {
dev_err(parent, "failed to register alternate mode (%d)\n",
This too ensures that the filter function returns a 1.
Kindly LMK which way (if any) would you prefer.
>
> Also, I don't understand how are the cable plug alt modes now
> prevented from being bind to the alt mode drivers?
Sorry about this; I am unable to test this out. I just based the observation on
the line in Documentation/driver-api/usb/typec_bus.rst
(Cable Plug Alternate Modes) : "The alternate mode drivers are not bound to
cable plug alternate mode devices,
only to the partner alternate mode devices" . I don't completely understand the
bus.c code yet, so assumed that the code
there checked for the partner type during bind attempts.
Of course, based on what the eventual solution is, this statement may no longer
be required and I can remove it from the commit message
I can amend the Documentation to specify that cable plug alt modes can bind to
alt mode drivers.
Thanks,
-Prashant
[1] https://elixir.bootlin.com/linux/v5.10-rc7/source/lib/kobject_uevent.c#L516
[2] https://elixir.bootlin.com/linux/v5.10-rc7/source/drivers/base/core.c#L1840
[PATCH] platform/chrome: cros_ec_typec: Check for device within remove function
In a couple of call sites, we use the same pattern of checking for a
partner or cable device before attempting to remove it. Simplify this by
moving those checks into the remove functions.
Cc: Benson Leung
Signed-off-by: Prashant Malani
---
drivers/platform/chrome/cros_ec_typec.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index 0811562deecc..8e7fde3e7032 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -220,6 +220,9 @@ static void cros_typec_remove_partner(struct
cros_typec_data *typec,
{
struct cros_typec_port *port = typec->ports[port_num];
+ if (!port->partner)
+ return;
+
cros_typec_unregister_altmodes(typec, port_num, true);
cros_typec_usb_disconnect_state(port);
@@ -235,6 +238,9 @@ static void cros_typec_remove_cable(struct cros_typec_data
*typec,
{
struct cros_typec_port *port = typec->ports[port_num];
+ if (!port->cable)
+ return;
+
cros_typec_unregister_altmodes(typec, port_num, false);
typec_unregister_plug(port->plug);
@@ -253,11 +259,8 @@ static void cros_unregister_ports(struct cros_typec_data
*typec)
if (!typec->ports[i])
continue;
- if (typec->ports[i]->partner)
- cros_typec_remove_partner(typec, i);
-
- if (typec->ports[i]->cable)
- cros_typec_remove_cable(typec, i);
+ cros_typec_remove_partner(typec, i);
+ cros_typec_remove_cable(typec, i);
usb_role_switch_put(typec->ports[i]->role_sw);
typec_switch_put(typec->ports[i]->ori_sw);
@@ -647,11 +650,8 @@ static void cros_typec_set_port_params_v1(struct
cros_typec_data *typec,
"Failed to register partner on port: %d\n",
port_num);
} else {
- if (typec->ports[port_num]->partner)
- cros_typec_remove_partner(typec, port_num);
-
- if (typec->ports[port_num]->cable)
- cros_typec_remove_cable(typec, port_num);
+ cros_typec_remove_partner(typec, port_num);
+ cros_typec_remove_cable(typec, port_num);
}
}
--
2.31.0.rc2.261.g7f71774620-goog
Re: [PATCH] platform/chrome: cros_ec_typec: Handle hard reset
Hi Enric, Thanks for taking a look. On Thu, Apr 15, 2021 at 10:39 PM Enric Balletbo Serra wrote: > > Hi Prashant, > > Thank you for your patch. > > Missatge de Prashant Malani del dia dj., 15 > d’abr. 2021 a les 4:15: > > > > The Chrome Embedded Controller (EC) generates a hard reset type C event > > when a USB Power Delivery (PD) hard reset is encountered. Handle this > > event by unregistering the partner and cable on the associated port and > > clearing the event flag. > > > > Also update the EC command header to include the new event bit. This bit > > is included in the latest version of the Chrome EC headers[1]. > > > > [1] > > https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h > > > > Cc: Benson Leung > > Signed-off-by: Prashant Malani > > --- > > drivers/platform/chrome/cros_ec_typec.c| 13 + > > include/linux/platform_data/cros_ec_commands.h | 1 + > > Could this be a separate patch? Sure. I'll send a v2 with them split up.
[PATCH v2 1/2] platform/chrome: cros_ec: Add Type C hard reset
Update the EC command header to include the new event bit. This bit
is included in the latest version of the Chrome EC headers[1].
[1]
https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h
Change-Id: I52a36e725d945665814d4e59ddd1e76a3692db9f
---
v2 is the first version the patch was introduced.
include/linux/platform_data/cros_ec_commands.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/platform_data/cros_ec_commands.h
b/include/linux/platform_data/cros_ec_commands.h
index 5ff8597ceabd..9156078c6fc6 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -5678,6 +5678,7 @@ enum tcpc_cc_polarity {
#define PD_STATUS_EVENT_SOP_DISC_DONE BIT(0)
#define PD_STATUS_EVENT_SOP_PRIME_DISC_DONEBIT(1)
+#define PD_STATUS_EVENT_HARD_RESET BIT(2)
struct ec_params_typec_status {
uint8_t port;
--
2.31.1.368.gbe11c130af-goog
[PATCH v2 2/2] platform/chrome: cros_ec_typec: Handle hard reset
The Chrome Embedded Controller (EC) generates a hard reset type C event
when a USB Power Delivery (PD) hard reset is encountered. Handle this
event by unregistering the partner and cable on the associated port and
clearing the event flag.
Cc: Benson Leung
Signed-off-by: Prashant Malani
---
Changes in v2:
- Split the header change into a separate patch.
- Updated the commit message to reflect the above.
v1:
https://lore.kernel.org/lkml/20210415021456.1805797-1-pmal...@chromium.org/
drivers/platform/chrome/cros_ec_typec.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index d3df1717a5fd..22052f569f2a 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -905,6 +905,19 @@ static void cros_typec_handle_status(struct
cros_typec_data *typec, int port_num
return;
}
+ /* If we got a hard reset, unregister everything and return. */
+ if (resp.events & PD_STATUS_EVENT_HARD_RESET) {
+ cros_typec_remove_partner(typec, port_num);
+ cros_typec_remove_cable(typec, port_num);
+
+ ret = cros_typec_send_clear_event(typec, port_num,
+ PD_STATUS_EVENT_HARD_RESET);
+ if (ret < 0)
+ dev_warn(typec->dev,
+"Failed hard reset event clear, port: %d\n",
port_num);
+ return;
+ }
+
/* Handle any events appropriately. */
if (resp.events & PD_STATUS_EVENT_SOP_DISC_DONE &&
!typec->ports[port_num]->sop_disc_done) {
u16 sop_revision;
--
2.31.1.368.gbe11c130af-goog
[PATCH] platform/chrome: cros_ec_typec: Handle hard reset
The Chrome Embedded Controller (EC) generates a hard reset type C event
when a USB Power Delivery (PD) hard reset is encountered. Handle this
event by unregistering the partner and cable on the associated port and
clearing the event flag.
Also update the EC command header to include the new event bit. This bit
is included in the latest version of the Chrome EC headers[1].
[1]
https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h
Cc: Benson Leung
Signed-off-by: Prashant Malani
---
drivers/platform/chrome/cros_ec_typec.c| 13 +
include/linux/platform_data/cros_ec_commands.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index d3df1717a5fd..22052f569f2a 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -905,6 +905,19 @@ static void cros_typec_handle_status(struct
cros_typec_data *typec, int port_num
return;
}
+ /* If we got a hard reset, unregister everything and return. */
+ if (resp.events & PD_STATUS_EVENT_HARD_RESET) {
+ cros_typec_remove_partner(typec, port_num);
+ cros_typec_remove_cable(typec, port_num);
+
+ ret = cros_typec_send_clear_event(typec, port_num,
+ PD_STATUS_EVENT_HARD_RESET);
+ if (ret < 0)
+ dev_warn(typec->dev,
+"Failed hard reset event clear, port: %d\n",
port_num);
+ return;
+ }
+
/* Handle any events appropriately. */
if (resp.events & PD_STATUS_EVENT_SOP_DISC_DONE &&
!typec->ports[port_num]->sop_disc_done) {
u16 sop_revision;
diff --git a/include/linux/platform_data/cros_ec_commands.h
b/include/linux/platform_data/cros_ec_commands.h
index 5ff8597ceabd..9156078c6fc6 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -5678,6 +5678,7 @@ enum tcpc_cc_polarity {
#define PD_STATUS_EVENT_SOP_DISC_DONE BIT(0)
#define PD_STATUS_EVENT_SOP_PRIME_DISC_DONEBIT(1)
+#define PD_STATUS_EVENT_HARD_RESET BIT(2)
struct ec_params_typec_status {
uint8_t port;
--
2.31.1.295.g9ea45b61b8-goog
Re: [PATCH v3 03/11] usb: typec: Add plug num_altmodes sysfs attr
Hi Heikki, On Tue, Nov 17, 2020 at 02:41:43PM +0200, Heikki Krogerus wrote: > On Mon, Nov 16, 2020 at 12:11:42PM -0800, Prashant Malani wrote: > > 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. > > Why couldn't we just keep it hidden for as long as the number of > alt modes is 0? If you already explained that, then I apologise, I've > forgotten. > No worries :) Succinctly, because 0 is a valid value for "number of altmodes supported". If we keep the attribute hidden for 0, then there won't be a way for userspace to determine that PD discovery is done and we don't expect any more cable plug altmodes to be registered by the kernel Type C port driver (it can determine this by comparing "number_of_altmodes" against the actual number of alt modes registered by the Type C port driver). If we keep "number_of_altmodes" hidden even for 0, the userspace cannot differentiate between "this cable doesn't support any altmodes" and "it does altmodes, but the PD stack hasn't completed PD Discovery including DiscoverIdentity yet". For reference, here is the initial patch and mini-discussion around it back in July for port-partner altmodes [1] (I've followed a similar logic here). Hope this helps the rationale a bit more. Best regards, -Prashant [1]: https://lore.kernel.org/linux-usb/20200701082230.gf856...@kuha.fi.intel.com/
Re: [PATCH v2 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode
Hi Utkarsh, On Fri, Nov 13, 2020 at 12:25:01PM -0800, Utkarsh Patel wrote: > Configure Thunderbolt3/USB4 cable generation value by filing Thunderbolt 3 > cable discover mode VDO to support rounded and non-rounded Thunderbolt3/ > USB4 cables. > While we are here use Thunderbolt 3 cable discover mode VDO to fill active > cable plug link training value. > > Suggested-by: Heikki Krogerus > Signed-off-by: Utkarsh Patel > > -- > Changes in v2: > - No change. > -- > --- > drivers/platform/chrome/cros_ec_typec.c | 14 -- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/platform/chrome/cros_ec_typec.c > b/drivers/platform/chrome/cros_ec_typec.c > index 8111ed1fc574..b7416e82c3b3 100644 > --- a/drivers/platform/chrome/cros_ec_typec.c > +++ b/drivers/platform/chrome/cros_ec_typec.c > @@ -514,8 +514,18 @@ static int cros_typec_enable_usb4(struct cros_typec_data > *typec, > else if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_CABLE) > data.eudo |= EUDO_CABLE_TYPE_RE_TIMER << EUDO_CABLE_TYPE_SHIFT; > > - data.active_link_training = !!(pd_ctrl->control_flags & > -USB_PD_CTRL_ACTIVE_LINK_UNIDIR); > + /* > + * This driver does not have access to the identity information or > + * capabilities of the cable, so we don't know is it a real USB4 or > + * TBT3 cable. Therefore pretending that it's always TBT3 cable by > + * filling the TBT3 Cable VDO. > + */ > + data.tbt_cable_vdo = TBT_MODE; Is it safe to be making this assumption unconditionally? It might work for Intel Mux agent but is it guaranteed to be safe for any other future mux implementation? In other words, what if a "true" USB4 cable is connected which doesn't have the Thunderbolt SVID alt mode? (Pre-fetching some alternatives in case the answer is no) You might want to check with the Cros EC team if you can repurpose a bit of the "reserved" field for specifying whether the cable is TBT or not. Either that or see if there is a way to determine from the pd_ctrl->cable_speed whether the cable is actually TBT or not. Failing all the above, perhaps you'll have to wait for the PD discovery stuff to make it's way through review and use that (note that there may be timing issues between the Mux update event and PD discovery complete event reaching the port driver). Best regards, -Prashant
Re: [PATCH] platform/chrome: cros_ec_typec: Tolerate unrecognized mux flags
Including Heikki, who I forgot to add in the original patch email.
On Thu, Nov 05, 2020 at 06:03:05PM -0800, Prashant Malani wrote:
> On occasion, the Chrome Embedded Controller (EC) can send a mux
> configuration which doesn't map to a particular data mode. For instance,
> dedicated Type C chargers, when connected, may cause only
> USB_PD_MUX_POLARITY_INVERTED to be set. This is a valid flag combination
> and should not lead to a driver abort.
>
> Modify the mux configuration handling to not return an error when an
> unrecognized mux flag combination is encountered. Concordantly, make the
> ensuing print a debug level print so as to not pollute the kernel logs.
>
> Cc: Keith Short
> Signed-off-by: Prashant Malani
> ---
> drivers/platform/chrome/cros_ec_typec.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c
> b/drivers/platform/chrome/cros_ec_typec.c
> index ce031a10eb1b..5b8db02ab84a 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -537,10 +537,9 @@ static int cros_typec_configure_mux(struct
> cros_typec_data *typec, int port_num,
> port->state.mode = TYPEC_STATE_USB;
> ret = typec_mux_set(port->mux, &port->state);
> } else {
> - dev_info(typec->dev,
> - "Unsupported mode requested, mux flags: %x\n",
> - mux_flags);
> - ret = -ENOTSUPP;
> + dev_dbg(typec->dev,
> + "Unrecognized mode requested, mux flags: %x\n",
> + mux_flags);
> }
>
> return ret;
> --
> 2.29.1.341.ge80a0c044ae-goog
>
Re: [PATCH v2 5/8] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message
Hi Utkarsh,
On Fri, Nov 13, 2020 at 12:25:00PM -0800, Utkarsh Patel wrote:
> USB4 also uses same cable properties as Thunderbolt 3 so use Thunderbolt 3
> cable discover mode VDO to fill details such as active cable plug link
> training and cable rounded support.
>
> Suggested-by: Heikki Krogerus
> Signed-off-by: Utkarsh Patel
>
> --
> Changes in v2:
> - No change.
> --
> ---
> include/linux/usb/typec.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 6be558045942..d91e09d9d91c 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -75,6 +75,7 @@ enum typec_orientation {
> /*
> * struct enter_usb_data - Enter_USB Message details
> * @eudo: Enter_USB Data Object
> + * @tbt_cable_vdo: TBT3 Cable Discover Mode Response
> * @active_link_training: Active Cable Plug Link Training
> *
> * @active_link_training is a flag that should be set with uni-directional
> SBRX
> @@ -83,6 +84,7 @@ enum typec_orientation {
> */
> struct enter_usb_data {
> u32 eudo;
> + u32 tbt_cable_vdo;
Can we instead just include a field for the rounded cable support property
, similar to what was done for active_link_training? That way this gets
decoupled
from whether a TBT VDO was present in the cable or not
> unsigned char active_link_training:1;
> };
>
> --
> 2.17.1
>
Best regards,
-Prashant
Re: [PATCH v2 5/8] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message
On Tue, Nov 17, 2020 at 1:16 PM Prashant Malani wrote: > > Hi Utkarsh, > > On Fri, Nov 13, 2020 at 12:25:00PM -0800, Utkarsh Patel wrote: > > USB4 also uses same cable properties as Thunderbolt 3 so use Thunderbolt 3 > > cable discover mode VDO to fill details such as active cable plug link > > training and cable rounded support. On digging into the Cros EC code further, sounds like active cable link training and cable rounded are necessarily only part of cables that have a TBT cable VDO, so sounds like the approach in the patch is fine. Sorry for the noise. Best regards, -Prashant
Re: [PATCH v2 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode
On Tue, Nov 17, 2020 at 10:19 AM Prashant Malani wrote: > > Hi Utkarsh, > > On Fri, Nov 13, 2020 at 12:25:01PM -0800, Utkarsh Patel wrote: > > Configure Thunderbolt3/USB4 cable generation value by filing Thunderbolt 3 > > cable discover mode VDO to support rounded and non-rounded Thunderbolt3/ > > USB4 cables. > > While we are here use Thunderbolt 3 cable discover mode VDO to fill active > > cable plug link training value. > > > > Suggested-by: Heikki Krogerus > > Signed-off-by: Utkarsh Patel > > > > -- > > Changes in v2: > > - No change. > > -- > > --- > > drivers/platform/chrome/cros_ec_typec.c | 14 -- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/platform/chrome/cros_ec_typec.c > > b/drivers/platform/chrome/cros_ec_typec.c > > index 8111ed1fc574..b7416e82c3b3 100644 > > --- a/drivers/platform/chrome/cros_ec_typec.c > > +++ b/drivers/platform/chrome/cros_ec_typec.c > > @@ -514,8 +514,18 @@ static int cros_typec_enable_usb4(struct > > cros_typec_data *typec, > > else if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_CABLE) > > data.eudo |= EUDO_CABLE_TYPE_RE_TIMER << > > EUDO_CABLE_TYPE_SHIFT; > > > > - data.active_link_training = !!(pd_ctrl->control_flags & > > -USB_PD_CTRL_ACTIVE_LINK_UNIDIR); > > + /* > > + * This driver does not have access to the identity information or > > + * capabilities of the cable, so we don't know is it a real USB4 or > > + * TBT3 cable. Therefore pretending that it's always TBT3 cable by > > + * filling the TBT3 Cable VDO. > > + */ > > + data.tbt_cable_vdo = TBT_MODE; > > Is it safe to be making this assumption unconditionally? It might work for > Intel Mux agent but is it guaranteed to be safe for any other future > mux implementation? In other words, what if a "true" USB4 cable is > connected which doesn't have the Thunderbolt SVID alt mode? I dug into this a bit more and can maybe articulate my concern better: Is there a situation where both of the following are true ? : - Cable type = EUDO_CABLE_TYPE_OPTICAL or EUDO_CABLE_TYPE_RE_TIMER - No TBT_CABLE_LINK_TRAINING or TBT_CABLE_ROUNDED_SUPPORT defined (both these are 0). If both the above are true, then in Patch 7/8, wouldn't we never hit the else condition (labeled "Active USB cable") and therefore not set the mode_data correctly? > > (Pre-fetching some alternatives in case the answer is no) > > You might want to check with the Cros EC team if you can repurpose a bit of > the "reserved" field for specifying whether the cable is TBT or not. > > Either that or see if there is a way to determine from the > pd_ctrl->cable_speed > whether the cable is actually TBT or not. It seems link cable_gen and USB_PD_CTRL_ACTIVE_LINK_UNIDIR are reasonable proxies for whether the cable has TBT support, so perhaps we should only set tbt_cable_vdo = TBT_MODE if either of those are non-zero? WDYT? Best regards, -Prashant
Re: [RFC PATCH 0/3] usb: typec: Product Type time
Hi Heikki, Thanks for developing these patches :) On Wed, Nov 18, 2020 at 06:00:56PM +0300, Heikki Krogerus wrote: > Hi Prashant, > > The original discussion [1]. > > This proposal is in practice a compromise. I came to the conclusion > that we probable should expose the product type separately after all. > The reason for that is because we may in some cases actually know the > product type even when we don't have access to the Discover Identity > response. UCSI for example in practice gives us at least the cable > product type even though it does not let us know the response to the > Discover Identity command. > > So my proposal here is that we add an attribute for the product type > itself, showing the product type as a string. Then we also add the > attribute for the product type specific VDOs which we place under the > identity directory more or less the way you originally proposed. Sounds good to me. Best regards, -Prashant
Re: [RFC PATCH 2/3] usb: typec: Add product_type sysfs attribute file for partners and cables
Hi Heikki, On Wed, Nov 18, 2020 at 06:00:58PM +0300, Heikki Krogerus wrote: > USB Power Delivery Specification defines a set of product > types for partners and cables. The product type is defined > in the ID Header VDO, which is the first object in the > response to the Discover Identity command. > > This sysfs attribute file is only created for the partners > and cables if the product type is really known in the > driver. Some interfaces do not give access to the Discover > Identity response from the partner or cable, but they may > still supply the product type separately in some cases. > > When the product type of the partner or cable is detected, > uevent is also raised with PRODUCT_TYPE set to show the > actual product type (for example PRODUCT_TYPE=host). > > Signed-off-by: Heikki Krogerus I tried this out with the following peripherals: - Thunderbolt 3 active cable. - Thunderbolt 3 passive cable. - Dell WD19TB dock. - Type C DisplayPort enabled monitor (which advertises as AMA). For the above, the product_type seems to be getting parsed and displayed correctly, so FWIW: Tested-by: Prashant Malani > --- > Documentation/ABI/testing/sysfs-class-typec | 55 > drivers/usb/typec/class.c | 132 ++-- > 2 files changed, 180 insertions(+), 7 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-class-typec > b/Documentation/ABI/testing/sysfs-class-typec > index b7794e02ad205..4c09e327c62be 100644 > --- a/Documentation/ABI/testing/sysfs-class-typec > +++ b/Documentation/ABI/testing/sysfs-class-typec > @@ -139,6 +139,42 @@ Description: > Shows if the partner supports USB Power Delivery communication: > Valid values: yes, no > > +What:/sys/class/typec/-partner/product_type > +Date:December 2020 > +Contact: Heikki Krogerus > +Description: USB Power Delivery Specification defines a set of product types > + for the partner devices. This file will show the product type of > + the partner if it is known. Dual-role capable partners will have > + both UFP and DFP product types defined, but only one that > + matches the current role will be active at the time. If the > + product type of the partner is not visible to the device driver, > + this file will not exist. > + > + When the partner product type is detected, or changed with role > + swap, uvevent is also raised that contains PRODUCT_TYPE= + type> (for example PRODUCT_TYPE=hub). > + > + Valid values: > + > + UFP / device role > + == > + undefined - > + hub PDUSB Hub > + peripheralPDUSB Peripheral > + psd Power Bank > + ama Alternate Mode Adapter > + vpd VCONN Powered USB Device > + == > + > + DFP / host role > + == > + undefined - > + hub PDUSB Hub > + host PDUSB Host > + power_brick Power Brick > + amc Alternate Mode Controller > + == > + > What:/sys/class/typec/-partner>/identity/ > Date:April 2017 > Contact: Heikki Krogerus > @@ -202,6 +238,25 @@ Description: > - type-c > - captive > > +What:/sys/class/typec/-cable/product_type > +Date:December 2020 > +Contact: Heikki Krogerus > +Description: USB Power Delivery Specification defines a set of product types > + for the cables. This file will show the product type of the > + cable if it is known. If the product type of the cable is not > + visible to the device driver, this file will not exist. > + > + When the cable product type is detected, uvevent is also raised > + with PRODUCT_TYPE showing the product type of the cable. > + > + Valid values: > + > + == > + undefined - > + activeActive Cable > + passive Passive Cable > +
Re: [PATCH v3 00/11] chrome/platform: cros_ec_typec: Register cables, partner altmodes and plug altmodes
Hi Greg, On Wed, Nov 18, 2020 at 01:16:49PM +0100, Greg KH wrote: > On Wed, Nov 18, 2020 at 12:59:59PM +0100, Greg KH wrote: > > On Mon, Nov 16, 2020 at 12:11:36PM -0800, Prashant Malani wrote: > > > This patch series adds support for the following bits of functionality, > > > parsing USB Type C Power Delivery information from the Chrome Embedded > > > Controller > > > and using the Type C connector class: > > > - Register cable objects (including plug type). > > > - Register "number of altmodes" attribute for partners. > > > - Register altmodes and "number of altmodes" attribute for cable plugs. > > > > > > The functionality was earlier part of multiple series ([1], [2], [3]), but > > > I've combined it into 1 series and re-ordered the patches to hopefully > > > make > > > it easier to peruse. I've maintained the patch Acked-by/Reviewed-by tags > > > where > > > they were received. > > > > > > Patches 1/11, 2/11, 3/11 introduce the changes needed in the USB > > > subsystem (PD VDO > > > header update, sysfs attribute additions) and hence the first three > > > patches > > > can go through Greg's tree. > > > > I've taken the first 2 patches in my usb tree now, waiting for Heikki's > > response on patch 3 before I touch that. > > Ok, now taken patch 3 too. > Thanks! > I can take the rest in my usb-next tree if the platform people don't > object as well, but would need an ack for that. Will defer to Enric on this, but Patch 4/11 and onwards have a dependency on some patches which are already in the chrome-platform tree [1], so they may have to go through there. [1] https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=for-next Best regards, -Prashant
Re: [PATCH] usb: typec: Add bus type for plug alt modes
Hi Heikki,
On Wed, Dec 9, 2020 at 8:14 AM Heikki Krogerus
wrote:
>
> On Tue, Dec 08, 2020 at 03:45:19PM -0800, Prashant Malani wrote:
> > Hi Heikki,
> >
> > Thanks a lot for looking at the patch.
> >
> > On Tue, Dec 8, 2020 at 1:37 AM Heikki Krogerus
> > wrote:
> > >
> > > On Wed, Dec 02, 2020 at 07:08:47PM -0800, Prashant Malani wrote:
> > > > Add the Type C bus for plug alternate modes which are being
> > > > registered via the Type C connector class. This ensures that udev events
> > > > get generated when plug alternate modes are registered (and not just for
> > > > partner/port alternate modes), even though the Type C bus doesn't link
> > > > plug alternate mode devices to alternate mode drivers.
> > >
> > > I still don't understand how is the uevent related to the bus? If you
> > > check the device_add() function, on line 2917, kobject_uevent() is
> > > called unconditionally. The device does not need a bus for that event
> > > to be generated.
> >
> > My initial thought process was to see what is the difference in the adev
> > device
> > initialization between partner altmode and plug altmode (the only
> > difference I saw in
> > typec_register_altmode() was regarding the bus field).
> >
> > Yes, kobject_uevent() is called unconditionally, but it's return value
> > isn't checked,
> > so we don't know if it succeeded or not.
> >
> > In the case of cable plug altmode, I see it fail with the following
> > error[1]:
> >
> > [ 114.431409] kobject: 'port1-plug0.0' (4ad42956):
> > kobject_uevent_env: filter function caused the event to drop!
> >
> > I think the filter function which is called is this one:
> > drivers/base/core.c: dev_uevent_filter() [2]
> >
> > static int dev_uevent_filter(struct kset *kset, struct kobject *kobj)
> > {
> > struct kobj_type *ktype = get_ktype(kobj);
> >
> > if (ktype == &device_ktype) {
> > struct device *dev = kobj_to_dev(kobj);
> > if (dev->bus)
> > return 1;
> > if (dev->class)
> > return 1;
> > }
> > return 0;
> > }
> >
> > So, both the "if (dev->bus)" and "if (dev->class)" checks are failing here.
> > In the case of partner alt modes, bus is set by the class.c code
> > so this check likely returns 1 in that case.
>
> OK. I understand the issue now. So I would say that the proper
> solution to this problem is to link the alt modes with the class
> instead of the bus. That is much smaller change IMO.
Got it. Just to confirm that I understand correctly, do you mean:
1. Only cable plug alt modes should be linked with the class instead of the bus.
2. All alt modes (cable plug, partner, port) should be linked with the
class instead of the bus
My initial interpretation is 1.) since the bus linkage would be
necessary to match alt mode drivers to partner alt mode devices.
But, my understanding of the bus code is limited so I could be wrong;
could you kindly clarify?
Thanks,
-Prashant
Re: [PATCH] usb: typec: Add bus type for plug alt modes
Hi Heikki,
On Wed, Dec 9, 2020 at 9:15 AM Heikki Krogerus
wrote:
>
> Hi Prashant,
>
> On Wed, Dec 09, 2020 at 08:22:52AM -0800, Prashant Malani wrote:
> > Hi Heikki,
> >
> > On Wed, Dec 9, 2020 at 8:14 AM Heikki Krogerus
> > wrote:
> > >
> > > On Tue, Dec 08, 2020 at 03:45:19PM -0800, Prashant Malani wrote:
> > > > Hi Heikki,
> > > >
> > > > Thanks a lot for looking at the patch.
> > > >
> > > > On Tue, Dec 8, 2020 at 1:37 AM Heikki Krogerus
> > > > wrote:
> > > > >
> > > > > On Wed, Dec 02, 2020 at 07:08:47PM -0800, Prashant Malani wrote:
> > > > > > Add the Type C bus for plug alternate modes which are being
> > > > > > registered via the Type C connector class. This ensures that udev
> > > > > > events
> > > > > > get generated when plug alternate modes are registered (and not
> > > > > > just for
> > > > > > partner/port alternate modes), even though the Type C bus doesn't
> > > > > > link
> > > > > > plug alternate mode devices to alternate mode drivers.
> > > > >
> > > > > I still don't understand how is the uevent related to the bus? If you
> > > > > check the device_add() function, on line 2917, kobject_uevent() is
> > > > > called unconditionally. The device does not need a bus for that event
> > > > > to be generated.
> > > >
> > > > My initial thought process was to see what is the difference in the
> > > > adev device
> > > > initialization between partner altmode and plug altmode (the only
> > > > difference I saw in
> > > > typec_register_altmode() was regarding the bus field).
> > > >
> > > > Yes, kobject_uevent() is called unconditionally, but it's return value
> > > > isn't checked,
> > > > so we don't know if it succeeded or not.
> > > >
> > > > In the case of cable plug altmode, I see it fail with the following
> > > > error[1]:
> > > >
> > > > [ 114.431409] kobject: 'port1-plug0.0' (4ad42956):
> > > > kobject_uevent_env: filter function caused the event to drop!
> > > >
> > > > I think the filter function which is called is this one:
> > > > drivers/base/core.c: dev_uevent_filter() [2]
> > > >
> > > > static int dev_uevent_filter(struct kset *kset, struct kobject *kobj)
> > > > {
> > > > struct kobj_type *ktype = get_ktype(kobj);
> > > >
> > > > if (ktype == &device_ktype) {
> > > > struct device *dev = kobj_to_dev(kobj);
> > > > if (dev->bus)
> > > > return 1;
> > > > if (dev->class)
> > > > return 1;
> > > > }
> > > > return 0;
> > > > }
> > > >
> > > > So, both the "if (dev->bus)" and "if (dev->class)" checks are failing
> > > > here. In the case of partner alt modes, bus is set by the class.c code
> > > > so this check likely returns 1 in that case.
> > >
> > > OK. I understand the issue now. So I would say that the proper
> > > solution to this problem is to link the alt modes with the class
> > > instead of the bus. That is much smaller change IMO.
> >
> > Got it. Just to confirm that I understand correctly, do you mean:
> > 1. Only cable plug alt modes should be linked with the class instead of the
> > bus.
> >
> >
> >
> > 2. All alt modes (cable plug, partner, port) should be linked with the
> > class instead of the bus
> >
> > My initial interpretation is 1.) since the bus linkage would be
> > necessary to match alt mode drivers to partner alt mode devices.
> > But, my understanding of the bus code is limited so I could be wrong;
> > could you kindly clarify?
>
> We don't need to care about the bus here. A device can be part of a
> bus and a class at the same time. I don't think there is any reason to
> limit the class to only plug alt modes, so let's just assign it to all
> of them.
I had actually tried this earlier, but here we run into errors.
If we always set the class, then "partner" altmode device creation
fails ("port" altmode creation will likely also fail, but I haven't
verified that)
T
Re: [PATCH] usb: typec: Add bus type for plug alt modes
On Wed, Dec 9, 2020 at 2:59 PM Prashant Malani wrote:
>
> Hi Heikki,
>
> On Wed, Dec 9, 2020 at 9:15 AM Heikki Krogerus
> wrote:
> >
> > Hi Prashant,
> >
> > On Wed, Dec 09, 2020 at 08:22:52AM -0800, Prashant Malani wrote:
> > > Hi Heikki,
> > >
> > > On Wed, Dec 9, 2020 at 8:14 AM Heikki Krogerus
> > > wrote:
> > > >
> > > > On Tue, Dec 08, 2020 at 03:45:19PM -0800, Prashant Malani wrote:
> > > > > Hi Heikki,
> > > > >
> > > > > Thanks a lot for looking at the patch.
> > > > >
> > > > > On Tue, Dec 8, 2020 at 1:37 AM Heikki Krogerus
> > > > > wrote:
> > > > > >
> > > > > > On Wed, Dec 02, 2020 at 07:08:47PM -0800, Prashant Malani wrote:
> > > > > > > Add the Type C bus for plug alternate modes which are being
> > > > > > > registered via the Type C connector class. This ensures that udev
> > > > > > > events
> > > > > > > get generated when plug alternate modes are registered (and not
> > > > > > > just for
> > > > > > > partner/port alternate modes), even though the Type C bus doesn't
> > > > > > > link
> > > > > > > plug alternate mode devices to alternate mode drivers.
> > > > > >
> > > > > > I still don't understand how is the uevent related to the bus? If
> > > > > > you
> > > > > > check the device_add() function, on line 2917, kobject_uevent() is
> > > > > > called unconditionally. The device does not need a bus for that
> > > > > > event
> > > > > > to be generated.
> > > > >
> > > > > My initial thought process was to see what is the difference in the
> > > > > adev device
> > > > > initialization between partner altmode and plug altmode (the only
> > > > > difference I saw in
> > > > > typec_register_altmode() was regarding the bus field).
> > > > >
> > > > > Yes, kobject_uevent() is called unconditionally, but it's return
> > > > > value isn't checked,
> > > > > so we don't know if it succeeded or not.
> > > > >
> > > > > In the case of cable plug altmode, I see it fail with the following
> > > > > error[1]:
> > > > >
> > > > > [ 114.431409] kobject: 'port1-plug0.0' (4ad42956):
> > > > > kobject_uevent_env: filter function caused the event to drop!
> > > > >
> > > > > I think the filter function which is called is this one:
> > > > > drivers/base/core.c: dev_uevent_filter() [2]
> > > > >
> > > > > static int dev_uevent_filter(struct kset *kset, struct kobject *kobj)
> > > > > {
> > > > > struct kobj_type *ktype = get_ktype(kobj);
> > > > >
> > > > > if (ktype == &device_ktype) {
> > > > > struct device *dev = kobj_to_dev(kobj);
> > > > > if (dev->bus)
> > > > > return 1;
> > > > > if (dev->class)
> > > > > return 1;
> > > > > }
> > > > > return 0;
> > > > > }
> > > > >
> > > > > So, both the "if (dev->bus)" and "if (dev->class)" checks are failing
> > > > > here. In the case of partner alt modes, bus is set by the class.c code
> > > > > so this check likely returns 1 in that case.
> > > >
> > > > OK. I understand the issue now. So I would say that the proper
> > > > solution to this problem is to link the alt modes with the class
> > > > instead of the bus. That is much smaller change IMO.
> > >
> > > Got it. Just to confirm that I understand correctly, do you mean:
> > > 1. Only cable plug alt modes should be linked with the class instead of
> > > the bus.
> > >
> > >
> > >
> > > 2. All alt modes (cable plug, partner, port) should be linked with the
> > > class instead of the bus
> > >
> > > My initial interpretation is 1.) since the bus linkage would be
> > > necessary to match alt mode drivers to partner alt mode devices.
> > > But, my understanding of the bus code i
Re: [PATCH v3 2/4] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode
Hi Utkarsh,
On Wed, Nov 18, 2020 at 10:32:09PM -0800, Utkarsh Patel wrote:
> Configure Thunderbolt 3 cable generation value by filling Thunderbolt 3
> cable discover mode VDO to support rounded Thunderbolt 3 cables.
> While we are here use Thunderbolt 3 cable discover mode VDO to fill active
> cable plug link training value.
>
> Suggested-by: Heikki Krogerus
> Signed-off-by: Utkarsh Patel
>
> --
> Changes in v3:
> - Added a check for cable's TBT support before filling TBT3 discover mode
> VDO.
>
> Changes in v2:
> - No change.
> --
> ---
> drivers/platform/chrome/cros_ec_typec.c | 14 --
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c
> b/drivers/platform/chrome/cros_ec_typec.c
> index 8111ed1fc574..68b17ee1d1ae 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -514,8 +514,18 @@ static int cros_typec_enable_usb4(struct cros_typec_data
> *typec,
> else if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_CABLE)
> data.eudo |= EUDO_CABLE_TYPE_RE_TIMER << EUDO_CABLE_TYPE_SHIFT;
>
> - data.active_link_training = !!(pd_ctrl->control_flags &
> -USB_PD_CTRL_ACTIVE_LINK_UNIDIR);
> + /*
> + * Filling TBT3 Cable VDO when TBT3 cable is being used to establish
> + * USB4 connection.
> + */
> + if (pd_ctrl->cable_gen) {
> + data.tbt_cable_vdo = TBT_MODE;
> +
> + if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
> + data.tbt_cable_vdo |= TBT_CABLE_LINK_TRAINING;
> +
> + data.tbt_cable_vdo |= TBT_SET_CABLE_ROUNDED(pd_ctrl->cable_gen);
> + }
I think the following would decouple Rounded Support and Active Cable Link
Training?:
struct typec_thunderbolt_data data = {};
...
if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
data.tbt_cable_vdo |= TBT_CABLE_LINK_TRAINING;
data.tbt_cable_vdo |= TBT_SET_CABLE_ROUNDED(pd_ctrl->cable_gen);
if (data.tbt_cable_vdo)
data.tbt_cable_vdo |= TBT_MODE;
Best regards,
-Prashant
Re: [RFC PATCH 2/3] usb: typec: Add product_type sysfs attribute file for partners and cables
Hi Heikki, On Thu, Nov 19, 2020 at 01:05:06PM +0200, Heikki Krogerus wrote: > On Wed, Nov 18, 2020 at 10:53:50AM -0800, Prashant Malani wrote: > > > +What:/sys/class/typec/-cable/product_type > > > +Date:December 2020 > > > +Contact: Heikki Krogerus > > > +Description: USB Power Delivery Specification defines a set of > > > product types > > > + for the cables. This file will show the product type of the > > > + cable if it is known. If the product type of the cable is not > > > + visible to the device driver, this file will not exist. > > > + > > > + When the cable product type is detected, uvevent is also raised > > > + with PRODUCT_TYPE showing the product type of the cable. > > > + > > > + Valid values: > > > + > > > + == > > > + undefined - > > > + activeActive Cable > > > + passive Passive Cable > > > + == > > > > There exists a /sys/class/typec/-cable/type attribute (connected > > to the "active" field in struct typec_cable [1]), which is supposed > > to be populated by the Type C port driver. Won't the newly introduced > > attribute duplicate the same information as "type"? > > True. So we don't need add this for the cable separately. I'll just > modify the code so that it considers also the response to Discover > Identity command if we have access to it. > > Would it be OK if we name the file "type" instead of "product_type" > also with the partners? That makes the naming consistent. Sounds good to me :) Best regards, -Prashant
Re: [PATCH v3 2/4] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode
Hi Utkarsh,
On Thu, Nov 19, 2020 at 6:32 PM Patel, Utkarsh H
wrote:
>
> Hi Prashant,
>
> > -Original Message-
> > From: Prashant Malani
> > Sent: Thursday, November 19, 2020 12:09 AM
> > To: Patel, Utkarsh H
> > Cc: linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
> > heikki.kroge...@linux.intel.com; enric.balle...@collabora.com; Mani,
> > Rajmohan ; Shaikh, Azhar
> >
> > Subject: Re: [PATCH v3 2/4] platform/chrome: cros_ec_typec: Use Thunderbolt
> > 3 cable discover mode VDO in USB4 mode
> >
> > Hi Utkarsh,
> >
> > On Wed, Nov 18, 2020 at 10:32:09PM -0800, Utkarsh Patel wrote:
> > > Configure Thunderbolt 3 cable generation value by filling Thunderbolt
> > > 3 cable discover mode VDO to support rounded Thunderbolt 3 cables.
> > > While we are here use Thunderbolt 3 cable discover mode VDO to fill
> > > active cable plug link training value.
> > >
> > > Suggested-by: Heikki Krogerus
> > > Signed-off-by: Utkarsh Patel
> > >
> > > --
> > > Changes in v3:
> > > - Added a check for cable's TBT support before filling TBT3 discover mode
> > > VDO.
> > >
> > > Changes in v2:
> > > - No change.
> > > --
> > > ---
> > > drivers/platform/chrome/cros_ec_typec.c | 14 --
> > > 1 file changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > > b/drivers/platform/chrome/cros_ec_typec.c
> > > index 8111ed1fc574..68b17ee1d1ae 100644
> > > --- a/drivers/platform/chrome/cros_ec_typec.c
> > > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > > @@ -514,8 +514,18 @@ static int cros_typec_enable_usb4(struct
> > cros_typec_data *typec,
> > > else if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_CABLE)
> > > data.eudo |= EUDO_CABLE_TYPE_RE_TIMER <<
> > EUDO_CABLE_TYPE_SHIFT;
> > >
> > > - data.active_link_training = !!(pd_ctrl->control_flags &
> > > - USB_PD_CTRL_ACTIVE_LINK_UNIDIR);
> > > + /*
> > > +* Filling TBT3 Cable VDO when TBT3 cable is being used to establish
> > > +* USB4 connection.
> > > +*/
> > > + if (pd_ctrl->cable_gen) {
> > > + data.tbt_cable_vdo = TBT_MODE;
> > > +
> > > + if (pd_ctrl->control_flags &
> > USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
> > > + data.tbt_cable_vdo |= TBT_CABLE_LINK_TRAINING;
> > > +
> > > + data.tbt_cable_vdo |= TBT_SET_CABLE_ROUNDED(pd_ctrl-
> > >cable_gen);
> > > + }
> >
> > I think the following would decouple Rounded Support and Active Cable Link
> > Training?:
>
> Any reason you would want to decouple them?
Is there anything in the spec that says Active Cable Link Training
needs Rounded Cable support (or vice versa)?
If yes, could you kindly point me to the relevant portion in the spec
that states this?
If no, then the two should be set independently based on the response
from the Chrome EC.
FWIW, Table F-11 ( TBT3 Cable Discover Mode VDO Responses) from the
USB Type-C Cable & Connector Spec (Rel 2.0) suggests
the two are independent bits although I don't have access to the TBT3
spec to confirm.
BR,
-Prashant
[PATCH] usb: typec: Fix num_altmodes kernel-doc error
The commit to introduce the num_altmodes attribute for partner had an
error where one of the parameters was named differently in the comment
and the function signature. Fix the version in the comment to align with
what is in the function signature.
This fixes the following htmldocs warning:
drivers/usb/typec/class.c:632: warning: Excess function parameter
'num_alt_modes' description in 'typec_partner_set_num_altmodes'
Fixes: a0ccdc4a77a1 ("usb: typec: Add number of altmodes partner attr")
Signed-off-by: Prashant Malani
---
drivers/usb/typec/class.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index e68798599ca8..cb1362187a7c 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -618,7 +618,7 @@ EXPORT_SYMBOL_GPL(typec_partner_set_identity);
/**
* typec_partner_set_num_altmodes - Set the number of available partner
altmodes
* @partner: The partner to be updated.
- * @num_alt_modes: The number of altmodes we want to specify as available.
+ * @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
* partner. This value is *not* enforced in alternate mode registration
routines.
--
2.29.2.454.gaff20da3a2-goog
Re: linux-next: build warning after merge of the usb tree
Hi Stephen,
On Fri, Nov 20, 2020 at 04:15:06PM +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the usb tree, today's linux-next build (htmldocs) produced
> this warning:
>
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> drivers/usb/typec/class.c:632: warning: Excess function parameter
> 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
>
> Introduced by commit
>
> a0ccdc4a77a1 ("usb: typec: Add number of altmodes partner attr")
Thank you for the email, and my apologies about the warning. I've sent a
patch the mailing lists which should hopefully fix this [1].
Please let me know if there is further action required from my side.
Best regards,
[1]:
https://lore.kernel.org/linux-usb/20201120063523.4159877-1-pmal...@chromium.org/
>
> --
> Cheers,
> Stephen Rothwell
Re: [PATCH v3 1/4] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message
On Fri, Nov 20, 2020 at 10:05:14AM +0200, Heikki Krogerus wrote:
> On Wed, Nov 18, 2020 at 10:32:08PM -0800, Utkarsh Patel wrote:
> > When Thunderbolt 3 cable is being used to create USB4 connection, use
> > Thunderbolt 3 discover mode VDO to fill details such as active cable plug
> > link training and cable rounded support.
> > With USB4 cables, these VDO members need not be filled.
> >
> > Suggested-by: Heikki Krogerus
> > Signed-off-by: Utkarsh Patel
> >
> > --
> > Changes in v3:
> > - Changed the commit mesage to reflect why TBT3 VDO is being used.
> > - Added more details in the header file about the usage of TBT3 VDO.
> >
> > Changes in v2:
> > - No change.
> > --
> > ---
> > include/linux/usb/typec.h | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> > index 6be558045942..25731ed863fa 100644
> > --- a/include/linux/usb/typec.h
> > +++ b/include/linux/usb/typec.h
> > @@ -75,6 +75,10 @@ enum typec_orientation {
> > /*
> > * struct enter_usb_data - Enter_USB Message details
> > * @eudo: Enter_USB Data Object
> > + * @tbt_cable_vdo: TBT3 Cable Discover Mode Response
>
> This is fine..
>
> > + * @tbt_cable_vdo needs to be filled with details of active cable plug link
> > + * training and cable rounded support when thunderbolt 3 cable is being
> > used to
> > + * create USB4 connection. Do not fill this in case of USB4 cable.
>
> But this is not. The description of the member tells what the member
> contains, but it does not make sense to explain also how to use the
> member in the same place.
Slightly tangential question here:
Is there a need to mention "active cable plug link training" and "cable
rounded support" at all? Wouldn't it be sufficient to omit those in the
description (in case some mux implementation wants to use the other fields
of the VDO) ?
> Instead you should explain how to use the
> member in the description of the structure. So..
>
> > * @active_link_training: Active Cable Plug Link Training
> > *
> > * @active_link_training is a flag that should be set with uni-directional
> > SBRX
>
> Put it here. That will make this much more readable.
>
>
> thanks,
>
> --
> heikki
Re: [PATCH] usb: typec: Fix num_altmodes kernel-doc error
On Fri, Nov 20, 2020 at 09:41:21AM +0100, Greg KH wrote:
> On Thu, Nov 19, 2020 at 10:35:22PM -0800, Prashant Malani wrote:
> > The commit to introduce the num_altmodes attribute for partner had an
> > error where one of the parameters was named differently in the comment
> > and the function signature. Fix the version in the comment to align with
> > what is in the function signature.
> >
> > This fixes the following htmldocs warning:
> >
> > drivers/usb/typec/class.c:632: warning: Excess function parameter
> > 'num_alt_modes' description in 'typec_partner_set_num_altmodes'
> >
> > Fixes: a0ccdc4a77a1 ("usb: typec: Add number of altmodes partner attr")
> > Signed-off-by: Prashant Malani
>
> You forgot a "Reported-by:" tag here :(
>
> I'll go add it by hand...
My bad :(, thank you for making the addition.
Best regards,
-Prashant
Re: [PATCH v3 2/4] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode
On Fri, Nov 20, 2020 at 01:22:18PM +0200, Heikki Krogerus wrote:
> On Thu, Nov 19, 2020 at 12:09:06AM -0800, Prashant Malani wrote:
> > Hi Utkarsh,
> >
> > On Wed, Nov 18, 2020 at 10:32:09PM -0800, Utkarsh Patel wrote:
> > > Configure Thunderbolt 3 cable generation value by filling Thunderbolt 3
> > > cable discover mode VDO to support rounded Thunderbolt 3 cables.
> > > While we are here use Thunderbolt 3 cable discover mode VDO to fill active
> > > cable plug link training value.
> > >
> > > Suggested-by: Heikki Krogerus
> > > Signed-off-by: Utkarsh Patel
> > >
> > > --
> > > Changes in v3:
> > > - Added a check for cable's TBT support before filling TBT3 discover mode
> > > VDO.
> > >
> > > Changes in v2:
> > > - No change.
> > > --
> > > ---
> > > drivers/platform/chrome/cros_ec_typec.c | 14 --
> > > 1 file changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > > b/drivers/platform/chrome/cros_ec_typec.c
> > > index 8111ed1fc574..68b17ee1d1ae 100644
> > > --- a/drivers/platform/chrome/cros_ec_typec.c
> > > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > > @@ -514,8 +514,18 @@ static int cros_typec_enable_usb4(struct
> > > cros_typec_data *typec,
> > > else if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_CABLE)
> > > data.eudo |= EUDO_CABLE_TYPE_RE_TIMER << EUDO_CABLE_TYPE_SHIFT;
> > >
> > > - data.active_link_training = !!(pd_ctrl->control_flags &
> > > -USB_PD_CTRL_ACTIVE_LINK_UNIDIR);
> > > + /*
> > > + * Filling TBT3 Cable VDO when TBT3 cable is being used to establish
> > > + * USB4 connection.
> > > + */
> > > + if (pd_ctrl->cable_gen) {
> > > + data.tbt_cable_vdo = TBT_MODE;
> > > +
> > > + if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
> > > + data.tbt_cable_vdo |= TBT_CABLE_LINK_TRAINING;
> > > +
> > > + data.tbt_cable_vdo |= TBT_SET_CABLE_ROUNDED(pd_ctrl->cable_gen);
> > > + }
> >
> > I think the following would decouple Rounded Support and Active Cable Link
> > Training?:
> >
> > struct typec_thunderbolt_data data = {};
> > ...
> > if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
> > data.tbt_cable_vdo |= TBT_CABLE_LINK_TRAINING;
> >
> > data.tbt_cable_vdo |= TBT_SET_CABLE_ROUNDED(pd_ctrl->cable_gen);
>
> I agree with this. We should not modify the data that we actually
> have access to.
>
> > if (data.tbt_cable_vdo)
> > data.tbt_cable_vdo |= TBT_MODE;
>
> That is wrong. If the LSRX communication is bi-directional, and/or the
> data rates are non-rounded, the cable has to be TBT3 cable. So I think
> what you would want is:
Thanks for pointing this out, I didn't consider this case.
>
> if (!data.tbt_cable_vdo)
> data.tbt_cable_vdo = TBT_MODE;
>
> But of course we can not do that either, because we have to set the
> TBT_MODE bit if there is any other data in tbt_cable_vdo (USB Type-C
> spec does not define any other valid values except 0x0001 = TBT_MODE
> for that field). Otherwise the mux driver should consider the data
> corrupted. So we would have to do this:
>
> if (pd_ctrl->cable_gen &&
> pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
> data.tbt_cable_vdo = 0; /* We assume USB4 cable */
> else
> data.tbt_cable_vdo |= TBT_MODE; /* It is for sure TBT3 cable */
>
> But I would not do that. TBT3 cable can also support unidirectional
> SBU communication and rounded data rates.
>
> IMO safer bet for now would be to just claim that the cable is always
> TBT3 cable until we have access to information that can really tell us
> is the cable USB4 or TBT3.
Which brings us back to v1 of the patch :S
That still leaves my underlying concern that we'll be telling the Mux
implementation that a TBT3 cable is connected when in fact it's a USB4
active cable.
How about we don't set the TBT_MODE bit at all ? IMO it's equally bad as
setting it
always, but with the additional advantage:
- USB4 active cable case : you are covered (since if we unilaterally set
TBT_MODE then the Active USB4 cable case never gets executed in
pmc_usb_mux_usb4() in drivers/usb/typec/mux/intel_pmc_mux.c Patch 3/4)
- Bidirectional LSRX non-rounded TBT: Still supported since
the code path in the Intel Mux agent is the same.
I understand neither of the options are ideal, but WDYT?
BR,
-Prashant
Re: [PATCH v3 2/4] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode
On Fri, Nov 20, 2020 at 4:07 AM Prashant Malani wrote:
>
> On Fri, Nov 20, 2020 at 01:22:18PM +0200, Heikki Krogerus wrote:
> > On Thu, Nov 19, 2020 at 12:09:06AM -0800, Prashant Malani wrote:
> > > Hi Utkarsh,
> > >
> > > On Wed, Nov 18, 2020 at 10:32:09PM -0800, Utkarsh Patel wrote:
> > > > Configure Thunderbolt 3 cable generation value by filling Thunderbolt 3
> > > > cable discover mode VDO to support rounded Thunderbolt 3 cables.
> > > > While we are here use Thunderbolt 3 cable discover mode VDO to fill
> > > > active
> > > > cable plug link training value.
> > > >
> > > > Suggested-by: Heikki Krogerus
> > > > Signed-off-by: Utkarsh Patel
> > > >
> > > > --
> > > > Changes in v3:
> > > > - Added a check for cable's TBT support before filling TBT3 discover
> > > > mode
> > > > VDO.
> > > >
> > > > Changes in v2:
> > > > - No change.
> > > > --
> > > > ---
> > > > drivers/platform/chrome/cros_ec_typec.c | 14 --
> > > > 1 file changed, 12 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > > > b/drivers/platform/chrome/cros_ec_typec.c
> > > > index 8111ed1fc574..68b17ee1d1ae 100644
> > > > --- a/drivers/platform/chrome/cros_ec_typec.c
> > > > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > > > @@ -514,8 +514,18 @@ static int cros_typec_enable_usb4(struct
> > > > cros_typec_data *typec,
> > > > else if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_CABLE)
> > > > data.eudo |= EUDO_CABLE_TYPE_RE_TIMER <<
> > > > EUDO_CABLE_TYPE_SHIFT;
> > > >
> > > > - data.active_link_training = !!(pd_ctrl->control_flags &
> > > > -USB_PD_CTRL_ACTIVE_LINK_UNIDIR);
> > > > + /*
> > > > + * Filling TBT3 Cable VDO when TBT3 cable is being used to establish
> > > > + * USB4 connection.
> > > > + */
> > > > + if (pd_ctrl->cable_gen) {
> > > > + data.tbt_cable_vdo = TBT_MODE;
> > > > +
> > > > + if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
> > > > + data.tbt_cable_vdo |= TBT_CABLE_LINK_TRAINING;
> > > > +
> > > > + data.tbt_cable_vdo |=
> > > > TBT_SET_CABLE_ROUNDED(pd_ctrl->cable_gen);
> > > > + }
> > >
> > > I think the following would decouple Rounded Support and Active Cable Link
> > > Training?:
> > >
> > > struct typec_thunderbolt_data data = {};
> > > ...
> > > if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
> > > data.tbt_cable_vdo |= TBT_CABLE_LINK_TRAINING;
> > >
> > > data.tbt_cable_vdo |= TBT_SET_CABLE_ROUNDED(pd_ctrl->cable_gen);
> >
> > I agree with this. We should not modify the data that we actually
> > have access to.
> >
> > > if (data.tbt_cable_vdo)
> > > data.tbt_cable_vdo |= TBT_MODE;
> >
> > That is wrong. If the LSRX communication is bi-directional, and/or the
> > data rates are non-rounded, the cable has to be TBT3 cable. So I think
> > what you would want is:
>
> Thanks for pointing this out, I didn't consider this case.
>
> >
> > if (!data.tbt_cable_vdo)
> > data.tbt_cable_vdo = TBT_MODE;
> >
> > But of course we can not do that either, because we have to set the
> > TBT_MODE bit if there is any other data in tbt_cable_vdo (USB Type-C
> > spec does not define any other valid values except 0x0001 = TBT_MODE
> > for that field). Otherwise the mux driver should consider the data
> > corrupted. So we would have to do this:
> >
> > if (pd_ctrl->cable_gen &&
> > pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
> > data.tbt_cable_vdo = 0; /* We assume USB4 cable */
> > else
> > data.tbt_cable_vdo |= TBT_MODE; /* It is for sure TBT3 cable
> > */
> >
> > But I would not do that. TBT3 cable can also support unidirectional
> > SBU communication and rounded data rates.
> >
> > IMO safer bet for now would be to just claim that the cable is always
> > TBT3 cable until we have access to information that can really tell us
> >
Re: [PATCH] platform/chrome: cros_ec_typec: Tolerate unrecognized mux flags
Friendly ping. If there are not other reservations, can we pick this
patch? It doesn't depend on any other patch series.
Thanks,
On Thu, Nov 19, 2020 at 11:32 PM Heikki Krogerus
wrote:
>
> On Thu, Nov 05, 2020 at 06:03:05PM -0800, Prashant Malani wrote:
> > On occasion, the Chrome Embedded Controller (EC) can send a mux
> > configuration which doesn't map to a particular data mode. For instance,
> > dedicated Type C chargers, when connected, may cause only
> > USB_PD_MUX_POLARITY_INVERTED to be set. This is a valid flag combination
> > and should not lead to a driver abort.
> >
> > Modify the mux configuration handling to not return an error when an
> > unrecognized mux flag combination is encountered. Concordantly, make the
> > ensuing print a debug level print so as to not pollute the kernel logs.
> >
> > Cc: Keith Short
> > Signed-off-by: Prashant Malani
>
> FWIW:
>
> Acked-by: Heikki Krogerus
>
> > ---
> > drivers/platform/chrome/cros_ec_typec.c | 7 +++
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > b/drivers/platform/chrome/cros_ec_typec.c
> > index ce031a10eb1b..5b8db02ab84a 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -537,10 +537,9 @@ static int cros_typec_configure_mux(struct
> > cros_typec_data *typec, int port_num,
> > port->state.mode = TYPEC_STATE_USB;
> > ret = typec_mux_set(port->mux, &port->state);
> > } else {
> > - dev_info(typec->dev,
> > - "Unsupported mode requested, mux flags: %x\n",
> > - mux_flags);
> > - ret = -ENOTSUPP;
> > + dev_dbg(typec->dev,
> > + "Unrecognized mode requested, mux flags: %x\n",
> > + mux_flags);
> > }
> >
> > return ret;
> > --
> > 2.29.1.341.ge80a0c044ae-goog
>
> thanks,
>
> --
> heikki
[PATCH v4 1/2] usb: typec: Consolidate sysfs ABI documentation
Both partner and cable have identity VDOs. These are listed separately
in the Documentation/ABI/testing/sysfs-class-typec. Factor these out
into a common location to avoid the duplication.
Signed-off-by: Prashant Malani
Acked-by: Heikki Krogerus
---
Changes in v4:
- Rebased on top of the usb-next tree.
- Added Acked-by tag from pevious version's review.
- Corrected a typo ('syfs' -> 'sysfs') in the subject line.
Patch first introduced in v3.
Documentation/ABI/testing/sysfs-class-typec | 59 ++---
1 file changed, 17 insertions(+), 42 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-typec
b/Documentation/ABI/testing/sysfs-class-typec
index 4eccb343fc7b..88ffc14d4cd2 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -147,42 +147,6 @@ Description:
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/-partner>/identity/
-Date: April 2017
-Contact: Heikki Krogerus
-Description:
- This directory appears only if the port device driver is capable
- of showing the result of Discover Identity USB power delivery
- command. That will not always be possible even when USB power
- delivery is supported, for example when USB power delivery
- communication for the port is mostly handled in firmware. If the
- directory exists, it will have an attribute file for every VDO
- in Discover Identity command result.
-
-What: /sys/class/typec/-partner/identity/id_header
-Date: April 2017
-Contact: Heikki Krogerus
-Description:
- ID Header VDO part of Discover Identity command result. The
- value will show 0 until Discover Identity command result becomes
- available. The value can be polled.
-
-What: /sys/class/typec/-partner/identity/cert_stat
-Date: April 2017
-Contact: Heikki Krogerus
-Description:
- Cert Stat VDO part of Discover Identity command result. The
- value will show 0 until Discover Identity command result becomes
- available. The value can be polled.
-
-What: /sys/class/typec/-partner/identity/product
-Date: April 2017
-Contact: Heikki Krogerus
-Description:
- Product VDO part of Discover Identity command result. The value
- will show 0 until Discover Identity command result becomes
- available. The value can be polled.
-
USB Type-C cable devices (eg. /sys/class/typec/port0-cable/)
@@ -219,17 +183,28 @@ Description:
This file remains hidden until a value greater than or equal to 0
is set by Type C port driver.
-What: /sys/class/typec/-cable/identity/
+
+USB Type-C partner/cable Power Delivery Identity objects
+
+NOTE: The following attributes will be applicable to both
+partner (e.g /sys/class/typec/port0-partner/) and
+cable (e.g /sys/class/typec/port0-cable/) devices. Consequently, the example
file
+paths below are prefixed with "/sys/class/typec/-{partner|cable}/" to
+reflect this.
+
+What: /sys/class/typec/-{partner|cable}/identity/
Date: April 2017
Contact: Heikki Krogerus
Description:
This directory appears only if the port device driver is capable
of showing the result of Discover Identity USB power delivery
command. That will not always be possible even when USB power
- delivery is supported. If the directory exists, it will have an
- attribute for every VDO returned by Discover Identity command.
+ delivery is supported, for example when USB power delivery
+ communication for the port is mostly handled in firmware. If the
+ directory exists, it will have an attribute file for every VDO
+ in Discover Identity command result.
-What: /sys/class/typec/-cable/identity/id_header
+What: /sys/class/typec/-{partner|cable}/identity/id_header
Date: April 2017
Contact: Heikki Krogerus
Description:
@@ -237,7 +212,7 @@ Description:
value will show 0 until Discover Identity command result becomes
available. The value can be polled.
-What: /sys/class/typec/-cable/identity/cert_stat
+What: /sys/class/typec/-{partner|cable}/identity/cert_stat
Date: April 2017
Contact: Heikki Krogerus
Description:
@@ -245,7 +220,7 @@ Description:
value will show 0 until Discover Identity command result becomes
available. The value can be polled.
-What: /sys/class/typec/-cable/identity/product
+What:
[PATCH v4 2/2] usb: typec: Expose Product Type VDOs via sysfs
A PD-capable device can return up to 3 Product Type VDOs as part of its
DiscoverIdentity Response (USB PD Spec, Rev 3.0, Version 2.0, Section
6.4.4.3.1). Add sysfs attributes to expose these to userspace.
Cc: Benson Leung
Cc: Heikki Krogerus
Signed-off-by: Prashant Malani
Reviewed-by: Heikki Krogerus
---
Changes in v4:
- Added Reviewed-by tag from v3's review.
- Rebased on top of usb-next + Patch 1/2
Changes in v3:
- Split each product type VDO into a separate attribute.
- Changed sprintf() to sysfs_emit().
- Changed ABI documentation based on consolidation of identity VDO
descriptions in the previous patch (1/2).
Changes in v2:
- Added sysfs_notify() call for the attribute.
- Added description for the attribute in
Documentation/ABI/testing/sysfs-class-typec.
Documentation/ABI/testing/sysfs-class-typec | 24 +++
drivers/usb/typec/class.c | 33 +
2 files changed, 57 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-class-typec
b/Documentation/ABI/testing/sysfs-class-typec
index 88ffc14d4cd2..619c4c67432b 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -228,6 +228,30 @@ Description:
will show 0 until Discover Identity command result becomes
available. The value can be polled.
+What:
/sys/class/typec/-{partner|cable}/identity/product_type_vdo1
+Date: October 2020
+Contact: Prashant Malani
+Description:
+ 1st Product Type VDO of Discover Identity command result.
+ The value will show 0 until Discover Identity command result
becomes
+ available and a valid Product Type VDO is returned.
+
+What:
/sys/class/typec/-{partner|cable}/identity/product_type_vdo2
+Date: October 2020
+Contact: Prashant Malani
+Description:
+ 2nd Product Type VDO of Discover Identity command result.
+ The value will show 0 until Discover Identity command result
becomes
+ available and a valid Product Type VDO is returned.
+
+What:
/sys/class/typec/-{partner|cable}/identity/product_type_vdo3
+Date: October 2020
+Contact: Prashant Malani
+Description:
+ 3rd Product Type VDO of Discover Identity command result.
+ The value will show 0 until Discover Identity command result
becomes
+ available and a valid Product Type VDO is returned.
+
USB Type-C port alternate mode devices.
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index cb1362187a7c..df4478baf95b 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -124,10 +124,40 @@ static ssize_t product_show(struct device *dev, struct
device_attribute *attr,
}
static DEVICE_ATTR_RO(product);
+static ssize_t product_type_vdo1_show(struct device *dev, struct
device_attribute *attr,
+ char *buf)
+{
+ struct usb_pd_identity *id = get_pd_identity(dev);
+
+ return sysfs_emit(buf, "0x%08x\n", id->vdo[0]);
+}
+static DEVICE_ATTR_RO(product_type_vdo1);
+
+static ssize_t product_type_vdo2_show(struct device *dev, struct
device_attribute *attr,
+ char *buf)
+{
+ struct usb_pd_identity *id = get_pd_identity(dev);
+
+ return sysfs_emit(buf, "0x%08x\n", id->vdo[1]);
+}
+static DEVICE_ATTR_RO(product_type_vdo2);
+
+static ssize_t product_type_vdo3_show(struct device *dev, struct
device_attribute *attr,
+ char *buf)
+{
+ struct usb_pd_identity *id = get_pd_identity(dev);
+
+ return sysfs_emit(buf, "0x%08x\n", id->vdo[2]);
+}
+static DEVICE_ATTR_RO(product_type_vdo3);
+
static struct attribute *usb_pd_id_attrs[] = {
&dev_attr_id_header.attr,
&dev_attr_cert_stat.attr,
&dev_attr_product.attr,
+ &dev_attr_product_type_vdo1.attr,
+ &dev_attr_product_type_vdo2.attr,
+ &dev_attr_product_type_vdo3.attr,
NULL
};
@@ -146,6 +176,9 @@ static void typec_report_identity(struct device *dev)
sysfs_notify(&dev->kobj, "identity", "id_header");
sysfs_notify(&dev->kobj, "identity", "cert_stat");
sysfs_notify(&dev->kobj, "identity", "product");
+ sysfs_notify(&dev->kobj, "identity", "product_type_vdo1");
+ sysfs_notify(&dev->kobj, "identity", "product_type_vdo2");
+ sysfs_notify(&dev->kobj, "identity", "product_type_vdo3");
}
/* - */
--
2.29.2.454.gaff20da3a2-goog
Re: [PATCH v3 1/2] usb: typec: Consolidate syfs ABI documentation
Hi Heikki, On Tue, Nov 24, 2020 at 5:23 AM Heikki Krogerus wrote: > > On Fri, Oct 23, 2020 at 02:43:26PM -0700, Prashant Malani wrote: > > Both partner and cable have identity VDOs. These are listed separately > > in the Documentation/ABI/testing/sysfs-class-typec. Factor these out > > into a common location to avoid the duplication. > > This does not apply any more. Cany you resend these. Thanks for the heads up. Resent here [1] [1]: https://lore.kernel.org/linux-usb/20201124201033.592576-2-pmal...@chromium.org/ BR, -Prashant
Re: [PATCH v4 1/2] usb: typec: Consolidate sysfs ABI documentation
Hi,
On Tue, Nov 24, 2020 at 12:10:31PM -0800, Prashant Malani wrote:
> Both partner and cable have identity VDOs. These are listed separately
> in the Documentation/ABI/testing/sysfs-class-typec. Factor these out
> into a common location to avoid the duplication.
>
> Signed-off-by: Prashant Malani
> Acked-by: Heikki Krogerus
I copied the Acked-by line from v3 [1] as is, but looks like there was a
typo there and the email address should be
"heikki.kroge...@linux.intel.com".
Please let me know if it's fine as is or whether I should send another
patchset.
[1]
https://lore.kernel.org/linux-usb/20201110105225.gh1224...@kuha.fi.intel.com/
> ---
>
> Changes in v4:
> - Rebased on top of the usb-next tree.
> - Added Acked-by tag from pevious version's review.
> - Corrected a typo ('syfs' -> 'sysfs') in the subject line.
>
> Patch first introduced in v3.
>
> Documentation/ABI/testing/sysfs-class-typec | 59 ++---
> 1 file changed, 17 insertions(+), 42 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-typec
> b/Documentation/ABI/testing/sysfs-class-typec
> index 4eccb343fc7b..88ffc14d4cd2 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -147,42 +147,6 @@ Description:
> 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/-partner>/identity/
> -Date:April 2017
> -Contact: Heikki Krogerus
> -Description:
> - This directory appears only if the port device driver is capable
> - of showing the result of Discover Identity USB power delivery
> - command. That will not always be possible even when USB power
> - delivery is supported, for example when USB power delivery
> - communication for the port is mostly handled in firmware. If the
> - directory exists, it will have an attribute file for every VDO
> - in Discover Identity command result.
> -
> -What:/sys/class/typec/-partner/identity/id_header
> -Date:April 2017
> -Contact: Heikki Krogerus
> -Description:
> - ID Header VDO part of Discover Identity command result. The
> - value will show 0 until Discover Identity command result becomes
> - available. The value can be polled.
> -
> -What:/sys/class/typec/-partner/identity/cert_stat
> -Date:April 2017
> -Contact: Heikki Krogerus
> -Description:
> - Cert Stat VDO part of Discover Identity command result. The
> - value will show 0 until Discover Identity command result becomes
> - available. The value can be polled.
> -
> -What:/sys/class/typec/-partner/identity/product
> -Date:April 2017
> -Contact: Heikki Krogerus
> -Description:
> - Product VDO part of Discover Identity command result. The value
> - will show 0 until Discover Identity command result becomes
> - available. The value can be polled.
> -
>
> USB Type-C cable devices (eg. /sys/class/typec/port0-cable/)
>
> @@ -219,17 +183,28 @@ Description:
> This file remains hidden until a value greater than or equal to > 0
> is set by Type C port driver.
>
> -What:/sys/class/typec/-cable/identity/
> +
> +USB Type-C partner/cable Power Delivery Identity objects
> +
> +NOTE: The following attributes will be applicable to both
> +partner (e.g /sys/class/typec/port0-partner/) and
> +cable (e.g /sys/class/typec/port0-cable/) devices. Consequently, the example
> file
> +paths below are prefixed with "/sys/class/typec/-{partner|cable}/" to
> +reflect this.
> +
> +What:/sys/class/typec/-{partner|cable}/identity/
> Date:April 2017
> Contact: Heikki Krogerus
> Description:
> This directory appears only if the port device driver is capable
> of showing the result of Discover Identity USB power delivery
> command. That will not always be possible even when USB power
> - delivery is supported. If the directory exists, it will have an
> - attribute for every VDO returned by Discover Identity command.
> + delivery is supported, for example when USB power delivery
> + communication for the port is mostly handled in firmware. If the
> + directory exists, it will have an attribute
[PATCH v5 2/2] usb: typec: Expose Product Type VDOs via sysfs
A PD-capable device can return up to 3 Product Type VDOs as part of its
DiscoverIdentity Response (USB PD Spec, Rev 3.0, Version 2.0, Section
6.4.4.3.1). Add sysfs attributes to expose these to userspace.
Cc: Benson Leung
Cc: Heikki Krogerus
Signed-off-by: Prashant Malani
Reviewed-by: Heikki Krogerus
---
Changes in v5:
- No changes.
Changes in v4:
- Added Reviewed-by tag from v3's review.
- Rebased on top of usb-next + Patch 1/2
Changes in v3:
- Split each product type VDO into a separate attribute.
- Changed sprintf() to sysfs_emit().
- Changed ABI documentation based on consolidation of identity VDO
descriptions in the previous patch (1/2).
Changes in v2:
- Added sysfs_notify() call for the attribute.
- Added description for the attribute in
Documentation/ABI/testing/sysfs-class-typec.
Documentation/ABI/testing/sysfs-class-typec | 24 +++
drivers/usb/typec/class.c | 33 +
2 files changed, 57 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-class-typec
b/Documentation/ABI/testing/sysfs-class-typec
index 88ffc14d4cd2..619c4c67432b 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -228,6 +228,30 @@ Description:
will show 0 until Discover Identity command result becomes
available. The value can be polled.
+What:
/sys/class/typec/-{partner|cable}/identity/product_type_vdo1
+Date: October 2020
+Contact: Prashant Malani
+Description:
+ 1st Product Type VDO of Discover Identity command result.
+ The value will show 0 until Discover Identity command result
becomes
+ available and a valid Product Type VDO is returned.
+
+What:
/sys/class/typec/-{partner|cable}/identity/product_type_vdo2
+Date: October 2020
+Contact: Prashant Malani
+Description:
+ 2nd Product Type VDO of Discover Identity command result.
+ The value will show 0 until Discover Identity command result
becomes
+ available and a valid Product Type VDO is returned.
+
+What:
/sys/class/typec/-{partner|cable}/identity/product_type_vdo3
+Date: October 2020
+Contact: Prashant Malani
+Description:
+ 3rd Product Type VDO of Discover Identity command result.
+ The value will show 0 until Discover Identity command result
becomes
+ available and a valid Product Type VDO is returned.
+
USB Type-C port alternate mode devices.
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index cb1362187a7c..df4478baf95b 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -124,10 +124,40 @@ static ssize_t product_show(struct device *dev, struct
device_attribute *attr,
}
static DEVICE_ATTR_RO(product);
+static ssize_t product_type_vdo1_show(struct device *dev, struct
device_attribute *attr,
+ char *buf)
+{
+ struct usb_pd_identity *id = get_pd_identity(dev);
+
+ return sysfs_emit(buf, "0x%08x\n", id->vdo[0]);
+}
+static DEVICE_ATTR_RO(product_type_vdo1);
+
+static ssize_t product_type_vdo2_show(struct device *dev, struct
device_attribute *attr,
+ char *buf)
+{
+ struct usb_pd_identity *id = get_pd_identity(dev);
+
+ return sysfs_emit(buf, "0x%08x\n", id->vdo[1]);
+}
+static DEVICE_ATTR_RO(product_type_vdo2);
+
+static ssize_t product_type_vdo3_show(struct device *dev, struct
device_attribute *attr,
+ char *buf)
+{
+ struct usb_pd_identity *id = get_pd_identity(dev);
+
+ return sysfs_emit(buf, "0x%08x\n", id->vdo[2]);
+}
+static DEVICE_ATTR_RO(product_type_vdo3);
+
static struct attribute *usb_pd_id_attrs[] = {
&dev_attr_id_header.attr,
&dev_attr_cert_stat.attr,
&dev_attr_product.attr,
+ &dev_attr_product_type_vdo1.attr,
+ &dev_attr_product_type_vdo2.attr,
+ &dev_attr_product_type_vdo3.attr,
NULL
};
@@ -146,6 +176,9 @@ static void typec_report_identity(struct device *dev)
sysfs_notify(&dev->kobj, "identity", "id_header");
sysfs_notify(&dev->kobj, "identity", "cert_stat");
sysfs_notify(&dev->kobj, "identity", "product");
+ sysfs_notify(&dev->kobj, "identity", "product_type_vdo1");
+ sysfs_notify(&dev->kobj, "identity", "product_type_vdo2");
+ sysfs_notify(&dev->kobj, "identity", "product_type_vdo3");
}
/* - */
--
2.29.2.454.gaff20da3a2-goog
[PATCH v5 1/2] usb: typec: Consolidate sysfs ABI documentation
Both partner and cable have identity VDOs. These are listed separately
in the Documentation/ABI/testing/sysfs-class-typec. Factor these out
into a common location to avoid the duplication.
Signed-off-by: Prashant Malani
Acked-by: Heikki Krogerus
---
Changes in v5:
- Corrected the email address in the Acked-by tag.
Changes in v4:
- Rebased on top of the usb-next tree.
- Added Acked-by tag from pevious version's review.
- Corrected a typo ('syfs' -> 'sysfs') in the subject line.
Patch first introduced in v3.
Documentation/ABI/testing/sysfs-class-typec | 59 ++---
1 file changed, 17 insertions(+), 42 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-typec
b/Documentation/ABI/testing/sysfs-class-typec
index 4eccb343fc7b..88ffc14d4cd2 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -147,42 +147,6 @@ Description:
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/-partner>/identity/
-Date: April 2017
-Contact: Heikki Krogerus
-Description:
- This directory appears only if the port device driver is capable
- of showing the result of Discover Identity USB power delivery
- command. That will not always be possible even when USB power
- delivery is supported, for example when USB power delivery
- communication for the port is mostly handled in firmware. If the
- directory exists, it will have an attribute file for every VDO
- in Discover Identity command result.
-
-What: /sys/class/typec/-partner/identity/id_header
-Date: April 2017
-Contact: Heikki Krogerus
-Description:
- ID Header VDO part of Discover Identity command result. The
- value will show 0 until Discover Identity command result becomes
- available. The value can be polled.
-
-What: /sys/class/typec/-partner/identity/cert_stat
-Date: April 2017
-Contact: Heikki Krogerus
-Description:
- Cert Stat VDO part of Discover Identity command result. The
- value will show 0 until Discover Identity command result becomes
- available. The value can be polled.
-
-What: /sys/class/typec/-partner/identity/product
-Date: April 2017
-Contact: Heikki Krogerus
-Description:
- Product VDO part of Discover Identity command result. The value
- will show 0 until Discover Identity command result becomes
- available. The value can be polled.
-
USB Type-C cable devices (eg. /sys/class/typec/port0-cable/)
@@ -219,17 +183,28 @@ Description:
This file remains hidden until a value greater than or equal to 0
is set by Type C port driver.
-What: /sys/class/typec/-cable/identity/
+
+USB Type-C partner/cable Power Delivery Identity objects
+
+NOTE: The following attributes will be applicable to both
+partner (e.g /sys/class/typec/port0-partner/) and
+cable (e.g /sys/class/typec/port0-cable/) devices. Consequently, the example
file
+paths below are prefixed with "/sys/class/typec/-{partner|cable}/" to
+reflect this.
+
+What: /sys/class/typec/-{partner|cable}/identity/
Date: April 2017
Contact: Heikki Krogerus
Description:
This directory appears only if the port device driver is capable
of showing the result of Discover Identity USB power delivery
command. That will not always be possible even when USB power
- delivery is supported. If the directory exists, it will have an
- attribute for every VDO returned by Discover Identity command.
+ delivery is supported, for example when USB power delivery
+ communication for the port is mostly handled in firmware. If the
+ directory exists, it will have an attribute file for every VDO
+ in Discover Identity command result.
-What: /sys/class/typec/-cable/identity/id_header
+What: /sys/class/typec/-{partner|cable}/identity/id_header
Date: April 2017
Contact: Heikki Krogerus
Description:
@@ -237,7 +212,7 @@ Description:
value will show 0 until Discover Identity command result becomes
available. The value can be polled.
-What: /sys/class/typec/-cable/identity/cert_stat
+What: /sys/class/typec/-{partner|cable}/identity/cert_stat
Date: April 2017
Contact: Heikki Krogerus
Description:
@@ -245,7 +220,7 @@ Description:
value will show 0 until Discover Identity command result becomes
available. The value can be polled.
Re: [PATCH v4 1/2] usb: typec: Consolidate sysfs ABI documentation
Hi Heikki, On Tue, Nov 24, 2020 at 11:53 PM Heikki Krogerus wrote: > > On Wed, Nov 25, 2020 at 09:46:06AM +0200, Heikki Krogerus wrote: > > On Tue, Nov 24, 2020 at 12:32:35PM -0800, Prashant Malani wrote: > > > Hi, > > > > > > On Tue, Nov 24, 2020 at 12:10:31PM -0800, Prashant Malani wrote: > > > > Both partner and cable have identity VDOs. These are listed separately > > > > in the Documentation/ABI/testing/sysfs-class-typec. Factor these out > > > > into a common location to avoid the duplication. > > > > > > > > Signed-off-by: Prashant Malani > > > > Acked-by: Heikki Krogerus > > > I copied the Acked-by line from v3 [1] as is, but looks like there was a > > > typo there and the email address should be > > > "heikki.kroge...@linux.intel.com". > > > > > > Please let me know if it's fine as is or whether I should send another > > > patchset. > > > > It is fine. Thanks for taking care of that :-) > > Arch, no. It's not fine (I don't know what I'm talking about there). I > think it would be better that you do resend. Got it. v5 sent [1] [1] https://lore.kernel.org/linux-usb/20201125084911.1077462-1-pmal...@chromium.org/ Thanks, -Prashant
[PATCH] platform/chrome: cros_ec_typec: Decouple cable remove on disconnect
Type C cable objects are independent of partner objects. Don't return
if a partner object is not registered (mostly in the case of an error
while registering a partner), and instead treat the removal of partner
and cable objects on disconnect independently.
Cc: Benson Leung
Signed-off-by: Prashant Malani
---
This patch should be applied on top of the series[1] which adds support
for cable & plugs to cros-ec-typec.
[1]
https://lore.kernel.org/linux-usb/20201116201150.2919178-1-pmal...@chromium.org/
drivers/platform/chrome/cros_ec_typec.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index 65c5d0090ccd..8294486908ca 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -626,9 +626,8 @@ static void cros_typec_set_port_params_v1(struct
cros_typec_data *typec,
"Failed to register partner on port: %d\n",
port_num);
} else {
- if (!typec->ports[port_num]->partner)
- return;
- cros_typec_remove_partner(typec, port_num);
+ if (typec->ports[port_num]->partner)
+ cros_typec_remove_partner(typec, port_num);
if (typec->ports[port_num]->cable)
cros_typec_remove_cable(typec, port_num);
--
2.29.2.454.gaff20da3a2-goog
[PATCH] usb: typec: Add bus type for plug alt modes
Add the Type C bus for plug alternate modes which are being registered via the Type C connector class. This ensures that udev events get generated when plug alternate modes are registered (and not just for partner/port alternate modes), even though the Type C bus doesn't link plug alternate mode devices to alternate mode drivers. Update the Type C bus documentation to mention that there are alternate mode devices for plugs as well. Signed-off-by: Prashant Malani Cc: Heikki Krogerus --- Documentation/driver-api/usb/typec_bus.rst | 6 +++--- drivers/usb/typec/class.c | 8 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Documentation/driver-api/usb/typec_bus.rst b/Documentation/driver-api/usb/typec_bus.rst index 21c890ae17e5..7874d2f37d9f 100644 --- a/Documentation/driver-api/usb/typec_bus.rst +++ b/Documentation/driver-api/usb/typec_bus.rst @@ -15,9 +15,9 @@ modes by using the SVID and the mode number. :ref:`USB Type-C Connector Class ` provides a device for every alternate mode a port supports, and separate device for every alternate mode the partner -supports. The drivers for the alternate modes are bound to the partner alternate -mode devices, and the port alternate mode devices must be handled by the port -drivers. +or cable plug supports. The drivers for the alternate modes are bound to the +partner alternate mode devices, and the port alternate mode devices must be +handled by the port drivers. When a new partner alternate mode device is registered, it is linked to the alternate mode device of the port that the partner is attached to, that has diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index 35eec707cb51..74061a699f16 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -478,8 +478,12 @@ typec_register_altmode(struct device *parent, if (!is_port) typec_altmode_set_partner(alt); - /* The partners are bind to drivers */ - if (is_typec_partner(parent)) + /* +* The partners are bind to drivers. +* Also set the bus field for plug alt modes so that the udev event occurs on device +* registration. +*/ + if (is_typec_partner(parent) || is_typec_plug(parent)) alt->adev.dev.bus = &typec_bus; ret = device_register(&alt->adev.dev); -- 2.29.2.454.gaff20da3a2-goog
[PATCH] usb: typec: Send uevent for num_altmodes update
Generate a change uevent when the "number_of_alternate_modes" sysfs file for partners and plugs is updated by a port driver. Cc: Heikki Krogerus Cc: Benson Leung Signed-off-by: Prashant Malani --- drivers/usb/typec/class.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index ebfd3113a9a8..8f77669f9cf4 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -766,6 +766,7 @@ int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmod return ret; sysfs_notify(&partner->dev.kobj, NULL, "number_of_alternate_modes"); + kobject_uevent(&partner->dev.kobj, KOBJ_CHANGE); return 0; } @@ -923,6 +924,7 @@ int typec_plug_set_num_altmodes(struct typec_plug *plug, int num_altmodes) return ret; sysfs_notify(&plug->dev.kobj, NULL, "number_of_alternate_modes"); + kobject_uevent(&plug->dev.kobj, KOBJ_CHANGE); return 0; } -- 2.29.2.729.g45daf8777d-goog
Re: [PATCH] usb: typec: Send uevent for num_altmodes update
Hi Greg, Thanks for taking a look at the patch. On Thu, Jan 7, 2021 at 1:16 AM Greg KH wrote: > > On Wed, Jan 06, 2021 at 07:49:04PM -0800, Prashant Malani wrote: > > Generate a change uevent when the "number_of_alternate_modes" sysfs file > > for partners and plugs is updated by a port driver. > > > > Cc: Heikki Krogerus > > Cc: Benson Leung > > Signed-off-by: Prashant Malani > > --- > > drivers/usb/typec/class.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > > index ebfd3113a9a8..8f77669f9cf4 100644 > > --- a/drivers/usb/typec/class.c > > +++ b/drivers/usb/typec/class.c > > @@ -766,6 +766,7 @@ int typec_partner_set_num_altmodes(struct typec_partner > > *partner, int num_altmod > > return ret; > > > > sysfs_notify(&partner->dev.kobj, NULL, "number_of_alternate_modes"); > > + kobject_uevent(&partner->dev.kobj, KOBJ_CHANGE); > > Shouldn't the sysfs_notify() handle the "something has changed" logic > good enough for userspace, as obviously someone is polling on the thing > (otherwise we wouldn't be calling sysfs_notify...) > > The kobject itself hasn't "changed", but rather an individual attribute > has changed. We don't want to create uevents for every individual sysfs > attribute changing values, do we? Fair point. I noticed other attributes in this source file use a similar approach (sysfs_notify + kobject_uevent) and took guidance from there in an attempt to remain consistent (though, of course, your point still stands). I'm guessing it is for processes that rely on udev events (subsystem=typec) rather than polling. > > What is preventing a normal "monitor the sysfs file" logic from working > here for anyone who wants to know that the alternate modes have changed? One limitation I can think of is that this sysfs file is hidden till it has a valid value (i.e >= 0), so a user-space process might not be able to poll on the file till it is visible (I suppose even then one could poll on the parent). Kindly disregard the patch if you reckon it is unnecessary. Best regards, -Prashant
Re: [PATCH] platform/chrome: cros_ec_typec: Handle NULL EC pointer during probe.
Hi Enric,
On Thu, Apr 30, 2020 at 8:26 AM Enric Balletbo i Serra
wrote:
>
> Hi Prashant,
>
> On 30/4/20 2:43, Prashant Malani wrote:
> > On Wed, Apr 29, 2020 at 5:38 PM Daniil Lunev wrote:
> >>
> >> [to make it appear on the mailing list as I didn't realize I was in
> >> hypertext sending mode]
> >>
> >> On Thu, Apr 30, 2020 at 10:11 AM Daniil Lunev wrote:
> >>>
> >>> Hi Enric.
> >>> I encountered the issue on a Hatch device when trying running 5.4 kernel
> >>> on that. After talking to Prashant it seems that any device with coreboot
> >>> built before a certain point (a particular fix for device hierarchy in
> >>> ACPI tables of Chrome devices which happened in mid-April) will not be
> >>> able to correctly initialize the driver and will get a kernel panic
> >>> trying to do so.
> >
> > A clarifying detail here: This should not be seen in any current
> > *production* device. No prod device firmware will carry the erroneous
> > ACPI device entry.
> >
>
> Thanks for the clarification. Then, I don't think we need to upstream this.
> This
> kind of "defensive-programming" it's not something that should matter to
> upstream.
Actually, on second thought, I am not 100% sure about this:
Daniil, is the erroneous ACPI device on a *production* firmware for
this device (I'm not sure about the vintage of that device's BIOS)?
My apologies for the confusion, Enric and Daniil; but would be good to
get clarification from Daniil.
Best regards,
>
> Thanks,
> Enric
>
>
> >>> Thanks,
> >>> Daniil
> >>>
> >>> On Thu, Apr 30, 2020 at 7:58 AM Enric Balletbo i Serra
> >>> wrote:
> >>>>
> >>>> Hi Daniil,
> >>>>
> >>>> Thank you for the patch.
> >>>>
> >>>> On 28/4/20 3:02, Daniil Lunev wrote:
> >>>>> Missing EC in device hierarchy causes NULL pointer to be returned to the
> >>>>> probe function which leads to NULL pointer dereference when trying to
> >>>>> send a command to the EC. This can be the case if the device is missing
> >>>>> or incorrectly configured in the firmware blob. Even if the situation
> >>>>
> >>>> There is any production device with a buggy firmware outside? Or this is
> >>>> just
> >>>> for defensive programming while developing the firmware? Which device is
> >>>> affected for this issue?
> >>>>
> >>>> Thanks,
> >>>> Enric
> >>>>
> >>>>> occures, the driver shall not cause a kernel panic as the condition is
> >>>>> not critical for the system functions.
> >>>>>
> >>>>> Signed-off-by: Daniil Lunev
> >>>>> ---
> >>>>>
> >>>>> drivers/platform/chrome/cros_ec_typec.c | 5 +
> >>>>> 1 file changed, 5 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/platform/chrome/cros_ec_typec.c
> >>>>> b/drivers/platform/chrome/cros_ec_typec.c
> >>>>> index 874269c07073..30d99c930445 100644
> >>>>> --- a/drivers/platform/chrome/cros_ec_typec.c
> >>>>> +++ b/drivers/platform/chrome/cros_ec_typec.c
> >>>>> @@ -301,6 +301,11 @@ static int cros_typec_probe(struct platform_device
> >>>>> *pdev)
> >>>>>
> >>>>> typec->dev = dev;
> >>>>> typec->ec = dev_get_drvdata(pdev->dev.parent);
> >>>>> + if (!typec->ec) {
> >>>>> + dev_err(dev, "Failed to get Cros EC data\n");
> >>>>> + return -EINVAL;
> >>>>> + }
> >>>>> +
> >>>>> platform_set_drvdata(pdev, typec);
> >>>>>
> >>>>> ret = cros_typec_get_cmd_version(typec);
> >>>>>
Re: [PATCH] platform/chrome: cros_ec_typec: Handle NULL EC pointer during probe.
Hi Daniil,
On Fri, May 01, 2020 at 10:15:18AM +1000, Daniil Lunev wrote:
> On the official revision of coreboot for hatch it doesn't even try to
> load Type C. However it gives some warning messages from
> cros-usbpd-notify-acpi about EC, So I wonder why the check of the same
> type is not appropriate in the typec driver?
I think the difference is that GOOG0003 is already present on shipped /
official versions of coreboot (so not having that check can cause
existing release images/devices to crash), whereas for GOOG0014 that is / isn't
the case.
Is GOOG0014 present on the official release coreboot image for this
device? If so, what's its path (/sys/bus/acpi/devices//path) ?
Best regards,
-Prashant
>
> ../chrome/cros_usbpd_notify.c
>
> /* Get the EC device pointer needed to talk to the EC. */
> ec_dev = dev_get_drvdata(dev->parent);
> if (!ec_dev) {
> /*
> * We continue even for older devices which don't have the
> * correct device heirarchy, namely, GOOG0003 is a child
> * of GOOG0004.
> */
> dev_warn(dev, "Couldn't get Chrome EC device pointer.\n");
> }
>
>
> # dmesg
> ...
> [8.513351] cros-ec-spi spi-PRP0001:02: EC failed to respond in time
> [8.722072] cros-ec-spi spi-PRP0001:02: EC failed to respond in time
> [8.729271] cros-ec-spi spi-PRP0001:02: Cannot identify the EC: error -110
> [8.736966] cros-ec-spi spi-PRP0001:02: cannot register EC,
> fallback to spidev
> [8.767017] cros_ec_lpcs GOOG0004:00: Chrome EC device registered
> [8.807537] cros-usbpd-notify-acpi GOOG0003:00: Couldn't get Chrome
> EC device pointer.
> ...
>
> On Fri, May 1, 2020 at 2:17 AM Prashant Malani wrote:
> >
> > Hi Enric,
> >
> > On Thu, Apr 30, 2020 at 8:26 AM Enric Balletbo i Serra
> > wrote:
> > >
> > > Hi Prashant,
> > >
> > > On 30/4/20 2:43, Prashant Malani wrote:
> > > > On Wed, Apr 29, 2020 at 5:38 PM Daniil Lunev
> > > > wrote:
> > > >>
> > > >> [to make it appear on the mailing list as I didn't realize I was in
> > > >> hypertext sending mode]
> > > >>
> > > >> On Thu, Apr 30, 2020 at 10:11 AM Daniil Lunev
> > > >> wrote:
> > > >>>
> > > >>> Hi Enric.
> > > >>> I encountered the issue on a Hatch device when trying running 5.4
> > > >>> kernel on that. After talking to Prashant it seems that any device
> > > >>> with coreboot built before a certain point (a particular fix for
> > > >>> device hierarchy in ACPI tables of Chrome devices which happened in
> > > >>> mid-April) will not be able to correctly initialize the driver and
> > > >>> will get a kernel panic trying to do so.
> > > >
> > > > A clarifying detail here: This should not be seen in any current
> > > > *production* device. No prod device firmware will carry the erroneous
> > > > ACPI device entry.
> > > >
> > >
> > > Thanks for the clarification. Then, I don't think we need to upstream
> > > this. This
> > > kind of "defensive-programming" it's not something that should matter to
> > > upstream.
> >
> > Actually, on second thought, I am not 100% sure about this:
> > Daniil, is the erroneous ACPI device on a *production* firmware for
> > this device (I'm not sure about the vintage of that device's BIOS)?
> >
> > My apologies for the confusion, Enric and Daniil; but would be good to
> > get clarification from Daniil.
> >
> > Best regards,
> > >
> > > Thanks,
> > > Enric
> > >
> > >
> > > >>> Thanks,
> > > >>> Daniil
> > > >>>
> > > >>> On Thu, Apr 30, 2020 at 7:58 AM Enric Balletbo i Serra
> > > >>> wrote:
> > > >>>>
> > > >>>> Hi Daniil,
> > > >>>>
> > > >>>> Thank you for the patch.
> > > >>>>
> > > >>>> On 28/4/20 3:02, Daniil Lunev wrote:
> > > >>>>> Missing EC in device hierarchy causes NULL pointer to be returned
> > > >>>>> to the
> > > >>>>> probe function which leads to NULL pointer dereference when trying
> > > >>>>> to
> > > >>>>> send a command to the EC. This can be the case if the device is
> > > >>>>> missing
> > > >>>>> or
[PATCH] usb: typec: mux: intel: Handle alt mode HPD_LVL
According to the PMC Type C Subsystem (TCSS) Mux programming guide rev
0.6, when a device is transitioning to DP Alternate Mode state, if the
HPD_LVL in the status update command VDO is set, the HPD_HIGH field in
the Alternate Mode request “mode_data” field (bit 14) should also be
set. Ensure the bit is correctly handled while issuing the Alternate
Mode request.
Signed-off-by: Prashant Malani
Fixes: 6701adfa9693 ("usb: typec: driver for Intel PMC mux control")
---
drivers/usb/typec/mux/intel_pmc_mux.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c
b/drivers/usb/typec/mux/intel_pmc_mux.c
index f5c5e0aef66f..c599112559e7 100644
--- a/drivers/usb/typec/mux/intel_pmc_mux.c
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -157,6 +157,10 @@ pmc_usb_mux_dp(struct pmc_usb_port *port, struct
typec_mux_state *state)
req.mode_data |= (state->mode - TYPEC_STATE_MODAL) <<
PMC_USB_ALTMODE_DP_MODE_SHIFT;
+ if (data->status & DP_STATUS_HPD_STATE)
+ req.mode_data |= PMC_USB_DP_HPD_LVL <<
+PMC_USB_ALTMODE_DP_MODE_SHIFT;
+
return pmc_usb_command(port, (void *)&req, sizeof(req));
}
--
2.26.2.303.gf8c07b1a785-goog
[PATCH v2] usb: typec: mux: intel: Handle alt mode HPD_HIGH
According to the PMC Type C Subsystem (TCSS) Mux programming guide rev
0.6, when a device is transitioning to DP Alternate Mode state, if the
HPD_STATE (bit 7) field in the status update command VDO is set to
HPD_HIGH, the HPD_HIGH field in the Alternate Mode request “mode_data”
field (bit 14) should also be set. Ensure the bit is correctly handled
while issuing the Alternate Mode request.
Signed-off-by: Prashant Malani
Fixes: 6701adfa9693 ("usb: typec: driver for Intel PMC mux control")
---
Changes in v2:
- Clarified the commit message to mention the proper field names.
drivers/usb/typec/mux/intel_pmc_mux.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c
b/drivers/usb/typec/mux/intel_pmc_mux.c
index f5c5e0aef66f..c599112559e7 100644
--- a/drivers/usb/typec/mux/intel_pmc_mux.c
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -157,6 +157,10 @@ pmc_usb_mux_dp(struct pmc_usb_port *port, struct
typec_mux_state *state)
req.mode_data |= (state->mode - TYPEC_STATE_MODAL) <<
PMC_USB_ALTMODE_DP_MODE_SHIFT;
+ if (data->status & DP_STATUS_HPD_STATE)
+ req.mode_data |= PMC_USB_DP_HPD_LVL <<
+PMC_USB_ALTMODE_DP_MODE_SHIFT;
+
return pmc_usb_command(port, (void *)&req, sizeof(req));
}
--
2.26.2.303.gf8c07b1a785-goog
Re: [PATCH] usb: typec: mux: intel: Handle alt mode HPD_LVL
Sorry, didn't compose the Commit message quite right, have sent out v2.
Thanks,
On Tue, Apr 28, 2020 at 10:34 PM Prashant Malani wrote:
>
> According to the PMC Type C Subsystem (TCSS) Mux programming guide rev
> 0.6, when a device is transitioning to DP Alternate Mode state, if the
> HPD_LVL in the status update command VDO is set, the HPD_HIGH field in
> the Alternate Mode request “mode_data” field (bit 14) should also be
> set. Ensure the bit is correctly handled while issuing the Alternate
> Mode request.
>
> Signed-off-by: Prashant Malani
> Fixes: 6701adfa9693 ("usb: typec: driver for Intel PMC mux control")
> ---
> drivers/usb/typec/mux/intel_pmc_mux.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c
> b/drivers/usb/typec/mux/intel_pmc_mux.c
> index f5c5e0aef66f..c599112559e7 100644
> --- a/drivers/usb/typec/mux/intel_pmc_mux.c
> +++ b/drivers/usb/typec/mux/intel_pmc_mux.c
> @@ -157,6 +157,10 @@ pmc_usb_mux_dp(struct pmc_usb_port *port, struct
> typec_mux_state *state)
> req.mode_data |= (state->mode - TYPEC_STATE_MODAL) <<
> PMC_USB_ALTMODE_DP_MODE_SHIFT;
>
> + if (data->status & DP_STATUS_HPD_STATE)
> + req.mode_data |= PMC_USB_DP_HPD_LVL <<
> +PMC_USB_ALTMODE_DP_MODE_SHIFT;
> +
> return pmc_usb_command(port, (void *)&req, sizeof(req));
> }
>
> --
> 2.26.2.303.gf8c07b1a785-goog
>
Re: [PATCH 2/2] platform/chrome: typec: Register Type C switches
Hi Enric,
Thanks for your review. Kindly see inline:
On Wed, Apr 29, 2020 at 3:22 PM Enric Balletbo i Serra
wrote:
>
> Hi Prashant,
>
> Thank you for your patch.
>
> On 23/4/20 0:22, Prashant Malani wrote:
> > Register Type C mux and switch handles, when provided via firmware
> > bindings. These will allow the cros-ec-typec driver, and also alternate
> > mode drivers to configure connected Muxes correctly, according to PD
> > information retrieved from the Chrome OS EC.
> >
> > Signed-off-by: Prashant Malani
> > ---
> > drivers/platform/chrome/cros_ec_typec.c | 47 +
> > 1 file changed, 47 insertions(+)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > b/drivers/platform/chrome/cros_ec_typec.c
> > index eda57db26f8d..324ead297c4d 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -14,6 +14,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> >
> > #define DRV_NAME "cros-ec-typec"
> >
> > @@ -25,6 +27,9 @@ struct cros_typec_port {
> > struct typec_partner *partner;
> > /* Port partner PD identity info. */
> > struct usb_pd_identity p_identity;
> > + struct typec_switch *ori_sw;
> > + struct typec_mux *mux;
> > + struct usb_role_switch *role_sw;
> > };
> >
> > /* Platform-specific data for the Chrome OS EC Type C controller. */
> > @@ -84,6 +89,40 @@ static int cros_typec_parse_port_props(struct
> > typec_capability *cap,
> > return 0;
> > }
> >
> > +static int cros_typec_get_switch_handles(struct cros_typec_port *port,
> > + struct fwnode_handle *fwnode,
> > + struct device *dev)
> > +{
> > + port->mux = fwnode_typec_mux_get(fwnode, NULL);
> > + if (IS_ERR(port->mux)) {
>
> Should you return an error if NULL is returned (IS_ERR_OR_NULL) ? I think that
> fwnode_typec_mux_get can return NULL too.
I think returning NULL can be considered "not an error" for devices
that don't have kernel-controlled muxes (which won't have this
property defined).
So this check should be fine as is.
>
>
> > + dev_info(dev, "Mux handle not found.\n");
> > + goto mux_err;
> > + }
> > +
> > + port->ori_sw = fwnode_typec_switch_get(fwnode);
> > + if (IS_ERR(port->ori_sw)) {
>
> ditto
>
> > + dev_info(dev, "Orientation switch handle not found.\n");
> > + goto ori_sw_err;
> > + }
> > +
> > + port->role_sw = fwnode_usb_role_switch_get(fwnode);
> > + if (IS_ERR(port->role_sw)) {
>
> ditto
>
> > + dev_info(dev, "USB role switch handle not found.\n");
> > + goto role_sw_err;
> > + }
> > +
> > + return 0;
> > +
> > +role_sw_err:
> > + usb_role_switch_put(port->role_sw);
> > +ori_sw_err:
> > + typec_switch_put(port->ori_sw);
> > +mux_err:
> > + typec_mux_put(port->mux);
> > +
> > + return -ENODEV;
> > +}
> > +
> > static void cros_unregister_ports(struct cros_typec_data *typec)
> > {
> > int i;
> > @@ -91,6 +130,9 @@ static void cros_unregister_ports(struct cros_typec_data
> > *typec)
> > for (i = 0; i < typec->num_ports; i++) {
> > if (!typec->ports[i])
> > continue;
> > + usb_role_switch_put(typec->ports[i]->role_sw);
> > + typec_switch_put(typec->ports[i]->ori_sw);
> > + typec_mux_put(typec->ports[i]->mux);
> > typec_unregister_port(typec->ports[i]->port);
> > }
> > }
> > @@ -153,6 +195,11 @@ static int cros_typec_init_ports(struct
> > cros_typec_data *typec)
> > ret = PTR_ERR(cros_port->port);
> > goto unregister_ports;
> > }
> > +
> > + ret = cros_typec_get_switch_handles(cros_port, fwnode, dev);
> > + if (ret)
> > + dev_info(dev, "No switch control for port %d\n",
> > + port_num);
>
> When drivers are working, they should not spit out any messages, make
> this dev_dbg() at the most. Be quiet, please.
Ack. Will update this in the next version.
>
>
> > }
> >
> > return 0;
> >
Re: [PATCH] platform/chrome: cros_ec_typec: Handle NULL EC pointer during probe.
On Wed, Apr 29, 2020 at 5:38 PM Daniil Lunev wrote:
>
> [to make it appear on the mailing list as I didn't realize I was in
> hypertext sending mode]
>
> On Thu, Apr 30, 2020 at 10:11 AM Daniil Lunev wrote:
> >
> > Hi Enric.
> > I encountered the issue on a Hatch device when trying running 5.4 kernel on
> > that. After talking to Prashant it seems that any device with coreboot
> > built before a certain point (a particular fix for device hierarchy in ACPI
> > tables of Chrome devices which happened in mid-April) will not be able to
> > correctly initialize the driver and will get a kernel panic trying to do so.
A clarifying detail here: This should not be seen in any current
*production* device. No prod device firmware will carry the erroneous
ACPI device entry.
> > Thanks,
> > Daniil
> >
> > On Thu, Apr 30, 2020 at 7:58 AM Enric Balletbo i Serra
> > wrote:
> >>
> >> Hi Daniil,
> >>
> >> Thank you for the patch.
> >>
> >> On 28/4/20 3:02, Daniil Lunev wrote:
> >> > Missing EC in device hierarchy causes NULL pointer to be returned to the
> >> > probe function which leads to NULL pointer dereference when trying to
> >> > send a command to the EC. This can be the case if the device is missing
> >> > or incorrectly configured in the firmware blob. Even if the situation
> >>
> >> There is any production device with a buggy firmware outside? Or this is
> >> just
> >> for defensive programming while developing the firmware? Which device is
> >> affected for this issue?
> >>
> >> Thanks,
> >> Enric
> >>
> >> > occures, the driver shall not cause a kernel panic as the condition is
> >> > not critical for the system functions.
> >> >
> >> > Signed-off-by: Daniil Lunev
> >> > ---
> >> >
> >> > drivers/platform/chrome/cros_ec_typec.c | 5 +
> >> > 1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> >> > b/drivers/platform/chrome/cros_ec_typec.c
> >> > index 874269c07073..30d99c930445 100644
> >> > --- a/drivers/platform/chrome/cros_ec_typec.c
> >> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> >> > @@ -301,6 +301,11 @@ static int cros_typec_probe(struct platform_device
> >> > *pdev)
> >> >
> >> > typec->dev = dev;
> >> > typec->ec = dev_get_drvdata(pdev->dev.parent);
> >> > + if (!typec->ec) {
> >> > + dev_err(dev, "Failed to get Cros EC data\n");
> >> > + return -EINVAL;
> >> > + }
> >> > +
> >> > platform_set_drvdata(pdev, typec);
> >> >
> >> > ret = cros_typec_get_cmd_version(typec);
> >> >
Re: [PATCH 1/2] dt-bindings: chrome: Add cros-ec-typec mux props
Hi Rob,
On Wed, Jun 10, 2020 at 9:53 AM Rob Herring wrote:
>
> On Wed, Jun 10, 2020 at 9:34 AM Heikki Krogerus
> wrote:
> >
> > On Tue, Jun 09, 2020 at 04:57:40PM -0700, Prashant Malani wrote:
> > > Hi Rob,
> > >
> > > Thanks again for the comments and feedback. Kindly see responses inline:
> > >
> > > (Trimming unrelated text from thread):
> > >
> > >
> > > ports {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > port@0 {
> > > reg = <0>;
> > > usb_con_hs: endpoint {
> > > remote-endpoint = <&foo_usb_hs_controller>;
> > > };
> > > };
> > >
> > > port@1 {
> > > reg = <1>;
> > > usb_con0_ss: endpoint@0 {
> > > remote-endpoint = <&mode_mux_in>;
> > > };
> > > };
> > >
> > > port@2 {
> > > reg = <2>;
> > > usb_con_sbu: endpoint {
> > > remote-endpoint = <&foo_dp_aux>;
> > > };
> > > };
> > > };
> >
> > The pins that can be reassigned can in practice go anywhere. We can't
> > group them in any way. What do we do for example when the two sideband
> > pins go to different locations?
>
> The sideband pins from the connector go to multiple places or the
> sideband signal from a controller go to multiple connectors? Either
> way, that's solved with multiple endpoints. In the former case, port@2
> would have multiple endpoints with all the possible connections. The
> general model of the graph is each port is a separate data channel and
> multiple endpoints are either a mux or fanout depending on the data
> direction.
>
> > It looks like the OF graph for the USB Type-C connectors expects the
> > pins to be grouped like that, which is too bad, because unfortunately
> > it will not work. It would require that we have a separate port for
> > every pin that can be reassigned on the connector, and let's not even
> > consider that.
>
> I guess you are referring to the 4 SS signal pairs and that they could
> be 2 USB links, 1 USB link and 1-2 Alt mode links, or 4 Alt mode
> links. I think the grouping of SS signals is fine as I'd expect
> there's a single entity deciding the routing. That would be 'mux' node
> I think, but 'mux' is the wrong abstraction here. I guess we could
> have 4 muxes (1 for each signal), but I'd hope we don't need that
> level of detail in DT. I think we're in agreement on that.
I think the updated example handles this grouping (port@1 going to a
"SS mux") although as you said it should probably be a group of muxes,
but I think the example illustrates the point. Is that assessment
correct?
>
> > We need higher level description of the connections for the USB Type-C
> > connectors. For example, a connector can be connected to this (or
> > these) DisplayPort(s), this USB 2.0 port, this USB 3.x port, this USB4
> > port, etc. And this is the mux that handles the pins on this
> > connector, and these are the retimers, etc. etc.
> >
> > We also need a way to identify those connections, and relying on
> > something like fixed port node addresses, so indexes in practice,
> > feels really risky to me. A conflict may seem unlikely now, but we
> > seen those so many times in the past other things like GPIOs, IRQs,
> > etc. We really need to define string identifiers for these
> > connections.
>
> I assume for IRQs you are referring to cases where we have a variable
> number such as 'interrupts = <1 2 3>;' where 'interrupts = <1 3>'
> doesn't work because we can't describe interrupt 2 is not present? The
> graph binding doesn't suffer from that issue since we can easily omit
> port or endpoint nodes.
>
> Also, the numbering is specific to a compatible string. If we need
> different numbering, then we can do a new compatible.
>
> Rob
Would this block the addition of the "*-switch" properties? IIUC the
two are related but not dependent on each other.
The *-switch properties are phandles which the Type C connector class
framework expects (and uses to get handles to those switches).
These would point to the "mux" or "group of mux" abstractions as noted earlier.
I'd suggest we work on updating the Type C connector class to a model
that can describe connections for both DT (using OF graph) and ACPI,
if something
like that exists, but let it not block the addition of these switches
to usb-connector.yaml; they will be needed by the Type C connector
class framework
regardless of the form the connection description takes.
Best regards,
Re: [PATCH 1/2] dt-bindings: chrome: Add cros-ec-typec mux props
Hi Rob,
Just following up on this. Would the below example align better with
OF graph requirements?
Example begins at , but in summary:
- port@1 (Superspeed) of usb-c-connector will have 3 endpoints (0 =
goes to mode switch, 1 = goes to orientation switch, 2 = goes to data
role switch)
- port@2 (SBU) of usb-c-connector will have 2 endpoints (0 = goes to
mode switch, 1 = goes to orientation switch)
-These end points can go through arbitrarily long paths (including
retimers) as long as they end up at the following devices:
a. device with compatible string "typec-mode-switch" for endpoint 0.
b. device with compatible string "typec-orientation-switch" for endpoint 1.
c. device with compatible string "typec-data-role-switch" for endpoint 2.
- Connector class framework will perform the traversal from
usb-c-connector port endpoints to the "*-switch" devices.
Best regards,
On Fri, Jun 12, 2020 at 10:34 AM Prashant Malani wrote:
>
> Hi Rob,
>
> Thanks as always for your help in reviewing this proposal!
>
> Kindly see inline
>
> (Trimming text);
> On Thu, Jun 11, 2020 at 02:00:47PM -0600, Rob Herring wrote:
> > On Wed, Jun 10, 2020 at 11:49 AM Prashant Malani
> > wrote:
> > >
> > > Hi Rob,
> > >
> > > On Wed, Jun 10, 2020 at 9:53 AM Rob Herring wrote:
> > > >
> > > > > On Tue, Jun 09, 2020 at 04:57:40PM -0700, Prashant Malani wrote:
> > >
> > > I think the updated example handles this grouping (port@1 going to a
> > > "SS mux") although as you said it should probably be a group of muxes,
> > > but I think the example illustrates the point. Is that assessment
> > > correct?
> >
> > Yes, but let's stop calling it a mux. It's a "USB Type C signal routing
> > blob".
>
> Ack.
>
> Let's go with "-switch" ? That's what the connector class uses and it
> conveys the meaning (unless that is a reserved keyword in DT).
>
> >
> > > Would this block the addition of the "*-switch" properties? IIUC the
> > > two are related but not dependent on each other.
> > >
> > > The *-switch properties are phandles which the Type C connector class
> > > framework expects (and uses to get handles to those switches).
> > > These would point to the "mux" or "group of mux" abstractions as noted
> > > earlier.
> >
> > You don't need them though. Walk the graph. You get the connector
> > port@1 remote endpoint and then get its parent.
> >
>
> I see; would it be something along the lines of this? (DT example
> follows; search for "example_end" to jump to bottom):
>
>
>
> connector@0 {
> compatible = "usb-c-connector";
> reg = <0>;
> power-role = "dual";
> data-role = "dual";
> try-power-role = "source";
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@0 {
> reg = <0>;
> usb_con_hs: endpoint {
> remote-endpoint = <&foo_usb_hs_controller>;
> };
> };
>
> port@1 {
> reg = <1>;
> #address-cells = <1>;
> #size-cells = <0>;
>
> usb_con0_ss_mode: endpoint@0 {
> reg = <0>
> remote-endpoint = <&mode_switch_ss_in>;
> };
>
> usb_con0_ss_orientation: endpoint@1 {
> reg = <1>
> remote-endpoint = <&orientation_switch_ss_in>;
> };
>
> usb_con0_ss_data_role: endpoint@2 {
> reg = <2>
> remote-endpoint = <&data_role_switch_in>;
> };
> };
>
> port@2 {
> reg = <2>;
> #address-cells = <1>;
> #size-cells = <0>;
> usb_con0_sbu_mode: endpoint@0 {
> reg = <0>
> remote-endpoint = <&mode_switch_sbu_in>;
> };
> usb_con0_sbu_orientation: endpoint@1 {
> reg = <1>
> remote-endpoint = <&orientation_switch_sbu_in>;
> };
> };
> };
> };
>
> mode_switch {
> compatible = "typec-mode-switch";
> mux-controls = <&mode_mux_controller>;
> mux-control-names = "mode"
Re: [PATCH v3 1/2] platform/chrome: cros_ec_typec: Use workqueue for port update
Hi Guenter,
Thanks for the comments.
On Mon, Jun 29, 2020 at 2:05 PM Guenter Roeck wrote:
>
> On Mon, Jun 29, 2020 at 9:38 AM Prashant Malani wrote:
> >
> > Use a work queue to call the port update routines, instead of doing it
> > directly in the PD notifier callback. This will prevent other drivers
> > with PD notifier callbacks from being blocked on the port update routine
> > completing.
> >
> > Signed-off-by: Prashant Malani
> > ---
> >
> > Changes in v3:
> > - Use new 100 character line length limit.
> >
> > Changes in v2:
> > - No changes.
> >
> > drivers/platform/chrome/cros_ec_typec.c | 25 -
> > 1 file changed, 20 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > b/drivers/platform/chrome/cros_ec_typec.c
> > index 0c041b79cbba..0beb62bf5adf 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -58,6 +58,7 @@ struct cros_typec_data {
> > /* Array of ports, indexed by port number. */
> > struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
> > struct notifier_block nb;
> > + struct work_struct port_work;
> > };
> >
> > static int cros_typec_parse_port_props(struct typec_capability *cap,
> > @@ -619,18 +620,26 @@ static int cros_typec_get_cmd_version(struct
> > cros_typec_data *typec)
> > return 0;
> > }
> >
> > -static int cros_ec_typec_event(struct notifier_block *nb,
> > - unsigned long host_event, void *_notify)
> > +static void cros_typec_port_work(struct work_struct *work)
> > {
> > - struct cros_typec_data *typec = container_of(nb, struct
> > cros_typec_data,
> > -nb);
> > - int ret, i;
> > + struct cros_typec_data *typec = container_of(work, struct
> > cros_typec_data, port_work);
> > + int ret;
> > + int i;
> >
>
> I know I am getting picky here, but this seems like a personal
> preference change. There is no "one variable declaration per line"
> coding style rule.
Done.
>
> > for (i = 0; i < typec->num_ports; i++) {
> > ret = cros_typec_port_update(typec, i);
> > if (ret < 0)
> > dev_warn(typec->dev, "Update failed for port:
> > %d\n", i);
> > }
> > +}
> > +
> > +
>
> ... but anyway, there should be no double empty lines.
>
Done.
> > +static int cros_ec_typec_event(struct notifier_block *nb,
> > + unsigned long host_event, void *_notify)
> > +{
> > + struct cros_typec_data *typec = container_of(nb, struct
> > cros_typec_data, nb);
> > +
> > + schedule_work(&typec->port_work);
> >
> > return NOTIFY_OK;
> > }
> > @@ -689,6 +698,12 @@ static int cros_typec_probe(struct platform_device
> > *pdev)
> > if (ret < 0)
> > return ret;
> >
> > + INIT_WORK(&typec->port_work, cros_typec_port_work);
> > +
> > + /*
> > +* Safe to call port update here, since we haven't registered the
> > +* PD notifier yet.
> > +*/
> > for (i = 0; i < typec->num_ports; i++) {
> > ret = cros_typec_port_update(typec, i);
> > if (ret < 0)
> > --
> > 2.27.0.212.ge8ba1cc988-goog
> >
Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Add PM support
Hi Guenter,
Thanks for reviewing the patch.
On Mon, Jun 29, 2020 at 9:25 AM Guenter Roeck wrote:
>
> On Mon, Jun 29, 2020 at 9:13 AM Prashant Malani wrote:
> >
> > Define basic suspend resume functions for cros-ec-typec. On suspend, we
> > simply ensure that any pending port update work is completed, and on
> > resume, we re-poll the port state to account for any
> > changes/disconnections that might have occurred during suspend.
> >
> > Signed-off-by: Prashant Malani
> > ---
> >
> > Changes in v2:
> > - Remove #ifdef-ery, add __maybe_unused tag to functions.
> >
> > drivers/platform/chrome/cros_ec_typec.c | 26 +
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > b/drivers/platform/chrome/cros_ec_typec.c
> > index 630170fb2cbe..b2e7e928e788 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -725,11 +725,37 @@ static int cros_typec_probe(struct platform_device
> > *pdev)
> > return ret;
> > }
> >
> > +static int __maybe_unused cros_typec_suspend(struct device *dev)
> > +{
> > + struct cros_typec_data *typec = dev_get_drvdata(dev);
> > +
> > + cancel_work_sync(&typec->port_work);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused cros_typec_resume(struct device *dev)
> > +{
> > + struct cros_typec_data *typec = dev_get_drvdata(dev);
> > +
> > + /* Refresh port state. */
> > + schedule_work(&typec->port_work);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct dev_pm_ops cros_typec_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(cros_typec_suspend, cros_typec_resume)
> > +};
> > +
> > +#define DEV_PM_OPS (&cros_typec_pm_ops)
>
> I don't think this define adds any value.
Right. Sorry, got sloppy here. Will push another patchset. Thanks for
catching this.
Best regards,
>
> > +
> > static struct platform_driver cros_typec_driver = {
> > .driver = {
> > .name = DRV_NAME,
> > .acpi_match_table = ACPI_PTR(cros_typec_acpi_id),
> > .of_match_table = of_match_ptr(cros_typec_of_match),
> > + .pm = DEV_PM_OPS,
> > },
> > .probe = cros_typec_probe,
> > };
> > --
> > 2.27.0.212.ge8ba1cc988-goog
> >
[PATCH v4 1/2] platform/chrome: cros_ec_typec: Use workqueue for port update
Use a work queue to call the port update routines, instead of doing it
directly in the PD notifier callback. This will prevent other drivers
with PD notifier callbacks from being blocked on the port update routine
completing.
Signed-off-by: Prashant Malani
---
Changes in v4:
- Removed extra newline.
- Moved both variable declarations into one line.
Changes in v3:
- Use new 100 character line length limit.
Changes in v2:
- No changes.
drivers/platform/chrome/cros_ec_typec.c | 23 ++-
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index 0c041b79cbba..69c4118e280c 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -58,6 +58,7 @@ struct cros_typec_data {
/* Array of ports, indexed by port number. */
struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
struct notifier_block nb;
+ struct work_struct port_work;
};
static int cros_typec_parse_port_props(struct typec_capability *cap,
@@ -619,18 +620,24 @@ static int cros_typec_get_cmd_version(struct
cros_typec_data *typec)
return 0;
}
-static int cros_ec_typec_event(struct notifier_block *nb,
- unsigned long host_event, void *_notify)
+static void cros_typec_port_work(struct work_struct *work)
{
- struct cros_typec_data *typec = container_of(nb, struct cros_typec_data,
-nb);
- int ret, i;
+ struct cros_typec_data *typec = container_of(work, struct
cros_typec_data, port_work);
+ int i, ret;
for (i = 0; i < typec->num_ports; i++) {
ret = cros_typec_port_update(typec, i);
if (ret < 0)
dev_warn(typec->dev, "Update failed for port: %d\n", i);
}
+}
+
+static int cros_ec_typec_event(struct notifier_block *nb,
+ unsigned long host_event, void *_notify)
+{
+ struct cros_typec_data *typec = container_of(nb, struct
cros_typec_data, nb);
+
+ schedule_work(&typec->port_work);
return NOTIFY_OK;
}
@@ -689,6 +696,12 @@ static int cros_typec_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
+ INIT_WORK(&typec->port_work, cros_typec_port_work);
+
+ /*
+* Safe to call port update here, since we haven't registered the
+* PD notifier yet.
+*/
for (i = 0; i < typec->num_ports; i++) {
ret = cros_typec_port_update(typec, i);
if (ret < 0)
--
2.27.0.212.ge8ba1cc988-goog
[PATCH v4 2/2] platform/chrome: cros_ec_typec: Add PM support
Define basic suspend resume functions for cros-ec-typec. On suspend, we
simply ensure that any pending port update work is completed, and on
resume, we re-poll the port state to account for any
changes/disconnections that might have occurred during suspend.
Signed-off-by: Prashant Malani
Reviewed-by: Guenter Roeck
---
Changes in v4:
- No changes (added Reviewed-by received in v3).
Changes in v3:
- Remove superfluous DEV_PM_OPS #define.
Changes in v2:
- Remove #ifdef-ery, add __maybe_unused tag to functions.
drivers/platform/chrome/cros_ec_typec.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index 69c4118e280c..cb95d190f06a 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -720,11 +720,35 @@ static int cros_typec_probe(struct platform_device *pdev)
return ret;
}
+static int __maybe_unused cros_typec_suspend(struct device *dev)
+{
+ struct cros_typec_data *typec = dev_get_drvdata(dev);
+
+ cancel_work_sync(&typec->port_work);
+
+ return 0;
+}
+
+static int __maybe_unused cros_typec_resume(struct device *dev)
+{
+ struct cros_typec_data *typec = dev_get_drvdata(dev);
+
+ /* Refresh port state. */
+ schedule_work(&typec->port_work);
+
+ return 0;
+}
+
+static const struct dev_pm_ops cros_typec_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(cros_typec_suspend, cros_typec_resume)
+};
+
static struct platform_driver cros_typec_driver = {
.driver = {
.name = DRV_NAME,
.acpi_match_table = ACPI_PTR(cros_typec_acpi_id),
.of_match_table = of_match_ptr(cros_typec_of_match),
+ .pm = &cros_typec_pm_ops,
},
.probe = cros_typec_probe,
};
--
2.27.0.212.ge8ba1cc988-goog
[PATCH v2 2/2] platform/chrome: cros_ec_typec: Add PM support
Define basic suspend resume functions for cros-ec-typec. On suspend, we
simply ensure that any pending port update work is completed, and on
resume, we re-poll the port state to account for any
changes/disconnections that might have occurred during suspend.
Signed-off-by: Prashant Malani
---
Changes in v2:
- Remove #ifdef-ery, add __maybe_unused tag to functions.
drivers/platform/chrome/cros_ec_typec.c | 26 +
1 file changed, 26 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index 630170fb2cbe..b2e7e928e788 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -725,11 +725,37 @@ static int cros_typec_probe(struct platform_device *pdev)
return ret;
}
+static int __maybe_unused cros_typec_suspend(struct device *dev)
+{
+ struct cros_typec_data *typec = dev_get_drvdata(dev);
+
+ cancel_work_sync(&typec->port_work);
+
+ return 0;
+}
+
+static int __maybe_unused cros_typec_resume(struct device *dev)
+{
+ struct cros_typec_data *typec = dev_get_drvdata(dev);
+
+ /* Refresh port state. */
+ schedule_work(&typec->port_work);
+
+ return 0;
+}
+
+static const struct dev_pm_ops cros_typec_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(cros_typec_suspend, cros_typec_resume)
+};
+
+#define DEV_PM_OPS (&cros_typec_pm_ops)
+
static struct platform_driver cros_typec_driver = {
.driver = {
.name = DRV_NAME,
.acpi_match_table = ACPI_PTR(cros_typec_acpi_id),
.of_match_table = of_match_ptr(cros_typec_of_match),
+ .pm = DEV_PM_OPS,
},
.probe = cros_typec_probe,
};
--
2.27.0.212.ge8ba1cc988-goog
Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Add PM support
Hi Enric,
Thanks for reviewing the patch.
On Mon, Jun 29, 2020 at 3:51 AM Enric Balletbo i Serra
wrote:
>
> Hi Prashant,
>
> Thank you for the patch.
>
> On 27/6/20 6:58, Prashant Malani wrote:
> > Define basic suspend resume functions for cros-ec-typec. On suspend, we
> > simply ensure that any pending port update work is completed, and on
> > resume, we re-poll the port state to account for any
> > changes/disconnections that might have occurred during suspend.
> >
> > Signed-off-by: Prashant Malani
> > ---
> > drivers/platform/chrome/cros_ec_typec.c | 34 +
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > b/drivers/platform/chrome/cros_ec_typec.c
> > index 630170fb2cbe..68f15a47450c 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -725,11 +725,45 @@ static int cros_typec_probe(struct platform_device
> > *pdev)
> > return ret;
> > }
> >
> > +#ifdef CONFIG_PM_SLEEP
> > +
> > +static int cros_typec_suspend(struct device *dev)
>
> I'd prefer if you can mark these function as __maybe_unused (to avoid the
> compiler warning) rather than wrapping it in a preprocessor conditional, it
> makes the code a bit more simple.
Done.
>
> > +{
> > + struct cros_typec_data *typec = dev_get_drvdata(dev);
> > +
> > + cancel_work_sync(&typec->port_work);
> > +
> > + return 0;
> > +}
> > +
> > +static int cros_typec_resume(struct device *dev)
>
> __maybe_unused ?
Done.
>
> > +{
> > + struct cros_typec_data *typec = dev_get_drvdata(dev);
> > +
> > + /* Refresh port state. */
> > + schedule_work(&typec->port_work);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct dev_pm_ops cros_typec_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(cros_typec_suspend, cros_typec_resume)
> > +};
> > +
> > +#define DEV_PM_OPS (&cros_typec_pm_ops)
> > +
> > +#else
> > +
> > +#define DEV_PM_OPS NULL
> > +
> > +#endif /* CONFIG_PM_SLEEP */
>
> and remove the ifdefy
>
Done.
> > +
> > static struct platform_driver cros_typec_driver = {
> > .driver = {
> > .name = DRV_NAME,
> > .acpi_match_table = ACPI_PTR(cros_typec_acpi_id),
> > .of_match_table = of_match_ptr(cros_typec_of_match),
> > + .pm = DEV_PM_OPS,
>
> .pm = &cros_typec_pm_ops,
>
> > },
> > .probe = cros_typec_probe,
> > };
> >
>
> Thanks,
> Enric
Re: [PATCH v4 1/2] platform/chrome: cros_ec_typec: Use workqueue for port update
On Mon, Jun 29, 2020 at 2:18 PM Guenter Roeck wrote:
>
> On Mon, Jun 29, 2020 at 2:13 PM Prashant Malani wrote:
> >
> > Use a work queue to call the port update routines, instead of doing it
> > directly in the PD notifier callback. This will prevent other drivers
> > with PD notifier callbacks from being blocked on the port update routine
> > completing.
> >
> > Signed-off-by: Prashant Malani
>
> I can see you are having fun with me :-)
Haha, my apologies , I'm being forgetful :)
>
> > - int ret, i;
> > + int i, ret;
>
> Reviewed-by: Guenter Roeck
>
> > ---
> >
> > Changes in v4:
> > - Removed extra newline.
> > - Moved both variable declarations into one line.
> >
> > Changes in v3:
> > - Use new 100 character line length limit.
> >
> > Changes in v2:
> > - No changes.
> >
> > drivers/platform/chrome/cros_ec_typec.c | 23 ++-
> > 1 file changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > b/drivers/platform/chrome/cros_ec_typec.c
> > index 0c041b79cbba..69c4118e280c 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -58,6 +58,7 @@ struct cros_typec_data {
> > /* Array of ports, indexed by port number. */
> > struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
> > struct notifier_block nb;
> > + struct work_struct port_work;
> > };
> >
> > static int cros_typec_parse_port_props(struct typec_capability *cap,
> > @@ -619,18 +620,24 @@ static int cros_typec_get_cmd_version(struct
> > cros_typec_data *typec)
> > return 0;
> > }
> >
> > -static int cros_ec_typec_event(struct notifier_block *nb,
> > - unsigned long host_event, void *_notify)
> > +static void cros_typec_port_work(struct work_struct *work)
> > {
> > - struct cros_typec_data *typec = container_of(nb, struct
> > cros_typec_data,
> > -nb);
> > - int ret, i;
> > + struct cros_typec_data *typec = container_of(work, struct
> > cros_typec_data, port_work);
> > + int i, ret;
> >
> > for (i = 0; i < typec->num_ports; i++) {
> > ret = cros_typec_port_update(typec, i);
> > if (ret < 0)
> > dev_warn(typec->dev, "Update failed for port:
> > %d\n", i);
> > }
> > +}
> > +
> > +static int cros_ec_typec_event(struct notifier_block *nb,
> > + unsigned long host_event, void *_notify)
> > +{
> > + struct cros_typec_data *typec = container_of(nb, struct
> > cros_typec_data, nb);
> > +
> > + schedule_work(&typec->port_work);
> >
> > return NOTIFY_OK;
> > }
> > @@ -689,6 +696,12 @@ static int cros_typec_probe(struct platform_device
> > *pdev)
> > if (ret < 0)
> > return ret;
> >
> > + INIT_WORK(&typec->port_work, cros_typec_port_work);
> > +
> > + /*
> > +* Safe to call port update here, since we haven't registered the
> > +* PD notifier yet.
> > +*/
> > for (i = 0; i < typec->num_ports; i++) {
> > ret = cros_typec_port_update(typec, i);
> > if (ret < 0)
> > --
> > 2.27.0.212.ge8ba1cc988-goog
> >
Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Add TBT compat support
Hi Heikki,
On Wed, Jun 24, 2020 at 12:20:40PM +0300, Heikki Krogerus wrote:
> On Wed, Jun 24, 2020 at 12:15:20PM +0300, Heikki Krogerus wrote:
> > On Wed, Jun 24, 2020 at 01:09:24AM -0700, Prashant Malani wrote:
> > > Add mux control support for Thunderbolt compatibility mode.
> > >
> > > Suggested-by: Heikki Krogerus
> > > Co-developed-by: Azhar Shaikh
> > > Co-developed-by: Casey Bowman
> > > Signed-off-by: Prashant Malani
> > > ---
> > > drivers/platform/chrome/cros_ec_typec.c | 70 -
> > > 1 file changed, 69 insertions(+), 1 deletion(-)
> >
> > Cool! Can you guys test also USB4 with the attached patch (still work
> > in progress)? It should apply on top of these.
> >
> > The mux driver is still missing USB4 support, but I'll send the
> > patches needed for that right now...
>
> Actually, I'll just attach that one here as well. Let me know if you
> guys can test these.
Thanks for the patches. Will try them out today and compare the PMC IPC
buffers against what extcon is doing.
Best regards,
-Prashant
>
> thanks,
>
> --
> heikki
> From 396bd399ac815165ec4992739d45d52ecf234acc Mon Sep 17 00:00:00 2001
> From: Heikki Krogerus
> Date: Wed, 3 Jun 2020 17:00:14 +0300
> Subject: [PATCH] usb: typec: intel_pmc_mux: Add support for USB4
>
> The PMC mux-agent can be used also when Enter_USB is used in
> order to enter USB4 mode. The mux-agent does not have USB4
> specific message, but instead needs to be put into TBT
> alternate mode also with USB4. That is OK as the controller
> is in any case the same with TBT3 and USB4.
>
> Signed-off-by: Heikki Krogerus
> ---
> drivers/usb/typec/mux/intel_pmc_mux.c | 65 +++
> 1 file changed, 56 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c
> b/drivers/usb/typec/mux/intel_pmc_mux.c
> index 70ddc9d6d49e4..6d223bd360b8e 100644
> --- a/drivers/usb/typec/mux/intel_pmc_mux.c
> +++ b/drivers/usb/typec/mux/intel_pmc_mux.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -227,6 +228,41 @@ pmc_usb_mux_tbt(struct pmc_usb_port *port, struct
> typec_mux_state *state)
> return pmc_usb_command(port, (void *)&req, sizeof(req));
> }
>
> +static int
> +pmc_usb_mux_usb4(struct pmc_usb_port *port, struct typec_mux_state *state)
> +{
> + u32 eudo = *(u32 *)state->data;
> + struct altmode_req req = { };
> + u8 cable_speed;
> +
> + req.usage = PMC_USB_ALT_MODE;
> + req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
> + req.mode_type = PMC_USB_MODE_TYPE_TBT << PMC_USB_MODE_TYPE_SHIFT;
> +
> + /* USB4 Mode */
> + req.mode_data = PMC_USB_ALTMODE_FORCE_LSR;
> + req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK;
> +
> + req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_SHIFT;
> + req.mode_data |= (port->role - 1) << PMC_USB_ALTMODE_UFP_SHIFT;
> +
> + switch ((eudo & EUDO_CABLE_TYPE_MASK) >> EUDO_CABLE_TYPE_SHIFT) {
> + case EUDO_CABLE_TYPE_PASSIVE:
> + break;
> + case EUDO_CABLE_TYPE_OPTICAL:
> + req.mode_data |= PMC_USB_ALTMODE_CABLE_TYPE;
> + /* fall through */
> + default:
> + req.mode_data |= PMC_USB_ALTMODE_ACTIVE_CABLE;
> + break;
> + }
> +
> + cable_speed = (eudo & EUDO_CABLE_SPEED_MASK) >> EUDO_CABLE_SPEED_SHIFT;
> + req.mode_data |= PMC_USB_ALTMODE_CABLE_SPD(cable_speed);
> +
> + return pmc_usb_command(port, (void *)&req, sizeof(req));
> +}
> +
> static int pmc_usb_mux_safe_state(struct pmc_usb_port *port)
> {
> u8 msg;
> @@ -268,17 +304,28 @@ pmc_usb_mux_set(struct typec_mux *mux, struct
> typec_mux_state *state)
> {
> struct pmc_usb_port *port = typec_mux_get_drvdata(mux);
>
> - if (!state->alt)
> - return 0;
> -
> if (state->mode == TYPEC_STATE_SAFE)
> return pmc_usb_mux_safe_state(port);
> -
> - switch (state->alt->svid) {
> - case USB_TYPEC_TBT_SID:
> - return pmc_usb_mux_tbt(port, state);
> - case USB_TYPEC_DP_SID:
> - return pmc_usb_mux_dp(port, state);
> + if (state->mode == TYPEC_STATE_USB)
> + return pmc_usb_connect(port);
> +
> + if (state->alt) {
> + switch (state->alt->svid) {
> + case USB_TYPEC_TBT_SID:
> + return pmc_usb_mux_tbt(port, state);
> +
Re: [PATCH 0/4] platform/chrome: typec: Add mux support
Thanks Enric! On Thu, Jun 25, 2020 at 4:56 AM Enric Balletbo i Serra wrote: > > Hi Prashant, > > On 28/5/20 13:36, Prashant Malani wrote: > > This series adds mux control support for USB and DP alternate modes on > > devices using the cros-ec-typec driver with Type C switch handles > > provided by firmware bindings. > > > > The first patch imports some recent updates to the > > EC_CMD_USB_PD_MUX_INFO bit fields from the Chrome EC > > code base[1], while the rest add the aforementioned functionality. > > > > This series depends on the following patch : > > https://lkml.org/lkml/2020/5/19/1219 > > > > [1] : > > https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/include/ec_commands.h > > > > Prashant Malani (4): > > platform/chrome: cros_ec: Update mux state bits > > platform/chrome: typec: Register PD CTRL cmd v2 > > platform/chrome: typec: Add USB mux control > > platform/chrome: typec: Support DP alt mode > > > > drivers/platform/chrome/cros_ec_typec.c | 190 -- > > .../linux/platform_data/cros_ec_commands.h| 14 +- > > 2 files changed, 187 insertions(+), 17 deletions(-) > > > > Tweaked a bit the subject, s/typec/cros_ec_typec/ and queued the four patches > for 5.9. > > Thanks, > Enric
Re: [PATCH 1/2] dt-bindings: chrome: Add cros-ec-typec mux props
Hi Heikki and Rob,
(trimming text):
On Mon, Jun 15, 2020 at 04:22:07PM +0300, Heikki Krogerus wrote:
> On Fri, Jun 12, 2020 at 10:34:06AM -0700, Prashant Malani wrote:
> > Hi Rob,
> > > Yes, but let's stop calling it a mux. It's a "USB Type C signal routing
> > > blob".
> >
> > Ack.
> >
> > Let's go with "-switch" ? That's what the connector class uses and it
> > conveys the meaning (unless that is a reserved keyword in DT).
>
> Just as a clarification here, we should not be even talking about
> signal routing here. We are talking about functions that an external
> components perform from the connector's perspective. It depends on the
> platform does that function require changing the routing of the lines
> from the connector. For example, data role swapping does not require
> muxing on platforms that have single dual-role USB controller, but
> platforms that have separate IPs for the USB host and USB device
> controllers will need a mux.
>
> Note, that it is even possible that switching from USB to DisplayPort
> mode does not require any pin reconfiguration from the mux, even if
> the platform has one, because the PHY can be shared between USB3 and
> DP. Then the PHY just needs to be told that it is now in DP mode when
> DP alt mode is entered instead of the mux.
>
> > > > Would this block the addition of the "*-switch" properties? IIUC the
> > > > two are related but not dependent on each other.
> > > >
> > > > The *-switch properties are phandles which the Type C connector class
> > > > framework expects (and uses to get handles to those switches).
> > > > These would point to the "mux" or "group of mux" abstractions as noted
> > > > earlier.
> > >
> > > You don't need them though. Walk the graph. You get the connector
> > > port@1 remote endpoint and then get its parent.
> > >
> >
> > I see; would it be something along the lines of this? (DT example
> > follows; search for "example_end" to jump to bottom):
>
> I just realized that you have in practice placed the mux-agent into
> the graph below, right? That we can not do, because it is not
> physically connected to the connector.
Is this a requirement? I read through the graph.txt file [1] and I couldn't
find anything stating that a physical connection between two devices was
required (but I may be misinterpreting that document)
[1]:
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/graph.txt
>
> >
> >
> >
> >
> > Would this be conformant to OF graph and usb-connector bindings
> > requirements? We'll certainly send out a format PATCH/RFC series for
> > this, but I was hoping to gauge whether we're thinking along the right
> > lines.
> >
> > So, in effect this would mean:
> > - New bindings(and compatible strings) to be added for:
> > typec-{orientation,data-role,mode}-switch.
> > - Handling in Type C connector class to parse switches from OF graph.
> > - Handling in Type C connector class for distinct switches for port@1
> > (SS lines) and port@2 (SBU lines).
> >
> > The only thing I'm confused about is how we can define these switch
> > remote-endpoint bindings in usb-connector.yaml; the port can have an
> > remote-endpoint, but can we specify what the parent of the remote-endpoint
> > should have as a compatible string? Or do we not need to?
>
> thanks,
>
> --
> heikki
Best regards,
-Prashant
Re: [PATCH v3 1/2] dt-bindings: chrome: Add cros-ec-typec mux props
Hi, >From the discussion in [1], it looks like reconciling ACPI and DT bindings for these properties to be identical is challenging. Let's drop this patch, and move forward with the Patch 2/2 in this series [2] alone. We'll continue the discussion about how we can add these properties to devicetree bindings in a conformant way. Thanks everyone, -Prashant, [1]: https://lore.kernel.org/lkml/CAL_Jsq+ORkzHchpD0qsH97zDJzXGj3jWy8=orxsvhnqd4kr...@mail.gmail.com/T/#t [2]: https://lkml.org/lkml/2020/5/19/1219 (Trimming text); On Tue, May 19, 2020 at 02:46:02PM -0700, Prashant Malani wrote: > Add properties for mode, orientation and USB data role switches for > Type C connectors. When available, these will allow the Type C connector > class port driver to configure the various switches according to USB PD > information (like orientation, alt mode etc.) provided by the Chrome OS > EC controller. > > Signed-off-by: Prashant Malani > Acked-by: Benson Leung > --- > > Changes in v3: > - Fixed Acked-by tag typo. > > Changes in v2: > - Added more text to the switch descriptions, explaining their purpose, > and relation to the Type C connector class framework. > > .../bindings/chrome/google,cros-ec-typec.yaml | 40 ++- > 1 file changed, 39 insertions(+), 1 deletion(-) >
Re: [PATCH 1/2] dt-bindings: chrome: Add cros-ec-typec mux props
Hi Rob,
Thanks as always for your help in reviewing this proposal!
Kindly see inline
(Trimming text);
On Thu, Jun 11, 2020 at 02:00:47PM -0600, Rob Herring wrote:
> On Wed, Jun 10, 2020 at 11:49 AM Prashant Malani wrote:
> >
> > Hi Rob,
> >
> > On Wed, Jun 10, 2020 at 9:53 AM Rob Herring wrote:
> > >
> > > > On Tue, Jun 09, 2020 at 04:57:40PM -0700, Prashant Malani wrote:
> >
> > I think the updated example handles this grouping (port@1 going to a
> > "SS mux") although as you said it should probably be a group of muxes,
> > but I think the example illustrates the point. Is that assessment
> > correct?
>
> Yes, but let's stop calling it a mux. It's a "USB Type C signal routing blob".
Ack.
Let's go with "-switch" ? That's what the connector class uses and it
conveys the meaning (unless that is a reserved keyword in DT).
>
> > Would this block the addition of the "*-switch" properties? IIUC the
> > two are related but not dependent on each other.
> >
> > The *-switch properties are phandles which the Type C connector class
> > framework expects (and uses to get handles to those switches).
> > These would point to the "mux" or "group of mux" abstractions as noted
> > earlier.
>
> You don't need them though. Walk the graph. You get the connector
> port@1 remote endpoint and then get its parent.
>
I see; would it be something along the lines of this? (DT example
follows; search for "example_end" to jump to bottom):
connector@0 {
compatible = "usb-c-connector";
reg = <0>;
power-role = "dual";
data-role = "dual";
try-power-role = "source";
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
usb_con_hs: endpoint {
remote-endpoint = <&foo_usb_hs_controller>;
};
};
port@1 {
reg = <1>;
#address-cells = <1>;
#size-cells = <0>;
usb_con0_ss_mode: endpoint@0 {
reg = <0>
remote-endpoint = <&mode_switch_ss_in>;
};
usb_con0_ss_orientation: endpoint@1 {
reg = <1>
remote-endpoint = <&orientation_switch_ss_in>;
};
usb_con0_ss_data_role: endpoint@2 {
reg = <2>
remote-endpoint = <&data_role_switch_in>;
};
};
port@2 {
reg = <2>;
#address-cells = <1>;
#size-cells = <0>;
usb_con0_sbu_mode: endpoint@0 {
reg = <0>
remote-endpoint = <&mode_switch_sbu_in>;
};
usb_con0_sbu_orientation: endpoint@1 {
reg = <1>
remote-endpoint = <&orientation_switch_sbu_in>;
};
};
};
};
mode_switch {
compatible = "typec-mode-switch";
mux-controls = <&mode_mux_controller>;
mux-control-names = "mode";
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
mode_switch_ss_in: endpoint {
remote-endpoint = <&usb_con0_ss_mode>
};
};
port@1 {
reg = <1>;
mode_switch_out_usb3: endpoint {
remote-endpoint = <&usb3_0_ep>
};
};
port@2 {
reg = <2>;
mode_switch_out_dp: endpoint {
remote-endpoint = <&dp0_out_ep>
};
};
port@3 {
reg = <3>;
mode_switch_sbu_in: endpoint {
remote-endpoint = <&usb_con0_sbu_mode>
};
};
// ... other ports similarly defined.
};
orientation_switch {
compatible = "typec-orientation-switch";
mux-controls = <&orientation_mux_controller>;
mux-control-names = "orientation";
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
orientation_switch_ss_in: endpoint {
remote-endpoint = <&usb_con0_ss_orientation>
};
};
port@1
reg = <1>;
orientation_switch_sbu_in: endpoint {
remote-endpoint = <&usb_con0_sbu_orientation>
};
};
// ... other ports similarly defined.
};
data_role_switch {
compatible = "typec-data-role-switch";
mux-controls = <&data_role_switch_controller>;
mux-control-names = "data_role";
[PATCH 0/4] platform/chrome: typec: Add mux support
This series adds mux control support for USB and DP alternate modes on devices using the cros-ec-typec driver with Type C switch handles provided by firmware bindings. The first patch imports some recent updates to the EC_CMD_USB_PD_MUX_INFO bit fields from the Chrome EC code base[1], while the rest add the aforementioned functionality. This series depends on the following patch : https://lkml.org/lkml/2020/5/19/1219 [1] : https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/include/ec_commands.h Prashant Malani (4): platform/chrome: cros_ec: Update mux state bits platform/chrome: typec: Register PD CTRL cmd v2 platform/chrome: typec: Add USB mux control platform/chrome: typec: Support DP alt mode drivers/platform/chrome/cros_ec_typec.c | 190 -- .../linux/platform_data/cros_ec_commands.h| 14 +- 2 files changed, 187 insertions(+), 17 deletions(-) -- 2.27.0.rc0.183.gde8f92d652-goog
[PATCH 1/4] platform/chrome: cros_ec: Update mux state bits
Sync the EC_CMD_USB_PD_MUX_INFO mux state bit fields with the Chrome EC
code base. The newly added bit fields will be used for cros-ec-typec mux
control.
Signed-off-by: Prashant Malani
---
include/linux/platform_data/cros_ec_commands.h | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/linux/platform_data/cros_ec_commands.h
b/include/linux/platform_data/cros_ec_commands.h
index 69210881ebac..a7b0fc440c35 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -5207,11 +5207,15 @@ struct ec_params_usb_pd_mux_info {
} __ec_align1;
/* Flags representing mux state */
-#define USB_PD_MUX_USB_ENABLED BIT(0) /* USB connected */
-#define USB_PD_MUX_DP_ENABLEDBIT(1) /* DP connected */
-#define USB_PD_MUX_POLARITY_INVERTED BIT(2) /* CC line Polarity inverted */
-#define USB_PD_MUX_HPD_IRQ BIT(3) /* HPD IRQ is asserted */
-#define USB_PD_MUX_HPD_LVL BIT(4) /* HPD level is asserted */
+#define USB_PD_MUX_NONE 0 /* Open switch */
+#define USB_PD_MUX_USB_ENABLEDBIT(0) /* USB connected */
+#define USB_PD_MUX_DP_ENABLED BIT(1) /* DP connected */
+#define USB_PD_MUX_POLARITY_INVERTED BIT(2) /* CC line Polarity inverted */
+#define USB_PD_MUX_HPD_IRQBIT(3) /* HPD IRQ is asserted */
+#define USB_PD_MUX_HPD_LVLBIT(4) /* HPD level is asserted */
+#define USB_PD_MUX_SAFE_MODE BIT(5) /* DP is in safe mode */
+#define USB_PD_MUX_TBT_COMPAT_ENABLED BIT(6) /* TBT compat enabled */
+#define USB_PD_MUX_USB4_ENABLED BIT(7) /* USB4 enabled */
struct ec_response_usb_pd_mux_info {
uint8_t flags; /* USB_PD_MUX_*-encoded USB mux state */
--
2.27.0.rc0.183.gde8f92d652-goog
[PATCH 2/4] platform/chrome: typec: Register PD CTRL cmd v2
Recognize EC_CMD_USB_PD_CONTROL command version 2. This is necessary in
order to process Type C mux information (like DP alt mode pin
configuration), which is needed by the Type C Connector class API to
configure the Type C muxes correctly
While we are here, rename the struct member storing this version number
from cmd_ver to pd_ctrl_ver, which more accurately reflects what is
being stored.
Also, slightly change the logic for calling
cros_typec_set_port_params_*(). Now, v0 is called when pd_ctrl_ver is 0,
and v1 is called otherwise.
Signed-off-by: Prashant Malani
---
drivers/platform/chrome/cros_ec_typec.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index 6e79f917314b..d69a88464cef 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -37,7 +37,7 @@ struct cros_typec_data {
struct device *dev;
struct cros_ec_device *ec;
int num_ports;
- unsigned int cmd_ver;
+ unsigned int pd_ctrl_ver;
/* Array of ports, indexed by port number. */
struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
struct notifier_block nb;
@@ -340,7 +340,7 @@ static int cros_typec_port_update(struct cros_typec_data
*typec, int port_num)
req.mux = USB_PD_CTRL_MUX_NO_CHANGE;
req.swap = USB_PD_CTRL_SWAP_NONE;
- ret = cros_typec_ec_command(typec, typec->cmd_ver,
+ ret = cros_typec_ec_command(typec, typec->pd_ctrl_ver,
EC_CMD_USB_PD_CONTROL, &req, sizeof(req),
&resp, sizeof(resp));
if (ret < 0)
@@ -351,7 +351,7 @@ static int cros_typec_port_update(struct cros_typec_data
*typec, int port_num)
dev_dbg(typec->dev, "Polarity %d: 0x%hhx\n", port_num, resp.polarity);
dev_dbg(typec->dev, "State %d: %s\n", port_num, resp.state);
- if (typec->cmd_ver == 1)
+ if (typec->pd_ctrl_ver != 0)
cros_typec_set_port_params_v1(typec, port_num, &resp);
else
cros_typec_set_port_params_v0(typec, port_num,
@@ -374,13 +374,15 @@ static int cros_typec_get_cmd_version(struct
cros_typec_data *typec)
if (ret < 0)
return ret;
- if (resp.version_mask & EC_VER_MASK(1))
- typec->cmd_ver = 1;
+ if (resp.version_mask & EC_VER_MASK(2))
+ typec->pd_ctrl_ver = 2;
+ else if (resp.version_mask & EC_VER_MASK(1))
+ typec->pd_ctrl_ver = 1;
else
- typec->cmd_ver = 0;
+ typec->pd_ctrl_ver = 0;
dev_dbg(typec->dev, "PD Control has version mask 0x%hhx\n",
- typec->cmd_ver);
+ typec->pd_ctrl_ver);
return 0;
}
--
2.27.0.rc0.183.gde8f92d652-goog
[PATCH 3/4] platform/chrome: typec: Add USB mux control
Add support to configure various Type C switches appropriately using the
Type C connector class API, when the Chrome OS EC informs the AP that
the USB operating mode has been entered.
Signed-off-by: Prashant Malani
---
drivers/platform/chrome/cros_ec_typec.c | 100 +++-
1 file changed, 97 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index d69a88464cef..9ebf9abed16f 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -30,6 +31,10 @@ struct cros_typec_port {
struct typec_switch *ori_sw;
struct typec_mux *mux;
struct usb_role_switch *role_sw;
+
+ /* Variables keeping track of switch state. */
+ struct typec_mux_state state;
+ uint8_t mux_flags;
};
/* Platform-specific data for the Chrome OS EC Type C controller. */
@@ -264,6 +269,23 @@ static int cros_typec_add_partner(struct cros_typec_data
*typec, int port_num,
return ret;
}
+static void cros_typec_remove_partner(struct cros_typec_data *typec,
+int port_num)
+{
+ struct cros_typec_port *port = typec->ports[port_num];
+
+ port->state.alt = NULL;
+ port->state.mode = TYPEC_STATE_USB;
+ port->state.data = NULL;
+
+ usb_role_switch_set_role(port->role_sw, USB_ROLE_NONE);
+ typec_switch_set(port->ori_sw, TYPEC_ORIENTATION_NONE);
+ typec_mux_set(port->mux, &port->state);
+
+ typec_unregister_partner(port->partner);
+ port->partner = NULL;
+}
+
static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
int port_num, struct ec_response_usb_pd_control *resp)
{
@@ -317,16 +339,69 @@ static void cros_typec_set_port_params_v1(struct
cros_typec_data *typec,
} else {
if (!typec->ports[port_num]->partner)
return;
+ cros_typec_remove_partner(typec, port_num);
+ }
+}
+
+static int cros_typec_get_mux_info(struct cros_typec_data *typec, int port_num,
+ struct ec_response_usb_pd_mux_info *resp)
+{
+ struct ec_params_usb_pd_mux_info req = {
+ .port = port_num,
+ };
+
+ return cros_typec_ec_command(typec, 0, EC_CMD_USB_PD_MUX_INFO, &req,
+sizeof(req), resp, sizeof(*resp));
+}
+
+static int cros_typec_usb_safe_state(struct cros_typec_port *port)
+{
+ port->state.mode = TYPEC_STATE_SAFE;
+
+ return typec_mux_set(port->mux, &port->state);
+}
- typec_unregister_partner(typec->ports[port_num]->partner);
- typec->ports[port_num]->partner = NULL;
+int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
+uint8_t mux_flags)
+{
+ struct cros_typec_port *port = typec->ports[port_num];
+ enum typec_orientation orientation;
+ int ret;
+
+ if (!port->partner)
+ return 0;
+
+ if (mux_flags & USB_PD_MUX_POLARITY_INVERTED)
+ orientation = TYPEC_ORIENTATION_REVERSE;
+ else
+ orientation = TYPEC_ORIENTATION_NORMAL;
+
+ ret = typec_switch_set(port->ori_sw, orientation);
+ if (ret)
+ return ret;
+
+ port->state.alt = NULL;
+ port->state.mode = TYPEC_STATE_USB;
+
+ if (mux_flags & USB_PD_MUX_SAFE_MODE)
+ ret = cros_typec_usb_safe_state(port);
+ else if (mux_flags & USB_PD_MUX_USB_ENABLED)
+ ret = typec_mux_set(port->mux, &port->state);
+ else {
+ dev_info(typec->dev,
+"Unsupported mode requested, mux flags: %x\n",
+mux_flags);
+ ret = -ENOTSUPP;
}
+
+ return ret;
}
static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
{
struct ec_params_usb_pd_control req;
struct ec_response_usb_pd_control_v1 resp;
+ struct ec_response_usb_pd_mux_info mux_resp;
int ret;
if (port_num < 0 || port_num >= typec->num_ports) {
@@ -357,7 +432,26 @@ static int cros_typec_port_update(struct cros_typec_data
*typec, int port_num)
cros_typec_set_port_params_v0(typec, port_num,
(struct ec_response_usb_pd_control *) &resp);
- return 0;
+ /* Update the switches if they exist, according to requested state */
+ ret = cros_typec_get_mux_info(typec, port_num, &mux_resp);
+ if (ret < 0) {
+ dev_warn(typec->dev,
+"Failed to get mux info for port: %d, err = %d\n",
+port_nu
[PATCH 4/4] platform/chrome: typec: Support DP alt mode
Handle Chrome EC mux events to configure on-board muxes correctly while
entering DP alternate mode. Since we don't surface SVID and VDO
information regarding the DP alternate mode, configure the Type C
muxes directly from the port driver. Later, when mode discovery
information is correctly surfaced to the driver, we can register the DP
alternate mode driver and let it handle the mux configuration.
Also, modify the struct_typec_state state management to account for the
addition of DP alternate mode.
Signed-off-by: Prashant Malani
---
drivers/platform/chrome/cros_ec_typec.c | 90 ++---
1 file changed, 80 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index 9ebf9abed16f..509fc761906b 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -15,11 +15,18 @@
#include
#include
#include
+#include
#include
#include
#define DRV_NAME "cros-ec-typec"
+/* Supported alt modes. */
+enum {
+ CROS_EC_ALTMODE_DP = 0,
+ CROS_EC_ALTMODE_MAX,
+};
+
/* Per port data. */
struct cros_typec_port {
struct typec_port *port;
@@ -35,6 +42,9 @@ struct cros_typec_port {
/* Variables keeping track of switch state. */
struct typec_mux_state state;
uint8_t mux_flags;
+
+ /* Port alt modes. */
+ struct typec_altmode p_altmode[CROS_EC_ALTMODE_MAX];
};
/* Platform-specific data for the Chrome OS EC Type C controller. */
@@ -142,6 +152,24 @@ static void cros_unregister_ports(struct cros_typec_data
*typec)
}
}
+/*
+ * Fake the alt mode structs until we actually start registering Type C port
+ * and partner alt modes.
+ */
+static void cros_typec_register_port_altmodes(struct cros_typec_data *typec,
+ int port_num)
+{
+ struct cros_typec_port *port = typec->ports[port_num];
+
+ /* All PD capable CrOS devices are assumed to support DP altmode. */
+ port->p_altmode[CROS_EC_ALTMODE_DP].svid = USB_TYPEC_DP_SID;
+ port->p_altmode[CROS_EC_ALTMODE_DP].mode = USB_TYPEC_DP_MODE;
+
+ port->state.alt = NULL;
+ port->state.mode = TYPEC_STATE_USB;
+ port->state.data = NULL;
+}
+
static int cros_typec_init_ports(struct cros_typec_data *typec)
{
struct device *dev = typec->dev;
@@ -205,6 +233,8 @@ static int cros_typec_init_ports(struct cros_typec_data
*typec)
if (ret)
dev_dbg(dev, "No switch control for port %d\n",
port_num);
+
+ cros_typec_register_port_altmodes(typec, port_num);
}
return 0;
@@ -361,8 +391,46 @@ static int cros_typec_usb_safe_state(struct
cros_typec_port *port)
return typec_mux_set(port->mux, &port->state);
}
+/* Spoof the VDOs that were likely communicated by the partner. */
+static int cros_typec_enable_dp(struct cros_typec_data *typec,
+ int port_num,
+ struct ec_response_usb_pd_control_v2 *pd_ctrl)
+{
+ struct cros_typec_port *port = typec->ports[port_num];
+ struct typec_displayport_data dp_data;
+ int ret;
+
+ if (typec->pd_ctrl_ver < 2) {
+ dev_err(typec->dev,
+ "PD_CTRL version too old: %d\n", typec->pd_ctrl_ver);
+ return -ENOTSUPP;
+ }
+
+ /* Status VDO. */
+ dp_data.status = DP_STATUS_ENABLED;
+ if (port->mux_flags & USB_PD_MUX_HPD_IRQ)
+ dp_data.status |= DP_STATUS_IRQ_HPD;
+ if (port->mux_flags & USB_PD_MUX_HPD_LVL)
+ dp_data.status |= DP_STATUS_HPD_STATE;
+
+ /* Configuration VDO. */
+ dp_data.conf = DP_CONF_SET_PIN_ASSIGN(pd_ctrl->dp_mode);
+ if (!port->state.alt) {
+ port->state.alt = &port->p_altmode[CROS_EC_ALTMODE_DP];
+ ret = cros_typec_usb_safe_state(port);
+ if (ret)
+ return ret;
+ }
+
+ port->state.data = &dp_data;
+ port->state.mode = TYPEC_MODAL_STATE(ffs(pd_ctrl->dp_mode));
+
+ return typec_mux_set(port->mux, &port->state);
+}
+
int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
-uint8_t mux_flags)
+uint8_t mux_flags,
+struct ec_response_usb_pd_control_v2 *pd_ctrl)
{
struct cros_typec_port *port = typec->ports[port_num];
enum typec_orientation orientation;
@@ -380,14 +448,15 @@ int cros_typec_configure_mux(struct cros_typec_data
*typec, int port_num,
if (ret)
return ret;
- port->state.alt = NULL;
- port->state.mode = TYPEC_STATE_USB;
-
- if (mux_flags & USB_PD_MUX_
Re: [PATCH v3 1/2] dt-bindings: chrome: Add cros-ec-typec mux props
Hi Rob,
Would you prefer these switches to be defined in the
usb-connector.yaml bindings file?
If there are no other concerns, I can push a fresh version of the
patch with the properties defined in usb-connector.yaml.
Thanks,
On Tue, May 19, 2020 at 2:46 PM Prashant Malani wrote:
>
> Add properties for mode, orientation and USB data role switches for
> Type C connectors. When available, these will allow the Type C connector
> class port driver to configure the various switches according to USB PD
> information (like orientation, alt mode etc.) provided by the Chrome OS
> EC controller.
>
> Signed-off-by: Prashant Malani
> Acked-by: Benson Leung
> ---
>
> Changes in v3:
> - Fixed Acked-by tag typo.
>
> Changes in v2:
> - Added more text to the switch descriptions, explaining their purpose,
> and relation to the Type C connector class framework.
>
> .../bindings/chrome/google,cros-ec-typec.yaml | 40 ++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git
> a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> index 6d7396ab8bee..800c005a0e44 100644
> --- a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> +++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> @@ -21,7 +21,34 @@ properties:
> const: google,cros-ec-typec
>
>connector:
> -$ref: /schemas/connector/usb-connector.yaml#
> +allOf:
> + - $ref: /schemas/connector/usb-connector.yaml#
> + - type: object
> +properties:
> + mode-switch:
> +description: Reference to a DT node for the USB Type C
> Multiplexer
> + for this connector. This switch controls the data lines routing
> + for this connector for various operation modes, including
> + Alternate Modes. This switch is assumed registered with the
> Type C
> + connector class framework by its driver. The Type C connector
> + class framework assumes that the mode switch property uses this
> + name.
> +
> + orientation-switch:
> +description: Reference to a DT node for the USB Type C
> orientation
> + switch for this connector. This switch controls routing the
> + correct data pairs depending on the cable plug orientation from
> + this connector to the USB / Alternate Mode controllers. This
> + switch is assumed registered with the Type C connector class
> + framework by its driver. The Type C connector class framework
> + assumes that the orientation switch property uses this name.
> +
> + usb-role-switch:
> +description: Reference to a DT node for the USB Data role switch
> + for this connector. This switch is assumed registered with the
> + Type C connector class framework by its driver. The Type C
> + connector class framework assumes that the USB role switch
> + property uses this name.
>
> required:
>- compatible
> @@ -49,6 +76,17 @@ examples:
> data-role = "dual";
> try-power-role = "source";
>};
> +
> + connector@1 {
> +compatible = "usb-c-connector";
> +reg = <1>;
> +power-role = "dual";
> +data-role = "host";
> +try-power-role = "source";
> +mode-switch = <&typec_mux>;
> +orientation-switch = <&typec_orientation_switch>;
> +usb-role-switch = <&typec_mux>;
> + };
> };
>};
> };
> --
> 2.26.2.761.g0e0b3e54be-goog
>
Re: [PATCH 1/2] dt-bindings: chrome: Add cros-ec-typec mux props
Hi Rob,
Thanks for reviewing the patch! Kindly see inline:
On Fri, May 29, 2020 at 2:55 PM Rob Herring wrote:
>
> > > " Reference to a DT node for the USB Type C Multiplexer controlling the
> > > data lines routing for this connector. This switch is assumed registered
> > > with the Type C connector class framework, which requires it to be named
> > > this way."
> > > >
> > > > > + mode-switch:
> > > > > +description: Reference to a DT node for the USB Type C
> > > > > Multiplexer
> > > > > + controlling the data lines routing for this connector.
> > > >
> > > > This is for alternate mode muxing I presume.
> > >
> > > Yes, that's right.
> > > >
> > > > We already have a mux-control binding. Why not use that here?
> > >
> > > Heikki might be able to offer more insight into why this is the case,
> > > since the connector class framework seems to expect a phandle and for
> > > the device driver to implement a "set" command. Heikki, would you happen
> > > to know?
> >
> > The mode-switch here would actually represent the "consumer" part in
> > the mux-control bindings. So the mux-controls would describe the
> > relationship between the "mode-switch" and the mux controller(s),
> > while the mode-switch property describes the relationship between
> > something like USB Type-C Port Manager (or this cros_ec function) and
> > the "mux consumer".
>
> The "USB Type-C Port Manager" is not just the parent node in your case?
>
> Can you point me to what you expect your DT to look like showing the
> mode switch node, the connector, the USB host(s), and the DP/HDMI
> bridge/output?
Caveat: I'm not a DT expert and not well-versed with the mux-control
bindings, so Heikki may be able to describe these better.
That said, here is my attempt to show the nodes you requested, cobbled
together from the Rockchip rk3399 DTSI[1] and
swboyd's connector binding example [2].
Nodes truncated and unrelated fields omitted in the interest of brevity:
// Chrome OS EC Type C Port Manager.
typec {
compatible = "google,cros-ec-typec";
#address-cells = <1>;
#size-cells = <0>;
connector@0 {
compatible = "usb-c-connector";
reg = <0>;
power-role = "dual";
data-role = "dual";
try-power-role = "source";
mode-switch = <&foo_mux>;
// Other switches can point to the same mux.
};
};
// Mux switch
// TODO: Can possibly embed this in the PHY controller node itself?
foo_mux {
compatible = "vendor,typec-mux";
mux-gpios = <&gpio_controller 23 GPIO_ACTIVE_HIGH>;
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
mux_dp_in: endpoint {
remote-endpoint = <&dp_phy_out>;
};
};
port@1 {
reg = <1>;
mux_usb_in: endpoint1 {
remote-endpoint = <&usb3_phy_out>;
};
};
};
};
// Type C PHY Controller.
tcphy0: phy@ff7c {
compatible = "rockchip,rk3399-typec-phy";
reg = <0x0 0xff7c 0x0 0x4>;
...
tcphy0_dp: phy@dc0 {
compatible = "soc,dp-phy";
reg = <0xdc0 0x1000>;
ports {
port@0 {
reg = <0>;
dp_phy_out: endpoint {
remote-endpoint = <&mux_dp_in>;
};
};
};
};
tcphy0_usb3: phy@db0 {
compatible = "soc,usb3-phy";
reg = <0xdb0 0x1000>;
ports {
port@0 {
reg = <0>;
usb3_phy_out: endpoint {
remote-endpoint = <&mux_usb3_in>;
};
};
};
};
};
// USB3 Host controller
usbdrd3_0: usb@fe80 {
compatible = "rockchip,rk3399-dwc3";
#address-cells = <2>;
#size-cells = <2>;
clocks = ...;
clock-names = ...;
status = "disabled";
usbdrd_dwc3_0: usb@fe80 {
compatible = "snps,dwc3";
reg = <0x0 0xfe80 0x0 0x10>;
interrupts = ;
clocks = ...;
clock-names = ...;
dr_mode = "otg";
phys = <&tcphy0_usb3>;
phy-names = "usb3-phy";
phy_type = "utmi_wide";
power-domains = <&power RK3399_PD_USB3>;
status = "disabled";
};
};
// DP controller
cdn_dp: dp@fec0 {
compatible = "rockchip,rk3399-cdn-dp";
reg = <0x0 0xfec0 0x0 0x10>;
interrupts = ...;
clocks = ...;
clock-names = ...;
phys = <&tcphy0_dp>;
...
ports {
dp_in: port {
#address-cells = <1>;
#size-cells = <0>;
dp_in_vopb: endpoint@0 {
reg = <0>;
remote-endpoint = <&vopb_out_dp>;
};
dp_in_vopl: endpoint@1 {
reg = <1>;
remote-endpoint = <&vopl_out_dp>;
};
};
};
};
[1] :
https://chromium.googlesource.com/chromiumos/th
