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

[Diogo Ivo]

On 3/4/26 10:30, Andrei Kuchynski wrote:
> On Tue, Mar 3, 2026 at 10:54 AM Diogo Ivo <diogo.ivo@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> 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);
> > >
> > > 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