From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Xu Yang <xu.yang_2@nxp.com>
Cc: badhri@google.com, gregkh@linuxfoundation.org,
m.grzeschik@pengutronix.de, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, imx@lists.linux.dev,
jun.li@nxp.com
Subject: Re: [PATCH v2] usb: typec: tcpm: fix debug accessory mode detection for sink ports
Date: Mon, 27 Apr 2026 12:33:29 +0300 [thread overview]
Message-ID: <ae8tabVDj2iH6IQf@kuha> (raw)
In-Reply-To: <20260424074009.2979266-1-xu.yang_2@nxp.com>
On Fri, Apr 24, 2026 at 03:40:09PM +0800, Xu Yang wrote:
> The port in debug accessory mode can be either a source or sink. The
> previous tcpm_port_is_debug() function only checked for source port.
>
> Commit 8db73e6a42b6 ("usb: typec: tcpm: allow sink (ufp) to toggle into
> accessory mode debug") changed the detection logic to support both roles,
> but left some logic in _tcpm_cc_change() unchanged, This causes the state
> machine to transition to an incorrect state when operating as a sink in
> debug accessory mode. Log as below:
>
> [ 978.637541] CC1: 0 -> 5, CC2: 0 -> 5 [state TOGGLING, polarity 0, connected]
> [ 978.637567] state change TOGGLING -> SRC_ATTACH_WAIT [rev1 NONE_AMS]
> [ 978.637596] pending state change SRC_ATTACH_WAIT -> DEBUG_ACC_ATTACHED @ 180 ms [rev1 NONE_AMS]
> [ 978.647098] CC1: 5 -> 0, CC2: 5 -> 5 [state SRC_ATTACH_WAIT, polarity 0, connected]
> [ 978.647115] state change SRC_ATTACH_WAIT -> SRC_ATTACH_WAIT [rev1 NONE_AMS]
>
> It should go to SNK_ATTACH_WAIT instead of SRC_ATTACH_WAIT state.
>
> To fix this, add tcpm_port_is_debug_source() and tcpm_port_is_debug_sink()
> helper to explicitly identify the power mode in debug accessory mode.
> Update the state transition logic in _tcpm_cc_change() to ensure the state
> machine transitions comply with Type-C specification. Also update the logic
> in run_state_machine() to keep consistency.
>
> Fixes: 8db73e6a42b6 ("usb: typec: tcpm: allow sink (ufp) to toggle into accessory mode debug")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
FWIW:
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> Changes in v2:
> - update tcpm_port_is_debug() in run_state_machine() as well
> as suggested by Amit
> ---
> drivers/usb/typec/tcpm/tcpm.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index dfbb94ddc98a..6d258efae6f0 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -732,9 +732,14 @@ static const char * const pd_rev[] = {
> (tcpm_cc_is_source((port)->cc2) && \
> !tcpm_cc_is_source((port)->cc1)))
>
> +#define tcpm_port_is_debug_source(port) \
> + (tcpm_cc_is_source((port)->cc1) && tcpm_cc_is_source((port)->cc2))
> +
> +#define tcpm_port_is_debug_sink(port) \
> + (tcpm_cc_is_sink((port)->cc1) && tcpm_cc_is_sink((port)->cc2))
> +
> #define tcpm_port_is_debug(port) \
> - ((tcpm_cc_is_source((port)->cc1) && tcpm_cc_is_source((port)->cc2)) || \
> - (tcpm_cc_is_sink((port)->cc1) && tcpm_cc_is_sink((port)->cc2)))
> + (tcpm_port_is_debug_source(port) || tcpm_port_is_debug_sink(port))
>
> #define tcpm_port_is_audio(port) \
> (tcpm_cc_is_audio((port)->cc1) && tcpm_cc_is_audio((port)->cc2))
> @@ -5176,7 +5181,7 @@ static void run_state_machine(struct tcpm_port *port)
> tcpm_set_state(port, SNK_UNATTACHED, PD_T_DRP_SNK);
> break;
> case SRC_ATTACH_WAIT:
> - if (tcpm_port_is_debug(port))
> + if (tcpm_port_is_debug_source(port))
> tcpm_set_state(port, DEBUG_ACC_ATTACHED,
> port->timings.cc_debounce_time);
> else if (tcpm_port_is_audio(port))
> @@ -5434,7 +5439,7 @@ static void run_state_machine(struct tcpm_port *port)
> tcpm_set_state(port, SRC_UNATTACHED, PD_T_DRP_SRC);
> break;
> case SNK_ATTACH_WAIT:
> - if (tcpm_port_is_debug(port))
> + if (tcpm_port_is_debug_sink(port))
> tcpm_set_state(port, DEBUG_ACC_ATTACHED,
> PD_T_CC_DEBOUNCE);
> else if (tcpm_port_is_audio(port))
> @@ -5454,7 +5459,7 @@ static void run_state_machine(struct tcpm_port *port)
> if (tcpm_port_is_disconnected(port))
> tcpm_set_state(port, SNK_UNATTACHED,
> PD_T_PD_DEBOUNCE);
> - else if (tcpm_port_is_debug(port))
> + else if (tcpm_port_is_debug_sink(port))
> tcpm_set_state(port, DEBUG_ACC_ATTACHED,
> PD_T_CC_DEBOUNCE);
> else if (tcpm_port_is_audio(port))
> @@ -6360,10 +6365,10 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,
>
> switch (port->state) {
> case TOGGLING:
> - if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
> + if (tcpm_port_is_debug_source(port) || tcpm_port_is_audio(port) ||
> tcpm_port_is_source(port))
> tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
> - else if (tcpm_port_is_sink(port))
> + else if (tcpm_port_is_debug_sink(port) || tcpm_port_is_sink(port))
> tcpm_set_state(port, SNK_ATTACH_WAIT, 0);
> break;
> case CHECK_CONTAMINANT:
> @@ -6371,9 +6376,11 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,
> break;
> case SRC_UNATTACHED:
> case ACC_UNATTACHED:
> - if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
> + if (tcpm_port_is_debug_source(port) || tcpm_port_is_audio(port) ||
> tcpm_port_is_source(port))
> tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
> + else if (tcpm_port_is_debug_sink(port))
> + tcpm_set_state(port, SNK_ATTACH_WAIT, 0);
> break;
> case SRC_ATTACH_WAIT:
> if (tcpm_port_is_disconnected(port) ||
> @@ -6395,7 +6402,7 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,
> }
> break;
> case SNK_UNATTACHED:
> - if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
> + if (tcpm_port_is_debug_sink(port) || tcpm_port_is_audio(port) ||
> tcpm_port_is_sink(port))
> tcpm_set_state(port, SNK_ATTACH_WAIT, 0);
> break;
> --
> 2.34.1
thanks,
--
heikki
prev parent reply other threads:[~2026-04-27 9:33 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-24 7:40 [PATCH v2] usb: typec: tcpm: fix debug accessory mode detection for sink ports Xu Yang
2026-04-24 21:24 ` Amit Sunil Dhamne
2026-04-27 9:33 ` Heikki Krogerus [this message]
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=ae8tabVDj2iH6IQf@kuha \
--to=heikki.krogerus@linux.intel.com \
--cc=badhri@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=imx@lists.linux.dev \
--cc=jun.li@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=m.grzeschik@pengutronix.de \
--cc=xu.yang_2@nxp.com \
/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