| Message ID | 20200114141647.109347-3-ardb@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | synquacer: add TPM support | expand |
On Tue, 2020-01-14 at 15:16 +0100, Ard Biesheuvel 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 set > of read and write accessors that take this into account. What is SynQuacer platform? I'm also missing a resolution why tpm_tis.c is extended to handle both and not add tpm_tis_something.c instead. It does not follow the pattern we have in place (e.g. look up tpm_tis_spi.c). /Jarkko
On Thu, 23 Jan 2020 at 13:27, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > > On Tue, 2020-01-14 at 15:16 +0100, Ard Biesheuvel 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 set > > of read and write accessors that take this into account. > > What is SynQuacer platform? > It is an arm64 SoC manufactured by Socionext. > I'm also missing a resolution why tpm_tis.c is extended to handle both > and not add tpm_tis_something.c instead. It does not follow the pattern > we have in place (e.g. look up tpm_tis_spi.c). > We could easily do that instead, if preferred. It's just that it would duplicate a bit of code.
On Thu, 2020-01-23 at 13:29 +0100, Ard Biesheuvel wrote: > On Thu, 23 Jan 2020 at 13:27, Jarkko Sakkinen > <jarkko.sakkinen@linux.intel.com> wrote: > > On Tue, 2020-01-14 at 15:16 +0100, Ard Biesheuvel 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 set > > > of read and write accessors that take this into account. > > > > What is SynQuacer platform? > > > > It is an arm64 SoC manufactured by Socionext. > > > I'm also missing a resolution why tpm_tis.c is extended to handle both > > and not add tpm_tis_something.c instead. It does not follow the pattern > > we have in place (e.g. look up tpm_tis_spi.c). > > > > We could easily do that instead, if preferred. It's just that it would > duplicate a bit of code. I'm fine with that. Overally I think it is cleaner flow. /Jarkko
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index e7df342a317d..2466295fcfe8 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -32,6 +32,7 @@ struct tpm_info { struct resource res; + const struct tpm_tis_phy_ops *ops; /* irq > 0 means: use irq $irq; * irq = 0 means: autoprobe for an irq; * irq = -1 means: no irq support @@ -186,6 +187,48 @@ static const struct tpm_tis_phy_ops tpm_tcg = { .write32 = tpm_tcg_write32, }; +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); + + *result = (ioread8(phy->iobase + addr)) | + (ioread8(phy->iobase + addr + 1) << 8); + + 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); + + *result = (ioread8(phy->iobase + addr)) | + (ioread8(phy->iobase + addr + 1) << 8) | + (ioread8(phy->iobase + addr + 2) << 16) | + (ioread8(phy->iobase + addr + 3) << 24); + + 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); + + iowrite8(value, phy->iobase + addr); + iowrite8(value >> 8, phy->iobase + addr + 1); + iowrite8(value >> 16, phy->iobase + addr + 2); + iowrite8(value >> 24, phy->iobase + addr + 3); + + 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_init(struct device *dev, struct tpm_info *tpm_info) { struct tpm_tis_tcg_phy *phy; @@ -210,7 +253,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info) if (itpm || is_itpm(ACPI_COMPANION(dev))) phy->priv.flags |= TPM_TIS_ITPM_WORKAROUND; - return tpm_tis_core_init(dev, &phy->priv, irq, &tpm_tcg, + return tpm_tis_core_init(dev, &phy->priv, irq, tpm_info->ops, ACPI_HANDLE(dev)); } @@ -219,7 +262,7 @@ static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume); static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev, const struct pnp_device_id *pnp_id) { - struct tpm_info tpm_info = {}; + struct tpm_info tpm_info = { .ops = &tpm_tcg }; struct resource *res; res = pnp_get_resource(pnp_dev, IORESOURCE_MEM, 0); @@ -295,6 +338,8 @@ static int tpm_tis_plat_probe(struct platform_device *pdev) tpm_info.irq = 0; } + tpm_info.ops = of_device_get_match_data(&pdev->dev) ?: &tpm_tcg; + return tpm_tis_init(&pdev->dev, &tpm_info); } @@ -311,6 +356,7 @@ static int tpm_tis_plat_remove(struct platform_device *pdev) #ifdef CONFIG_OF static const struct of_device_id tis_of_platform_match[] = { {.compatible = "tcg,tpm-tis-mmio"}, + {.compatible = "socionext,synquacer-tpm-mmio", .data = &tpm_tcg_bw}, {}, }; MODULE_DEVICE_TABLE(of, tis_of_platform_match);
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 set of read and write accessors that take this into account. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- drivers/char/tpm/tpm_tis.c | 50 +++++++++++++++++++- 1 file changed, 48 insertions(+), 2 deletions(-)