[v4,3/6] tpm: tpm_tis_spi: Add a pre-transfer callback
diff mbox series

Message ID 20190812223622.73297-4-swboyd@chromium.org
State New
Headers show
Series
  • tpm: Add driver for cr50
Related show

Commit Message

Stephen Boyd Aug. 12, 2019, 10:36 p.m. UTC
Cr50 firmware has a requirement to wait for the TPM to wakeup before
sending commands over the SPI bus. Otherwise, the firmware could be in
deep sleep and not respond. Add a hook to tpm_tis_spi_transfer() before
we start a SPI transfer so we can keep track of the last time the TPM
driver accessed the SPI bus.

Cc: Andrey Pronin <apronin@chromium.org>
Cc: Duncan Laurie <dlaurie@chromium.org>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Guenter Roeck <groeck@chromium.org>
Cc: Alexander Steffen <Alexander.Steffen@infineon.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/char/tpm/tpm_tis_spi.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jarkko Sakkinen Aug. 19, 2019, 4:35 p.m. UTC | #1
On Mon, Aug 12, 2019 at 03:36:19PM -0700, Stephen Boyd wrote:
> Cr50 firmware has a requirement to wait for the TPM to wakeup before
> sending commands over the SPI bus. Otherwise, the firmware could be in
> deep sleep and not respond. Add a hook to tpm_tis_spi_transfer() before
> we start a SPI transfer so we can keep track of the last time the TPM
> driver accessed the SPI bus.
> 
> Cc: Andrey Pronin <apronin@chromium.org>
> Cc: Duncan Laurie <dlaurie@chromium.org>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Alexander Steffen <Alexander.Steffen@infineon.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/char/tpm/tpm_tis_spi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c
> index 819602e85b34..93f49b1941f0 100644
> --- a/drivers/char/tpm/tpm_tis_spi.c
> +++ b/drivers/char/tpm/tpm_tis_spi.c
> @@ -44,6 +44,7 @@ struct tpm_tis_spi_phy {
>  	struct spi_device *spi_device;
>  	int (*flow_control)(struct tpm_tis_spi_phy *phy,
>  			    struct spi_transfer *xfer);
> +	void (*pre_transfer)(struct tpm_tis_spi_phy *phy);

A callback should have somewhat well defined purpose. A callback named
as pre_transfer() could have any purpose.

/Jarkko
Stephen Boyd Aug. 19, 2019, 5:07 p.m. UTC | #2
Quoting Jarkko Sakkinen (2019-08-19 09:35:05)
> On Mon, Aug 12, 2019 at 03:36:19PM -0700, Stephen Boyd wrote:
> > Cr50 firmware has a requirement to wait for the TPM to wakeup before
> > sending commands over the SPI bus. Otherwise, the firmware could be in
> > deep sleep and not respond. Add a hook to tpm_tis_spi_transfer() before
> > we start a SPI transfer so we can keep track of the last time the TPM
> > driver accessed the SPI bus.
> > 
> > Cc: Andrey Pronin <apronin@chromium.org>
> > Cc: Duncan Laurie <dlaurie@chromium.org>
> > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Guenter Roeck <groeck@chromium.org>
> > Cc: Alexander Steffen <Alexander.Steffen@infineon.com>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >  drivers/char/tpm/tpm_tis_spi.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c
> > index 819602e85b34..93f49b1941f0 100644
> > --- a/drivers/char/tpm/tpm_tis_spi.c
> > +++ b/drivers/char/tpm/tpm_tis_spi.c
> > @@ -44,6 +44,7 @@ struct tpm_tis_spi_phy {
> >       struct spi_device *spi_device;
> >       int (*flow_control)(struct tpm_tis_spi_phy *phy,
> >                           struct spi_transfer *xfer);
> > +     void (*pre_transfer)(struct tpm_tis_spi_phy *phy);
> 
> A callback should have somewhat well defined purpose. A callback named
> as pre_transfer() could have any purpose.
> 

Any name is fine for me. Any suggestions?
Jarkko Sakkinen Aug. 21, 2019, 7:11 p.m. UTC | #3
On Mon, Aug 19, 2019 at 10:07:41AM -0700, Stephen Boyd wrote:
> Any name is fine for me. Any suggestions?

What if just add @ready to struct tpm_tis_spi_phy add drop this patch
altogether?

It is only used only by CR50 but I think it is less of an overkill than
adding a callback.

/Jarkko
Stephen Boyd Aug. 21, 2019, 9:44 p.m. UTC | #4
Quoting Jarkko Sakkinen (2019-08-21 12:11:31)
> On Mon, Aug 19, 2019 at 10:07:41AM -0700, Stephen Boyd wrote:
> > Any name is fine for me. Any suggestions?
> 
> What if just add @ready to struct tpm_tis_spi_phy add drop this patch
> altogether?
> 
> It is only used only by CR50 but I think it is less of an overkill than
> adding a callback.
> 

Ok, sounds good.

Patch
diff mbox series

diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c
index 819602e85b34..93f49b1941f0 100644
--- a/drivers/char/tpm/tpm_tis_spi.c
+++ b/drivers/char/tpm/tpm_tis_spi.c
@@ -44,6 +44,7 @@  struct tpm_tis_spi_phy {
 	struct spi_device *spi_device;
 	int (*flow_control)(struct tpm_tis_spi_phy *phy,
 			    struct spi_transfer *xfer);
+	void (*pre_transfer)(struct tpm_tis_spi_phy *phy);
 	u8 *iobuf;
 };
 
@@ -129,6 +130,8 @@  static int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
 
 		spi_message_init(&m);
 		spi_message_add_tail(&spi_xfer, &m);
+		if (phy->pre_transfer)
+			phy->pre_transfer(phy);
 		ret = spi_sync_locked(phy->spi_device, &m);
 		if (ret < 0)
 			goto exit;