[PATCH v4 4/8] tpm: Add the RNG child device

[Simon Glass]

Hi Sughosh,

On Wed, 9 Mar 2022 at 05:28, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> The TPM device comes with the random number generator(RNG)
> functionality which is built into the TPM device. Add logic to add the
> RNG child device in the TPM uclass post probe callback.
>
> The RNG device can then be used to pass a set of random bytes to the
> linux kernel, need for address space randomisation through the
> EFI_RNG_PROTOCOL interface.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
>
> Changes since V3:
>
> * Build the RNG child addition only for the u-boot proper stage using
>   the CONFIG_{SPL,TPL}_BUILD guards instead of CONFIG_TPM config which
>   gets included in all stages.
> * Remove the child_pre_probe callback which was starting the TPM
>   device based on review from Simon.
>
>  drivers/tpm/tpm-uclass.c | 40 ++++++++++++++++++++++++++++++++++++----
>  lib/Kconfig              |  1 +
>  2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c
> index f67fe1019b..241ed01e68 100644
> --- a/drivers/tpm/tpm-uclass.c
> +++ b/drivers/tpm/tpm-uclass.c
> @@ -11,10 +11,16 @@
>  #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"
>
> +#include <dm/lists.h>
> +
> +#define TPM_RNG1_DRV_NAME      "tpm1-rng"
> +#define TPM_RNG2_DRV_NAME      "tpm2-rng"
> +
>  int tpm_open(struct udevice *dev)
>  {
>         struct tpm_ops *ops = tpm_get_ops(dev);
> @@ -136,12 +142,38 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size,
>         return 0;
>  }
>
> +#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)

Drop that

> +static int tpm_uclass_post_probe(struct udevice *dev)
> +{
> +       int ret;
> +       const char *drv = tpm_is_v1(dev) ?
> +               TPM_RNG1_DRV_NAME : TPM_RNG2_DRV_NAME;
> +       struct udevice *child;
> +

if (CONFIG_IS_ENABLED(TPM_RNG))  {

> +       ret = device_bind_driver(dev, drv, "tpm-rng0", &child);
> +       if (ret == -ENOENT) {
> +               log_err("No driver configured for tpm-rng device\n");

That is a lot of code size for the string that will likely never
happen. You can use

> +               return 0;

You should report the error.

> +       }
> +
> +       if (ret) {
> +               log_err("Unable to bind rng driver with the tpm-rng device\n");

You might want to use

   return log_msg_ret("bind", ret)

since it doesn't add to code since unless you turn on CONFIG_LOG_ERROR_RETURN

> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +#endif /* !CONFIG_SPL_BUILD && !CONFIG_TPL_BUILD */
> +
>  UCLASS_DRIVER(tpm) = {
> -       .id             = UCLASS_TPM,
> -       .name           = "tpm",
> -       .flags          = DM_UC_FLAG_SEQ_ALIAS,
> +       .id                     = UCLASS_TPM,
> +       .name                   = "tpm",
> +       .flags                  = DM_UC_FLAG_SEQ_ALIAS,
>  #if CONFIG_IS_ENABLED(OF_REAL)
> -       .post_bind      = dm_scan_fdt_dev,
> +       .post_bind              = dm_scan_fdt_dev,
> +#endif
> +#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)

Drop that and do it in the function as above

> +       .post_probe             = tpm_uclass_post_probe,
>  #endif
>         .per_device_auto        = sizeof(struct tpm_chip_priv),
>  };
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 3c6fa99b1a..0f05c97afc 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -341,6 +341,7 @@ source lib/crypt/Kconfig
>  config TPM
>         bool "Trusted Platform Module (TPM) Support"
>         depends on DM
> +       select DM_RNG

imply might be better, so boards can disable it.

>         help
>           This enables support for TPMs which can be used to provide security
>           features for your board. The TPM can be connected via LPC or I2C
> --
> 2.25.1
>

Regards,
Simon