diff mbox series

[2/4] tpm: tegra: Support SPI tpm wait state detect

Message ID 20230202161750.21210-3-kyarlagadda@nvidia.com (mailing list archive)
State Superseded
Headers show
Series Tegra TPM driver with hw flow control | expand

Commit Message

Krishna Yarlagadda Feb. 2, 2023, 4:17 p.m. UTC 
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

Comments

kernel test robot Feb. 2, 2023, 8:08 p.m. UTC | #1 
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-20230202]
[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-002113
patch link:    https://lore.kernel.org/r/20230202161750.21210-3-kyarlagadda%40nvidia.com
patch subject: [PATCH 2/4] tpm: tegra: Support SPI tpm wait state detect
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230203/202302030428.MRyuAj03-lkp@intel.com/config)
compiler: sparc64-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/9a454b022e5273e483b968f1998e0b177e71fcb2
        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-002113
        git checkout 9a454b022e5273e483b968f1998e0b177e71fcb2
        # 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=sparc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc 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
Jarkko Sakkinen Feb. 8, 2023, 2:39 a.m. UTC | #2 
On Thu, Feb 02, 2023 at 09:47:48PM +0530, Krishna Yarlagadda wrote:
> 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

s/Added/Add/

> all transfers in single message.

I've never heard the term "extended driver" before.

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

Why do you need to initialize ret?

> +	struct spi_message m;
> +	struct spi_transfer spi_xfer[3];
> +	u8 transfer_len;

Please reorder declarations to reverse christmas tree order.

> +
> +	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;
> +		}

Empty line here.

> +		if (in) {
> +			spi_xfer[2].tx_buf = NULL;
> +			spi_xfer[2].rx_buf = &phy->iobuf[4];
> +		}

Ditto.

> +		spi_xfer[2].len = transfer_len;
> +		spi_message_add_tail(&spi_xfer[2], &m);
> +
> +		reinit_completion(&phy->ready);

Ditto.

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

BR, Jarkko
diff mbox series

Patch

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);
+}