Re: [PATCH v2] usb: typec: tcpci: don't touch CC line if it's Vconn source

[Guenter Roeck <linux@xxxxxxxxxxxx>]

On Wed, Jan 12, 2022 at 05:41:16PM +0800, Xu Yang wrote:
> With the AMS and Collision Avoidance, tcpm often needs to change the CC's
> termination. When one CC line is souring Vconn, if we still change its

s/souring/sourcing/

> termination, the voltage of the another CC line is likely to be fluctuant
> and unstable.
> 
> Therefore, we should verify whether a CC line is soucing Vconn before

s/soucing/sourcing/

> changing its termination. And only changing the termination that is
> not a Vconn line. This can be done by reading the VCONN Present bit of

... and only change ...

maybe to make it a full sentence.

> POWER_ STATUS register. To determinate the polarity, we can read the

determine

> Plug Orientation bit of TCPC_CONTROL register. Since only if Plug
> Orientation is set, Vconn can be sourced.

Maybe something like

Vconn can only be be sourced if Plug Orientation is set.

> 
> Fixes: 0908c5aca31e ("usb: typec: tcpm: AMS and Collision Avoidance")
> cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> 
> ---
> v2: changed subject line
> ---
>  drivers/usb/typec/tcpm/tcpci.c | 27 +++++++++++++++++++++++++++
>  drivers/usb/typec/tcpm/tcpci.h |  1 +
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index 35a1307349a2..0bf4cbfaa21c 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -75,9 +75,26 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
>  static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
>  {
>  	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	bool vconn_pres = false;
> +	enum typec_cc_polarity polarity = TYPEC_POLARITY_CC1;
>  	unsigned int reg;
>  	int ret;
>  
> +	ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (reg & TCPC_POWER_STATUS_VCONN_PRES) {
> +		vconn_pres = true;

How about
	vconn_pres = !!(reg & TCPC_POWER_STATUS_VCONN_PRES);

	if (vconn_pres) {
...

> +
> +		ret = regmap_read(tcpci->regmap, TCPC_TCPC_CTRL, &reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (reg & TCPC_TCPC_CTRL_ORIENTATION)
> +			polarity = TYPEC_POLARITY_CC2;
> +	}
> +
>  	switch (cc) {
>  	case TYPEC_CC_RA:
>  		reg = (TCPC_ROLE_CTRL_CC_RA << TCPC_ROLE_CTRL_CC1_SHIFT) |
> @@ -112,6 +129,16 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
>  		break;
>  	}
>  
> +	if (vconn_pres) {
> +		if (polarity == TYPEC_POLARITY_CC2) {
> +			reg &= ~(TCPC_ROLE_CTRL_CC1_MASK << TCPC_ROLE_CTRL_CC1_SHIFT);
> +			reg |= (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT);
> +		} else {
> +			reg &= ~(TCPC_ROLE_CTRL_CC2_MASK << TCPC_ROLE_CTRL_CC2_SHIFT);
> +			reg |= (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT);
> +		}
> +	}
> +
>  	ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
>  	if (ret < 0)
>  		return ret;
> diff --git a/drivers/usb/typec/tcpm/tcpci.h b/drivers/usb/typec/tcpm/tcpci.h
> index 2be7a77d400e..b2edd45f13c6 100644
> --- a/drivers/usb/typec/tcpm/tcpci.h
> +++ b/drivers/usb/typec/tcpm/tcpci.h
> @@ -98,6 +98,7 @@
>  #define TCPC_POWER_STATUS_SOURCING_VBUS	BIT(4)
>  #define TCPC_POWER_STATUS_VBUS_DET	BIT(3)
>  #define TCPC_POWER_STATUS_VBUS_PRES	BIT(2)
> +#define TCPC_POWER_STATUS_VCONN_PRES	BIT(1)
>  #define TCPC_POWER_STATUS_SINKING_VBUS	BIT(0)
>  
>  #define TCPC_FAULT_STATUS		0x1f
> -- 
> 2.25.1
>