lkml.org 
[lkml]   [2023]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [Patch V6 1/3] spi: Add TPM HW flow flag
    Date
    > -----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

    \
     
     \ /
      Last update: 2023-03-27 00:40    [W:2.407 / U:0.120 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site