[U-Boot] [PATCH 09/25] tpm: tpm_tis_i2c: Merge struct tpm_dev into tpm_chip

[christophe.ricard <christophe.ricard@gmail.com>]

Hi Simon,

As per my delivery tentative, don't you think driver specific data could 
be stored in a void *priv field in struct tpm_vendor_specific ?
This how we manage such information in Linux drivers.

Please have a look in tpm_private.h changes:
http://lists.denx.de/pipermail/u-boot/2015-August/222598.html

Best Regards
Christophe
On 11/08/2015 16:48, Simon Glass wrote:
> There are too many structures storing the same sort of information. Move the
> fields from struct tpm_dev into struct tpm_chip and remove the former
> struct.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   drivers/tpm/tpm_tis_i2c.c | 52 +++++++++++++----------------------------------
>   drivers/tpm/tpm_tis_i2c.h | 17 ++++++++++------
>   2 files changed, 25 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/tpm/tpm_tis_i2c.c b/drivers/tpm/tpm_tis_i2c.c
> index 3c4e6c5..70d21ca 100644
> --- a/drivers/tpm/tpm_tis_i2c.c
> +++ b/drivers/tpm/tpm_tis_i2c.c
> @@ -34,9 +34,6 @@
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> -/* Max buffer size supported by our tpm */
> -#define TPM_DEV_BUFSIZE		1260
> -
>   /* Max number of iterations after i2c NAK */
>   #define MAX_COUNT		3
>   
> @@ -78,12 +75,6 @@ enum tis_defaults {
>   #define TPM_TIS_I2C_DID_VID_9635 0x000b15d1L
>   #define TPM_TIS_I2C_DID_VID_9645 0x001a15d1L
>   
> -enum i2c_chip_type {
> -	SLB9635,
> -	SLB9645,
> -	UNKNOWN,
> -};
> -
>   static const char * const chip_name[] = {
>   	[SLB9635] = "slb9635tt",
>   	[SLB9645] = "slb9645tt",
> @@ -390,18 +381,8 @@ struct tpm {
>   	char inited;
>   } tpm;
>   
> -/* Global structure for tpm chip data */
>   static struct tpm_chip g_chip;
>   
> -/* Structure to store I2C TPM specific stuff */
> -struct tpm_dev {
> -	struct udevice *dev;
> -	u8 buf[TPM_DEV_BUFSIZE + sizeof(u8)];  /* Max buffer size + addr */
> -	enum i2c_chip_type chip_type;
> -};
> -
> -static struct tpm_dev tpm_dev;
> -
>   /*
>    * iic_tpm_read() - read from TPM register
>    * @addr: register address to read from
> @@ -422,10 +403,10 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
>   	int count;
>   	uint32_t addrbuf = addr;
>   
> -	if ((tpm_dev.chip_type == SLB9635) || (tpm_dev.chip_type == UNKNOWN)) {
> +	if ((g_chip.chip_type == SLB9635) || (g_chip.chip_type == UNKNOWN)) {
>   		/* slb9635 protocol should work in both cases */
>   		for (count = 0; count < MAX_COUNT; count++) {
> -			rc = dm_i2c_write(tpm_dev.dev, 0, (uchar *)&addrbuf, 1);
> +			rc = dm_i2c_write(g_chip.dev, 0, (uchar *)&addrbuf, 1);
>   			if (rc == 0)
>   				break;  /* Success, break to skip sleep */
>   			udelay(SLEEP_DURATION);
> @@ -439,7 +420,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
>   		 */
>   		for (count = 0; count < MAX_COUNT; count++) {
>   			udelay(SLEEP_DURATION);
> -			rc = dm_i2c_read(tpm_dev.dev, 0, buffer, len);
> +			rc = dm_i2c_read(g_chip.dev, 0, buffer, len);
>   			if (rc == 0)
>   				break;  /* success, break to skip sleep */
>   		}
> @@ -452,7 +433,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++) {
> -			rc = dm_i2c_read(tpm_dev.dev, addr, buffer, len);
> +			rc = dm_i2c_read(g_chip.dev, addr, buffer, len);
>   			if (rc == 0)
>   				break;  /* break here to skip sleep */
>   			udelay(SLEEP_DURATION);
> @@ -474,7 +455,7 @@ static int iic_tpm_write_generic(u8 addr, u8 *buffer, size_t len,
>   	int count;
>   
>   	for (count = 0; count < max_count; count++) {
> -		rc = dm_i2c_write(tpm_dev.dev, addr, buffer, len);
> +		rc = dm_i2c_write(g_chip.dev, addr, buffer, len);
>   		if (rc == 0)
>   			break;  /* Success, break to skip sleep */
>   		udelay(sleep_time);
> @@ -809,7 +790,7 @@ out_err:
>   	return rc;
>   }
>   
> -static enum i2c_chip_type tpm_vendor_chip_type(void)
> +static enum i2c_chip_type tpm_tis_i2c_chip_type(void)
>   {
>   #ifdef CONFIG_OF_CONTROL
>   	const void *blob = gd->fdt_blob;
> @@ -823,14 +804,14 @@ static enum i2c_chip_type tpm_vendor_chip_type(void)
>   	return UNKNOWN;
>   }
>   
> -int tpm_vendor_init(struct udevice *dev)
> +static int tpm_tis_i2c_init(struct udevice *dev)
>   {
>   	struct tpm_chip *chip = &g_chip;
>   	u32 vendor;
>   	u32 expected_did_vid;
>   
> -	tpm_dev.dev = dev;
> -	tpm_dev.chip_type = tpm_vendor_chip_type();
> +	g_chip.dev = dev;
> +	g_chip.chip_type = tpm_tis_i2c_chip_type();
>   	chip->is_open = 1;
>   
>   	/* Disable interrupts (not supported) */
> @@ -854,7 +835,7 @@ int tpm_vendor_init(struct udevice *dev)
>   		return -EIO;
>   	}
>   
> -	if (tpm_dev.chip_type == SLB9635) {
> +	if (g_chip.chip_type == SLB9635) {
>   		vendor = be32_to_cpu(vendor);
>   		expected_did_vid = TPM_TIS_I2C_DID_VID_9635;
>   	} else {
> @@ -862,13 +843,13 @@ int tpm_vendor_init(struct udevice *dev)
>   		expected_did_vid = TPM_TIS_I2C_DID_VID_9645;
>   	}
>   
> -	if (tpm_dev.chip_type != UNKNOWN && vendor != expected_did_vid) {
> +	if (g_chip.chip_type != UNKNOWN && vendor != expected_did_vid) {
>   		error("Vendor id did not match! ID was %08x\n", vendor);
>   		return -ENODEV;
>   	}
>   
>   	debug("1.2 TPM (chip type %s device-id 0x%X)\n",
> -	      chip_name[tpm_dev.chip_type], vendor >> 16);
> +	      chip_name[g_chip.chip_type], vendor >> 16);
>   
>   	/*
>   	 * A timeout query to TPM can be placed here.
> @@ -878,11 +859,6 @@ int tpm_vendor_init(struct udevice *dev)
>   	return 0;
>   }
>   
> -void tpm_vendor_cleanup(struct tpm_chip *chip)
> -{
> -	release_locality(chip, chip->locality, 1);
> -}
> -
>   /* Returns max number of milliseconds to wait */
>   static unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
>   		u32 ordinal)
> @@ -980,7 +956,7 @@ 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);
> +	rc = tpm_tis_i2c_init(dev);
>   	if (rc < 0)
>   		g_chip.is_open = 0;
>   	return rc;
> @@ -989,7 +965,7 @@ static int tpm_open_dev(struct udevice *dev)
>   static void tpm_close(void)
>   {
>   	if (g_chip.is_open) {
> -		tpm_vendor_cleanup(&g_chip);
> +		release_locality(&g_chip, g_chip.locality, 1);
>   		g_chip.is_open = 0;
>   	}
>   }
> diff --git a/drivers/tpm/tpm_tis_i2c.h b/drivers/tpm/tpm_tis_i2c.h
> index 2a4ad77..0fec464 100644
> --- a/drivers/tpm/tpm_tis_i2c.h
> +++ b/drivers/tpm/tpm_tis_i2c.h
> @@ -33,7 +33,14 @@ enum tpm_timeout {
>   #define TPM_RSP_SIZE_BYTE	2
>   #define TPM_RSP_RC_BYTE		6
>   
> -struct tpm_chip;
> +/* Max buffer size supported by our tpm */
> +#define TPM_DEV_BUFSIZE		1260
> +
> +enum i2c_chip_type {
> +	SLB9635,
> +	SLB9645,
> +	UNKNOWN,
> +};
>   
>   struct tpm_chip {
>   	int is_open;
> @@ -44,6 +51,9 @@ struct tpm_chip {
>   	int locality;
>   	unsigned long timeout_a, timeout_b, timeout_c, timeout_d;  /* msec */
>   	unsigned long duration[3];  /* msec */
> +	struct udevice *dev;
> +	u8 buf[TPM_DEV_BUFSIZE + sizeof(u8)];  /* Max buffer size + addr */
> +	enum i2c_chip_type chip_type;
>   };
>   
>   struct tpm_input_header {
> @@ -102,9 +112,4 @@ struct tpm_cmd_t {
>   	union tpm_cmd_params params;
>   } __packed;
>   
> -struct udevice;
> -int tpm_vendor_init(struct udevice *dev);
> -
> -void tpm_vendor_cleanup(struct tpm_chip *chip);
> -
>   #endif