Hi Ilias,
On Mon, 12 Jul 2021 at 23:51, Ilias Apalodimas ilias.apalodimas@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