On Tue, Sep 01, 2026 at 11:32:15AM -0500, Mario Limonciello wrote:
> Some TPM implementations, particularly fTPM using the CRB interface,
> require the TPM to transition through idle and ready states for each
> command rather than once per session.
>
> The current implementation calls cmd_ready once during tpm_chip_start()
> and go_idle once during tpm_chip_stop(). For fTPM, when multiple commands
> are sent without per-command idle transitions, subsequent commands timeout
> as the TPM is waiting for the transition.
>
> Fix this by calling cmd_ready before each command and go_idle on all exit
> paths in tpm_try_transmit(). For TPM implementations that don't require
> per-command transitions, the callbacks return immediately based on the
> start method.
>
> Remove the now-redundant per-session calls from tpm_chip_start() and
> tpm_chip_stop() along with their helpers.
>
> This resolves timeout errors during TPM initialization on systems where
> BIOS has already performed TPM startup.
>
> Signed-off-by: Mario Limonciello <[email protected]>
> ---
> v2:
> * Drop redundant calls
> * Update commit message
> ---
> drivers/char/tpm/tpm-chip.c | 24 ------------------------
> drivers/char/tpm/tpm-interface.c | 26 ++++++++++++++++++++++----
> 2 files changed, 22 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 12b7394b34bdc..0be5dbfaa72eb 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -66,22 +66,6 @@ static void tpm_relinquish_locality(struct tpm_chip *chip)
> chip->locality = -1;
> }
>
> -static int tpm_cmd_ready(struct tpm_chip *chip)
> -{
> - if (!chip->ops->cmd_ready)
> - return 0;
> -
> - return chip->ops->cmd_ready(chip);
> -}
> -
> -static int tpm_go_idle(struct tpm_chip *chip)
> -{
> - if (!chip->ops->go_idle)
> - return 0;
> -
> - return chip->ops->go_idle(chip);
> -}
> -
> static void tpm_clk_enable(struct tpm_chip *chip)
> {
> if (chip->ops->clk_enable)
> @@ -116,13 +100,6 @@ int tpm_chip_start(struct tpm_chip *chip)
> }
> }
>
> - ret = tpm_cmd_ready(chip);
> - if (ret) {
> - tpm_relinquish_locality(chip);
> - tpm_clk_disable(chip);
> - return ret;
> - }
> -
> return 0;
> }
> EXPORT_SYMBOL_GPL(tpm_chip_start);
> @@ -137,7 +114,6 @@ EXPORT_SYMBOL_GPL(tpm_chip_start);
> */
> void tpm_chip_stop(struct tpm_chip *chip)
> {
> - tpm_go_idle(chip);
> tpm_relinquish_locality(chip);
> tpm_clk_disable(chip);
> }
> diff --git a/drivers/char/tpm/tpm-interface.c
> b/drivers/char/tpm/tpm-interface.c
> index f745a098908b3..c5b5b5ba41821 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -113,12 +113,21 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> void *buf, size_t bufsiz)
> return -E2BIG;
> }
>
> + if (chip->ops->cmd_ready) {
> + rc = chip->ops->cmd_ready(chip);
> + if (rc) {
> + dev_err(&chip->dev,
> + "%s: cmd_ready(): error %d\n", __func__, rc);
> + return rc;
> + }
> + }
I'd consider retaining tpm_go_idle and do this after arming the command:
chip = chip __free(tpm_go_idle);
> +
> rc = chip->ops->send(chip, buf, bufsiz, count);
> if (rc < 0) {
> if (rc != -EPIPE)
> dev_err(&chip->dev,
> "%s: send(): error %d\n", __func__, rc);
> - return rc;
> + goto out;
> }
>
> /*
> @@ -152,7 +161,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> void *buf, size_t bufsiz)
>
> if (tpm_chip_req_canceled(chip, status)) {
> dev_err(&chip->dev, "Operation Canceled\n");
> - return -ECANCELED;
> + rc = -ECANCELED;
> + goto out;
> }
>
> tpm_msleep(TPM_TIMEOUT_POLL);
> @@ -168,19 +178,27 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> void *buf, size_t bufsiz)
>
> tpm_chip_cancel(chip);
> dev_err(&chip->dev, "Operation Timed out\n");
> - return -ETIME;
> + rc = -ETIME;
> + goto out;
>
> out_recv:
> len = chip->ops->recv(chip, buf, bufsiz);
> if (len < 0) {
> rc = len;
> dev_err(&chip->dev, "tpm_transmit: tpm_recv: error %d\n", rc);
> - return rc;
> + goto out;
> }
> out_sync:
> if (len < TPM_HEADER_SIZE || len != be32_to_cpu(header->length))
> rc = -EFAULT;
>
> +out:
> + if (chip->ops->go_idle) {
> + int idle_rc = chip->ops->go_idle(chip);
> + if (idle_rc && !rc)
> + rc = idle_rc;
> + }
> +
> return rc ? rc : len;
> }
>
> --
> 2.43.0
>
BR, Jarkko
