[PATCH v5 4/9] tpm: Add the RNG child device

[Simon Glass]

Hi Sughosh,

On Sun, 13 Mar 2022 at 08:48, 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 V4:
>
> * Put a check for CONFIG_TPM_RNG for binding the RNG device with it's
>   driver in the post_probe callback instead of putting
>   CONFIG_{SPL,TPL}_BUILD guards
>
>  drivers/tpm/tpm-uclass.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>

This looks a lot better, please see below.

> diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c
> index f67fe1019b..e1c61d26f0 100644
> --- a/drivers/tpm/tpm-uclass.c
> +++ b/drivers/tpm/tpm-uclass.c
> @@ -11,10 +11,15 @@
>  #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_RNG_DRV_NAME       "tpm-rng"
> +
>  int tpm_open(struct udevice *dev)
>  {
>         struct tpm_ops *ops = tpm_get_ops(dev);
> @@ -136,12 +141,28 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size,
>         return 0;
>  }
>
> +static int tpm_uclass_post_probe(struct udevice *dev)
> +{
> +       int ret;
> +       const char *drv = TPM_RNG_DRV_NAME;
> +       struct udevice *child;
> +
> +       if (CONFIG_IS_ENABLED(TPM_RNG)) {
> +               ret = device_bind_driver(dev, drv, "tpm-rng0", &child);
> +               if (ret)
> +                       return log_msg_ret("bind", ret);
> +       }

This really should be in the device tree so what you are doing here is
quite strange. If you want to manually bind it, please call
device_find_first_child_by_uclass() first to make sure it isn't
already there.

Also you should bind it in the bind() method, not in probe().

This is the code used for the same thing in the bootstd series:

struct udevice *bdev;
int ret;

ret = device_find_first_child_by_uclass(parent, UCLASS_BOOTDEV, &bdev);
if (ret) {
   if (ret != -ENODEV) {
   log_debug("Cannot access bootdev device\n");
   return ret;
   }

   ret = bootdev_bind(parent, drv_name, "bootdev", &bdev);
   if (ret) {
      log_debug("Cannot create bootdev device\n");
      return ret;
   }
}


> +
> +       return 0;
> +}
> +
>  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
> +       .post_probe             = tpm_uclass_post_probe,

Should be post_bind.

>         .per_device_auto        = sizeof(struct tpm_chip_priv),
>  };
> --
> 2.25.1
>

Regards,
Simon