linux-kernel - Re: [PATCH 7/8] platform/chrome: cros_ec

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <pl6xv55ohx3jprgc67umftjrwzlvass75d2pqzzdnktvt3njxh@gbujd5qf3tpz>
Date: Thu, 26 Sep 2024 00:20:23 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
Cc: heikki.krogerus@...ux.intel.com, tzungbi@...nel.org, jthies@...gle.com, 
	pmalani@...omium.org, akuchynski@...gle.com, Benson Leung <bleung@...omium.org>, 
	Guenter Roeck <groeck@...omium.org>, chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 7/8] platform/chrome: cros_ec_typec: Thunderbolt support

On Wed, Sep 25, 2024 at 11:42:46AM GMT, Abhishek Pandit-Subedi wrote:
> On Wed, Sep 25, 2024 at 10:13 AM Dmitry Baryshkov
> <dmitry.baryshkov@...aro.org> wrote:
> >
> > On Wed, Sep 25, 2024 at 09:25:08AM GMT, Abhishek Pandit-Subedi wrote:
> > > Add support for entering and exiting Thunderbolt alt-mode using AP
> > > driven alt-mode.
> > >
> > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
> > > ---
> > >
> > >  drivers/platform/chrome/Makefile              |   1 +
> > >  drivers/platform/chrome/cros_ec_typec.c       |  29 +--
> > >  drivers/platform/chrome/cros_typec_altmode.h  |  14 ++
> > >  .../platform/chrome/cros_typec_thunderbolt.c  | 184 ++++++++++++++++++
> > >  4 files changed, 216 insertions(+), 12 deletions(-)
> > >  create mode 100644 drivers/platform/chrome/cros_typec_thunderbolt.c
> >
> > This patch looks like nearly exact 1:1 copy of the previous one. Please
> > merge both altmode implementations in the same way as tcpm.c does.
> 
> It's easier for tcpm.c to have a merged implementation because it
> simply forwards VDMs to the internal state machine to handle without
> doing anything with them. Our implementation is closer to
> ucsi/displayport.c which needs to maintain an internal state machine
> for DP and TBT VDMs and respond differently.
> 
> I can merge the two but I'd like to understand intent (reduce code
> duplication? reduce the number of files?). As it is, keeping the files
> separate makes it easier to understand how each alt-mode operates in
> my opinion.

Separate common code and AltMode-specific code. This way we reduce a
risk of errors fixed in only one of two drivers and at the same time the
driver clearly separates common vs specific code paths (e.g. VDM
handling is mode-specific, while the rest of the code is common).

> 
> >
> > --
> > With best wishes
> > Dmitry

-- 
With best wishes
Dmitry