On Wed, Sep 09, 2026 at 11:08:59AM -0500, Mario Limonciello wrote: > > > On 9/1/26 17:55, Jarkko Sakkinen wrote: > > On Tue, Sep 01, 2026 at 04:37:22PM -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]> > > > --- > > > v3: > > > * Use __free() to clean up > > > v2: > > > * Drop redundant calls > > > * Update commit message > > > --- > > > drivers/char/tpm/tpm-chip.c | 24 ------------------------ > > > drivers/char/tpm/tpm-interface.c | 21 +++++++++++++++++++++ > > > 2 files changed, 21 insertions(+), 24 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..8d434d7fa5bab 100644 > > > --- a/drivers/char/tpm/tpm-interface.c > > > +++ b/drivers/char/tpm/tpm-interface.c > > > @@ -19,6 +19,7 @@ > > > * calls to msleep. > > > */ > > > +#include <linux/cleanup.h> > > > #include <linux/poll.h> > > > #include <linux/slab.h> > > > #include <linux/mutex.h> > > > @@ -89,8 +90,16 @@ static bool tpm_transmit_completed(u8 status, struct > > > tpm_chip *chip) > > > return status_masked == chip->ops->req_complete_val; > > > } > > > +static void tpm_go_idle(struct tpm_chip *chip) > > > +{ > > > + if (chip->ops->go_idle) > > > + chip->ops->go_idle(chip); > > > +} > > > +DEFINE_FREE(tpm_go_idle, struct tpm_chip *, if (_T) tpm_go_idle(_T)) > > > + > > > static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, > > > size_t bufsiz) > > > { > > > + struct tpm_chip *chip_idle __free(tpm_go_idle) = NULL; > > > struct tpm_header *header = buf; > > > int rc; > > > ssize_t len = 0; > > > @@ -113,6 +122,18 @@ 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; > > > + } > > > + } > > > + > > > + /* Ensure go_idle() is called on every exit path from here on. */ > > > + chip_idle = chip; > > > + > > > rc = chip->ops->send(chip, buf, bufsiz, count); > > > if (rc < 0) { > > > if (rc != -EPIPE) > > > -- > > > 2.43.0 > > > > > > > This is now much better as it addition adds better guarantees that > > resources always get released after use no matter how this function > > is modified later on. > > > > Reviewed-by: Jarkko Sakkinen <[email protected]> > > > > BR, Jarkko > > Thanks. If no other concerns, can this be picked up now? >
Sorry for the delay, done BR, Jarkko
