Re: [PATCH v7 0/6] tpm: Add driver for cr50

From: Jarkko Sakkinen
Date: Sun Oct 06 2019 - 18:39:22 EST


On Fri, Sep 20, 2019 at 11:32:34AM -0700, Stephen Boyd wrote:
> This patch series adds support for the H1 secure microcontroller
> running cr50 firmware found on various recent Chromebooks. This driver
> is necessary to boot into a ChromeOS userspace environment. It
> implements support for several functions, including TPM-like
> functionality over a SPI interface.
>
> The last time this was series sent looks to be [1]. I've looked over the
> patches and review comments and tried to address any feedback that
> Andrey didn't address (really minor things like newlines). I've reworked
> the patches from the last version to layer on top of the existing TPM
> TIS SPI implementation in tpm_tis_spi.c. Hopefully this is more
> palatable than combining the two drivers together into one file.
>
> Please review so we can get the approach to supporting this device
> sorted out.
>
> [1] https://lkml.kernel.org/r/1469757314-116169-1-git-send-email-apronin@xxxxxxxxxxxx
>
> TODO:
> * Add a patch to spit out WARN_ON() when TPM is suspended and some
> kernel code attempts to use it
> * Rework the i2c driver per Alexander's comments on v2
>
> Changes from v6 (https://lkml.kernel.org/r/20190829224110.91103-1-swboyd@xxxxxxxxxxxx):
> * Two new patches to cleanup includes and module usage
> * Moved cr50 C file to tpm_tis_spi_cr50.c
> * Used the tpm_tis_spi_mod target approach to make the module work
> * Brought back Kconfig option to allow user to disable cr50 code
> * Rebased to v5.3
>
> Changes from v5 (https://lkml.kernel.org/r/20190828082150.42194-1-swboyd@xxxxxxxxxxxx):
> * Picked up Jarkko's ack/review tags
> * Fixed bug with irqs happening before completion is initialized
> * Dropped is_cr50 bool
> * Moved wake_after to tpm_tis_spi struct
> * Changed authorship of main cr50 patch to Andrey as I'm just shuffling
> code here
>
> Changes from v4 (https://lkml.kernel.org/r/20190812223622.73297-1-swboyd@xxxxxxxxxxxx):
> * Dropped the 'pre-transfer' hook patch and added a 'ready' member instead
> * Combined cr50_spi and tpm_tis_spi into one kernel module
> * Introduced a swizzle in tpm_tis_spi probe routine to jump to cr50
> probe path
> * Moved binding to start of the thread
> * Picked up Jarkko reviewed-by tag on new flag for suspend/resume
> * Added a comment to flow control patch indicating what it's all about
>
> Changes from v3:
> * Split out hooks into separate patches
> * Update commit text to not say "libify"
> * Collapse if statement into one for first patch
> * Update commit text on first patch to mention flag
> * Drop TIS_IS_CR50 as it's unused
>
> Changes from v2:
> * Sent khwrng thread patch separately
> * New patch to expose TPM SPI functionality from tpm_tis_spi.c
> * Usage of that new patch in cr50 SPI driver
> * Drop i2c version of cr50 SPI driver for now (will resend later)
> * New patch to add a TPM chip flag indicating TPM shouldn't be reset
> over suspend. Allows us to get rid of the cr50 suspend/resume functions
> that are mostly generic
>
> Changes from v1:
> * Dropped symlink and sysfs patches
> * Removed 'is_suspended' bits
> * Added new patch to freeze khwrng thread
> * Moved binding to google,cr50.txt and added Reviewed-by tag from Rob
>
> Cc: Andrey Pronin <apronin@xxxxxxxxxxxx>
> Cc: Duncan Laurie <dlaurie@xxxxxxxxxxxx>
> Cc: Jason Gunthorpe <jgg@xxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Guenter Roeck <groeck@xxxxxxxxxxxx>
> Cc: Alexander Steffen <Alexander.Steffen@xxxxxxxxxxxx>
> Cc: Heiko Stuebner <heiko@xxxxxxxxx>

OK, so, I put these to my master in hopes to get testing exposure.
I think the changes are in great shape now. Thank you.

/Jarkko