[PATCH 1/2 v3] tcpm: During PR_SWAP, source caps should be sent only after tSwapSourceStart

[PATCH 1/2 v3] tcpm: During PR_SWAP, source caps should be sent only after tSwapSourceStart

[Badhri Jagan Sridharan]

From the spec:
"6.6.8.1 SwapSourceStartTimer
The SwapSourceStartTimer Shall be used by the new Source, after a Power
Role Swap or Fast Role Swap, to ensure that it does not send
Source_Capabilities Message before the new Sink is ready to receive the
Source_Capabilities Message. The new Source Shall Not send the
Source_Capabilities Message earlier than tSwapSourceStart after the
last bit of the EOP of GoodCRC Message sent in response to the PS_RDY
Message sent by the new Source indicating that its power supply is
ready."

This fixes TD.PD.CP.E3, TD.PD.CP.E4, TD.PD.CP.E5 failures

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
Changes since V1:
- Comment on the permissible values of tSwapSourceStart

Changes since V2:
- Fixing alignment issue pointed out by Guenter.
- Added Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/usb/typec/tcpm/tcpm.c | 2 +-
 include/linux/usb/pd.h        | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 3ef37202ee37..d38347bd3335 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -3555,7 +3555,7 @@ static void run_state_machine(struct tcpm_port *port)
 		 */
 		tcpm_set_pwr_role(port, TYPEC_SOURCE);
 		tcpm_pd_send_control(port, PD_CTRL_PS_RDY);
-		tcpm_set_state(port, SRC_STARTUP, 0);
+		tcpm_set_state(port, SRC_STARTUP, PD_T_SWAP_SRC_START);
 		break;
 
 	case VCONN_SWAP_ACCEPT:
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index b6c233e79bd4..1df895e4680b 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -473,6 +473,7 @@ static inline unsigned int rdo_max_power(u32 rdo)
 #define PD_T_ERROR_RECOVERY	100	/* minimum 25 is insufficient */
 #define PD_T_SRCSWAPSTDBY      625     /* Maximum of 650ms */
 #define PD_T_NEWSRC            250     /* Maximum of 275ms */
+#define PD_T_SWAP_SRC_START	20	/* Minimum of 20ms */
 
 #define PD_T_DRP_TRY		100	/* 75 - 150 ms */
 #define PD_T_DRP_TRYWAIT	600	/* 400 - 800 ms */
-- 
2.28.0.236.gb10cc79966-goog

[PATCH 2/2 v3] usb: typec: pd: Fix formatting in pd.h header

[Badhri Jagan Sridharan]

Replacing spaces with tabs for PD_T_* constants.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
Change history:
First version. Keeping the version number same as the parent.

---
 include/linux/usb/pd.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 1df895e4680b..f842e4589bd2 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -471,9 +471,10 @@ static inline unsigned int rdo_max_power(u32 rdo)
 #define PD_T_VCONN_SOURCE_ON	100
 #define PD_T_SINK_REQUEST	100	/* 100 ms minimum */
 #define PD_T_ERROR_RECOVERY	100	/* minimum 25 is insufficient */
-#define PD_T_SRCSWAPSTDBY      625     /* Maximum of 650ms */
-#define PD_T_NEWSRC            250     /* Maximum of 275ms */
+#define PD_T_SRCSWAPSTDBY	625	/* Maximum of 650ms */
+#define PD_T_NEWSRC		250	/* Maximum of 275ms */
 #define PD_T_SWAP_SRC_START	20	/* Minimum of 20ms */
+#define PD_T_BIST_CONT_MODE	50	/* 30 - 60 ms */
 
 #define PD_T_DRP_TRY		100	/* 75 - 150 ms */
 #define PD_T_DRP_TRYWAIT	600	/* 400 - 800 ms */
@@ -484,5 +485,4 @@ static inline unsigned int rdo_max_power(u32 rdo)
 #define PD_N_CAPS_COUNT		(PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
 #define PD_N_HARD_RESET_COUNT	2
 
-#define PD_T_BIST_CONT_MODE	50 /* 30 - 60 ms */
 #endif /* __LINUX_USB_PD_H */
-- 
2.28.0.236.gb10cc79966-goog

Re: [PATCH 2/2 v3] usb: typec: pd: Fix formatting in pd.h header

[Guenter Roeck]

On Tue, Aug 11, 2020 at 07:29:34PM -0700, Badhri Jagan Sridharan wrote:
> Replacing spaces with tabs for PD_T_* constants.
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Change history:
> First version. Keeping the version number same as the parent.
> 
> ---
>  include/linux/usb/pd.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index 1df895e4680b..f842e4589bd2 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -471,9 +471,10 @@ static inline unsigned int rdo_max_power(u32 rdo)
>  #define PD_T_VCONN_SOURCE_ON	100
>  #define PD_T_SINK_REQUEST	100	/* 100 ms minimum */
>  #define PD_T_ERROR_RECOVERY	100	/* minimum 25 is insufficient */
> -#define PD_T_SRCSWAPSTDBY      625     /* Maximum of 650ms */
> -#define PD_T_NEWSRC            250     /* Maximum of 275ms */
> +#define PD_T_SRCSWAPSTDBY	625	/* Maximum of 650ms */
> +#define PD_T_NEWSRC		250	/* Maximum of 275ms */
>  #define PD_T_SWAP_SRC_START	20	/* Minimum of 20ms */
> +#define PD_T_BIST_CONT_MODE	50	/* 30 - 60 ms */
>  
>  #define PD_T_DRP_TRY		100	/* 75 - 150 ms */
>  #define PD_T_DRP_TRYWAIT	600	/* 400 - 800 ms */
> @@ -484,5 +485,4 @@ static inline unsigned int rdo_max_power(u32 rdo)
>  #define PD_N_CAPS_COUNT		(PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
>  #define PD_N_HARD_RESET_COUNT	2
>  
> -#define PD_T_BIST_CONT_MODE	50 /* 30 - 60 ms */
>  #endif /* __LINUX_USB_PD_H */
> -- 
> 2.28.0.236.gb10cc79966-goog
> 

Re: [PATCH 1/2 v3] tcpm: During PR_SWAP, source caps should be sent only after tSwapSourceStart

[Heikki Krogerus]

Hi,

> This fixes TD.PD.CP.E3, TD.PD.CP.E4, TD.PD.CP.E5 failures

I'm sorry, but that statement is way too cryptic. You really need to
explain what those are.

thanks,

-- 
heikki

Re: [PATCH 2/2 v3] usb: typec: pd: Fix formatting in pd.h header

[Heikki Krogerus]

On Tue, Aug 11, 2020 at 07:29:34PM -0700, Badhri Jagan Sridharan wrote:
> Replacing spaces with tabs for PD_T_* constants.
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>

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

> ---
> Change history:
> First version. Keeping the version number same as the parent.
> 
> ---
>  include/linux/usb/pd.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index 1df895e4680b..f842e4589bd2 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -471,9 +471,10 @@ static inline unsigned int rdo_max_power(u32 rdo)
>  #define PD_T_VCONN_SOURCE_ON	100
>  #define PD_T_SINK_REQUEST	100	/* 100 ms minimum */
>  #define PD_T_ERROR_RECOVERY	100	/* minimum 25 is insufficient */
> -#define PD_T_SRCSWAPSTDBY      625     /* Maximum of 650ms */
> -#define PD_T_NEWSRC            250     /* Maximum of 275ms */
> +#define PD_T_SRCSWAPSTDBY	625	/* Maximum of 650ms */
> +#define PD_T_NEWSRC		250	/* Maximum of 275ms */
>  #define PD_T_SWAP_SRC_START	20	/* Minimum of 20ms */
> +#define PD_T_BIST_CONT_MODE	50	/* 30 - 60 ms */
>  
>  #define PD_T_DRP_TRY		100	/* 75 - 150 ms */
>  #define PD_T_DRP_TRYWAIT	600	/* 400 - 800 ms */
> @@ -484,5 +485,4 @@ static inline unsigned int rdo_max_power(u32 rdo)
>  #define PD_N_CAPS_COUNT		(PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
>  #define PD_N_HARD_RESET_COUNT	2
>  
> -#define PD_T_BIST_CONT_MODE	50 /* 30 - 60 ms */
>  #endif /* __LINUX_USB_PD_H */
> -- 
> 2.28.0.236.gb10cc79966-goog

thanks,

-- 
heikki

Re: [PATCH 1/2 v3] tcpm: During PR_SWAP, source caps should be sent only after tSwapSourceStart

[Badhri Jagan Sridharan]

Hi Heikki,

Sure. Same as the other patch I will try to address your question here
and will update the commit description once you are satisfied with the
description.

Subject:
During PR_SWAP, source caps should be sent only after tSwapSourceStart

Commit description:
The patch addresses the compliance test failures while running
TD.PD.CP.E3, TD.PD.CP.E4, TD.PD.CP.E5 of the "Deterministic PD
Compliance MOI" test plan published in https://www.usb.org/usbc.
For a product to be Type-C compliant, it's expected that these tests
are run on usb.org certified Type-C compliance tester as mentioned in
https://www.usb.org/usbc.

The purpose of the tests TD.PD.CP.E3, TD.PD.CP.E4, TD.PD.CP.E5 is to
verify the PR_SWAP response of the device. While doing so, the test
asserts that Source Capabilities message is NOT received from the test
device within tSwapSourceStart min (20 ms) from the time the last bit
of GoodCRC corresponding to the RS_RDY message sent by the UUT was
sent. If it does then the test fails.

This is in line with the requirements from the USB Power Delivery
Specification Revision 3.0, Version 1.2:
"6.6.8.1 SwapSourceStartTimer
The SwapSourceStartTimer Shall be used by the new Source, after a
Power Role Swap or Fast Role Swap, to ensure that it does not send
Source_Capabilities Message before the new Sink is ready to receive
the
Source_Capabilities Message. The new Source Shall Not send the
Source_Capabilities Message earlier than tSwapSourceStart after the
last bit of the EOP of GoodCRC Message sent in response to the PS_RDY
Message sent by the new Source indicating that its power supply is
ready."

The patch makes sure that TCPM does not send the Source_Capabilities
Message within tSwapSourceStart(20ms) by transitioning into
SRC_STARTUP only after  tSwapSourceStart(20ms).

Thanks,
Badhri

On Thu, Aug 13, 2020 at 1:25 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi,
>
> > This fixes TD.PD.CP.E3, TD.PD.CP.E4, TD.PD.CP.E5 failures
>
> I'm sorry, but that statement is way too cryptic. You really need to
> explain what those are.
>
> thanks,
>
> --
> heikki

Re: [PATCH 1/2 v3] tcpm: During PR_SWAP, source caps should be sent only after tSwapSourceStart

[Heikki Krogerus]

On Thu, Aug 13, 2020 at 12:40:04PM -0700, Badhri Jagan Sridharan wrote:
> Hi Heikki,
> 
> Sure. Same as the other patch I will try to address your question here
> and will update the commit description once you are satisfied with the
> description.
> 
> Subject:
> During PR_SWAP, source caps should be sent only after tSwapSourceStart
> 
> Commit description:
> The patch addresses the compliance test failures while running
> TD.PD.CP.E3, TD.PD.CP.E4, TD.PD.CP.E5 of the "Deterministic PD
> Compliance MOI" test plan published in https://www.usb.org/usbc.
> For a product to be Type-C compliant, it's expected that these tests
> are run on usb.org certified Type-C compliance tester as mentioned in
> https://www.usb.org/usbc.
> 
> The purpose of the tests TD.PD.CP.E3, TD.PD.CP.E4, TD.PD.CP.E5 is to
> verify the PR_SWAP response of the device. While doing so, the test
> asserts that Source Capabilities message is NOT received from the test
> device within tSwapSourceStart min (20 ms) from the time the last bit
> of GoodCRC corresponding to the RS_RDY message sent by the UUT was
> sent. If it does then the test fails.
> 
> This is in line with the requirements from the USB Power Delivery
> Specification Revision 3.0, Version 1.2:
> "6.6.8.1 SwapSourceStartTimer
> The SwapSourceStartTimer Shall be used by the new Source, after a
> Power Role Swap or Fast Role Swap, to ensure that it does not send
> Source_Capabilities Message before the new Sink is ready to receive
> the
> Source_Capabilities Message. The new Source Shall Not send the
> Source_Capabilities Message earlier than tSwapSourceStart after the
> last bit of the EOP of GoodCRC Message sent in response to the PS_RDY
> Message sent by the new Source indicating that its power supply is
> ready."
> 
> The patch makes sure that TCPM does not send the Source_Capabilities
> Message within tSwapSourceStart(20ms) by transitioning into
> SRC_STARTUP only after  tSwapSourceStart(20ms).

That looks good to me. Thanks.

Br,

-- 
heikki