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

christophe.ricard christophe.ricard at gmail.com
Tue Aug 11 23:41:38 CEST 2015


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 at 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 at 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);



More information about the U-Boot mailing list