linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Patch V6 0/3] Tegra TPM driver with HW flow control
@ 2023-02-27 17:21 Krishna Yarlagadda
  2023-02-27 17:21 ` [Patch V6 1/3] spi: Add TPM HW flow flag Krishna Yarlagadda
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Krishna Yarlagadda @ 2023-02-27 17:21 UTC (permalink / raw)
  To: robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel
  Cc: thierry.reding, jonathanh, skomatineni, ldewangan, Krishna Yarlagadda

TPM interface spec defines flow control where TPM device would drive
MISO at same cycle as last address bit sent by controller on MOSI. This
state of wait can be detected by software reading the MISO line or
by controller hardware. Support sending transfers to controller in
single message and handle flow control in hardware. Half duplex
controllers have to support flow control in hardware.

Tegra234 and Tegra241 chips have QSPI controller that supports TPM
Interface Specification (TIS) flow control.
Since the controller only supports half duplex, SW wait polling
(flow control using full duplex transfers) method implemented in
tpm_tis_spi_main.c will not work and have to us HW flow control.

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.

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: Support hardware wait polling
  spi: tegra210-quad: Enable TPM wait polling

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

-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Patch V6 1/3] spi: Add TPM HW flow flag
  2023-02-27 17:21 [Patch V6 0/3] Tegra TPM driver with HW flow control Krishna Yarlagadda
@ 2023-02-27 17:21 ` Krishna Yarlagadda
  2023-03-01 13:34   ` Thierry Reding
  2023-02-27 17:21 ` [Patch V6 2/3] tpm_tis-spi: Support hardware wait polling Krishna Yarlagadda
  2023-02-27 17:21 ` [Patch V6 3/3] spi: tegra210-quad: Enable TPM " Krishna Yarlagadda
  2 siblings, 1 reply; 10+ messages in thread
From: Krishna Yarlagadda @ 2023-02-27 17:21 UTC (permalink / raw)
  To: robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel
  Cc: thierry.reding, jonathanh, skomatineni, ldewangan, Krishna Yarlagadda

TPM spec defines flow control over SPI. Client device can insert a wait
state on MISO when address is trasmitted by controller on MOSI. It can
work only on full duplex.
Half duplex controllers need to implement flow control in HW.
Add a flag for TPM to indicate flow control is expected in controller.

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 */
 	/*
 	 * 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 +196,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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Patch V6 2/3] tpm_tis-spi: Support hardware wait polling
  2023-02-27 17:21 [Patch V6 0/3] Tegra TPM driver with HW flow control Krishna Yarlagadda
  2023-02-27 17:21 ` [Patch V6 1/3] spi: Add TPM HW flow flag Krishna Yarlagadda
@ 2023-02-27 17:21 ` Krishna Yarlagadda
  2023-03-01 13:37   ` Thierry Reding
  2023-02-27 17:21 ` [Patch V6 3/3] spi: tegra210-quad: Enable TPM " Krishna Yarlagadda
  2 siblings, 1 reply; 10+ messages in thread
From: Krishna Yarlagadda @ 2023-02-27 17:21 UTC (permalink / raw)
  To: robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel
  Cc: thierry.reding, jonathanh, skomatineni, ldewangan, Krishna Yarlagadda

TPM devices raise wait signal on last addr cycle. This can be detected
by software driver by reading MISO line on same clock which requires
full duplex support. In case of half duplex controllers wait detection
has to be implemented in HW.
Support hardware wait state detection by sending entire message and let
controller handle flow control.
QSPI controller in Tegra234 & Tegra241 implement TPM wait polling.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/char/tpm/tpm_tis_spi_main.c | 92 ++++++++++++++++++++++++++++-
 1 file changed, 90 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..5f66448ee09e 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
+ * Tegra241 need cmd, addr & data sent in single message to manage HW flow
+ * control. Each phase sent in different transfer for controller to idenity
+ * phase.
+ */
+int tpm_tis_spi_hw_flow_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_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_locked(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;
+}
+
+int tpm_tis_spi_sw_flow_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);
 	int ret = 0;
@@ -140,6 +206,28 @@ 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) {
+		phy->spi_device->mode |= SPI_TPM_HW_FLOW;
+		return tpm_tis_spi_hw_flow_transfer(data, addr, len, in,
+						    out);
+	} else {
+		return tpm_tis_spi_sw_flow_transfer(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)
 {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Patch V6 3/3] spi: tegra210-quad: Enable TPM wait polling
  2023-02-27 17:21 [Patch V6 0/3] Tegra TPM driver with HW flow control Krishna Yarlagadda
  2023-02-27 17:21 ` [Patch V6 1/3] spi: Add TPM HW flow flag Krishna Yarlagadda
  2023-02-27 17:21 ` [Patch V6 2/3] tpm_tis-spi: Support hardware wait polling Krishna Yarlagadda
@ 2023-02-27 17:21 ` Krishna Yarlagadda
  2023-03-01 13:39   ` Thierry Reding
                     ` (2 more replies)
  2 siblings, 3 replies; 10+ messages in thread
From: Krishna Yarlagadda @ 2023-02-27 17:21 UTC (permalink / raw)
  To: robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel
  Cc: thierry.reding, jonathanh, skomatineni, ldewangan, 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.
Tegra241 QSPI controller has TPM wait state detection feature which is
enabled for TPM client devices reported in SPI device mode bits.
Set half duplex flag for TPM device to detect and send entire message
to controller in one shot.

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 b967576b6c96..e1165584a20a 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 support_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->support_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 */
@@ -1192,6 +1200,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->support_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;
@@ -1450,24 +1460,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,
+	.support_tpm = false,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
 	.has_dma = true,
 	.cmb_xfer_capable = true,
+	.support_tpm = false,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
 	.has_dma = false,
 	.cmb_xfer_capable = true,
+	.support_tpm = true,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra241_qspi_soc_data = {
 	.has_dma = false,
 	.cmb_xfer_capable = true,
+	.support_tpm = true,
 	.cs_count = 4,
 };
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Patch V6 1/3] spi: Add TPM HW flow flag
  2023-02-27 17:21 ` [Patch V6 1/3] spi: Add TPM HW flow flag Krishna Yarlagadda
@ 2023-03-01 13:34   ` Thierry Reding
  2023-03-01 15:27     ` Krishna Yarlagadda
  0 siblings, 1 reply; 10+ messages in thread
From: Thierry Reding @ 2023-03-01 13:34 UTC (permalink / raw)
  To: Krishna Yarlagadda
  Cc: robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel, jonathanh, skomatineni, ldewangan

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

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).

> 
> 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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Patch V6 2/3] tpm_tis-spi: Support hardware wait polling
  2023-02-27 17:21 ` [Patch V6 2/3] tpm_tis-spi: Support hardware wait polling Krishna Yarlagadda
@ 2023-03-01 13:37   ` Thierry Reding
  0 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2023-03-01 13:37 UTC (permalink / raw)
  To: Krishna Yarlagadda
  Cc: robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel, jonathanh, skomatineni, ldewangan

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

On Mon, Feb 27, 2023 at 10:51:07PM +0530, Krishna Yarlagadda wrote:
[...]
> diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
[...]
> @@ -140,6 +206,28 @@ 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) {
> +		phy->spi_device->mode |= SPI_TPM_HW_FLOW;

Can this be pushed up to tpm_tis_spi_probe()? The SPI controller flags
are fixed, so this isn't going to change at runtime, is it?

Thierry

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Patch V6 3/3] spi: tegra210-quad: Enable TPM wait polling
  2023-02-27 17:21 ` [Patch V6 3/3] spi: tegra210-quad: Enable TPM " Krishna Yarlagadda
@ 2023-03-01 13:39   ` Thierry Reding
  2023-03-01 13:41   ` Thierry Reding
  2023-03-01 13:51   ` Jon Hunter
  2 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2023-03-01 13:39 UTC (permalink / raw)
  To: Krishna Yarlagadda
  Cc: robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel, jonathanh, skomatineni, ldewangan

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

On Mon, Feb 27, 2023 at 10:51:08PM +0530, 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.
> Tegra241 QSPI controller has TPM wait state detection feature which is
> enabled for TPM client devices reported in SPI device mode bits.
> Set half duplex flag for TPM device to detect and send entire message
> to controller in one shot.
> 
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/spi/spi-tegra210-quad.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Acked-by: Thierry Reding <treding@nvidia.com>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Patch V6 3/3] spi: tegra210-quad: Enable TPM wait polling
  2023-02-27 17:21 ` [Patch V6 3/3] spi: tegra210-quad: Enable TPM " Krishna Yarlagadda
  2023-03-01 13:39   ` Thierry Reding
@ 2023-03-01 13:41   ` Thierry Reding
  2023-03-01 13:51   ` Jon Hunter
  2 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2023-03-01 13:41 UTC (permalink / raw)
  To: Krishna Yarlagadda
  Cc: robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel, jonathanh, skomatineni, ldewangan

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

On Mon, Feb 27, 2023 at 10:51:08PM +0530, 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.
> Tegra241 QSPI controller has TPM wait state detection feature which is
> enabled for TPM client devices reported in SPI device mode bits.
> Set half duplex flag for TPM device to detect and send entire message
> to controller in one shot.
> 
> 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 b967576b6c96..e1165584a20a 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 support_tpm;

Nit: this could be "supports_tpm" for slightly more consistency with
"has_dma".

Thierry

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Patch V6 3/3] spi: tegra210-quad: Enable TPM wait polling
  2023-02-27 17:21 ` [Patch V6 3/3] spi: tegra210-quad: Enable TPM " Krishna Yarlagadda
  2023-03-01 13:39   ` Thierry Reding
  2023-03-01 13:41   ` Thierry Reding
@ 2023-03-01 13:51   ` Jon Hunter
  2 siblings, 0 replies; 10+ messages in thread
From: Jon Hunter @ 2023-03-01 13:51 UTC (permalink / raw)
  To: Krishna Yarlagadda, robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel
  Cc: thierry.reding, skomatineni, ldewangan



On 27/02/2023 17:21, 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.
> Tegra241 QSPI controller has TPM wait state detection feature which is
> enabled for TPM client devices reported in SPI device mode bits.

Earlier in patch 2/3 we say Tegra234 and Tegra241 but here we just say 
Tegra241.

>   static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
>   	.has_dma = true,
>   	.cmb_xfer_capable = true,
> +	.support_tpm = false,
>   	.cs_count = 1,
>   };
>   
>   static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
>   	.has_dma = false,
>   	.cmb_xfer_capable = true,
> +	.support_tpm = true,
>   	.cs_count = 1,
>   };
>   
>   static struct tegra_qspi_soc_data tegra241_qspi_soc_data = {
>   	.has_dma = false,
>   	.cmb_xfer_capable = true,
> +	.support_tpm = true,
>   	.cs_count = 4,
>   };
>   

We enable for both Tegra234 and Tegra241 and so the commit message 
should say so.

Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [Patch V6 1/3] spi: Add TPM HW flow flag
  2023-03-01 13:34   ` Thierry Reding
@ 2023-03-01 15:27     ` Krishna Yarlagadda
  0 siblings, 0 replies; 10+ messages in thread
From: Krishna Yarlagadda @ 2023-03-01 15:27 UTC (permalink / raw)
  To: Thierry Reding
  Cc: robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel, Jonathan Hunter, Sowjanya Komatineni,
	Laxman Dewangan

> -----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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-03-01 15:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-27 17:21 [Patch V6 0/3] Tegra TPM driver with HW flow control Krishna Yarlagadda
2023-02-27 17:21 ` [Patch V6 1/3] spi: Add TPM HW flow flag Krishna Yarlagadda
2023-03-01 13:34   ` Thierry Reding
2023-03-01 15:27     ` Krishna Yarlagadda
2023-02-27 17:21 ` [Patch V6 2/3] tpm_tis-spi: Support hardware wait polling Krishna Yarlagadda
2023-03-01 13:37   ` Thierry Reding
2023-02-27 17:21 ` [Patch V6 3/3] spi: tegra210-quad: Enable TPM " Krishna Yarlagadda
2023-03-01 13:39   ` Thierry Reding
2023-03-01 13:41   ` Thierry Reding
2023-03-01 13:51   ` Jon Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).