[PATCH 4/7] tpm: Move TCG into a separate library

[Ilias Apalodimas]

Hi

again many thanks for the quick review

On Sat, 22 Jun 2024 at 19:25, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 22.06.24 16:35, Ilias Apalodimas wrote:
> > commit 97707f12fdab ("tpm: Support boot measurements") moved out code
> > from the EFI subsystem into the TPM one to support measurements when
> > booting with !EFI.
> >
> > Those were moved directly into the TPM subsystem and in the tpm-v2.c
> > library. In hindsight, it would have been better to move it in new
> > files since the TCG2 is governed by its own spec and it's cleaner
> > when we want to enable certian parts of the TPM functionality.
>
> Nits:
>
> %s/certian/certain/
>
> >
> > So let's create a header file and another library and move the TCG
> > specific bits there.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > ---
> >   boot/bootm.c       |   1 +
> >   include/efi_tcg2.h |   1 +
> >   include/tpm-v2.h   | 474 +++++-------------------------
> >   include/tpm_tcg2.h | 336 ++++++++++++++++++++++
> >   lib/Makefile       |   2 +
> >   lib/tpm-v2.c       | 676 +------------------------------------------
> >   lib/tpm_tcg2.c     | 696 +++++++++++++++++++++++++++++++++++++++++++++
>
> The patch series were easier to review if moving header definitions were
> separated from moving implementations.
>
> This patch contains changes that are not described in the commit
> message, e.g.
>
>                 if (elog->log_size) {
>                         if (log.found) {
>                                 if (elog->log_size < log.log_position)
> -                                      return -ENOBUFS;
> +                                      return -ENOSPC;

And this is a great catch. this changed in patch#1 and the correct
return is -ENOBUFS. I started working on 2 trees and obviously messed
up this rebase... Thanks!

>
> I guess you wanted to put this into patch 1.
>
> Please, separate the patches adequately.

Fair enough. I thought it was going to be hard not breaking
compilation hence the big patch. I'll try splitting it

>
> + * Copyright (c) 2020 Linaro
> + * Copyright (c) 2023 Linaro Limited
>
> The copyright lines look inconsistent. Linaro Limited exists under this
> name since April 13th, 2010. Is the 2020 copyright for a different company?
>

I'll keep the older one, same company

[...]

> > +}
> > +
> > +__weak void tcg2_platform_startup_error(struct udevice *dev, int rc) {}
> > +
>
> git warning: "new blank line at EOF".
>
> Otherwise looks good.
>
> Best regards
>
> Heinrich

Thanks
/Ilias