On Wed Jun 5, 2024 at 1:14 AM EEST, wrote:
> On 6/4/24 1:27 PM, Jarkko Sakkinen wrote:
> > On Fri May 31, 2024 at 4:03 AM EEST, Ross Philipson wrote:
> >> Curently the locality is hard coded to 0 but for DRTM support, access
> >> is needed to localities 1 through 4.
> >>
> >> Signed-off-by: Ross Philipson <ross.philipson@xxxxxxxxxx>
> >> ---
> >> drivers/char/tpm/tpm-chip.c | 24 +++++++++++++++++++++++-
> >> drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++
> >> drivers/char/tpm/tpm.h | 1 +
> >> include/linux/tpm.h | 4 ++++
> >> 4 files changed, 43 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> >> index 854546000c92..73eac54d61fb 100644
> >> --- a/drivers/char/tpm/tpm-chip.c
> >> +++ b/drivers/char/tpm/tpm-chip.c
> >> @@ -44,7 +44,7 @@ static int tpm_request_locality(struct tpm_chip *chip)
> >> if (!chip->ops->request_locality)
> >> return 0;
> >>
> >> - rc = chip->ops->request_locality(chip, 0);
> >> + rc = chip->ops->request_locality(chip, chip->pref_locality);
> >> if (rc < 0)
> >> return rc;
> >>
> >> @@ -143,6 +143,27 @@ void tpm_chip_stop(struct tpm_chip *chip)
> >> }
> >> EXPORT_SYMBOL_GPL(tpm_chip_stop);
> >>
> >> +/**
> >> + * tpm_chip_preferred_locality() - set the TPM chip preferred locality to open
> >> + * @chip: a TPM chip to use
> >> + * @locality: the preferred locality
> >> + *
> >> + * Return:
> >> + * * true - Preferred locality set
> >> + * * false - Invalid locality specified
> >> + */
> >> +bool tpm_chip_preferred_locality(struct tpm_chip *chip, int locality)
> >> +{
> >> + if (locality < 0 || locality >=TPM_MAX_LOCALITY)
> >> + return false;
> >> +
> >> + mutex_lock(&chip->tpm_mutex);
> >> + chip->pref_locality = locality;
> >> + mutex_unlock(&chip->tpm_mutex);
> >> + return true;
> >> +}
> >> +EXPORT_SYMBOL_GPL(tpm_chip_preferred_locality);
> >> +
> >> /**
> >> * tpm_try_get_ops() - Get a ref to the tpm_chip
> >> * @chip: Chip to ref
> >> @@ -374,6 +395,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
> >> }
> >>
> >> chip->locality = -1;
> >> + chip->pref_locality = 0;
> >> return chip;
> >>
> >> out:
> >> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> >> index 5da134f12c9a..35f14ccecf0e 100644
> >> --- a/drivers/char/tpm/tpm-interface.c
> >> +++ b/drivers/char/tpm/tpm-interface.c
> >> @@ -274,6 +274,21 @@ int tpm_is_tpm2(struct tpm_chip *chip)
> >> }
> >> EXPORT_SYMBOL_GPL(tpm_is_tpm2);
> >>
> >> +/**
> >> + * tpm_preferred_locality() - set the TPM chip preferred locality to open
> >> + * @chip: a TPM chip to use
> >> + * @locality: the preferred locality
> >> + *
> >> + * Return:
> >> + * * true - Preferred locality set
> >> + * * false - Invalid locality specified
> >> + */
> >> +bool tpm_preferred_locality(struct tpm_chip *chip, int locality)
> >> +{
> >> + return tpm_chip_preferred_locality(chip, locality);
> >> +}
> >> +EXPORT_SYMBOL_GPL(tpm_preferred_locality);
> >
> > What good does this extra wrapping do?
> >
> > tpm_set_default_locality() and default_locality would make so much more
> > sense in any case.
>
> Are you mainly just talking about my naming choices here and in the
> follow-on response? Can you clarify what you are requesting?
I'd prefer:
1. Name the variable as default_locality.
2. Only create a single expored to function to tpm-chip.c:
tpm_chip_set_default_locality().
3. Call this function in all call sites.
"tpm_preferred_locality" should be just removed, as tpm_chip_*
is exported anyway.
BR, Jarkko
