Messages in this thread | | | Date | Tue, 22 Mar 2022 17:04:53 -0700 | From | Guenter Roeck <> | Subject | Re: [PATCH] watchdog: gpio_wdt: Support GPO lines with the toggle algorithm |
| |
On 3/22/22 15:29, Tobias Waldekranz wrote: > Support using GPO lines (i.e. GPIOs that are output-only) with > gpio_wdt using the "toggle" algorithm. > > Since its inception, gpio_wdt has configured its GPIO line as an input > when using the "toggle" algorithm, even though it is used as an output > when kicking. This needlessly barred hardware with output-only pins > from using the driver. > > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > --- > > Hi, > > This patch has been in our downstream tree for a long time. We need it > because our kick GPIO can't be used as an input. > > What I really can't figure out is why the driver would request the pin > as in input, when it's always going to end up being used as an output > anyway. > > So I thought I'd send it upstream in the hopes of either getting it > merged, or an explanation as to why it is needed. >
I _think_ the assumption / idea was that "toggle" implies that the output is connected to a pull-up resistor and that the pin either floats or is pulled down to ground, causing the signal to toggle. I don't know if/how that works in practice, though.
Guenter
> drivers/watchdog/gpio_wdt.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c > index 0923201ce874..f7686688e0e2 100644 > --- a/drivers/watchdog/gpio_wdt.c > +++ b/drivers/watchdog/gpio_wdt.c > @@ -108,7 +108,6 @@ static int gpio_wdt_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > struct gpio_wdt_priv *priv; > - enum gpiod_flags gflags; > unsigned int hw_margin; > const char *algo; > int ret; > @@ -122,17 +121,15 @@ static int gpio_wdt_probe(struct platform_device *pdev) > ret = of_property_read_string(np, "hw_algo", &algo); > if (ret) > return ret; > - if (!strcmp(algo, "toggle")) { > + > + if (!strcmp(algo, "toggle")) > priv->hw_algo = HW_ALGO_TOGGLE; > - gflags = GPIOD_IN; > - } else if (!strcmp(algo, "level")) { > + else if (!strcmp(algo, "level")) > priv->hw_algo = HW_ALGO_LEVEL; > - gflags = GPIOD_OUT_LOW; > - } else { > + else > return -EINVAL; > - } > > - priv->gpiod = devm_gpiod_get(dev, NULL, gflags); > + priv->gpiod = devm_gpiod_get(dev, NULL, GPIOD_OUT_LOW); > if (IS_ERR(priv->gpiod)) > return PTR_ERR(priv->gpiod); >
| |