linux-kernel - Re: [PATCH 3/4] platform/chrome: cros_ec_typec: Configure muxes at start of port update

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACeCKafm3o2Ht3F0njQZ+JptDM=yZMpi0qUS=DR07LiCC_O4Ww@mail.gmail.com>
Date:   Tue, 8 Feb 2022 19:41:58 -0800
From:   Prashant Malani <pmalani@...omium.org>
To:     Tzung-Bi Shih <tzungbi@...gle.com>
Cc:     linux-kernel@...r.kernel.org, Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        "open list:CHROMEOS EC USB TYPE-C DRIVER" 
        <chrome-platform@...ts.linux.dev>
Subject: Re: [PATCH 3/4] platform/chrome: cros_ec_typec: Configure muxes at
 start of port update

On Tue, Feb 8, 2022 at 7:31 PM Tzung-Bi Shih <tzungbi@...gle.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@...omium.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