Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Avoid setting usb role during disconnect
From: Prashant Malani
Date: Wed Aug 05 2020 - 21:00:03 EST
On Wed, Aug 05, 2020 at 08:28:22PM +0000, Shaikh, Azhar wrote:
> Hi Prashant,
> > >
> > > Hi Prashant,
> > >
> > > > Is this documented anywhere? Kindly provide the links to that if so.
> > > > I wasn't aware of any ordering requirements (but I may be missing
> > something).
> > >
> > > The configuration of the connector should always happen in the order
> > > defined in the USB Type-C specification. Check ch. 2.3 (USB Type-C Spec
> > R2.0). So that will basically give you:
> > >
> > > 1. orientation
> > > 2. role(s)
> > > 3. the rest.
> > Thanks for the link. Are you referring to Section 2.3 (Configuration
> > Process) ? I couldn't find anything there which implied any required ordering
> > (I'm reading Release 2.0, Aug 2019, so I don't know if something has been
> > added since).
> > Could you kindly point me to the appropriate subsection?
> That is the section I was referring to.
(Followed up with Azhar in a side-thread)
I don't see anything in that section that enforces an ordering on how
these switches should be configured. That said, I may be misinterpreting
it so I'm happy to follow up and withdraw my reservations to the change
given valid reasoning :)
As things stand, it looks like this is being reordered because it is
necessary for the particular switch driver (and architecture) in
question, i.e intel_pmc_mux.c to fix an edge use case
there (correcting the sequencing of PMC IPC messages when handling DP in warm
I'd prefer the ordering be kept the way it was because:
1. We should preserve the existing ordering, and only fix the bug
described in the commit message, i.e avoid setting usb_role switch to
anything other than "NONE" during disconnect.
2. The CL should do 1 thing or call out why it's doing the re-ordering.
Here it is not only fixing the double call to usb_role_switch_set_role (which is
addressed in the commit message), but also re-ordering the call (which
is not addressed at all).
1.) and 2.) are sort-of related.
3. We should avoid fixing platform specific issues by changing the top
level cros-ec-typec driver. Fixing this in the mux driver will make
the driver more robust against any other sequencing issues that may
crop up later.
Also, if for example some other mux driver (driverB) requires a
different ordering (e.g mode-switch before role-switch), changing
that in cros-ec-typec will end up breaking the mux agent driver.
driverB should handle the ordering issues internally, and so should
> > >
> > > Regards,
> > > Azhar