[PATCH v3 0/2] synquacer: add TPM support

[PATCH v3 0/2] synquacer: add TPM support

[Masahisa Kojima]

This adds support for driving the TPM on Socionext SynQuacer platform
using the driver for a memory mapped TIS frame.

v3:
- prepare new module to handle TPM MMIO access on SynQuacer platform

v2:
- don't use read/write_bytes() to implement read/write16/32 since that uses
  the wrong address

Cc: jarkko.sakkinen@linux.intel.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: ardb@kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: peterhuewe@gmx.de
Cc: jgg@ziepe.ca

Masahisa Kojima (2):
  tpm: tis: add support for MMIO TPM on SynQuacer
  dt-bindings: Add SynQucer TPM MMIO as a trivial device

 .../devicetree/bindings/trivial-devices.yaml  |   2 +
 drivers/char/tpm/Kconfig                      |  12 ++
 drivers/char/tpm/Makefile                     |   1 +
 drivers/char/tpm/tpm_tis_synquacer.c          | 196 ++++++++++++++++++
 4 files changed, 211 insertions(+)
 create mode 100644 drivers/char/tpm/tpm_tis_synquacer.c

-- 
2.20.1

[PATCH v3 1/2] tpm: tis: add support for MMIO TPM on SynQuacer

[Masahisa Kojima]

When fitted, the SynQuacer platform exposes its SPI TPM via a MMIO
window that is backed by the SPI command sequencer in the SPI bus
controller. This arrangement has the limitation that only byte size
accesses are supported, and so we'll need to provide a separate module
that take this into account.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
 drivers/char/tpm/Kconfig             |  12 ++
 drivers/char/tpm/Makefile            |   1 +
 drivers/char/tpm/tpm_tis_synquacer.c | 196 +++++++++++++++++++++++++++
 3 files changed, 209 insertions(+)
 create mode 100644 drivers/char/tpm/tpm_tis_synquacer.c

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 58b4c573d176..a18c314da211 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -74,6 +74,18 @@ config TCG_TIS_SPI_CR50
 	  If you have a H1 secure module running Cr50 firmware on SPI bus,
 	  say Yes and it will be accessible from within Linux.
 
+config TCG_TIS_SYNQUACER
+	tristate "TPM Interface Specification 1.2 Interface / TPM 2.0 FIFO Interface (MMIO - SynQuacer)"
+	depends on ARCH_SYNQUACER
+	select TCG_TIS_CORE
+	help
+	  If you have a TPM security chip that is compliant with the
+	  TCG TIS 1.2 TPM specification (TPM1.2) or the TCG PTP FIFO
+	  specification (TPM2.0) say Yes and it will be accessible from
+	  within Linux on Socionext SynQuacer platform.
+	  To compile this driver as a module, choose  M here;
+	  the module will be called tpm_tis_synquacer.
+
 config TCG_TIS_I2C_ATMEL
 	tristate "TPM Interface Specification 1.2 Interface (I2C - Atmel)"
 	depends on I2C
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 9567e5197f74..84db4fb3a9c9 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -21,6 +21,7 @@ tpm-$(CONFIG_EFI) += eventlog/efi.o
 tpm-$(CONFIG_OF) += eventlog/of.o
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o
+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
diff --git a/drivers/char/tpm/tpm_tis_synquacer.c b/drivers/char/tpm/tpm_tis_synquacer.c
new file mode 100644
index 000000000000..51f0aedcedcc
--- /dev/null
+++ b/drivers/char/tpm/tpm_tis_synquacer.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Linaro Ltd.
+ *
+ * This device driver implements MMIO TPM on SynQuacer Platform.
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/kernel.h>
+#include "tpm.h"
+#include "tpm_tis_core.h"
+
+struct tpm_info {
+	struct resource res;
+	/* irq > 0 means: use irq $irq;
+	 * irq = 0 means: autoprobe for an irq;
+	 * irq = -1 means: no irq support
+	 */
+	int irq;
+};
+
+struct tpm_tis_tcg_phy {
+	struct tpm_tis_data priv;
+	void __iomem *iobase;
+};
+
+static inline struct tpm_tis_tcg_phy *to_tpm_tis_tcg_phy(struct tpm_tis_data *data)
+{
+	return container_of(data, struct tpm_tis_tcg_phy, priv);
+}
+
+static int tpm_tcg_read_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
+			      u8 *result)
+{
+	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
+
+	while (len--)
+		*result++ = ioread8(phy->iobase + addr);
+
+	return 0;
+}
+
+static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
+			       const u8 *value)
+{
+	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
+
+	while (len--)
+		iowrite8(*value++, phy->iobase + addr);
+
+	return 0;
+}
+
+static int tpm_tcg_read16_bw(struct tpm_tis_data *data, u32 addr, u16 *result)
+{
+	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
+
+	/*
+	 * Due to the limitation of SPI controller on SynQuacer,
+	 * 16/32 bits access must be done in byte-wise and descending order.
+	 */
+	*result = (ioread8(phy->iobase + addr + 1) << 8) |
+		  (ioread8(phy->iobase + addr));
+
+	return 0;
+}
+
+static int tpm_tcg_read32_bw(struct tpm_tis_data *data, u32 addr, u32 *result)
+{
+	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
+
+	/*
+	 * Due to the limitation of SPI controller on SynQuacer,
+	 * 16/32 bits access must be done in byte-wise and descending order.
+	 */
+	*result = (ioread8(phy->iobase + addr + 3) << 24) |
+		  (ioread8(phy->iobase + addr + 2) << 16) |
+		  (ioread8(phy->iobase + addr + 1) << 8) |
+		  (ioread8(phy->iobase + addr));
+
+	return 0;
+}
+
+static int tpm_tcg_write32_bw(struct tpm_tis_data *data, u32 addr, u32 value)
+{
+	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
+
+	/*
+	 * Due to the limitation of SPI controller on SynQuacer,
+	 * 16/32 bits access must be done in byte-wise and descending order.
+	 */
+	iowrite8(value >> 24, phy->iobase + addr + 3);
+	iowrite8(value >> 16, phy->iobase + addr + 2);
+	iowrite8(value >> 8, phy->iobase + addr + 1);
+	iowrite8(value, phy->iobase + addr);
+
+	return 0;
+}
+
+static const struct tpm_tis_phy_ops tpm_tcg_bw = {
+	.read_bytes	= tpm_tcg_read_bytes,
+	.write_bytes	= tpm_tcg_write_bytes,
+	.read16		= tpm_tcg_read16_bw,
+	.read32		= tpm_tcg_read32_bw,
+	.write32	= tpm_tcg_write32_bw,
+};
+
+static int tpm_tis_synquacer_init(struct device *dev, struct tpm_info *tpm_info)
+{
+	struct tpm_tis_tcg_phy *phy;
+	int irq = -1;
+
+	phy = devm_kzalloc(dev, sizeof(struct tpm_tis_tcg_phy), GFP_KERNEL);
+	if (phy == NULL)
+		return -ENOMEM;
+
+	phy->iobase = devm_ioremap_resource(dev, &tpm_info->res);
+	if (IS_ERR(phy->iobase))
+		return PTR_ERR(phy->iobase);
+
+	return tpm_tis_core_init(dev, &phy->priv, irq, &tpm_tcg_bw,
+				 ACPI_HANDLE(dev));
+}
+
+static SIMPLE_DEV_PM_OPS(tpm_tis_synquacer_pm, tpm_pm_suspend, tpm_tis_resume);
+
+static int tpm_tis_synquacer_probe(struct platform_device *pdev)
+{
+	struct tpm_info tpm_info = {};
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res == NULL) {
+		dev_err(&pdev->dev, "no memory resource defined\n");
+		return -ENODEV;
+	}
+	tpm_info.res = *res;
+
+	tpm_info.irq = -1;
+
+	return tpm_tis_synquacer_init(&pdev->dev, &tpm_info);
+}
+
+static int tpm_tis_synquacer_remove(struct platform_device *pdev)
+{
+	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
+
+	tpm_chip_unregister(chip);
+	tpm_tis_remove(chip);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id tis_synquacer_of_platform_match[] = {
+	{.compatible = "socionext,synquacer-tpm-mmio"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, tis_synquacer_of_platform_match);
+#endif
+
+static struct platform_driver tis_synquacer_drv = {
+	.probe = tpm_tis_synquacer_probe,
+	.remove = tpm_tis_synquacer_remove,
+	.driver = {
+		.name		= "tpm_tis_synquacer",
+		.pm		= &tpm_tis_synquacer_pm,
+		.of_match_table = of_match_ptr(tis_synquacer_of_platform_match),
+	},
+};
+
+static int __init init_tis_synquacer(void)
+{
+	int rc;
+
+	rc = platform_driver_register(&tis_synquacer_drv);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static void __exit cleanup_tis_synquacer(void)
+{
+	platform_driver_unregister(&tis_synquacer_drv);
+}
+
+module_init(init_tis_synquacer);
+module_exit(cleanup_tis_synquacer);
+MODULE_AUTHOR("Masahisa Kojima (masahisa.kojima@linaro.org)");
+MODULE_DESCRIPTION("TPM MMIO Driver for Socionext SynQuacer platform");
+MODULE_VERSION("2.0");
+MODULE_LICENSE("GPL");
-- 
2.20.1

[PATCH v3 2/2] dt-bindings: Add SynQucer TPM MMIO as a trivial device

[Masahisa Kojima]

Add a compatible string for the SynQuacer TPM to the binding for a
TPM exposed via a memory mapped TIS frame. The MMIO window behaves
slightly differently on this hardware, so it requires its own
identifier.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 4165352a590a..814148939e5a 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -328,6 +328,8 @@ properties:
           - silabs,si7020
             # Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
           - skyworks,sky81452
+            # Socionext SynQuacer TPM MMIO module
+          - socionext,synquacer-tpm-mmio
             # i2c serial eeprom  (24cxx)
           - st,24c256
             # Ambient Light Sensor with SMBUS/Two Wire Serial Interface
-- 
2.20.1

Re: [PATCH v3 1/2] tpm: tis: add support for MMIO TPM on SynQuacer

[Ard Biesheuvel]

On Wed, 8 Jul 2020 at 16:15, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
>
> When fitted, the SynQuacer platform exposes its SPI TPM via a MMIO
> window that is backed by the SPI command sequencer in the SPI bus
> controller. This arrangement has the limitation that only byte size
> accesses are supported, and so we'll need to provide a separate module
> that take this into account.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

Hello Masahisa,

This looks fine to me, but I won't be able to test it any time soon,
unfortunately.

One suggestion: could you allocate a ACPI _HID for this device, and
add support for it to the driver as well?

> ---
>  drivers/char/tpm/Kconfig             |  12 ++
>  drivers/char/tpm/Makefile            |   1 +
>  drivers/char/tpm/tpm_tis_synquacer.c | 196 +++++++++++++++++++++++++++
>  3 files changed, 209 insertions(+)
>  create mode 100644 drivers/char/tpm/tpm_tis_synquacer.c
>
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 58b4c573d176..a18c314da211 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -74,6 +74,18 @@ config TCG_TIS_SPI_CR50
>           If you have a H1 secure module running Cr50 firmware on SPI bus,
>           say Yes and it will be accessible from within Linux.
>
> +config TCG_TIS_SYNQUACER
> +       tristate "TPM Interface Specification 1.2 Interface / TPM 2.0 FIFO Interface (MMIO - SynQuacer)"
> +       depends on ARCH_SYNQUACER
> +       select TCG_TIS_CORE
> +       help
> +         If you have a TPM security chip that is compliant with the
> +         TCG TIS 1.2 TPM specification (TPM1.2) or the TCG PTP FIFO
> +         specification (TPM2.0) say Yes and it will be accessible from
> +         within Linux on Socionext SynQuacer platform.
> +         To compile this driver as a module, choose  M here;
> +         the module will be called tpm_tis_synquacer.
> +
>  config TCG_TIS_I2C_ATMEL
>         tristate "TPM Interface Specification 1.2 Interface (I2C - Atmel)"
>         depends on I2C
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 9567e5197f74..84db4fb3a9c9 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -21,6 +21,7 @@ tpm-$(CONFIG_EFI) += eventlog/efi.o
>  tpm-$(CONFIG_OF) += eventlog/of.o
>  obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
>  obj-$(CONFIG_TCG_TIS) += tpm_tis.o
> +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
> diff --git a/drivers/char/tpm/tpm_tis_synquacer.c b/drivers/char/tpm/tpm_tis_synquacer.c
> new file mode 100644
> index 000000000000..51f0aedcedcc
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_tis_synquacer.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Linaro Ltd.
> + *
> + * This device driver implements MMIO TPM on SynQuacer Platform.
> + */
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/kernel.h>
> +#include "tpm.h"
> +#include "tpm_tis_core.h"
> +
> +struct tpm_info {
> +       struct resource res;
> +       /* irq > 0 means: use irq $irq;
> +        * irq = 0 means: autoprobe for an irq;
> +        * irq = -1 means: no irq support
> +        */
> +       int irq;
> +};
> +
> +struct tpm_tis_tcg_phy {
> +       struct tpm_tis_data priv;
> +       void __iomem *iobase;
> +};
> +
> +static inline struct tpm_tis_tcg_phy *to_tpm_tis_tcg_phy(struct tpm_tis_data *data)
> +{
> +       return container_of(data, struct tpm_tis_tcg_phy, priv);
> +}
> +
> +static int tpm_tcg_read_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
> +                             u8 *result)
> +{
> +       struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> +
> +       while (len--)
> +               *result++ = ioread8(phy->iobase + addr);
> +
> +       return 0;
> +}
> +
> +static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
> +                              const u8 *value)
> +{
> +       struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> +
> +       while (len--)
> +               iowrite8(*value++, phy->iobase + addr);
> +
> +       return 0;
> +}
> +
> +static int tpm_tcg_read16_bw(struct tpm_tis_data *data, u32 addr, u16 *result)
> +{
> +       struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> +
> +       /*
> +        * Due to the limitation of SPI controller on SynQuacer,
> +        * 16/32 bits access must be done in byte-wise and descending order.
> +        */
> +       *result = (ioread8(phy->iobase + addr + 1) << 8) |
> +                 (ioread8(phy->iobase + addr));
> +
> +       return 0;
> +}
> +
> +static int tpm_tcg_read32_bw(struct tpm_tis_data *data, u32 addr, u32 *result)
> +{
> +       struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> +
> +       /*
> +        * Due to the limitation of SPI controller on SynQuacer,
> +        * 16/32 bits access must be done in byte-wise and descending order.
> +        */
> +       *result = (ioread8(phy->iobase + addr + 3) << 24) |
> +                 (ioread8(phy->iobase + addr + 2) << 16) |
> +                 (ioread8(phy->iobase + addr + 1) << 8) |
> +                 (ioread8(phy->iobase + addr));
> +
> +       return 0;
> +}
> +
> +static int tpm_tcg_write32_bw(struct tpm_tis_data *data, u32 addr, u32 value)
> +{
> +       struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> +
> +       /*
> +        * Due to the limitation of SPI controller on SynQuacer,
> +        * 16/32 bits access must be done in byte-wise and descending order.
> +        */
> +       iowrite8(value >> 24, phy->iobase + addr + 3);
> +       iowrite8(value >> 16, phy->iobase + addr + 2);
> +       iowrite8(value >> 8, phy->iobase + addr + 1);
> +       iowrite8(value, phy->iobase + addr);
> +
> +       return 0;
> +}
> +
> +static const struct tpm_tis_phy_ops tpm_tcg_bw = {
> +       .read_bytes     = tpm_tcg_read_bytes,
> +       .write_bytes    = tpm_tcg_write_bytes,
> +       .read16         = tpm_tcg_read16_bw,
> +       .read32         = tpm_tcg_read32_bw,
> +       .write32        = tpm_tcg_write32_bw,
> +};
> +
> +static int tpm_tis_synquacer_init(struct device *dev, struct tpm_info *tpm_info)
> +{
> +       struct tpm_tis_tcg_phy *phy;
> +       int irq = -1;
> +
> +       phy = devm_kzalloc(dev, sizeof(struct tpm_tis_tcg_phy), GFP_KERNEL);
> +       if (phy == NULL)
> +               return -ENOMEM;
> +
> +       phy->iobase = devm_ioremap_resource(dev, &tpm_info->res);
> +       if (IS_ERR(phy->iobase))
> +               return PTR_ERR(phy->iobase);
> +
> +       return tpm_tis_core_init(dev, &phy->priv, irq, &tpm_tcg_bw,
> +                                ACPI_HANDLE(dev));
> +}
> +
> +static SIMPLE_DEV_PM_OPS(tpm_tis_synquacer_pm, tpm_pm_suspend, tpm_tis_resume);
> +
> +static int tpm_tis_synquacer_probe(struct platform_device *pdev)
> +{
> +       struct tpm_info tpm_info = {};
> +       struct resource *res;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (res == NULL) {
> +               dev_err(&pdev->dev, "no memory resource defined\n");
> +               return -ENODEV;
> +       }
> +       tpm_info.res = *res;
> +
> +       tpm_info.irq = -1;
> +
> +       return tpm_tis_synquacer_init(&pdev->dev, &tpm_info);
> +}
> +
> +static int tpm_tis_synquacer_remove(struct platform_device *pdev)
> +{
> +       struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
> +
> +       tpm_chip_unregister(chip);
> +       tpm_tis_remove(chip);
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id tis_synquacer_of_platform_match[] = {
> +       {.compatible = "socionext,synquacer-tpm-mmio"},
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, tis_synquacer_of_platform_match);
> +#endif
> +
> +static struct platform_driver tis_synquacer_drv = {
> +       .probe = tpm_tis_synquacer_probe,
> +       .remove = tpm_tis_synquacer_remove,
> +       .driver = {
> +               .name           = "tpm_tis_synquacer",
> +               .pm             = &tpm_tis_synquacer_pm,
> +               .of_match_table = of_match_ptr(tis_synquacer_of_platform_match),
> +       },
> +};
> +
> +static int __init init_tis_synquacer(void)
> +{
> +       int rc;
> +
> +       rc = platform_driver_register(&tis_synquacer_drv);
> +       if (rc)
> +               return rc;
> +
> +       return 0;
> +}
> +
> +static void __exit cleanup_tis_synquacer(void)
> +{
> +       platform_driver_unregister(&tis_synquacer_drv);
> +}
> +
> +module_init(init_tis_synquacer);
> +module_exit(cleanup_tis_synquacer);
> +MODULE_AUTHOR("Masahisa Kojima (masahisa.kojima@linaro.org)");
> +MODULE_DESCRIPTION("TPM MMIO Driver for Socionext SynQuacer platform");
> +MODULE_VERSION("2.0");
> +MODULE_LICENSE("GPL");
> --
> 2.20.1
>

Re: [PATCH v3 0/2] synquacer: add TPM support

[Jarkko Sakkinen]

On Wed, Jul 08, 2020 at 10:14:22PM +0900, Masahisa Kojima wrote:
> This adds support for driving the TPM on Socionext SynQuacer platform
> using the driver for a memory mapped TIS frame.
> 
> v3:
> - prepare new module to handle TPM MMIO access on SynQuacer platform
> 
> v2:
> - don't use read/write_bytes() to implement read/write16/32 since that uses
>   the wrong address
> 
> Cc: jarkko.sakkinen@linux.intel.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: ardb@kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-integrity@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: peterhuewe@gmx.de
> Cc: jgg@ziepe.ca
> 
> Masahisa Kojima (2):
>   tpm: tis: add support for MMIO TPM on SynQuacer
>   dt-bindings: Add SynQucer TPM MMIO as a trivial device
> 
>  .../devicetree/bindings/trivial-devices.yaml  |   2 +
>  drivers/char/tpm/Kconfig                      |  12 ++
>  drivers/char/tpm/Makefile                     |   1 +
>  drivers/char/tpm/tpm_tis_synquacer.c          | 196 ++++++++++++++++++
>  4 files changed, 211 insertions(+)
>  create mode 100644 drivers/char/tpm/tpm_tis_synquacer.c
> 
> -- 
> 2.20.1
> 

Overally the code looks great. You've run it through checkpatch.pl?

/Jarkko

Re: [PATCH v3 1/2] tpm: tis: add support for MMIO TPM on SynQuacer

[Jarkko Sakkinen]

On Wed, Jul 08, 2020 at 10:14:23PM +0900, Masahisa Kojima wrote:
> When fitted, the SynQuacer platform exposes its SPI TPM via a MMIO
> window that is backed by the SPI command sequencer in the SPI bus
> controller. This arrangement has the limitation that only byte size
> accesses are supported, and so we'll need to provide a separate module
> that take this into account.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>  drivers/char/tpm/Kconfig             |  12 ++
>  drivers/char/tpm/Makefile            |   1 +
>  drivers/char/tpm/tpm_tis_synquacer.c | 196 +++++++++++++++++++++++++++
>  3 files changed, 209 insertions(+)
>  create mode 100644 drivers/char/tpm/tpm_tis_synquacer.c
> 
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 58b4c573d176..a18c314da211 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -74,6 +74,18 @@ config TCG_TIS_SPI_CR50
>  	  If you have a H1 secure module running Cr50 firmware on SPI bus,
>  	  say Yes and it will be accessible from within Linux.
>  
> +config TCG_TIS_SYNQUACER
> +	tristate "TPM Interface Specification 1.2 Interface / TPM 2.0 FIFO Interface (MMIO - SynQuacer)"
> +	depends on ARCH_SYNQUACER
> +	select TCG_TIS_CORE
> +	help
> +	  If you have a TPM security chip that is compliant with the
> +	  TCG TIS 1.2 TPM specification (TPM1.2) or the TCG PTP FIFO
> +	  specification (TPM2.0) say Yes and it will be accessible from
> +	  within Linux on Socionext SynQuacer platform.
> +	  To compile this driver as a module, choose  M here;
> +	  the module will be called tpm_tis_synquacer.
> +
>  config TCG_TIS_I2C_ATMEL
>  	tristate "TPM Interface Specification 1.2 Interface (I2C - Atmel)"
>  	depends on I2C
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 9567e5197f74..84db4fb3a9c9 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -21,6 +21,7 @@ tpm-$(CONFIG_EFI) += eventlog/efi.o
>  tpm-$(CONFIG_OF) += eventlog/of.o
>  obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
>  obj-$(CONFIG_TCG_TIS) += tpm_tis.o
> +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
> diff --git a/drivers/char/tpm/tpm_tis_synquacer.c b/drivers/char/tpm/tpm_tis_synquacer.c
> new file mode 100644
> index 000000000000..51f0aedcedcc
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_tis_synquacer.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Linaro Ltd.
> + *
> + * This device driver implements MMIO TPM on SynQuacer Platform.
> + */
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/kernel.h>
> +#include "tpm.h"
> +#include "tpm_tis_core.h"
> +
> +struct tpm_info {

struct tpm_tis_synquacer_info

> +	struct resource res;
> +	/* irq > 0 means: use irq $irq;
> +	 * irq = 0 means: autoprobe for an irq;
> +	 * irq = -1 means: no irq support
> +	 */
> +	int irq;
> +};
> +
> +struct tpm_tis_tcg_phy {

struct tpm_tis_synquacer_phy

> +	struct tpm_tis_data priv;
> +	void __iomem *iobase;
> +};
> +
> +static inline struct tpm_tis_tcg_phy *to_tpm_tis_tcg_phy(struct tpm_tis_data *data)
> +{
> +	return container_of(data, struct tpm_tis_tcg_phy, priv);
> +}
> +
> +static int tpm_tcg_read_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
> +			      u8 *result)

tpm_tis_synquacer_read_bytes (similar suggestions to similar stuff that
follows).

> +{
> +	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> +
> +	while (len--)
> +		*result++ = ioread8(phy->iobase + addr);
> +
> +	return 0;
> +}
> +
> +static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
> +			       const u8 *value)
> +{
> +	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> +
> +	while (len--)
> +		iowrite8(*value++, phy->iobase + addr);
> +
> +	return 0;
> +}
> +
> +static int tpm_tcg_read16_bw(struct tpm_tis_data *data, u32 addr, u16 *result)
> +{
> +	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> +
> +	/*
> +	 * Due to the limitation of SPI controller on SynQuacer,
> +	 * 16/32 bits access must be done in byte-wise and descending order.
> +	 */
> +	*result = (ioread8(phy->iobase + addr + 1) << 8) |
> +		  (ioread8(phy->iobase + addr));
> +
> +	return 0;
> +}
> +
> +static int tpm_tcg_read32_bw(struct tpm_tis_data *data, u32 addr, u32 *result)
> +{
> +	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> +
> +	/*
> +	 * Due to the limitation of SPI controller on SynQuacer,
> +	 * 16/32 bits access must be done in byte-wise and descending order.
> +	 */
> +	*result = (ioread8(phy->iobase + addr + 3) << 24) |
> +		  (ioread8(phy->iobase + addr + 2) << 16) |
> +		  (ioread8(phy->iobase + addr + 1) << 8) |
> +		  (ioread8(phy->iobase + addr));
> +
> +	return 0;
> +}
> +
> +static int tpm_tcg_write32_bw(struct tpm_tis_data *data, u32 addr, u32 value)
> +{
> +	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> +
> +	/*
> +	 * Due to the limitation of SPI controller on SynQuacer,
> +	 * 16/32 bits access must be done in byte-wise and descending order.
> +	 */
> +	iowrite8(value >> 24, phy->iobase + addr + 3);
> +	iowrite8(value >> 16, phy->iobase + addr + 2);
> +	iowrite8(value >> 8, phy->iobase + addr + 1);
> +	iowrite8(value, phy->iobase + addr);
> +
> +	return 0;
> +}
> +
> +static const struct tpm_tis_phy_ops tpm_tcg_bw = {
> +	.read_bytes	= tpm_tcg_read_bytes,
> +	.write_bytes	= tpm_tcg_write_bytes,
> +	.read16		= tpm_tcg_read16_bw,
> +	.read32		= tpm_tcg_read32_bw,
> +	.write32	= tpm_tcg_write32_bw,
> +};
> +
> +static int tpm_tis_synquacer_init(struct device *dev, struct tpm_info *tpm_info)
> +{
> +	struct tpm_tis_tcg_phy *phy;
> +	int irq = -1;
> +
> +	phy = devm_kzalloc(dev, sizeof(struct tpm_tis_tcg_phy), GFP_KERNEL);
> +	if (phy == NULL)
> +		return -ENOMEM;
> +
> +	phy->iobase = devm_ioremap_resource(dev, &tpm_info->res);
> +	if (IS_ERR(phy->iobase))
> +		return PTR_ERR(phy->iobase);
> +
> +	return tpm_tis_core_init(dev, &phy->priv, irq, &tpm_tcg_bw,
> +				 ACPI_HANDLE(dev));
> +}
> +
> +static SIMPLE_DEV_PM_OPS(tpm_tis_synquacer_pm, tpm_pm_suspend, tpm_tis_resume);
> +
> +static int tpm_tis_synquacer_probe(struct platform_device *pdev)
> +{
> +	struct tpm_info tpm_info = {};
> +	struct resource *res;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (res == NULL) {
> +		dev_err(&pdev->dev, "no memory resource defined\n");
> +		return -ENODEV;
> +	}
> +	tpm_info.res = *res;
> +
> +	tpm_info.irq = -1;
> +
> +	return tpm_tis_synquacer_init(&pdev->dev, &tpm_info);
> +}
> +
> +static int tpm_tis_synquacer_remove(struct platform_device *pdev)
> +{
> +	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
> +
> +	tpm_chip_unregister(chip);
> +	tpm_tis_remove(chip);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id tis_synquacer_of_platform_match[] = {
> +	{.compatible = "socionext,synquacer-tpm-mmio"},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, tis_synquacer_of_platform_match);
> +#endif
> +
> +static struct platform_driver tis_synquacer_drv = {
> +	.probe = tpm_tis_synquacer_probe,
> +	.remove = tpm_tis_synquacer_remove,
> +	.driver = {
> +		.name		= "tpm_tis_synquacer",
> +		.pm		= &tpm_tis_synquacer_pm,
> +		.of_match_table = of_match_ptr(tis_synquacer_of_platform_match),
> +	},
> +};
> +
> +static int __init init_tis_synquacer(void)

tpm_tis_synquacer_module_init

> +{
> +	int rc;
> +
> +	rc = platform_driver_register(&tis_synquacer_drv);
> +	if (rc)
> +		return rc;
> +
> +	return 0;
> +}
> +
> +static void __exit cleanup_tis_synquacer(void)

tpm_tis_synquacer_module_exit

> +{
> +	platform_driver_unregister(&tis_synquacer_drv);
> +}
> +
> +module_init(init_tis_synquacer);
> +module_exit(cleanup_tis_synquacer);
> +MODULE_AUTHOR("Masahisa Kojima (masahisa.kojima@linaro.org)");
> +MODULE_DESCRIPTION("TPM MMIO Driver for Socionext SynQuacer platform");
> +MODULE_VERSION("2.0");
> +MODULE_LICENSE("GPL");
> -- 
> 2.20.1
> 

/Jarkko

Re: [PATCH v3 0/2] synquacer: add TPM support

[Masahisa Kojima]

Hi Jakko,

> Overally the code looks great. You've run it through checkpatch.pl?

Yes, I have run checkpatch.pl and removed errors.

Regards,
Masahisa

On Thu, 9 Jul 2020 at 01:20, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Wed, Jul 08, 2020 at 10:14:22PM +0900, Masahisa Kojima wrote:
> > This adds support for driving the TPM on Socionext SynQuacer platform
> > using the driver for a memory mapped TIS frame.
> >
> > v3:
> > - prepare new module to handle TPM MMIO access on SynQuacer platform
> >
> > v2:
> > - don't use read/write_bytes() to implement read/write16/32 since that uses
> >   the wrong address
> >
> > Cc: jarkko.sakkinen@linux.intel.com
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: ardb@kernel.org
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-integrity@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: peterhuewe@gmx.de
> > Cc: jgg@ziepe.ca
> >
> > Masahisa Kojima (2):
> >   tpm: tis: add support for MMIO TPM on SynQuacer
> >   dt-bindings: Add SynQucer TPM MMIO as a trivial device
> >
> >  .../devicetree/bindings/trivial-devices.yaml  |   2 +
> >  drivers/char/tpm/Kconfig                      |  12 ++
> >  drivers/char/tpm/Makefile                     |   1 +
> >  drivers/char/tpm/tpm_tis_synquacer.c          | 196 ++++++++++++++++++
> >  4 files changed, 211 insertions(+)
> >  create mode 100644 drivers/char/tpm/tpm_tis_synquacer.c
> >
> > --
> > 2.20.1
> >
>
> Overally the code looks great. You've run it through checkpatch.pl?
>
> /Jarkko

Re: [PATCH v3 1/2] tpm: tis: add support for MMIO TPM on SynQuacer

[Masahisa Kojima]

Hi Ard, Jakko,

Thank you very much for your comments.
I will update code including ACPI  support, then send the next version.

Regards,
Masahisa

On Thu, 9 Jul 2020 at 01:30, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Wed, Jul 08, 2020 at 10:14:23PM +0900, Masahisa Kojima wrote:
> > When fitted, the SynQuacer platform exposes its SPI TPM via a MMIO
> > window that is backed by the SPI command sequencer in the SPI bus
> > controller. This arrangement has the limitation that only byte size
> > accesses are supported, and so we'll need to provide a separate module
> > that take this into account.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> >  drivers/char/tpm/Kconfig             |  12 ++
> >  drivers/char/tpm/Makefile            |   1 +
> >  drivers/char/tpm/tpm_tis_synquacer.c | 196 +++++++++++++++++++++++++++
> >  3 files changed, 209 insertions(+)
> >  create mode 100644 drivers/char/tpm/tpm_tis_synquacer.c
> >
> > diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> > index 58b4c573d176..a18c314da211 100644
> > --- a/drivers/char/tpm/Kconfig
> > +++ b/drivers/char/tpm/Kconfig
> > @@ -74,6 +74,18 @@ config TCG_TIS_SPI_CR50
> >         If you have a H1 secure module running Cr50 firmware on SPI bus,
> >         say Yes and it will be accessible from within Linux.
> >
> > +config TCG_TIS_SYNQUACER
> > +     tristate "TPM Interface Specification 1.2 Interface / TPM 2.0 FIFO Interface (MMIO - SynQuacer)"
> > +     depends on ARCH_SYNQUACER
> > +     select TCG_TIS_CORE
> > +     help
> > +       If you have a TPM security chip that is compliant with the
> > +       TCG TIS 1.2 TPM specification (TPM1.2) or the TCG PTP FIFO
> > +       specification (TPM2.0) say Yes and it will be accessible from
> > +       within Linux on Socionext SynQuacer platform.
> > +       To compile this driver as a module, choose  M here;
> > +       the module will be called tpm_tis_synquacer.
> > +
> >  config TCG_TIS_I2C_ATMEL
> >       tristate "TPM Interface Specification 1.2 Interface (I2C - Atmel)"
> >       depends on I2C
> > diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> > index 9567e5197f74..84db4fb3a9c9 100644
> > --- a/drivers/char/tpm/Makefile
> > +++ b/drivers/char/tpm/Makefile
> > @@ -21,6 +21,7 @@ tpm-$(CONFIG_EFI) += eventlog/efi.o
> >  tpm-$(CONFIG_OF) += eventlog/of.o
> >  obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
> >  obj-$(CONFIG_TCG_TIS) += tpm_tis.o
> > +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
> > diff --git a/drivers/char/tpm/tpm_tis_synquacer.c b/drivers/char/tpm/tpm_tis_synquacer.c
> > new file mode 100644
> > index 000000000000..51f0aedcedcc
> > --- /dev/null
> > +++ b/drivers/char/tpm/tpm_tis_synquacer.c
> > @@ -0,0 +1,196 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 Linaro Ltd.
> > + *
> > + * This device driver implements MMIO TPM on SynQuacer Platform.
> > + */
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/kernel.h>
> > +#include "tpm.h"
> > +#include "tpm_tis_core.h"
> > +
> > +struct tpm_info {
>
> struct tpm_tis_synquacer_info
>
> > +     struct resource res;
> > +     /* irq > 0 means: use irq $irq;
> > +      * irq = 0 means: autoprobe for an irq;
> > +      * irq = -1 means: no irq support
> > +      */
> > +     int irq;
> > +};
> > +
> > +struct tpm_tis_tcg_phy {
>
> struct tpm_tis_synquacer_phy
>
> > +     struct tpm_tis_data priv;
> > +     void __iomem *iobase;
> > +};
> > +
> > +static inline struct tpm_tis_tcg_phy *to_tpm_tis_tcg_phy(struct tpm_tis_data *data)
> > +{
> > +     return container_of(data, struct tpm_tis_tcg_phy, priv);
> > +}
> > +
> > +static int tpm_tcg_read_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
> > +                           u8 *result)
>
> tpm_tis_synquacer_read_bytes (similar suggestions to similar stuff that
> follows).
>
> > +{
> > +     struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> > +
> > +     while (len--)
> > +             *result++ = ioread8(phy->iobase + addr);
> > +
> > +     return 0;
> > +}
> > +
> > +static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
> > +                            const u8 *value)
> > +{
> > +     struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> > +
> > +     while (len--)
> > +             iowrite8(*value++, phy->iobase + addr);
> > +
> > +     return 0;
> > +}
> > +
> > +static int tpm_tcg_read16_bw(struct tpm_tis_data *data, u32 addr, u16 *result)
> > +{
> > +     struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> > +
> > +     /*
> > +      * Due to the limitation of SPI controller on SynQuacer,
> > +      * 16/32 bits access must be done in byte-wise and descending order.
> > +      */
> > +     *result = (ioread8(phy->iobase + addr + 1) << 8) |
> > +               (ioread8(phy->iobase + addr));
> > +
> > +     return 0;
> > +}
> > +
> > +static int tpm_tcg_read32_bw(struct tpm_tis_data *data, u32 addr, u32 *result)
> > +{
> > +     struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> > +
> > +     /*
> > +      * Due to the limitation of SPI controller on SynQuacer,
> > +      * 16/32 bits access must be done in byte-wise and descending order.
> > +      */
> > +     *result = (ioread8(phy->iobase + addr + 3) << 24) |
> > +               (ioread8(phy->iobase + addr + 2) << 16) |
> > +               (ioread8(phy->iobase + addr + 1) << 8) |
> > +               (ioread8(phy->iobase + addr));
> > +
> > +     return 0;
> > +}
> > +
> > +static int tpm_tcg_write32_bw(struct tpm_tis_data *data, u32 addr, u32 value)
> > +{
> > +     struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> > +
> > +     /*
> > +      * Due to the limitation of SPI controller on SynQuacer,
> > +      * 16/32 bits access must be done in byte-wise and descending order.
> > +      */
> > +     iowrite8(value >> 24, phy->iobase + addr + 3);
> > +     iowrite8(value >> 16, phy->iobase + addr + 2);
> > +     iowrite8(value >> 8, phy->iobase + addr + 1);
> > +     iowrite8(value, phy->iobase + addr);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct tpm_tis_phy_ops tpm_tcg_bw = {
> > +     .read_bytes     = tpm_tcg_read_bytes,
> > +     .write_bytes    = tpm_tcg_write_bytes,
> > +     .read16         = tpm_tcg_read16_bw,
> > +     .read32         = tpm_tcg_read32_bw,
> > +     .write32        = tpm_tcg_write32_bw,
> > +};
> > +
> > +static int tpm_tis_synquacer_init(struct device *dev, struct tpm_info *tpm_info)
> > +{
> > +     struct tpm_tis_tcg_phy *phy;
> > +     int irq = -1;
> > +
> > +     phy = devm_kzalloc(dev, sizeof(struct tpm_tis_tcg_phy), GFP_KERNEL);
> > +     if (phy == NULL)
> > +             return -ENOMEM;
> > +
> > +     phy->iobase = devm_ioremap_resource(dev, &tpm_info->res);
> > +     if (IS_ERR(phy->iobase))
> > +             return PTR_ERR(phy->iobase);
> > +
> > +     return tpm_tis_core_init(dev, &phy->priv, irq, &tpm_tcg_bw,
> > +                              ACPI_HANDLE(dev));
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(tpm_tis_synquacer_pm, tpm_pm_suspend, tpm_tis_resume);
> > +
> > +static int tpm_tis_synquacer_probe(struct platform_device *pdev)
> > +{
> > +     struct tpm_info tpm_info = {};
> > +     struct resource *res;
> > +
> > +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +     if (res == NULL) {
> > +             dev_err(&pdev->dev, "no memory resource defined\n");
> > +             return -ENODEV;
> > +     }
> > +     tpm_info.res = *res;
> > +
> > +     tpm_info.irq = -1;
> > +
> > +     return tpm_tis_synquacer_init(&pdev->dev, &tpm_info);
> > +}
> > +
> > +static int tpm_tis_synquacer_remove(struct platform_device *pdev)
> > +{
> > +     struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
> > +
> > +     tpm_chip_unregister(chip);
> > +     tpm_tis_remove(chip);
> > +
> > +     return 0;
> > +}
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id tis_synquacer_of_platform_match[] = {
> > +     {.compatible = "socionext,synquacer-tpm-mmio"},
> > +     {},
> > +};
> > +MODULE_DEVICE_TABLE(of, tis_synquacer_of_platform_match);
> > +#endif
> > +
> > +static struct platform_driver tis_synquacer_drv = {
> > +     .probe = tpm_tis_synquacer_probe,
> > +     .remove = tpm_tis_synquacer_remove,
> > +     .driver = {
> > +             .name           = "tpm_tis_synquacer",
> > +             .pm             = &tpm_tis_synquacer_pm,
> > +             .of_match_table = of_match_ptr(tis_synquacer_of_platform_match),
> > +     },
> > +};
> > +
> > +static int __init init_tis_synquacer(void)
>
> tpm_tis_synquacer_module_init
>
> > +{
> > +     int rc;
> > +
> > +     rc = platform_driver_register(&tis_synquacer_drv);
> > +     if (rc)
> > +             return rc;
> > +
> > +     return 0;
> > +}
> > +
> > +static void __exit cleanup_tis_synquacer(void)
>
> tpm_tis_synquacer_module_exit
>
> > +{
> > +     platform_driver_unregister(&tis_synquacer_drv);
> > +}
> > +
> > +module_init(init_tis_synquacer);
> > +module_exit(cleanup_tis_synquacer);
> > +MODULE_AUTHOR("Masahisa Kojima (masahisa.kojima@linaro.org)");
> > +MODULE_DESCRIPTION("TPM MMIO Driver for Socionext SynQuacer platform");
> > +MODULE_VERSION("2.0");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.20.1
> >
>
> /Jarkko

Re: [PATCH v3 0/2] synquacer: add TPM support

[Masahisa Kojima]

Hi Jarkko,

> Hi Jakko,
I apologize for mis-spelling of your name.
Same mistake also appears in my another reply to "[PATCH v3 1/2] tpm:
tis: add support for MMIO TPM on SynQuacer"

On Thu, 9 Jul 2020 at 08:34, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
>
> Hi Jakko,
>
> > Overally the code looks great. You've run it through checkpatch.pl?
>
> Yes, I have run checkpatch.pl and removed errors.
>
> Regards,
> Masahisa
>
> On Thu, 9 Jul 2020 at 01:20, Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Wed, Jul 08, 2020 at 10:14:22PM +0900, Masahisa Kojima wrote:
> > > This adds support for driving the TPM on Socionext SynQuacer platform
> > > using the driver for a memory mapped TIS frame.
> > >
> > > v3:
> > > - prepare new module to handle TPM MMIO access on SynQuacer platform
> > >
> > > v2:
> > > - don't use read/write_bytes() to implement read/write16/32 since that uses
> > >   the wrong address
> > >
> > > Cc: jarkko.sakkinen@linux.intel.com
> > > Cc: linux-arm-kernel@lists.infradead.org
> > > Cc: ardb@kernel.org
> > > Cc: devicetree@vger.kernel.org
> > > Cc: linux-integrity@vger.kernel.org
> > > Cc: linux-kernel@vger.kernel.org
> > > Cc: peterhuewe@gmx.de
> > > Cc: jgg@ziepe.ca
> > >
> > > Masahisa Kojima (2):
> > >   tpm: tis: add support for MMIO TPM on SynQuacer
> > >   dt-bindings: Add SynQucer TPM MMIO as a trivial device
> > >
> > >  .../devicetree/bindings/trivial-devices.yaml  |   2 +
> > >  drivers/char/tpm/Kconfig                      |  12 ++
> > >  drivers/char/tpm/Makefile                     |   1 +
> > >  drivers/char/tpm/tpm_tis_synquacer.c          | 196 ++++++++++++++++++
> > >  4 files changed, 211 insertions(+)
> > >  create mode 100644 drivers/char/tpm/tpm_tis_synquacer.c
> > >
> > > --
> > > 2.20.1
> > >
> >
> > Overally the code looks great. You've run it through checkpatch.pl?
> >
> > /Jarkko

Re: [PATCH v3 0/2] synquacer: add TPM support

[Jarkko Sakkinen]

On Thu, Jul 09, 2020 at 08:34:27AM +0900, Masahisa Kojima wrote:
> Hi Jakko,
> 
> > Overally the code looks great. You've run it through checkpatch.pl?
> 
> Yes, I have run checkpatch.pl and removed errors.

OK, cool.

/Jarkko

Re: [PATCH v3 0/2] synquacer: add TPM support

[Jarkko Sakkinen]

On Thu, Jul 09, 2020 at 08:46:35AM +0900, Masahisa Kojima wrote:
> Hi Jarkko,
> 
> > Hi Jakko,
> I apologize for mis-spelling of your name.
> Same mistake also appears in my another reply to "[PATCH v3 1/2] tpm:
> tis: add support for MMIO TPM on SynQuacer"

No worries :-)

/Jarkko

Re: [PATCH v3 2/2] dt-bindings: Add SynQucer TPM MMIO as a trivial device

[Rob Herring]

On Wed, 08 Jul 2020 22:14:24 +0900, Masahisa Kojima wrote:
> Add a compatible string for the SynQuacer TPM to the binding for a
> TPM exposed via a memory mapped TIS frame. The MMIO window behaves
> slightly differently on this hardware, so it requires its own
> identifier.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>  Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>