Re: [PATCH] usb: typec: fusb302: Drop unacknowledged Get_Source_Cap_Extended

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: typec: fusb302: incomplete RX FIFO drain in fusb302_pd_read_message()
--

commit 2a38943c4db3b547e9d3ad8f9cba90c33a8b3dfa
Author: Alexey Charkov <alchark@flipper.net>

usb: typec: fusb302: Drop unacknowledged Get_Source_Cap_Extended

This commit drops Get_Source_Cap_Extended messages read from the FUSB302 RX
FIFO in fusb302_pd_read_message() because the hardware mistakes them for
GoodCRC messages and queues them in the FIFO without sending a GoodCRC or
raising an interrupt.

> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 3319f6a2b0c9b..96dc48250d6a6 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -1463,6 +1463,19 @@ static int fusb302_pd_read_message(struct fusb302_chip *chip,
>  	fusb302_log(chip, "PD message header: %x", msg->header);
>  	fusb302_log(chip, "PD message len: %d", len);
>  
> +	/*
> +	 * The chip only decodes the low four bits of the message type, so it
> +	 * mistakes Get_Source_Cap_Extended (17) for a GoodCRC (1): it neither
> +	 * acknowledges it nor raises GCRCSENT, yet still queues it in the RX
> +	 * FIFO. Without a GoodCRC the partner never considers it delivered and
> +	 * will retry and then soft reset, so it must not be answered.
> +	 */
> +	if ((!len) &&
> +	    (pd_header_type_le(msg->header) == PD_CTRL_GET_SOURCE_CAP_EXT)) {
> +		fusb302_log(chip, "dropping unacknowledged PD message");
> +		return ret;
> +	}

[Severity: High]
When fusb302_pd_read_message() drops a PD_CTRL_GET_SOURCE_CAP_EXT message and
returns early here, doesn't that leave any retried copies and the actual
interrupt-triggering PD message unread in FUSB_REG_FIFOS?

Because the FUSB302 does not raise an interrupt when it queues
PD_CTRL_GET_SOURCE_CAP_EXT, the partner will retry it (queueing up to 3 copies
in FUSB_REG_FIFOS in PD 3.0) before sending a follow-up message such as
Soft_Reset that finally raises FUSB_REG_INTERRUPTB_GCRCSENT (or a GoodCRC for
a local transmission that raises FUSB_REG_INTERRUPTA_TX_SUCCESS).

In drivers/usb/typec/tcpm/fusb302.c, fusb302_irq_work() clears the interrupt
status registers on read and only calls fusb302_pd_read_message() once for
that event:

	if (interrupta & FUSB_REG_INTERRUPTA_TX_SUCCESS) {
		fusb302_log(chip, "IRQ: PD tx success");
		ret = fusb302_pd_read_message(chip, &pd_msg);
		...
	}
	...
	if (interruptb & FUSB_REG_INTERRUPTB_GCRCSENT) {
		fusb302_log(chip, "IRQ: PD sent good CRC");
		ret = fusb302_pd_read_message(chip, &pd_msg);
		...
	}

If fusb302_pd_read_message() returns after popping only the first
PD_CTRL_GET_SOURCE_CAP_EXT frame, the remaining retries and the message that
actually triggered GCRCSENT or TX_SUCCESS stay behind in the RX FIFO and are
not delivered to TCPM on this interrupt.

Should fusb302_pd_read_message() loop back to read the next message from
FUSB_REG_FIFOS whenever it drops an unacknowledged PD_CTRL_GET_SOURCE_CAP_EXT
message?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-fusb302-getsourcecap-v1-1-26b5bfea4b2b@flipper.net?part=1