[U-Boot] [PATCH 12/25] tpm: tpm_tis_i2c: Simplify init code

christophe.ricard
2015-08-11T21:45:15Z

Hi Simon,

Nothing much to say on tpm_tis_i2c, however this could be even more
generic if providing init ops to be specified by TPM driver developers.
For example:

+struct dm_tpm_ops {
+ int (*init)(struct udevice *);
+ int (*open)(struct udevice *);
+ int (*close)(struct udevice *);
+ int (*sendrecv)(struct udevice *,
+ const uint8_t *, size_t,
+ uint8_t *, size_t *);
+};

I tried to propose a structure like this one in
http://lists.denx.de/pipermail/u-boot/2015-August/222598.html very
closed to yours when moving to DM.
I would just add the init handler.

Best Regards
Christophe
On 11/08/2015 16:48, Simon Glass wrote:
> Move all the init and uninit code into one place.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> drivers/tpm/tpm_tis_i2c.c | 33 ++++++++++-----------------------
> 1 file changed, 10 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/tpm/tpm_tis_i2c.c b/drivers/tpm/tpm_tis_i2c.c
> index 60d97d3..329e8fc 100644
> --- a/drivers/tpm/tpm_tis_i2c.c
> +++ b/drivers/tpm/tpm_tis_i2c.c
> @@ -608,27 +608,6 @@ out:
> return rc;
> }
>
> -static int tpm_open_dev(struct udevice *dev)
> -{
> - int rc;
> -
> - debug("%s: start\n", __func__);
> - if (g_chip.is_open)
> - return -EBUSY;
> - rc = tpm_tis_i2c_init(dev);
> - if (rc < 0)
> - g_chip.is_open = 0;
> - return rc;
> -}
> -
> -static void tpm_close(void)
> -{
> - if (g_chip.is_open) {
> - release_locality(&g_chip, g_chip.locality, 1);
> - g_chip.is_open = 0;
> - }
> -}
> -
> /**
> * Decode TPM configuration.
> *
> @@ -712,7 +691,12 @@ int tis_open(void)
> if (!g_chip.inited)
> return -1;
>
> - rc = tpm_open_dev(g_chip.dev);
> + debug("%s: start\n", __func__);
> + if (g_chip.is_open)
> + return -EBUSY;
> + rc = tpm_tis_i2c_init(g_chip.dev);
> + if (rc < 0)
> + g_chip.is_open = 0;
>
> return rc;
> }
> @@ -722,7 +706,10 @@ int tis_close(void)
> if (!g_chip.inited)
> return -1;
>
> - tpm_close();
> + if (g_chip.is_open) {
> + release_locality(&g_chip, g_chip.locality, 1);
> + g_chip.is_open = 0;
> + }
>
> return 0;
> }