On 01/06/21, Michal Simek wrote:
On 6/1/21 8:09 AM, Jorge Ramirez-Ortiz wrote:
Use the more generic reset-gpios propery name.
Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io
v2: kept gpio-reset as legacy
.../tpm2/tis-tpm2-spi.txt | 2 +- drivers/tpm/tpm2_tis_spi.c | 21 ++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/doc/device-tree-bindings/tpm2/tis-tpm2-spi.txt b/doc/device-tree-bindings/tpm2/tis-tpm2-spi.txt index 3a2ee4bd17..bbcd12950f 100644 --- a/doc/device-tree-bindings/tpm2/tis-tpm2-spi.txt +++ b/doc/device-tree-bindings/tpm2/tis-tpm2-spi.txt @@ -6,7 +6,7 @@ Required properties:
- reg : SPI Chip select
Optional properties: -- gpio-reset : Reset GPIO (if not connected to the SoC reset line)
As I said you shouldn't remove this. Just extend description that it is deprecated and reset-gpios should be used instead.
I dont really agree with that. IMO we should remove the documentation since it is obsolete after this commit and anyone reading it should not care about the gpio-reset property.
+- reset-gpios : Reset GPIO (if not connected to the SoC reset line)
- spi-max-frequency : See spi-bus.txt
Example: diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c index 4b33ac8fd3..1f9f89f68f 100644 --- a/drivers/tpm/tpm2_tis_spi.c +++ b/drivers/tpm/tpm2_tis_spi.c @@ -589,18 +589,23 @@ static int tpm_tis_spi_probe(struct udevice *dev) if (CONFIG_IS_ENABLED(DM_GPIO)) { struct gpio_desc reset_gpio;
ret = gpio_request_by_name(dev, "gpio-reset", 0,
ret = gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio, GPIOD_IS_OUT);
log(LOGC_NONE, LOGL_NOTICE, "%s: missing reset GPIO\n",__func__);} else {dm_gpio_set_value(&reset_gpio, 1);mdelay(1);dm_gpio_set_value(&reset_gpio, 0);
/* legacy reset */ret = gpio_request_by_name(dev, "gpio-reset", 0,&reset_gpio, GPIOD_IS_OUT);if (ret) {log(LOGC_NONE, LOGL_NOTICE,"%s: missing reset GPIO\n", __func__);goto init;}And here it is clear that gpio-reset is used which should deprecated that's why you should print message about it here.
yes, I can do that. makes sense
}
dm_gpio_set_value(&reset_gpio, 1);mdelay(1);dm_gpio_set_value(&reset_gpio, 0);What about this to remove that goto?
um, what is the problem with the goto (IMO tidier than yet another conditional); it is not as if this goto is making the code obscure.
with the change below you just removed previous functionality (ie indicating that there is no GPIO reset provided, hence why at first sight might look cleaner than a goto)
ret = gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio, GPIOD_IS_OUT); if (ret) { /* trying legacy reset */ ret = gpio_request_by_name(dev, "gpio-reset", 0, &reset_gpio, GPIOD_IS_OUT); if (!ret) log(LOGC_NONE, LOGL_NOTICE, "%s: Using deprecated gpio-reset switch to reset-gpios\n", __func__); }
if (!ret) { dm_gpio_set_value(&reset_gpio, 1); mdelay(1); dm_gpio_set_value(&reset_gpio, 0); }
+init: /* Ensure a minimum amount of time elapsed since reset of the TPM */ mdelay(drv_data->time_before_first_cmd_ms);
M