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

[Ilias Apalodimas]

Hi Simon, 

A bit late on this, but at least I found it...

On Wed, Mar 16, 2022 at 10:15:34AM +1300, Simon Glass wrote:
> Hi Ilias,
> 
> On Tue, 15 Mar 2022 at 00:34, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Mon, 14 Mar 2022 at 20:24, Simon Glass <sjg at chromium.org> wrote:
> > >
> >
> > [...]
> >
> > > > > > +       }
> > > > >
> > > > > This really should be in the device tree so what you are doing here is
> > > > > quite strange.
> > > >
> > > > Like I had mentioned in my earlier emails, the TPM device has a
> > > > builtin RNG functionality, which is non-optional. So I don't
> > > > understand why we need to use a device tree subnode here. Whether the
> > > > device is being bound to the parent is being controlled by the TPM_RNG
> > > > config that you asked me to put in my previous version, which I am
> > > > doing.
> > >
> > > See how PMICs work, for example. We have GPIOs, regulators and
> > > suchlike in the PMIC and we add subnodes for them in the DT. It is
> > > just how it is done.
> > >
> > > Driver model is designed to automatically bind devices based on the
> > > device tree. There are cases where it is necessary to manually bind
> > > things, but we mustn't prevent people from doing it 'properly'.
> >
> > There's a small difference here though.  The RNG is not a device.  The
> > TPM is the device and an encoded command to that device returns a
> > random number.  There's no binding initiating etc.
> 
> A device is just something with a struct udevice, so I don't see the
> random number generator as anything different from another device. We
> might have a white-noise generator which produces random numbers. Just
> because the feature is packaged inside a single chip doesn't make it
> any less a device. Just like the PMIC.
> 
> >
> > >
> > > Finally, I know you keep saying that random numbers are only needed in
> > > U-Boot proper, but if I want a random number in SPL, it may not work,
> > > since device_bind() is often not available, for code-size reasons.
> >
> > And the entire tpm framework will fit?
> 
> Yes. For verified boot it has to, since you cannot init RAM until you
> have selected your A/B/recovery image.

If so I still don't see why this needs a DT node.  If, as you say, the TPM
framework fits in then you just call tpm_get_random().  Getting the RNG is
a series of commands to the TPM.  Sou you'll need the TPM lib in SPL
regardless. 

> 
> >
> > >
> > > So that is why I say that what you are doing is quite strange. Perhaps
> > > you are coming from a different project, which does things
> > > differently.
> >
> > I don't personally find it strange.  The device is already described
> > in the DTS and I don't see a strong reason to deviate for the upstream
> > version again.
> 
> Linux tends to rely a lot more on manually adding devices. It can have
> a pretty dramatic bloating effect on code size in U-Boot.
> 
> Anyway, so long as we can detect an existing device, as I explained
> below, it is fine to manually add it when it is missing.
> 

You can detect a TPM.  Once you add that tpm then you *know* you can get an
RNG


[...]

Regards
/Ilias