Re: [PATCH v2] usb: typec: tcpm: fix debug accessory mode detection for sink ports
From: Badhri Jagan Sridharan
Date: Thu May 14 2026 - 14:40:22 EST
On Mon, Apr 27, 2026 at 2:33 AM Heikki Krogerus
<heikki.krogerus@xxxxxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxxx
> > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
>
> FWIW:
>
> Acked-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
Reviewed-by: Badhri Jagan Sridharan <badhri@xxxxxxxxxx>
>
> > ---
> > 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