| Message ID | 20220228120638.678137-5-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: rng: Move TPM RNG functionality to driver model | expand |
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 [..]
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 > > [..]
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 --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)) {