Linux USB
 help / color / mirror / Atom feed
* [PATCH] usb: typec: fusb302: Drop unacknowledged Get_Source_Cap_Extended
@ 2026-11-02 13:09 Alexey Charkov
  2026-11-02 13:20 ` sashiko-bot
  2026-11-06 12:38 ` Heikki Krogerus
  0 siblings, 2 replies; 4+ messages in thread
From: Alexey Charkov @ 2026-11-02 13:09 UTC (permalink / raw)
  To: Heikki Krogerus, Greg Kroah-Hartman
  Cc: Sebastian Reichel, linux-usb, linux-kernel, Alexey Charkov

The chip mistakes Get_Source_Cap_Extended for a GoodCRC, as it apparently
decodes only the low four bits of the message type. As a result, it sends
no GoodCRC for it, but still queues it in the RX FIFO. The partner never
sees the message as delivered due to no GoodCRC, so it retries and then
sends a Soft_Reset. When FUSB302 receives the Soft_Reset, it finally
raises its interrupt, at which point TCPM sees Get_Source_Cap_Extended in
the RX FIFO instead of the Soft_Reset it just received, and answers it
Not_Supported, which is not a valid reply to Soft_Reset, so the partner
escalates to a hard reset instead of getting a chance to gracefully handle
a lost Get_Source_Cap_Extended.

Discard Get_Source_Cap_Extended, which FUSB302 can never properly receive
due to its inability to automatically GoodCRC it, and answering it from
the host within the protocol specified 250us window without so much as an
interrupt cannot be made to work.

This way the TCPM view of the PD exchange at least matches what the
partner saw on the wire. This lets the TCPM complete a PD negotiation over
the FUSB302 with e.g. an Apple Studio Display, which otherwise always ends
in hard resets.

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

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 3319f6a2b0c9..96dc48250d6a 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;
+	}
+
 	/*
 	 * Check if we've read off a GoodCRC message. If so then indicate to
 	 * TCPM that the previous transmission has completed. Otherwise we pass

---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
change-id: 20261102-fusb302-getsourcecap-01c38dccb230

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb: typec: fusb302: Drop unacknowledged Get_Source_Cap_Extended
  2026-11-02 13:09 [PATCH] usb: typec: fusb302: Drop unacknowledged Get_Source_Cap_Extended Alexey Charkov
@ 2026-11-02 13:20 ` sashiko-bot
  2026-11-02 13:21   ` Alexey Charkov
  2026-11-06 12:38 ` Heikki Krogerus
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-11-02 13:20 UTC (permalink / raw)
  To: Alexey Charkov; +Cc: linux-usb

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb: typec: fusb302: Drop unacknowledged Get_Source_Cap_Extended
  2026-11-02 13:20 ` sashiko-bot
@ 2026-11-02 13:21   ` Alexey Charkov
  0 siblings, 0 replies; 4+ messages in thread
From: Alexey Charkov @ 2026-11-02 13:21 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-usb

On Mon, Nov 2, 2026 at 5:20 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: 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?

Addressed separately in [1]

[1] https://lore.kernel.org/linux-usb/20261102-fusb302-drain-rxfifo-v1-1-bd9b232ab345@flipper.net/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb: typec: fusb302: Drop unacknowledged Get_Source_Cap_Extended
  2026-11-02 13:09 [PATCH] usb: typec: fusb302: Drop unacknowledged Get_Source_Cap_Extended Alexey Charkov
  2026-11-02 13:20 ` sashiko-bot
@ 2026-11-06 12:38 ` Heikki Krogerus
  1 sibling, 0 replies; 4+ messages in thread
From: Heikki Krogerus @ 2026-11-06 12:38 UTC (permalink / raw)
  To: Alexey Charkov
  Cc: Greg Kroah-Hartman, Sebastian Reichel, linux-usb, linux-kernel

On Mon, Nov 02, 2026 at 05:09:05PM +0400, Alexey Charkov wrote:
> The chip mistakes Get_Source_Cap_Extended for a GoodCRC, as it apparently
> decodes only the low four bits of the message type. As a result, it sends
> no GoodCRC for it, but still queues it in the RX FIFO. The partner never
> sees the message as delivered due to no GoodCRC, so it retries and then
> sends a Soft_Reset. When FUSB302 receives the Soft_Reset, it finally
> raises its interrupt, at which point TCPM sees Get_Source_Cap_Extended in
> the RX FIFO instead of the Soft_Reset it just received, and answers it
> Not_Supported, which is not a valid reply to Soft_Reset, so the partner
> escalates to a hard reset instead of getting a chance to gracefully handle
> a lost Get_Source_Cap_Extended.
> 
> Discard Get_Source_Cap_Extended, which FUSB302 can never properly receive
> due to its inability to automatically GoodCRC it, and answering it from
> the host within the protocol specified 250us window without so much as an
> interrupt cannot be made to work.
> 
> This way the TCPM view of the PD exchange at least matches what the
> partner saw on the wire. This lets the TCPM complete a PD negotiation over
> the FUSB302 with e.g. an Apple Studio Display, which otherwise always ends
> in hard resets.
> 
> Signed-off-by: Alexey Charkov <alchark@flipper.net>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tcpm/fusb302.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 3319f6a2b0c9..96dc48250d6a 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;
> +	}
> +
>  	/*
>  	 * Check if we've read off a GoodCRC message. If so then indicate to
>  	 * TCPM that the previous transmission has completed. Otherwise we pass
> 
> ---
> base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
> change-id: 20261102-fusb302-getsourcecap-01c38dccb230
> 
> Best regards,
> --  
> Alexey Charkov <alchark@flipper.net>

-- 
heikki

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-06 12:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-02 13:09 [PATCH] usb: typec: fusb302: Drop unacknowledged Get_Source_Cap_Extended Alexey Charkov
2026-11-02 13:20 ` sashiko-bot
2026-11-02 13:21   ` Alexey Charkov
2026-11-06 12:38 ` Heikki Krogerus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox