[U-Boot] [PATCH 01/25] tpm: Remove old pre-driver-model I2C code

christophe.ricard
2015-08-11T21:41:38Z

Hi Simon,

In here you are removing usage of i2c_read/i2c_write for
dm_i2c_read/dm_i2c_write.
This require boards to support DM_I2C which is not the case with example
beagleboard or boards using omap.

Will you drop all drivers not supporting DM in the future ?

Other than that i have proposed about the same change here:
http://lists.denx.de/pipermail/u-boot/2015-August/222598.html

Acked-by: Christophe Ricard <christophe-h.ricard@st.com>

Best Regards
Christophe
On 11/08/2015 16:47, Simon Glass wrote:
> This is not used anymore by any board so drop it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> drivers/tpm/tpm.c | 108 ++--------------------------------------------
> drivers/tpm/tpm_private.h | 4 +-
> drivers/tpm/tpm_tis_i2c.c | 74 +------------------------------
> 3 files changed, 7 insertions(+), 179 deletions(-)
>
> diff --git a/drivers/tpm/tpm.c b/drivers/tpm/tpm.c
> index a650892..24997f6 100644
> --- a/drivers/tpm/tpm.c
> +++ b/drivers/tpm/tpm.c
> @@ -49,13 +49,7 @@ DECLARE_GLOBAL_DATA_PTR;
>
> /* TPM configuration */
> struct tpm {
> -#ifdef CONFIG_DM_I2C
> struct udevice *dev;
> -#else
> - int i2c_bus;
> - int slave_addr;
> - int old_bus;
> -#endif
> char inited;
> } tpm;
>
> @@ -441,7 +435,6 @@ out:
> return rc;
> }
>
> -#ifdef CONFIG_DM_I2C
> static int tpm_open_dev(struct udevice *dev)
> {
> int rc;
> @@ -449,24 +442,12 @@ static int tpm_open_dev(struct udevice *dev)
> debug("%s: start\n", __func__);
> if (g_chip.is_open)
> return -EBUSY;
> - rc = tpm_vendor_init_dev(dev);
> + rc = tpm_vendor_init(dev);
> if (rc < 0)
> g_chip.is_open = 0;
> return rc;
> }
> -#else
> -static int tpm_open(uint32_t dev_addr)
> -{
> - int rc;
>
> - if (g_chip.is_open)
> - return -EBUSY;
> - rc = tpm_vendor_init(dev_addr);
> - if (rc < 0)
> - g_chip.is_open = 0;
> - return rc;
> -}
> -#endif
> static void tpm_close(void)
> {
> if (g_chip.is_open) {
> @@ -475,42 +456,6 @@ static void tpm_close(void)
> }
> }
>
> -static int tpm_select(void)
> -{
> -#ifndef CONFIG_DM_I2C
> - int ret;
> -
> - tpm.old_bus = i2c_get_bus_num();
> - if (tpm.old_bus != tpm.i2c_bus) {
> - ret = i2c_set_bus_num(tpm.i2c_bus);
> - if (ret) {
> - debug("%s: Fail to set i2c bus %d\n", __func__,
> - tpm.i2c_bus);
> - return -1;
> - }
> - }
> -#endif
> - return 0;
> -}
> -
> -static int tpm_deselect(void)
> -{
> -#ifndef CONFIG_DM_I2C
> - int ret;
> -
> - if (tpm.old_bus != i2c_get_bus_num()) {
> - ret = i2c_set_bus_num(tpm.old_bus);
> - if (ret) {
> - debug("%s: Fail to restore i2c bus %d\n",
> - __func__, tpm.old_bus);
> - return -1;
> - }
> - }
> - tpm.old_bus = -1;
> -#endif
> - return 0;
> -}
> -
> /**
> * Decode TPM configuration.
> *
> @@ -520,8 +465,11 @@ static int tpm_deselect(void)
> static int tpm_decode_config(struct tpm *dev)
> {
> const void *blob = gd->fdt_blob;
> + struct udevice *bus;
> + int chip_addr;
> int parent;
> int node;
> + int ret;
>
> node = fdtdec_next_compatible(blob, 0, COMPAT_INFINEON_SLB9635_TPM);
> if (node < 0) {
> @@ -537,10 +485,6 @@ static int tpm_decode_config(struct tpm *dev)
> debug("%s: Cannot find node parent\n", __func__);
> return -1;
> }
> -#ifdef CONFIG_DM_I2C
> - struct udevice *bus;
> - int chip_addr;
> - int ret;
>
> /*
> * TODO(sjg at chromium.org): Remove this when driver model supports
> @@ -569,15 +513,6 @@ static int tpm_decode_config(struct tpm *dev)
> fdt_get_name(blob, node, NULL), ret);
> return ret;
> }
> -#else
> - int i2c_bus;
> -
> - i2c_bus = i2c_get_bus_num_fdt(parent);
> - if (i2c_bus < 0)
> - return -1;
> - dev->i2c_bus = i2c_bus;
> - dev->slave_addr = fdtdec_get_addr(blob, node, "reg");
> -#endif
>
> return 0;
> }
> @@ -602,22 +537,6 @@ int tis_init(void)
> if (tpm_decode_config(&tpm))
> return -1;
>
> - if (tpm_select())
> - return -1;
> -
> -#ifndef CONFIG_DM_I2C
> - /*
> - * Probe TPM twice; the first probing might fail because TPM is asleep,
> - * and the probing can wake up TPM.
> - */
> - if (i2c_probe(tpm.slave_addr) && i2c_probe(tpm.slave_addr)) {
> - debug("%s: fail to probe i2c addr 0x%x\n", __func__,
> - tpm.slave_addr);
> - return -1;
> - }
> -#endif
> -
> - tpm_deselect();
> debug("%s: done\n", __func__);
>
> tpm.inited = 1;
> @@ -632,16 +551,7 @@ int tis_open(void)
> if (!tpm.inited)
> return -1;
>
> - if (tpm_select())
> - return -1;
> -
> -#ifdef CONFIG_DM_I2C
> rc = tpm_open_dev(tpm.dev);
> -#else
> - rc = tpm_open(tpm.slave_addr);
> -#endif
> -
> - tpm_deselect();
>
> return rc;
> }
> @@ -651,13 +561,8 @@ int tis_close(void)
> if (!tpm.inited)
> return -1;
>
> - if (tpm_select())
> - return -1;
> -
> tpm_close();
>
> - tpm_deselect();
> -
> return 0;
> }
>
> @@ -675,13 +580,8 @@ int tis_sendrecv(const uint8_t *sendbuf, size_t sbuf_size,
>
> memcpy(buf, sendbuf, sbuf_size);
>
> - if (tpm_select())
> - return -1;
> -
> len = tpm_transmit(buf, sbuf_size);
>
> - tpm_deselect();
> -
> if (len < 10) {
> *rbuf_len = 0;
> return -1;
> diff --git a/drivers/tpm/tpm_private.h b/drivers/tpm/tpm_private.h
> index 8894c98..daaf8b8 100644
> --- a/drivers/tpm/tpm_private.h
> +++ b/drivers/tpm/tpm_private.h
> @@ -129,10 +129,8 @@ struct tpm_cmd_t {
>
> struct tpm_chip *tpm_register_hardware(const struct tpm_vendor_specific *);
>
> -int tpm_vendor_init(uint32_t dev_addr);
> -
> struct udevice;
> -int tpm_vendor_init_dev(struct udevice *dev);
> +int tpm_vendor_init(struct udevice *dev);
>
> void tpm_vendor_cleanup(struct tpm_chip *chip);
>
> diff --git a/drivers/tpm/tpm_tis_i2c.c b/drivers/tpm/tpm_tis_i2c.c
> index ee4dfea..a7082e6 100644
> --- a/drivers/tpm/tpm_tis_i2c.c
> +++ b/drivers/tpm/tpm_tis_i2c.c
> @@ -50,9 +50,6 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> -/* Address of the TPM on the I2C bus */
> -#define TPM_I2C_ADDR 0x20
> -
> /* Max buffer size supported by our tpm */
> #define TPM_DEV_BUFSIZE 1260
>
> @@ -73,13 +70,6 @@ DECLARE_GLOBAL_DATA_PTR;
>
> #define TPM_HEADER_SIZE 10
>
> -/*
> - * Expected value for DIDVID register
> - *
> - * The only device the system knows about at this moment is Infineon slb9635.
> - */
> -#define TPM_TIS_I2C_DID_VID 0x000b15d1L
> -
> enum tis_access {
> TPM_ACCESS_VALID = 0x80,
> TPM_ACCESS_ACTIVE_LOCALITY = 0x20,
> @@ -123,21 +113,11 @@ static const char * const chip_name[] = {
>
> /* Structure to store I2C TPM specific stuff */
> struct tpm_dev {
> -#ifdef CONFIG_DM_I2C
> struct udevice *dev;
> -#else
> - uint addr;
> -#endif
> u8 buf[TPM_DEV_BUFSIZE + sizeof(u8)]; /* Max buffer size + addr */
> enum i2c_chip_type chip_type;
> };
>
> -static struct tpm_dev tpm_dev = {
> -#ifndef CONFIG_DM_I2C
> - .addr = TPM_I2C_ADDR
> -#endif
> -};
> -
> static struct tpm_dev tpm_dev;
>
> /*
> @@ -163,12 +143,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
> if ((tpm_dev.chip_type == SLB9635) || (tpm_dev.chip_type == UNKNOWN)) {
> /* slb9635 protocol should work in both cases */
> for (count = 0; count < MAX_COUNT; count++) {
> -#ifdef CONFIG_DM_I2C
> rc = dm_i2c_write(tpm_dev.dev, 0, (uchar *)&addrbuf, 1);
> -#else
> - rc = i2c_write(tpm_dev.addr, 0, 0,
> - (uchar *)&addrbuf, 1);
> -#endif
> if (rc == 0)
> break; /* Success, break to skip sleep */
> udelay(SLEEP_DURATION);
> @@ -182,11 +157,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
> */
> for (count = 0; count < MAX_COUNT; count++) {
> udelay(SLEEP_DURATION);
> -#ifdef CONFIG_DM_I2C
> rc = dm_i2c_read(tpm_dev.dev, 0, buffer, len);
> -#else
> - rc = i2c_read(tpm_dev.addr, 0, 0, buffer, len);
> -#endif
> if (rc == 0)
> break; /* success, break to skip sleep */
> }
> @@ -199,11 +170,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
> * be safe on the safe side.
> */
> for (count = 0; count < MAX_COUNT; count++) {
> -#ifdef CONFIG_DM_I2C
> rc = dm_i2c_read(tpm_dev.dev, addr, buffer, len);
> -#else
> - rc = i2c_read(tpm_dev.addr, addr, 1, buffer, len);
> -#endif
> if (rc == 0)
> break; /* break here to skip sleep */
> udelay(SLEEP_DURATION);
> @@ -224,20 +191,8 @@ static int iic_tpm_write_generic(u8 addr, u8 *buffer, size_t len,
> int rc = 0;
> int count;
>
> - /* Prepare send buffer */
> -#ifndef CONFIG_DM_I2C
> - tpm_dev.buf[0] = addr;
> - memcpy(&(tpm_dev.buf[1]), buffer, len);
> - buffer = tpm_dev.buf;
> - len++;
> -#endif
> -
> for (count = 0; count < max_count; count++) {
> -#ifdef CONFIG_DM_I2C
> rc = dm_i2c_write(tpm_dev.dev, addr, buffer, len);
> -#else
> - rc = i2c_write(tpm_dev.addr, 0, 0, buffer, len);
> -#endif
> if (rc == 0)
> break; /* Success, break to skip sleep */
> udelay(sleep_time);
> @@ -597,12 +552,13 @@ static enum i2c_chip_type tpm_vendor_chip_type(void)
> return UNKNOWN;
> }
>
> -static int tpm_vendor_init_common(void)
> +int tpm_vendor_init(struct udevice *dev)
> {
> struct tpm_chip *chip;
> u32 vendor;
> u32 expected_did_vid;
>
> + tpm_dev.dev = dev;
> tpm_dev.chip_type = tpm_vendor_chip_type();
>
> chip = tpm_register_hardware(&tpm_tis_i2c);
> @@ -651,32 +607,6 @@ static int tpm_vendor_init_common(void)
> return 0;
> }
>
> -#ifdef CONFIG_DM_I2C
> -/* Initialisation of i2c tpm */
> -int tpm_vendor_init_dev(struct udevice *dev)
> -{
> - tpm_dev.dev = dev;
> - return tpm_vendor_init_common();
> -}
> -#else
> -/* Initialisation of i2c tpm */
> -int tpm_vendor_init(uint32_t dev_addr)
> -{
> - uint old_addr;
> - int rc = 0;
> -
> - old_addr = tpm_dev.addr;
> - if (dev_addr != 0)
> - tpm_dev.addr = dev_addr;
> -
> - rc = tpm_vendor_init_common();
> - if (rc)
> - tpm_dev.addr = old_addr;
> -
> - return rc;
> -}
> -#endif
> -
> void tpm_vendor_cleanup(struct tpm_chip *chip)
> {
> release_locality(chip, chip->vendor.locality, 1);