On Thu, Nov 03, 2022 at 03:54:50PM +0100, Jan Dabros wrote:
> Move i2c locking primitives to request_locality and relinquish_locality
> callbacks, what effectively blocks TPM bus for the whole duration of
> logical TPM operation.
>
> With this in place, cr50-equipped TPM may be shared with external CPUs -
> assuming that underneath i2c controller driver is aware of this setup
> (see i2c-designware-amdpsp as an example).
Nit: s/CPUs/peripherals/ ? I'm not sure why you want to emphasize external
chips having CPU.
>
> Signed-off-by: Jan Dabros <jsd@xxxxxxxxxxxx>
> ---
> drivers/char/tpm/tpm_tis_i2c_cr50.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> index 517d8410d7da0..f8a67fc2382cc 100644
> --- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
> +++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> @@ -202,8 +202,6 @@ static int tpm_cr50_i2c_read(struct tpm_chip *chip, u8 addr, u8 *buffer, size_t
> };
> int rc;
>
> - i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
> -
> /* Prepare for completion interrupt */
> tpm_cr50_i2c_enable_tpm_irq(chip);
>
> @@ -222,7 +220,6 @@ static int tpm_cr50_i2c_read(struct tpm_chip *chip, u8 addr, u8 *buffer, size_t
>
> out:
> tpm_cr50_i2c_disable_tpm_irq(chip);
> - i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
>
> if (rc < 0)
> return rc;
> @@ -264,8 +261,6 @@ static int tpm_cr50_i2c_write(struct tpm_chip *chip, u8 addr, u8 *buffer,
> priv->buf[0] = addr;
> memcpy(priv->buf + 1, buffer, len);
>
> - i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
> -
> /* Prepare for completion interrupt */
> tpm_cr50_i2c_enable_tpm_irq(chip);
>
> @@ -279,7 +274,6 @@ static int tpm_cr50_i2c_write(struct tpm_chip *chip, u8 addr, u8 *buffer,
>
> out:
> tpm_cr50_i2c_disable_tpm_irq(chip);
> - i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
>
> if (rc < 0)
> return rc;
> @@ -323,6 +317,7 @@ static int tpm_cr50_check_locality(struct tpm_chip *chip, int loc)
> */
> static int tpm_cr50_release_locality(struct tpm_chip *chip, int loc)
> {
> + struct i2c_client *client = to_i2c_client(chip->dev.parent);
> u8 mask = TPM_ACCESS_VALID | TPM_ACCESS_REQUEST_PENDING;
> u8 addr = TPM_I2C_ACCESS(loc);
> u8 buf;
> @@ -330,13 +325,15 @@ static int tpm_cr50_release_locality(struct tpm_chip *chip, int loc)
>
> rc = tpm_cr50_i2c_read(chip, addr, &buf, sizeof(buf));
> if (rc < 0)
> - return rc;
> + goto unlock_out;
>
> if ((buf & mask) == mask) {
> buf = TPM_ACCESS_ACTIVE_LOCALITY;
> rc = tpm_cr50_i2c_write(chip, addr, &buf, sizeof(buf));
> }
>
> +unlock_out:
> + i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
> return rc;
> }
>
> @@ -351,16 +348,19 @@ static int tpm_cr50_release_locality(struct tpm_chip *chip, int loc)
> */
> static int tpm_cr50_request_locality(struct tpm_chip *chip, int loc)
> {
> + struct i2c_client *client = to_i2c_client(chip->dev.parent);
> u8 buf = TPM_ACCESS_REQUEST_USE;
> unsigned long stop;
> int rc;
>
> + i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
> +
> if (!tpm_cr50_check_locality(chip, loc))
> return loc;
>
> rc = tpm_cr50_i2c_write(chip, TPM_I2C_ACCESS(loc), &buf, sizeof(buf));
> if (rc < 0)
> - return rc;
> + goto unlock_out;
>
> stop = jiffies + chip->timeout_a;
> do {
> @@ -370,7 +370,11 @@ static int tpm_cr50_request_locality(struct tpm_chip *chip, int loc)
> msleep(TPM_CR50_TIMEOUT_SHORT_MS);
> } while (time_before(jiffies, stop));
>
> - return -ETIMEDOUT;
> + rc = -ETIMEDOUT;
> +
> +unlock_out:
> + i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
> + return rc;
> }
>
> /**
> --
> 2.38.1.273.g43a17bfeac-goog
>
BR, Jarkko
