Re: [PATCH] usb: typec: tcpci: pass correct rx_type to tcpm_pd_receive()

On Mon, Jul 20, 2026 at 04:09:59AM -0700, Badhri Jagan Sridharan wrote:
> On Thu, Jul 2, 2026 at 11:29 PM Xu Yang <[email protected]> wrote:
> >
> > From: Xu Yang <[email protected]>
> >
> > Previously, tcpci_irq() always passed TCPC_TX_SOP as the receive type
> > to tcpm_pd_receive(), ignoring the actual frame type reported by the
> > TCPC_RX_BUF_FRAME_TYPE register.
> >
> > Read TCPC_RX_BUF_FRAME_TYPE after receiving a PD message and map it to
> > the appropriate tcpm_transmit_type:
> >   - TCPC_RX_BUF_FRAME_TYPE_SOP1 -> TCPC_TX_SOP_PRIME
> >   - TCPC_RX_BUF_FRAME_TYPE_SOP  -> TCPC_TX_SOP (default)
> >
> > This allows TCPM to correctly distinguish SOP from SOP' messages, which
> > is required for proper cable plug communication during PD negotiation.
> >
> 
> Can you add "Fixes:" as well ?

OK. I will add below fix tag because only SOP was supported before: 
Fixes: fb7ff25ae433 ("usb: typec: tcpm: add discover identity support for SOP'")

> 
> > Signed-off-by: Xu Yang <[email protected]>
> > ---
> >  drivers/usb/typec/tcpm/tcpci.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> > index 7ac7000b2d13..c19413f41bcb 100644
> > --- a/drivers/usb/typec/tcpm/tcpci.c
> > +++ b/drivers/usb/typec/tcpm/tcpci.c
> > @@ -748,7 +748,8 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
> >
> >         if (status & TCPC_ALERT_RX_STATUS) {
> >                 struct pd_message msg;
> > -               unsigned int cnt, payload_cnt;
> > +               unsigned int cnt, type, payload_cnt;
> > +               enum tcpm_transmit_type rx_type;
> >                 u16 header;
> >
> >                 regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
> > @@ -763,6 +764,17 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
> >                 else
> >                         payload_cnt = 0;
> >
> > +               regmap_read(tcpci->regmap, TCPC_RX_BUF_FRAME_TYPE, &type);
> 
> 
> If regmap_read() fails, type would be uninitialized containing random
> values from the stack, can you make the code to handle that ?

OK.

> 
> >
> > +               switch (type) {
> > +               case TCPC_RX_BUF_FRAME_TYPE_SOP1:
> > +                       rx_type = TCPC_TX_SOP_PRIME;
> > +                       break;
> > +               case TCPC_RX_BUF_FRAME_TYPE_SOP:
> 
> 
> TCPCI Spec Table 4-38 also defines SOP'' (SOP2 = 2), SOP_DBG' (3), and
> SOP_DBG'' (4). Since enum tcpm_transmit_type defines
> TCPC_TX_SOP_PRIME_PRIME, TCPC_TX_SOP_DEBUG_PRIME, and
> TCPC_TX_SOP_DEBUG_PRIME_PRIME, why not expand the switch statement  to
> handle SOP'' and Debug frame types as well to cover all cases ?

OK. I will expend the rx_sop_type arguments for future use, although they
are not supported in TCPM now.

> 
> 
> >
> > +               default:
> > +                       rx_type = TCPC_TX_SOP;
> > +                       break;
> > +               }
> >
> > +
> >                 tcpci_read16(tcpci, TCPC_RX_HDR, &header);
> >                 msg.header = cpu_to_le16(header);
> >
> > @@ -776,7 +788,7 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
> >                 /* Read complete, clear RX status alert bit */
> >                 tcpci_write16(tcpci, TCPC_ALERT, TCPC_ALERT_RX_STATUS);
> >
> > -               tcpm_pd_receive(tcpci->port, &msg, TCPC_TX_SOP);
> > +               tcpm_pd_receive(tcpci->port, &msg, rx_type);
> 
> Please also check whether TCPC_RX_DETECT was actually programmed to
> receive the specific "rx_type" to begin with. Perhaps while
> programming TCPC_RX_DETECT the allowed received messages should be
> cached and cross checked here. This would prevent unsolicited messages
> from being passed to TCPM which poses a security risk.

OK, Make sense. Will improve it in v2.
Thanks for the suggestion!

Thanks,
Xu Yang