Re: [PATCH 2/5] usb: typec: tcpci: Receive SOP' only while sourcing VCONN

[Sebastian Reichel <sebastian.reichel@collabora.com>]

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

Hi,

On Tue, Nov 03, 2026 at 03:21:23PM +0400, Alexey Charkov wrote:
> A TCPC acks every message type enabled in RECEIVE_DETECT and passes it
> on to TCPM. With SOP' enabled while the partner sources VCONN, the port
> acks the partner's requests to the cable plug and the plug's replies to
> the partner, colliding with the GoodCRC that the plug or the partner
> sends. TCPM also tries to answer these messages as if they were destined
> to it, which for the case of a partner's SOP' Discover Identity on a
> HUSB311 made the partner soft reset the contract every time.
> 
> TCPM talks to the cable plug only while it is the VCONN Source, as USB
> PD 3.0 section 6.3.11 requires, so listen for SOP' only then.
> 
> Switch SOP' reception off before VCONN goes off and on only once it is
> on. If that fails, warn instead of failing the VCONN switch: SOP'
> reception left off only costs cable discovery, and left on is no worse
> than before this change.
> 
> Cc: stable@vger.kernel.org
> Fixes: 3bbb9ba4f660 ("usb: typec: tcpci: add tcpm_transmit_type to tcpm_pd_receive")
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
> ---

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

Greetings,

-- Sebastian

>  drivers/usb/typec/tcpm/tcpci.c | 72 ++++++++++++++++++++++++++++--------------
>  1 file changed, 49 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index 6717ac914c6a..af8db6b4497a 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -41,6 +41,7 @@ struct tcpci {
>  	unsigned int rx_type_mask;
>  
>  	bool controls_vbus;
> +	bool vconn_enabled;
>  
>  	struct tcpc_dev tcpc;
>  	struct tcpci_data *data;
> @@ -344,11 +345,45 @@ static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capab
>  		tcpci->data->set_partner_usb_comm_capable(tcpci, tcpci->data, capable);
>  }
>  
> +static int tcpci_set_pd_rx(struct tcpc_dev *tcpc, bool enable)
> +{
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	unsigned int reg = 0;
> +	int ret;
> +
> +	if (enable) {
> +		reg = TCPC_RX_DETECT_SOP | TCPC_RX_DETECT_HARD_RESET;
> +		/* Only the VCONN Source talks to the cable plug */
> +		if (tcpci->data->cable_comm_capable && tcpci->vconn_enabled)
> +			reg |= TCPC_RX_DETECT_SOP1;
> +	}
> +
> +	tcpci->rx_type_mask = reg;
> +	ret = regmap_write(tcpci->regmap, TCPC_RX_DETECT, reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
>  {
>  	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
>  	int ret;
>  
> +	/*
> +	 * Without VCONN from us, SOP' traffic is between the partner and the
> +	 * cable plug, and our GoodCRC would collide with theirs. So stop
> +	 * receiving SOP' before VCONN goes off, and start only once it is on.
> +	 * Neither is worth failing the VCONN switch over.
> +	 */
> +	if (!enable) {
> +		tcpci->vconn_enabled = false;
> +		if ((tcpci->rx_type_mask & TCPC_RX_DETECT_SOP1) &&
> +		    tcpci_set_pd_rx(tcpc, true) < 0)
> +			dev_warn(tcpci->dev, "Failed to disable SOP' reception\n");
> +	}
> +
>  	/* Handle vendor set vconn */
>  	if (tcpci->data->set_vconn) {
>  		ret = tcpci->data->set_vconn(tcpci, tcpci->data, enable);
> @@ -356,9 +391,20 @@ static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
>  			return ret;
>  	}
>  
> -	return regmap_update_bits(tcpci->regmap, TCPC_POWER_CTRL,
> -				TCPC_POWER_CTRL_VCONN_ENABLE,
> -				enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
> +	ret = regmap_update_bits(tcpci->regmap, TCPC_POWER_CTRL,
> +				 TCPC_POWER_CTRL_VCONN_ENABLE,
> +				 enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (enable) {
> +		tcpci->vconn_enabled = true;
> +		if (tcpci->data->cable_comm_capable && tcpci->rx_type_mask &&
> +		    tcpci_set_pd_rx(tcpc, true) < 0)
> +			dev_warn(tcpci->dev, "Failed to enable SOP' reception\n");
> +	}
> +
> +	return 0;
>  }
>  
>  static int tcpci_enable_auto_vbus_discharge(struct tcpc_dev *dev, bool enable)
> @@ -478,26 +524,6 @@ static int tcpci_set_roles(struct tcpc_dev *tcpc, bool attached,
>  	return 0;
>  }
>  
> -static int tcpci_set_pd_rx(struct tcpc_dev *tcpc, bool enable)
> -{
> -	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> -	unsigned int reg = 0;
> -	int ret;
> -
> -	if (enable) {
> -		reg = TCPC_RX_DETECT_SOP | TCPC_RX_DETECT_HARD_RESET;
> -		if (tcpci->data->cable_comm_capable)
> -			reg |= TCPC_RX_DETECT_SOP1;
> -	}
> -
> -	tcpci->rx_type_mask = reg;
> -	ret = regmap_write(tcpci->regmap, TCPC_RX_DETECT, reg);
> -	if (ret < 0)
> -		return ret;
> -
> -	return 0;
> -}
> -
>  static int tcpci_get_vbus(struct tcpc_dev *tcpc)
>  {
>  	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> 
> -- 
> 2.56.0
> 

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