Hi
again many thanks for the quick review
On Sat, 22 Jun 2024 at 19:25, Heinrich Schuchardt <xypron.g...@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.apalodi...@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
