> -----Original Message----- > From: Thierry Reding <thierry.reding@xxxxxxxxx> > Sent: 01 March 2023 19:04 > To: Krishna Yarlagadda <kyarlagadda@xxxxxxxxxx> > Cc: robh+dt@xxxxxxxxxx; broonie@xxxxxxxxxx; peterhuewe@xxxxxx; > jgg@xxxxxxxx; jarkko@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; linux- > spi@xxxxxxxxxxxxxxx; linux-tegra@xxxxxxxxxxxxxxx; linux- > integrity@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Jonathan Hunter > <jonathanh@xxxxxxxxxx>; Sowjanya Komatineni > <skomatineni@xxxxxxxxxx>; Laxman Dewangan <ldewangan@xxxxxxxxxx> > 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@xxxxxxxxxx> > > --- > > 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
