mbox series

[v2,0/2] synquacer: add TPM support

Message ID 20200114141647.109347-1-ardb@kernel.org
Headers show
Series synquacer: add TPM support | expand

Message

Ard Biesheuvel Jan. 14, 2020, 2:16 p.m. UTC
This adds support for driving the TPM on Socionext SynQuacer platforms
using the existing driver for a memory mapped TIS frame.

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: masahisa.kojima@linaro.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

Ard Biesheuvel (2):
  dt-bindings: tpm-tis-mmio: add compatible string for SynQuacer TPM
  tpm: tis: add support for MMIO TPM on SynQuacer

 Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt |  1 +
 drivers/char/tpm/tpm_tis.c                                      | 50 +++++++++++++++++++-
 2 files changed, 49 insertions(+), 2 deletions(-)

Comments

Jarkko Sakkinen Jan. 23, 2020, 12:27 p.m. UTC | #1
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
Ard Biesheuvel Jan. 23, 2020, 12:29 p.m. UTC | #2
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.
Jarkko Sakkinen Jan. 30, 2020, 8:37 a.m. UTC | #3
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