Re: [PATCH 3/3] Add support for TPM devices over I2C bus

[Ninad Palsule]

On 3/22/23 8:04 AM, Stefan Berger wrote:
> On 3/22/23 07:50, Stefan Berger wrote:
> > On 3/22/23 07:28, Ninad Palsule wrote:
> > > On 3/21/23 8:30 PM, Stefan Berger wrote:
> > > >
>
> > > > I think there should be tpm_tis_set_data_buffer function that you 
> > > > can call rather than transferring the data byte-by-byte.
> > > >
> > > > Thanks for the series!
> > > >
> > > >   Stefan
> > >
> > > I thought about it but the FIFO case performs multiple operations 
> > > hence I did not want to change it. Currently there is no function to 
> > > set data buffer in the common code.
> >
> > It may not be correct to transfer it in one go, either. I just 
> > printed the I2C specs and I am going to look at them now.
> > When one writes TPM command data to the TIS the STS register has its 
> > TPM_TIS_STS_VALID bit set and TPM_TIS_STS_EXPECT bit reset once the 
> > command is complete. This would imply that you should not have a 
> > holding area for the command bytes but pass them on to the TIS 
> > immediately to get the effect of the STS register...
>
> Regarding the registers defined for the I2C: You can pass the data 
> onto the TIS but you should mask out input flags that are not defined 
> for I2C and if the return value has flags not defined for I2C you 
> should also mask those out as well. This applies to the TPM_INT_ENABLE 
> & TPM_STS registers on read and write and to the TPM_INT_CAPABILITY on 
> read. Also you should implement support for 
> TPM_I2C_INTERACE_CAPABILITY on the I2C layer and return sensible 
> values for the defined bits. The TPM_I2C_DEVICE_ADDRESS register 
> should be handled probably assuming fixed address support only.
Good catch.

- Added capability conversion for TPM_I2C_INTERFACE_CAPABILITY.

- Added clearing of bits in TPM_STS register.

- Adde check to reject TPM_I2C_DEVICE_ADDRESS register.

- No changes are required for TPM_INT_ENABLE and TPM_INT_CAPABILITY as 
they have same bits between TPM TIS and TPM I2C.


> Ideally there would be a test case similar to this one here 
> https://github.com/qemu/qemu/blob/master/tests/qtest/tpm-tis-util.c . 
> However, I am not sure how easy it is to talk to I2C without a driver 
> for it.
Ok, Thanks.
>   Stefan


Thanks for the review!

Ninad Palsule