From:   Krishna Yarlagadda <kyarlagadda@nvidia.com>
To:     Thierry Reding <thierry.reding@gmail.com>
CC:     "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "broonie@kernel.org" <broonie@kernel.org>,
        "peterhuewe@gmx.de" <peterhuewe@gmx.de>,
        "jgg@ziepe.ca" <jgg@ziepe.ca>,
        "jarkko@kernel.org" <jarkko@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
        "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
        "linux-integrity@vger.kernel.org" <linux-integrity@vger.kernel.org>,
        "linux-kernel@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
Thread-Topic: [Patch V6 1/3] spi: Add TPM HW flow flag
Thread-Index: AQHZSs/1XgRJnlUcCE+IdtrfY6Yn4a7l72OAgAAev4A=
Date:   Wed, 1 Mar 2023 15:27:16 +0000
Message-ID: <DM4PR12MB57698120C06D056E79C077F5C3AD9@DM4PR12MB5769.namprd12.prod.outlook.com>
References: <20230227172108.8206-1-kyarlagadda@nvidia.com>
 <20230227172108.8206-2-kyarlagadda@nvidia.com> <Y/9UU8UA5R9eJFuZ@orome>
In-Reply-To: <Y/9UU8UA5R9eJFuZ@orome>
Accept-Language: en-IN, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?R3EH8cJ3XbnN1FPi16tCT/YtxsQTh853asysqbkCaw/1WtDfmdOQmF7i8PmG?=
 =?us-ascii?Q?Fa/4PfUy59pZGa1rQE+znGdUT9kKAYH3ih+5vBLR71svch3HUFL8oj3iVxPD?=
 =?us-ascii?Q?co3Plm6Aj7pB1PAAnoUpIeaIgdHsqJj86kcvdC77zS4Bb3OC81FauZQ7kYTG?=
 =?us-ascii?Q?PlygrbYN5d9Nvr8ZMjDfR06XbWTY3+4ga3lD9nsm3KXpvDycCeTvd8bDxIOS?=
 =?us-ascii?Q?UDeMHfgqAHlS5NtyiYzO/r+WxR0YLxX77eEzHPJ7V8198Qvze1JRpgH4XoR3?=
 =?us-ascii?Q?kh0gYIcNZ1TnQLshLcPr/TO6DrP9RyVTQcr0X8nv/u39G8WVcAbh3Z4C0+L1?=
 =?us-ascii?Q?ZzQVbCbHd0UNyi45MFKdoN4bT6vHxckFvGBSXmL4VrSTqkRFdQMPRpRYJR6Z?=
 =?us-ascii?Q?CAnbzKVbVTwW07LVEqZiy+Pf1X5cOEJXobVyvCWYSGm1fEK84wOoACYpZwn0?=
 =?us-ascii?Q?sgYzjQ+MEbRybtXEmjhGwFH0eYtZJjMBfYmQhHaqXqk3IYYjiuepxpDSeC4q?=
 =?us-ascii?Q?pSdY215iZidjyeSCSOnqDdYQPszD3Fzr6kkUakDg/Y+hmHmTcm9jIFZkaNGL?=
 =?us-ascii?Q?vbi042ka0/VQj87P6pvFxWgo8M86nEuXjK9ZCdHfbjN5GmCXfpXDSkqHj405?=
 =?us-ascii?Q?j+g2yA/0ATBPyS2jty8Q0WErPLClhhEcEQi/XZDmFAgj3kNJO4M58EjetARk?=
 =?us-ascii?Q?6RSrKPhCrI5MKSEKkhTNift9aZittD23pekwIZUsnxthnMhfVFaDMn4m1OS4?=
 =?us-ascii?Q?e1KLBpsRdGSkhMMazDwXN8TFUe2SPZzAqV+kC+6G95FPQIDfXmPqjfSy3a7l?=
 =?us-ascii?Q?T96ZbycwZklWbHgkmZJHiD7crCiZ6HyPbpWNdmf8kUYB9uqe3F7U1W4BrGzB?=
 =?us-ascii?Q?oPe3Jjxb7O1F87gqplYuk26/CGiRcsFyIZapMXB6Ab3nILSesAM8iFgJJP2N?=
 =?us-ascii?Q?ESDkMHK1j14hKtxoWFpseIAI1DLAwpLaBl/mWZw1/8wwnPHK1rvgV1P5wdS2?=
 =?us-ascii?Q?KP1poCFr9O1XyK5lrSvQSf3sAc0NFtHxBeIZBC7UmN2PPFJlOsTPnYj0XmWK?=
 =?us-ascii?Q?rjdek979FKy3vic49ZzNqbYIdziHiWHLRoFcw7qju3fFUzAHBzt0T1VeVqNm?=
 =?us-ascii?Q?URx6l2HSsKlsSIUK4aqIHyRVuNL340h5Bj6P8NNTxrrM4bTl7mhHMNQC87GO?=
 =?us-ascii?Q?IQtrM8mXyjcyH+iHwVWN3ta0evhvTyJS5cwIDVwTGS7ayLMEFt4N18Nj7LaL?=
 =?us-ascii?Q?zMlJ+ubT/7iFhvMx2wOGVtVjGNH+P+HQxhslxmfAQpPra46gW43wXgLBqFbH?=
 =?us-ascii?Q?JqCxSsa/v9u2hzOYGF+ETfgLRtH5pQx+R3bGn+HyP/WCA7L8vZFS1l/ABw50?=
 =?us-ascii?Q?CuXm/0MbN+DyJeBONxGFXOKcxYPg9ZHKWx0CQZiO384sJ56fOn08n4hBnj1d?=
 =?us-ascii?Q?SYrcNjRbnmrifeBb74Pce9G+ctxvRvh5l8QpUcxceanKTBLIyrNfdEuNVObT?=
 =?us-ascii?Q?l9b4uKSOjvXWeDwaYIG5Zq1YrJegWJ9ybGrdzP1RW1VxWgeae1pTEdtXE66e?=
 =?us-ascii?Q?gLQbRCvYW0pDxt/bM7PNFkg+5ZDjuYTMKp+gbIMU?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5769.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 10b6c38e-0661-4302-d610-08db1a697076
X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Mar 2023 15:27:16.3625
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Dw0DceNlJ1gjZyx8YLGjxFWM4vovkN5j+KYNOGLxGRIxBBw0U5tEinkx5nGgwuxf+015hEVIOBMOuHAi1EiS9Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR12MB6334
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----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
>=20
> 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
>=20
> 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:
>=20
> 	https://trustedcomputinggroup.org/resource/pc-client-work-group-
> pc-client-specific-tpm-interface-specification-tis/
>=20
> 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".
>=20
> > state on MISO when address is trasmitted by controller on MOSI. It can
>=20
> "transmitted"
>=20
> > work only on full duplex.
> > Half duplex controllers need to implement flow control in HW.
>=20
> 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.
>=20
> Maybe reword this to something like:
>=20
> 	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.
>=20
> > Add a flag for TPM to indicate flow control is expected in controller.
>=20
> 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.
>=20
> >
> > 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 */
>=20
> 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.
>=20
> 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.
>=20
> Thierry