[RFC,v2,1/1] tpm: don't return bool from update_timeouts

Commit Message

Jerry Snitselaar Jan. 29, 2019, 8:14 p.m. UTC

Set tpm_chip->timeouts_adjusted directly in the update_timeouts
code instead of returning bool. In case of tpm read failing
print warning that the read failed and continue on.

Signed-off-by: Jerry Snitselaar <[e-mail]>
---
v2: changed update_timeouts to void and print warning in case
    of tpm read failing.

drivers/char/tpm/tpm1-cmd.c     |  3 +--
 drivers/char/tpm/tpm_tis_core.c | 14 ++++++++------
 include/linux/tpm.h             |  2 +-
 3 files changed, 10 insertions(+), 9 deletions(-)

Comments

Jarkko Sakkinen Jan. 30, 2019, 9:23 p.m. UTC | #1

On Tue, Jan 29, 2019 at 01:14:15PM -0700, Jerry Snitselaar wrote:
> Set tpm_chip->timeouts_adjusted directly in the update_timeouts
> code instead of returning bool. In case of tpm read failing
> print warning that the read failed and continue on.
> 
> Signed-off-by: Jerry Snitselaar <[e-mail]>

Reviewed-by: Jarkko Sakkinen <[e-mail]>

/Jarkko

Jarkko Sakkinen Jan. 30, 2019, 9:24 p.m. UTC | #2

On Wed, Jan 30, 2019 at 11:23:03PM +0200, Jarkko Sakkinen wrote:
> On Tue, Jan 29, 2019 at 01:14:15PM -0700, Jerry Snitselaar wrote:
> > Set tpm_chip->timeouts_adjusted directly in the update_timeouts
> > code instead of returning bool. In case of tpm read failing
> > print warning that the read failed and continue on.
> > 
> > Signed-off-by: Jerry Snitselaar <[e-mail]>
> 
> Reviewed-by: Jarkko Sakkinen <[e-mail]>

Hey, can you-re-send this without RFC and the earlier that I already
applied to my tree, with CC to linux-kernel and linux-security-module?

Keep my reviewed-by in both.

Otherwise, I cannot include these into my next PR.

/Jarkko

Patch

diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 6f306338953b..bda9a16b44f6 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -380,8 +380,7 @@  int tpm1_get_timeouts(struct tpm_chip *chip)
 	 * of misreporting.
 	 */
 	if (chip->ops->update_timeouts)
-		chip->timeout_adjusted =
-			chip->ops->update_timeouts(chip, timeout_eff);
+		chip->ops->update_timeouts(chip, timeout_eff);
 
 	if (!chip->timeout_adjusted) {
 		/* Restore default if chip reported 0 */
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index bf7e49cfa643..aec57bd25dd2 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -521,35 +521,37 @@  static const struct tis_vendor_timeout_override vendor_timeout_overrides[] = {
 			(TIS_SHORT_TIMEOUT*1000), (TIS_SHORT_TIMEOUT*1000) } },
 };
 
-static bool tpm_tis_update_timeouts(struct tpm_chip *chip,
+static void tpm_tis_update_timeouts(struct tpm_chip *chip,
 				    unsigned long *timeout_cap)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 	int i, rc;
 	u32 did_vid;
 
+	chip->timeout_adjusted = false;
+
 	if (chip->ops->clk_enable != NULL)
 		chip->ops->clk_enable(chip, true);
 
 	rc = tpm_tis_read32(priv, TPM_DID_VID(0), &did_vid);
-	if (rc < 0)
+	if (rc < 0) {
+		dev_warn(&chip->dev, "%s: failed to read did_vid: %d\n", __func__, rc);
 		goto out;
+	}
 
 	for (i = 0; i != ARRAY_SIZE(vendor_timeout_overrides); i++) {
 		if (vendor_timeout_overrides[i].did_vid != did_vid)
 			continue;
 		memcpy(timeout_cap, vendor_timeout_overrides[i].timeout_us,
 		       sizeof(vendor_timeout_overrides[i].timeout_us));
-		rc = true;
+		chip->timeout_adjusted = true;
 	}
 
-	rc = false;
-
 out:
 	if (chip->ops->clk_enable != NULL)
 		chip->ops->clk_enable(chip, false);
 
-	return rc;
+	return;
 }
 
 /*
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index b49a55cf775f..13563b8c0c3a 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -41,7 +41,7 @@  struct tpm_class_ops {
 	int (*send) (struct tpm_chip *chip, u8 *buf, size_t len);
 	void (*cancel) (struct tpm_chip *chip);
 	u8 (*status) (struct tpm_chip *chip);
-	bool (*update_timeouts)(struct tpm_chip *chip,
+	void (*update_timeouts)(struct tpm_chip *chip,
 				unsigned long *timeout_cap);
 	int (*go_idle)(struct tpm_chip *chip);
 	int (*cmd_ready)(struct tpm_chip *chip);