| Message ID | 20230227172108.8206-2-kyarlagadda@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Tegra TPM driver with HW flow control | expand |
On Mon, Feb 27, 2023 at 10:51:06PM +0530, Krishna Yarlagadda wrote: > TPM spec defines flow control over SPI. Client device can insert a wait Maybe add a reference to where in the TPM specification this can be found? It looks like the specifications are publicly available, though I'm less sure about stability of the links, so perhaps it's enough to name the document and section that this can be found in. QEMU seems to be using this link to point to the specification, which I suppose has a good chance of remaining stable: https://trustedcomputinggroup.org/resource/pc-client-work-group-pc-client-specific-tpm-interface-specification-tis/ It looks like the latest version is 1.3 revision 27 and the details of this flow control mechanism are in section "6.4.5. Flow Control". > state on MISO when address is trasmitted by controller on MOSI. It can "transmitted" > work only on full duplex. > Half duplex controllers need to implement flow control in HW. This is a bit confusing because you first say it will only work for full duplex controllers and then you say it's also possible for half-duplex controllers. Maybe reword this to something like: Detecting the wait state in software is only possible for full duplex controllers. For controllers that support only half- duplex, the wait state detection needs to be implemented in hardware. > Add a flag for TPM to indicate flow control is expected in controller. That's not exactly what the flag indicates, though, is it? It primarily indicates that the device uses TPM flow control. It's then up to the controller to configure itself accordingly (i.e. if it supports half- duplex, enable detection of the wait state, otherwise leave it up to the client driver to detect the wait state). > > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com> > --- > include/linux/spi/spi.h | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > index 4fa26b9a3572..6b32c90e9e20 100644 > --- a/include/linux/spi/spi.h > +++ b/include/linux/spi/spi.h > @@ -184,8 +184,9 @@ struct spi_device { > u8 chip_select; > u8 bits_per_word; > bool rt; > -#define SPI_NO_TX BIT(31) /* No transmit wire */ > -#define SPI_NO_RX BIT(30) /* No receive wire */ > +#define SPI_NO_TX BIT(31) /* No transmit wire */ > +#define SPI_NO_RX BIT(30) /* No receive wire */ > +#define SPI_TPM_HW_FLOW BIT(29) /* TPM flow control */ Maybe some (or all?) of the information in the commit message should be duplicated here? That way people wouldn't need to go look for the commit message in order to find out. Given what I said above about the flag, it may be better to name this SPI_TPM_FLOW_CONTROL, but I suppose what you have here is fine, too. Thierry
> -----Original Message----- > From: Thierry Reding <thierry.reding@gmail.com> > Sent: 01 March 2023 19:04 > To: Krishna Yarlagadda <kyarlagadda@nvidia.com> > Cc: robh+dt@kernel.org; broonie@kernel.org; peterhuewe@gmx.de; > jgg@ziepe.ca; jarkko@kernel.org; krzysztof.kozlowski+dt@linaro.org; linux- > spi@vger.kernel.org; linux-tegra@vger.kernel.org; linux- > integrity@vger.kernel.org; linux-kernel@vger.kernel.org; Jonathan Hunter > <jonathanh@nvidia.com>; Sowjanya Komatineni > <skomatineni@nvidia.com>; Laxman Dewangan <ldewangan@nvidia.com> > Subject: Re: [Patch V6 1/3] spi: Add TPM HW flow flag > > On Mon, Feb 27, 2023 at 10:51:06PM +0530, Krishna Yarlagadda wrote: > > TPM spec defines flow control over SPI. Client device can insert a wait > > Maybe add a reference to where in the TPM specification this can be > found? It looks like the specifications are publicly available, though > I'm less sure about stability of the links, so perhaps it's enough to > name the document and section that this can be found in. QEMU seems to > be using this link to point to the specification, which I suppose has a > good chance of remaining stable: > > https://trustedcomputinggroup.org/resource/pc-client-work-group- > pc-client-specific-tpm-interface-specification-tis/ > > It looks like the latest version is 1.3 revision 27 and the details of > this flow control mechanism are in section "6.4.5. Flow Control". > > > state on MISO when address is trasmitted by controller on MOSI. It can > > "transmitted" > > > work only on full duplex. > > Half duplex controllers need to implement flow control in HW. > > This is a bit confusing because you first say it will only work for full > duplex controllers and then you say it's also possible for half-duplex > controllers. > > Maybe reword this to something like: > > Detecting the wait state in software is only possible for full > duplex controllers. For controllers that support only half- > duplex, the wait state detection needs to be implemented in > hardware. > > > Add a flag for TPM to indicate flow control is expected in controller. > > That's not exactly what the flag indicates, though, is it? It primarily > indicates that the device uses TPM flow control. It's then up to the > controller to configure itself accordingly (i.e. if it supports half- > duplex, enable detection of the wait state, otherwise leave it up to the > client driver to detect the wait state). Flag is enabled only if controller is half-duplex and HW flow control is needed. Will change wording to say it is enabled for HW flow control. > > > > > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com> > > --- > > include/linux/spi/spi.h | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > > index 4fa26b9a3572..6b32c90e9e20 100644 > > --- a/include/linux/spi/spi.h > > +++ b/include/linux/spi/spi.h > > @@ -184,8 +184,9 @@ struct spi_device { > > u8 chip_select; > > u8 bits_per_word; > > bool rt; > > -#define SPI_NO_TX BIT(31) /* No transmit wire */ > > -#define SPI_NO_RX BIT(30) /* No receive wire */ > > +#define SPI_NO_TX BIT(31) /* No transmit wire */ > > +#define SPI_NO_RX BIT(30) /* No receive wire */ > > +#define SPI_TPM_HW_FLOW BIT(29) /* TPM flow > control */ > > Maybe some (or all?) of the information in the commit message should be > duplicated here? That way people wouldn't need to go look for the commit > message in order to find out. > > Given what I said above about the flag, it may be better to name this > SPI_TPM_FLOW_CONTROL, but I suppose what you have here is fine, too. > > Thierry
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 4fa26b9a3572..6b32c90e9e20 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -184,8 +184,9 @@ struct spi_device { u8 chip_select; u8 bits_per_word; bool rt; -#define SPI_NO_TX BIT(31) /* No transmit wire */ -#define SPI_NO_RX BIT(30) /* No receive wire */ +#define SPI_NO_TX BIT(31) /* No transmit wire */ +#define SPI_NO_RX BIT(30) /* No receive wire */ +#define SPI_TPM_HW_FLOW BIT(29) /* TPM flow control */ /* * All bits defined above should be covered by SPI_MODE_KERNEL_MASK. * The SPI_MODE_KERNEL_MASK has the SPI_MODE_USER_MASK counterpart, @@ -195,7 +196,7 @@ struct spi_device { * These bits must not overlap. A static assert check should make sure of that. * If adding extra bits, make sure to decrease the bit index below as well. */ -#define SPI_MODE_KERNEL_MASK (~(BIT(30) - 1)) +#define SPI_MODE_KERNEL_MASK (~(BIT(29) - 1)) u32 mode; int irq; void *controller_state;
TPM spec defines flow control over SPI. Client device can insert a wait state on MISO when address is trasmitted by controller on MOSI. It can work only on full duplex. Half duplex controllers need to implement flow control in HW. Add a flag for TPM to indicate flow control is expected in controller. Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com> --- include/linux/spi/spi.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)