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

christophe.ricard
2015-08-11T21:46:56Z

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