[Patch V10 0/3] Tegra TPM driver with HW flow control

[Patch V10 0/3] Tegra TPM driver with HW flow control

[Krishna Yarlagadda]

TPM devices may insert wait state on last clock cycle of ADDR phase.
For SPI controllers that support full-duplex transfers, this can be
detected using software by reading the MISO line. For SPI controllers
that only support half-duplex transfers, such as the Tegra QSPI, it is
not possible to detect the wait signal from software. The QSPI
controller in Tegra234 and Tegra241 implement hardware detection of the
wait signal which can be enabled in the controller for TPM devices.

Add HW flow control in TIS driver and a flag in SPI data to indicate
wait detection is required in HW. SPI controller driver determines if
this is supported. Add HW detection in Tegra QSPI controller.

Updates in this patch set 
 - Tegra QSPI identifies itself as half duplex.
 - TPM TIS SPI driver skips flow control for half duplex and send
   transfers in single message for controller to handle it.
 - TPM device identifies as TPM device for controller to detect and
   enable HW TPM wait poll feature.

Verified with a TPM device on Tegra241 ref board using TPM2 tools.

V10
 - use spi_sync in place of spi_sync_locked
V9
 - renamed tpm spi transfer functions
V8:
 - fix compile warning.
V7:
 - updated patch description.
 - TPM flag set in probe.
 - minor comments.
V6:
 - Fix typo in chip name Tegra234.
 - Debug logs change skipped to be sent later.
 - Consistent usage of soc flag.
V5:
 - No SPI bus locking.
V4:
 - Split api change to different patch.
 - Describe TPM HW flow control.
V3:
 - Use SPI device mode flag and SPI controller flags.
 - Drop usage of device tree flags.
 - Generic TPM half duplex controller handling.
 - HW & SW flow control for TPM. Drop additional driver.
V2:
 - Fix dt schema errors.

Krishna Yarlagadda (3):
  spi: Add TPM HW flow flag
  tpm_tis-spi: Add hardware wait polling
  spi: tegra210-quad: Enable TPM wait polling

 drivers/char/tpm/tpm_tis_spi_main.c | 91 ++++++++++++++++++++++++++++-
 drivers/spi/spi-tegra210-quad.c     | 14 +++++
 include/linux/spi/spi.h             | 16 ++++-
 3 files changed, 116 insertions(+), 5 deletions(-)

-- 
2.17.1

[Patch V10 1/3] spi: Add TPM HW flow flag

[Krishna Yarlagadda]

TPM specification [1] defines flow control over SPI. Client device can
insert a wait state on MISO when address is transmitted by controller
on MOSI. 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 SPI_TPM_HW_FLOW for TPM device to set when software flow
control is not possible and hardware flow control is expected from
SPI controller.

Reference:
[1] https://trustedcomputinggroup.org/resource/pc-client-platform-tpm
-profile-ptp-specification/

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 include/linux/spi/spi.h | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 873ced6ae4ca..cfe42f8cd7a4 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -184,8 +184,18 @@ 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 */
+	/*
+	 * TPM specification defines flow control over SPI. Client device
+	 * can insert a wait state on MISO when address is transmitted by
+	 * controller on MOSI. 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. TPM devices would set this flag when hardware flow
+	 * control is expected from SPI controller.
+	 */
+#define SPI_TPM_HW_FLOW		BIT(29)		/* TPM HW 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 +205,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;
-- 
2.17.1

[Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Krishna Yarlagadda]

TPM devices may insert wait state on last clock cycle of ADDR phase.
For SPI controllers that support full-duplex transfers, this can be
detected using software by reading the MISO line. For SPI controllers
that only support half-duplex transfers, such as the Tegra QSPI, it is
not possible to detect the wait signal from software. The QSPI
controller in Tegra234 and Tegra241 implement hardware detection of the
wait signal which can be enabled in the controller for TPM devices.

The current TPM TIS driver only supports software detection of the wait
signal. To support SPI controllers that use hardware to detect the wait
signal, add the function tpm_tis_spi_hw_flow_transfer() and move the
existing code for software based detection into a function called
tpm_tis_spi_sw_flow_transfer(). SPI controllers that only support
half-duplex transfers will always call tpm_tis_spi_hw_flow_transfer()
because they cannot support software based detection. The bit
SPI_TPM_HW_FLOW is set to indicate to the SPI controller that hardware
detection is required and it is the responsibility of the SPI controller
driver to determine if this is supported or not.

For hardware flow control, CMD-ADDR-DATA messages are combined into a
single message where as for software flow control exiting method of
CMD-ADDR in a message and DATA in another is followed.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/char/tpm/tpm_tis_spi_main.c | 91 ++++++++++++++++++++++++++++-
 1 file changed, 89 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
index a0963a3e92bd..8967f179f808 100644
--- a/drivers/char/tpm/tpm_tis_spi_main.c
+++ b/drivers/char/tpm/tpm_tis_spi_main.c
@@ -71,8 +71,74 @@ static int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy,
 	return 0;
 }
 
-int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
-			 u8 *in, const u8 *out)
+/*
+ * Half duplex controller with support for TPM wait state detection like
+ * Tegra QSPI need CMD, ADDR & DATA sent in single message to manage HW flow
+ * control. Each phase sent in different transfer for controller to idenity
+ * phase.
+ */
+static int tpm_tis_spi_transfer_half(struct tpm_tis_data *data,	u32 addr,
+				     u16 len, u8 *in, const u8 *out)
+{
+	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
+	struct spi_transfer spi_xfer[3];
+	struct spi_message m;
+	u8 transfer_len;
+	int ret;
+
+	while (len) {
+		transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
+
+		spi_message_init(&m);
+		phy->iobuf[0] = (in ? 0x80 : 0) | (transfer_len - 1);
+		phy->iobuf[1] = 0xd4;
+		phy->iobuf[2] = addr >> 8;
+		phy->iobuf[3] = addr;
+
+		memset(&spi_xfer, 0, sizeof(spi_xfer));
+
+		spi_xfer[0].tx_buf = phy->iobuf;
+		spi_xfer[0].len = 1;
+		spi_message_add_tail(&spi_xfer[0], &m);
+
+		spi_xfer[1].tx_buf = phy->iobuf + 1;
+		spi_xfer[1].len = 3;
+		spi_message_add_tail(&spi_xfer[1], &m);
+
+		if (out) {
+			spi_xfer[2].tx_buf = &phy->iobuf[4];
+			spi_xfer[2].rx_buf = NULL;
+			memcpy(&phy->iobuf[4], out, transfer_len);
+			out += transfer_len;
+		}
+
+		if (in) {
+			spi_xfer[2].tx_buf = NULL;
+			spi_xfer[2].rx_buf = &phy->iobuf[4];
+		}
+
+		spi_xfer[2].len = transfer_len;
+		spi_message_add_tail(&spi_xfer[2], &m);
+
+		reinit_completion(&phy->ready);
+
+		ret = spi_sync(phy->spi_device, &m);
+		if (ret < 0)
+			return ret;
+
+		if (in) {
+			memcpy(in, &phy->iobuf[4], transfer_len);
+			in += transfer_len;
+		}
+
+		len -= transfer_len;
+	}
+
+	return ret;
+}
+
+static int tpm_tis_spi_transfer_full(struct tpm_tis_data *data, u32 addr,
+				     u16 len, u8 *in, const u8 *out)
 {
 	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
 	int ret = 0;
@@ -140,6 +206,24 @@ int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
 	return ret;
 }
 
+int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
+			 u8 *in, const u8 *out)
+{
+	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
+	struct spi_controller *ctlr = phy->spi_device->controller;
+
+	/*
+	 * TPM flow control over SPI requires full duplex support.
+	 * Send entire message to a half duplex controller to handle
+	 * wait polling in controller.
+	 * Set TPM HW flow control flag..
+	 */
+	if (ctlr->flags & SPI_CONTROLLER_HALF_DUPLEX)
+		return tpm_tis_spi_transfer_half(data, addr, len, in, out);
+	else
+		return tpm_tis_spi_transfer_full(data, addr, len, in, out);
+}
+
 static int tpm_tis_spi_read_bytes(struct tpm_tis_data *data, u32 addr,
 				  u16 len, u8 *result, enum tpm_tis_io_mode io_mode)
 {
@@ -181,6 +265,9 @@ static int tpm_tis_spi_probe(struct spi_device *dev)
 
 	phy->flow_control = tpm_tis_spi_flow_control;
 
+	if (dev->controller->flags & SPI_CONTROLLER_HALF_DUPLEX)
+		dev->mode |= SPI_TPM_HW_FLOW;
+
 	/* If the SPI device has an IRQ then use that */
 	if (dev->irq > 0)
 		irq = dev->irq;
-- 
2.17.1

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Jarkko Sakkinen]

On Fri Apr 21, 2023 at 12:13 PM EEST, Krishna Yarlagadda wrote:
> TPM devices may insert wait state on last clock cycle of ADDR phase.
> For SPI controllers that support full-duplex transfers, this can be
> detected using software by reading the MISO line. For SPI controllers
> that only support half-duplex transfers, such as the Tegra QSPI, it is
> not possible to detect the wait signal from software. The QSPI
> controller in Tegra234 and Tegra241 implement hardware detection of the
> wait signal which can be enabled in the controller for TPM devices.
>
> The current TPM TIS driver only supports software detection of the wait
> signal. To support SPI controllers that use hardware to detect the wait
> signal, add the function tpm_tis_spi_hw_flow_transfer() and move the
> existing code for software based detection into a function called
> tpm_tis_spi_sw_flow_transfer(). SPI controllers that only support
> half-duplex transfers will always call tpm_tis_spi_hw_flow_transfer()
> because they cannot support software based detection. The bit
> SPI_TPM_HW_FLOW is set to indicate to the SPI controller that hardware
> detection is required and it is the responsibility of the SPI controller
> driver to determine if this is supported or not.
>
> For hardware flow control, CMD-ADDR-DATA messages are combined into a
> single message where as for software flow control exiting method of
> CMD-ADDR in a message and DATA in another is followed.
>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/char/tpm/tpm_tis_spi_main.c | 91 ++++++++++++++++++++++++++++-
>  1 file changed, 89 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
> index a0963a3e92bd..8967f179f808 100644
> --- a/drivers/char/tpm/tpm_tis_spi_main.c
> +++ b/drivers/char/tpm/tpm_tis_spi_main.c
> @@ -71,8 +71,74 @@ static int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy,
>  	return 0;
>  }
>  
> -int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
> -			 u8 *in, const u8 *out)
> +/*
> + * Half duplex controller with support for TPM wait state detection like
> + * Tegra QSPI need CMD, ADDR & DATA sent in single message to manage HW flow
> + * control. Each phase sent in different transfer for controller to idenity
> + * phase.
> + */
> +static int tpm_tis_spi_transfer_half(struct tpm_tis_data *data,	u32 addr,
> +				     u16 len, u8 *in, const u8 *out)
> +{
> +	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
> +	struct spi_transfer spi_xfer[3];
> +	struct spi_message m;
> +	u8 transfer_len;
> +	int ret;
> +
> +	while (len) {
> +		transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
> +
> +		spi_message_init(&m);
> +		phy->iobuf[0] = (in ? 0x80 : 0) | (transfer_len - 1);
> +		phy->iobuf[1] = 0xd4;
> +		phy->iobuf[2] = addr >> 8;
> +		phy->iobuf[3] = addr;
> +
> +		memset(&spi_xfer, 0, sizeof(spi_xfer));
> +
> +		spi_xfer[0].tx_buf = phy->iobuf;
> +		spi_xfer[0].len = 1;
> +		spi_message_add_tail(&spi_xfer[0], &m);
> +
> +		spi_xfer[1].tx_buf = phy->iobuf + 1;
> +		spi_xfer[1].len = 3;
> +		spi_message_add_tail(&spi_xfer[1], &m);
> +
> +		if (out) {
> +			spi_xfer[2].tx_buf = &phy->iobuf[4];
> +			spi_xfer[2].rx_buf = NULL;
> +			memcpy(&phy->iobuf[4], out, transfer_len);
> +			out += transfer_len;
> +		}
> +
> +		if (in) {
> +			spi_xfer[2].tx_buf = NULL;
> +			spi_xfer[2].rx_buf = &phy->iobuf[4];
> +		}
> +
> +		spi_xfer[2].len = transfer_len;
> +		spi_message_add_tail(&spi_xfer[2], &m);
> +
> +		reinit_completion(&phy->ready);
> +
> +		ret = spi_sync(phy->spi_device, &m);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (in) {
> +			memcpy(in, &phy->iobuf[4], transfer_len);
> +			in += transfer_len;
> +		}
> +
> +		len -= transfer_len;
> +	}
> +
> +	return ret;
> +}
> +
> +static int tpm_tis_spi_transfer_full(struct tpm_tis_data *data, u32 addr,
> +				     u16 len, u8 *in, const u8 *out)
>  {
>  	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
>  	int ret = 0;
> @@ -140,6 +206,24 @@ int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
>  	return ret;
>  }
>  
> +int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
> +			 u8 *in, const u8 *out)
> +{
> +	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
> +	struct spi_controller *ctlr = phy->spi_device->controller;
> +
> +	/*
> +	 * TPM flow control over SPI requires full duplex support.
> +	 * Send entire message to a half duplex controller to handle
> +	 * wait polling in controller.
> +	 * Set TPM HW flow control flag..
> +	 */
> +	if (ctlr->flags & SPI_CONTROLLER_HALF_DUPLEX)
> +		return tpm_tis_spi_transfer_half(data, addr, len, in, out);
> +	else
> +		return tpm_tis_spi_transfer_full(data, addr, len, in, out);
> +}
> +
>  static int tpm_tis_spi_read_bytes(struct tpm_tis_data *data, u32 addr,
>  				  u16 len, u8 *result, enum tpm_tis_io_mode io_mode)
>  {
> @@ -181,6 +265,9 @@ static int tpm_tis_spi_probe(struct spi_device *dev)
>  
>  	phy->flow_control = tpm_tis_spi_flow_control;
>  
> +	if (dev->controller->flags & SPI_CONTROLLER_HALF_DUPLEX)
> +		dev->mode |= SPI_TPM_HW_FLOW;
> +
>  	/* If the SPI device has an IRQ then use that */
>  	if (dev->irq > 0)
>  		irq = dev->irq;
> -- 
> 2.17.1

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Should I pick these patches?

BR, Jarkko

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 184 bytes --]

On Sun, Apr 23, 2023 at 06:08:16PM +0300, Jarkko Sakkinen wrote:

> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

> Should I pick these patches?

I've queued the spi side already.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Jarkko Sakkinen]

On Mon, 2023-04-24 at 12:56 +0100, Mark Brown wrote:
> On Sun, Apr 23, 2023 at 06:08:16PM +0300, Jarkko Sakkinen wrote:
> 
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> > Should I pick these patches?
> 
> I've queued the spi side already.

OK, great, thanks.

BR, Jarkko

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]

On Mon, Apr 24, 2023 at 12:56:25PM +0100, Mark Brown wrote:
> On Sun, Apr 23, 2023 at 06:08:16PM +0300, Jarkko Sakkinen wrote:
> 
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> > Should I pick these patches?
> 
> I've queued the spi side already.

Mark,

Would it make sense for you to pick up patch 2/3 as well? As far as I
can tell there's a build dependency on patch 1/3 because of the newly
added SPI_TPM_HW_FLOW symbol.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]

On Mon, Apr 24, 2023 at 04:46:24PM +0200, Thierry Reding wrote:

> Would it make sense for you to pick up patch 2/3 as well? As far as I
> can tell there's a build dependency on patch 1/3 because of the newly
> added SPI_TPM_HW_FLOW symbol.

I'll include it in my pull request for spi this time round so it should
end up in -rc1, my thinking was that I was happy with the SPI bits and
if it was in -rc1 then the TPM bits could be handled without cross tree
issues when the review was sorted (which it is now but wasn't at the
time).  If the SPI side doesn't make -rc1 for some reason I can pick up
the TPM bit as well, and/or do a signed tag.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

On Mon, Apr 24, 2023 at 04:18:45PM +0100, Mark Brown wrote:
> On Mon, Apr 24, 2023 at 04:46:24PM +0200, Thierry Reding wrote:
> 
> > Would it make sense for you to pick up patch 2/3 as well? As far as I
> > can tell there's a build dependency on patch 1/3 because of the newly
> > added SPI_TPM_HW_FLOW symbol.
> 
> I'll include it in my pull request for spi this time round so it should
> end up in -rc1, my thinking was that I was happy with the SPI bits and
> if it was in -rc1 then the TPM bits could be handled without cross tree
> issues when the review was sorted (which it is now but wasn't at the
> time).  If the SPI side doesn't make -rc1 for some reason I can pick up
> the TPM bit as well, and/or do a signed tag.

Sounds good.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Krishna Yarlagadda]

> -----Original Message-----
> From: Thierry Reding <thierry.reding@gmail.com>
> Sent: 24 April 2023 21:02
> To: Mark Brown <broonie@kernel.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>; Krishna Yarlagadda
> <kyarlagadda@nvidia.com>; jsnitsel@redhat.com; robh+dt@kernel.org;
> peterhuewe@gmx.de; jgg@ziepe.ca; 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 V10 2/3] tpm_tis-spi: Add hardware wait polling
> 
> On Mon, Apr 24, 2023 at 04:18:45PM +0100, Mark Brown wrote:
> > On Mon, Apr 24, 2023 at 04:46:24PM +0200, Thierry Reding wrote:
> >
> > > Would it make sense for you to pick up patch 2/3 as well? As far as I
> > > can tell there's a build dependency on patch 1/3 because of the newly
> > > added SPI_TPM_HW_FLOW symbol.
> >
> > I'll include it in my pull request for spi this time round so it should
> > end up in -rc1, my thinking was that I was happy with the SPI bits and
> > if it was in -rc1 then the TPM bits could be handled without cross tree
> > issues when the review was sorted (which it is now but wasn't at the
> > time).  If the SPI side doesn't make -rc1 for some reason I can pick up
> > the TPM bit as well, and/or do a signed tag.
> 
> Sounds good.
> 
> Thanks,
> Thierry

Mark,
Now that SPI changes are in, can we pull this TPM change for rc2.
Will this be picked into SPI or TPM list?
Thanks,
KY

RE: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Krishna Yarlagadda]

> -----Original Message-----
> From: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Sent: Wednesday, May 10, 2023 8:41 PM
> To: Thierry Reding <thierry.reding@gmail.com>; Mark Brown
> <broonie@kernel.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>; jsnitsel@redhat.com;
> robh+dt@kernel.org; peterhuewe@gmx.de; jgg@ziepe.ca;
> 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 V10 2/3] tpm_tis-spi: Add hardware wait polling
> 
> 
> > -----Original Message-----
> > From: Thierry Reding <thierry.reding@gmail.com>
> > Sent: 24 April 2023 21:02
> > To: Mark Brown <broonie@kernel.org>
> > Cc: Jarkko Sakkinen <jarkko@kernel.org>; Krishna Yarlagadda
> > <kyarlagadda@nvidia.com>; jsnitsel@redhat.com; robh+dt@kernel.org;
> > peterhuewe@gmx.de; jgg@ziepe.ca; 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 V10 2/3] tpm_tis-spi: Add hardware wait polling
> >
> > On Mon, Apr 24, 2023 at 04:18:45PM +0100, Mark Brown wrote:
> > > On Mon, Apr 24, 2023 at 04:46:24PM +0200, Thierry Reding wrote:
> > >
> > > > Would it make sense for you to pick up patch 2/3 as well? As far as I
> > > > can tell there's a build dependency on patch 1/3 because of the newly
> > > > added SPI_TPM_HW_FLOW symbol.
> > >
> > > I'll include it in my pull request for spi this time round so it should
> > > end up in -rc1, my thinking was that I was happy with the SPI bits and
> > > if it was in -rc1 then the TPM bits could be handled without cross tree
> > > issues when the review was sorted (which it is now but wasn't at the
> > > time).  If the SPI side doesn't make -rc1 for some reason I can pick up
> > > the TPM bit as well, and/or do a signed tag.
> >
> > Sounds good.
> >
> > Thanks,
> > Thierry
> 
> Mark,
> Now that SPI changes are in, can we pull this TPM change for rc2.
> Will this be picked into SPI or TPM list?
Jarkko, Mark,
Can we pick this change in TPM list since SPI header changes are in.

Regards
KY
> Thanks,
> KY

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2846 bytes --]

On Wed, May 24, 2023 at 12:43:12PM +0000, Krishna Yarlagadda wrote:
> > -----Original Message-----
> > From: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > Sent: Wednesday, May 10, 2023 8:41 PM
> > To: Thierry Reding <thierry.reding@gmail.com>; Mark Brown
> > <broonie@kernel.org>
> > Cc: Jarkko Sakkinen <jarkko@kernel.org>; jsnitsel@redhat.com;
> > robh+dt@kernel.org; peterhuewe@gmx.de; jgg@ziepe.ca;
> > 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 V10 2/3] tpm_tis-spi: Add hardware wait polling
> > 
> > 
> > > -----Original Message-----
> > > From: Thierry Reding <thierry.reding@gmail.com>
> > > Sent: 24 April 2023 21:02
> > > To: Mark Brown <broonie@kernel.org>
> > > Cc: Jarkko Sakkinen <jarkko@kernel.org>; Krishna Yarlagadda
> > > <kyarlagadda@nvidia.com>; jsnitsel@redhat.com; robh+dt@kernel.org;
> > > peterhuewe@gmx.de; jgg@ziepe.ca; 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 V10 2/3] tpm_tis-spi: Add hardware wait polling
> > >
> > > On Mon, Apr 24, 2023 at 04:18:45PM +0100, Mark Brown wrote:
> > > > On Mon, Apr 24, 2023 at 04:46:24PM +0200, Thierry Reding wrote:
> > > >
> > > > > Would it make sense for you to pick up patch 2/3 as well? As far as I
> > > > > can tell there's a build dependency on patch 1/3 because of the newly
> > > > > added SPI_TPM_HW_FLOW symbol.
> > > >
> > > > I'll include it in my pull request for spi this time round so it should
> > > > end up in -rc1, my thinking was that I was happy with the SPI bits and
> > > > if it was in -rc1 then the TPM bits could be handled without cross tree
> > > > issues when the review was sorted (which it is now but wasn't at the
> > > > time).  If the SPI side doesn't make -rc1 for some reason I can pick up
> > > > the TPM bit as well, and/or do a signed tag.
> > >
> > > Sounds good.
> > >
> > > Thanks,
> > > Thierry
> > 
> > Mark,
> > Now that SPI changes are in, can we pull this TPM change for rc2.
> > Will this be picked into SPI or TPM list?
> Jarkko, Mark,
> Can we pick this change in TPM list since SPI header changes are in.

Hey Mark, Jarkko,

any ideas on how we can best get this merged? I guess at this point it
could go through either tree since the SPI dependency has been in Linus'
tree since v6.4-rc1.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]

On Thu, Jun 01, 2023 at 10:29:51AM +0200, Thierry Reding wrote:

> any ideas on how we can best get this merged? I guess at this point it
> could go through either tree since the SPI dependency has been in Linus'
> tree since v6.4-rc1.

I would expect it to go via whatever path TPM patches usually take given
that it's a TPM patch.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

On Thu, Jun 01, 2023 at 12:04:59PM +0100, Mark Brown wrote:
> On Thu, Jun 01, 2023 at 10:29:51AM +0200, Thierry Reding wrote:
> 
> > any ideas on how we can best get this merged? I guess at this point it
> > could go through either tree since the SPI dependency has been in Linus'
> > tree since v6.4-rc1.
> 
> I would expect it to go via whatever path TPM patches usually take given
> that it's a TPM patch.

There might have been a misunderstanding. My recollection was that you
had said a few weeks ago that you would pick this up. Going through the
thread again I realize that may not have been what you meant. Perhaps
Jarkko misinterpreted this in the same way.

Jarkko, can you pick this up for v6.5?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

On Thu, Jun 01, 2023 at 02:36:51PM +0200, Thierry Reding wrote:
> On Thu, Jun 01, 2023 at 12:04:59PM +0100, Mark Brown wrote:

> > On Thu, Jun 01, 2023 at 10:29:51AM +0200, Thierry Reding wrote:

> > > any ideas on how we can best get this merged? I guess at this point it
> > > could go through either tree since the SPI dependency has been in Linus'
> > > tree since v6.4-rc1.

> > I would expect it to go via whatever path TPM patches usually take given
> > that it's a TPM patch.

> There might have been a misunderstanding. My recollection was that you
> had said a few weeks ago that you would pick this up. Going through the
> thread again I realize that may not have been what you meant. Perhaps
> Jarkko misinterpreted this in the same way.

> Jarkko, can you pick this up for v6.5?

No, I said that I had applied the SPI parts for v6.4 so there would be
no blocker whenever people got round to reviewing the TPM side.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Jarkko Sakkinen]

On Thu Jun 1, 2023 at 3:40 PM EEST, Mark Brown wrote:
> On Thu, Jun 01, 2023 at 02:36:51PM +0200, Thierry Reding wrote:
> > On Thu, Jun 01, 2023 at 12:04:59PM +0100, Mark Brown wrote:
>
> > > On Thu, Jun 01, 2023 at 10:29:51AM +0200, Thierry Reding wrote:
>
> > > > any ideas on how we can best get this merged? I guess at this point it
> > > > could go through either tree since the SPI dependency has been in Linus'
> > > > tree since v6.4-rc1.
>
> > > I would expect it to go via whatever path TPM patches usually take given
> > > that it's a TPM patch.
>
> > There might have been a misunderstanding. My recollection was that you
> > had said a few weeks ago that you would pick this up. Going through the
> > thread again I realize that may not have been what you meant. Perhaps
> > Jarkko misinterpreted this in the same way.
>
> > Jarkko, can you pick this up for v6.5?
>
> No, I said that I had applied the SPI parts for v6.4 so there would be
> no blocker whenever people got round to reviewing the TPM side.

I'm totally cool with this: won't pick the patch then.

BR, Jarkko

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 661 bytes --]

On Fri, Jun 09, 2023 at 05:19:15PM +0300, Jarkko Sakkinen wrote:
> On Thu Jun 1, 2023 at 3:40 PM EEST, Mark Brown wrote:
> > On Thu, Jun 01, 2023 at 02:36:51PM +0200, Thierry Reding wrote:
> > > On Thu, Jun 01, 2023 at 12:04:59PM +0100, Mark Brown wrote:
> > > > On Thu, Jun 01, 2023 at 10:29:51AM +0200, Thierry Reding wrote:

> > > Jarkko, can you pick this up for v6.5?

> > No, I said that I had applied the SPI parts for v6.4 so there would be
> > no blocker whenever people got round to reviewing the TPM side.

> I'm totally cool with this: won't pick the patch then.

I have no intention of applying the patch, I am expecting it to go via
the TPM tree.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[Jarkko Sakkinen]

On Fri Jun 9, 2023 at 5:22 PM EEST, Mark Brown wrote:
> On Fri, Jun 09, 2023 at 05:19:15PM +0300, Jarkko Sakkinen wrote:
> > On Thu Jun 1, 2023 at 3:40 PM EEST, Mark Brown wrote:
> > > On Thu, Jun 01, 2023 at 02:36:51PM +0200, Thierry Reding wrote:
> > > > On Thu, Jun 01, 2023 at 12:04:59PM +0100, Mark Brown wrote:
> > > > > On Thu, Jun 01, 2023 at 10:29:51AM +0200, Thierry Reding wrote:
>
> > > > Jarkko, can you pick this up for v6.5?
>
> > > No, I said that I had applied the SPI parts for v6.4 so there would be
> > > no blocker whenever people got round to reviewing the TPM side.
>
> > I'm totally cool with this: won't pick the patch then.
>
> I have no intention of applying the patch, I am expecting it to go via
> the TPM tree.

https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=8638bedb01ab6170d7dbd1ceaefa5e82639c432d

I'll mirror publish this in my next branch (mirrored to linux-next) soon.

BR, Jarkko

[Patch V10 3/3] spi: tegra210-quad: Enable TPM wait polling

[Krishna Yarlagadda]

Trusted Platform Module requires flow control. As defined in TPM
interface specification, client would drive MISO line at same cycle as
last address bit on MOSI.
Tegra234 and Tegra241 QSPI controllers have TPM wait state detection
feature which is enabled for TPM client devices reported in SPI device
mode bits.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index bea376acea1f..fbd14dd7be44 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -142,6 +142,7 @@
 
 #define QSPI_GLOBAL_CONFIG			0X1a4
 #define QSPI_CMB_SEQ_EN				BIT(0)
+#define QSPI_TPM_WAIT_POLL_EN			BIT(1)
 
 #define QSPI_CMB_SEQ_ADDR			0x1a8
 #define QSPI_ADDRESS_VALUE_SET(X)		(((x) & 0xFFFF) << 0)
@@ -164,6 +165,7 @@
 struct tegra_qspi_soc_data {
 	bool has_dma;
 	bool cmb_xfer_capable;
+	bool supports_tpm;
 	unsigned int cs_count;
 };
 
@@ -1065,6 +1067,12 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 
 	/* Enable Combined sequence mode */
 	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
+	if (spi->mode & SPI_TPM_HW_FLOW) {
+		if (tqspi->soc_data->supports_tpm)
+			val |= QSPI_TPM_WAIT_POLL_EN;
+		else
+			return -EIO;
+	}
 	val |= QSPI_CMB_SEQ_EN;
 	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
 	/* Process individual transfer list */
@@ -1196,6 +1204,8 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
 	/* Disable Combined sequence mode */
 	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
 	val &= ~QSPI_CMB_SEQ_EN;
+	if (tqspi->soc_data->supports_tpm)
+		val &= ~QSPI_TPM_WAIT_POLL_EN;
 	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
 	list_for_each_entry(transfer, &msg->transfers, transfer_list) {
 		struct spi_transfer *xfer = transfer;
@@ -1454,24 +1464,28 @@ static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
 static struct tegra_qspi_soc_data tegra210_qspi_soc_data = {
 	.has_dma = true,
 	.cmb_xfer_capable = false,
+	.supports_tpm = false,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
 	.has_dma = true,
 	.cmb_xfer_capable = true,
+	.supports_tpm = false,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
 	.has_dma = false,
 	.cmb_xfer_capable = true,
+	.supports_tpm = true,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra241_qspi_soc_data = {
 	.has_dma = false,
 	.cmb_xfer_capable = true,
+	.supports_tpm = true,
 	.cs_count = 4,
 };
 
-- 
2.17.1

Re: [Patch V10 3/3] spi: tegra210-quad: Enable TPM wait polling

[Jon Hunter]

On 21/04/2023 10:13, Krishna Yarlagadda wrote:
> Trusted Platform Module requires flow control. As defined in TPM
> interface specification, client would drive MISO line at same cycle as
> last address bit on MOSI.
> Tegra234 and Tegra241 QSPI controllers have TPM wait state detection
> feature which is enabled for TPM client devices reported in SPI device
> mode bits.
> 
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>   drivers/spi/spi-tegra210-quad.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
> index bea376acea1f..fbd14dd7be44 100644
> --- a/drivers/spi/spi-tegra210-quad.c
> +++ b/drivers/spi/spi-tegra210-quad.c
> @@ -142,6 +142,7 @@
>   
>   #define QSPI_GLOBAL_CONFIG			0X1a4
>   #define QSPI_CMB_SEQ_EN				BIT(0)
> +#define QSPI_TPM_WAIT_POLL_EN			BIT(1)
>   
>   #define QSPI_CMB_SEQ_ADDR			0x1a8
>   #define QSPI_ADDRESS_VALUE_SET(X)		(((x) & 0xFFFF) << 0)
> @@ -164,6 +165,7 @@
>   struct tegra_qspi_soc_data {
>   	bool has_dma;
>   	bool cmb_xfer_capable;
> +	bool supports_tpm;
>   	unsigned int cs_count;
>   };
>   
> @@ -1065,6 +1067,12 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
>   
>   	/* Enable Combined sequence mode */
>   	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
> +	if (spi->mode & SPI_TPM_HW_FLOW) {
> +		if (tqspi->soc_data->supports_tpm)
> +			val |= QSPI_TPM_WAIT_POLL_EN;
> +		else
> +			return -EIO;
> +	}
>   	val |= QSPI_CMB_SEQ_EN;
>   	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
>   	/* Process individual transfer list */
> @@ -1196,6 +1204,8 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
>   	/* Disable Combined sequence mode */
>   	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
>   	val &= ~QSPI_CMB_SEQ_EN;
> +	if (tqspi->soc_data->supports_tpm)
> +		val &= ~QSPI_TPM_WAIT_POLL_EN;
>   	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
>   	list_for_each_entry(transfer, &msg->transfers, transfer_list) {
>   		struct spi_transfer *xfer = transfer;
> @@ -1454,24 +1464,28 @@ static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
>   static struct tegra_qspi_soc_data tegra210_qspi_soc_data = {
>   	.has_dma = true,
>   	.cmb_xfer_capable = false,
> +	.supports_tpm = false,
>   	.cs_count = 1,
>   };
>   
>   static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
>   	.has_dma = true,
>   	.cmb_xfer_capable = true,
> +	.supports_tpm = false,
>   	.cs_count = 1,
>   };
>   
>   static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
>   	.has_dma = false,
>   	.cmb_xfer_capable = true,
> +	.supports_tpm = true,
>   	.cs_count = 1,
>   };
>   
>   static struct tegra_qspi_soc_data tegra241_qspi_soc_data = {
>   	.has_dma = false,
>   	.cmb_xfer_capable = true,
> +	.supports_tpm = true,
>   	.cs_count = 4,
>   };
>   


Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

The Tegra change looks good to me, assuming that everyone is happy with 
the other patches in the series.

Jon

-- 
nvpublic

Re: [Patch V10 0/3] Tegra TPM driver with HW flow control

[Jerry Snitselaar]

On Fri, Apr 21, 2023 at 02:43:06PM +0530, Krishna Yarlagadda wrote:
> TPM devices may insert wait state on last clock cycle of ADDR phase.
> For SPI controllers that support full-duplex transfers, this can be
> detected using software by reading the MISO line. For SPI controllers
> that only support half-duplex transfers, such as the Tegra QSPI, it is
> not possible to detect the wait signal from software. The QSPI
> controller in Tegra234 and Tegra241 implement hardware detection of the
> wait signal which can be enabled in the controller for TPM devices.
> 
> Add HW flow control in TIS driver and a flag in SPI data to indicate
> wait detection is required in HW. SPI controller driver determines if
> this is supported. Add HW detection in Tegra QSPI controller.
> 
> Updates in this patch set 
>  - Tegra QSPI identifies itself as half duplex.
>  - TPM TIS SPI driver skips flow control for half duplex and send
>    transfers in single message for controller to handle it.
>  - TPM device identifies as TPM device for controller to detect and
>    enable HW TPM wait poll feature.
> 
> Verified with a TPM device on Tegra241 ref board using TPM2 tools.
> 
> V10
>  - use spi_sync in place of spi_sync_locked
> V9
>  - renamed tpm spi transfer functions
> V8:
>  - fix compile warning.
> V7:
>  - updated patch description.
>  - TPM flag set in probe.
>  - minor comments.
> V6:
>  - Fix typo in chip name Tegra234.
>  - Debug logs change skipped to be sent later.
>  - Consistent usage of soc flag.
> V5:
>  - No SPI bus locking.
> V4:
>  - Split api change to different patch.
>  - Describe TPM HW flow control.
> V3:
>  - Use SPI device mode flag and SPI controller flags.
>  - Drop usage of device tree flags.
>  - Generic TPM half duplex controller handling.
>  - HW & SW flow control for TPM. Drop additional driver.
> V2:
>  - Fix dt schema errors.
> 
> Krishna Yarlagadda (3):
>   spi: Add TPM HW flow flag
>   tpm_tis-spi: Add hardware wait polling
>   spi: tegra210-quad: Enable TPM wait polling
> 
>  drivers/char/tpm/tpm_tis_spi_main.c | 91 ++++++++++++++++++++++++++++-
>  drivers/spi/spi-tegra210-quad.c     | 14 +++++
>  include/linux/spi/spi.h             | 16 ++++-
>  3 files changed, 116 insertions(+), 5 deletions(-)
> 
> -- 
> 2.17.1
> 

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

Re: (subset) [Patch V10 0/3] Tegra TPM driver with HW flow control

[Mark Brown]

On Fri, 21 Apr 2023 14:43:06 +0530, Krishna Yarlagadda wrote:
> TPM devices may insert wait state on last clock cycle of ADDR phase.
> For SPI controllers that support full-duplex transfers, this can be
> detected using software by reading the MISO line. For SPI controllers
> that only support half-duplex transfers, such as the Tegra QSPI, it is
> not possible to detect the wait signal from software. The QSPI
> controller in Tegra234 and Tegra241 implement hardware detection of the
> wait signal which can be enabled in the controller for TPM devices.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/3] spi: Add TPM HW flow flag
      commit: 67a142dc9eb96a5cc018e5db62390665eb5f038c
[3/3] spi: tegra210-quad: Enable TPM wait polling
      commit: 967ca91a996f82219f2883e9e53d8e20df49025a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230421091309.2672-3-kyarlagadda@nvidia.com>
References: <20230421091309.2672-3-kyarlagadda@nvidia.com>
TO: Krishna Yarlagadda <kyarlagadda@nvidia.com>
TO: jsnitsel@redhat.com
TO: robh+dt@kernel.org
TO: broonie@kernel.org
TO: peterhuewe@gmx.de
TO: jgg@ziepe.ca
TO: jarkko@kernel.org
TO: krzysztof.kozlowski+dt@linaro.org
TO: linux-spi@vger.kernel.org
TO: linux-tegra@vger.kernel.org
TO: linux-integrity@vger.kernel.org
TO: linux-kernel@vger.kernel.org
CC: thierry.reding@gmail.com
CC: jonathanh@nvidia.com
CC: skomatineni@nvidia.com
CC: ldewangan@nvidia.com
CC: Krishna Yarlagadda <kyarlagadda@nvidia.com>

Hi Krishna,

kernel test robot noticed the following build warnings:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.3-rc7 next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Krishna-Yarlagadda/spi-Add-TPM-HW-flow-flag/20230421-171759
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link:    https://lore.kernel.org/r/20230421091309.2672-3-kyarlagadda%40nvidia.com
patch subject: [Patch V10 2/3] tpm_tis-spi: Add hardware wait polling
:::::: branch date: 22 hours ago
:::::: commit date: 22 hours ago
config: arc-randconfig-m041-20230421 (https://download.01.org/0day-ci/archive/20230422/202304221457.9URN171w-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202304221457.9URN171w-lkp@intel.com/

smatch warnings:
drivers/char/tpm/tpm_tis_spi_main.c:137 tpm_tis_spi_transfer_half() error: uninitialized symbol 'ret'.

vim +/ret +137 drivers/char/tpm/tpm_tis_spi_main.c

8ab5e82afa969b drivers/char/tpm/tpm_tis_spi.c      Stephen Boyd       2019-09-20   73  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   74  /*
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   75   * Half duplex controller with support for TPM wait state detection like
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   76   * Tegra QSPI need CMD, ADDR & DATA sent in single message to manage HW flow
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   77   * control. Each phase sent in different transfer for controller to idenity
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   78   * phase.
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   79   */
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   80  static int tpm_tis_spi_transfer_half(struct tpm_tis_data *data,	u32 addr,
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   81  				     u16 len, u8 *in, const u8 *out)
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   82  {
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   83  	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   84  	struct spi_transfer spi_xfer[3];
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   85  	struct spi_message m;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   86  	u8 transfer_len;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   87  	int ret;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   88  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   89  	while (len) {
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   90  		transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   91  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   92  		spi_message_init(&m);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   93  		phy->iobuf[0] = (in ? 0x80 : 0) | (transfer_len - 1);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   94  		phy->iobuf[1] = 0xd4;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   95  		phy->iobuf[2] = addr >> 8;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   96  		phy->iobuf[3] = addr;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   97  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   98  		memset(&spi_xfer, 0, sizeof(spi_xfer));
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21   99  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  100  		spi_xfer[0].tx_buf = phy->iobuf;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  101  		spi_xfer[0].len = 1;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  102  		spi_message_add_tail(&spi_xfer[0], &m);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  103  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  104  		spi_xfer[1].tx_buf = phy->iobuf + 1;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  105  		spi_xfer[1].len = 3;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  106  		spi_message_add_tail(&spi_xfer[1], &m);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  107  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  108  		if (out) {
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  109  			spi_xfer[2].tx_buf = &phy->iobuf[4];
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  110  			spi_xfer[2].rx_buf = NULL;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  111  			memcpy(&phy->iobuf[4], out, transfer_len);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  112  			out += transfer_len;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  113  		}
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  114  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  115  		if (in) {
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  116  			spi_xfer[2].tx_buf = NULL;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  117  			spi_xfer[2].rx_buf = &phy->iobuf[4];
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  118  		}
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  119  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  120  		spi_xfer[2].len = transfer_len;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  121  		spi_message_add_tail(&spi_xfer[2], &m);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  122  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  123  		reinit_completion(&phy->ready);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  124  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  125  		ret = spi_sync(phy->spi_device, &m);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  126  		if (ret < 0)
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  127  			return ret;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  128  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  129  		if (in) {
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  130  			memcpy(in, &phy->iobuf[4], transfer_len);
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  131  			in += transfer_len;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  132  		}
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  133  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  134  		len -= transfer_len;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  135  	}
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  136  
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21 @137  	return ret;
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  138  }
c09d1b20ab5d73 drivers/char/tpm/tpm_tis_spi_main.c Krishna Yarlagadda 2023-04-21  139  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests