| Message ID | 20240919075120.328469-3-amitsd@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for time DT property in TCPM | expand |
On Thu, Sep 19, 2024 at 12:51:14AM GMT, Amit Sunil Dhamne wrote: > Add support for DT time properties to allow users to define platform > specific timing deadlines of certain timers rather than using hardcoded > ones. For values that have not been explicitly defined in DT using this > property, default values will be set therefore, making this change > backward compatible. > > Signed-off-by: Amit Sunil Dhamne <amitsd@google.com> > --- > drivers/usb/typec/tcpm/tcpm.c | 81 ++++++++++++++++++++++++++++------- > 1 file changed, 65 insertions(+), 16 deletions(-) > @@ -7053,6 +7068,35 @@ static int tcpm_port_register_pd(struct tcpm_port *port) > return ret; > } > > +static int tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fwnode) > +{ > + int ret; > + u32 val; > + > + if (!fwnode) > + return -EINVAL; I think this check isn't really necessary here. Other than that: Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > + > + ret = fwnode_property_read_u32(fwnode, "sink-wait-cap-time-ms", &val); > + if (!ret) > + port->timings.sink_wait_cap_time = val; > + else > + port->timings.sink_wait_cap_time = PD_T_SINK_WAIT_CAP; > + > + ret = fwnode_property_read_u32(fwnode, "ps-source-off-time-ms", &val); > + if (!ret) > + port->timings.ps_src_off_time = val; > + else > + port->timings.ps_src_off_time = PD_T_PS_SOURCE_OFF; > + > + ret = fwnode_property_read_u32(fwnode, "cc-debounce-time-ms", &val); > + if (!ret) > + port->timings.cc_debounce_time = val; > + else > + port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE; > + > + return 0; > +} > + > static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode) > { > struct fwnode_handle *capabilities, *child, *caps = NULL; > @@ -7608,9 +7652,14 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) > init_completion(&port->pps_complete); > tcpm_debugfs_init(port); > > + err = tcpm_fw_get_timings(port, tcpc->fwnode); > + if (err < 0) > + goto out_destroy_wq; > + > err = tcpm_fw_get_caps(port, tcpc->fwnode); > if (err < 0) > goto out_destroy_wq; > + > err = tcpm_fw_get_snk_vdos(port, tcpc->fwnode); > if (err < 0) > goto out_destroy_wq; > -- > 2.46.0.792.g87dc391469-goog >
On Thu, Sep 19, 2024 at 12:51:14AM -0700, Amit Sunil Dhamne wrote: > Add support for DT time properties to allow users to define platform > specific timing deadlines of certain timers rather than using hardcoded > ones. For values that have not been explicitly defined in DT using this > property, default values will be set therefore, making this change > backward compatible. > > Signed-off-by: Amit Sunil Dhamne <amitsd@google.com> > --- > drivers/usb/typec/tcpm/tcpm.c | 81 ++++++++++++++++++++++++++++------- > 1 file changed, 65 insertions(+), 16 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index 4b02d6474259..e6c243bc44f7 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -310,6 +310,17 @@ struct pd_data { > unsigned int operating_snk_mw; > }; > > +/* > + * @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 > + */ > +struct pd_timings { > + u32 sink_wait_cap_time; > + u32 ps_src_off_time; > + u32 cc_debounce_time; > +}; > + > struct tcpm_port { > struct device *dev; > > @@ -552,6 +563,9 @@ struct tcpm_port { > */ > unsigned int message_id_prime; > unsigned int rx_msgid_prime; > + > + /* Timer deadline values configured at runtime */ > + struct pd_timings timings; > #ifdef CONFIG_DEBUG_FS > struct dentry *dentry; > struct mutex logbuffer_lock; /* log buffer access lock */ > @@ -4639,15 +4653,15 @@ static void run_state_machine(struct tcpm_port *port) > case SRC_ATTACH_WAIT: > if (tcpm_port_is_debug(port)) > tcpm_set_state(port, DEBUG_ACC_ATTACHED, > - PD_T_CC_DEBOUNCE); > + port->timings.cc_debounce_time); > else if (tcpm_port_is_audio(port)) > tcpm_set_state(port, AUDIO_ACC_ATTACHED, > - PD_T_CC_DEBOUNCE); > + port->timings.cc_debounce_time); > else if (tcpm_port_is_source(port) && port->vbus_vsafe0v) > tcpm_set_state(port, > tcpm_try_snk(port) ? SNK_TRY > : SRC_ATTACHED, > - PD_T_CC_DEBOUNCE); > + port->timings.cc_debounce_time); > break; > > case SNK_TRY: > @@ -4698,7 +4712,7 @@ static void run_state_machine(struct tcpm_port *port) > } > break; > case SRC_TRYWAIT_DEBOUNCE: > - tcpm_set_state(port, SRC_ATTACHED, PD_T_CC_DEBOUNCE); > + tcpm_set_state(port, SRC_ATTACHED, port->timings.cc_debounce_time); > break; > case SRC_TRYWAIT_UNATTACHED: > tcpm_set_state(port, SNK_UNATTACHED, 0); > @@ -4901,7 +4915,7 @@ static void run_state_machine(struct tcpm_port *port) > (port->cc1 != TYPEC_CC_OPEN && > port->cc2 == TYPEC_CC_OPEN)) > tcpm_set_state(port, SNK_DEBOUNCED, > - PD_T_CC_DEBOUNCE); > + port->timings.cc_debounce_time); > else if (tcpm_port_is_disconnected(port)) > tcpm_set_state(port, SNK_UNATTACHED, > PD_T_PD_DEBOUNCE); > @@ -4941,7 +4955,7 @@ static void run_state_machine(struct tcpm_port *port) > break; > case SNK_TRYWAIT: > tcpm_set_cc(port, TYPEC_CC_RD); > - tcpm_set_state(port, SNK_TRYWAIT_VBUS, PD_T_CC_DEBOUNCE); > + tcpm_set_state(port, SNK_TRYWAIT_VBUS, port->timings.cc_debounce_time); > break; > case SNK_TRYWAIT_VBUS: > /* > @@ -5014,7 +5028,7 @@ static void run_state_machine(struct tcpm_port *port) > break; > case SNK_DISCOVERY_DEBOUNCE: > tcpm_set_state(port, SNK_DISCOVERY_DEBOUNCE_DONE, > - PD_T_CC_DEBOUNCE); > + port->timings.cc_debounce_time); > break; > case SNK_DISCOVERY_DEBOUNCE_DONE: > if (!tcpm_port_is_disconnected(port) && > @@ -5041,10 +5055,10 @@ static void run_state_machine(struct tcpm_port *port) > if (port->vbus_never_low) { > port->vbus_never_low = false; > tcpm_set_state(port, SNK_SOFT_RESET, > - PD_T_SINK_WAIT_CAP); > + port->timings.sink_wait_cap_time); > } else { > tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, > - PD_T_SINK_WAIT_CAP); > + port->timings.sink_wait_cap_time); > } > break; > case SNK_WAIT_CAPABILITIES_TIMEOUT: > @@ -5066,7 +5080,8 @@ static void run_state_machine(struct tcpm_port *port) > if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP)) > tcpm_set_state_cond(port, hard_reset_state(port), 0); > else > - tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP); > + tcpm_set_state(port, hard_reset_state(port), > + port->timings.sink_wait_cap_time); > break; > case SNK_NEGOTIATE_CAPABILITIES: > port->pd_capable = true; > @@ -5203,7 +5218,7 @@ static void run_state_machine(struct tcpm_port *port) > tcpm_set_state(port, ACC_UNATTACHED, 0); > break; > case AUDIO_ACC_DEBOUNCE: > - tcpm_set_state(port, ACC_UNATTACHED, PD_T_CC_DEBOUNCE); > + tcpm_set_state(port, ACC_UNATTACHED, port->timings.cc_debounce_time); > break; > > /* Hard_Reset states */ > @@ -5420,7 +5435,7 @@ static void run_state_machine(struct tcpm_port *port) > tcpm_set_state(port, ERROR_RECOVERY, 0); > break; > case FR_SWAP_SNK_SRC_TRANSITION_TO_OFF: > - tcpm_set_state(port, ERROR_RECOVERY, PD_T_PS_SOURCE_OFF); > + tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time); > break; > case FR_SWAP_SNK_SRC_NEW_SINK_READY: > if (port->vbus_source) > @@ -5475,7 +5490,7 @@ static void run_state_machine(struct tcpm_port *port) > tcpm_set_cc(port, TYPEC_CC_RD); > /* allow CC debounce */ > tcpm_set_state(port, PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED, > - PD_T_CC_DEBOUNCE); > + port->timings.cc_debounce_time); > break; > case PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED: > /* > @@ -5510,7 +5525,7 @@ static void run_state_machine(struct tcpm_port *port) > port->pps_data.active, 0); > tcpm_set_charge(port, false); > tcpm_set_state(port, hard_reset_state(port), > - PD_T_PS_SOURCE_OFF); > + port->timings.ps_src_off_time); > break; > case PR_SWAP_SNK_SRC_SOURCE_ON: > tcpm_enable_auto_vbus_discharge(port, true); > @@ -5666,7 +5681,7 @@ static void run_state_machine(struct tcpm_port *port) > case PORT_RESET_WAIT_OFF: > tcpm_set_state(port, > tcpm_default_state(port), > - port->vbus_present ? PD_T_PS_SOURCE_OFF : 0); > + port->vbus_present ? port->timings.ps_src_off_time : 0); > break; > > /* AMS intermediate state */ > @@ -6157,7 +6172,7 @@ static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port) > case SRC_ATTACH_WAIT: > if (tcpm_port_is_source(port)) > tcpm_set_state(port, tcpm_try_snk(port) ? SNK_TRY : SRC_ATTACHED, > - PD_T_CC_DEBOUNCE); > + port->timings.cc_debounce_time); > break; > case SRC_STARTUP: > case SRC_SEND_CAPABILITIES: > @@ -7053,6 +7068,35 @@ static int tcpm_port_register_pd(struct tcpm_port *port) > return ret; > } > > +static int tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fwnode) > +{ > + int ret; > + u32 val; > + > + if (!fwnode) > + return -EINVAL; > + > + ret = fwnode_property_read_u32(fwnode, "sink-wait-cap-time-ms", &val); > + if (!ret) > + port->timings.sink_wait_cap_time = val; > + else > + port->timings.sink_wait_cap_time = PD_T_SINK_WAIT_CAP; > + > + ret = fwnode_property_read_u32(fwnode, "ps-source-off-time-ms", &val); > + if (!ret) > + port->timings.ps_src_off_time = val; > + else > + port->timings.ps_src_off_time = PD_T_PS_SOURCE_OFF; > + > + ret = fwnode_property_read_u32(fwnode, "cc-debounce-time-ms", &val); > + if (!ret) > + port->timings.cc_debounce_time = val; > + else > + port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE; > + > + return 0; > +} > + > static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode) > { > struct fwnode_handle *capabilities, *child, *caps = NULL; > @@ -7608,9 +7652,14 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) > init_completion(&port->pps_complete); > tcpm_debugfs_init(port); > > + err = tcpm_fw_get_timings(port, tcpc->fwnode); > + if (err < 0) > + goto out_destroy_wq; This is somehow wrong. You are using default values in case of failure, so this should not be a reason to fail port registration under any circumstance. That function should just return void. I would also just call it after tcpm_fw_get_caps() (or maybe even from tcpm_fw_get_caps()), because tcpm_fw_get_caps() checks fwnode in any case. > err = tcpm_fw_get_caps(port, tcpc->fwnode); > if (err < 0) > goto out_destroy_wq; > + > err = tcpm_fw_get_snk_vdos(port, tcpc->fwnode); > if (err < 0) > goto out_destroy_wq; thanks,
Hi Dmitry, Thanks for the review! On 9/19/24 1:56 AM, Dmitry Baryshkov wrote: > On Thu, Sep 19, 2024 at 12:51:14AM GMT, Amit Sunil Dhamne wrote: >> Add support for DT time properties to allow users to define platform >> specific timing deadlines of certain timers rather than using hardcoded >> ones. For values that have not been explicitly defined in DT using this >> property, default values will be set therefore, making this change >> backward compatible. >> >> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com> >> --- >> drivers/usb/typec/tcpm/tcpm.c | 81 ++++++++++++++++++++++++++++------- >> 1 file changed, 65 insertions(+), 16 deletions(-) >> @@ -7053,6 +7068,35 @@ static int tcpm_port_register_pd(struct tcpm_port *port) >> return ret; >> } >> >> +static int tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fwnode) >> +{ >> + int ret; >> + u32 val; >> + >> + if (!fwnode) >> + return -EINVAL; > I think this check isn't really necessary here. Other than that: > > Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Will remove this check in next rev as suggested in https://lore.kernel.org/all/Zuwn1Fn1DrLGvPK9@kuha.fi.intel.com/ . Regards, Amit >> + >> + ret = fwnode_property_read_u32(fwnode, "sink-wait-cap-time-ms", &val); >> + if (!ret) >> + port->timings.sink_wait_cap_time = val; >> + else >> + port->timings.sink_wait_cap_time = PD_T_SINK_WAIT_CAP; >> + >> + ret = fwnode_property_read_u32(fwnode, "ps-source-off-time-ms", &val); >> + if (!ret) >> + port->timings.ps_src_off_time = val; >> + else >> + port->timings.ps_src_off_time = PD_T_PS_SOURCE_OFF; >> + >> + ret = fwnode_property_read_u32(fwnode, "cc-debounce-time-ms", &val); >> + if (!ret) >> + port->timings.cc_debounce_time = val; >> + else >> + port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE; >> + >> + return 0; >> +} >> + >> static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode) >> { >> struct fwnode_handle *capabilities, *child, *caps = NULL; >> @@ -7608,9 +7652,14 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) >> init_completion(&port->pps_complete); >> tcpm_debugfs_init(port); >> >> + err = tcpm_fw_get_timings(port, tcpc->fwnode); >> + if (err < 0) >> + goto out_destroy_wq; >> + >> err = tcpm_fw_get_caps(port, tcpc->fwnode); >> if (err < 0) >> goto out_destroy_wq; >> + >> err = tcpm_fw_get_snk_vdos(port, tcpc->fwnode); >> if (err < 0) >> goto out_destroy_wq; >> -- >> 2.46.0.792.g87dc391469-goog >>
Hi Heikki, On 9/19/24 6:32 AM, Heikki Krogerus wrote: > On Thu, Sep 19, 2024 at 12:51:14AM -0700, Amit Sunil Dhamne wrote: >> Add support for DT time properties to allow users to define platform >> specific timing deadlines of certain timers rather than using hardcoded >> ones. For values that have not been explicitly defined in DT using this >> property, default values will be set therefore, making this change >> backward compatible. >> >> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com> >> --- >> drivers/usb/typec/tcpm/tcpm.c | 81 ++++++++++++++++++++++++++++------- >> 1 file changed, 65 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c >> index 4b02d6474259..e6c243bc44f7 100644 >> --- a/drivers/usb/typec/tcpm/tcpm.c >> +++ b/drivers/usb/typec/tcpm/tcpm.c >> @@ -310,6 +310,17 @@ struct pd_data { >> unsigned int operating_snk_mw; >> }; >> >> +/* >> + * @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 >> + */ >> +struct pd_timings { >> + u32 sink_wait_cap_time; >> + u32 ps_src_off_time; >> + u32 cc_debounce_time; >> +}; >> + >> struct tcpm_port { >> struct device *dev; >> >> @@ -552,6 +563,9 @@ struct tcpm_port { >> */ >> unsigned int message_id_prime; >> unsigned int rx_msgid_prime; >> + >> + /* Timer deadline values configured at runtime */ >> + struct pd_timings timings; >> #ifdef CONFIG_DEBUG_FS >> struct dentry *dentry; >> struct mutex logbuffer_lock; /* log buffer access lock */ >> @@ -4639,15 +4653,15 @@ static void run_state_machine(struct tcpm_port *port) >> case SRC_ATTACH_WAIT: >> if (tcpm_port_is_debug(port)) >> tcpm_set_state(port, DEBUG_ACC_ATTACHED, >> - PD_T_CC_DEBOUNCE); >> + port->timings.cc_debounce_time); >> else if (tcpm_port_is_audio(port)) >> tcpm_set_state(port, AUDIO_ACC_ATTACHED, >> - PD_T_CC_DEBOUNCE); >> + port->timings.cc_debounce_time); >> else if (tcpm_port_is_source(port) && port->vbus_vsafe0v) >> tcpm_set_state(port, >> tcpm_try_snk(port) ? SNK_TRY >> : SRC_ATTACHED, >> - PD_T_CC_DEBOUNCE); >> + port->timings.cc_debounce_time); >> break; >> >> case SNK_TRY: >> @@ -4698,7 +4712,7 @@ static void run_state_machine(struct tcpm_port *port) >> } >> break; >> case SRC_TRYWAIT_DEBOUNCE: >> - tcpm_set_state(port, SRC_ATTACHED, PD_T_CC_DEBOUNCE); >> + tcpm_set_state(port, SRC_ATTACHED, port->timings.cc_debounce_time); >> break; >> case SRC_TRYWAIT_UNATTACHED: >> tcpm_set_state(port, SNK_UNATTACHED, 0); >> @@ -4901,7 +4915,7 @@ static void run_state_machine(struct tcpm_port *port) >> (port->cc1 != TYPEC_CC_OPEN && >> port->cc2 == TYPEC_CC_OPEN)) >> tcpm_set_state(port, SNK_DEBOUNCED, >> - PD_T_CC_DEBOUNCE); >> + port->timings.cc_debounce_time); >> else if (tcpm_port_is_disconnected(port)) >> tcpm_set_state(port, SNK_UNATTACHED, >> PD_T_PD_DEBOUNCE); >> @@ -4941,7 +4955,7 @@ static void run_state_machine(struct tcpm_port *port) >> break; >> case SNK_TRYWAIT: >> tcpm_set_cc(port, TYPEC_CC_RD); >> - tcpm_set_state(port, SNK_TRYWAIT_VBUS, PD_T_CC_DEBOUNCE); >> + tcpm_set_state(port, SNK_TRYWAIT_VBUS, port->timings.cc_debounce_time); >> break; >> case SNK_TRYWAIT_VBUS: >> /* >> @@ -5014,7 +5028,7 @@ static void run_state_machine(struct tcpm_port *port) >> break; >> case SNK_DISCOVERY_DEBOUNCE: >> tcpm_set_state(port, SNK_DISCOVERY_DEBOUNCE_DONE, >> - PD_T_CC_DEBOUNCE); >> + port->timings.cc_debounce_time); >> break; >> case SNK_DISCOVERY_DEBOUNCE_DONE: >> if (!tcpm_port_is_disconnected(port) && >> @@ -5041,10 +5055,10 @@ static void run_state_machine(struct tcpm_port *port) >> if (port->vbus_never_low) { >> port->vbus_never_low = false; >> tcpm_set_state(port, SNK_SOFT_RESET, >> - PD_T_SINK_WAIT_CAP); >> + port->timings.sink_wait_cap_time); >> } else { >> tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, >> - PD_T_SINK_WAIT_CAP); >> + port->timings.sink_wait_cap_time); >> } >> break; >> case SNK_WAIT_CAPABILITIES_TIMEOUT: >> @@ -5066,7 +5080,8 @@ static void run_state_machine(struct tcpm_port *port) >> if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP)) >> tcpm_set_state_cond(port, hard_reset_state(port), 0); >> else >> - tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP); >> + tcpm_set_state(port, hard_reset_state(port), >> + port->timings.sink_wait_cap_time); >> break; >> case SNK_NEGOTIATE_CAPABILITIES: >> port->pd_capable = true; >> @@ -5203,7 +5218,7 @@ static void run_state_machine(struct tcpm_port *port) >> tcpm_set_state(port, ACC_UNATTACHED, 0); >> break; >> case AUDIO_ACC_DEBOUNCE: >> - tcpm_set_state(port, ACC_UNATTACHED, PD_T_CC_DEBOUNCE); >> + tcpm_set_state(port, ACC_UNATTACHED, port->timings.cc_debounce_time); >> break; >> >> /* Hard_Reset states */ >> @@ -5420,7 +5435,7 @@ static void run_state_machine(struct tcpm_port *port) >> tcpm_set_state(port, ERROR_RECOVERY, 0); >> break; >> case FR_SWAP_SNK_SRC_TRANSITION_TO_OFF: >> - tcpm_set_state(port, ERROR_RECOVERY, PD_T_PS_SOURCE_OFF); >> + tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time); >> break; >> case FR_SWAP_SNK_SRC_NEW_SINK_READY: >> if (port->vbus_source) >> @@ -5475,7 +5490,7 @@ static void run_state_machine(struct tcpm_port *port) >> tcpm_set_cc(port, TYPEC_CC_RD); >> /* allow CC debounce */ >> tcpm_set_state(port, PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED, >> - PD_T_CC_DEBOUNCE); >> + port->timings.cc_debounce_time); >> break; >> case PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED: >> /* >> @@ -5510,7 +5525,7 @@ static void run_state_machine(struct tcpm_port *port) >> port->pps_data.active, 0); >> tcpm_set_charge(port, false); >> tcpm_set_state(port, hard_reset_state(port), >> - PD_T_PS_SOURCE_OFF); >> + port->timings.ps_src_off_time); >> break; >> case PR_SWAP_SNK_SRC_SOURCE_ON: >> tcpm_enable_auto_vbus_discharge(port, true); >> @@ -5666,7 +5681,7 @@ static void run_state_machine(struct tcpm_port *port) >> case PORT_RESET_WAIT_OFF: >> tcpm_set_state(port, >> tcpm_default_state(port), >> - port->vbus_present ? PD_T_PS_SOURCE_OFF : 0); >> + port->vbus_present ? port->timings.ps_src_off_time : 0); >> break; >> >> /* AMS intermediate state */ >> @@ -6157,7 +6172,7 @@ static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port) >> case SRC_ATTACH_WAIT: >> if (tcpm_port_is_source(port)) >> tcpm_set_state(port, tcpm_try_snk(port) ? SNK_TRY : SRC_ATTACHED, >> - PD_T_CC_DEBOUNCE); >> + port->timings.cc_debounce_time); >> break; >> case SRC_STARTUP: >> case SRC_SEND_CAPABILITIES: >> @@ -7053,6 +7068,35 @@ static int tcpm_port_register_pd(struct tcpm_port *port) >> return ret; >> } >> >> +static int tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fwnode) >> +{ >> + int ret; >> + u32 val; >> + >> + if (!fwnode) >> + return -EINVAL; >> + >> + ret = fwnode_property_read_u32(fwnode, "sink-wait-cap-time-ms", &val); >> + if (!ret) >> + port->timings.sink_wait_cap_time = val; >> + else >> + port->timings.sink_wait_cap_time = PD_T_SINK_WAIT_CAP; >> + >> + ret = fwnode_property_read_u32(fwnode, "ps-source-off-time-ms", &val); >> + if (!ret) >> + port->timings.ps_src_off_time = val; >> + else >> + port->timings.ps_src_off_time = PD_T_PS_SOURCE_OFF; >> + >> + ret = fwnode_property_read_u32(fwnode, "cc-debounce-time-ms", &val); >> + if (!ret) >> + port->timings.cc_debounce_time = val; >> + else >> + port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE; >> + >> + return 0; >> +} >> + >> static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode) >> { >> struct fwnode_handle *capabilities, *child, *caps = NULL; >> @@ -7608,9 +7652,14 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) >> init_completion(&port->pps_complete); >> tcpm_debugfs_init(port); >> >> + err = tcpm_fw_get_timings(port, tcpc->fwnode); >> + if (err < 0) >> + goto out_destroy_wq; > This is somehow wrong. You are using default values in case of > failure, so this should not be a reason to fail port registration > under any circumstance. That function should just return void. > > I would also just call it after tcpm_fw_get_caps() (or maybe even from > tcpm_fw_get_caps()), because tcpm_fw_get_caps() checks fwnode in any > case. Thanks for the review! I will update in the next revision with this change. Will send the next revision once I get review from DT maintainers for DT patch. Regards, Amit >> err = tcpm_fw_get_caps(port, tcpc->fwnode); >> if (err < 0) >> goto out_destroy_wq; >> + >> err = tcpm_fw_get_snk_vdos(port, tcpc->fwnode); >> if (err < 0) >> goto out_destroy_wq; > thanks, >
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index 4b02d6474259..e6c243bc44f7 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -310,6 +310,17 @@ struct pd_data { unsigned int operating_snk_mw; }; +/* + * @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 + */ +struct pd_timings { + u32 sink_wait_cap_time; + u32 ps_src_off_time; + u32 cc_debounce_time; +}; + struct tcpm_port { struct device *dev; @@ -552,6 +563,9 @@ struct tcpm_port { */ unsigned int message_id_prime; unsigned int rx_msgid_prime; + + /* Timer deadline values configured at runtime */ + struct pd_timings timings; #ifdef CONFIG_DEBUG_FS struct dentry *dentry; struct mutex logbuffer_lock; /* log buffer access lock */ @@ -4639,15 +4653,15 @@ static void run_state_machine(struct tcpm_port *port) case SRC_ATTACH_WAIT: if (tcpm_port_is_debug(port)) tcpm_set_state(port, DEBUG_ACC_ATTACHED, - PD_T_CC_DEBOUNCE); + port->timings.cc_debounce_time); else if (tcpm_port_is_audio(port)) tcpm_set_state(port, AUDIO_ACC_ATTACHED, - PD_T_CC_DEBOUNCE); + port->timings.cc_debounce_time); else if (tcpm_port_is_source(port) && port->vbus_vsafe0v) tcpm_set_state(port, tcpm_try_snk(port) ? SNK_TRY : SRC_ATTACHED, - PD_T_CC_DEBOUNCE); + port->timings.cc_debounce_time); break; case SNK_TRY: @@ -4698,7 +4712,7 @@ static void run_state_machine(struct tcpm_port *port) } break; case SRC_TRYWAIT_DEBOUNCE: - tcpm_set_state(port, SRC_ATTACHED, PD_T_CC_DEBOUNCE); + tcpm_set_state(port, SRC_ATTACHED, port->timings.cc_debounce_time); break; case SRC_TRYWAIT_UNATTACHED: tcpm_set_state(port, SNK_UNATTACHED, 0); @@ -4901,7 +4915,7 @@ static void run_state_machine(struct tcpm_port *port) (port->cc1 != TYPEC_CC_OPEN && port->cc2 == TYPEC_CC_OPEN)) tcpm_set_state(port, SNK_DEBOUNCED, - PD_T_CC_DEBOUNCE); + port->timings.cc_debounce_time); else if (tcpm_port_is_disconnected(port)) tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE); @@ -4941,7 +4955,7 @@ static void run_state_machine(struct tcpm_port *port) break; case SNK_TRYWAIT: tcpm_set_cc(port, TYPEC_CC_RD); - tcpm_set_state(port, SNK_TRYWAIT_VBUS, PD_T_CC_DEBOUNCE); + tcpm_set_state(port, SNK_TRYWAIT_VBUS, port->timings.cc_debounce_time); break; case SNK_TRYWAIT_VBUS: /* @@ -5014,7 +5028,7 @@ static void run_state_machine(struct tcpm_port *port) break; case SNK_DISCOVERY_DEBOUNCE: tcpm_set_state(port, SNK_DISCOVERY_DEBOUNCE_DONE, - PD_T_CC_DEBOUNCE); + port->timings.cc_debounce_time); break; case SNK_DISCOVERY_DEBOUNCE_DONE: if (!tcpm_port_is_disconnected(port) && @@ -5041,10 +5055,10 @@ static void run_state_machine(struct tcpm_port *port) if (port->vbus_never_low) { port->vbus_never_low = false; tcpm_set_state(port, SNK_SOFT_RESET, - PD_T_SINK_WAIT_CAP); + port->timings.sink_wait_cap_time); } else { tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, - PD_T_SINK_WAIT_CAP); + port->timings.sink_wait_cap_time); } break; case SNK_WAIT_CAPABILITIES_TIMEOUT: @@ -5066,7 +5080,8 @@ static void run_state_machine(struct tcpm_port *port) if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP)) tcpm_set_state_cond(port, hard_reset_state(port), 0); else - tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP); + tcpm_set_state(port, hard_reset_state(port), + port->timings.sink_wait_cap_time); break; case SNK_NEGOTIATE_CAPABILITIES: port->pd_capable = true; @@ -5203,7 +5218,7 @@ static void run_state_machine(struct tcpm_port *port) tcpm_set_state(port, ACC_UNATTACHED, 0); break; case AUDIO_ACC_DEBOUNCE: - tcpm_set_state(port, ACC_UNATTACHED, PD_T_CC_DEBOUNCE); + tcpm_set_state(port, ACC_UNATTACHED, port->timings.cc_debounce_time); break; /* Hard_Reset states */ @@ -5420,7 +5435,7 @@ static void run_state_machine(struct tcpm_port *port) tcpm_set_state(port, ERROR_RECOVERY, 0); break; case FR_SWAP_SNK_SRC_TRANSITION_TO_OFF: - tcpm_set_state(port, ERROR_RECOVERY, PD_T_PS_SOURCE_OFF); + tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time); break; case FR_SWAP_SNK_SRC_NEW_SINK_READY: if (port->vbus_source) @@ -5475,7 +5490,7 @@ static void run_state_machine(struct tcpm_port *port) tcpm_set_cc(port, TYPEC_CC_RD); /* allow CC debounce */ tcpm_set_state(port, PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED, - PD_T_CC_DEBOUNCE); + port->timings.cc_debounce_time); break; case PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED: /* @@ -5510,7 +5525,7 @@ static void run_state_machine(struct tcpm_port *port) port->pps_data.active, 0); tcpm_set_charge(port, false); tcpm_set_state(port, hard_reset_state(port), - PD_T_PS_SOURCE_OFF); + port->timings.ps_src_off_time); break; case PR_SWAP_SNK_SRC_SOURCE_ON: tcpm_enable_auto_vbus_discharge(port, true); @@ -5666,7 +5681,7 @@ static void run_state_machine(struct tcpm_port *port) case PORT_RESET_WAIT_OFF: tcpm_set_state(port, tcpm_default_state(port), - port->vbus_present ? PD_T_PS_SOURCE_OFF : 0); + port->vbus_present ? port->timings.ps_src_off_time : 0); break; /* AMS intermediate state */ @@ -6157,7 +6172,7 @@ static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port) case SRC_ATTACH_WAIT: if (tcpm_port_is_source(port)) tcpm_set_state(port, tcpm_try_snk(port) ? SNK_TRY : SRC_ATTACHED, - PD_T_CC_DEBOUNCE); + port->timings.cc_debounce_time); break; case SRC_STARTUP: case SRC_SEND_CAPABILITIES: @@ -7053,6 +7068,35 @@ static int tcpm_port_register_pd(struct tcpm_port *port) return ret; } +static int tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fwnode) +{ + int ret; + u32 val; + + if (!fwnode) + return -EINVAL; + + ret = fwnode_property_read_u32(fwnode, "sink-wait-cap-time-ms", &val); + if (!ret) + port->timings.sink_wait_cap_time = val; + else + port->timings.sink_wait_cap_time = PD_T_SINK_WAIT_CAP; + + ret = fwnode_property_read_u32(fwnode, "ps-source-off-time-ms", &val); + if (!ret) + port->timings.ps_src_off_time = val; + else + port->timings.ps_src_off_time = PD_T_PS_SOURCE_OFF; + + ret = fwnode_property_read_u32(fwnode, "cc-debounce-time-ms", &val); + if (!ret) + port->timings.cc_debounce_time = val; + else + port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE; + + return 0; +} + static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode) { struct fwnode_handle *capabilities, *child, *caps = NULL; @@ -7608,9 +7652,14 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) init_completion(&port->pps_complete); tcpm_debugfs_init(port); + err = tcpm_fw_get_timings(port, tcpc->fwnode); + if (err < 0) + goto out_destroy_wq; + err = tcpm_fw_get_caps(port, tcpc->fwnode); if (err < 0) goto out_destroy_wq; + err = tcpm_fw_get_snk_vdos(port, tcpc->fwnode); if (err < 0) goto out_destroy_wq;
Add support for DT time properties to allow users to define platform specific timing deadlines of certain timers rather than using hardcoded ones. For values that have not been explicitly defined in DT using this property, default values will be set therefore, making this change backward compatible. Signed-off-by: Amit Sunil Dhamne <amitsd@google.com> --- drivers/usb/typec/tcpm/tcpm.c | 81 ++++++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 16 deletions(-)