lkml.org 
[lkml]   [2020]   [Aug]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Avoid setting usb role during disconnect
    Hi,

    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
    reboots).

    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
    intel_pmc_mux.c

    BR,

    -Prashant


    >
    > >
    > > >
    > > > Regards,
    > > > Azhar

    \
     
     \ /
      Last update: 2020-08-06 02:58    [W:3.853 / U:0.160 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site