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

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

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

Cheers
/Ilias
> 
> Regards,
> Siomn