Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220207214026.1526151-1-pmalani@chromium.org>
 <20220207214026.1526151-4-pmalani@chromium.org> <YgIBv2SQdwXm7RLt@google.com>
 <CACeCKaezRNgsqrq3QJ1ViHjYScFrjpC-kCaYmE8kOXCcW9MRrw@mail.gmail.com>
 <YgJQ30D8nc4LWvUy@google.com> <CACeCKaf4+jxSkKFqx9g3YSPWj-FkbL=jrmHE3YEu7oU0ySPJ4Q@mail.gmail.com>
 <CACeCKadujMj4pJqf_jNNmO4UkQinw9QNu_MXe5zkTySUn6R=dw@mail.gmail.com> <YgM1pjwN2MHaMQCJ@google.com>
In-Reply-To: <YgM1pjwN2MHaMQCJ@google.com>
From:   Prashant Malani <pmalani@chromium.org>
Date:   Tue, 8 Feb 2022 19:41:58 -0800
Message-ID: <CACeCKafm3o2Ht3F0njQZ+JptDM=yZMpi0qUS=DR07LiCC_O4Ww@mail.gmail.com>
Subject: Re: [PATCH 3/4] platform/chrome: cros_ec_typec: Configure muxes at
 start of port update
To:     Tzung-Bi Shih <tzungbi@google.com>
Cc:     linux-kernel@vger.kernel.org, Benson Leung <bleung@chromium.org>,
        Guenter Roeck <groeck@chromium.org>,
        "open list:CHROMEOS EC USB TYPE-C DRIVER" 
        <chrome-platform@lists.linux.dev>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Feb 8, 2022 at 7:31 PM Tzung-Bi Shih <tzungbi@google.com> wrote:
>
> On Tue, Feb 08, 2022 at 02:58:51PM -0800, Prashant Malani wrote:
> > On Tue, Feb 8, 2022 at 10:35 AM Prashant Malani <pmalani@chromium.org> wrote:
> > > In general, I think you may benefit from reading:
> > > - The entire cros_ec_typec driver
> > > - The EC Type C state machine [2] and interfaces [3][4]
> > >
> > > The above 2 will help understand how this entire stack works. Without
> > > it, it is challenging
> > > to process the flow (just from code review).
> > >
> > > If you have further questions our would like to better understand the
> > > drivers, feel free to reach
> > > out to me over IM/email. I don't think public list code review is the
> > > best option for this
> > > sort of explanation.
> > >
> > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/platform/chrome/cros_ec_typec.c#L549
> > > [2] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/common/usbc/
> > > [3] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/driver/usb_mux/usb_mux.c
> > > [4] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/common/usb_pd_host_cmd.c
>
> Thanks for the link pointers.  It would take me some time to understand the
> background for reviewing the patch though.
>
> But in general, I would suggest to change the title for a stronger signal
> that it fixes something (per [1]).  Otherwise, the title and the description
> in cover letter looks like a move refactor.

I'm hesitant to use the term "fix" in the title; IMO the commit
message describes the potential race involved.
Since it is an unlikely edge case, brought about by limitations in how
the EC updates its state,
I'd like to avoid assigning it a "Fixes" label  too.

I can perhaps add some info to the cover letter, but I don't think
that alone is worth a v3.
I'll let this series hang for a few days; if there are other comments
which necessitate a respin, I'll
reword the cover letter.

-Prashant

>
> [1]: https://patchwork.kernel.org/project/chrome-platform/patch/20220207214026.1526151-4-pmalani@chromium.org/#24727676