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

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

> > > >         TPM_STS_SELF_TEST_DONE          = 1 << 2,

[...]

> > > >         TPM_STS_RESPONSE_RETRY          = 1 << 1,
> > > > +       TPM_STS_READ_ZERO               = 0x23
> > >
> > > Does this below in another patch?
> > >
> >
> > It's a general tpm2 update. I can move it to the driver patch if it makes
> > more sense.
> >
> > > >  };
> > > >
> > > >  enum {
> > > > --
> > > > 2.32.0.rc0
> > > >
> > >
> > > I feel that this API could be useful in reducing code duplication, but
> > > in fact it has just created more, so far as I can see from this series
> > > :-) So I think you should convert at least one driver to show its
> > > value (and not make things any worse).
> >
> > The mmio tpm driver uses it and instead of ~700 lines (like the tpmv2 spi
> > driver) it drops down to ~100.  I don't have access to any other TPM
> > hardware to rewrite any of those.
> 
> 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.

> 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. 
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).

I am happy to work with you on the cr50 i2c driver if that would help.

Regards
/Ilias

> 
> Regards,
> Simon