linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Patch V2 0/4]  Tegra TPM driver with hw flow control
@ 2023-02-03 13:01 Krishna Yarlagadda
  2023-02-03 13:01 ` [Patch V2 1/4] dt-bindings: tpm: Add compatible for Tegra TPM Krishna Yarlagadda
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Krishna Yarlagadda @ 2023-02-03 13:01 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

Tegra234 and Tegra241 chips have QSPI controller that supports TCG
PC Client Specific 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 does not suffice.
Added extended driver to disable sw flow control and send
all transfers in single message. Flow control is handled by hardware.

V2: Fix dt schema errors.

Krishna Yarlagadda (4):
  dt-bindings: tpm: Add compatible for Tegra TPM
  tpm: tegra: Support SPI tpm wait state detect
  spi: dt-bindings: Add Tegra TPM wait polling flag
  spi: tegra210-quad: Enable TPM wait polling

 .../security/tpm/nvidia,tegra-tpm-spi.yaml    |  34 +++++
 ...nvidia,tegra210-quad-peripheral-props.yaml |   6 +
 drivers/char/tpm/Makefile                     |   1 +
 drivers/char/tpm/tpm_tis_spi.h                |   1 +
 drivers/char/tpm/tpm_tis_spi_main.c           |   4 +-
 drivers/char/tpm/tpm_tis_spi_tegra.c          | 123 ++++++++++++++++++
 drivers/spi/spi-tegra210-quad.c               |  16 +++
 7 files changed, 184 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/nvidia,tegra-tpm-spi.yaml
 create mode 100644 drivers/char/tpm/tpm_tis_spi_tegra.c

-- 
2.17.1


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

* [Patch V2 1/4] dt-bindings: tpm: Add compatible for Tegra TPM
  2023-02-03 13:01 [Patch V2 0/4] Tegra TPM driver with hw flow control Krishna Yarlagadda
@ 2023-02-03 13:01 ` Krishna Yarlagadda
  2023-02-03 18:57   ` Rob Herring
  2023-02-03 13:01 ` [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect Krishna Yarlagadda
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Krishna Yarlagadda @ 2023-02-03 13:01 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

Tegra234 and Tegra241 devices have QSPI controller that supports TPM
devices. Since the controller only supports half duplex, sw wait polling
method implemented in tpm_tis_spi does not suffice. Wait polling as per
protocol is a hardware feature.

Add compatible for Tegra TPM driver with hardware flow control.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 .../security/tpm/nvidia,tegra-tpm-spi.yaml    | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/nvidia,tegra-tpm-spi.yaml

diff --git a/Documentation/devicetree/bindings/security/tpm/nvidia,tegra-tpm-spi.yaml b/Documentation/devicetree/bindings/security/tpm/nvidia,tegra-tpm-spi.yaml
new file mode 100644
index 000000000000..dcb78db7355c
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/nvidia,tegra-tpm-spi.yaml
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/security/tpm/nvidia,tegra-tpm-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Tegra QSPI TPM driver
+
+maintainers:
+  - Thierry Reding <thierry.reding@gmail.com>
+  - Jonathan Hunter <jonathanh@nvidia.com>
+
+properties:
+  compatible:
+    enum:
+      - nvidia,tegra-tpm-spi
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    qspi1@3250000 {
+      tpm@0 {
+        compatible = "nvidia,tegra-tpm-spi";
+        reg = <0>;
+      };
+    };
-- 
2.17.1


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

* [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect
  2023-02-03 13:01 [Patch V2 0/4] Tegra TPM driver with hw flow control Krishna Yarlagadda
  2023-02-03 13:01 ` [Patch V2 1/4] dt-bindings: tpm: Add compatible for Tegra TPM Krishna Yarlagadda
@ 2023-02-03 13:01 ` Krishna Yarlagadda
  2023-02-03 23:43   ` kernel test robot
                     ` (2 more replies)
  2023-02-03 13:01 ` [Patch V2 3/4] spi: dt-bindings: Add Tegra TPM wait polling flag Krishna Yarlagadda
  2023-02-03 13:01 ` [Patch V2 4/4] spi: tegra210-quad: Enable TPM wait polling Krishna Yarlagadda
  3 siblings, 3 replies; 13+ messages in thread
From: Krishna Yarlagadda @ 2023-02-03 13:01 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

Tegra234 and Tegra241 chips have QSPI controller that supports TCG
TIS hardware flow control. Since the controller only supports half
duplex, sw wait polling method implemented in tpm_tis_spi does not
suffice. Added extending driver to disable sw flow control and send
all transfers in single message.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/char/tpm/Makefile            |   1 +
 drivers/char/tpm/tpm_tis_spi.h       |   1 +
 drivers/char/tpm/tpm_tis_spi_main.c  |   4 +-
 drivers/char/tpm/tpm_tis_spi_tegra.c | 123 +++++++++++++++++++++++++++
 4 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 drivers/char/tpm/tpm_tis_spi_tegra.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 0222b1ddb310..445b15493cb3 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_TCG_TIS_SYNQUACER) += tpm_tis_synquacer.o
 
 obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
 tpm_tis_spi-y := tpm_tis_spi_main.o
+tpm_tis_spi-y += tpm_tis_spi_tegra.o
 tpm_tis_spi-$(CONFIG_TCG_TIS_SPI_CR50) += tpm_tis_spi_cr50.o
 
 obj-$(CONFIG_TCG_TIS_I2C_CR50) += tpm_tis_i2c_cr50.o
diff --git a/drivers/char/tpm/tpm_tis_spi.h b/drivers/char/tpm/tpm_tis_spi.h
index d0f66f6f1931..feaea14b428b 100644
--- a/drivers/char/tpm/tpm_tis_spi.h
+++ b/drivers/char/tpm/tpm_tis_spi.h
@@ -31,6 +31,7 @@ extern int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy,
 extern int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
 				u8 *in, const u8 *out);
 
+extern int tegra_tpm_spi_probe(struct spi_device *spi);
 #ifdef CONFIG_TCG_TIS_SPI_CR50
 extern int cr50_spi_probe(struct spi_device *spi);
 #else
diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
index a0963a3e92bd..5d4502a4461a 100644
--- a/drivers/char/tpm/tpm_tis_spi_main.c
+++ b/drivers/char/tpm/tpm_tis_spi_main.c
@@ -198,7 +198,7 @@ static int tpm_tis_spi_driver_probe(struct spi_device *spi)
 	const struct spi_device_id *spi_dev_id = spi_get_device_id(spi);
 	tpm_tis_spi_probe_func probe_func;
 
-	probe_func = of_device_get_match_data(&spi->dev);
+	probe_func = device_get_match_data(&spi->dev);
 	if (!probe_func) {
 		if (spi_dev_id) {
 			probe_func = (tpm_tis_spi_probe_func)spi_dev_id->driver_data;
@@ -227,6 +227,7 @@ static const struct spi_device_id tpm_tis_spi_id[] = {
 	{ "tpm_tis_spi", (unsigned long)tpm_tis_spi_probe },
 	{ "tpm_tis-spi", (unsigned long)tpm_tis_spi_probe },
 	{ "cr50", (unsigned long)cr50_spi_probe },
+	{ "tegra-tpm-spi", (unsigned long)tegra_tpm_spi_probe },
 	{}
 };
 MODULE_DEVICE_TABLE(spi, tpm_tis_spi_id);
@@ -236,6 +237,7 @@ static const struct of_device_id of_tis_spi_match[] = {
 	{ .compatible = "infineon,slb9670", .data = tpm_tis_spi_probe },
 	{ .compatible = "tcg,tpm_tis-spi", .data = tpm_tis_spi_probe },
 	{ .compatible = "google,cr50", .data = cr50_spi_probe },
+	{ .compatible = "nvidia,tegra-tpm-spi", .data = tegra_tpm_spi_probe },
 	{}
 };
 MODULE_DEVICE_TABLE(of, of_tis_spi_match);
diff --git a/drivers/char/tpm/tpm_tis_spi_tegra.c b/drivers/char/tpm/tpm_tis_spi_tegra.c
new file mode 100644
index 000000000000..23f20684513d
--- /dev/null
+++ b/drivers/char/tpm/tpm_tis_spi_tegra.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 NVIDIA CORPORATION.
+ *
+ * This device driver implements TEGRA QSPI hw wait detection for chips
+ *
+ * It is based on tpm_tis_spi driver by Peter Huewe and Christophe Ricard.
+ */
+
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pm.h>
+#include <linux/spi/spi.h>
+#include <linux/wait.h>
+
+#include "tpm_tis_core.h"
+#include "tpm_tis_spi.h"
+
+#define MAX_SPI_FRAMESIZE 64
+
+int tpm_tis_spi_tegra_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;
+	struct spi_message m;
+	struct spi_transfer spi_xfer[3];
+	u8 transfer_len;
+
+	spi_bus_lock(phy->spi_device->master);
+
+	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)
+			goto exit;
+
+		if (in) {
+			memcpy(in, &phy->iobuf[4], transfer_len);
+			in += transfer_len;
+		}
+
+		len -= transfer_len;
+	}
+
+exit:
+	spi_bus_unlock(phy->spi_device->master);
+	return ret;
+}
+
+static int tpm_tis_spi_tegra_read_bytes(struct tpm_tis_data *data, u32 addr,
+					u16 len, u8 *result,
+					enum tpm_tis_io_mode io_mode)
+{
+	return tpm_tis_spi_tegra_transfer(data, addr, len, result, NULL);
+}
+
+static int tpm_tis_spi_tegra_write_bytes(struct tpm_tis_data *data, u32 addr,
+					 u16 len, const u8 *value,
+					 enum tpm_tis_io_mode io_mode)
+{
+	return tpm_tis_spi_tegra_transfer(data, addr, len, NULL, value);
+}
+
+static const struct tpm_tis_phy_ops tegra_tpm_spi_phy_ops = {
+	.read_bytes = tpm_tis_spi_tegra_read_bytes,
+	.write_bytes = tpm_tis_spi_tegra_write_bytes,
+};
+
+int tegra_tpm_spi_probe(struct spi_device *dev)
+{
+	struct tpm_tis_spi_phy *phy;
+	int irq;
+
+	phy = devm_kzalloc(&dev->dev, sizeof(struct tpm_tis_spi_phy),
+			   GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	phy->flow_control = NULL;
+
+	/* If the SPI device has an IRQ then use that */
+	if (dev->irq > 0)
+		irq = dev->irq;
+	else
+		irq = -1;
+
+	init_completion(&phy->ready);
+	return tpm_tis_spi_init(dev, phy, irq, &tegra_tpm_spi_phy_ops);
+}
-- 
2.17.1


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

* [Patch V2 3/4] spi: dt-bindings: Add Tegra TPM wait polling flag
  2023-02-03 13:01 [Patch V2 0/4] Tegra TPM driver with hw flow control Krishna Yarlagadda
  2023-02-03 13:01 ` [Patch V2 1/4] dt-bindings: tpm: Add compatible for Tegra TPM Krishna Yarlagadda
  2023-02-03 13:01 ` [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect Krishna Yarlagadda
@ 2023-02-03 13:01 ` Krishna Yarlagadda
  2023-02-03 19:49   ` Rob Herring
  2023-02-03 13:01 ` [Patch V2 4/4] spi: tegra210-quad: Enable TPM wait polling Krishna Yarlagadda
  3 siblings, 1 reply; 13+ messages in thread
From: Krishna Yarlagadda @ 2023-02-03 13:01 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

Add "nvidia,wait-polling" flag to enable TCG TIS hardware flow control.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 .../bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
index 2c3cada75339..19d2b30cadbf 100644
--- a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
@@ -29,4 +29,10 @@ properties:
     minimum: 0
     maximum: 255
 
+  nvidia,wait-polling:
+    description:
+      Enable TPM wait polling feature for QSPI as specified in TCG PC Client
+      Specific TPM Interface Specification (TIS).
+    $ref: /schemas/types.yaml#/definitions/flag
+
 additionalProperties: true
-- 
2.17.1


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

* [Patch V2 4/4] spi: tegra210-quad: Enable TPM wait polling
  2023-02-03 13:01 [Patch V2 0/4] Tegra TPM driver with hw flow control Krishna Yarlagadda
                   ` (2 preceding siblings ...)
  2023-02-03 13:01 ` [Patch V2 3/4] spi: dt-bindings: Add Tegra TPM wait polling flag Krishna Yarlagadda
@ 2023-02-03 13:01 ` Krishna Yarlagadda
  3 siblings, 0 replies; 13+ messages in thread
From: Krishna Yarlagadda @ 2023-02-03 13:01 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 defines flow control where slave will drive
data line at specified clock cycles. Tegra241 has TPM wait state
detections support when using combined sequence transfers. Enabling
the feature based on device tree flag.

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

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 9f356612ba7e..ea8a08a3d838 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)
@@ -170,6 +171,7 @@ struct tegra_qspi_soc_data {
 struct tegra_qspi_client_data {
 	int tx_clk_tap_delay;
 	int rx_clk_tap_delay;
+	bool wait_polling;
 };
 
 struct tegra_qspi {
@@ -934,6 +936,8 @@ static struct tegra_qspi_client_data *tegra_qspi_parse_cdata_dt(struct spi_devic
 				 &cdata->tx_clk_tap_delay);
 	device_property_read_u32(&spi->dev, "nvidia,rx-clk-tap-delay",
 				 &cdata->rx_clk_tap_delay);
+	cdata->wait_polling =
+		device_property_read_bool(&spi->dev, "nvidia,wait-polling");
 
 	return cdata;
 }
@@ -991,6 +995,14 @@ static void tegra_qspi_dump_regs(struct tegra_qspi *tqspi)
 	dev_dbg(tqspi->dev, "TRANS_STAT:  0x%08x | FIFO_STATUS: 0x%08x\n",
 		tegra_qspi_readl(tqspi, QSPI_TRANS_STATUS),
 		tegra_qspi_readl(tqspi, QSPI_FIFO_STATUS));
+	dev_dbg(tqspi->dev, "GLOBAL_CFG: 0x%08x\n",
+		tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG));
+	dev_dbg(tqspi->dev, "CMB_CMD: 0x%08x | CMB_CMD_CFG: 0x%08x\n",
+		tegra_qspi_readl(tqspi, QSPI_CMB_SEQ_CMD),
+		tegra_qspi_readl(tqspi, QSPI_CMB_SEQ_CMD_CFG));
+	dev_dbg(tqspi->dev, "CMB_ADDR: 0x%08x | CMB_ADDR_CFG: 0x%08x\n",
+		tegra_qspi_readl(tqspi, QSPI_CMB_SEQ_ADDR),
+		tegra_qspi_readl(tqspi, QSPI_CMB_SEQ_ADDR_CFG));
 }
 
 static void tegra_qspi_handle_error(struct tegra_qspi *tqspi)
@@ -1056,6 +1068,7 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 	bool is_first_msg = true;
 	struct spi_transfer *xfer;
 	struct spi_device *spi = msg->spi;
+	struct tegra_qspi_client_data *cdata = spi->controller_data;
 	u8 transfer_phase = 0;
 	u32 cmd1 = 0, dma_ctl = 0;
 	int ret = 0;
@@ -1065,6 +1078,8 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 
 	/* Enable Combined sequence mode */
 	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
+	if (cdata->wait_polling)
+		val |= QSPI_TPM_WAIT_POLL_EN;
 	val |= QSPI_CMB_SEQ_EN;
 	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
 	/* Process individual transfer list */
@@ -1192,6 +1207,7 @@ 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;
+	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;
-- 
2.17.1


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

* Re: [Patch V2 1/4] dt-bindings: tpm: Add compatible for Tegra TPM
  2023-02-03 13:01 ` [Patch V2 1/4] dt-bindings: tpm: Add compatible for Tegra TPM Krishna Yarlagadda
@ 2023-02-03 18:57   ` Rob Herring
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2023-02-03 18:57 UTC (permalink / raw)
  To: Krishna Yarlagadda
  Cc: broonie, peterhuewe, jgg, jarkko, krzysztof.kozlowski+dt,
	linux-spi, linux-tegra, linux-integrity, linux-kernel,
	thierry.reding, jonathanh, skomatineni, ldewangan

On Fri, Feb 3, 2023 at 7:02 AM Krishna Yarlagadda
<kyarlagadda@nvidia.com> wrote:
>

Please use get_maintainers.pl. In particular, resend to DT list so
automated checks work.

> Tegra234 and Tegra241 devices have QSPI controller that supports TPM
> devices. Since the controller only supports half duplex, sw wait polling
> method implemented in tpm_tis_spi does not suffice. Wait polling as per
> protocol is a hardware feature.
>
> Add compatible for Tegra TPM driver with hardware flow control.
>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  .../security/tpm/nvidia,tegra-tpm-spi.yaml    | 34 +++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/security/tpm/nvidia,tegra-tpm-spi.yaml
>
> diff --git a/Documentation/devicetree/bindings/security/tpm/nvidia,tegra-tpm-spi.yaml b/Documentation/devicetree/bindings/security/tpm/nvidia,tegra-tpm-spi.yaml
> new file mode 100644
> index 000000000000..dcb78db7355c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/security/tpm/nvidia,tegra-tpm-spi.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/security/tpm/nvidia,tegra-tpm-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Tegra QSPI TPM driver

Bindings are for h/w, not drivers.

> +
> +maintainers:
> +  - Thierry Reding <thierry.reding@gmail.com>
> +  - Jonathan Hunter <jonathanh@nvidia.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nvidia,tegra-tpm-spi
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    qspi1@3250000 {

spi@...

> +      tpm@0 {
> +        compatible = "nvidia,tegra-tpm-spi";

Tegra has a TPM chip/block? This doesn't seem right.

> +        reg = <0>;
> +      };
> +    };
> --
> 2.17.1
>

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

* Re: [Patch V2 3/4] spi: dt-bindings: Add Tegra TPM wait polling flag
  2023-02-03 13:01 ` [Patch V2 3/4] spi: dt-bindings: Add Tegra TPM wait polling flag Krishna Yarlagadda
@ 2023-02-03 19:49   ` Rob Herring
  2023-02-23 16:26     ` Krishna Yarlagadda
  0 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2023-02-03 19:49 UTC (permalink / raw)
  To: Krishna Yarlagadda
  Cc: broonie, peterhuewe, jgg, jarkko, krzysztof.kozlowski+dt,
	linux-spi, linux-tegra, linux-integrity, linux-kernel,
	thierry.reding, jonathanh, skomatineni, ldewangan

On Fri, Feb 3, 2023 at 7:02 AM Krishna Yarlagadda
<kyarlagadda@nvidia.com> wrote:
>
> Add "nvidia,wait-polling" flag to enable TCG TIS hardware flow control.

Tell me something that the diff doesn't.

>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  .../bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
> index 2c3cada75339..19d2b30cadbf 100644
> --- a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
> @@ -29,4 +29,10 @@ properties:
>      minimum: 0
>      maximum: 255
>
> +  nvidia,wait-polling:
> +    description:
> +      Enable TPM wait polling feature for QSPI as specified in TCG PC Client
> +      Specific TPM Interface Specification (TIS).
> +    $ref: /schemas/types.yaml#/definitions/flag

Why do you need this flag when you have a compatible that also
indicates you have a quirk.

If this a TPM feature, why is it enabled for every single SPI slave device?

If the fundamental issue is the controller only supports half-duplex,
why can't you just check that from the driver? Can't the SPI subsystem
tell you that the host controller is half-duplex? Though sometimes
that may be board level property I suppose. If so, define the h/w
quirk, not the driver mode in DT. Half-duplex is probably something
everyone could use, not just Nvidia.

Please discuss this series internally with the folks you marked as
maintainers. It has issues I'm sure they would have also pointed out.

Rob

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

* Re: [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect
  2023-02-03 13:01 ` [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect Krishna Yarlagadda
@ 2023-02-03 23:43   ` kernel test robot
  2023-02-04  9:48   ` kernel test robot
  2023-02-06 11:02   ` Paul Menzel
  2 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2023-02-03 23:43 UTC (permalink / raw)
  To: Krishna Yarlagadda, robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel
  Cc: llvm, oe-kbuild-all, thierry.reding, jonathanh, skomatineni,
	ldewangan, Krishna Yarlagadda

Hi Krishna,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus broonie-spi/for-next robh/for-next linus/master v6.2-rc6 next-20230203]
[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/dt-bindings-tpm-Add-compatible-for-Tegra-TPM/20230203-210314
patch link:    https://lore.kernel.org/r/20230203130133.32901-3-kyarlagadda%40nvidia.com
patch subject: [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect
config: x86_64-randconfig-k001 (https://download.01.org/0day-ci/archive/20230204/202302040739.VbsIprzH-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/825363f7e8d0d426c45bbba6cb3c5d9b79b7e6aa
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Krishna-Yarlagadda/dt-bindings-tpm-Add-compatible-for-Tegra-TPM/20230203-210314
        git checkout 825363f7e8d0d426c45bbba6cb3c5d9b79b7e6aa
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/char/tpm/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/char/tpm/tpm_tis_spi_tegra.c:18:
   In file included from drivers/char/tpm/tpm_tis_core.h:22:
   In file included from drivers/char/tpm/tpm.h:28:
   include/linux/tpm_eventlog.h:167:6: warning: variable 'mapping_size' set but not used [-Wunused-but-set-variable]
           int mapping_size;
               ^
>> drivers/char/tpm/tpm_tis_spi_tegra.c:23:5: warning: no previous prototype for function 'tpm_tis_spi_tegra_transfer' [-Wmissing-prototypes]
   int tpm_tis_spi_tegra_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
       ^
   drivers/char/tpm/tpm_tis_spi_tegra.c:23:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int tpm_tis_spi_tegra_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
   ^
   static 
   2 warnings generated.


vim +/tpm_tis_spi_tegra_transfer +23 drivers/char/tpm/tpm_tis_spi_tegra.c

    22	
  > 23	int tpm_tis_spi_tegra_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
    24				       u8 *in, const u8 *out)
    25	{
    26		struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
    27		int ret = 0;
    28		struct spi_message m;
    29		struct spi_transfer spi_xfer[3];
    30		u8 transfer_len;
    31	
    32		spi_bus_lock(phy->spi_device->master);
    33	
    34		while (len) {
    35			transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
    36	
    37			spi_message_init(&m);
    38			phy->iobuf[0] = (in ? 0x80 : 0) | (transfer_len - 1);
    39			phy->iobuf[1] = 0xd4;
    40			phy->iobuf[2] = addr >> 8;
    41			phy->iobuf[3] = addr;
    42	
    43			memset(&spi_xfer, 0, sizeof(spi_xfer));
    44	
    45			spi_xfer[0].tx_buf = phy->iobuf;
    46			spi_xfer[0].len = 1;
    47			spi_message_add_tail(&spi_xfer[0], &m);
    48	
    49			spi_xfer[1].tx_buf = phy->iobuf + 1;
    50			spi_xfer[1].len = 3;
    51			spi_message_add_tail(&spi_xfer[1], &m);
    52	
    53			if (out) {
    54				spi_xfer[2].tx_buf = &phy->iobuf[4];
    55				spi_xfer[2].rx_buf = NULL;
    56				memcpy(&phy->iobuf[4], out, transfer_len);
    57				out += transfer_len;
    58			}
    59			if (in) {
    60				spi_xfer[2].tx_buf = NULL;
    61				spi_xfer[2].rx_buf = &phy->iobuf[4];
    62			}
    63			spi_xfer[2].len = transfer_len;
    64			spi_message_add_tail(&spi_xfer[2], &m);
    65	
    66			reinit_completion(&phy->ready);
    67			ret = spi_sync_locked(phy->spi_device, &m);
    68			if (ret < 0)
    69				goto exit;
    70	
    71			if (in) {
    72				memcpy(in, &phy->iobuf[4], transfer_len);
    73				in += transfer_len;
    74			}
    75	
    76			len -= transfer_len;
    77		}
    78	
    79	exit:
    80		spi_bus_unlock(phy->spi_device->master);
    81		return ret;
    82	}
    83	

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

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

* Re: [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect
  2023-02-03 13:01 ` [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect Krishna Yarlagadda
  2023-02-03 23:43   ` kernel test robot
@ 2023-02-04  9:48   ` kernel test robot
  2023-02-06 11:02   ` Paul Menzel
  2 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2023-02-04  9:48 UTC (permalink / raw)
  To: Krishna Yarlagadda, robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel
  Cc: oe-kbuild-all, thierry.reding, jonathanh, skomatineni, ldewangan,
	Krishna Yarlagadda

Hi Krishna,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus broonie-spi/for-next robh/for-next linus/master v6.2-rc6 next-20230203]
[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/dt-bindings-tpm-Add-compatible-for-Tegra-TPM/20230203-210314
patch link:    https://lore.kernel.org/r/20230203130133.32901-3-kyarlagadda%40nvidia.com
patch subject: [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect
config: loongarch-buildonly-randconfig-r003-20230204 (https://download.01.org/0day-ci/archive/20230204/202302041737.v2JywR8Y-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/825363f7e8d0d426c45bbba6cb3c5d9b79b7e6aa
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Krishna-Yarlagadda/dt-bindings-tpm-Add-compatible-for-Tegra-TPM/20230203-210314
        git checkout 825363f7e8d0d426c45bbba6cb3c5d9b79b7e6aa
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/char/tpm/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/char/tpm/tpm_tis_spi_tegra.c:23:5: warning: no previous prototype for 'tpm_tis_spi_tegra_transfer' [-Wmissing-prototypes]
      23 | int tpm_tis_spi_tegra_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/tpm_tis_spi_tegra_transfer +23 drivers/char/tpm/tpm_tis_spi_tegra.c

    22	
  > 23	int tpm_tis_spi_tegra_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
    24				       u8 *in, const u8 *out)
    25	{
    26		struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
    27		int ret = 0;
    28		struct spi_message m;
    29		struct spi_transfer spi_xfer[3];
    30		u8 transfer_len;
    31	
    32		spi_bus_lock(phy->spi_device->master);
    33	
    34		while (len) {
    35			transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
    36	
    37			spi_message_init(&m);
    38			phy->iobuf[0] = (in ? 0x80 : 0) | (transfer_len - 1);
    39			phy->iobuf[1] = 0xd4;
    40			phy->iobuf[2] = addr >> 8;
    41			phy->iobuf[3] = addr;
    42	
    43			memset(&spi_xfer, 0, sizeof(spi_xfer));
    44	
    45			spi_xfer[0].tx_buf = phy->iobuf;
    46			spi_xfer[0].len = 1;
    47			spi_message_add_tail(&spi_xfer[0], &m);
    48	
    49			spi_xfer[1].tx_buf = phy->iobuf + 1;
    50			spi_xfer[1].len = 3;
    51			spi_message_add_tail(&spi_xfer[1], &m);
    52	
    53			if (out) {
    54				spi_xfer[2].tx_buf = &phy->iobuf[4];
    55				spi_xfer[2].rx_buf = NULL;
    56				memcpy(&phy->iobuf[4], out, transfer_len);
    57				out += transfer_len;
    58			}
    59			if (in) {
    60				spi_xfer[2].tx_buf = NULL;
    61				spi_xfer[2].rx_buf = &phy->iobuf[4];
    62			}
    63			spi_xfer[2].len = transfer_len;
    64			spi_message_add_tail(&spi_xfer[2], &m);
    65	
    66			reinit_completion(&phy->ready);
    67			ret = spi_sync_locked(phy->spi_device, &m);
    68			if (ret < 0)
    69				goto exit;
    70	
    71			if (in) {
    72				memcpy(in, &phy->iobuf[4], transfer_len);
    73				in += transfer_len;
    74			}
    75	
    76			len -= transfer_len;
    77		}
    78	
    79	exit:
    80		spi_bus_unlock(phy->spi_device->master);
    81		return ret;
    82	}
    83	

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

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

* Re: [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect
  2023-02-03 13:01 ` [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect Krishna Yarlagadda
  2023-02-03 23:43   ` kernel test robot
  2023-02-04  9:48   ` kernel test robot
@ 2023-02-06 11:02   ` Paul Menzel
  2023-02-06 13:19     ` Mark Brown
  2 siblings, 1 reply; 13+ messages in thread
From: Paul Menzel @ 2023-02-06 11:02 UTC (permalink / raw)
  To: Krishna Yarlagadda
  Cc: robh+dt, broonie, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel, thierry.reding, jonathanh, skomatineni, ldewangan

Dear Krishna,


Thank you for your patch.

Am 03.02.23 um 14:01 schrieb Krishna Yarlagadda:
> Tegra234 and Tegra241 chips have QSPI controller that supports TCG
> TIS hardware flow control. Since the controller only supports half
> duplex, sw wait polling method implemented in tpm_tis_spi does not
> suffice. Added extending driver to disable sw flow control and send

I’d use imperative mood and maybe use another verb:

Add dedicated Tegra driver …

> all transfers in single message.

Please add how you tested and benchmarked this patch.

> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>   drivers/char/tpm/Makefile            |   1 +
>   drivers/char/tpm/tpm_tis_spi.h       |   1 +
>   drivers/char/tpm/tpm_tis_spi_main.c  |   4 +-
>   drivers/char/tpm/tpm_tis_spi_tegra.c | 123 +++++++++++++++++++++++++++
>   4 files changed, 128 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/char/tpm/tpm_tis_spi_tegra.c
> 
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 0222b1ddb310..445b15493cb3 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_TCG_TIS_SYNQUACER) += tpm_tis_synquacer.o
>   
>   obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
>   tpm_tis_spi-y := tpm_tis_spi_main.o
> +tpm_tis_spi-y += tpm_tis_spi_tegra.o
>   tpm_tis_spi-$(CONFIG_TCG_TIS_SPI_CR50) += tpm_tis_spi_cr50.o
>   
>   obj-$(CONFIG_TCG_TIS_I2C_CR50) += tpm_tis_i2c_cr50.o
> diff --git a/drivers/char/tpm/tpm_tis_spi.h b/drivers/char/tpm/tpm_tis_spi.h
> index d0f66f6f1931..feaea14b428b 100644
> --- a/drivers/char/tpm/tpm_tis_spi.h
> +++ b/drivers/char/tpm/tpm_tis_spi.h
> @@ -31,6 +31,7 @@ extern int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy,
>   extern int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
>   				u8 *in, const u8 *out);
>   
> +extern int tegra_tpm_spi_probe(struct spi_device *spi);
>   #ifdef CONFIG_TCG_TIS_SPI_CR50
>   extern int cr50_spi_probe(struct spi_device *spi);
>   #else
> diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
> index a0963a3e92bd..5d4502a4461a 100644
> --- a/drivers/char/tpm/tpm_tis_spi_main.c
> +++ b/drivers/char/tpm/tpm_tis_spi_main.c
> @@ -198,7 +198,7 @@ static int tpm_tis_spi_driver_probe(struct spi_device *spi)
>   	const struct spi_device_id *spi_dev_id = spi_get_device_id(spi);
>   	tpm_tis_spi_probe_func probe_func;
>   
> -	probe_func = of_device_get_match_data(&spi->dev);
> +	probe_func = device_get_match_data(&spi->dev);
>   	if (!probe_func) {
>   		if (spi_dev_id) {
>   			probe_func = (tpm_tis_spi_probe_func)spi_dev_id->driver_data;
> @@ -227,6 +227,7 @@ static const struct spi_device_id tpm_tis_spi_id[] = {
>   	{ "tpm_tis_spi", (unsigned long)tpm_tis_spi_probe },
>   	{ "tpm_tis-spi", (unsigned long)tpm_tis_spi_probe },
>   	{ "cr50", (unsigned long)cr50_spi_probe },
> +	{ "tegra-tpm-spi", (unsigned long)tegra_tpm_spi_probe },
>   	{}
>   };
>   MODULE_DEVICE_TABLE(spi, tpm_tis_spi_id);
> @@ -236,6 +237,7 @@ static const struct of_device_id of_tis_spi_match[] = {
>   	{ .compatible = "infineon,slb9670", .data = tpm_tis_spi_probe },
>   	{ .compatible = "tcg,tpm_tis-spi", .data = tpm_tis_spi_probe },
>   	{ .compatible = "google,cr50", .data = cr50_spi_probe },
> +	{ .compatible = "nvidia,tegra-tpm-spi", .data = tegra_tpm_spi_probe },
>   	{}
>   };
>   MODULE_DEVICE_TABLE(of, of_tis_spi_match);
> diff --git a/drivers/char/tpm/tpm_tis_spi_tegra.c b/drivers/char/tpm/tpm_tis_spi_tegra.c
> new file mode 100644
> index 000000000000..23f20684513d
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_tis_spi_tegra.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 NVIDIA CORPORATION.
> + *
> + * This device driver implements TEGRA QSPI hw wait detection for chips
> + *
> + * It is based on tpm_tis_spi driver by Peter Huewe and Christophe Ricard.
> + */
> +
> +#include <linux/completion.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm.h>
> +#include <linux/spi/spi.h>
> +#include <linux/wait.h>
> +
> +#include "tpm_tis_core.h"
> +#include "tpm_tis_spi.h"
> +
> +#define MAX_SPI_FRAMESIZE 64
> +
> +int tpm_tis_spi_tegra_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;
> +	struct spi_message m;
> +	struct spi_transfer spi_xfer[3];
> +	u8 transfer_len;

Just use `unsigned int`? [1]

> +
> +	spi_bus_lock(phy->spi_device->master);
> +
> +	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)
> +			goto exit;
> +
> +		if (in) {
> +			memcpy(in, &phy->iobuf[4], transfer_len);
> +			in += transfer_len;
> +		}
> +
> +		len -= transfer_len;
> +	}
> +
> +exit:
> +	spi_bus_unlock(phy->spi_device->master);
> +	return ret;
> +}
> +
> +static int tpm_tis_spi_tegra_read_bytes(struct tpm_tis_data *data, u32 addr,
> +					u16 len, u8 *result,
> +					enum tpm_tis_io_mode io_mode)
> +{
> +	return tpm_tis_spi_tegra_transfer(data, addr, len, result, NULL);
> +}
> +
> +static int tpm_tis_spi_tegra_write_bytes(struct tpm_tis_data *data, u32 addr,
> +					 u16 len, const u8 *value,
> +					 enum tpm_tis_io_mode io_mode)
> +{
> +	return tpm_tis_spi_tegra_transfer(data, addr, len, NULL, value);
> +}
> +
> +static const struct tpm_tis_phy_ops tegra_tpm_spi_phy_ops = {
> +	.read_bytes = tpm_tis_spi_tegra_read_bytes,
> +	.write_bytes = tpm_tis_spi_tegra_write_bytes,
> +};
> +
> +int tegra_tpm_spi_probe(struct spi_device *dev)
> +{
> +	struct tpm_tis_spi_phy *phy;
> +	int irq;
> +
> +	phy = devm_kzalloc(&dev->dev, sizeof(struct tpm_tis_spi_phy),
> +			   GFP_KERNEL);
> +	if (!phy)
> +		return -ENOMEM;
> +
> +	phy->flow_control = NULL;
> +
> +	/* If the SPI device has an IRQ then use that */
> +	if (dev->irq > 0)
> +		irq = dev->irq;
> +	else
> +		irq = -1;

Use ternary operator?

     irq = dev->irq > 0 ? dev->irq : -1;

> +
> +	init_completion(&phy->ready);
> +	return tpm_tis_spi_init(dev, phy, irq, &tegra_tpm_spi_phy_ops);
> +}


Kind regards,

Paul


[1]: https://notabs.org/coding/smallIntsBigPenalty.htm

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

* Re: [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect
  2023-02-06 11:02   ` Paul Menzel
@ 2023-02-06 13:19     ` Mark Brown
  2023-02-06 16:10       ` Thierry Reding
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2023-02-06 13:19 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Krishna Yarlagadda, robh+dt, peterhuewe, jgg, jarkko,
	krzysztof.kozlowski+dt, linux-spi, linux-tegra, linux-integrity,
	linux-kernel, thierry.reding, jonathanh, skomatineni, ldewangan

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

On Mon, Feb 06, 2023 at 12:02:56PM +0100, Paul Menzel wrote:
> Am 03.02.23 um 14:01 schrieb Krishna Yarlagadda:

> > +	/* If the SPI device has an IRQ then use that */
> > +	if (dev->irq > 0)
> > +		irq = dev->irq;
> > +	else
> > +		irq = -1;

> Use ternary operator?

>     irq = dev->irq > 0 ? dev->irq : -1;

No, please write the code using normal conditional instructions.  This
isn't the IOCCC and the ternery operator is rarely a legibility aid.

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

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

* Re: [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect
  2023-02-06 13:19     ` Mark Brown
@ 2023-02-06 16:10       ` Thierry Reding
  0 siblings, 0 replies; 13+ messages in thread
From: Thierry Reding @ 2023-02-06 16:10 UTC (permalink / raw)
  To: Mark Brown
  Cc: Paul Menzel, Krishna Yarlagadda, robh+dt, peterhuewe, jgg,
	jarkko, krzysztof.kozlowski+dt, linux-spi, linux-tegra,
	linux-integrity, linux-kernel, jonathanh, skomatineni, ldewangan

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

On Mon, Feb 06, 2023 at 01:19:04PM +0000, Mark Brown wrote:
> On Mon, Feb 06, 2023 at 12:02:56PM +0100, Paul Menzel wrote:
> > Am 03.02.23 um 14:01 schrieb Krishna Yarlagadda:
> 
> > > +	/* If the SPI device has an IRQ then use that */
> > > +	if (dev->irq > 0)
> > > +		irq = dev->irq;
> > > +	else
> > > +		irq = -1;
> 
> > Use ternary operator?
> 
> >     irq = dev->irq > 0 ? dev->irq : -1;
> 
> No, please write the code using normal conditional instructions.  This
> isn't the IOCCC and the ternery operator is rarely a legibility aid.

Looks like the SPI core sets dev->irq = 0 for any error other than
-EPROBE_DEFER and the TPM TIS core checks for irq != 0 before trying to
setup that IRQ, so seems like we can just skip this altogether and pass
in dev->irq directly.

Thierry

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

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

* RE: [Patch V2 3/4] spi: dt-bindings: Add Tegra TPM wait polling flag
  2023-02-03 19:49   ` Rob Herring
@ 2023-02-23 16:26     ` Krishna Yarlagadda
  0 siblings, 0 replies; 13+ messages in thread
From: Krishna Yarlagadda @ 2023-02-23 16:26 UTC (permalink / raw)
  To: Rob Herring
  Cc: broonie, peterhuewe, jgg, jarkko, krzysztof.kozlowski+dt,
	linux-spi, linux-tegra, linux-integrity, linux-kernel,
	thierry.reding, Jonathan Hunter, Sowjanya Komatineni,
	Laxman Dewangan

> -----Original Message-----
> From: Rob Herring <robh+dt@kernel.org>
> Sent: 04 February 2023 01:20
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: 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;
> thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>;
> Sowjanya Komatineni <skomatineni@nvidia.com>; Laxman Dewangan
> <ldewangan@nvidia.com>
> Subject: Re: [Patch V2 3/4] spi: dt-bindings: Add Tegra TPM wait polling flag
> 
> External email: Use caution opening links or attachments
> 
> 
> On Fri, Feb 3, 2023 at 7:02 AM Krishna Yarlagadda
> <kyarlagadda@nvidia.com> wrote:
> >
> > Add "nvidia,wait-polling" flag to enable TCG TIS hardware flow control.
> 
> Tell me something that the diff doesn't.
> 
> >
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  .../bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-
> peripheral-props.yaml
> b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-
> props.yaml
> > index 2c3cada75339..19d2b30cadbf 100644
> > --- a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-
> peripheral-props.yaml
> > +++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-
> peripheral-props.yaml
> > @@ -29,4 +29,10 @@ properties:
> >      minimum: 0
> >      maximum: 255
> >
> > +  nvidia,wait-polling:
> > +    description:
> > +      Enable TPM wait polling feature for QSPI as specified in TCG PC Client
> > +      Specific TPM Interface Specification (TIS).
> > +    $ref: /schemas/types.yaml#/definitions/flag
> 
> Why do you need this flag when you have a compatible that also
> indicates you have a quirk.
> 
> If this a TPM feature, why is it enabled for every single SPI slave device?
> 
> If the fundamental issue is the controller only supports half-duplex,
> why can't you just check that from the driver? Can't the SPI subsystem
> tell you that the host controller is half-duplex? Though sometimes
> that may be board level property I suppose. If so, define the h/w
> quirk, not the driver mode in DT. Half-duplex is probably something
> everyone could use, not just Nvidia.
> 
> Please discuss this series internally with the folks you marked as
> maintainers. It has issues I'm sure they would have also pointed out.
> 
> Rob
QSPI is a multi-chip-select controller and HW wait polling is only for TPM
Both controller and device would need a flag/setting to identify support
for this feature. Using SPI controller flags and SPI device mode flags to
avoid device tree flags. Moved HW/alternate TPM flow control into existing
driver. No need of new compatible now.

KY

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

end of thread, other threads:[~2023-02-23 16:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-03 13:01 [Patch V2 0/4] Tegra TPM driver with hw flow control Krishna Yarlagadda
2023-02-03 13:01 ` [Patch V2 1/4] dt-bindings: tpm: Add compatible for Tegra TPM Krishna Yarlagadda
2023-02-03 18:57   ` Rob Herring
2023-02-03 13:01 ` [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect Krishna Yarlagadda
2023-02-03 23:43   ` kernel test robot
2023-02-04  9:48   ` kernel test robot
2023-02-06 11:02   ` Paul Menzel
2023-02-06 13:19     ` Mark Brown
2023-02-06 16:10       ` Thierry Reding
2023-02-03 13:01 ` [Patch V2 3/4] spi: dt-bindings: Add Tegra TPM wait polling flag Krishna Yarlagadda
2023-02-03 19:49   ` Rob Herring
2023-02-23 16:26     ` Krishna Yarlagadda
2023-02-03 13:01 ` [Patch V2 4/4] spi: tegra210-quad: Enable TPM wait polling Krishna Yarlagadda

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