[PATCH] usb: typec: fusb302: cache PD RX state

[PATCH] usb: typec: fusb302: cache PD RX state

[Sebastian Reichel]

This patch fixes a race condition communication error, which ends up in
PD hard resets when losing the race. Some systems, like the Radxa ROCK
5B are powered through USB-C without any backup power source and use a
FUSB302 chip to do the PD negotiation. This means it is quite important
to avoid hard resets, since that effectively kills the system's
power-supply.

I've found the following race condition while debugging unplanned power
loss during booting the board every now and then:

1. lots of TCPM/FUSB302/PD initialization stuff
2. TCPM ends up in SNK_WAIT_CAPABILITIES (tcpm_set_pd_rx is enabled here)
3. the remote PD source does not send anything, so TCPM does a SOFT RESET
4. TCPM ends up in SNK_WAIT_CAPABILITIES for the second time
   (tcpm_set_pd_rx is enabled again, even though it is still on)

At this point I've seen broken CRC good messages being send by the
FUSB302 with a logic analyzer sniffing the CC lines. Also it looks like
messages are being lost and things generally going haywire with one of
the two sides doing a hard reset once a broken CRC good message was send
to the bus.

I think the system is running into a race condition, that the FIFOs are
being cleared and/or the automatic good CRC message generation flag is
being updated while a message is already arriving.

Let's avoid this by caching the PD RX enabled state, as we have already
processed anything in the FIFOs and are in a good state. As a side
effect that this also optimizes I2C bus usage :)

As far as I can tell the problem theoretically also exists when TCPM
enters SNK_WAIT_CAPABILITIES the first time, but I believe this is less
critical for the following reason:

On devices like the ROCK 5B, which are powered through a TCPM backed
USB-C port, the bootloader must have done some prior PD communication
(initial communication must happen within 5 seconds after plugging the
USB-C plug). This means the first time the kernel TCPM state machine
reaches SNK_WAIT_CAPABILITIES, the remote side is not sending messages
actively. On other devices a hard reset simply adds some extra delay and
things should be good afterwards.

Fixes: c034a43e72dda ("staging: typec: Fairchild FUSB302 Type-c chip driver")
Cc: stable@vger.kernel.org
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/usb/typec/tcpm/fusb302.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index f15c63d3a8f441569ec98302f5b241430d8e4547..870a71f953f6cd8dfc618caea56f72782e40ee1c 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -104,6 +104,7 @@ struct fusb302_chip {
 	bool vconn_on;
 	bool vbus_on;
 	bool charge_on;
+	bool pd_rx_on;
 	bool vbus_present;
 	enum typec_cc_polarity cc_polarity;
 	enum typec_cc_status cc1;
@@ -841,6 +842,11 @@ static int tcpm_set_pd_rx(struct tcpc_dev *dev, bool on)
 	int ret = 0;
 
 	mutex_lock(&chip->lock);
+	if (chip->pd_rx_on == on) {
+		fusb302_log(chip, "pd is already %s", str_on_off(on));
+		goto done;
+	}
+
 	ret = fusb302_pd_rx_flush(chip);
 	if (ret < 0) {
 		fusb302_log(chip, "cannot flush pd rx buffer, ret=%d", ret);
@@ -863,6 +869,8 @@ static int tcpm_set_pd_rx(struct tcpc_dev *dev, bool on)
 			    str_on_off(on), ret);
 		goto done;
 	}
+
+	chip->pd_rx_on = on;
 	fusb302_log(chip, "pd := %s", str_on_off(on));
 done:
 	mutex_unlock(&chip->lock);

---
base-commit: c435a4f487e8c6a3b23dafbda87d971d4fd14e0b
change-id: 20250704-fusb302-race-condition-fix-9cc9de73f05d

Best regards,
-- 
Sebastian Reichel <sre@kernel.org>

Re: [PATCH] usb: typec: fusb302: cache PD RX state (fix for 6.16)

[Sebastian Reichel]

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

Hi,

> This patch fixes a race condition communication error, which ends up in
> PD hard resets when losing the race. [...]

Gentle ping, since it has been sent two weeks ago in a few hours
and ideally I would like to see this fix in 6.16, which isn't that
far away anymore :)

If my mail somehow got lost in your INBOX, lore has a copy:

https://lore.kernel.org/linux-usb/20250704-fusb302-race-condition-fix-v1-1-239012c0e27a@kernel.org/

Greetings,

-- Sebastian

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

Re: [PATCH] usb: typec: fusb302: cache PD RX state

[Heikki Krogerus]

On Fri, Jul 04, 2025 at 07:55:06PM +0200, Sebastian Reichel wrote:
> This patch fixes a race condition communication error, which ends up in
> PD hard resets when losing the race. Some systems, like the Radxa ROCK
> 5B are powered through USB-C without any backup power source and use a
> FUSB302 chip to do the PD negotiation. This means it is quite important
> to avoid hard resets, since that effectively kills the system's
> power-supply.
> 
> I've found the following race condition while debugging unplanned power
> loss during booting the board every now and then:
> 
> 1. lots of TCPM/FUSB302/PD initialization stuff
> 2. TCPM ends up in SNK_WAIT_CAPABILITIES (tcpm_set_pd_rx is enabled here)
> 3. the remote PD source does not send anything, so TCPM does a SOFT RESET
> 4. TCPM ends up in SNK_WAIT_CAPABILITIES for the second time
>    (tcpm_set_pd_rx is enabled again, even though it is still on)
> 
> At this point I've seen broken CRC good messages being send by the
> FUSB302 with a logic analyzer sniffing the CC lines. Also it looks like
> messages are being lost and things generally going haywire with one of
> the two sides doing a hard reset once a broken CRC good message was send
> to the bus.
> 
> I think the system is running into a race condition, that the FIFOs are
> being cleared and/or the automatic good CRC message generation flag is
> being updated while a message is already arriving.
> 
> Let's avoid this by caching the PD RX enabled state, as we have already
> processed anything in the FIFOs and are in a good state. As a side
> effect that this also optimizes I2C bus usage :)
> 
> As far as I can tell the problem theoretically also exists when TCPM
> enters SNK_WAIT_CAPABILITIES the first time, but I believe this is less
> critical for the following reason:
> 
> On devices like the ROCK 5B, which are powered through a TCPM backed
> USB-C port, the bootloader must have done some prior PD communication
> (initial communication must happen within 5 seconds after plugging the
> USB-C plug). This means the first time the kernel TCPM state machine
> reaches SNK_WAIT_CAPABILITIES, the remote side is not sending messages
> actively. On other devices a hard reset simply adds some extra delay and
> things should be good afterwards.
> 
> Fixes: c034a43e72dda ("staging: typec: Fairchild FUSB302 Type-c chip driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

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

> ---
>  drivers/usb/typec/tcpm/fusb302.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index f15c63d3a8f441569ec98302f5b241430d8e4547..870a71f953f6cd8dfc618caea56f72782e40ee1c 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -104,6 +104,7 @@ struct fusb302_chip {
>  	bool vconn_on;
>  	bool vbus_on;
>  	bool charge_on;
> +	bool pd_rx_on;
>  	bool vbus_present;
>  	enum typec_cc_polarity cc_polarity;
>  	enum typec_cc_status cc1;
> @@ -841,6 +842,11 @@ static int tcpm_set_pd_rx(struct tcpc_dev *dev, bool on)
>  	int ret = 0;
>  
>  	mutex_lock(&chip->lock);
> +	if (chip->pd_rx_on == on) {
> +		fusb302_log(chip, "pd is already %s", str_on_off(on));
> +		goto done;
> +	}
> +
>  	ret = fusb302_pd_rx_flush(chip);
>  	if (ret < 0) {
>  		fusb302_log(chip, "cannot flush pd rx buffer, ret=%d", ret);
> @@ -863,6 +869,8 @@ static int tcpm_set_pd_rx(struct tcpc_dev *dev, bool on)
>  			    str_on_off(on), ret);
>  		goto done;
>  	}
> +
> +	chip->pd_rx_on = on;
>  	fusb302_log(chip, "pd := %s", str_on_off(on));
>  done:
>  	mutex_unlock(&chip->lock);
> 
> ---
> base-commit: c435a4f487e8c6a3b23dafbda87d971d4fd14e0b
> change-id: 20250704-fusb302-race-condition-fix-9cc9de73f05d
> 
> Best regards,
> -- 
> Sebastian Reichel <sre@kernel.org>

-- 
heikki