| Message ID | 20241215125013.70671-1-joswang1221@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
Hi, On Sun, Dec 15, 2024 at 08:50:13PM +0800, joswang wrote: > From: Jos Wang <joswang@lenovo.com> > > According to the USB PD3 CTS specification, the requirements What is "USB PD3 CTS specification"? Please open it here. > 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: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)") > Cc: stable@vger.kernel.org > Signed-off-by: Jos Wang <joswang@lenovo.com> If this is a fix, then I think it's fixing commit 2eadc33f40d4 ("typec: tcpm: Add core support for sink side PPS"). That's where the pd_revision was changed to 3.0. Badhri, could you take a look at this (and how about that maintainer role? :-) ). > --- > 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; > } I can't see the whole thread, but I guess those properties were okay(?). thanks,
Thank you for your help in reviewing the code. USB PD3 CTS specification： https://usb.org/document-library/usb-power-delivery-compliance-test-specification-0 Should this link be added to the commit message? I will resubmit the code as soon as possible and put patch 1/2 and 2/2 in the same thread. On Wed, Dec 18, 2024 at 6:03 PM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > Hi, > > On Sun, Dec 15, 2024 at 08:50:13PM +0800, joswang wrote: > > From: Jos Wang <joswang@lenovo.com> > > > > According to the USB PD3 CTS specification, the requirements > > What is "USB PD3 CTS specification"? Please open it here. > > > 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: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)") > > Cc: stable@vger.kernel.org > > Signed-off-by: Jos Wang <joswang@lenovo.com> > > If this is a fix, then I think it's fixing commit 2eadc33f40d4 > ("typec: tcpm: Add core support for sink side PPS"). That's where the > pd_revision was changed to 3.0. > > Badhri, could you take a look at this (and how about that > maintainer role? :-) ). > > > --- > > 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; > > } > > I can't see the whole thread, but I guess those properties were > okay(?). > > thanks, > > -- > heikki
On Fri, Dec 20, 2024 at 5:39 AM Jos Wang <joswang1221@gmail.com> wrote: > > Thank you for your help in reviewing the code. > > USB PD3 CTS specification： > https://usb.org/document-library/usb-power-delivery-compliance-test-specification-0 > Should this link be added to the commit message? > > I will resubmit the code as soon as possible and put patch 1/2 and 2/2 > in the same thread. > > On Wed, Dec 18, 2024 at 6:03 PM Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: > > > > Hi, > > > > On Sun, Dec 15, 2024 at 08:50:13PM +0800, joswang wrote: > > > From: Jos Wang <joswang@lenovo.com> > > > > > > According to the USB PD3 CTS specification, the requirements > > > > What is "USB PD3 CTS specification"? Please open it here. > > > > > 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: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)") > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Jos Wang <joswang@lenovo.com> > > > > If this is a fix, then I think it's fixing commit 2eadc33f40d4 > > ("typec: tcpm: Add core support for sink side PPS"). That's where the > > pd_revision was changed to 3.0. Hmm, this patch also relies upon https://lore.kernel.org/all/20241022-pd-dt-time-props-v1-2-fea96f51b302@google.com/ so not sure whether it could be Fixes: 2eadc33f40d4 ("typec: tcpm: Add core support for sink side PPS") The patch looks good otherwise. Will wait for Jos's resubmission based on his previous response. > > > > Badhri, could you take a look at this (and how about that > > maintainer role? :-) ). This took time was lining up the logistics. Will send out a patch proposing me and a few others right after the new year break. Regards, Badhri > > > > > --- > > > 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; > > > } > > > > I can't see the whole thread, but I guess those properties were > > okay(?). > > > > thanks, > > > > -- > > heikki
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