[U-Boot] [PATCH 08/25] tpm: tpm_tis_i2c: Drop struct tpm_vendor_specific

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

Hi Simon,

Locality concept are valid almost on any chip assuming if no locality 
are supported the default one is locality 0.
I would leave this change open for discussion.

However, as per patch 06 & 07, i would keep req_complete_mask, 
req_complete_val, req_canceled, timeout_a, timeout_b, timeout_c, 
timeout_d in tpm_vendor_specific structure as this is chip specific.

I really think tpm_vendor_specific is usefull for managing different 
kind of TPM "the same way"/following standards.

Best Regards
Christophe
On 11/08/2015 16:48, Simon Glass wrote:
> This function is misnamed since it only applies to a single driver. Merge
> its fields into its parent.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   drivers/tpm/tpm_tis_i2c.c | 79 ++++++++++++++++++-----------------------------
>   drivers/tpm/tpm_tis_i2c.h | 16 +++-------
>   2 files changed, 35 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/tpm/tpm_tis_i2c.c b/drivers/tpm/tpm_tis_i2c.c
> index 5615ffc..3c4e6c5 100644
> --- a/drivers/tpm/tpm_tis_i2c.c
> +++ b/drivers/tpm/tpm_tis_i2c.c
> @@ -531,7 +531,7 @@ static int check_locality(struct tpm_chip *chip, int loc)
>   		return rc;
>   
>   	if ((buf & mask) == mask) {
> -		chip->vendor.locality = loc;
> +		chip->locality = loc;
>   		return loc;
>   	}
>   
> @@ -567,7 +567,7 @@ static int request_locality(struct tpm_chip *chip, int loc)
>   
>   	/* Wait for burstcount */
>   	start = get_timer(0);
> -	stop = chip->vendor.timeout_a;
> +	stop = chip->timeout_a;
>   	do {
>   		if (check_locality(chip, loc) >= 0)
>   			return loc;
> @@ -582,7 +582,7 @@ static u8 tpm_tis_i2c_status(struct tpm_chip *chip)
>   	/* NOTE: Since i2c read may fail, return 0 in this case --> time-out */
>   	u8 buf;
>   
> -	if (iic_tpm_read(TPM_STS(chip->vendor.locality), &buf, 1) < 0)
> +	if (iic_tpm_read(TPM_STS(chip->locality), &buf, 1) < 0)
>   		return 0;
>   	else
>   		return buf;
> @@ -596,7 +596,7 @@ static void tpm_tis_i2c_ready(struct tpm_chip *chip)
>   	u8 buf = TPM_STS_COMMAND_READY;
>   
>   	debug("%s\n", __func__);
> -	rc = iic_tpm_write_long(TPM_STS(chip->vendor.locality), &buf, 1);
> +	rc = iic_tpm_write_long(TPM_STS(chip->locality), &buf, 1);
>   	if (rc)
>   		debug("%s: rc=%d\n", __func__, rc);
>   }
> @@ -610,10 +610,10 @@ static ssize_t get_burstcount(struct tpm_chip *chip)
>   	/* Wait for burstcount */
>   	/* XXX: Which timeout value? Spec has 2 answers (c & d) */
>   	start = get_timer(0);
> -	stop = chip->vendor.timeout_d;
> +	stop = chip->timeout_d;
>   	do {
>   		/* Note: STS is little endian */
> -		addr = TPM_STS(chip->vendor.locality) + 1;
> +		addr = TPM_STS(chip->locality) + 1;
>   		if (iic_tpm_read(addr, buf, 3) < 0)
>   			burstcnt = 0;
>   		else
> @@ -666,7 +666,7 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
>   		if (burstcnt > (count - size))
>   			burstcnt = count - size;
>   
> -		rc = iic_tpm_read(TPM_DATA_FIFO(chip->vendor.locality),
> +		rc = iic_tpm_read(TPM_DATA_FIFO(chip->locality),
>   				&(buf[size]), burstcnt);
>   		if (rc == 0)
>   			size += burstcnt;
> @@ -708,7 +708,7 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>   		goto out;
>   	}
>   
> -	wait_for_stat(chip, TPM_STS_VALID, chip->vendor.timeout_c, &status);
> +	wait_for_stat(chip, TPM_STS_VALID, chip->timeout_c, &status);
>   	if (status & TPM_STS_DATA_AVAIL) {  /* Retry? */
>   		error("Error left over data\n");
>   		size = -EIO;
> @@ -722,7 +722,7 @@ out:
>   	 * so we sleep rather than keeping the bus busy
>   	 */
>   	udelay(2000);
> -	release_locality(chip, chip->vendor.locality, 0);
> +	release_locality(chip, chip->locality, 0);
>   
>   	return size;
>   }
> @@ -746,7 +746,7 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
>   	if ((status & TPM_STS_COMMAND_READY) == 0) {
>   		tpm_tis_i2c_ready(chip);
>   		if (wait_for_stat(chip, TPM_STS_COMMAND_READY,
> -				  chip->vendor.timeout_b, &status) < 0) {
> +				  chip->timeout_b, &status) < 0) {
>   			rc = -ETIME;
>   			goto out_err;
>   		}
> @@ -768,7 +768,7 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
>   			burstcnt = CONFIG_TPM_TIS_I2C_BURST_LIMITATION;
>   #endif /* CONFIG_TPM_TIS_I2C_BURST_LIMITATION */
>   
> -		rc = iic_tpm_write(TPM_DATA_FIFO(chip->vendor.locality),
> +		rc = iic_tpm_write(TPM_DATA_FIFO(chip->locality),
>   				&(buf[count]), burstcnt);
>   		if (rc == 0)
>   			count += burstcnt;
> @@ -779,7 +779,7 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
>   				goto out_err;
>   			}
>   			rc = wait_for_stat(chip, TPM_STS_VALID,
> -					   chip->vendor.timeout_c, &status);
> +					   chip->timeout_c, &status);
>   			if (rc)
>   				goto out_err;
>   
> @@ -791,7 +791,7 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
>   	}
>   
>   	/* Go and do it */
> -	iic_tpm_write(TPM_STS(chip->vendor.locality), &sts, 1);
> +	iic_tpm_write(TPM_STS(chip->locality), &sts, 1);
>   	debug("done\n");
>   
>   	return len;
> @@ -804,18 +804,11 @@ out_err:
>   	 * so we sleep rather than keeping the bus busy
>   	 */
>   	udelay(2000);
> -	release_locality(chip, chip->vendor.locality, 0);
> +	release_locality(chip, chip->locality, 0);
>   
>   	return rc;
>   }
>   
> -static struct tpm_vendor_specific tpm_tis_i2c = {
> -	.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,
> -};
> -
> -
>   static enum i2c_chip_type tpm_vendor_chip_type(void)
>   {
>   #ifdef CONFIG_OF_CONTROL
> @@ -832,25 +825,25 @@ static enum i2c_chip_type tpm_vendor_chip_type(void)
>   
>   int tpm_vendor_init(struct udevice *dev)
>   {
> -	struct tpm_chip *chip;
> +	struct tpm_chip *chip = &g_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);
> -	if (chip < 0)
> -		return -ENODEV;
> +	chip->is_open = 1;
>   
>   	/* Disable interrupts (not supported) */
> -	chip->vendor.irq = 0;
> +	chip->irq = 0;
>   
>   	/* Default timeouts */
> -	chip->vendor.timeout_a = TIS_SHORT_TIMEOUT;
> -	chip->vendor.timeout_b = TIS_LONG_TIMEOUT;
> -	chip->vendor.timeout_c = TIS_SHORT_TIMEOUT;
> -	chip->vendor.timeout_d = TIS_SHORT_TIMEOUT;
> +	chip->timeout_a = TIS_SHORT_TIMEOUT;
> +	chip->timeout_b = TIS_LONG_TIMEOUT;
> +	chip->timeout_c = TIS_SHORT_TIMEOUT;
> +	chip->timeout_d = TIS_SHORT_TIMEOUT;
> +	chip->req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID;
> +	chip->req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID;
> +	chip->req_canceled = TPM_STS_COMMAND_READY;
>   
>   	if (request_locality(chip, 0) < 0)
>   		return  -ENODEV;
> @@ -887,7 +880,7 @@ int tpm_vendor_init(struct udevice *dev)
>   
>   void tpm_vendor_cleanup(struct tpm_chip *chip)
>   {
> -	release_locality(chip, chip->vendor.locality, 1);
> +	release_locality(chip, chip->locality, 1);
>   }
>   
>   /* Returns max number of milliseconds to wait */
> @@ -906,7 +899,7 @@ static unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
>   	}
>   
>   	if (duration_idx != TPM_UNDEFINED)
> -		duration = chip->vendor.duration[duration_idx];
> +		duration = chip->duration[duration_idx];
>   
>   	if (duration <= 0)
>   		return 2 * 60 * HZ; /* Two minutes timeout */
> @@ -943,7 +936,7 @@ static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz)
>   		goto out;
>   	}
>   
> -	if (chip->vendor.irq)
> +	if (chip->irq)
>   		goto out_recv;
>   
>   	start = get_timer(0);
> @@ -951,13 +944,13 @@ static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz)
>   	do {
>   		debug("waiting for status... %ld %ld\n", start, stop);
>   		u8 status = tpm_tis_i2c_status(chip);
> -		if ((status & chip->vendor.req_complete_mask) ==
> -		    chip->vendor.req_complete_val) {
> +		if ((status & chip->req_complete_mask) ==
> +		    chip->req_complete_val) {
>   			debug("...got it;\n");
>   			goto out_recv;
>   		}
>   
> -		if (status == chip->vendor.req_canceled) {
> +		if (status == chip->req_canceled) {
>   			error("Operation Canceled\n");
>   			rc = -ECANCELED;
>   			goto out;
> @@ -1062,18 +1055,6 @@ static int tpm_decode_config(struct tpm *dev)
>   	return 0;
>   }
>   
> -struct tpm_chip *tpm_register_hardware(const struct tpm_vendor_specific *entry)
> -{
> -	struct tpm_chip *chip;
> -
> -	/* Driver specific per-device data */
> -	chip = &g_chip;
> -	memcpy(&chip->vendor, entry, sizeof(struct tpm_vendor_specific));
> -	chip->is_open = 1;
> -
> -	return chip;
> -}
> -
>   int tis_init(void)
>   {
>   	if (tpm.inited)
> diff --git a/drivers/tpm/tpm_tis_i2c.h b/drivers/tpm/tpm_tis_i2c.h
> index 426c519..2a4ad77 100644
> --- a/drivers/tpm/tpm_tis_i2c.h
> +++ b/drivers/tpm/tpm_tis_i2c.h
> @@ -35,21 +35,17 @@ enum tpm_timeout {
>   
>   struct tpm_chip;
>   
> -struct tpm_vendor_specific {
> -	const u8 req_complete_mask;
> -	const u8 req_complete_val;
> -	const u8 req_canceled;
> +struct tpm_chip {
> +	int is_open;
> +	u8 req_complete_mask;
> +	u8 req_complete_val;
> +	u8 req_canceled;
>   	int irq;
>   	int locality;
>   	unsigned long timeout_a, timeout_b, timeout_c, timeout_d;  /* msec */
>   	unsigned long duration[3];  /* msec */
>   };
>   
> -struct tpm_chip {
> -	int is_open;
> -	struct tpm_vendor_specific vendor;
> -};
> -
>   struct tpm_input_header {
>   	__be16 tag;
>   	__be32 length;
> @@ -106,8 +102,6 @@ struct tpm_cmd_t {
>   	union tpm_cmd_params params;
>   } __packed;
>   
> -struct tpm_chip *tpm_register_hardware(const struct tpm_vendor_specific *);
> -
>   struct udevice;
>   int tpm_vendor_init(struct udevice *dev);
>