| Message ID | 20211107213312.263357-2-ilias.apalodimas@linaro.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | TPM cleanups and MMIO driver | expand |
On 11/7/21 22:33, Ilias Apalodimas wrote: > With the upcoming TPM2 API, some of the function names are part of the new > header file. So switch conflicting driver defined function names and > defines. > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > drivers/tpm/tpm_tis_infineon.c | 34 +++++++++++++++++----------------- > 1 file changed, 17 insertions(+), 17 deletions(-) > > diff --git a/drivers/tpm/tpm_tis_infineon.c b/drivers/tpm/tpm_tis_infineon.c > index f414e5657db9..aa66e931bada 100644 > --- a/drivers/tpm/tpm_tis_infineon.c > +++ b/drivers/tpm/tpm_tis_infineon.c > @@ -50,10 +50,10 @@ static const char * const chip_name[] = { > [UNKNOWN] = "unknown/fallback to slb9635", > }; > > -#define TPM_ACCESS(l) (0x0000 | ((l) << 4)) > -#define TPM_STS(l) (0x0001 | ((l) << 4)) > -#define TPM_DATA_FIFO(l) (0x0005 | ((l) << 4)) > -#define TPM_DID_VID(l) (0x0006 | ((l) << 4)) > +#define TPM1_ACCESS(l) (0x0000 | ((l) << 4)) > +#define TPM1_STS(l) (0x0001 | ((l) << 4)) > +#define TPM1_DATA_FIFO(l) (0x0005 | ((l) << 4)) > +#define TPM1_DID_VID(l) (0x0006 | ((l) << 4)) To what does '1' relate here? * Do you mean TPMv1? * Or are the constants Infenion specific? If the constants are TPMv1 specific, they should be in a header file. If the constants are Infineon specific we could put _INFINEON_ into the constant name. Linux has resolved the conflict by putting the TPMv2 TIS constants into drivers/char/tpm/tpm_tis_core.h which is not included by drivers/char/tpm/tpm_i2c_infineon.c. Best regards Heinrich > > /* > * tpm_tis_i2c_read() - read from TPM register > @@ -197,7 +197,7 @@ static int tpm_tis_i2c_check_locality(struct udevice *dev, int loc) > u8 buf; > int rc; > > - rc = tpm_tis_i2c_read(dev, TPM_ACCESS(loc), &buf, 1); > + rc = tpm_tis_i2c_read(dev, TPM1_ACCESS(loc), &buf, 1); > if (rc < 0) > return rc; > > @@ -215,12 +215,12 @@ static void tpm_tis_i2c_release_locality(struct udevice *dev, int loc, > const u8 mask = TPM_ACCESS_REQUEST_PENDING | TPM_ACCESS_VALID; > u8 buf; > > - if (tpm_tis_i2c_read(dev, TPM_ACCESS(loc), &buf, 1) < 0) > + if (tpm_tis_i2c_read(dev, TPM1_ACCESS(loc), &buf, 1) < 0) > return; > > if (force || (buf & mask) == mask) { > buf = TPM_ACCESS_ACTIVE_LOCALITY; > - tpm_tis_i2c_write(dev, TPM_ACCESS(loc), &buf, 1); > + tpm_tis_i2c_write(dev, TPM1_ACCESS(loc), &buf, 1); > } > } > > @@ -240,7 +240,7 @@ static int tpm_tis_i2c_request_locality(struct udevice *dev, int loc) > return rc; > } > > - rc = tpm_tis_i2c_write(dev, TPM_ACCESS(loc), &buf, 1); > + rc = tpm_tis_i2c_write(dev, TPM1_ACCESS(loc), &buf, 1); > if (rc) { > debug("%s: Failed to write to TPM: %d\n", __func__, rc); > return rc; > @@ -271,7 +271,7 @@ static u8 tpm_tis_i2c_status(struct udevice *dev) > /* NOTE: Since i2c read may fail, return 0 in this case --> time-out */ > u8 buf; > > - if (tpm_tis_i2c_read(dev, TPM_STS(chip->locality), &buf, 1) < 0) > + if (tpm_tis_i2c_read(dev, TPM1_STS(chip->locality), &buf, 1) < 0) > return 0; > else > return buf; > @@ -286,7 +286,7 @@ static int tpm_tis_i2c_ready(struct udevice *dev) > u8 buf = TPM_STS_COMMAND_READY; > > debug("%s\n", __func__); > - rc = tpm_tis_i2c_write_long(dev, TPM_STS(chip->locality), &buf, 1); > + rc = tpm_tis_i2c_write_long(dev, TPM1_STS(chip->locality), &buf, 1); > if (rc) > debug("%s: rc=%d\n", __func__, rc); > > @@ -306,7 +306,7 @@ static ssize_t tpm_tis_i2c_get_burstcount(struct udevice *dev) > stop = chip->timeout_d; > do { > /* Note: STS is little endian */ > - addr = TPM_STS(chip->locality) + 1; > + addr = TPM1_STS(chip->locality) + 1; > if (tpm_tis_i2c_read(dev, addr, buf, 3) < 0) > burstcnt = 0; > else > @@ -360,7 +360,7 @@ static int tpm_tis_i2c_recv_data(struct udevice *dev, u8 *buf, size_t count) > if (burstcnt > (count - size)) > burstcnt = count - size; > > - rc = tpm_tis_i2c_read(dev, TPM_DATA_FIFO(chip->locality), > + rc = tpm_tis_i2c_read(dev, TPM1_DATA_FIFO(chip->locality), > &(buf[size]), burstcnt); > if (rc == 0) > size += burstcnt; > @@ -462,7 +462,7 @@ static int tpm_tis_i2c_send(struct udevice *dev, const u8 *buf, size_t len) > burstcnt = CONFIG_TPM_TIS_I2C_BURST_LIMITATION_LEN; > #endif /* CONFIG_TPM_TIS_I2C_BURST_LIMITATION */ > > - rc = tpm_tis_i2c_write(dev, TPM_DATA_FIFO(chip->locality), > + rc = tpm_tis_i2c_write(dev, TPM1_DATA_FIFO(chip->locality), > &(buf[count]), burstcnt); > if (rc == 0) > count += burstcnt; > @@ -482,7 +482,7 @@ static int tpm_tis_i2c_send(struct udevice *dev, const u8 *buf, size_t len) > } > > /* Go and do it */ > - rc = tpm_tis_i2c_write(dev, TPM_STS(chip->locality), &sts, 1); > + rc = tpm_tis_i2c_write(dev, TPM1_STS(chip->locality), &sts, 1); > if (rc < 0) > return rc; > debug("%s: done, rc=%d\n", __func__, rc); > @@ -525,7 +525,7 @@ static int tpm_tis_i2c_init(struct udevice *dev) > return rc; > > /* Read four bytes from DID_VID register */ > - if (tpm_tis_i2c_read(dev, TPM_DID_VID(0), (uchar *)&vendor, 4) < 0) { > + if (tpm_tis_i2c_read(dev, TPM1_DID_VID(0), (uchar *)&vendor, 4) < 0) { > tpm_tis_i2c_release_locality(dev, 0, 1); > return -EIO; > } > @@ -583,7 +583,7 @@ static int tpm_tis_i2c_close(struct udevice *dev) > return 0; > } > > -static int tpm_tis_get_desc(struct udevice *dev, char *buf, int size) > +static int tpm_tis_i2c_get_desc(struct udevice *dev, char *buf, int size) > { > struct tpm_chip *chip = dev_get_priv(dev); > > @@ -615,7 +615,7 @@ static int tpm_tis_i2c_probe(struct udevice *dev) > static const struct tpm_ops tpm_tis_i2c_ops = { > .open = tpm_tis_i2c_open, > .close = tpm_tis_i2c_close, > - .get_desc = tpm_tis_get_desc, > + .get_desc = tpm_tis_i2c_get_desc, > .send = tpm_tis_i2c_send, > .recv = tpm_tis_i2c_recv, > .cleanup = tpm_tis_i2c_cleanup, >
On Sun, 7 Nov 2021 at 14:33, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > With the upcoming TPM2 API, some of the function names are part of the new > header file. So switch conflicting driver defined function names and > defines. > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > drivers/tpm/tpm_tis_infineon.c | 34 +++++++++++++++++----------------- > 1 file changed, 17 insertions(+), 17 deletions(-) > Reviewed-by: Simon Glass <sjg@chromium.org>
Hi Heinrich On Mon, 8 Nov 2021 at 17:55, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > On 11/7/21 22:33, Ilias Apalodimas wrote: > > With the upcoming TPM2 API, some of the function names are part of the new > > header file. So switch conflicting driver defined function names and > > defines. > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > drivers/tpm/tpm_tis_infineon.c | 34 +++++++++++++++++----------------- > > 1 file changed, 17 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/tpm/tpm_tis_infineon.c b/drivers/tpm/tpm_tis_infineon.c > > index f414e5657db9..aa66e931bada 100644 > > --- a/drivers/tpm/tpm_tis_infineon.c > > +++ b/drivers/tpm/tpm_tis_infineon.c > > @@ -50,10 +50,10 @@ static const char * const chip_name[] = { > > [UNKNOWN] = "unknown/fallback to slb9635", > > }; > > > > -#define TPM_ACCESS(l) (0x0000 | ((l) << 4)) > > -#define TPM_STS(l) (0x0001 | ((l) << 4)) > > -#define TPM_DATA_FIFO(l) (0x0005 | ((l) << 4)) > > -#define TPM_DID_VID(l) (0x0006 | ((l) << 4)) > > +#define TPM1_ACCESS(l) (0x0000 | ((l) << 4)) > > +#define TPM1_STS(l) (0x0001 | ((l) << 4)) > > +#define TPM1_DATA_FIFO(l) (0x0005 | ((l) << 4)) > > +#define TPM1_DID_VID(l) (0x0006 | ((l) << 4)) > > To what does '1' relate here? > > * Do you mean TPMv1? Yea that was the intention > * Or are the constants Infenion specific? I think so. I can't find that definition in any of the TCG specs [1] [2] (search for 0f04 to get to the description table). > > If the constants are TPMv1 specific, they should be in a header file. > If the constants are Infineon specific we could put _INFINEON_ into the > constant name. Ok I'll swap 1 with INFINEON > > Linux has resolved the conflict by putting the TPMv2 TIS constants into > drivers/char/tpm/tpm_tis_core.h which is not included by > drivers/char/tpm/tpm_i2c_infineon.c. Yea and they still define them differently into that infineon TPM. [1] https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientTPMInterfaceSpecification_TIS__1-3_27_03212013.pdf [2] https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2-0-v1-03-22-170516_final.pdf Thanks /Ilias > > Best regards > > Heinrich > > > [...]
diff --git a/drivers/tpm/tpm_tis_infineon.c b/drivers/tpm/tpm_tis_infineon.c index f414e5657db9..aa66e931bada 100644 --- a/drivers/tpm/tpm_tis_infineon.c +++ b/drivers/tpm/tpm_tis_infineon.c @@ -50,10 +50,10 @@ static const char * const chip_name[] = { [UNKNOWN] = "unknown/fallback to slb9635", }; -#define TPM_ACCESS(l) (0x0000 | ((l) << 4)) -#define TPM_STS(l) (0x0001 | ((l) << 4)) -#define TPM_DATA_FIFO(l) (0x0005 | ((l) << 4)) -#define TPM_DID_VID(l) (0x0006 | ((l) << 4)) +#define TPM1_ACCESS(l) (0x0000 | ((l) << 4)) +#define TPM1_STS(l) (0x0001 | ((l) << 4)) +#define TPM1_DATA_FIFO(l) (0x0005 | ((l) << 4)) +#define TPM1_DID_VID(l) (0x0006 | ((l) << 4)) /* * tpm_tis_i2c_read() - read from TPM register @@ -197,7 +197,7 @@ static int tpm_tis_i2c_check_locality(struct udevice *dev, int loc) u8 buf; int rc; - rc = tpm_tis_i2c_read(dev, TPM_ACCESS(loc), &buf, 1); + rc = tpm_tis_i2c_read(dev, TPM1_ACCESS(loc), &buf, 1); if (rc < 0) return rc; @@ -215,12 +215,12 @@ static void tpm_tis_i2c_release_locality(struct udevice *dev, int loc, const u8 mask = TPM_ACCESS_REQUEST_PENDING | TPM_ACCESS_VALID; u8 buf; - if (tpm_tis_i2c_read(dev, TPM_ACCESS(loc), &buf, 1) < 0) + if (tpm_tis_i2c_read(dev, TPM1_ACCESS(loc), &buf, 1) < 0) return; if (force || (buf & mask) == mask) { buf = TPM_ACCESS_ACTIVE_LOCALITY; - tpm_tis_i2c_write(dev, TPM_ACCESS(loc), &buf, 1); + tpm_tis_i2c_write(dev, TPM1_ACCESS(loc), &buf, 1); } } @@ -240,7 +240,7 @@ static int tpm_tis_i2c_request_locality(struct udevice *dev, int loc) return rc; } - rc = tpm_tis_i2c_write(dev, TPM_ACCESS(loc), &buf, 1); + rc = tpm_tis_i2c_write(dev, TPM1_ACCESS(loc), &buf, 1); if (rc) { debug("%s: Failed to write to TPM: %d\n", __func__, rc); return rc; @@ -271,7 +271,7 @@ static u8 tpm_tis_i2c_status(struct udevice *dev) /* NOTE: Since i2c read may fail, return 0 in this case --> time-out */ u8 buf; - if (tpm_tis_i2c_read(dev, TPM_STS(chip->locality), &buf, 1) < 0) + if (tpm_tis_i2c_read(dev, TPM1_STS(chip->locality), &buf, 1) < 0) return 0; else return buf; @@ -286,7 +286,7 @@ static int tpm_tis_i2c_ready(struct udevice *dev) u8 buf = TPM_STS_COMMAND_READY; debug("%s\n", __func__); - rc = tpm_tis_i2c_write_long(dev, TPM_STS(chip->locality), &buf, 1); + rc = tpm_tis_i2c_write_long(dev, TPM1_STS(chip->locality), &buf, 1); if (rc) debug("%s: rc=%d\n", __func__, rc); @@ -306,7 +306,7 @@ static ssize_t tpm_tis_i2c_get_burstcount(struct udevice *dev) stop = chip->timeout_d; do { /* Note: STS is little endian */ - addr = TPM_STS(chip->locality) + 1; + addr = TPM1_STS(chip->locality) + 1; if (tpm_tis_i2c_read(dev, addr, buf, 3) < 0) burstcnt = 0; else @@ -360,7 +360,7 @@ static int tpm_tis_i2c_recv_data(struct udevice *dev, u8 *buf, size_t count) if (burstcnt > (count - size)) burstcnt = count - size; - rc = tpm_tis_i2c_read(dev, TPM_DATA_FIFO(chip->locality), + rc = tpm_tis_i2c_read(dev, TPM1_DATA_FIFO(chip->locality), &(buf[size]), burstcnt); if (rc == 0) size += burstcnt; @@ -462,7 +462,7 @@ static int tpm_tis_i2c_send(struct udevice *dev, const u8 *buf, size_t len) burstcnt = CONFIG_TPM_TIS_I2C_BURST_LIMITATION_LEN; #endif /* CONFIG_TPM_TIS_I2C_BURST_LIMITATION */ - rc = tpm_tis_i2c_write(dev, TPM_DATA_FIFO(chip->locality), + rc = tpm_tis_i2c_write(dev, TPM1_DATA_FIFO(chip->locality), &(buf[count]), burstcnt); if (rc == 0) count += burstcnt; @@ -482,7 +482,7 @@ static int tpm_tis_i2c_send(struct udevice *dev, const u8 *buf, size_t len) } /* Go and do it */ - rc = tpm_tis_i2c_write(dev, TPM_STS(chip->locality), &sts, 1); + rc = tpm_tis_i2c_write(dev, TPM1_STS(chip->locality), &sts, 1); if (rc < 0) return rc; debug("%s: done, rc=%d\n", __func__, rc); @@ -525,7 +525,7 @@ static int tpm_tis_i2c_init(struct udevice *dev) return rc; /* Read four bytes from DID_VID register */ - if (tpm_tis_i2c_read(dev, TPM_DID_VID(0), (uchar *)&vendor, 4) < 0) { + if (tpm_tis_i2c_read(dev, TPM1_DID_VID(0), (uchar *)&vendor, 4) < 0) { tpm_tis_i2c_release_locality(dev, 0, 1); return -EIO; } @@ -583,7 +583,7 @@ static int tpm_tis_i2c_close(struct udevice *dev) return 0; } -static int tpm_tis_get_desc(struct udevice *dev, char *buf, int size) +static int tpm_tis_i2c_get_desc(struct udevice *dev, char *buf, int size) { struct tpm_chip *chip = dev_get_priv(dev); @@ -615,7 +615,7 @@ static int tpm_tis_i2c_probe(struct udevice *dev) static const struct tpm_ops tpm_tis_i2c_ops = { .open = tpm_tis_i2c_open, .close = tpm_tis_i2c_close, - .get_desc = tpm_tis_get_desc, + .get_desc = tpm_tis_i2c_get_desc, .send = tpm_tis_i2c_send, .recv = tpm_tis_i2c_recv, .cleanup = tpm_tis_i2c_cleanup,
With the upcoming TPM2 API, some of the function names are part of the new header file. So switch conflicting driver defined function names and defines. Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- drivers/tpm/tpm_tis_infineon.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)