From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miquel Raynal Date: Tue, 15 May 2018 10:56:56 +0200 Subject: [U-Boot] [PATCH v3 03/25] tpm: disociate TPMv1.x specific and generic code In-Reply-To: <20180514194307.GW12235@bill-the-cat.ec.rr.com> References: <20180502085934.29292-1-miquel.raynal@bootlin.com> <20180502085934.29292-4-miquel.raynal@bootlin.com> <20180514200157.25773607@xps13> <20180514194307.GW12235@bill-the-cat.ec.rr.com> Message-ID: <20180515105656.667cbecb@xps13> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: u-boot@lists.denx.de Hi Tom, On Mon, 14 May 2018 15:43:07 -0400, Tom Rini wrote: > On Mon, May 14, 2018 at 08:01:57PM +0200, Miquel Raynal wrote: > > Hi Simon, > >=20 > > On Wed, 2 May 2018 20:31:48 -0600, Simon Glass wrote: > > =20 > > > Hi Miquel, > > >=20 > > > On 2 May 2018 at 02:59, Miquel Raynal wro= te: =20 > > > > There are no changes in this commit unless: > > > > 1/ a new organization of the code as follow. > > > > 2/ some *very* basic checkpatch.pl corrections that polluated my re= ports > > > > like s/uint_t/u/, blank spaces and non-aligned parameters = on > > > > parenthesis. > > > > > > > > * cmd/ directory: =20 > > > > > move existing code from cmd/tpm.c in cmd/tpm-common.c > > > > > move specific code in cmd/tpm-v1.c > > > > > create a specific header file with generic definitions fo= r =20 > > > > commands only called cmd/tpm-user-utils.h > > > > > > > > * lib/ directory: =20 > > > > > move existing code from lib/tpm.c in lib/tpm-common.c > > > > > move specific code in lib/tpm-v1.c > > > > > create a specific header file with generic definitions fo= r =20 > > > > the library itself called lib/tpm-utils.h > > > > > > > > * include/ directory: =20 > > > > > move existing code from include/tpm.h in include/tpm-comm= on.h > > > > > move specific code in include/tpm-v1.h =20 > > > > > > > > Code designated as 'common' is compiled if TPM are used. Code desig= nated > > > > as 'specific' is compiled only if the right specification has been > > > > selected. > > > > > > > > All files include tpm-common.h. > > > > Files in cmd/ include tpm-user-utils.h. > > > > Files in lib/ include tpm-utils.h. > > > > Depending on the specification, files may include either (not both) > > > > tpm-v1.h or tpm-v2.h. > > > > > > > > Signed-off-by: Miquel Raynal > > > > --- > > > > cmd/Makefile | 3 +- > > > > cmd/tpm-common.c | 289 +++++++++++++++++++++++++= ++++++++++ > > > > cmd/tpm-user-utils.h | 25 +++ > > > > cmd/{tpm.c =3D> tpm-v1.c} | 305 ++---------------------= -------------- > > > > cmd/tpm_test.c | 2 +- > > > > drivers/tpm/tpm-uclass.c | 4 +- > > > > drivers/tpm/tpm_atmel_twi.c | 2 +- > > > > drivers/tpm/tpm_tis_infineon.c | 2 +- > > > > drivers/tpm/tpm_tis_lpc.c | 2 +- > > > > drivers/tpm/tpm_tis_sandbox.c | 2 +- > > > > drivers/tpm/tpm_tis_st33zp24_i2c.c | 2 +- > > > > drivers/tpm/tpm_tis_st33zp24_spi.c | 2 +- > > > > include/tpm-common.h | 214 ++++++++++++++++++++++++++ > > > > include/{tpm.h =3D> tpm-v1.h} | 274 ++++++-----------------= ---------- > > > > lib/Makefile | 3 +- > > > > lib/tpm-common.c | 189 +++++++++++++++++++++++ > > > > lib/tpm-utils.h | 96 ++++++++++++ > > > > lib/{tpm.c =3D> tpm-v1.c} | 248 +----------------------= ------- > > > > 18 files changed, 886 insertions(+), 778 deletions(-) > > > > create mode 100644 cmd/tpm-common.c > > > > create mode 100644 cmd/tpm-user-utils.h > > > > rename cmd/{tpm.c =3D> tpm-v1.c} (76%) > > > > create mode 100644 include/tpm-common.h > > > > rename include/{tpm.h =3D> tpm-v1.h} (62%) > > > > create mode 100644 lib/tpm-common.c > > > > create mode 100644 lib/tpm-utils.h > > > > rename lib/{tpm.c =3D> tpm-v1.c} (81%) > > > > =20 > > >=20 > > > This is a bit hard to review as there is so much going on. > > >=20 > > > Can you do the patman/chcekpatch clean-up in a separate patch before > > > this one? Then hopefully most of this becomes just a rename? =20 > >=20 > > I understand your point and made all the checkpatch.pl changes in > > different commits previously to this one. > >=20 > > Unfortunately, as I split one file ( being include/cmd/lib): > > - /tpm.x > > in two files: > > - /tpm-common.x > > - /tpm-v1.x > >=20 > > There will never be just a rename :/ > > =20 > > >=20 > > > Also do you have to do it all at once in one patch? It seem slike you > > > could move lib/ around separately from cmd/ for example. > > >=20 > > > At present all I can give is a rubber stamp. =20 > >=20 > > I think it would add more changes doing so because of the includes > > being renamed. Plus, I don't think the understanding would be enhanced > > as the point of this commit is to clearly separate shared code and > > specific code for TPMv1 only chips. From my point of view doing so in > > several commits does not clarify the goal, nor it would simplify the > > review :( > >=20 > > Otherwise, in the next version there will be nothing more than just code > > moves in this commit. > >=20 > > Hope this new split of the changes will be enough? =20 >=20 > You're really sure you've only got just file renames, content moving and > related header additions here? Thanks! >=20 I tried my best, yes :) Thanks, Miqu=C3=A8l