| Message ID | 1439304497-10081-2-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
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@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@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@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);
Hi Christophe, On 11 August 2015 at 15:41, christophe.ricard <christophe.ricard@gmail.com> wrote: > 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. As far as I know those boards don't use a TPM. We can really only deal with source code which is present in mainline. > > Will you drop all drivers not supporting DM in the future ? At some point, but I suspect it might be quite a while before we even start that. In any case this is not what I am doing here. > > 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@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@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(-) >> [snip] Regards, Simon
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@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);
This is not used anymore by any board so drop it. Signed-off-by: Simon Glass <sjg@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(-)