On Mon, 2025-02-17 at 16:49 -0600, Stuart Yoder wrote: > -Clean up unnecessary parenthesis usage > -Refactor the two checks for whether the TPM supports idle into a > single > inline function.
IMHO, this would be better: "Refactor TPM idle check to tpm_crb_has_idle(), and reduce paraentheses usage in start method checks." > > Signed-off-by: Stuart Yoder <[email protected]> > --- > drivers/char/tpm/tpm_crb.c | 36 +++++++++++++++++++++--------------- > 1 file changed, 21 insertions(+), 15 deletions(-) > > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c > index ea085b14ab7c..31db879f1324 100644 > --- a/drivers/char/tpm/tpm_crb.c > +++ b/drivers/char/tpm/tpm_crb.c > @@ -115,6 +115,16 @@ struct tpm2_crb_pluton { > u64 reply_addr; > }; > > +/* > + * Returns true if the start method supports idle. > + */ > +static inline bool tpm_crb_has_idle(u32 start_method) > +{ > + return start_method == ACPI_TPM2_START_METHOD || > + start_method == > ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD || > + start_method == > ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC; > +} > + > static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 > value, > unsigned long timeout) > { > @@ -173,9 +183,7 @@ static int __crb_go_idle(struct device *dev, > struct crb_priv *priv) > { > int rc; > > - if ((priv->sm == ACPI_TPM2_START_METHOD) || > - (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) > || > - (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC)) > + if (!tpm_crb_has_idle(priv->sm)) > return 0; > > iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs_t->ctrl_req); > @@ -222,9 +230,7 @@ static int __crb_cmd_ready(struct device *dev, > struct crb_priv *priv) > { > int rc; > > - if ((priv->sm == ACPI_TPM2_START_METHOD) || > - (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) > || > - (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC)) > + if (!tpm_crb_has_idle(priv->sm)) > return 0; > > iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req); > @@ -423,13 +429,13 @@ static int crb_send(struct tpm_chip *chip, u8 > *buf, size_t len) > * report only ACPI start but in practice seems to require > both > * CRB start, hence invoking CRB start method if hid == > MSFT0101. > */ > - if ((priv->sm == ACPI_TPM2_COMMAND_BUFFER) || > - (priv->sm == ACPI_TPM2_MEMORY_MAPPED) || > - (!strcmp(priv->hid, "MSFT0101"))) > + if (priv->sm == ACPI_TPM2_COMMAND_BUFFER || > + priv->sm == ACPI_TPM2_MEMORY_MAPPED || > + !strcmp(priv->hid, "MSFT0101")) > iowrite32(CRB_START_INVOKE, &priv->regs_t- > >ctrl_start); > > - if ((priv->sm == ACPI_TPM2_START_METHOD) || > - (priv->sm == > ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)) > + if (priv->sm == ACPI_TPM2_START_METHOD || > + priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) > rc = crb_do_acpi_start(chip); > > if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) { > @@ -449,8 +455,8 @@ static void crb_cancel(struct tpm_chip *chip) > > iowrite32(CRB_CANCEL_INVOKE, &priv->regs_t->ctrl_cancel); > > - if (((priv->sm == ACPI_TPM2_START_METHOD) || > - (priv->sm == > ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)) && > + if ((priv->sm == ACPI_TPM2_START_METHOD || > + priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) > && > crb_do_acpi_start(chip)) > dev_err(&chip->dev, "ACPI Start failed\n"); > } > @@ -609,8 +615,8 @@ static int crb_map_io(struct acpi_device *device, > struct crb_priv *priv, > * the control area, as one nice sane region except for some > older > * stuff that puts the control area outside the ACPI IO > region. > */ > - if ((priv->sm == ACPI_TPM2_COMMAND_BUFFER) || > - (priv->sm == ACPI_TPM2_MEMORY_MAPPED)) { > + if (priv->sm == ACPI_TPM2_COMMAND_BUFFER || > + priv->sm == ACPI_TPM2_MEMORY_MAPPED) { > if (iores && > buf->control_address == iores->start + > sizeof(*priv->regs_h)) BR, Jarkko
