From:   Krishna Yarlagadda <kyarlagadda@nvidia.com>
To:     Mark Brown <broonie@kernel.org>
CC:     "robh+dt@kernel.org" <robh+dt@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>,
        "thierry.reding@gmail.com" <thierry.reding@gmail.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Sowjanya Komatineni <skomatineni@nvidia.com>,
        Laxman Dewangan <ldewangan@nvidia.com>
Subject: RE: [Patch V3 1/3] tpm_tis-spi: Support hardware wait polling
Thread-Topic: [Patch V3 1/3] tpm_tis-spi: Support hardware wait polling
Thread-Index: AQHZR6OqOBQhj0zBTUWL/FYm6RTrP67cxooAgAAUpuA=
Date:   Thu, 23 Feb 2023 18:41:43 +0000
Message-ID: <DM4PR12MB57695DE127286D1DA436E804C3AB9@DM4PR12MB5769.namprd12.prod.outlook.com>
References: <20230223162635.19747-1-kyarlagadda@nvidia.com>
 <20230223162635.19747-2-kyarlagadda@nvidia.com>
 <Y/egACRAp6nKZWdN@sirena.org.uk>
In-Reply-To: <Y/egACRAp6nKZWdN@sirena.org.uk>
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?O02mfb0f6iy2hlJDqVfla/rVspQJEmkscU6eT5+sGISiMd5Huk7A7J5hB0CM?=
 =?us-ascii?Q?sF7AJ4LgZT110Yjg9s9poFY6ZMEJlUrOsxVTVmnllTA/IxQpuGhmPu5aSr8m?=
 =?us-ascii?Q?tllaUJtKrEXmQ9HQKAYNJPfNqMW+auwPdic9aGiGCBQshY2PXAEliBZ6Q96k?=
 =?us-ascii?Q?0XYenqzrn1ASKCHnEdDKKCubkoYb0ybXEGxuoBUlX4EbLqr4c2rV/K3TSyy+?=
 =?us-ascii?Q?p5ax+uJOK/S0hDkdy1jC32Z92szGqSvPyBrNe/1TQCQgqGnf70E7Hxbtw+t2?=
 =?us-ascii?Q?frQcDsQ3gktGA7VjTTFGxngC80Fy0XB2O9If/hQoDlw/vTql7xLnsTlcbDeP?=
 =?us-ascii?Q?Ed5lbE/4Aanro9yLzXNnuqkcukFGPT5i31gSgWKlDYcTUe67h7NckT+MISNK?=
 =?us-ascii?Q?Nf+UOMdfVlyscHGjlPNbvITgZV/rPx4//53lTS7xV+ukS91QW1PSFw18yGAL?=
 =?us-ascii?Q?sJQs/dJaB43bJJlilhY7fJ/kGRUDGaxezj5OahnDATRGyenZCbvSDe4G/2mN?=
 =?us-ascii?Q?pw7juX27S2j/QjCIYaURMmrUpdSxaMPUDSX9Xm58NdWW/K1PoSdbiuSlGraj?=
 =?us-ascii?Q?vU4c/a1bpOZKxf//PtCB8AOGLQEJWHXEhHw4N1MalKq9d6d5Sb1kpHQ+Rqu2?=
 =?us-ascii?Q?9upYw6DOq7CC3G7L5LKWS77S7534JKqMr8+yDabw3vPHHnMltRugiHog05uH?=
 =?us-ascii?Q?SoELoCptMglrxMm9T9SynpeI4bpMc++VdoyeLxfznOOcrLa8qPKr9NcaYD2s?=
 =?us-ascii?Q?YwR5aCUGCi4cJKLA2wdPn1qMgRNxYtpqPzxm9mECrhd4qUD25ufEZEor7DfC?=
 =?us-ascii?Q?9sbIi9esYvPbCHooz9kCKpns16Wl1fAFzu5M3P0jQCvISflhpM28xGXlc10m?=
 =?us-ascii?Q?i4Jbh+Dgg3+Ootg0CI4ueMZG+eKn0W0gXusw6UTr0QGVhn23Qtrf2VpcfIAR?=
 =?us-ascii?Q?m2F2LRFT/m7qJ+mosblx8Zr4tZ4m/PuJ2n+SeOs1Ks/DahkVWvmkMe3oc6qX?=
 =?us-ascii?Q?dEei6KHDqtBEX1Y4T6WieWWTVGr8eG8/X0oFuWYVyWHW1ZooqCClfIHB8kCi?=
 =?us-ascii?Q?J6JjQzahGPTOVcggYf6uLwbYymwAXAWygq9xP+YNlaMIH1CZcjzF42rit8tK?=
 =?us-ascii?Q?ovzy6xqHJH5OiwXLTYolB/ZIkC2+kjUjLPXfGVw2AdPwjesmopegghcXwQbq?=
 =?us-ascii?Q?eGZFoDVuJSWAmFfYwAyq2AoW+qeFkigQGIiOgRGHtYkjTiDBZ3nWpTe3FSEG?=
 =?us-ascii?Q?CwSn79peW4FFssMjVVXEgl54QXcRikWaUQF/V820iH59jvaPESs4mP4tbgAR?=
 =?us-ascii?Q?0C9Cu3vowFu+GUUNO9hMeFY9oeUpQ7e9pOJ3kZoVHPmwMI4aSc4aHiQAin8+?=
 =?us-ascii?Q?OaazXeF1wkF2Hr59ASJpeJs1i2VHI8S7HkWlp7JEM3isaLQZzvIXkxZJP34w?=
 =?us-ascii?Q?X5qA1LUSYq58ip5oq4iQi7/xi9XpkonmRlfo7ZEZFnKwkaqTqHXu6WT/1kTa?=
 =?us-ascii?Q?RNx6RH64UgIigLpz0QyR/3FOPrL7busfPASZ+99T6DcM6GOXweFPM6Rzqxlk?=
 =?us-ascii?Q?206qRyCFnR/B2OKaf0ioxW+hhRPgl88YSuTppg2D?=
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: ebb17f6a-29df-4260-0208-08db15cd9bff
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Feb 2023 18:41:43.2405
 (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: aW5KKc8Mrtv3TmdGXLJ1AgXZJsII1ek4mgNN986fDsJSsbqRzLnjxy5acrm9eQsyxXi9fJgQcCTz9fjHUgguoA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR12MB7033
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: 23 February 2023 22:49
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: robh+dt@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;
> thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>;
> Sowjanya Komatineni <skomatineni@nvidia.com>; Laxman Dewangan
> <ldewangan@nvidia.com>
> Subject: Re: [Patch V3 1/3] tpm_tis-spi: Support hardware wait polling
>=20
> On Thu, Feb 23, 2023 at 09:56:33PM +0530, Krishna Yarlagadda wrote:
>=20
> > +       spi_bus_lock(phy->spi_device->master);
> > +
> > +       while (len) {
>=20
> Why?
TPM supports max 64B in single transaction. Loop to send rest of it.
>=20
> > +		spi_xfer[0].tx_buf =3D phy->iobuf;
> > +		spi_xfer[0].len =3D 1;
> > +		spi_message_add_tail(&spi_xfer[0], &m);
> > +
> > +		spi_xfer[1].tx_buf =3D phy->iobuf + 1;
> > +		spi_xfer[1].len =3D 3;
> > +		spi_message_add_tail(&spi_xfer[1], &m);
>=20
> Why would we make these two separate transfers?
Tegra QSPI combined sequence requires cmd, addr and data in different
transfers. This eliminates need for additional flag for combined sequence.
>=20
> > +		if (out) {
> > +			spi_xfer[2].tx_buf =3D &phy->iobuf[4];
> > +			spi_xfer[2].rx_buf =3D NULL;
> > +			memcpy(&phy->iobuf[4], out, transfer_len);
> > +			out +=3D transfer_len;
> > +		}
> > +
> > +		if (in) {
> > +			spi_xfer[2].tx_buf =3D NULL;
> > +			spi_xfer[2].rx_buf =3D &phy->iobuf[4];
> > +		}
>=20
> This will use the same buffer for rx and tx if some bug manages to leave
> them both set.  That shouldn't be an issue but it's an alarm bell
> reading the code.
>=20
> > index 988aabc31871..b88494e31239 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
> Additions to the SPI API should be a separate commit for SPI rather than
> merged into a driver change.
Will split this into new patch.