[PATCH v2 04/10] tpm: Move the TPM version detection functions to the uclass driver

[Simon Glass]

Hi Sughosh,

On Tue, 1 Mar 2022 at 21:40, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> hi Simon,
>
> On Tue, 1 Mar 2022 at 20:29, Simon Glass <sjg at chromium.org> wrote:
> >
> > On Mon, 28 Feb 2022 at 05:07, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > >
> > > Make the TPM version detection functions as external symbols and move
> > > them to the TPM uclass driver. These are useful functions to check the
> > > TPM device version and should not be static functions.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > > ---
> > >
> > > Changes since V1: None
> > >
> > >  drivers/tpm/tpm-uclass.c | 11 +++++++++++
> > >  include/tpm_api.h        | 20 ++++++++++++++++++++
> > >  lib/tpm_api.c            | 10 ----------
> > >  3 files changed, 31 insertions(+), 10 deletions(-)
> > >
> >
> > I just sent a similar patch a few days ago.
> >
> > > diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c
> > > index f67fe1019b..8619da89d8 100644
> > > --- a/drivers/tpm/tpm-uclass.c
> > > +++ b/drivers/tpm/tpm-uclass.c
> > > @@ -11,10 +11,21 @@
> > >  #include <log.h>
> > >  #include <linux/delay.h>
> > >  #include <linux/unaligned/be_byteshift.h>
> > > +#include <tpm_api.h>
> > >  #include <tpm-v1.h>
> > >  #include <tpm-v2.h>
> > >  #include "tpm_internal.h"
> > >
> > > +bool is_tpm1(struct udevice *dev)
> > > +{
> > > +       return IS_ENABLED(CONFIG_TPM_V1) && tpm_get_version(dev) == TPM_V1;
> > > +}
> > > +
> > > +bool is_tpm2(struct udevice *dev)
> > > +{
> > > +       return IS_ENABLED(CONFIG_TPM_V2) && tpm_get_version(dev) == TPM_V2;
> > > +}
> > > +
> >
> > I think a better name is tpm_is_v1() since this is in the tpm uclass.
> > It should have a tpm_ prefix.
>
> Okay, but do we keep this patch, or use the approach which you have
> taken to define these as inline functions in tpm_api.h. If we are to
> keep these definitions in the uclass driver, I will rename them as you
> suggest. Thanks.

Well if you put them in the .c file then they don't work property and
you have to enable by TPMv1 and TPMv2 code. The idea of the inline
functions is that they work even if you don't enable both APIs.

Regards,
Simon