[PATCH] usb: typec: fusb302: Drain the RX FIFO on each PD interrupt

[PATCH] usb: typec: fusb302: Drain the RX FIFO on each PD interrupt

[Alexey Charkov]

The driver reads one message from the RX FIFO per GCRCSENT or TX_SUCCESS
interrupt, assuming every queued message raises one. The chip, however,
queues some messages without acknowledging them or raising any interrupt,
e.g. it decodes only the low four bits of the message type and mistakes
Get_Source_Cap_Extended (17) for a GoodCRC (1). From then on every read
returns the message queued before the one that raised the interrupt, so
replies answer the wrong message and a Soft_Reset is never seen. The
partner escalates to a Hard Reset, which an Apple Studio Display repeats
every 1.5s.

Furthermore, two different messages received in quick succession may be
queued without raising separate interrupts, causing all subsequent PD
communications to be permanently out of sync, which usually also leads to
a hard reset eventually.

Read messages until the FIFO reports empty, so that each one is handled
on the event it arrived with, whatever the chip left unannounced before it.

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 drivers/usb/typec/tcpm/fusb302.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 3319f6a2b0c9..6841db172e11 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -1481,6 +1481,36 @@ static int fusb302_pd_read_message(struct fusb302_chip *chip,
 	return ret;
 }
 
+/* The 80-byte RX FIFO holds at most 11 messages without data objects */
+#define FUSB302_RX_FIFO_MAX_MSGS	11
+
+static int fusb302_pd_read_messages(struct fusb302_chip *chip)
+{
+	struct pd_message pd_msg;
+	u8 status1;
+	int ret;
+	int i;
+
+	/*
+	 * Drain the whole RX FIFO rather than one message per interrupt:
+	 * messages the chip does not acknowledge raise no interrupt, and
+	 * reading one message per event would then leave every later read
+	 * returning the message queued before the one that raised it.
+	 */
+	for (i = 0; i < FUSB302_RX_FIFO_MAX_MSGS; i++) {
+		ret = fusb302_i2c_read(chip, FUSB_REG_STATUS1, &status1);
+		if (ret < 0)
+			return ret;
+		if (status1 & FUSB_REG_STATUS1_RX_EMPTY)
+			return 0;
+		ret = fusb302_pd_read_message(chip, &pd_msg);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static irqreturn_t fusb302_irq_intn(int irq, void *dev_id)
 {
 	struct fusb302_chip *chip = dev_id;
@@ -1513,7 +1543,6 @@ static void fusb302_irq_work(struct work_struct *work)
 	bool intr_togdone;
 	bool intr_bc_lvl;
 	bool intr_comp_chng;
-	struct pd_message pd_msg;
 
 	mutex_lock(&chip->lock);
 	/* grab a snapshot of intr flags */
@@ -1602,7 +1631,7 @@ static void fusb302_irq_work(struct work_struct *work)
 
 	if (interrupta & FUSB_REG_INTERRUPTA_TX_SUCCESS) {
 		fusb302_log(chip, "IRQ: PD tx success");
-		ret = fusb302_pd_read_message(chip, &pd_msg);
+		ret = fusb302_pd_read_messages(chip);
 		if (ret < 0) {
 			fusb302_log(chip,
 				    "cannot read in PD message, ret=%d", ret);
@@ -1622,7 +1651,7 @@ static void fusb302_irq_work(struct work_struct *work)
 
 	if (interruptb & FUSB_REG_INTERRUPTB_GCRCSENT) {
 		fusb302_log(chip, "IRQ: PD sent good CRC");
-		ret = fusb302_pd_read_message(chip, &pd_msg);
+		ret = fusb302_pd_read_messages(chip);
 		if (ret < 0) {
 			fusb302_log(chip,
 				    "cannot read in PD message, ret=%d", ret);

---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
change-id: 20261102-fusb302-drain-rxfifo-58ea00906afb

Best regards,
--  
Alexey Charkov <alchark@flipper.net>

Re: [PATCH] usb: typec: fusb302: Drain the RX FIFO on each PD interrupt

[Alexey Charkov]

On Mon, Nov 2, 2026 at 4:54 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] usb: typec: fusb302: unacknowledged messages passed to tcpm_pd_receive()
> --
>
> commit c6b027a56fce877cc0a50df66b78e46a0dbd88cb
> Author: Alexey Charkov <alchark@flipper.net>
>
> usb: typec: fusb302: Drain the RX FIFO on each PD interrupt
>
> This commit adds fusb302_pd_read_messages() to drain all messages from the
> 80-byte RX FIFO on each TX_SUCCESS or GCRCSENT interrupt until
> FUSB_REG_STATUS1_RX_EMPTY is set. This prevents the driver from falling one
> message behind when the hardware queues unacknowledged messages without
> raising an interrupt or when multiple messages arrive in quick succession.
>
> > diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> > index 3319f6a2b0c9b..6841db172e117 100644
> > --- a/drivers/usb/typec/tcpm/fusb302.c
> > +++ b/drivers/usb/typec/tcpm/fusb302.c
> > @@ -1481,6 +1481,36 @@ static int fusb302_pd_read_message(struct fusb302_chip *chip,
> >       return ret;
> >  }
> >
> > +/* The 80-byte RX FIFO holds at most 11 messages without data objects */
> > +#define FUSB302_RX_FIFO_MAX_MSGS     11
> > +
> > +static int fusb302_pd_read_messages(struct fusb302_chip *chip)
> > +{
> > +     struct pd_message pd_msg;
> > +     u8 status1;
> > +     int ret;
> > +     int i;
> > +
> > +     /*
> > +      * Drain the whole RX FIFO rather than one message per interrupt:
> > +      * messages the chip does not acknowledge raise no interrupt, and
> > +      * reading one message per event would then leave every later read
> > +      * returning the message queued before the one that raised it.
> > +      */
> > +     for (i = 0; i < FUSB302_RX_FIFO_MAX_MSGS; i++) {
> > +             ret = fusb302_i2c_read(chip, FUSB_REG_STATUS1, &status1);
> > +             if (ret < 0)
> > +                     return ret;
> > +             if (status1 & FUSB_REG_STATUS1_RX_EMPTY)
> > +                     return 0;
> > +             ret = fusb302_pd_read_message(chip, &pd_msg);
> > +             if (ret < 0)
> > +                     return ret;
> > +     }
> > +
> > +     return 0;
> > +}
>
> [Severity: High]
> When a PD 3.0 partner sends PD_CTRL_GET_SOURCE_CAP_EXT (type 17, 0x11, with
> len == 0), the FUSB302 hardware decodes only the low four bits of the message
> type (0x11 & 0xf == PD_CTRL_GOOD_CRC) and places the message and its retries
> into the RX FIFO without sending a GoodCRC on the CC line or raising an
> interrupt.
>
> After those retries time out, the partner sends PD_CTRL_SOFT_RESET, which the
> FUSB302 acknowledges with GoodCRC, raising FUSB_REG_INTERRUPTB_GCRCSENT. When
> fusb302_irq_work() -> fusb302_pd_read_messages() drains the RX FIFO, it calls
> fusb302_pd_read_message() for the unacknowledged PD_CTRL_GET_SOURCE_CAP_EXT
> queued ahead of PD_CTRL_SOFT_RESET:
>
> drivers/usb/typec/tcpm/fusb302.c:fusb302_pd_read_message() {
>     ...
>         if ((!len) && (pd_header_type_le(msg->header) == PD_CTRL_GOOD_CRC))
>                 tcpm_pd_transmit_complete(chip->tcpm_port, TCPC_TX_SUCCESS);
>         else
>                 tcpm_pd_receive(chip->tcpm_port, msg, TCPC_TX_SOP);
>     ...
> }
>
> Because pd_header_type_le(msg->header) checks all five bits,
> fusb302_pd_read_message() forwards the unacknowledged
> PD_CTRL_GET_SOURCE_CAP_EXT to tcpm_pd_receive().
>
> In tcpm_pd_rx_handler() -> tcpm_pd_ctrl_request() -> tcpm_pd_handle_msg() ->
> tcpm_queue_message(), TCPM updates port->rx_msgid and sets
> port->queued_message = PD_MSG_CTRL_NOT_SUPP:
>
> drivers/usb/typec/tcpm/tcpm.c:tcpm_pd_ctrl_request() {
>     ...
>         case PD_CTRL_GET_SOURCE_CAP_EXT:
>         case PD_CTRL_GET_STATUS:
>         case PD_CTRL_FR_SWAP:
>         case PD_CTRL_GET_PPS_STATUS:
>         case PD_CTRL_GET_COUNTRY_CODES:
>                 /* Currently not supported */
>                 tcpm_pd_handle_msg(port,
>                                    port->negotiated_rev < PD_REV30 ?
>                                    PD_MSG_CTRL_REJECT :
>                                    PD_MSG_CTRL_NOT_SUPP,
>                                    NONE_AMS);
>                 break;
>     ...
> }
>
> When the subsequent PD_CTRL_SOFT_RESET is processed, tcpm_pd_ctrl_request()
> calls tcpm_set_state(port, SOFT_RESET, 0) without clearing
> port->queued_message. Then, in tcpm_state_machine_work(), TCPM sends the
> queued PD_MSG_CTRL_NOT_SUPP before run_state_machine() runs the SOFT_RESET
> state to send PD_CTRL_ACCEPT:
>
> drivers/usb/typec/tcpm/tcpm.c:tcpm_state_machine_work() {
>     ...
>         if (port->queued_message && tcpm_send_queued_message(port))
>                 goto done;
>     ...
> }
>
> Doesn't forwarding unacknowledged control messages (where !len and the low
> four bits of the type equal PD_CTRL_GOOD_CRC, but the full type is not
> PD_CTRL_GOOD_CRC) to tcpm_pd_receive() cause TCPM to transmit a spurious
> PD_CTRL_NOT_SUPP on the wire while the partner is waiting for PD_CTRL_ACCEPT
> in response to its Soft_Reset?

That is exactly the case, and I have just sent a patch [1] dropping
Get_Source_Cap_Extended from the queue separately, as they are only
tangentially related and can be applied in any order, and the current
broken behavior remains broken until both patches are applied.

[1] https://lore.kernel.org/all/20261102-fusb302-getsourcecap-v1-1-26b5bfea4b2b@flipper.net/

Best regards,
Alexey

Re: [PATCH] usb: typec: fusb302: Drain the RX FIFO on each PD interrupt

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3977 bytes --]

Hi,

On Mon, Nov 02, 2026 at 04:37:59PM +0400, Alexey Charkov wrote:
> The driver reads one message from the RX FIFO per GCRCSENT or TX_SUCCESS
> interrupt, assuming every queued message raises one. The chip, however,
> queues some messages without acknowledging them or raising any interrupt,
> e.g. it decodes only the low four bits of the message type and mistakes
> Get_Source_Cap_Extended (17) for a GoodCRC (1). From then on every read
> returns the message queued before the one that raised the interrupt, so
> replies answer the wrong message and a Soft_Reset is never seen. The
> partner escalates to a Hard Reset, which an Apple Studio Display repeats
> every 1.5s.
> 
> Furthermore, two different messages received in quick succession may be
> queued without raising separate interrupts, causing all subsequent PD
> communications to be permanently out of sync, which usually also leads to
> a hard reset eventually.
> 
> Read messages until the FIFO reports empty, so that each one is handled
> on the event it arrived with, whatever the chip left unannounced before it.
> 
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/usb/typec/tcpm/fusb302.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 3319f6a2b0c9..6841db172e11 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -1481,6 +1481,36 @@ static int fusb302_pd_read_message(struct fusb302_chip *chip,
>  	return ret;
>  }
>  
> +/* The 80-byte RX FIFO holds at most 11 messages without data objects */
> +#define FUSB302_RX_FIFO_MAX_MSGS	11
> +
> +static int fusb302_pd_read_messages(struct fusb302_chip *chip)
> +{
> +	struct pd_message pd_msg;
> +	u8 status1;
> +	int ret;
> +	int i;
> +
> +	/*
> +	 * Drain the whole RX FIFO rather than one message per interrupt:
> +	 * messages the chip does not acknowledge raise no interrupt, and
> +	 * reading one message per event would then leave every later read
> +	 * returning the message queued before the one that raised it.
> +	 */
> +	for (i = 0; i < FUSB302_RX_FIFO_MAX_MSGS; i++) {
> +		ret = fusb302_i2c_read(chip, FUSB_REG_STATUS1, &status1);
> +		if (ret < 0)
> +			return ret;
> +		if (status1 & FUSB_REG_STATUS1_RX_EMPTY)
> +			return 0;
> +		ret = fusb302_pd_read_message(chip, &pd_msg);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static irqreturn_t fusb302_irq_intn(int irq, void *dev_id)
>  {
>  	struct fusb302_chip *chip = dev_id;
> @@ -1513,7 +1543,6 @@ static void fusb302_irq_work(struct work_struct *work)
>  	bool intr_togdone;
>  	bool intr_bc_lvl;
>  	bool intr_comp_chng;
> -	struct pd_message pd_msg;
>  
>  	mutex_lock(&chip->lock);
>  	/* grab a snapshot of intr flags */
> @@ -1602,7 +1631,7 @@ static void fusb302_irq_work(struct work_struct *work)
>  
>  	if (interrupta & FUSB_REG_INTERRUPTA_TX_SUCCESS) {
>  		fusb302_log(chip, "IRQ: PD tx success");
> -		ret = fusb302_pd_read_message(chip, &pd_msg);
> +		ret = fusb302_pd_read_messages(chip);
>  		if (ret < 0) {
>  			fusb302_log(chip,
>  				    "cannot read in PD message, ret=%d", ret);
> @@ -1622,7 +1651,7 @@ static void fusb302_irq_work(struct work_struct *work)
>  
>  	if (interruptb & FUSB_REG_INTERRUPTB_GCRCSENT) {
>  		fusb302_log(chip, "IRQ: PD sent good CRC");
> -		ret = fusb302_pd_read_message(chip, &pd_msg);
> +		ret = fusb302_pd_read_messages(chip);
>  		if (ret < 0) {
>  			fusb302_log(chip,
>  				    "cannot read in PD message, ret=%d", ret);
> 
> ---
> base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
> change-id: 20261102-fusb302-drain-rxfifo-58ea00906afb
> 
> Best regards,
> --  
> Alexey Charkov <alchark@flipper.net>
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]