| Message ID | 1439304497-10081-8-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Simon, As per my comment on patch 6, i would disagree as well on this one. It tpm_vendor_specific structure is convenient for ST33ZP24 for example. Best Regards Christophe On 11/08/2015 16:47, Simon Glass wrote: > The function methods in struct tpm_vendor_specific just call local functions. > Change the code to use a direct call. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > drivers/tpm/tpm_tis_i2c.c | 12 ++++-------- > drivers/tpm/tpm_tis_i2c.h | 4 ---- > 2 files changed, 4 insertions(+), 12 deletions(-) > > diff --git a/drivers/tpm/tpm_tis_i2c.c b/drivers/tpm/tpm_tis_i2c.c > index 2b3ce8c..5615ffc 100644 > --- a/drivers/tpm/tpm_tis_i2c.c > +++ b/drivers/tpm/tpm_tis_i2c.c > @@ -810,10 +810,6 @@ out_err: > } > > static struct tpm_vendor_specific tpm_tis_i2c = { > - .status = tpm_tis_i2c_status, > - .recv = tpm_tis_i2c_recv, > - .send = tpm_tis_i2c_send, > - .cancel = tpm_tis_i2c_ready, > .req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID, > .req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID, > .req_canceled = TPM_STS_COMMAND_READY, > @@ -940,7 +936,7 @@ static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz) > } > > debug("Calling send\n"); > - rc = chip->vendor.send(chip, (u8 *)buf, count); > + rc = tpm_tis_i2c_send(chip, (u8 *)buf, count); > debug(" ... done calling send\n"); > if (rc < 0) { > error("tpm_transmit: tpm_send: error %d\n", rc); > @@ -954,7 +950,7 @@ static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz) > stop = tpm_calc_ordinal_duration(chip, ordinal); > do { > debug("waiting for status... %ld %ld\n", start, stop); > - u8 status = chip->vendor.status(chip); > + u8 status = tpm_tis_i2c_status(chip); > if ((status & chip->vendor.req_complete_mask) == > chip->vendor.req_complete_val) { > debug("...got it;\n"); > @@ -969,14 +965,14 @@ static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz) > udelay(TPM_TIMEOUT * 1000); > } while (get_timer(start) < stop); > > - chip->vendor.cancel(chip); > + tpm_tis_i2c_ready(chip); > error("Operation Timed out\n"); > rc = -ETIME; > goto out; > > out_recv: > debug("out_recv: reading response...\n"); > - rc = chip->vendor.recv(chip, (u8 *)buf, TPM_BUFSIZE); > + rc = tpm_tis_i2c_recv(chip, (u8 *)buf, TPM_BUFSIZE); > if (rc < 0) > error("tpm_transmit: tpm_recv: error %d\n", rc); > > diff --git a/drivers/tpm/tpm_tis_i2c.h b/drivers/tpm/tpm_tis_i2c.h > index 75fa829..426c519 100644 > --- a/drivers/tpm/tpm_tis_i2c.h > +++ b/drivers/tpm/tpm_tis_i2c.h > @@ -40,10 +40,6 @@ struct tpm_vendor_specific { > const u8 req_complete_val; > const u8 req_canceled; > int irq; > - int (*recv) (struct tpm_chip *, u8 *, size_t); > - int (*send) (struct tpm_chip *, u8 *, size_t); > - void (*cancel) (struct tpm_chip *); > - u8(*status) (struct tpm_chip *); > int locality; > unsigned long timeout_a, timeout_b, timeout_c, timeout_d; /* msec */ > unsigned long duration[3]; /* msec */
Hi Christophe, On 11 August 2015 at 15:47, christophe.ricard <christophe.ricard@gmail.com> wrote: > Hi Simon, > > As per my comment on patch 6, i would disagree as well on this one. > It tpm_vendor_specific structure is convenient for ST33ZP24 for example. > > Best Regards > Christophe > As things stand they are only used in one place. I think we should work out the best uclass API for the TPM and then go with that. [snip] Regards, Simon
Hi Simon, On 13/08/2015 03:30, Simon Glass wrote: > Hi Christophe, > > On 11 August 2015 at 15:47, christophe.ricard > <christophe.ricard@gmail.com> wrote: >> Hi Simon, >> >> As per my comment on patch 6, i would disagree as well on this one. >> It tpm_vendor_specific structure is convenient for ST33ZP24 for example. >> >> Best Regards >> Christophe >> > As things stand they are only used in one place. I think we should > work out the best uclass API for the TPM and then go with that. I agree with that. How do you want to proceed ? Do you agree to integrate the results of our exchanges for a new round ? [...] Best Regards Christophe
Hi Christophe, On 13 August 2015 at 14:28, Christophe Ricard <christophe.ricard@gmail.com> wrote: > Hi Simon, > > On 13/08/2015 03:30, Simon Glass wrote: >> >> Hi Christophe, >> >> On 11 August 2015 at 15:47, christophe.ricard >> <christophe.ricard@gmail.com> wrote: >>> >>> Hi Simon, >>> >>> As per my comment on patch 6, i would disagree as well on this one. >>> It tpm_vendor_specific structure is convenient for ST33ZP24 for example. >>> >>> Best Regards >>> Christophe >>> >> As things stand they are only used in one place. I think we should >> work out the best uclass API for the TPM and then go with that. > > I agree with that. > How do you want to proceed ? Do you agree to integrate the results of our > exchanges for a new round ? Yes OK, I'll have a crack at this. Regards, Simon
diff --git a/drivers/tpm/tpm_tis_i2c.c b/drivers/tpm/tpm_tis_i2c.c index 2b3ce8c..5615ffc 100644 --- a/drivers/tpm/tpm_tis_i2c.c +++ b/drivers/tpm/tpm_tis_i2c.c @@ -810,10 +810,6 @@ out_err: } static struct tpm_vendor_specific tpm_tis_i2c = { - .status = tpm_tis_i2c_status, - .recv = tpm_tis_i2c_recv, - .send = tpm_tis_i2c_send, - .cancel = tpm_tis_i2c_ready, .req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID, .req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID, .req_canceled = TPM_STS_COMMAND_READY, @@ -940,7 +936,7 @@ static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz) } debug("Calling send\n"); - rc = chip->vendor.send(chip, (u8 *)buf, count); + rc = tpm_tis_i2c_send(chip, (u8 *)buf, count); debug(" ... done calling send\n"); if (rc < 0) { error("tpm_transmit: tpm_send: error %d\n", rc); @@ -954,7 +950,7 @@ static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz) stop = tpm_calc_ordinal_duration(chip, ordinal); do { debug("waiting for status... %ld %ld\n", start, stop); - u8 status = chip->vendor.status(chip); + u8 status = tpm_tis_i2c_status(chip); if ((status & chip->vendor.req_complete_mask) == chip->vendor.req_complete_val) { debug("...got it;\n"); @@ -969,14 +965,14 @@ static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz) udelay(TPM_TIMEOUT * 1000); } while (get_timer(start) < stop); - chip->vendor.cancel(chip); + tpm_tis_i2c_ready(chip); error("Operation Timed out\n"); rc = -ETIME; goto out; out_recv: debug("out_recv: reading response...\n"); - rc = chip->vendor.recv(chip, (u8 *)buf, TPM_BUFSIZE); + rc = tpm_tis_i2c_recv(chip, (u8 *)buf, TPM_BUFSIZE); if (rc < 0) error("tpm_transmit: tpm_recv: error %d\n", rc); diff --git a/drivers/tpm/tpm_tis_i2c.h b/drivers/tpm/tpm_tis_i2c.h index 75fa829..426c519 100644 --- a/drivers/tpm/tpm_tis_i2c.h +++ b/drivers/tpm/tpm_tis_i2c.h @@ -40,10 +40,6 @@ struct tpm_vendor_specific { const u8 req_complete_val; const u8 req_canceled; int irq; - int (*recv) (struct tpm_chip *, u8 *, size_t); - int (*send) (struct tpm_chip *, u8 *, size_t); - void (*cancel) (struct tpm_chip *); - u8(*status) (struct tpm_chip *); int locality; unsigned long timeout_a, timeout_b, timeout_c, timeout_d; /* msec */ unsigned long duration[3]; /* msec */
The function methods in struct tpm_vendor_specific just call local functions. Change the code to use a direct call. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/tpm/tpm_tis_i2c.c | 12 ++++-------- drivers/tpm/tpm_tis_i2c.h | 4 ---- 2 files changed, 4 insertions(+), 12 deletions(-)