[PATCH v3 2/2] platform/chrome: cros_ec_typec: Handle hard reset

[Prashant Malani]

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 v3 1/2] platform/chrome: cros_ec: Add Type C hard reset

[Prashant Malani]

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

Re: [PATCH v2 1/2] platform/chrome: cros_ec: Add Type C hard reset

[Prashant Malani]

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] platform/chrome: cros_ec_typec: Track port role

[Prashant Malani]

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, );
if (ret)
dev_warn(typec->dev, "Configure muxes failed, err = %d\n", ret);
-- 
2.31.1.368.gbe11c130af-goog

[PATCH v2 2/2] platform/chrome: cros_ec_typec: Handle hard reset

[Prashant Malani]

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 v2 1/2] platform/chrome: cros_ec: Add Type C hard reset

[Prashant Malani]

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

Re: [PATCH] platform/chrome: cros_ec_typec: Handle hard reset

[Prashant Malani]

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] platform/chrome: cros_ec_typec: Handle hard reset

[Prashant Malani]

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

[PATCH] platform/chrome: cros_ec_typec: Check for device within remove function

[Prashant Malani]

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

[PATCH] platform/chrome: cros_ec_typec: Flush pending work

[Prashant Malani]

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(>port_work);
schedule_work(>port_work);
 
return NOTIFY_OK;
-- 
2.30.0.478.g8a0d178c01-goog

Re: [PATCH 1/2] platform/chrome: cros_ec_typec: Skip port partner check in configure_mux()

[Prashant Malani]

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
>

Re: [PATCH 2/2] platform/chrome: cros_ec_types: Support disconnect events without partners

[Prashant Malani]

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 2/2] platform/chrome: cros_ec_types: Support disconnect events without partners

[Prashant Malani]

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 v2 1/2] platform/chrome: cros_ec: Import Type C control command

[Prashant Malani]

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?

[PATCH v2 2/2] platform/chrome: cros_ec_typec: Clear Type C disc events

[Prashant Malani]

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

[PATCH v2 1/2] platform/chrome: cros_ec: Import Type C control command

[Prashant Malani]

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

Re: [PATCH 1/2] platform/chrome: cros_ec: Import Type C control command

[Prashant Malani]

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 2/2] platform/chrome: cros_ec_typec: Clear Type C disc events

[Prashant Malani]

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

[PATCH 1/2] platform/chrome: cros_ec: Import Type C control command

[Prashant Malani]

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] platform/chrome: cros_ec_typec: Decouple partner removal

[Prashant Malani]

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

Re: [PATCH 6/6] platform/chrome: cros_ec_typec: Set opmode to PD on SOP connected

[Prashant Malani]

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 5/6] platform/chrome: cros_ec_typec: Set Partner PD revision from status

[Prashant Malani]

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, , 
> 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 4/6] platform/chrome: cros_ec_typec: Report SOP' PD revision from status

[Prashant Malani]

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 = >c_identity;
> +   c_desc.pd_revision = pd_revision;
>
> port->cable = typec_register_cable(port->port, _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] usb: typec: Send uevent for num_altmodes update

[Prashant Malani]

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(>dev.kobj, NULL, "number_of_alternate_modes");
> > + kobject_uevent(>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

[PATCH] usb: typec: Send uevent for num_altmodes update

[Prashant Malani]

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(>dev.kobj, NULL, "number_of_alternate_modes");
+   kobject_uevent(>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(>dev.kobj, NULL, "number_of_alternate_modes");
+   kobject_uevent(>dev.kobj, KOBJ_CHANGE);
 
return 0;
 }
-- 
2.29.2.729.g45daf8777d-goog

Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Send mux configuration acknowledgment to EC

[Prashant Malani]

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 1/2] platform/chrome: cros_ec_typec: Parameterize cros_typec_cmds_supported()

[Prashant Malani]

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

[PATCH v2] usb: typec: Add class for plug alt mode device

[Prashant Malani]

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 = _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(>adev.dev);
if (ret) {
dev_err(parent, "failed to register alternate mode (%d)\n",
-- 
2.29.2.576.ga3fc446d84-goog

Re: [PATCH] usb: typec: Add bus type for plug alt modes

[Prashant Malani]

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 == _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;
> > >

Re: [PATCH] usb: typec: Add bus type for plug alt modes

[Prashant Malani]

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 == _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)

The issue is that if we set both "class" and 

Re: [PATCH] usb: typec: Add bus type for plug alt modes

[Prashant Malani]

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

[Prashant Malani]

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 == _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 = _bus;
 
+   if (is_typec_plug(parent))
+   alt->adev.dev.class = typec_class;
+
ret = device_register(>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

Re: [PATCH] platform/chrome: cros_ec_typec: Tolerate unrecognized mux flags

[Prashant Malani]

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, >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] usb: typec: Add bus type for plug alt modes

[Prashant Malani]

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 = _bus;
 
ret = device_register(>adev.dev);
-- 
2.29.2.454.gaff20da3a2-goog

[PATCH] platform/chrome: cros_ec_typec: Decouple cable remove on disconnect

[Prashant Malani]

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

Re: [PATCH v4 1/2] usb: typec: Consolidate sysfs ABI documentation

[Prashant Malani]

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 v5 2/2] usb: typec: Expose Product Type VDOs via sysfs

[Prashant Malani]

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[] = {
_attr_id_header.attr,
_attr_cert_stat.attr,
_attr_product.attr,
+   _attr_product_type_vdo1.attr,
+   _attr_product_type_vdo2.attr,
+   _attr_product_type_vdo3.attr,
NULL
 };
 
@@ -146,6 +176,9 @@ static void typec_report_identity(struct device *dev)
sysfs_notify(>kobj, "identity", "id_header");
sysfs_notify(>kobj, "identity", "cert_stat");
sysfs_notify(>kobj, "identity", "product");
+   sysfs_notify(>kobj, "identity", "product_type_vdo1");
+   sysfs_notify(>kobj, "identity", "product_type_vdo2");
+   sysfs_notify(>kobj, "identity", "product_type_vdo3");
 }
 
 /* - */
-- 
2.29.2.454.gaff20da3a2-goog

[PATCH v5 1/2] usb: typec: Consolidate sysfs ABI documentation

[Prashant Malani]

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.
 
-What:  /sys/cl

Re: [PATCH v4 1/2] usb: typec: Consolidate sysfs ABI documentation

[Prashant Malani]

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 file for every VDO
> +

[PATCH v4 2/2] usb: typec: Expose Product Type VDOs via sysfs

[Prashant Malani]

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[] = {
_attr_id_header.attr,
_attr_cert_stat.attr,
_attr_product.attr,
+   _attr_product_type_vdo1.attr,
+   _attr_product_type_vdo2.attr,
+   _attr_product_type_vdo3.attr,
NULL
 };
 
@@ -146,6 +176,9 @@ static void typec_report_identity(struct device *dev)
sysfs_notify(>kobj, "identity", "id_header");
sysfs_notify(>kobj, "identity", "cert_stat");
sysfs_notify(>kobj, "identity", "product");
+   sysfs_notify(>kobj, "identity", "product_type_vdo1");
+   sysfs_notify(>kobj, "identity", "product_type_vdo2");
+   sysfs_notify(>kobj, "identity", "product_type_vdo3");
 }
 
 /* - */
-- 
2.29.2.454.gaff20da3a2-goog

Re: [PATCH v3 1/2] usb: typec: Consolidate syfs ABI documentation

[Prashant Malani]

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

[PATCH v4 1/2] usb: typec: Consolidate sysfs ABI documentation

[Prashant Malani]

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:  /sys/class/typec

Re: [PATCH v3 2/4] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

[Prashant Malani]

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
> > is the c

Re: [PATCH v3 2/4] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

[Prashant Malani]

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] usb: typec: Fix num_altmodes kernel-doc error

[Prashant Malani]

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 1/4] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message

[Prashant Malani]

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: linux-next: build warning after merge of the usb tree

[Prashant Malani]

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

[PATCH] usb: typec: Fix num_altmodes kernel-doc error

[Prashant Malani]

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: [PATCH v3 2/4] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

[Prashant Malani]

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

Re: [RFC PATCH 2/3] usb: typec: Add product_type sysfs attribute file for partners and cables

[Prashant Malani]

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

[Prashant Malani]

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: [PATCH v3 00/11] chrome/platform: cros_ec_typec: Register cables, partner altmodes and plug altmodes

[Prashant Malani]

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: [RFC PATCH 2/3] usb: typec: Add product_type sysfs attribute file for partners and cables

[Prashant Malani]

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: [RFC PATCH 0/3] usb: typec: Product Type time

[Prashant Malani]

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: [PATCH v2 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

[Prashant Malani]

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: [PATCH v2 5/8] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message

[Prashant Malani]

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 5/8] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message

[Prashant Malani]

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] platform/chrome: cros_ec_typec: Tolerate unrecognized mux flags

[Prashant Malani]

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, >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 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

[Prashant Malani]

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 v3 03/11] usb: typec: Add plug num_altmodes sysfs attr

[Prashant Malani]

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/

[PATCH v3 11/11] platform/chrome: cros_ec_typec: Register plug altmodes

[Prashant Malani]

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

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

Signed-off-by: Prashant Malani 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.

No version v2.

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

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

[PATCH v3 10/11] platform/chrome: cros_ec_typec: Register SOP' cable plug

[Prashant Malani]

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

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

Signed-off-by: Prashant Malani 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.

No version v2.

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

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

[PATCH v3 05/11] platform/chrome: cros_ec_typec: Factor out PD identity parsing

[Prashant Malani]

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

No functional changes are introduced by this patch.

Signed-off-by: Prashant Malani 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Heikki Krogerus 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.
- Added Reviewed-by tags

Changes in v2:
- No changes.

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

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

[PATCH v3 06/11] platform/chrome: cros_ec_typec: Rename discovery struct

[Prashant Malani]

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

Signed-off-by: Prashant Malani 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Heikki Krogerus 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.
- Added Reviewed-by tags

Changes in v2:
- No changes.

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

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

[PATCH v3 01/11] usb: pd: Add captive Type C cable type

[Prashant Malani]

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

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

Cc: Heikki Krogerus 
Signed-off-by: Prashant Malani 
Reviewed-by: Benson Leung 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Heikki Krogerus 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.
- Added Reviewed-by tags

Changes in v2:
- No changes.

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

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

[PATCH v3 02/11] usb: typec: Add number of altmodes partner attr

[Prashant Malani]

Add a user-visible attribute for the number of alternate modes available
in a partner. 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 partner.

Also add a sysfs file which exposes this attribute. The file remains
hidden as long as the attribute value is -1.

Cc: Benson Leung 
Cc: Heikki Krogerus 
Signed-off-by: Prashant Malani 
Reviewed-by: Heikki Krogerus 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.
- Added Reviewed-by tags

Changes in v2:
- Added ABI/testing documentation entry for added sysfs file.
- Changed name of the sysfs file to "number_of_alternate_modes" based on
  review comments.
- Added is_visible() logic suggested by Heikki in the comments of v1.
- Updated commit message.

 Documentation/ABI/testing/sysfs-class-typec |  8 +++
 drivers/usb/typec/class.c   | 66 -
 include/linux/usb/typec.h   |  1 +
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-class-typec 
b/Documentation/ABI/testing/sysfs-class-typec
index b834671522d6..73ac7b461ae5 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -134,6 +134,14 @@ Description:
Shows if the partner supports USB Power Delivery communication:
Valid values: yes, no
 
+What:  /sys/class/typec/-partner/number_of_alternate_modes
+Date:  November 2020
+Contact:   Prashant Malani 
+Description:
+   Shows the number of alternate modes which are advertised by the 
partner
+   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 
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 35eec707cb51..c7412ddbd311 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -33,6 +33,7 @@ struct typec_partner {
struct usb_pd_identity  *identity;
enum typec_accessoryaccessory;
struct ida  mode_ids;
+   int num_altmodes;
 };
 
 struct typec_port {
@@ -532,12 +533,43 @@ static ssize_t supports_usb_power_delivery_show(struct 
device *dev,
 }
 static DEVICE_ATTR_RO(supports_usb_power_delivery);
 
+static ssize_t number_of_alternate_modes_show(struct device *dev, struct 
device_attribute *attr,
+ char *buf)
+{
+   struct typec_partner *p = to_typec_partner(dev);
+
+   return sysfs_emit(buf, "%d\n", p->num_altmodes);
+}
+static DEVICE_ATTR_RO(number_of_alternate_modes);
+
 static struct attribute *typec_partner_attrs[] = {
_attr_accessory_mode.attr,
_attr_supports_usb_power_delivery.attr,
+   _attr_number_of_alternate_modes.attr,
+   NULL
+};
+
+static umode_t typec_partner_attr_is_visible(struct kobject *kobj, struct 
attribute *attr, int n)
+{
+   struct typec_partner *partner = to_typec_partner(kobj_to_dev(kobj));
+
+   if (attr == _attr_number_of_alternate_modes.attr) {
+   if (partner->num_altmodes < 0)
+   return 0;
+   }
+
+   return attr->mode;
+}
+
+static struct attribute_group typec_partner_group = {
+   .is_visible = typec_partner_attr_is_visible,
+   .attrs = typec_partner_attrs
+};
+
+static const struct attribute_group *typec_partner_groups[] = {
+   _partner_group,
NULL
 };
-ATTRIBUTE_GROUPS(typec_partner);
 
 static void typec_partner_release(struct device *dev)
 {
@@ -570,6 +602,37 @@ int typec_partner_set_identity(struct typec_partner 
*partner)
 }
 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.
+ *
+ * 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.
+ *
+ * @partner.num_altmodes is set to -1 on partner registration, denoting that
+ * a valid value has not been set for it yet.
+ *
+ * Returns 0 on success or negative error number on failure.
+ */
+int typec_partner_set_num_altmodes(struct typec_partner *partner, int 
num_altmodes)
+{
+   int ret;
+
+   if (num_altmodes < 0)
+   return -EINVAL

[PATCH v3 08/11] platform/chrome: cros_ec_typec: Store cable plug type

[Prashant Malani]

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

Cc: Heikki Krogerus 
Cc: Greg Kroah-Hartman 
Signed-off-by: Prashant Malani 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Heikki Krogerus 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.
- Added Reviewed-by tags

Changes in v2:
- No changes.

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

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

[PATCH v3 07/11] platform/chrome: cros_ec_typec: Register cable

[Prashant Malani]

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

Cc: Heikki Krogerus 
Signed-off-by: Prashant Malani 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Heikki Krogerus 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.
- Added Reviewed-by tags

Changes in v2:
- No changes.

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

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

[PATCH v3 03/11] usb: typec: Add plug num_altmodes sysfs attr

[Prashant Malani]

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

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

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

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

Signed-off-by: Prashant Malani 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.

No version v2.

 Documentation/ABI/testing/sysfs-class-typec |  9 +++
 drivers/usb/typec/class.c   | 77 -
 include/linux/usb/typec.h   |  1 +
 3 files changed, 85 insertions(+), 2 deletions(-)

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

[PATCH v3 09/11] platform/chrome: cros_ec_typec: Set partner num_altmodes

[Prashant Malani]

Set the number of altmodes available for a registered partner using the
Type C connector class framework routine.

Cc: Heikki Krogerus 
Signed-off-by: Prashant Malani 
Reviewed-by: Heikki Krogerus 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.
- Added Reviewed-by tags

Changes in v2:
- Patch introduced for the first time in v2.

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

diff --git a/drivers/platform/chrome/cros_ec_typec.c 
b/drivers/platform/chrome/cros_ec_typec.c
index cf609aa10567..ad5e37bfd45d 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -642,6 +642,7 @@ static int cros_typec_register_altmodes(struct 
cros_typec_data *typec, int port_
struct cros_typec_altmode_node *node;
struct typec_altmode_desc desc;
struct typec_altmode *amode;
+   int num_altmodes = 0;
int ret = 0;
int i, j;
 
@@ -668,9 +669,16 @@ static int cros_typec_register_altmodes(struct 
cros_typec_data *typec, int port_
 
node->amode = amode;
list_add_tail(>list, >partner_mode_list);
+   num_altmodes++;
}
}
 
+   ret = typec_partner_set_num_altmodes(port->partner, num_altmodes);
+   if (ret < 0) {
+   dev_err(typec->dev, "Unable to set partner num_altmodes for 
port: %d\n", port_num);
+   goto err_cleanup;
+   }
+
return 0;
 
 err_cleanup:
-- 
2.29.2.299.gdc1121823c-goog

[PATCH v3 04/11] platform/chrome: cros_ec_typec: Make disc_done flag partner-only

[Prashant Malani]

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

Signed-off-by: Prashant Malani 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Heikki Krogerus 
---

Changes in v3:
- Re-arranged patch order and combined it with related series of
  patches.
- Added Reviewed-by tags

Changes in v2:
- No changes.

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

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

[PATCH v3 00/11] chrome/platform: cros_ec_typec: Register cables, partner altmodes and plug altmodes

[Prashant Malani]

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.
The others are users of the newly introduced USB changes and can go through
the chrome-platform tree.

Of course, the above is only a suggestion, so I'd be happy to follow
another means of integrating the patches if available.

The series is based on the following git branch and commit
Branch: chrome-platform for-next [4]
Commit: de0f49487db3 ("platform/chrome: cros_ec_typec: Register partner 
altmodes")

For reference, the patches in this series which are yet to be reviewed are
Patch 3/11, Patch 10/11 and Patch 11/11.

Version history:
- No v2 or v1, as mentioned earlier these patches were uploaded as separate
  series [1], [2] and [3] but have now been coalesced.

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

Prashant Malani (11):
  usb: pd: Add captive Type C cable type
  usb: typec: Add number of altmodes partner attr
  usb: typec: Add plug num_altmodes sysfs attr
  platform/chrome: cros_ec_typec: Make disc_done flag partner-only
  platform/chrome: cros_ec_typec: Factor out PD identity parsing
  platform/chrome: cros_ec_typec: Rename discovery struct
  platform/chrome: cros_ec_typec: Register cable
  platform/chrome: cros_ec_typec: Store cable plug type
  platform/chrome: cros_ec_typec: Set partner num_altmodes
  platform/chrome: cros_ec_typec: Register SOP' cable plug
  platform/chrome: cros_ec_typec: Register plug altmodes

 Documentation/ABI/testing/sysfs-class-typec |  17 ++
 drivers/platform/chrome/cros_ec_typec.c | 219 
 drivers/usb/typec/class.c   | 139 -
 include/linux/usb/pd_vdo.h  |   4 +-
 include/linux/usb/typec.h   |   2 +
 5 files changed, 343 insertions(+), 38 deletions(-)

-- 
2.29.2.299.gdc1121823c-goog

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

[Prashant Malani]

Hi Greg,

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

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

This series depends on those earlier series.

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

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

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

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

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

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

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

BR,

-Prashant

Re: [PATCH v3 2/2] usb: typec: Expose Product Type VDOs via sysfs

[Prashant Malani]

Hi Heikki,

On Thu, Nov 12, 2020 at 4:43 AM Heikki Krogerus
 wrote:
>
> On Wed, Nov 11, 2020 at 06:40:55PM -0800, Prashant Malani wrote:
> > Hi Heikki,
> >
> > On Tue, Nov 10, 2020 at 01:54:53PM +0200, Heikki Krogerus wrote:
> > > On Fri, Oct 23, 2020 at 02:43:28PM -0700, Prashant Malani wrote:
> > >
> > > I've now come to the conclusion that this is not the correct approach.
> > > Instead, the whole identity, all six VDOs, should be supplied
> > > separately with a "raw" sysfs attribute file after all.
> > >
> > > The three attribute files that we already have - so id_header,
> > > cert_stat and product - can always supply the actual VDO as is,
> > > regardless of the product type, so they are fine. But these new
> > > attribute files, product_type_vdoX, would behave differently as they
> > > supply different information depending on the product type. That just
> > > does not feel right to me.
> >
> > OOI: I'd like to understand the reservations around this approach. Can't
> > userspace just read these and then interpret them appropriately according
> > to the id_header as well as PD revision (and version number) if that's 
> > exposed?
> > The only thing I see changing is how we name those product_type_vdoX
> > sysfs files, i.e product_type_vdo0 == passive_cable_vdo OR active_cable_vdo1
> > depending on the product type.
> >
> > That said, perhaps I'm missing some aspect of this.
>
> I don't think the userspace should have to interpret any of these
> VDOs. If the userspace has to interpret the information, then the
> userspace should interpret everything for the sake of consistency (so
> the "raw" attribute file).
>
> But I still think that defining separate device types for every
> product type would be the best way to handle the identity. We could
> then have sysfs attribute files that are specific for each product
> type. It does not even matter that some of the product types are going
> to be removed. We will have to handle all of them in any case,
> including the ones that were removed. This way things would be much
> more clear for the userspace.
>
> The only problem IMO with the separate device types for each product
> type is that we don't always have access to the Discover Identity
> result. It means depending on your system we will claim the
> partner device type is "default" (no identity information) or the
> actual product type. That is also a bit inconsistent, but is is
> acceptable? I would really like to here what Greg thinks about all
> this.

Thanks for explaining the rationale.
Of course, I defer to Greg & your decision on this :)

I'm yet unable to grasp what benefit userspace gets from having the kernel parse
and present this data in appropriately named sysfs files when the userspace has
enough info to do so itself.

For that reason and also because the "raw" approach is IMO a bit more
resilient to the changes
we talk about (some product type VDOs being dropped off across PD spec
uprevs [1] etc)
the "raw" proposal sounded appealing to me.

>
> > > So lets just add the "raw" sysfs attribute file. We can think about
> > > extracting some other details from the product type VDOs once the
> > > specification has settled down a bit and we can be quite certain that
> > > those details will always be available.
> > >
> > > Would this be OK to you? I think we should be able to dump the data to
> > > the "raw" sysfs attribute file with something like hex_dump_to_buffer().
> >
> > FWIW, "raw" option SGTM (the product type VDOs can be parsed from the
> > buffer since the format is fixed).
>
> Well, I'm starting to think that what if we just prepare patches where
> we propose separate device type for every product type? Of course, if
> they are OK to you?
>
SG. To clarify, will you prepare these patches?

Thanks & best regards,

-Prashant

[1]: 
https://lore.kernel.org/linux-usb/canlzekskrwxwlc+csobywb+jufdh+p6v6gimhtsky-l61ct...@mail.gmail.com/

Re: [PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[Prashant Malani]

On Thu, Nov 12, 2020 at 05:07:05PM +0800, Pi-Hsun Shih wrote:
> Hi Prashant,
> 
> Please see inline reply as below.
> 
> On Thu, Nov 12, 2020 at 4:59 PM Prashant Malani  wrote:
> >
> > Hi Pi-Hsun,
> >
> > I haven't gone through the code, but did have a high-level comment
> > (kindly see inline)
> >
> > On Thu, Nov 12, 2020 at 02:40:40PM +0800, Pi-Hsun Shih wrote:
> > > When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or
> > > TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two
> > > orientations of typec.
> > >
> > > On some board one anx7625 is used as DPI to DP converter for two typec
> > > ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb
> > > muxes, which mux the DP data with the rest of the USB3 data, and
> > > connects to the two typec ports.
> > >
> > > This patch adds option for anx7625 to acts as a usb typec switch and
> > > switch output lanes based on the typec orientation, or acts as two usb
> > > typec mux and switch output lanes depending on whether the two ports
> > > currently has DP enabled.
> > >
> > > Signed-off-by: Pi-Hsun Shih 
> > >
> > > 
> > >
> > > This is an attempt to use typec framework with how we're using anx7625
> > > on Chrome OS asurada board.
> > >
> > > An example of the dts for the two ports case can be found at
> > > https://crrev.com/c/2507199/6
> >
> > Do you plan on submitting DT schemas & bindings documentation for the 
> > switch(es)
> > that are intended to be used?
> 
> Yes I plan to submit corresponding DT schemas & bindings documentation
> changes if this change looks good.
> 

That's great. Thanks for confirming :)


BR,

-Prashant

Re: [PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[Prashant Malani]

Hi Pi-Hsun,

I haven't gone through the code, but did have a high-level comment
(kindly see inline)

On Thu, Nov 12, 2020 at 02:40:40PM +0800, Pi-Hsun Shih wrote:
> When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or
> TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two
> orientations of typec.
> 
> On some board one anx7625 is used as DPI to DP converter for two typec
> ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb
> muxes, which mux the DP data with the rest of the USB3 data, and
> connects to the two typec ports.
> 
> This patch adds option for anx7625 to acts as a usb typec switch and
> switch output lanes based on the typec orientation, or acts as two usb
> typec mux and switch output lanes depending on whether the two ports
> currently has DP enabled.
> 
> Signed-off-by: Pi-Hsun Shih 
> 
> 
> 
> This is an attempt to use typec framework with how we're using anx7625
> on Chrome OS asurada board.
> 
> An example of the dts for the two ports case can be found at
> https://crrev.com/c/2507199/6

Do you plan on submitting DT schemas & bindings documentation for the switch(es)
that are intended to be used?

I would strongly recommend that for usb-c-connector since AFAIK they don't 
exist, and
I don't believe there is explicit support for them in the Type C connector 
class framework
(even .

IMO this would be needed to ensure an implementation here doesn't break
in the event of modifications to the connector class framework (or Type
C port drivers like cros-ec-typec) in the future. I think some patches
were floated for this for orientation switch [1] so those might provide
some hints about how to proceed.

I've CC-ed Heikki (Type C maintainer) in case he has additional comments 
regarding this.

> 
> Sending this as a RFC patch since I'm not sure about the best approach
> here. Should the logic of switching output lanes depends on ports be
> coupled inside anx7625 driver, or in another driver, or is there
> any existing way to accomplish this?

Might be good to add [RFC] as a tag instead of [PATCH] in case this
iteration is chiefly to solicit comments.

Best regards,

-Prashant

[1]:
https://lore.kernel.org/linux-usb/1604403610-16577-1-git-send-email-jun...@nxp.com/
> 

Re: [PATCH v3 2/2] usb: typec: Expose Product Type VDOs via sysfs

[Prashant Malani]

Hi Heikki,

On Tue, Nov 10, 2020 at 01:54:53PM +0200, Heikki Krogerus wrote:
> On Fri, Oct 23, 2020 at 02:43:28PM -0700, Prashant Malani wrote:
> 
> I've now come to the conclusion that this is not the correct approach.
> Instead, the whole identity, all six VDOs, should be supplied
> separately with a "raw" sysfs attribute file after all.
> 
> The three attribute files that we already have - so id_header,
> cert_stat and product - can always supply the actual VDO as is,
> regardless of the product type, so they are fine. But these new
> attribute files, product_type_vdoX, would behave differently as they
> supply different information depending on the product type. That just
> does not feel right to me.

OOI: I'd like to understand the reservations around this approach. Can't
userspace just read these and then interpret them appropriately according
to the id_header as well as PD revision (and version number) if that's exposed?
The only thing I see changing is how we name those product_type_vdoX
sysfs files, i.e product_type_vdo0 == passive_cable_vdo OR active_cable_vdo1
depending on the product type.

That said, perhaps I'm missing some aspect of this.

> 
> So lets just add the "raw" sysfs attribute file. We can think about
> extracting some other details from the product type VDOs once the
> specification has settled down a bit and we can be quite certain that
> those details will always be available.
> 
> Would this be OK to you? I think we should be able to dump the data to
> the "raw" sysfs attribute file with something like hex_dump_to_buffer().

FWIW, "raw" option SGTM (the product type VDOs can be parsed from the
buffer since the format is fixed).

> 
> thanks,
> 
> -- 
> heikki

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

[Prashant Malani]

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

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

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

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

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

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

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

-- 
2.29.2.222.g5d2a92d10f8-goog

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

[Prashant Malani]

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

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

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

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

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

[Prashant Malani]

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

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

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

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

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

[Prashant Malani]

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

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

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

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

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

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

[PATCH v2 2/2] platform/chrome: cros_ec_typec: Set partner num_altmodes

[Prashant Malani]

Set the number of altmodes available for a registered partner using the
Type C connector class framework routine.

Cc: Heikki Krogerus 
Signed-off-by: Prashant Malani 
---

Changes in v2:
- Patch introduced for the first time in v2.

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

diff --git a/drivers/platform/chrome/cros_ec_typec.c 
b/drivers/platform/chrome/cros_ec_typec.c
index ce031a10eb1b..743a28426f98 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -621,6 +621,7 @@ static int cros_typec_register_altmodes(struct 
cros_typec_data *typec, int port_
struct cros_typec_altmode_node *node;
struct typec_altmode_desc desc;
struct typec_altmode *amode;
+   int num_altmodes = 0;
int ret = 0;
int i, j;
 
@@ -647,9 +648,16 @@ static int cros_typec_register_altmodes(struct 
cros_typec_data *typec, int port_
 
node->amode = amode;
list_add_tail(>list, >partner_mode_list);
+   num_altmodes++;
}
}
 
+   ret = typec_partner_set_num_altmodes(port->partner, num_altmodes);
+   if (ret < 0) {
+   dev_err(typec->dev, "Unable to set partner num_altmodes for 
port: %d\n", port_num);
+   goto err_cleanup;
+   }
+
return 0;
 
 err_cleanup:
-- 
2.29.2.222.g5d2a92d10f8-goog

[PATCH v2 1/2] usb: typec: Add number of altmodes partner attr

[Prashant Malani]

Add a user-visible attribute for the number of alternate modes available
in a partner. 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 partner.

Also add a sysfs file which exposes this attribute. The file remains
hidden as long as the attribute value is -1.

Cc: Benson Leung 
Cc: Heikki Krogerus 
Signed-off-by: Prashant Malani 
---

Changes in v2:
- Added ABI/testing documentation entry for added sysfs file.
- Changed name of the sysfs file to "number_of_alternate_modes" based on
  review comments.
- Added is_visible() logic suggested by Heikki in the comments of v1.
- Updated commit message.

v1:
https://lore.kernel.org/lkml/20200701003149.3101219-1-pmal...@chromium.org/

 Documentation/ABI/testing/sysfs-class-typec |  8 +++
 drivers/usb/typec/class.c   | 66 -
 include/linux/usb/typec.h   |  1 +
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-class-typec 
b/Documentation/ABI/testing/sysfs-class-typec
index b834671522d6..73ac7b461ae5 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -134,6 +134,14 @@ Description:
Shows if the partner supports USB Power Delivery communication:
Valid values: yes, no
 
+What:  /sys/class/typec/-partner/number_of_alternate_modes
+Date:  November 2020
+Contact:   Prashant Malani 
+Description:
+   Shows the number of alternate modes which are advertised by the 
partner
+   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 
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 35eec707cb51..c7412ddbd311 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -33,6 +33,7 @@ struct typec_partner {
struct usb_pd_identity  *identity;
enum typec_accessoryaccessory;
struct ida  mode_ids;
+   int num_altmodes;
 };
 
 struct typec_port {
@@ -532,12 +533,43 @@ static ssize_t supports_usb_power_delivery_show(struct 
device *dev,
 }
 static DEVICE_ATTR_RO(supports_usb_power_delivery);
 
+static ssize_t number_of_alternate_modes_show(struct device *dev, struct 
device_attribute *attr,
+ char *buf)
+{
+   struct typec_partner *p = to_typec_partner(dev);
+
+   return sysfs_emit(buf, "%d\n", p->num_altmodes);
+}
+static DEVICE_ATTR_RO(number_of_alternate_modes);
+
 static struct attribute *typec_partner_attrs[] = {
_attr_accessory_mode.attr,
_attr_supports_usb_power_delivery.attr,
+   _attr_number_of_alternate_modes.attr,
+   NULL
+};
+
+static umode_t typec_partner_attr_is_visible(struct kobject *kobj, struct 
attribute *attr, int n)
+{
+   struct typec_partner *partner = to_typec_partner(kobj_to_dev(kobj));
+
+   if (attr == _attr_number_of_alternate_modes.attr) {
+   if (partner->num_altmodes < 0)
+   return 0;
+   }
+
+   return attr->mode;
+}
+
+static struct attribute_group typec_partner_group = {
+   .is_visible = typec_partner_attr_is_visible,
+   .attrs = typec_partner_attrs
+};
+
+static const struct attribute_group *typec_partner_groups[] = {
+   _partner_group,
NULL
 };
-ATTRIBUTE_GROUPS(typec_partner);
 
 static void typec_partner_release(struct device *dev)
 {
@@ -570,6 +602,37 @@ int typec_partner_set_identity(struct typec_partner 
*partner)
 }
 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.
+ *
+ * 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.
+ *
+ * @partner.num_altmodes is set to -1 on partner registration, denoting that
+ * a valid value has not been set for it yet.
+ *
+ * Returns 0 on success or negative error number on failure.
+ */
+int typec_partner_set_num_altmodes(struct typec_partner *partner, int 
num_altmodes)
+{
+   int ret;
+
+   if (num_altmodes < 0)
+   return -EINVAL;
+
+   partner->num_altmodes = num_altmodes;
+  

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

[Prashant Malani]

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

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

Cc: Heikki Krogerus 
Signed-off-by: Prashant Malani 
---

Changes in v2:
- No changes.

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

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

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

[Prashant Malani]

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

Cc: Heikki Krogerus 
Cc: Greg Kroah-Hartman 
Signed-off-by: Prashant Malani 
---

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

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

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

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

[Prashant Malani]

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

No functional changes are introduced by this patch.

Signed-off-by: Prashant Malani 
---

Changes in v2:
- No changes.

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

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

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

[Prashant Malani]

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

Cc: Heikki Krogerus 
Signed-off-by: Prashant Malani 
---

Changes in v2:
- No changes.

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

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

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

[Prashant Malani]

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

Signed-off-by: Prashant Malani 
---

Changes in v2:
- No changes.

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

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

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

[Prashant Malani]

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

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

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

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

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

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

-- 
2.29.1.341.ge80a0c044ae-goog

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

[Prashant Malani]

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

Signed-off-by: Prashant Malani 
---

Changes in v2:
- No changes.

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

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

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

[Prashant Malani]

Hi Greg,

On Fri, Nov 06, 2020 at 10:33:02AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Nov 06, 2020 at 12:59:07AM -0800, Prashant Malani wrote:
> > Hi Greg,
> > 
> > Did you not receive these?
> 
> Ah, I got 1, 2, and 5, and now 6.  That's confusing, think about if you
> were to receive such a series, what would you think to do with it?
> 

Yeah, I agree it looks confusing. Sorry about that.

> > > So you save it but what happens with the value?
> > 
> > The type C connector class framework exposes it via syfs to user-space when 
> > we
> > register the cable via typec_register_cable() in patch 4/6 [2].
> 
> So you added a new sysfs file and api without updating
> Documentation/ABI/?  That's not good :(

This is a pre-existing API[1] and sysfs file[2] so we are using those
and not adding anything new.

[1]:
https://www.kernel.org/doc/html/latest/driver-api/usb/typec.html#c.typec_register_cable
[2]: https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-class-typec
(see /sys/class/-cable/plug_type)


Best regards,

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

[Prashant Malani]

Hi Greg,

Thanks for looking at the patch.

On Fri, Nov 06, 2020 at 08:20:59AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Nov 05, 2020 at 05:28:03PM -0800, Prashant Malani wrote:
> > Use the PD VDO Type C cable plug type macro to retrieve and store the
> > cable plug type in the cable descriptor.
> > 
> > Cc: Heikki Krogerus 
> > Cc: Greg Kroah-Hartman 
> > Signed-off-by: Prashant Malani 
> > ---
> >  drivers/platform/chrome/cros_ec_typec.c | 21 -
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> Where are the first 5 patches in this series?

I believe I'd cc-ed you (and linux-usb) on the following patches in the
series(modified git_cc[1] which adds individual patch get_maintainers.pl to the 
cover
letter, patch 1 and patch 2):
cover letter:
https://lore.kernel.org/lkml/20201106012758.525472-1-pmal...@chromium.org/
patch 1/6 (cros-ec-typec cleanup):
https://lore.kernel.org/lkml/20201106012758.525472-2-pmal...@chromium.org/
patch 2/6 (cros-ec-typec cleanup):
https://lore.kernel.org/lkml/20201106012758.525472-2-pmal...@chromium.org/
patch 5/6 (PD VDO header patch):
https://lore.kernel.org/lkml/20201106012758.525472-6-pmal...@chromium.org/
patch 6/6 (this patch):
https://lore.kernel.org/lkml/20201106012758.525472-7-pmal...@chromium.org/

Did you not receive these?
> 
> 
> > 
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c 
> > b/drivers/platform/chrome/cros_ec_typec.c
> > index 5e7f0b4ebbec..0a2a8b0f8115 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -709,6 +709,7 @@ static int cros_typec_handle_sop_prime_disc(struct 
> > cros_typec_data *typec, int p
> > .port = port_num,
> > .partner_type = TYPEC_PARTNER_SOP_PRIME,
> > };
> > +   uint32_t cable_plug_type;
> 
> u32 please, this isn't userspace code :)

Will fix this in the next version.
> 
> > int ret = 0;
> >  
> > memset(disc, 0, EC_PROTO2_MAX_RESPONSE_SIZE);
> > @@ -722,8 +723,26 @@ static int cros_typec_handle_sop_prime_disc(struct 
> > cros_typec_data *typec, int p
> > /* Parse the PD identity data, even if only 0s were returned. */
> > cros_typec_parse_pd_identity(>c_identity, disc);
> >  
> > -   if (disc->identity_count != 0)
> > +   if (disc->identity_count != 0) {
> > +   cable_plug_type = VDO_TYPEC_CABLE_TYPE(port->c_identity.vdo[0]);
> > +   switch (cable_plug_type) {
> > +   case CABLE_ATYPE:
> > +   desc.type = USB_PLUG_TYPE_A;
> > +   break;
> > +   case CABLE_BTYPE:
> > +   desc.type = USB_PLUG_TYPE_B;
> > +   break;
> > +   case CABLE_CTYPE:
> > +   desc.type = USB_PLUG_TYPE_C;
> > +   break;
> > +   case CABLE_CAPTIVE:
> > +   desc.type = USB_PLUG_CAPTIVE;
> > +   break;
> > +   default:
> > +   desc.type = USB_PLUG_NONE;
> > +   }
> > desc.active = PD_IDH_PTYPE(port->c_identity.id_header) == 
> > IDH_PTYPE_ACABLE;
> > +   }
> 
> So you save it but what happens with the value?

The type C connector class framework exposes it via syfs to user-space when we
register the cable via typec_register_cable() in patch 4/6 [2].

I'll go ahead and CC
you and linux-usb on the entire series for the next version.

[1]: https://lwn.net/Articles/585782/
[2]:
https://lore.kernel.org/lkml/20201106012758.525472-5-pmal...@chromium.org/

Best regards,

[PATCH] platform/chrome: cros_ec_typec: Tolerate unrecognized mux flags

[Prashant Malani]

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

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

[Prashant Malani]

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

Cc: Heikki Krogerus 
Cc: Greg Kroah-Hartman 
Signed-off-by: Prashant Malani 
---
 drivers/platform/chrome/cros_ec_typec.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

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

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

[Prashant Malani]

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

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

Cc: Heikki Krogerus 
Signed-off-by: Prashant Malani 
---
 include/linux/usb/pd_vdo.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

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

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

[Prashant Malani]

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

Signed-off-by: Prashant Malani 
---
 drivers/platform/chrome/cros_ec_typec.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

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

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

[Prashant Malani]

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

Cc: Heikki Krogerus 
Signed-off-by: Prashant Malani 
---
 drivers/platform/chrome/cros_ec_typec.c | 67 +
 1 file changed, 67 insertions(+)

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

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

[Prashant Malani]

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

No functional changes are introduced by this patch.

Signed-off-by: Prashant Malani 
---
 drivers/platform/chrome/cros_ec_typec.c | 35 -
 1 file changed, 23 insertions(+), 12 deletions(-)

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

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

[Prashant Malani]

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

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

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

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

-- 
2.29.1.341.ge80a0c044ae-goog

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

[Prashant Malani]

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

Signed-off-by: Prashant Malani 
---
 drivers/platform/chrome/cros_ec_typec.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

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