Re: [PATCH 2/3] usb: typec: fusb302: Toggle with the default Rp current

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] usb: typec: fusb302: default Rp kept after attach in
  tcpm_start_toggling()
- [Low] usb: typec: fusb302: missing Fixes trailer for
  tcpm_start_toggling() fix
--

commit acebede2c4487a9e19e13abaaf64491fba2fe4e2
Author: Arun Iyengar <aiyengar1990@gmail.com>

> usb: typec: fusb302: Toggle with the default Rp current
>
> tcpm_start_toggling() programs the Rp current that tcpm asks for, which
> is the current of the source PDO (Rp 3.0A on boards that can source
> 3A) when the port starts toggling from Unattached.SRC, and the default
> current when it starts from Unattached.SNK after a detach.
>
> A Genesys Logic based USB-C to DisplayPort cable connected to the
> Orange Pi 5 is often not detected after a warm reboot: the FUSB302
> toggle state machine raises no interrupt with it while it drives the
> high Rp current, but finds it within seconds once toggling restarts
> with the default current after another device was detached.
>
> Unattached sources may advertise any Rp level, and tcpm sets the
> configured level through set_cc() once the port is attached, so always
> toggle with the default current.
>
> Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>

[Severity: Low]
Since this fixes sink detection when toggling starts from Unattached.SRC,
should this include a Fixes tag?

  Fixes: c034a43e72dd ("staging: typec: Fairchild FUSB302 Type-c chip driver")

> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 3319f6a2b0c9b..53c5671fe45b1 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -942,10 +942,19 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>  	}
>  
>  	mutex_lock(&chip->lock);
> -	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
> +	/*
> +	 * Look for a partner with the default USB current advertisement.
> +	 * Some sinks (seen with a Genesys Logic based USB-C to DisplayPort
> +	 * cable) present an Rd that the FUSB302 toggle state machine does
> +	 * not detect while it drives the high (3.0 A) Rp current, so they
> +	 * are only found when toggling starts from the sink side. Unattached
> +	 * sources may advertise any Rp level, and tcpm sets the configured
> +	 * level through set_cc() once the port is attached.
> +	 */
> +	ret = fusb302_set_src_current(chip, SRC_CURRENT_DEFAULT);
>  	if (ret < 0) {
>  		fusb302_log(chip, "unable to set src current %s, ret=%d",
> -			    typec_cc_status_name[cc], ret);
> +			    typec_cc_status_name[TYPEC_CC_RP_DEF], ret);
>  		goto done;
>  	}

[Severity: Medium]
Can this leave the FUSB302 advertising SRC_CURRENT_DEFAULT on the CC line
after a sink attaches, even when the port is configured for 1.5A or 3.0A?

When toggling finishes as source, fusb302_handle_togdone_src() enables the
CC pull-up without changing chip->src_current_status or FUSB_REG_CONTROL0.
On attach, tcpm_src_attach() does not call tcpm_set_cc().  In SRC_STARTUP,
non-PD ports (!port->pd_supported) jump directly to SRC_READY with
port->ams == NONE_AMS, so neither tcpm_ams_start() nor tcpm_ams_finish()
ever calls tcpm_set_cc(), leaving the hardware stuck at default Rp for the
entire connection while typec_set_pwr_opmode() reports 1.5A or 3.0A:

drivers/usb/typec/tcpm/tcpm.c:run_state_machine() {
    ...
	case SRC_STARTUP:
		opmode =  tcpm_get_pwr_opmode(tcpm_rp_cc(port));
		typec_set_pwr_opmode(port->typec_port, opmode);
    ...
		if (!port->pd_supported) {
			tcpm_set_state(port, SRC_READY, 0);
			break;
		}
		tcpm_ams_start(port, POWER_NEGOTIATION);
    ...
}

Similarly, debug and audio accessory attach paths (tcpm_acc_attach())
never call tcpm_set_cc(), and on PD 2.0 ports (port->negotiated_rev <
PD_REV30) connected to a non-PD sink, tcpm_ams_start() skips
tcpm_set_cc(port, SINK_TX_NG) so default Rp remains advertised throughout
all SRC_SEND_CAPABILITIES retries until SRC_READY calls tcpm_ams_finish().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-tcpm-attach-reboot-v1-0-7657cc1e2dca@gmail.com?part=2