* [PATCH] usb: typec: tcpm: fix debug accessory mode detection for sink ports
@ 2026-04-20 3:03 Xu Yang
2026-04-23 22:50 ` Amit Sunil Dhamne
0 siblings, 1 reply; 3+ messages in thread
From: Xu Yang @ 2026-04-20 3:03 UTC (permalink / raw)
To: badhri, heikki.krogerus, gregkh, m.grzeschik
Cc: linux-usb, linux-kernel, imx, jun.li
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.
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>
---
drivers/usb/typec/tcpm/tcpm.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index dfbb94ddc98a..1ee00025de56 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))
@@ -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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: typec: tcpm: fix debug accessory mode detection for sink ports
2026-04-20 3:03 [PATCH] usb: typec: tcpm: fix debug accessory mode detection for sink ports Xu Yang
@ 2026-04-23 22:50 ` Amit Sunil Dhamne
2026-04-24 6:57 ` Xu Yang
0 siblings, 1 reply; 3+ messages in thread
From: Amit Sunil Dhamne @ 2026-04-23 22:50 UTC (permalink / raw)
To: Xu Yang, badhri, heikki.krogerus, gregkh, m.grzeschik
Cc: linux-usb, linux-kernel, imx, jun.li
Hi Xu,
On 4/19/26 8:03 PM, 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.
>
> 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>
> ---
> drivers/usb/typec/tcpm/tcpm.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index dfbb94ddc98a..1ee00025de56 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))
> @@ -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;
Thanks for submitting the patch.
Please can you also add changes in run_state_machine()? I think we can
replace tcpm_port_is_debug() in SNK_ATTACH_WAIT case with
tcpm_port_is_debug_sink(). Also, similar changes for SRC_ATTACH_WAIT case.
BR,
Amit
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: typec: tcpm: fix debug accessory mode detection for sink ports
2026-04-23 22:50 ` Amit Sunil Dhamne
@ 2026-04-24 6:57 ` Xu Yang
0 siblings, 0 replies; 3+ messages in thread
From: Xu Yang @ 2026-04-24 6:57 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: badhri, heikki.krogerus, gregkh, m.grzeschik, linux-usb,
linux-kernel, imx, jun.li
On Thu, Apr 23, 2026 at 03:50:51PM -0700, Amit Sunil Dhamne wrote:
> Hi Xu,
>
> On 4/19/26 8:03 PM, 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.
> >
> > 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>
> > ---
> > drivers/usb/typec/tcpm/tcpm.c | 19 +++++++++++++------
> > 1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index dfbb94ddc98a..1ee00025de56 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))
> > @@ -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;
>
>
> Thanks for submitting the patch.
>
> Please can you also add changes in run_state_machine()? I think we can
> replace tcpm_port_is_debug() in SNK_ATTACH_WAIT case with
> tcpm_port_is_debug_sink(). Also, similar changes for SRC_ATTACH_WAIT case.
OK. Will do.
Thanks,
Xu Yang
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-24 6:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-20 3:03 [PATCH] usb: typec: tcpm: fix debug accessory mode detection for sink ports Xu Yang
2026-04-23 22:50 ` Amit Sunil Dhamne
2026-04-24 6:57 ` Xu Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox