[PATCH v7 3/7] tpm: Add the RNG child device

[Ilias Apalodimas]

Hi Simon,

On Fri, 22 Jul 2022 at 12:00, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Wed, 20 Jul 2022 at 06:30, 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.
>
> Ilias has asked me to review this patch again.
>
> I don't want my review tag on it since it is not correct, so far as
> driver model / device tree go. But I have no objection to it going in
> since my understanding is we can disable TPM_RNG later as needed.

Yes you can

>
> I prefer to have some acknowledgement of the previous discussion, e.g.:

Sure

>
>    No compatible string is provided because this is not available in
> the binding defined by Linux. If multiple rand devices are in the
> system, then some method of selecting them (other than device tree)
> will need to be used, or a binding will need to be added.
>
> since I cannot imagine people remembering to look up the previous
> version in patchwork.

Comment in the code or commit message?  I'd prefer adding it as a comment

>
> But if you don't want that, then that's fine.

No I am fine that makes sense.  What I was looking for was basically
your acknowledgement that we are going to pick this up.

Shall I carry it on the TPM tree?

Thanks
/Ilias
>
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> > Changes since V6: None
> >
> >  drivers/tpm/tpm-uclass.c | 37 +++++++++++++++++++++++++++++++++----
> >  1 file changed, 33 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c
> > index f67fe1019b..e1f1ef01e1 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>
>
> BTW this should be above the previous header.
>
> > +
> > +#define TPM_RNG_DRV_NAME       "tpm-rng"
> > +
> >  int tpm_open(struct udevice *dev)
> >  {
> >         struct tpm_ops *ops = tpm_get_ops(dev);
> > @@ -136,12 +141,36 @@ 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_find_first_child_by_uclass(dev, UCLASS_RNG,
> > +                                                       &child);
> > +
> > +               if (ret != -ENODEV) {
> > +                       log_debug("RNG child already added to the TPM device\n");
> > +                       return ret;
> > +               }
> > +
> > +               ret = device_bind_driver(dev, drv, "tpm-rng0", &child);
>
> If this does get re-issued, I think this could just use
> TPM_RNG_DRV_NAME directory.
>
> > +               if (ret)
> > +                       return log_msg_ret("bind", 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,
> >         .per_device_auto        = sizeof(struct tpm_chip_priv),
> >  };
> > --
> > 2.34.1
> >
>
> Regards,
> Simon