Re: [PATCH v4] tee: optee: rework TA bus scanning code

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Simon,

> > > > > >
> > > > > > Late versions of OP-TEE support a pseudo bus.  TAs that behave as
> > > > > > hardware blocks (e.g TPM, RNG etc) present themselves on a bus which we can
> > > > > > scan.  Unfortunately U-Boot doesn't support that yet. It's worth noting
> > > > > > that we already have a workaround for RNG.  The details are in
> > > > > > commit 70812bb83da6 ("tee: optee: bind rng optee driver")
> > > > > >
> > > > > > So let's add a list of devices based on U-Boot Kconfig options that we will
> > > > > > scan until we properly implement the tee-bus functionality.
> > > > > >
> > > > > > While at it change the behaviour of the tee core itself wrt to device
> > > > > > binding.  If some device binding fails, print a warning instead of
> > > > > > disabling OP-TEE.
> > > > > >
> > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > > Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > > > > Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
> > > > > > ---
> > > > > > Changes since v3:
> > > > > > - Use NULL instead of a child ptr on device_bind_driver(), since it's not
> > > > > > really needed
> > > > > > - Changed the style of the optee_bus_probe[] definition to
> > > > > >   {.drv_name = xxx, .dev_name = yyy }
> > > > > >
> > > > > > Changes since v2:
> > > > > > - Fixed typo on driver name ftpm-tee -> ftpm_tee
> > > > > >
> > > > > > Changes since v1:
> > > > > > - remove a macro and use ARRAY_SIZE directly
> > > > > >  drivers/tee/optee/core.c | 24 +++++++++++++++++++-----
> > > > > >  1 file changed, 19 insertions(+), 5 deletions(-)
> > > > > >

[...]

> >
> > Some things *are* working without a DT entry.  You had similar
> > concerns on FF-A (where you requested a DT node again) and people gave
> > the exact same response.  As long as a bus is scanable in any way,
> > it's preferable to than adding a DT entry.  Moreover this code does
> > not prevent anyone from adding a DT entry.
> >
> > To make things even worse if the TA is compiled as 'scanable' and has
> > a DT entry, it might cause issues down the road when being probed by
> > the kernel.  So really this is just a patch that makes u-boot behave
> > and plug in properly to the rest of the ecosystem
> 
> Calling device_bind() is supposed to be used in extremis. I don't see
> any scanning of an OP-TEE bus here. I just see it binding two child
> devices which are hard-coded in U-Boot. What am I missing?

The commit description describes the current state of U-Boot

> 
> This appears to be a Linaro binding, so you should be able to update
> it easily enough.

Linaro binding?  The DT is governed by a spec, we commit everything
upstream there.  OTOH I still don't see what we need to put in there.
As we discussed this is a bus that can be used to scan devices.


Thanks
/Ilias
> 
> Regards,
> Simon