diff mbox series

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

Message ID 20220228120638.678137-5-sughosh.ganu@linaro.org
State New
Headers show
Series tpm: rng: Move TPM RNG functionality to driver model | expand

Commit Message

Sughosh Ganu Feb. 28, 2022, 12:06 p.m. UTC
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@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@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(-)

Comments

Simon Glass March 1, 2022, 2:58 p.m. UTC | #1
On Mon, 28 Feb 2022 at 05:07, Sughosh Ganu <sughosh.ganu@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@linaro.org>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@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.

Regards,
Simon

[..]
Sughosh Ganu March 2, 2022, 4:40 a.m. UTC | #2
hi Simon,

On Tue, 1 Mar 2022 at 20:29, Simon Glass <sjg@chromium.org> wrote:
>
> On Mon, 28 Feb 2022 at 05:07, Sughosh Ganu <sughosh.ganu@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@linaro.org>
> > Reviewed-by: Heinrich Schuchardt <xypron.glpk@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.

-sughosh

>
> Regards,
> Simon
>
> [..]
Simon Glass March 3, 2022, 3:47 a.m. UTC | #3
Hi Sughosh,

On Tue, 1 Mar 2022 at 21:40, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Tue, 1 Mar 2022 at 20:29, Simon Glass <sjg@chromium.org> wrote:
> >
> > On Mon, 28 Feb 2022 at 05:07, Sughosh Ganu <sughosh.ganu@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@linaro.org>
> > > Reviewed-by: Heinrich Schuchardt <xypron.glpk@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
diff mbox series

Patch

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;
+}
+
 int tpm_open(struct udevice *dev)
 {
 	struct tpm_ops *ops = tpm_get_ops(dev);
diff --git a/include/tpm_api.h b/include/tpm_api.h
index fb6ee14e23..c19639a688 100644
--- a/include/tpm_api.h
+++ b/include/tpm_api.h
@@ -11,6 +11,26 @@ 
 #include <tpm-v1.h>
 #include <tpm-v2.h>
 
+/**
+ * is_tpm1() - Check if it is a tpmv1 device
+ * @param dev		TPM device
+ *
+ * Check if the TPM device is a TPMv1 device
+ *
+ * Return: 1 if TPMv1, 0 otherwise
+ */
+bool is_tpm1(struct udevice *dev);
+
+/**
+ * is_tpm2() - Check if it is a tpmv2 device
+ * @param dev		TPM device
+ *
+ * Check if the TPM device is a TPMv2 device
+ *
+ * Return: 1 if TPMv2, 0 otherwise
+ */
+bool is_tpm2(struct udevice *dev);
+
 /**
  * Issue a TPM_Startup command.
  *
diff --git a/lib/tpm_api.c b/lib/tpm_api.c
index b762202866..9dd9606fa8 100644
--- a/lib/tpm_api.c
+++ b/lib/tpm_api.c
@@ -11,16 +11,6 @@ 
 #include <tpm-v2.h>
 #include <tpm_api.h>
 
-static bool is_tpm1(struct udevice *dev)
-{
-	return IS_ENABLED(CONFIG_TPM_V1) && tpm_get_version(dev) == TPM_V1;
-}
-
-static bool is_tpm2(struct udevice *dev)
-{
-	return IS_ENABLED(CONFIG_TPM_V2) && tpm_get_version(dev) == TPM_V2;
-}
-
 int tpm_startup(struct udevice *dev, enum tpm_startup_type mode)
 {
 	if (is_tpm1(dev)) {