* [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices
@ 2026-02-05 10:04 Diogo Ivo
2026-03-02 9:09 ` Diogo Ivo
2026-03-03 7:17 ` Andrei Kuchynski
0 siblings, 2 replies; 7+ messages in thread
From: Diogo Ivo @ 2026-02-05 10:04 UTC (permalink / raw)
To: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
Andrei Kuchynski, Tzung-Bi Shih, Guenter Roeck
Cc: chrome-platform, linux-kernel, Diogo Ivo
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@tecnico.ulisboa.pt>
---
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,
--
Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices
2026-02-05 10:04 [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices Diogo Ivo
@ 2026-03-02 9:09 ` Diogo Ivo
2026-03-03 2:49 ` Tzung-Bi Shih
2026-03-03 7:17 ` Andrei Kuchynski
1 sibling, 1 reply; 7+ messages in thread
From: Diogo Ivo @ 2026-03-02 9:09 UTC (permalink / raw)
To: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
Andrei Kuchynski, Tzung-Bi Shih, Guenter Roeck
Cc: chrome-platform, linux-kernel
Hello,
Gentle ping on this patch.
Best regards,
Diogo
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@tecnico.ulisboa.pt>
> ---
> 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,
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices
2026-03-02 9:09 ` Diogo Ivo
@ 2026-03-03 2:49 ` Tzung-Bi Shih
0 siblings, 0 replies; 7+ messages in thread
From: Tzung-Bi Shih @ 2026-03-03 2:49 UTC (permalink / raw)
To: Diogo Ivo
Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
Andrei Kuchynski, Guenter Roeck, chrome-platform, linux-kernel
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@tecnico.ulisboa.pt>
> > ---
> > 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,
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices
2026-02-05 10:04 [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices Diogo Ivo
2026-03-02 9:09 ` Diogo Ivo
@ 2026-03-03 7:17 ` Andrei Kuchynski
2026-03-03 9:54 ` Diogo Ivo
1 sibling, 1 reply; 7+ messages in thread
From: Andrei Kuchynski @ 2026-03-03 7:17 UTC (permalink / raw)
To: Diogo Ivo
Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
Tzung-Bi Shih, Guenter Roeck, chrome-platform, linux-kernel
Hi Diogo,
On Thu, Feb 5, 2026 at 11:04 AM Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> 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@tecnico.ulisboa.pt>
> ---
> 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);
While testing on Brya, I've got a role switch failure:
cros-ec-typec GOOG0014:00: Failed USB role switch, err = -22
cros-ec-typec GOOG0014:00: Update failed for port: 1
Steps to reproduce: deactivate any alt-mode.
Verified on the Brya platform.
This may be attributed to the altered sequence of mux and switch settings.
Thanks,
Andrei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices
2026-03-03 7:17 ` Andrei Kuchynski
@ 2026-03-03 9:54 ` Diogo Ivo
2026-03-04 10:30 ` Andrei Kuchynski
0 siblings, 1 reply; 7+ messages in thread
From: Diogo Ivo @ 2026-03-03 9:54 UTC (permalink / raw)
To: Andrei Kuchynski
Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
Tzung-Bi Shih, Guenter Roeck, chrome-platform, linux-kernel
Hi Andrei, thanks for testing.
On 3/3/26 07:17, Andrei Kuchynski wrote:
> Hi Diogo,
>
> On Thu, Feb 5, 2026 at 11:04 AM Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> 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@tecnico.ulisboa.pt>
>> ---
>> 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);
>
> While testing on Brya, I've got a role switch failure:
> cros-ec-typec GOOG0014:00: Failed USB role switch, err = -22
> cros-ec-typec GOOG0014:00: Update failed for port: 1
>
> Steps to reproduce: deactivate any alt-mode.
I'm unsure of what you mean by this, could you clarify?
Thanks,
Diogo
> Verified on the Brya platform.
>
> This may be attributed to the altered sequence of mux and switch settings.
>
>
> Thanks,
> Andrei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices
2026-03-03 9:54 ` Diogo Ivo
@ 2026-03-04 10:30 ` Andrei Kuchynski
2026-03-09 15:25 ` Diogo Ivo
0 siblings, 1 reply; 7+ messages in thread
From: Andrei Kuchynski @ 2026-03-04 10:30 UTC (permalink / raw)
To: Diogo Ivo
Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
Tzung-Bi Shih, Guenter Roeck, chrome-platform, linux-kernel
On Tue, Mar 3, 2026 at 10:54 AM Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> wrote:
>
> Hi Andrei, thanks for testing.
>
> On 3/3/26 07:17, Andrei Kuchynski wrote:
> > Hi Diogo,
> >
> > On Thu, Feb 5, 2026 at 11:04 AM Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> 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@tecnico.ulisboa.pt>
> >> ---
> >> 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);
> >
> > While testing on Brya, I've got a role switch failure:
> > cros-ec-typec GOOG0014:00: Failed USB role switch, err = -22
> > cros-ec-typec GOOG0014:00: Update failed for port: 1
> >
> > Steps to reproduce: deactivate any alt-mode.
>
> I'm unsure of what you mean by this, could you clarify?
>
The cros_ec_typec driver supports DisplayPort and Thunderbolt alternate
modes. During altmode activation or deactivation, the driver handles
Embedded Controller (EC) events via the cros_typec_port_update.
The `active` sysfs attribute is used to initiate these processes; writing 0
to `active` will exit DP or TBT alternate mode.
Thanks,
Andrei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices
2026-03-04 10:30 ` Andrei Kuchynski
@ 2026-03-09 15:25 ` Diogo Ivo
0 siblings, 0 replies; 7+ messages in thread
From: Diogo Ivo @ 2026-03-09 15:25 UTC (permalink / raw)
To: Andrei Kuchynski
Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
Tzung-Bi Shih, Guenter Roeck, chrome-platform, linux-kernel
On 3/4/26 10:30, Andrei Kuchynski wrote:
> On Tue, Mar 3, 2026 at 10:54 AM Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> wrote:
>>
>> Hi Andrei, thanks for testing.
>>
>> On 3/3/26 07:17, Andrei Kuchynski wrote:
>>> Hi Diogo,
>>>
>>> On Thu, Feb 5, 2026 at 11:04 AM Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> 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@tecnico.ulisboa.pt>
>>>> ---
>>>> 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);
>>>
>>> While testing on Brya, I've got a role switch failure:
>>> cros-ec-typec GOOG0014:00: Failed USB role switch, err = -22
>>> cros-ec-typec GOOG0014:00: Update failed for port: 1
>>>
>>> Steps to reproduce: deactivate any alt-mode.
>>
>> I'm unsure of what you mean by this, could you clarify?
>>
>
> The cros_ec_typec driver supports DisplayPort and Thunderbolt alternate
> modes. During altmode activation or deactivation, the driver handles
> Embedded Controller (EC) events via the cros_typec_port_update.
>
> The `active` sysfs attribute is used to initiate these processes; writing 0
> to `active` will exit DP or TBT alternate mode.
Thank you for the clarification. In any case err = -22 seems odd when the
call to usb_role_switch_set_role() is not receiving any parameters that
are obviously wrong. What is the underlying usb_role_switch
implementation in your platform?
Thanks,
Diogo
>
> Thanks,
> Andrei
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-03-09 15:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-05 10:04 [PATCH] platform/chrome: cros_ec_typec: Skip Type-C mux handling in EC-driven devices Diogo Ivo
2026-03-02 9:09 ` Diogo Ivo
2026-03-03 2:49 ` Tzung-Bi Shih
2026-03-03 7:17 ` Andrei Kuchynski
2026-03-03 9:54 ` Diogo Ivo
2026-03-04 10:30 ` Andrei Kuchynski
2026-03-09 15:25 ` Diogo Ivo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox