| Message ID | 20250217224946.113951-3-stuart.yoder@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for the TPM FF-A start method | expand |
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 <stuart.yoder@arm.com> > --- > 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
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))
-Clean up unnecessary parenthesis usage -Refactor the two checks for whether the TPM supports idle into a single inline function. Signed-off-by: Stuart Yoder <stuart.yoder@arm.com> --- drivers/char/tpm/tpm_crb.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-)