LinuxLists.cc - [PATCH v2 0/8] Thunderbolt3/USB4 cable rounded and active cable plug link training support

[permalink] [raw]

Subject: [PATCH v2 2/8] platform/chrome: cros_ec_typec: Correct the Thunderbolt rounded/non-rounded cable support

Thunderbolt rounded/non-rounded cable support is two bits value. Correcting
it as per the Thunderbolt 3 cable discover mode VDO changes done in the
Thunderbolt 3 alternate mode header.

Signed-off-by: Utkarsh Patel <[email protected]>
--
Changes in v2:
- Removed the fixes tag as there is no functional implication.
--
---
drivers/platform/chrome/cros_ec_typec.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 31be31161350..8111ed1fc574 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -438,8 +438,7 @@ static int cros_typec_enable_tbt(struct cros_typec_data *typec,
if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
data.cable_mode |= TBT_CABLE_LINK_TRAINING;

- if (pd_ctrl->cable_gen)
- data.cable_mode |= TBT_CABLE_ROUNDED;
+ data.cable_mode |= TBT_SET_CABLE_ROUNDED(pd_ctrl->cable_gen);

/* Enter Mode VDO */
data.enter_vdo = TBT_SET_CABLE_SPEED(pd_ctrl->cable_speed);
--
2.17.1

2020-11-13 20:31:44

[permalink] [raw]

Subject: [PATCH v2 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

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

--
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;
+
+ 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);

port->state.alt = NULL;
port->state.data = &data;
--
2.17.1

2020-11-13 20:31:51

[permalink] [raw]

Subject: [PATCH v2 5/8] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message

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

--
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;
unsigned char active_link_training:1;
};

--
2.17.1

2020-11-13 20:32:10

[permalink] [raw]

Subject: [PATCH v2 7/8] usb: typec: intel_pmc_mux: Configure active cable properties for USB4

Value received as a part of Thunderbolt 3 cable discover mode VDO needs
to be configured in the USB4 mode for the Thunderbolt rounded support and
active cable plug link training.

Suggested-by: Heikki Krogerus <[email protected]>
Signed-off-by: Utkarsh Patel <[email protected]>

--
Changes in v2:
- No change.
--
---
drivers/usb/typec/mux/intel_pmc_mux.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
index aa3211f1c4c3..61feb358aad3 100644
--- a/drivers/usb/typec/mux/intel_pmc_mux.c
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -295,6 +295,7 @@ pmc_usb_mux_usb4(struct pmc_usb_port *port, struct typec_mux_state *state)
{
struct enter_usb_data *data = state->data;
struct altmode_req req = { };
+ u8 cable_rounded;
u8 cable_speed;

if (IOM_PORT_ACTIVITY_IS(port->iom_status, TBT) ||
@@ -308,9 +309,6 @@ pmc_usb_mux_usb4(struct pmc_usb_port *port, struct typec_mux_state *state)
/* USB4 Mode */
req.mode_data = PMC_USB_ALTMODE_FORCE_LSR;

- if (data->active_link_training)
- req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK;
-
req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_SHIFT;
req.mode_data |= (port->role - 1) << PMC_USB_ALTMODE_UFP_SHIFT;

@@ -322,6 +320,20 @@ pmc_usb_mux_usb4(struct pmc_usb_port *port, struct typec_mux_state *state)
fallthrough;
default:
req.mode_data |= PMC_USB_ALTMODE_ACTIVE_CABLE;
+
+ if (data->tbt_cable_vdo) {
+ /* Active Thunderbolt 3 cable */
+ if (data->tbt_cable_vdo & TBT_CABLE_LINK_TRAINING)
+ req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK;
+
+ cable_rounded =
+ TBT_CABLE_ROUNDED_SUPPORT(data->tbt_cable_vdo);
+ req.mode_data |= PMC_USB_ALTMODE_TBT_GEN(cable_rounded);
+ } else {
+ /* Active USB4 cable */
+ req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK |
+ PMC_USB_ALTMODE_TBT_GEN(1);
+ }
break;
}

--
2.17.1

2020-11-17 10:47:07

[permalink] [raw]

Subject: Re: [PATCH v2 2/8] platform/chrome: cros_ec_typec: Correct the Thunderbolt rounded/non-rounded cable support

On Fri, Nov 13, 2020 at 12:24:57PM -0800, Utkarsh Patel wrote:
> Thunderbolt rounded/non-rounded cable support is two bits value. Correcting
> it as per the Thunderbolt 3 cable discover mode VDO changes done in the
> Thunderbolt 3 alternate mode header.
>
> Signed-off-by: Utkarsh Patel <[email protected]>

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

> --
> Changes in v2:
> - Removed the fixes tag as there is no functional implication.
> --
> ---
> drivers/platform/chrome/cros_ec_typec.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 31be31161350..8111ed1fc574 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -438,8 +438,7 @@ static int cros_typec_enable_tbt(struct cros_typec_data *typec,
> if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
> data.cable_mode |= TBT_CABLE_LINK_TRAINING;
>
> - if (pd_ctrl->cable_gen)
> - data.cable_mode |= TBT_CABLE_ROUNDED;
> + data.cable_mode |= TBT_SET_CABLE_ROUNDED(pd_ctrl->cable_gen);
>
> /* Enter Mode VDO */
> data.enter_vdo = TBT_SET_CABLE_SPEED(pd_ctrl->cable_speed);
> --
> 2.17.1

thanks,

--
heikki

2020-11-17 10:51:17

by Enric Balletbo i Serra

[permalink] [raw]

Subject: Re: [PATCH v2 2/8] platform/chrome: cros_ec_typec: Correct the Thunderbolt rounded/non-rounded cable support

On 13/11/20 21:24, Utkarsh Patel wrote:
> Thunderbolt rounded/non-rounded cable support is two bits value. Correcting
> it as per the Thunderbolt 3 cable discover mode VDO changes done in the
> Thunderbolt 3 alternate mode header.
>
> Signed-off-by: Utkarsh Patel <[email protected]>

Acked-by: Enric Balletbo i Serra <[email protected]>

> --
> Changes in v2:
> - Removed the fixes tag as there is no functional implication.
> --
> ---
> drivers/platform/chrome/cros_ec_typec.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 31be31161350..8111ed1fc574 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -438,8 +438,7 @@ static int cros_typec_enable_tbt(struct cros_typec_data *typec,
> if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_LINK_UNIDIR)
> data.cable_mode |= TBT_CABLE_LINK_TRAINING;
>
> - if (pd_ctrl->cable_gen)
> - data.cable_mode |= TBT_CABLE_ROUNDED;
> + data.cable_mode |= TBT_SET_CABLE_ROUNDED(pd_ctrl->cable_gen);
>
> /* Enter Mode VDO */
> data.enter_vdo = TBT_SET_CABLE_SPEED(pd_ctrl->cable_speed);
>

2020-11-17 10:51:31

by Enric Balletbo i Serra

[permalink] [raw]

Subject: Re: [PATCH v2 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

On 13/11/20 21:25, 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 <[email protected]>
> Signed-off-by: Utkarsh Patel <[email protected]>

Acked-by: Enric Balletbo i Serra <[email protected]>

>
> --
> 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;
> +
> + 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);
>
> port->state.alt = NULL;
> port->state.data = &data;
>

2020-11-17 12:12:30

[permalink] [raw]

Subject: Re: [PATCH v2 5/8] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message

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.

I'm sorry, but I think that has to be explained better. We only need
the Thunderbolt 3 properties when we create the USB4 connection with
Thunderbolt 3 cables. With USB4 cables that information is simply not
available. Claiming that USB4 uses the same properties in general is
not true.

> Suggested-by: Heikki Krogerus <[email protected]>
> Signed-off-by: Utkarsh Patel <[email protected]>
> --
> 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

Please also explain the same here with a short comment. So basically,
if the USB4 connection is created using TBT3 cable, then we need to
supply also the TBT3 Cable VDO as part of this data. But if USB4
cable is used, then that member should not be filled at all.

> @@ -83,6 +84,7 @@ enum typec_orientation {
> */
> struct enter_usb_data {
> u32 eudo;
> + u32 tbt_cable_vdo;
> unsigned char active_link_training:1;
> };

thanks,

--
heikki

2020-11-17 12:13:52

[permalink] [raw]

Subject: Re: [PATCH v2 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

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

FWIW:

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

> --
> 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;
> +
> + 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);
>
> port->state.alt = NULL;
> port->state.data = &data;
> --
> 2.17.1

--
heikki

2020-11-17 12:23:58

[permalink] [raw]

Subject: Re: [PATCH v2 7/8] usb: typec: intel_pmc_mux: Configure active cable properties for USB4

On Fri, Nov 13, 2020 at 12:25:02PM -0800, Utkarsh Patel wrote:
> Value received as a part of Thunderbolt 3 cable discover mode VDO needs
> to be configured in the USB4 mode for the Thunderbolt rounded support and
> active cable plug link training.
>
> Suggested-by: Heikki Krogerus <[email protected]>
> Signed-off-by: Utkarsh Patel <[email protected]>
>
> --
> Changes in v2:
> - No change.
> --
> ---
> drivers/usb/typec/mux/intel_pmc_mux.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
> index aa3211f1c4c3..61feb358aad3 100644
> --- a/drivers/usb/typec/mux/intel_pmc_mux.c
> +++ b/drivers/usb/typec/mux/intel_pmc_mux.c
> @@ -295,6 +295,7 @@ pmc_usb_mux_usb4(struct pmc_usb_port *port, struct typec_mux_state *state)
> {
> struct enter_usb_data *data = state->data;
> struct altmode_req req = { };
> + u8 cable_rounded;
> u8 cable_speed;
>
> if (IOM_PORT_ACTIVITY_IS(port->iom_status, TBT) ||
> @@ -308,9 +309,6 @@ pmc_usb_mux_usb4(struct pmc_usb_port *port, struct typec_mux_state *state)
> /* USB4 Mode */
> req.mode_data = PMC_USB_ALTMODE_FORCE_LSR;
>
> - if (data->active_link_training)
> - req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK;
> -
> req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_SHIFT;
> req.mode_data |= (port->role - 1) << PMC_USB_ALTMODE_UFP_SHIFT;
>
> @@ -322,6 +320,20 @@ pmc_usb_mux_usb4(struct pmc_usb_port *port, struct typec_mux_state *state)
> fallthrough;
> default:
> req.mode_data |= PMC_USB_ALTMODE_ACTIVE_CABLE;
> +
> + if (data->tbt_cable_vdo) {
> + /* Active Thunderbolt 3 cable */
> + if (data->tbt_cable_vdo & TBT_CABLE_LINK_TRAINING)
> + req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK;
> +
> + cable_rounded =
> + TBT_CABLE_ROUNDED_SUPPORT(data->tbt_cable_vdo);

No need for the newline:

cable_rounded = TBT_CABLE_ROUNDED_SUPPORT(data->tbt_cable_vdo);

> + req.mode_data |= PMC_USB_ALTMODE_TBT_GEN(cable_rounded);
> + } else {
> + /* Active USB4 cable */
> + req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK |
> + PMC_USB_ALTMODE_TBT_GEN(1);
> + }
> break;
> }

thanks,

--
heikki

2020-11-17 18:21:33

[permalink] [raw]

Subject: Re: [PATCH v2 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

Hi Utkarsh,

On Fri, Nov 13, 2020 at 12:25:01PM -0800, Utkarsh Patel wrote:
> Configure Thunderbolt3/USB4 cable generation value by filing Thunderbolt 3
> cable discover mode VDO to support rounded and non-rounded Thunderbolt3/
> USB4 cables.
> While we are here use Thunderbolt 3 cable discover mode VDO to fill active
> cable plug link training value.
>
> Suggested-by: Heikki Krogerus <[email protected]>
> Signed-off-by: Utkarsh Patel <[email protected]>
>
> --
> 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

2020-11-17 21:21:16

[permalink] [raw]

Subject: Re: [PATCH v2 5/8] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message

Hi Utkarsh,

On Fri, Nov 13, 2020 at 12:25:00PM -0800, Utkarsh Patel wrote:
> USB4 also uses same cable properties as Thunderbolt 3 so use Thunderbolt 3
> cable discover mode VDO to fill details such as active cable plug link
> training and cable rounded support.
>
> Suggested-by: Heikki Krogerus <[email protected]>
> Signed-off-by: Utkarsh Patel <[email protected]>
>
> --
> 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

2020-11-17 22:40:57

[permalink] [raw]

Subject: RE: [PATCH v2 7/8] usb: typec: intel_pmc_mux: Configure active cable properties for USB4

Hi Heikki,

> -----Original Message-----
> From: Heikki Krogerus <[email protected]>
> Sent: Tuesday, November 17, 2020 4:22 AM
> To: Patel, Utkarsh H <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Mani, Rajmohan
> <[email protected]>; Shaikh, Azhar <[email protected]>
> Subject: Re: [PATCH v2 7/8] usb: typec: intel_pmc_mux: Configure active cable
> properties for USB4
>
> On Fri, Nov 13, 2020 at 12:25:02PM -0800, Utkarsh Patel wrote:
> > Value received as a part of Thunderbolt 3 cable discover mode VDO
> > needs to be configured in the USB4 mode for the Thunderbolt rounded
> > support and active cable plug link training.
> >
> > Suggested-by: Heikki Krogerus <[email protected]>
> > Signed-off-by: Utkarsh Patel <[email protected]>
> >
> > --
> > Changes in v2:
> > - No change.
> > --
> > ---
> > drivers/usb/typec/mux/intel_pmc_mux.c | 18 +++++++++++++++---
> > 1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c
> > b/drivers/usb/typec/mux/intel_pmc_mux.c
> > index aa3211f1c4c3..61feb358aad3 100644
> > --- a/drivers/usb/typec/mux/intel_pmc_mux.c
> > +++ b/drivers/usb/typec/mux/intel_pmc_mux.c
> > @@ -295,6 +295,7 @@ pmc_usb_mux_usb4(struct pmc_usb_port *port,
> struct
> > typec_mux_state *state) {
> > struct enter_usb_data *data = state->data;
> > struct altmode_req req = { };
> > + u8 cable_rounded;
> > u8 cable_speed;
> >
> > if (IOM_PORT_ACTIVITY_IS(port->iom_status, TBT) || @@ -308,9
> +309,6
> > @@ pmc_usb_mux_usb4(struct pmc_usb_port *port, struct
> typec_mux_state *state)
> > /* USB4 Mode */
> > req.mode_data = PMC_USB_ALTMODE_FORCE_LSR;
> >
> > - if (data->active_link_training)
> > - req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK;
> > -
> > req.mode_data |= (port->orientation - 1) <<
> PMC_USB_ALTMODE_ORI_SHIFT;
> > req.mode_data |= (port->role - 1) << PMC_USB_ALTMODE_UFP_SHIFT;
> >
> > @@ -322,6 +320,20 @@ pmc_usb_mux_usb4(struct pmc_usb_port *port,
> struct typec_mux_state *state)
> > fallthrough;
> > default:
> > req.mode_data |= PMC_USB_ALTMODE_ACTIVE_CABLE;
> > +
> > + if (data->tbt_cable_vdo) {
> > + /* Active Thunderbolt 3 cable */
> > + if (data->tbt_cable_vdo &
> TBT_CABLE_LINK_TRAINING)
> > + req.mode_data |=
> PMC_USB_ALTMODE_ACTIVE_LINK;
> > +
> > + cable_rounded =
> > + TBT_CABLE_ROUNDED_SUPPORT(data-
> >tbt_cable_vdo);
>
> No need for the newline:

Ack

>
> cable_rounded =
> TBT_CABLE_ROUNDED_SUPPORT(data->tbt_cable_vdo);
>
> > + req.mode_data |=
> PMC_USB_ALTMODE_TBT_GEN(cable_rounded);
> > + } else {
> > + /* Active USB4 cable */
> > + req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK
> |
> > + PMC_USB_ALTMODE_TBT_GEN(1);
> > + }
> > break;
> > }
>
> thanks,
>
> --
> Heikki

Sincerely,
Utkarsh Patel.

2020-11-17 23:30:48

[permalink] [raw]

Subject: RE: [PATCH v2 5/8] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message

Hi Heikki,

> -----Original Message-----
> From: Heikki Krogerus <[email protected]>
> Sent: Tuesday, November 17, 2020 4:10 AM
> To: Patel, Utkarsh H <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Mani, Rajmohan
> <[email protected]>; Shaikh, Azhar <[email protected]>
> Subject: Re: [PATCH v2 5/8] usb: typec: Use Thunderbolt 3 cable discover
> mode VDO in Enter_USB message
>
> 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.
>
> I'm sorry, but I think that has to be explained better. We only need the
> Thunderbolt 3 properties when we create the USB4 connection with
> Thunderbolt 3 cables. With USB4 cables that information is simply not
> available. Claiming that USB4 uses the same properties in general is not true.

Ack. I will change the commit message.

>
> > Suggested-by: Heikki Krogerus <[email protected]>
> > Signed-off-by: Utkarsh Patel <[email protected]>
> > --
> > 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
>
> Please also explain the same here with a short comment. So basically, if the
> USB4 connection is created using TBT3 cable, then we need to supply also the
> TBT3 Cable VDO as part of this data. But if USB4 cable is used, then that
> member should not be filled at all.

Ack.

>
> > @@ -83,6 +84,7 @@ enum typec_orientation {
> > */
> > struct enter_usb_data {
> > u32 eudo;
> > + u32 tbt_cable_vdo;
> > unsigned char active_link_training:1;
> > };
>
> thanks,
>
> --
> Heikki

Sincerely,
Utkarsh Patel.

2020-11-18 00:46:54

[permalink] [raw]

Subject: Re: [PATCH v2 5/8] usb: typec: Use Thunderbolt 3 cable discover mode VDO in Enter_USB message

On Tue, Nov 17, 2020 at 1:16 PM Prashant Malani <[email protected]> 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

2020-11-18 01:10:19

[permalink] [raw]

Subject: Re: [PATCH v2 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

On Tue, Nov 17, 2020 at 10:19 AM Prashant Malani <[email protected]> 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 <[email protected]>
> > Signed-off-by: Utkarsh Patel <[email protected]>
> >
> > --
> > 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

2020-11-18 04:10:08

[permalink] [raw]

Subject: RE: [PATCH v2 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3 cable discover mode VDO in USB4 mode

Hi Prashant,

Thank you for the review and feedback.

> On Tue, Nov 17, 2020 at 10:19 AM Prashant Malani <[email protected]>
> 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 <[email protected]>
> > > Signed-off-by: Utkarsh Patel <[email protected]>
> > >
> > > --
> > > 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).

No, not in the case of USB4.

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

Since we do not have these information available with USB4 cables, we can use them to check for TBT support and then set tbt_cable_vdo.

>
> Best regards,
>
> -Prashant

Sincerely,
Utkarsh Patel.

2020-11-18 11:57:32

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH v2 0/8] Thunderbolt3/USB4 cable rounded and active cable plug link training support

On Fri, Nov 13, 2020 at 12:24:55PM -0800, Utkarsh Patel wrote:
> This patch series adds the support for Thunderbolt3/USB4 rounded and
> non-rounded frequencies cables and fixes the active cable plug link
> training support.
>
> Changes in v2:
> - Removed the fixes tag as there is no functional implication from patches
> 1/8, 2/8 and 4/8.

I've queued up the first 4 patches of this series. Feel free to redo
the rest and resend.

thanks,

greg k-h

2020-11-18 22:05:47