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

[Badhri Jagan Sridharan]

On Thu, Jul 2, 2026 at 11:29 PM Xu Yang <xu.yang_2@xxxxxxxxxxx> wrote:
>
> From: Xu Yang <xu.yang_2@xxxxxxx>
>
> 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 ?

> Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> ---
>  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 ?

>
> +               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 ?


>
> +               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.


>         }
>
>         if (tcpci->data->vbus_vsafe0v && (status & TCPC_ALERT_EXTENDED_STATUS)) {
> --
> 2.34.1
>