On Mon, Oct 31, 2022 at 8:04 PM Jan Dabros <jsd@xxxxxxxxxxxx> wrote:
> /**
> - * tpm_cr50_check_locality() - Verify TPM locality 0 is active.
> + * tpm_cr50_check_locality() - Verify if required TPM locality is active.
> * @chip: A TPM chip.
> + * @loc: Locality to be verified
> *
> * Return:
> - * - 0: Success.
> + * - loc: Success.
> * - -errno: A POSIX error code.
> */
> -static int tpm_cr50_check_locality(struct tpm_chip *chip)
> +static int tpm_cr50_check_locality(struct tpm_chip *chip, int loc)
> {
> u8 mask = TPM_ACCESS_VALID | TPM_ACCESS_ACTIVE_LOCALITY;
> u8 buf;
> int rc;
>
> - rc = tpm_cr50_i2c_read(chip, TPM_I2C_ACCESS(0), &buf, sizeof(buf));
> + rc = tpm_cr50_i2c_read(chip, TPM_I2C_ACCESS(loc), &buf, sizeof(buf));
> if (rc < 0)
> return rc;
>
> if ((buf & mask) == mask)
> - return 0;
> + return loc;
>
> return -EIO;
> }
Why is it useful to return the same `loc` value that was passed in,
rather than just returning `0`? The caller already knows the value of
`loc`, so they aren't being told anything new.
I think this should continue to return `0` for success.
> - * tpm_cr50_request_locality() - Request TPM locality 0.
> + * tpm_cr50_request_locality() - Request TPM locality.
> * @chip: A TPM chip.
> + * @loc: Locality to be requested.
> *
> * Return:
> - * - 0: Success.
> + * - loc: Success.
Same as above. Return `0`.
> @@ -374,7 +386,9 @@ static u8 tpm_cr50_i2c_tis_status(struct tpm_chip *chip)
> {
> u8 buf[4];
>
> - if (tpm_cr50_i2c_read(chip, TPM_I2C_STS(0), buf, sizeof(buf)) < 0)
> + WARN_ONCE((chip->locality < 0), "Incorrect tpm locality value\n");
For each of these ` WARN_ONCE((chip->locality < 0), ...).`, can it
return immediately rather than attempting to continue using an invalid
locality value? Do the following commands have a chance of succeeding
with the invalid value?
--
Tim Van Patten | ChromeOS | timvp@xxxxxxxxxx | (720) 432-0997
