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

[Rob Herring]

On Tue, Jul 12, 2022 at 5:04 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Ilias,
>
> On Fri, 8 Jul 2022 at 02:24, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > [...]
> >
> > > > +
> > > >  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.25.1
> > > >
> > >
> > > The driver needs a compatible string so it can be in the device tree.
> >
> > Why?  I've tried to hint this on the previous iteration of the patch.
> > The RNG here is not a *device*.  The TPM is the device and you are
> > guaranteed to have an RNG.  The way to get a random number is send a
> > special command to the TPM. So all that we should do here is leverage
> > the fact that the TPM is already in the device tree.
> >
> > And fwiw we should stick to try to stick to what the DT spec defines
> > as much as possible.  I personally don't see this as a special usecase
> > were deviating from the spec is justified.
>
> This is not a deviation from a spec. What spec? Also, I don't want to
> get into another discussion about what a device is. We can disagree on
> that if you like.
>
> One reason is that U-Boot generally requires compatible strings, e.g.
> with of-platdata. But also we can refer to the rand device from
> elsewhere in the tree. I know that Linux does lots of ad-hoc device
> creation and doesn't really have the concept of a uclass consistently
> applied, but this is U-Boot.

You are letting client/OS details define your binding. Doing so
doesn't result in OS agnostic bindings. Sure, it would be nice if DT
nodes and drivers were always a nice clean 1:1 ratio, but h/w is messy
sometimes and DT is not an abstraction layer. The general guidance on
whether there are child nodes for sub-blocks is do they have their own
resources in DT or are the sub-blocks reusable on multiple devices.
Neither of those are the case here.

Besides, we already have TPM device bindings defined. Requiring
binding changes when adding a new client/OS feature is not good
practice either.

Rob