On 18.07.2019 20:11, Stephen Boyd wrote:
Quoting Alexander Steffen (2019-07-18 09:47:22)
On 17.07.2019 23:38, Stephen Boyd wrote:
Quoting Stephen Boyd (2019-07-17 12:57:34)
Quoting Alexander Steffen (2019-07-17 05:00:06)
Can't the code be shared more explicitly, e.g. by cr50_spi wrapping
tpm_tis_spi, so that it can intercept the calls, execute the additional
actions (like waking up the device), but then let tpm_tis_spi do the
common work?
I suppose the read{16,32} and write32 functions could be reused. I'm not
sure how great it will be if we combine these two drivers, but I can
give it a try today and see how it looks.
Here's the patch. I haven't tested it besides compile testing.
The code seems to work but I haven't done any extensive testing besides
making sure that the TPM responds to pcr reads and some commands like
reading random numbers.
Thanks for providing this. Makes it much easier to see what the actual
differences between the devices are.
Do we have a general policy on how to support devices that are very
similar but need special handling in some places? Not duplicating the
whole driver just to change a few things definitely seems like an
improvement (and has already been done in the past, as with
TPM_TIS_ITPM_WORKAROUND). But should all the code just be added to
tpm_tis_spi.c? Or is there some way to keep a clearer separation,
especially when (in the future) we have multiple devices that all have
their own set of deviations from the spec?
If you have any ideas on how to do it please let me know. At this point,
I'd prefer if the maintainers could provide direction on what they want.
Sure, I'd expect Jarkko will say something once he's back from vacation.
Alexander
