Re: [PATCH v2] tpm: Call cmd_ready/go_idle for each command transmission

[Jarkko Sakkinen <jarkko@kernel.org>]

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 <mario.limonciello@amd.com>
> ---
> 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