[PATCH 1/2 v2] tpm2: Introduce TIS tpm core

[Simon Glass]

Hi Ilias,

On Mon, 12 Jul 2021 at 23:51, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
> > > >
>
> [...]
>
> > > > Yes, but I hope you see my point, that you have added a new interface.
> > > > It is definitely better than adding a new driver and duplicating all
> > > > the code, but it is still one more copy and in fact, the code is
> > > > duplicated.
> > > >
> > >
> > > I get the point but I don't exactly agree here.  It's not duplicated code.
> > > We need to plug in the mmio driver.  The original code was just doing what
> > > every TPM does.  It carried the TIS relevant code in the new driver.
> > > The new approach defines an API for everyone to use and the new driver uses it.
> > > So I don't see the duplication here.  You need the TIS code one way or the
> > > other.  Now it's on a common interface for everyone to use.
> >
> > Well how about converting a TPM blindly and then we'll find someone to test it?
>
> Let's do the cr50
>
> >
> > >
> > > > Can you get access to TPM hardware? I see that you have offered to be
> > > > the maintainer for this subsystem, so I think that would be useful.
> > > > Can sandbox use your new API?
> > >
> > > It depends, is the sandbox TIS compatible? If it is sure we could use it.
> >
> > At present sandbox implements the tpm_ops API. So if we did that we
> > would need to tear it apart to insert this new API as well.
>
> Ok then that might make too much sense for the sandbox.
>
> >
> > > I offered to maintain the drivers because I wrote the API and I have an
> > > idea of how TPMs should work.  If that means I'll have to go and get every
> > > hardware we support, I'll just volunteer into maintaining the TIS layer.
> > > Moreover I dont see why I should start porting drivers to use that API.
> > > People decided to duplicate that code in every driver (in fact multiple times).
> >
> > See https://xkcd.com/927/ :-)
> >
>
> Yea I don't disagree with that.  That's one of the points of adding myself
> as a maintainer for the entire tpm/drivers/*.  I can just reply 'no thanks'
> at least for new drivers that don't use it.  But frankly I don't see why,
> adding a new drivers, while using the TIS API boils down to a few lines of
> code defining the bus accesses
>
> > >
> > > I am happy to work with you on the cr50 i2c driver if that would help.
> >
> > Sure that might be easier as I can definitely test it.
>
> Ok, let me have a look at that, I still think the patch should go in
> regardless though.  We can always send a follow up for cr50 once we are
> done testing

Well if you set it up as an MMIO uclass with a sandbox test then I
think there is enough value to this approach and we avoid the xkcd
issue.

Regards,
Simon