[PATCH v2, 1/2] dt-bindings: connector: Add time property for sender response

[PATCH v2, 1/2] dt-bindings: connector: Add time property for sender response

[joswang]

From: Jos Wang <joswang@lenovo.com>

Add "pd2-sender-response-time-ms" and "pd3-sender-response-time-ms"
DT attributes to better solve the following test items in the usb
pd compliance test:
  TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
  TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout

This is to enable setting of platform/board specific timer values as
these timers have a range of acceptable values.

[1] https://usb.org/document-library/
    usb-power-delivery-compliance-test-specification-0/USB_PD3_CTS_Q4_2024_OR.zip

Signed-off-by: Jos Wang <joswang@lenovo.com>
---
v1 -> v2:
- modify the commit message
- patch 1/2 and patch 2/2 are placed in the same thread

 .../bindings/connector/usb-connector.yaml     | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 67700440e23b..83be66f6d406 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -293,6 +293,26 @@ properties:
       PD negotiation till BC1.2 detection completes.
     default: 0
 
+  pd2-sender-response-time-ms:
+    description: Represents the PD20 max time in ms that port started from
+      the time the last bit of the GoodCRC Message EOP has been received by
+      the Physical Layer, it shall be stopped when the last bit of the expected
+      response Message EOP has been received by the Physical Layer. A timeout
+      leads to a hard reset message by the port.
+    minimum: 24
+    maximum: 30
+    default: 27
+
+  pd3-sender-response-time-ms:
+    description: Represents the PD30 max time in ms that port started from
+      the time the last bit of the GoodCRC Message EOP has been received by
+      the Physical Layer, it shall be stopped when the last bit of the expected
+      response Message EOP has been received by the Physical Layer. A timeout
+      leads to a hard reset message by the port.
+    minimum: 27
+    maximum: 33
+    default: 30
+
 dependencies:
   sink-vdos-v1: [ sink-vdos ]
   sink-vdos: [ sink-vdos-v1 ]
-- 
2.17.1

[PATCH v2, 2/2] usb: typec: tcpm: fix the sender response time issue

[joswang]

From: Jos Wang <joswang@lenovo.com>

According to the USB PD3 CTS specification
(https://usb.org/document-library/
usb-power-delivery-compliance-test-specification-0/
USB_PD3_CTS_Q4_2024_OR.zip), the requirements for
tSenderResponse are different in PD2 and PD3 modes, see
Table 19 Timing Table & Calculations. For PD2 mode, the
tSenderResponse min 24ms and max 30ms; for PD3 mode, the
tSenderResponse min 27ms and max 33ms.

For the "TEST.PD.PROT.SRC.2 Get_Source_Cap No Request" test
item, after receiving the Source_Capabilities Message sent by
the UUT, the tester deliberately does not send a Request Message
in order to force the SenderResponse timer on the Source UUT to
timeout. The Tester checks that a Hard Reset is detected between
tSenderResponse min and max，the delay is between the last bit of
the GoodCRC Message EOP has been sent and the first bit of Hard
Reset SOP has been received. The current code does not distinguish
between PD2 and PD3 modes, and tSenderResponse defaults to 60ms.
This will cause this test item and the following tests to fail:
TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout

Considering factors such as SOC performance, i2c rate, and the speed
of PD chip sending data, "pd2-sender-response-time-ms" and
"pd3-sender-response-time-ms" DT time properties are added to allow
users to define platform timing. For values that have not been
explicitly defined in DT using this property, a default value of 27ms
for PD2 tSenderResponse and 30ms for PD3 tSenderResponse is set.

Fixes: 2eadc33f40d4 ("typec: tcpm: Add core support for sink side PPS")
Cc: stable@vger.kernel.org
Signed-off-by: Jos Wang <joswang@lenovo.com>
---
v1 -> v2:
- modify the commit message
- patch 1/2 and patch 2/2 are placed in the same thread

 drivers/usb/typec/tcpm/tcpm.c | 50 +++++++++++++++++++++++------------
 include/linux/usb/pd.h        |  3 ++-
 2 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 6021eeb903fe..3a159bfcf382 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -314,12 +314,16 @@ struct pd_data {
  * @sink_wait_cap_time: Deadline (in ms) for tTypeCSinkWaitCap timer
  * @ps_src_wait_off_time: Deadline (in ms) for tPSSourceOff timer
  * @cc_debounce_time: Deadline (in ms) for tCCDebounce timer
+ * @pd2_sender_response_time: Deadline (in ms) for pd20 tSenderResponse timer
+ * @pd3_sender_response_time: Deadline (in ms) for pd30 tSenderResponse timer
  */
 struct pd_timings {
 	u32 sink_wait_cap_time;
 	u32 ps_src_off_time;
 	u32 cc_debounce_time;
 	u32 snk_bc12_cmpletion_time;
+	u32 pd2_sender_response_time;
+	u32 pd3_sender_response_time;
 };
 
 struct tcpm_port {
@@ -3776,7 +3780,9 @@ static bool tcpm_send_queued_message(struct tcpm_port *port)
 			} else if (port->pwr_role == TYPEC_SOURCE) {
 				tcpm_ams_finish(port);
 				tcpm_set_state(port, HARD_RESET_SEND,
-					       PD_T_SENDER_RESPONSE);
+					       port->negotiated_rev >= PD_REV30 ?
+					       port->timings.pd3_sender_response_time :
+					       port->timings.pd2_sender_response_time);
 			} else {
 				tcpm_ams_finish(port);
 			}
@@ -4619,6 +4625,9 @@ static void run_state_machine(struct tcpm_port *port)
 	enum typec_pwr_opmode opmode;
 	unsigned int msecs;
 	enum tcpm_state upcoming_state;
+	u32 sender_response_time = port->negotiated_rev >= PD_REV30 ?
+				   port->timings.pd3_sender_response_time :
+				   port->timings.pd2_sender_response_time;
 
 	if (port->tcpc->check_contaminant && port->state != CHECK_CONTAMINANT)
 		port->potential_contaminant = ((port->enter_state == SRC_ATTACH_WAIT &&
@@ -5113,7 +5122,7 @@ static void run_state_machine(struct tcpm_port *port)
 			tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
 		} else {
 			tcpm_set_state_cond(port, hard_reset_state(port),
-					    PD_T_SENDER_RESPONSE);
+					    sender_response_time);
 		}
 		break;
 	case SNK_NEGOTIATE_PPS_CAPABILITIES:
@@ -5135,7 +5144,7 @@ static void run_state_machine(struct tcpm_port *port)
 				tcpm_set_state(port, SNK_READY, 0);
 		} else {
 			tcpm_set_state_cond(port, hard_reset_state(port),
-					    PD_T_SENDER_RESPONSE);
+					    sender_response_time);
 		}
 		break;
 	case SNK_TRANSITION_SINK:
@@ -5387,7 +5396,7 @@ static void run_state_machine(struct tcpm_port *port)
 			port->message_id_prime = 0;
 			port->rx_msgid_prime = -1;
 			tcpm_pd_send_control(port, PD_CTRL_SOFT_RESET, TCPC_TX_SOP_PRIME);
-			tcpm_set_state_cond(port, ready_state(port), PD_T_SENDER_RESPONSE);
+			tcpm_set_state_cond(port, ready_state(port), sender_response_time);
 		} else {
 			port->message_id = 0;
 			port->rx_msgid = -1;
@@ -5398,7 +5407,7 @@ static void run_state_machine(struct tcpm_port *port)
 				tcpm_set_state_cond(port, hard_reset_state(port), 0);
 			else
 				tcpm_set_state_cond(port, hard_reset_state(port),
-						    PD_T_SENDER_RESPONSE);
+						    sender_response_time);
 		}
 		break;
 
@@ -5409,8 +5418,7 @@ static void run_state_machine(struct tcpm_port *port)
 			port->send_discover = true;
 			port->send_discover_prime = false;
 		}
-		tcpm_set_state_cond(port, DR_SWAP_SEND_TIMEOUT,
-				    PD_T_SENDER_RESPONSE);
+		tcpm_set_state_cond(port, DR_SWAP_SEND_TIMEOUT, sender_response_time);
 		break;
 	case DR_SWAP_ACCEPT:
 		tcpm_pd_send_control(port, PD_CTRL_ACCEPT, TCPC_TX_SOP);
@@ -5444,7 +5452,7 @@ static void run_state_machine(struct tcpm_port *port)
 			tcpm_set_state(port, ERROR_RECOVERY, 0);
 			break;
 		}
-		tcpm_set_state_cond(port, FR_SWAP_SEND_TIMEOUT, PD_T_SENDER_RESPONSE);
+		tcpm_set_state_cond(port, FR_SWAP_SEND_TIMEOUT, sender_response_time);
 		break;
 	case FR_SWAP_SEND_TIMEOUT:
 		tcpm_set_state(port, ERROR_RECOVERY, 0);
@@ -5475,8 +5483,7 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 	case PR_SWAP_SEND:
 		tcpm_pd_send_control(port, PD_CTRL_PR_SWAP, TCPC_TX_SOP);
-		tcpm_set_state_cond(port, PR_SWAP_SEND_TIMEOUT,
-				    PD_T_SENDER_RESPONSE);
+		tcpm_set_state_cond(port, PR_SWAP_SEND_TIMEOUT, sender_response_time);
 		break;
 	case PR_SWAP_SEND_TIMEOUT:
 		tcpm_swap_complete(port, -ETIMEDOUT);
@@ -5574,8 +5581,7 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 	case VCONN_SWAP_SEND:
 		tcpm_pd_send_control(port, PD_CTRL_VCONN_SWAP, TCPC_TX_SOP);
-		tcpm_set_state(port, VCONN_SWAP_SEND_TIMEOUT,
-			       PD_T_SENDER_RESPONSE);
+		tcpm_set_state(port, VCONN_SWAP_SEND_TIMEOUT, sender_response_time);
 		break;
 	case VCONN_SWAP_SEND_TIMEOUT:
 		tcpm_swap_complete(port, -ETIMEDOUT);
@@ -5656,23 +5662,21 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 	case GET_STATUS_SEND:
 		tcpm_pd_send_control(port, PD_CTRL_GET_STATUS, TCPC_TX_SOP);
-		tcpm_set_state(port, GET_STATUS_SEND_TIMEOUT,
-			       PD_T_SENDER_RESPONSE);
+		tcpm_set_state(port, GET_STATUS_SEND_TIMEOUT, sender_response_time);
 		break;
 	case GET_STATUS_SEND_TIMEOUT:
 		tcpm_set_state(port, ready_state(port), 0);
 		break;
 	case GET_PPS_STATUS_SEND:
 		tcpm_pd_send_control(port, PD_CTRL_GET_PPS_STATUS, TCPC_TX_SOP);
-		tcpm_set_state(port, GET_PPS_STATUS_SEND_TIMEOUT,
-			       PD_T_SENDER_RESPONSE);
+		tcpm_set_state(port, GET_PPS_STATUS_SEND_TIMEOUT, sender_response_time);
 		break;
 	case GET_PPS_STATUS_SEND_TIMEOUT:
 		tcpm_set_state(port, ready_state(port), 0);
 		break;
 	case GET_SINK_CAP:
 		tcpm_pd_send_control(port, PD_CTRL_GET_SINK_CAP, TCPC_TX_SOP);
-		tcpm_set_state(port, GET_SINK_CAP_TIMEOUT, PD_T_SENDER_RESPONSE);
+		tcpm_set_state(port, GET_SINK_CAP_TIMEOUT, sender_response_time);
 		break;
 	case GET_SINK_CAP_TIMEOUT:
 		port->sink_cap_done = true;
@@ -7109,6 +7113,18 @@ static void tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fw
 	ret = fwnode_property_read_u32(fwnode, "sink-bc12-completion-time-ms", &val);
 	if (!ret)
 		port->timings.snk_bc12_cmpletion_time = val;
+
+	ret = fwnode_property_read_u32(fwnode, "pd2-sender-response-time-ms", &val);
+	if (!ret)
+		port->timings.pd2_sender_response_time = val;
+	else
+		port->timings.pd2_sender_response_time = PD_T_PD2_SENDER_RESPONSE;
+
+	ret = fwnode_property_read_u32(fwnode, "pd3-sender-response-time-ms", &val);
+	if (!ret)
+		port->timings.pd3_sender_response_time = val;
+	else
+		port->timings.pd3_sender_response_time = PD_T_PD3_SENDER_RESPONSE;
 }
 
 static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index d50098fb16b5..9c599e851b9a 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -457,7 +457,6 @@ static inline unsigned int rdo_max_power(u32 rdo)
 #define PD_T_NO_RESPONSE	5000	/* 4.5 - 5.5 seconds */
 #define PD_T_DB_DETECT		10000	/* 10 - 15 seconds */
 #define PD_T_SEND_SOURCE_CAP	150	/* 100 - 200 ms */
-#define PD_T_SENDER_RESPONSE	60	/* 24 - 30 ms, relaxed */
 #define PD_T_RECEIVER_RESPONSE	15	/* 15ms max */
 #define PD_T_SOURCE_ACTIVITY	45
 #define PD_T_SINK_ACTIVITY	135
@@ -491,6 +490,8 @@ static inline unsigned int rdo_max_power(u32 rdo)
 #define PD_T_CC_DEBOUNCE	200	/* 100 - 200 ms */
 #define PD_T_PD_DEBOUNCE	20	/* 10 - 20 ms */
 #define PD_T_TRY_CC_DEBOUNCE	15	/* 10 - 20 ms */
+#define PD_T_PD2_SENDER_RESPONSE	27	/* PD20 spec 24 - 30 ms */
+#define PD_T_PD3_SENDER_RESPONSE	30	/* PD30 spec 27 - 33 ms */
 
 #define PD_N_CAPS_COUNT		(PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
 #define PD_N_HARD_RESET_COUNT	2
-- 
2.17.1

Re: [PATCH v2, 2/2] usb: typec: tcpm: fix the sender response time issue

[Dmitry Baryshkov]

On Sun, Dec 22, 2024 at 06:52:39PM +0800, joswang wrote:
> From: Jos Wang <joswang@lenovo.com>
> 
> According to the USB PD3 CTS specification
> (https://usb.org/document-library/
> usb-power-delivery-compliance-test-specification-0/
> USB_PD3_CTS_Q4_2024_OR.zip), the requirements for
> tSenderResponse are different in PD2 and PD3 modes, see
> Table 19 Timing Table & Calculations. For PD2 mode, the
> tSenderResponse min 24ms and max 30ms; for PD3 mode, the
> tSenderResponse min 27ms and max 33ms.
> 
> For the "TEST.PD.PROT.SRC.2 Get_Source_Cap No Request" test
> item, after receiving the Source_Capabilities Message sent by
> the UUT, the tester deliberately does not send a Request Message
> in order to force the SenderResponse timer on the Source UUT to
> timeout. The Tester checks that a Hard Reset is detected between
> tSenderResponse min and max，the delay is between the last bit of
> the GoodCRC Message EOP has been sent and the first bit of Hard
> Reset SOP has been received. The current code does not distinguish
> between PD2 and PD3 modes, and tSenderResponse defaults to 60ms.
> This will cause this test item and the following tests to fail:
> TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
> TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout
> 
> Considering factors such as SOC performance, i2c rate, and the speed
> of PD chip sending data, "pd2-sender-response-time-ms" and
> "pd3-sender-response-time-ms" DT time properties are added to allow
> users to define platform timing. For values that have not been
> explicitly defined in DT using this property, a default value of 27ms
> for PD2 tSenderResponse and 30ms for PD3 tSenderResponse is set.

You have several different changes squashed into the same commit:
- Change the timeout from 60 ms to 27-30 ms (I'd recommend using 27 ms
  as it fits both 24-30 ms and 27-33 ms ranges,
- Make timeout depend on the PD version,
- Make timeouts configurable via DT.

Only the first item is a fix per se and only that change should be
considered for backporting. Please unsquash your changes into logical
commits.  Theoretically the second change can be thought about as a part
of the third change (making timeouts configurable) or of the fist change
(fix the timeout to follow the standard), but I'd suggest having three
separate commits.

> 
> Fixes: 2eadc33f40d4 ("typec: tcpm: Add core support for sink side PPS")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jos Wang <joswang@lenovo.com>
> ---
> v1 -> v2:
> - modify the commit message
> - patch 1/2 and patch 2/2 are placed in the same thread

-- 
With best wishes
Dmitry

Re: [PATCH v2, 1/2] dt-bindings: connector: Add time property for sender response

[Rob Herring]

On Sun, Dec 22, 2024 at 06:52:38PM +0800, joswang wrote:
> From: Jos Wang <joswang@lenovo.com>
> 
> Add "pd2-sender-response-time-ms" and "pd3-sender-response-time-ms"
> DT attributes to better solve the following test items in the usb
> pd compliance test:
>   TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
>   TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout
> 
> This is to enable setting of platform/board specific timer values as
> these timers have a range of acceptable values.
> 
> [1] https://usb.org/document-library/
>     usb-power-delivery-compliance-test-specification-0/USB_PD3_CTS_Q4_2024_OR.zip

Don't wrap this line.

> 
> Signed-off-by: Jos Wang <joswang@lenovo.com>
> ---
> v1 -> v2:
> - modify the commit message
> - patch 1/2 and patch 2/2 are placed in the same thread
> 
>  .../bindings/connector/usb-connector.yaml     | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index 67700440e23b..83be66f6d406 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -293,6 +293,26 @@ properties:
>        PD negotiation till BC1.2 detection completes.
>      default: 0
>  
> +  pd2-sender-response-time-ms:
> +    description: Represents the PD20 max time in ms that port started from
> +      the time the last bit of the GoodCRC Message EOP has been received by
> +      the Physical Layer, it shall be stopped when the last bit of the expected
> +      response Message EOP has been received by the Physical Layer. A timeout
> +      leads to a hard reset message by the port.

I can only guess that 'pd2' and 'PD20' refer to Power Delivery 2.0.

> +    minimum: 24
> +    maximum: 30
> +    default: 27
> +
> +  pd3-sender-response-time-ms:
> +    description: Represents the PD30 max time in ms that port started from
> +      the time the last bit of the GoodCRC Message EOP has been received by
> +      the Physical Layer, it shall be stopped when the last bit of the expected
> +      response Message EOP has been received by the Physical Layer. A timeout
> +      leads to a hard reset message by the port.

And pd3?

> +    minimum: 27
> +    maximum: 33
> +    default: 30
> +
>  dependencies:
>    sink-vdos-v1: [ sink-vdos ]
>    sink-vdos: [ sink-vdos-v1 ]
> -- 
> 2.17.1
> 

Re: [PATCH v2, 2/2] usb: typec: tcpm: fix the sender response time issue

[Jos Wang]

Hi, thanks for your help in reviewing the code, and happy new year to
you and your family!

For the first commit you mentioned (modification time is 27ms), I
understand that just modify the include/linux/usb/pd.h file:
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index d50098fb16b5..cd2cc535d21d 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -457,7 +457,7 @@ static inline unsigned int rdo_max_power(u32 rdo)
 #define PD_T_NO_RESPONSE       5000    /* 4.5 - 5.5 seconds */
 #define PD_T_DB_DETECT         10000   /* 10 - 15 seconds */
 #define PD_T_SEND_SOURCE_CAP   150     /* 100 - 200 ms */
-#define PD_T_SENDER_RESPONSE   60      /* 24 - 30 ms, relaxed */
+#define PD_T_SENDER_RESPONSE   27      /* 24 - 30 ms, relaxed */
 #define PD_T_RECEIVER_RESPONSE 15      /* 15ms max */
 #define PD_T_SOURCE_ACTIVITY   45
 #define PD_T_SINK_ACTIVITY     135

Is my understanding correct?


Thanks
Jos Wang

On Sun, Dec 22, 2024 at 9:14 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Sun, Dec 22, 2024 at 06:52:39PM +0800, joswang wrote:
> > From: Jos Wang <joswang@lenovo.com>
> >
> > According to the USB PD3 CTS specification
> > (https://usb.org/document-library/
> > usb-power-delivery-compliance-test-specification-0/
> > USB_PD3_CTS_Q4_2024_OR.zip), the requirements for
> > tSenderResponse are different in PD2 and PD3 modes, see
> > Table 19 Timing Table & Calculations. For PD2 mode, the
> > tSenderResponse min 24ms and max 30ms; for PD3 mode, the
> > tSenderResponse min 27ms and max 33ms.
> >
> > For the "TEST.PD.PROT.SRC.2 Get_Source_Cap No Request" test
> > item, after receiving the Source_Capabilities Message sent by
> > the UUT, the tester deliberately does not send a Request Message
> > in order to force the SenderResponse timer on the Source UUT to
> > timeout. The Tester checks that a Hard Reset is detected between
> > tSenderResponse min and max，the delay is between the last bit of
> > the GoodCRC Message EOP has been sent and the first bit of Hard
> > Reset SOP has been received. The current code does not distinguish
> > between PD2 and PD3 modes, and tSenderResponse defaults to 60ms.
> > This will cause this test item and the following tests to fail:
> > TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
> > TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout
> >
> > Considering factors such as SOC performance, i2c rate, and the speed
> > of PD chip sending data, "pd2-sender-response-time-ms" and
> > "pd3-sender-response-time-ms" DT time properties are added to allow
> > users to define platform timing. For values that have not been
> > explicitly defined in DT using this property, a default value of 27ms
> > for PD2 tSenderResponse and 30ms for PD3 tSenderResponse is set.
>
> You have several different changes squashed into the same commit:
> - Change the timeout from 60 ms to 27-30 ms (I'd recommend using 27 ms
>   as it fits both 24-30 ms and 27-33 ms ranges,
> - Make timeout depend on the PD version,
> - Make timeouts configurable via DT.
>
> Only the first item is a fix per se and only that change should be
> considered for backporting. Please unsquash your changes into logical
> commits.  Theoretically the second change can be thought about as a part
> of the third change (making timeouts configurable) or of the fist change
> (fix the timeout to follow the standard), but I'd suggest having three
> separate commits.
>
> >
> > Fixes: 2eadc33f40d4 ("typec: tcpm: Add core support for sink side PPS")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jos Wang <joswang@lenovo.com>
> > ---
> > v1 -> v2:
> > - modify the commit message
> > - patch 1/2 and patch 2/2 are placed in the same thread
>
> --
> With best wishes
> Dmitry

Re: [PATCH v2, 2/2] usb: typec: tcpm: fix the sender response time issue

[Dmitry Baryshkov]

On Wed, Jan 01, 2025 at 09:00:01PM +0800, Jos Wang wrote:
> Hi, thanks for your help in reviewing the code, and happy new year to
> you and your family!
> 
> For the first commit you mentioned (modification time is 27ms), I
> understand that just modify the include/linux/usb/pd.h file:

In future please respond under the comment, not at the top of the
message. Thank you.

> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index d50098fb16b5..cd2cc535d21d 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -457,7 +457,7 @@ static inline unsigned int rdo_max_power(u32 rdo)
>  #define PD_T_NO_RESPONSE       5000    /* 4.5 - 5.5 seconds */
>  #define PD_T_DB_DETECT         10000   /* 10 - 15 seconds */
>  #define PD_T_SEND_SOURCE_CAP   150     /* 100 - 200 ms */
> -#define PD_T_SENDER_RESPONSE   60      /* 24 - 30 ms, relaxed */
> +#define PD_T_SENDER_RESPONSE   27      /* 24 - 30 ms, relaxed */
>  #define PD_T_RECEIVER_RESPONSE 15      /* 15ms max */
>  #define PD_T_SOURCE_ACTIVITY   45
>  #define PD_T_SINK_ACTIVITY     135
> 
> Is my understanding correct?

Yes.

> 
> 
> Thanks
> Jos Wang
> 
> On Sun, Dec 22, 2024 at 9:14 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Sun, Dec 22, 2024 at 06:52:39PM +0800, joswang wrote:
> > > From: Jos Wang <joswang@lenovo.com>
> > >
> > > According to the USB PD3 CTS specification
> > > (https://usb.org/document-library/
> > > usb-power-delivery-compliance-test-specification-0/
> > > USB_PD3_CTS_Q4_2024_OR.zip), the requirements for
> > > tSenderResponse are different in PD2 and PD3 modes, see
> > > Table 19 Timing Table & Calculations. For PD2 mode, the
> > > tSenderResponse min 24ms and max 30ms; for PD3 mode, the
> > > tSenderResponse min 27ms and max 33ms.
> > >
> > > For the "TEST.PD.PROT.SRC.2 Get_Source_Cap No Request" test
> > > item, after receiving the Source_Capabilities Message sent by
> > > the UUT, the tester deliberately does not send a Request Message
> > > in order to force the SenderResponse timer on the Source UUT to
> > > timeout. The Tester checks that a Hard Reset is detected between
> > > tSenderResponse min and max，the delay is between the last bit of
> > > the GoodCRC Message EOP has been sent and the first bit of Hard
> > > Reset SOP has been received. The current code does not distinguish
> > > between PD2 and PD3 modes, and tSenderResponse defaults to 60ms.
> > > This will cause this test item and the following tests to fail:
> > > TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
> > > TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout
> > >
> > > Considering factors such as SOC performance, i2c rate, and the speed
> > > of PD chip sending data, "pd2-sender-response-time-ms" and
> > > "pd3-sender-response-time-ms" DT time properties are added to allow
> > > users to define platform timing. For values that have not been
> > > explicitly defined in DT using this property, a default value of 27ms
> > > for PD2 tSenderResponse and 30ms for PD3 tSenderResponse is set.
> >
> > You have several different changes squashed into the same commit:
> > - Change the timeout from 60 ms to 27-30 ms (I'd recommend using 27 ms
> >   as it fits both 24-30 ms and 27-33 ms ranges,
> > - Make timeout depend on the PD version,
> > - Make timeouts configurable via DT.
> >
> > Only the first item is a fix per se and only that change should be
> > considered for backporting. Please unsquash your changes into logical
> > commits.  Theoretically the second change can be thought about as a part
> > of the third change (making timeouts configurable) or of the fist change
> > (fix the timeout to follow the standard), but I'd suggest having three
> > separate commits.
> >
> > >
> > > Fixes: 2eadc33f40d4 ("typec: tcpm: Add core support for sink side PPS")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Jos Wang <joswang@lenovo.com>
> > > ---
> > > v1 -> v2:
> > > - modify the commit message
> > > - patch 1/2 and patch 2/2 are placed in the same thread
> >
> > --
> > With best wishes
> > Dmitry

-- 
With best wishes
Dmitry

Re: [PATCH v2, 1/2] dt-bindings: connector: Add time property for sender response

[Jos Wang]

On Tue, Dec 31, 2024 at 9:47 PM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Dec 22, 2024 at 06:52:38PM +0800, joswang wrote:
> > From: Jos Wang <joswang@lenovo.com>
> >
> > Add "pd2-sender-response-time-ms" and "pd3-sender-response-time-ms"
> > DT attributes to better solve the following test items in the usb
> > pd compliance test:
> >   TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
> >   TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout
> >
> > This is to enable setting of platform/board specific timer values as
> > these timers have a range of acceptable values.
> >
> > [1] https://usb.org/document-library/
> >     usb-power-delivery-compliance-test-specification-0/USB_PD3_CTS_Q4_2024_OR.zip
>
> Don't wrap this line.
>
OK

> >
> > Signed-off-by: Jos Wang <joswang@lenovo.com>
> > ---
> > v1 -> v2:
> > - modify the commit message
> > - patch 1/2 and patch 2/2 are placed in the same thread
> >
> >  .../bindings/connector/usb-connector.yaml     | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > index 67700440e23b..83be66f6d406 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > @@ -293,6 +293,26 @@ properties:
> >        PD negotiation till BC1.2 detection completes.
> >      default: 0
> >
> > +  pd2-sender-response-time-ms:
> > +    description: Represents the PD20 max time in ms that port started from
> > +      the time the last bit of the GoodCRC Message EOP has been received by
> > +      the Physical Layer, it shall be stopped when the last bit of the expected
> > +      response Message EOP has been received by the Physical Layer. A timeout
> > +      leads to a hard reset message by the port.
>
> I can only guess that 'pd2' and 'PD20' refer to Power Delivery 2.0.
>
Yes,   'pd2' and 'PD20' refer to Power Delivery 2.0.

> > +    minimum: 24
> > +    maximum: 30
> > +    default: 27
> > +
> > +  pd3-sender-response-time-ms:
> > +    description: Represents the PD30 max time in ms that port started from
> > +      the time the last bit of the GoodCRC Message EOP has been received by
> > +      the Physical Layer, it shall be stopped when the last bit of the expected
> > +      response Message EOP has been received by the Physical Layer. A timeout
> > +      leads to a hard reset message by the port.
>
> And pd3?
>
Is your suggestion to change the description of PD20 and PD30 to Power
Delivery 2.0 and 3.0?

> > +    minimum: 27
> > +    maximum: 33
> > +    default: 30
> > +
> >  dependencies:
> >    sink-vdos-v1: [ sink-vdos ]
> >    sink-vdos: [ sink-vdos-v1 ]
> > --
> > 2.17.1
> >

Re: [PATCH v2, 2/2] usb: typec: tcpm: fix the sender response time issue

[Jos Wang]

On Fri, Jan 3, 2025 at 1:39 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Wed, Jan 01, 2025 at 09:00:01PM +0800, Jos Wang wrote:
> > Hi, thanks for your help in reviewing the code, and happy new year to
> > you and your family!
> >
> > For the first commit you mentioned (modification time is 27ms), I
> > understand that just modify the include/linux/usb/pd.h file:
>
> In future please respond under the comment, not at the top of the
> message. Thank you.
>
OK

> > diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> > index d50098fb16b5..cd2cc535d21d 100644
> > --- a/include/linux/usb/pd.h
> > +++ b/include/linux/usb/pd.h
> > @@ -457,7 +457,7 @@ static inline unsigned int rdo_max_power(u32 rdo)
> >  #define PD_T_NO_RESPONSE       5000    /* 4.5 - 5.5 seconds */
> >  #define PD_T_DB_DETECT         10000   /* 10 - 15 seconds */
> >  #define PD_T_SEND_SOURCE_CAP   150     /* 100 - 200 ms */
> > -#define PD_T_SENDER_RESPONSE   60      /* 24 - 30 ms, relaxed */
> > +#define PD_T_SENDER_RESPONSE   27      /* 24 - 30 ms, relaxed */
> >  #define PD_T_RECEIVER_RESPONSE 15      /* 15ms max */
> >  #define PD_T_SOURCE_ACTIVITY   45
> >  #define PD_T_SINK_ACTIVITY     135
> >
> > Is my understanding correct?
>
> Yes.
>
Thanks you

> >
> >
> > Thanks
> > Jos Wang
> >
> > On Sun, Dec 22, 2024 at 9:14 PM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > On Sun, Dec 22, 2024 at 06:52:39PM +0800, joswang wrote:
> > > > From: Jos Wang <joswang@lenovo.com>
> > > >
> > > > According to the USB PD3 CTS specification
> > > > (https://usb.org/document-library/
> > > > usb-power-delivery-compliance-test-specification-0/
> > > > USB_PD3_CTS_Q4_2024_OR.zip), the requirements for
> > > > tSenderResponse are different in PD2 and PD3 modes, see
> > > > Table 19 Timing Table & Calculations. For PD2 mode, the
> > > > tSenderResponse min 24ms and max 30ms; for PD3 mode, the
> > > > tSenderResponse min 27ms and max 33ms.
> > > >
> > > > For the "TEST.PD.PROT.SRC.2 Get_Source_Cap No Request" test
> > > > item, after receiving the Source_Capabilities Message sent by
> > > > the UUT, the tester deliberately does not send a Request Message
> > > > in order to force the SenderResponse timer on the Source UUT to
> > > > timeout. The Tester checks that a Hard Reset is detected between
> > > > tSenderResponse min and max，the delay is between the last bit of
> > > > the GoodCRC Message EOP has been sent and the first bit of Hard
> > > > Reset SOP has been received. The current code does not distinguish
> > > > between PD2 and PD3 modes, and tSenderResponse defaults to 60ms.
> > > > This will cause this test item and the following tests to fail:
> > > > TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
> > > > TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout
> > > >
> > > > Considering factors such as SOC performance, i2c rate, and the speed
> > > > of PD chip sending data, "pd2-sender-response-time-ms" and
> > > > "pd3-sender-response-time-ms" DT time properties are added to allow
> > > > users to define platform timing. For values that have not been
> > > > explicitly defined in DT using this property, a default value of 27ms
> > > > for PD2 tSenderResponse and 30ms for PD3 tSenderResponse is set.
> > >
> > > You have several different changes squashed into the same commit:
> > > - Change the timeout from 60 ms to 27-30 ms (I'd recommend using 27 ms
> > >   as it fits both 24-30 ms and 27-33 ms ranges,
> > > - Make timeout depend on the PD version,
> > > - Make timeouts configurable via DT.
> > >
> > > Only the first item is a fix per se and only that change should be
> > > considered for backporting. Please unsquash your changes into logical
> > > commits.  Theoretically the second change can be thought about as a part
> > > of the third change (making timeouts configurable) or of the fist change
> > > (fix the timeout to follow the standard), but I'd suggest having three
> > > separate commits.
> > >
> > > >
> > > > Fixes: 2eadc33f40d4 ("typec: tcpm: Add core support for sink side PPS")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Jos Wang <joswang@lenovo.com>
> > > > ---
> > > > v1 -> v2:
> > > > - modify the commit message
> > > > - patch 1/2 and patch 2/2 are placed in the same thread
> > >
> > > --
> > > With best wishes
> > > Dmitry
>
> --
> With best wishes
> Dmitry

Re: [PATCH v2, 2/2] usb: typec: tcpm: fix the sender response time issue

[Jos Wang]

On Sun, Dec 22, 2024 at 9:14 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Sun, Dec 22, 2024 at 06:52:39PM +0800, joswang wrote:
> > From: Jos Wang <joswang@lenovo.com>
> >
> > According to the USB PD3 CTS specification
> > (https://usb.org/document-library/
> > usb-power-delivery-compliance-test-specification-0/
> > USB_PD3_CTS_Q4_2024_OR.zip), the requirements for
> > tSenderResponse are different in PD2 and PD3 modes, see
> > Table 19 Timing Table & Calculations. For PD2 mode, the
> > tSenderResponse min 24ms and max 30ms; for PD3 mode, the
> > tSenderResponse min 27ms and max 33ms.
> >
> > For the "TEST.PD.PROT.SRC.2 Get_Source_Cap No Request" test
> > item, after receiving the Source_Capabilities Message sent by
> > the UUT, the tester deliberately does not send a Request Message
> > in order to force the SenderResponse timer on the Source UUT to
> > timeout. The Tester checks that a Hard Reset is detected between
> > tSenderResponse min and max，the delay is between the last bit of
> > the GoodCRC Message EOP has been sent and the first bit of Hard
> > Reset SOP has been received. The current code does not distinguish
> > between PD2 and PD3 modes, and tSenderResponse defaults to 60ms.
> > This will cause this test item and the following tests to fail:
> > TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
> > TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout
> >
> > Considering factors such as SOC performance, i2c rate, and the speed
> > of PD chip sending data, "pd2-sender-response-time-ms" and
> > "pd3-sender-response-time-ms" DT time properties are added to allow
> > users to define platform timing. For values that have not been
> > explicitly defined in DT using this property, a default value of 27ms
> > for PD2 tSenderResponse and 30ms for PD3 tSenderResponse is set.
>
> You have several different changes squashed into the same commit:
> - Change the timeout from 60 ms to 27-30 ms (I'd recommend using 27 ms
>   as it fits both 24-30 ms and 27-33 ms ranges,
> - Make timeout depend on the PD version,
> - Make timeouts configurable via DT.
>
> Only the first item is a fix per se and only that change should be
> considered for backporting. Please unsquash your changes into logical
> commits.  Theoretically the second change can be thought about as a part
> of the third change (making timeouts configurable) or of the fist change
> (fix the timeout to follow the standard), but I'd suggest having three
> separate commits.
>
The patch is divided into patch1 (fix the timeout to follow the
standard), patch2 (Make timeout depend on the PD version)
and patch3 (Make timeouts configurable via DT). Do you suggest that
these three patches should be submitted as
V3 version, or patch1 and patch2 should be submitted separately?
Please help to confirm, thank you.

> >
> > Fixes: 2eadc33f40d4 ("typec: tcpm: Add core support for sink side PPS")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jos Wang <joswang@lenovo.com>
> > ---
> > v1 -> v2:
> > - modify the commit message
> > - patch 1/2 and patch 2/2 are placed in the same thread
>
> --
> With best wishes
> Dmitry

Re: [PATCH v2, 2/2] usb: typec: tcpm: fix the sender response time issue

[Dmitry Baryshkov]

On Sun, 5 Jan 2025 at 04:51, Jos Wang <joswang1221@gmail.com> wrote:
>
> On Sun, Dec 22, 2024 at 9:14 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Sun, Dec 22, 2024 at 06:52:39PM +0800, joswang wrote:
> > > From: Jos Wang <joswang@lenovo.com>
> > >
> > > According to the USB PD3 CTS specification
> > > (https://usb.org/document-library/
> > > usb-power-delivery-compliance-test-specification-0/
> > > USB_PD3_CTS_Q4_2024_OR.zip), the requirements for
> > > tSenderResponse are different in PD2 and PD3 modes, see
> > > Table 19 Timing Table & Calculations. For PD2 mode, the
> > > tSenderResponse min 24ms and max 30ms; for PD3 mode, the
> > > tSenderResponse min 27ms and max 33ms.
> > >
> > > For the "TEST.PD.PROT.SRC.2 Get_Source_Cap No Request" test
> > > item, after receiving the Source_Capabilities Message sent by
> > > the UUT, the tester deliberately does not send a Request Message
> > > in order to force the SenderResponse timer on the Source UUT to
> > > timeout. The Tester checks that a Hard Reset is detected between
> > > tSenderResponse min and max，the delay is between the last bit of
> > > the GoodCRC Message EOP has been sent and the first bit of Hard
> > > Reset SOP has been received. The current code does not distinguish
> > > between PD2 and PD3 modes, and tSenderResponse defaults to 60ms.
> > > This will cause this test item and the following tests to fail:
> > > TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
> > > TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout
> > >
> > > Considering factors such as SOC performance, i2c rate, and the speed
> > > of PD chip sending data, "pd2-sender-response-time-ms" and
> > > "pd3-sender-response-time-ms" DT time properties are added to allow
> > > users to define platform timing. For values that have not been
> > > explicitly defined in DT using this property, a default value of 27ms
> > > for PD2 tSenderResponse and 30ms for PD3 tSenderResponse is set.
> >
> > You have several different changes squashed into the same commit:
> > - Change the timeout from 60 ms to 27-30 ms (I'd recommend using 27 ms
> >   as it fits both 24-30 ms and 27-33 ms ranges,
> > - Make timeout depend on the PD version,
> > - Make timeouts configurable via DT.
> >
> > Only the first item is a fix per se and only that change should be
> > considered for backporting. Please unsquash your changes into logical
> > commits.  Theoretically the second change can be thought about as a part
> > of the third change (making timeouts configurable) or of the fist change
> > (fix the timeout to follow the standard), but I'd suggest having three
> > separate commits.
> >
> The patch is divided into patch1 (fix the timeout to follow the
> standard), patch2 (Make timeout depend on the PD version)
> and patch3 (Make timeouts configurable via DT). Do you suggest that
> these three patches should be submitted as
> V3 version, or patch1 and patch2 should be submitted separately?
> Please help to confirm, thank you.

Single series should be fine.

>
> > >
> > > Fixes: 2eadc33f40d4 ("typec: tcpm: Add core support for sink side PPS")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Jos Wang <joswang@lenovo.com>
> > > ---
> > > v1 -> v2:
> > > - modify the commit message
> > > - patch 1/2 and patch 2/2 are placed in the same thread
> >
> > --
> > With best wishes
> > Dmitry



-- 
With best wishes
Dmitry

Re: [PATCH v2, 2/2] usb: typec: tcpm: fix the sender response time issue

[Jos Wang]

On Sun, Jan 5, 2025 at 2:32 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Sun, 5 Jan 2025 at 04:51, Jos Wang <joswang1221@gmail.com> wrote:
> >
> > On Sun, Dec 22, 2024 at 9:14 PM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > On Sun, Dec 22, 2024 at 06:52:39PM +0800, joswang wrote:
> > > > From: Jos Wang <joswang@lenovo.com>
> > > >
> > > > According to the USB PD3 CTS specification
> > > > (https://usb.org/document-library/
> > > > usb-power-delivery-compliance-test-specification-0/
> > > > USB_PD3_CTS_Q4_2024_OR.zip), the requirements for
> > > > tSenderResponse are different in PD2 and PD3 modes, see
> > > > Table 19 Timing Table & Calculations. For PD2 mode, the
> > > > tSenderResponse min 24ms and max 30ms; for PD3 mode, the
> > > > tSenderResponse min 27ms and max 33ms.
> > > >
> > > > For the "TEST.PD.PROT.SRC.2 Get_Source_Cap No Request" test
> > > > item, after receiving the Source_Capabilities Message sent by
> > > > the UUT, the tester deliberately does not send a Request Message
> > > > in order to force the SenderResponse timer on the Source UUT to
> > > > timeout. The Tester checks that a Hard Reset is detected between
> > > > tSenderResponse min and max，the delay is between the last bit of
> > > > the GoodCRC Message EOP has been sent and the first bit of Hard
> > > > Reset SOP has been received. The current code does not distinguish
> > > > between PD2 and PD3 modes, and tSenderResponse defaults to 60ms.
> > > > This will cause this test item and the following tests to fail:
> > > > TEST.PD.PROT.SRC3.2 SenderResponseTimer Timeout
> > > > TEST.PD.PROT.SNK.6 SenderResponseTimer Timeout
> > > >
> > > > Considering factors such as SOC performance, i2c rate, and the speed
> > > > of PD chip sending data, "pd2-sender-response-time-ms" and
> > > > "pd3-sender-response-time-ms" DT time properties are added to allow
> > > > users to define platform timing. For values that have not been
> > > > explicitly defined in DT using this property, a default value of 27ms
> > > > for PD2 tSenderResponse and 30ms for PD3 tSenderResponse is set.
> > >
> > > You have several different changes squashed into the same commit:
> > > - Change the timeout from 60 ms to 27-30 ms (I'd recommend using 27 ms
> > >   as it fits both 24-30 ms and 27-33 ms ranges,
> > > - Make timeout depend on the PD version,
> > > - Make timeouts configurable via DT.
> > >
> > > Only the first item is a fix per se and only that change should be
> > > considered for backporting. Please unsquash your changes into logical
> > > commits.  Theoretically the second change can be thought about as a part
> > > of the third change (making timeouts configurable) or of the fist change
> > > (fix the timeout to follow the standard), but I'd suggest having three
> > > separate commits.
> > >
> > The patch is divided into patch1 (fix the timeout to follow the
> > standard), patch2 (Make timeout depend on the PD version)
> > and patch3 (Make timeouts configurable via DT). Do you suggest that
> > these three patches should be submitted as
> > V3 version, or patch1 and patch2 should be submitted separately?
> > Please help to confirm, thank you.
>
> Single series should be fine.
>
OK，One by one, after the first patch (fix the timeout to follow the
standard) is merged, submit the second patch (Make timeout depend on
the PD version).
The first patch can be found at:
https://patchwork.kernel.org/project/linux-usb/patch/20250105125251.5190-1-joswang1221@gmail.com/

> >
> > > >
> > > > Fixes: 2eadc33f40d4 ("typec: tcpm: Add core support for sink side PPS")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Jos Wang <joswang@lenovo.com>
> > > > ---
> > > > v1 -> v2:
> > > > - modify the commit message
> > > > - patch 1/2 and patch 2/2 are placed in the same thread
> > >
> > > --
> > > With best wishes
> > > Dmitry
>
>
>
> --
> With best wishes
> Dmitry