Re: [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices

[Tzung-Bi Shih]

On Mon, Mar 02, 2026 at 09:09:22AM +0000, Diogo Ivo wrote:
> Hello,
> 
> Gentle ping on this patch.

CHROMEOS EC USB TYPE-C DRIVER maintainers: Could you take a look on the patch?

> 
> On 2/5/26 10:04, Diogo Ivo wrote:
> > Currently the code assumes that the EC firmware will leave Type-C mux
> > handling logic to the AP, exposing an interface to query and change the
> > mux state via a request from the AP. However, in devices such as Smaug
> > the EC automatically takes care of mux configuration and only USB role
> > switching needs to be handled by the AP. In fact, in such devices this
> > interface is not exposed by the EC making the whole process fail,
> > including role switching.
> > 
> > Fix this by first separating Type-C mux handling and USB role switching,
> > explicitly querying the behaviour of the EC and execute each part
> > conditionally according to what the EC reported.
> > 
> > Signed-off-by: Diogo Ivo <diogo.ivo@xxxxxxxxxxxxxxxxxx>
> > ---
> >   drivers/platform/chrome/cros_ec_typec.c | 49 +++++++++++++++++++++++----------
> >   drivers/platform/chrome/cros_ec_typec.h |  2 +-
> >   2 files changed, 35 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> > index b712bcff6fb2..bd7e6c365cab 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -751,11 +751,10 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
> >   	}
> >   	/* No change needs to be made, let's exit early. */
> > -	if (port->mux_flags == resp.flags && port->role == pd_ctrl->role)
> > +	if (port->mux_flags == resp.flags)
> >   		return 0;
> >   	port->mux_flags = resp.flags;
> > -	port->role = pd_ctrl->role;
> >   	if (port->mux_flags == USB_PD_MUX_NONE) {
> >   		ret = cros_typec_usb_disconnect_state(port);
> > @@ -771,12 +770,6 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
> >   	if (ret)
> >   		return ret;
> > -	ret = usb_role_switch_set_role(typec->ports[port_num]->role_sw,
> > -					pd_ctrl->role & PD_CTRL_RESP_ROLE_DATA
> > -					? USB_ROLE_HOST : USB_ROLE_DEVICE);
> > -	if (ret)
> > -		return ret;
> > -
> >   	if (port->mux_flags & USB_PD_MUX_USB4_ENABLED) {
> >   		ret = cros_typec_enable_usb4(typec, port_num, pd_ctrl);
> >   	} else if (port->mux_flags & USB_PD_MUX_TBT_COMPAT_ENABLED) {
> > @@ -822,6 +815,25 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
> >   	return ret;
> >   }
> > +static int cros_typec_set_role(struct cros_typec_data *typec, int port_num,
> > +			       struct ec_response_usb_pd_control_v1 *resp)
> > +{
> > +	enum usb_role cur_role = usb_role_switch_get_role(typec->ports[port_num]->role_sw);
> > +	enum usb_role role = resp->role & PD_CTRL_RESP_ROLE_DATA ? USB_ROLE_HOST :
> > +								   USB_ROLE_DEVICE;
> > +	bool connected = resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED;
> > +	int ret;
> > +
> > +	if (!connected || cur_role == role)
> > +		return 0;
> > +
> > +	ret = usb_role_switch_set_role(typec->ports[port_num]->role_sw, role);
> > +	if (ret)
> > +		dev_err(typec->dev, "Failed USB role switch, err = %d\n", ret);
> > +
> > +	return ret;
> > +}
> > +
> >   static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
> >   		int port_num, struct ec_response_usb_pd_control *resp)
> >   {
> > @@ -1251,27 +1263,32 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
> >   	if (ret < 0)
> >   		return ret;
> > -	/* Update the switches if they exist, according to requested state */
> > -	ret = cros_typec_configure_mux(typec, port_num, &resp);
> > -	if (ret)
> > -		dev_warn(typec->dev, "Configure muxes failed, err = %d\n", ret);
> > +	if (typec->ap_driven_mux) {
> > +		/* Update the switches if they exist, according to requested state */
> > +		ret = cros_typec_configure_mux(typec, port_num, &resp);
> > +		if (ret)
> > +			dev_warn(typec->dev, "Configure muxes failed, err = %d\n", ret);
> > +	}
> >   	dev_dbg(typec->dev, "Enabled %d: 0x%hhx\n", port_num, resp.enabled);
> >   	dev_dbg(typec->dev, "Role %d: 0x%hhx\n", port_num, resp.role);
> >   	dev_dbg(typec->dev, "Polarity %d: 0x%hhx\n", port_num, resp.polarity);
> >   	dev_dbg(typec->dev, "State %d: %s\n", port_num, resp.state);
> > -	if (typec->pd_ctrl_ver != 0)
> > +	if (typec->pd_ctrl_ver != 0) {
> > +		ret = cros_typec_set_role(typec, port_num,
> > +			(struct ec_response_usb_pd_control_v1 *)&resp);
> >   		cros_typec_set_port_params_v1(typec, port_num,
> >   			(struct ec_response_usb_pd_control_v1 *)&resp);
> > -	else
> > +	} else {
> >   		cros_typec_set_port_params_v0(typec, port_num,
> >   			(struct ec_response_usb_pd_control *) &resp);
> > +	}
> >   	if (typec->typec_cmd_supported)
> >   		cros_typec_handle_status(typec, port_num);
> > -	return 0;
> > +	return ret;
> >   }
> >   static int cros_typec_get_cmd_version(struct cros_typec_data *typec)
> > @@ -1375,6 +1392,8 @@ static int cros_typec_probe(struct platform_device *pdev)
> >   	typec->needs_mux_ack = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_MUX_REQUIRE_AP_ACK);
> >   	typec->ap_driven_altmode = cros_ec_check_features(
> >   		ec_dev, EC_FEATURE_TYPEC_REQUIRE_AP_MODE_ENTRY);
> > +	typec->ap_driven_mux = cros_ec_check_features(
> > +		ec_dev, EC_FEATURE_USBC_SS_MUX_VIRTUAL);
> >   	ret = cros_ec_cmd(typec->ec, 0, EC_CMD_USB_PD_PORTS, NULL, 0,
> >   			  &resp, sizeof(resp));
> > diff --git a/drivers/platform/chrome/cros_ec_typec.h b/drivers/platform/chrome/cros_ec_typec.h
> > index f9c31f04c102..9698f27169c6 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.h
> > +++ b/drivers/platform/chrome/cros_ec_typec.h
> > @@ -41,6 +41,7 @@ struct cros_typec_data {
> >   	bool typec_cmd_supported;
> >   	bool needs_mux_ack;
> >   	bool ap_driven_altmode;
> > +	bool ap_driven_mux;
> >   };
> >   /* Per port data. */
> > @@ -65,7 +66,6 @@ struct cros_typec_port {
> >   	/* Variables keeping track of switch state. */
> >   	struct typec_mux_state state;
> >   	uint8_t mux_flags;
> > -	uint8_t role;
> >   	struct typec_altmode *port_altmode[CROS_EC_ALTMODE_MAX];
> > 
> > ---
> > base-commit: 1cf43b664b8b2f9cd04b9906193713e4b693dac7
> > change-id: 20260128-smaug-type_c-d34b9510fb5e
> > 
> > Best regards,
>