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
next prev parent reply other threads:[~2026-04-27 9:33 UTC|newest]
Thread overview: 4+ 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]
2026-05-14 18:37 ` Badhri Jagan Sridharan
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.