From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Amit Sunil Dhamne <amitsd@google.com>
Cc: gregkh@linuxfoundation.org, robh@kernel.org,
dmitry.baryshkov@linaro.org, badhri@google.com,
kyletso@google.com, rdbabiera@google.com,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
devicetree@vger.kernel.org
Subject: Re: [RFC v2 2/2] usb: typec: tcpm: Add support for parsing time dt properties
Date: Thu, 19 Sep 2024 16:32:04 +0300 [thread overview]
Message-ID: <Zuwn1Fn1DrLGvPK9@kuha.fi.intel.com> (raw)
In-Reply-To: <20240919075120.328469-3-amitsd@google.com>
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,
--
heikki
next prev parent reply other threads:[~2024-09-19 13:32 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-19 7:51 [RFC v2 0/2] Add support for time DT property in TCPM Amit Sunil Dhamne
2024-09-19 7:51 ` [RFC v2 1/2] dt-bindings: connector: Add properties to define time values Amit Sunil Dhamne
2024-09-21 0:56 ` Rob Herring
2024-09-19 7:51 ` [RFC v2 2/2] usb: typec: tcpm: Add support for parsing time dt properties Amit Sunil Dhamne
2024-09-19 8:56 ` Dmitry Baryshkov
2024-09-19 22:08 ` Amit Sunil Dhamne
2024-09-19 13:32 ` Heikki Krogerus [this message]
2024-09-19 22:15 ` Amit Sunil Dhamne
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zuwn1Fn1DrLGvPK9@kuha.fi.intel.com \
--to=heikki.krogerus@linux.intel.com \
--cc=amitsd@google.com \
--cc=badhri@google.com \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=kyletso@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=rdbabiera@google.com \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®