Warn but do not fail when devm_gpiod_get_optional returns an error when
trying to get the ECHO pin. When creating a pps-gpio device using
platform data and GPIO lookup tables, the call to gpiod_get_optional
will match on the unlabeled pin meant as the input when searching for
the "echo" pin. Since it is already in use as the PPS input, it will
fail with -EBUSY. As the ECHO pin is optional, we just warn on the error
and continue the initialization. This allows us to support devices
created using GPIO lookup tables instead of ACPI, DT, swnode, etc.
Signed-off-by: Curtis Klein <[email protected]>
---
drivers/pps/clients/pps-gpio.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 2f4b11b4dfcd..b7db4a3ee97e 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -114,9 +114,12 @@ static int pps_gpio_setup(struct device *dev)
device_property_read_bool(dev, "assert-falling-edge");
data->echo_pin = devm_gpiod_get_optional(dev, "echo", GPIOD_OUT_LOW);
- if (IS_ERR(data->echo_pin))
- return dev_err_probe(dev, PTR_ERR(data->echo_pin),
- "failed to request ECHO GPIO\n");
+ if (IS_ERR(data->echo_pin)) {
+ dev_warn(dev, "failed to request ECHO GPIO: %ld\n",
+ PTR_ERR(data->echo_pin));
+ data->echo_pin = NULL;
+ return 0;
+ }
if (!data->echo_pin)
return 0;
--
2.34.1
On 01/06/24 00:44, Curtis Klein wrote:
> Warn but do not fail when devm_gpiod_get_optional returns an error when
> trying to get the ECHO pin. When creating a pps-gpio device using
> platform data and GPIO lookup tables, the call to gpiod_get_optional
> will match on the unlabeled pin meant as the input when searching for
> the "echo" pin. Since it is already in use as the PPS input, it will
I'm not sure to well understand what you mean here: why the "echo" pin should be
already in use as the PPS input?
Can you please explain better this situation with a real example?
> fail with -EBUSY. As the ECHO pin is optional, we just warn on the error
> and continue the initialization. This allows us to support devices
> created using GPIO lookup tables instead of ACPI, DT, swnode, etc.
>
> Signed-off-by: Curtis Klein <[email protected]>
> ---
> drivers/pps/clients/pps-gpio.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
> index 2f4b11b4dfcd..b7db4a3ee97e 100644
> --- a/drivers/pps/clients/pps-gpio.c
> +++ b/drivers/pps/clients/pps-gpio.c
> @@ -114,9 +114,12 @@ static int pps_gpio_setup(struct device *dev)
> device_property_read_bool(dev, "assert-falling-edge");
>
> data->echo_pin = devm_gpiod_get_optional(dev, "echo", GPIOD_OUT_LOW);
> - if (IS_ERR(data->echo_pin))
> - return dev_err_probe(dev, PTR_ERR(data->echo_pin),
> - "failed to request ECHO GPIO\n");
> + if (IS_ERR(data->echo_pin)) {
> + dev_warn(dev, "failed to request ECHO GPIO: %ld\n",
> + PTR_ERR(data->echo_pin));
> + data->echo_pin = NULL;
> + return 0;
> + }
>
> if (!data->echo_pin)
> return 0;
Thanks,
Rodolfo
--
GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming
On Sunday, June 2, 2024 11:33 PM, Rodolfo Giometti wrote:
> On 01/06/24 00:44, Curtis Klein wrote:
> > Warn but do not fail when devm_gpiod_get_optional returns an error when
> > trying to get the ECHO pin. When creating a pps-gpio device using
> > platform data and GPIO lookup tables, the call to gpiod_get_optional
> > will match on the unlabeled pin meant as the input when searching for
> > the "echo" pin. Since it is already in use as the PPS input, it will
>
> I'm not sure to well understand what you mean here: why the "echo" pin
> should be
> already in use as the PPS input?
>
> Can you please explain better this situation with a real example?
>
Sure, sorry it's kind of hard to explain with words. Here's the gpio
lookup table I'm using:
static struct gpiod_lookup_table pps_gpio_table = {
.dev_id = "pps-gpio.0",
.table = {
GPIO_LOOKUP_IDX("gpio.0", 23, NULL, 0, GPIO_ACTIVE_HIGH),
{} /* Terminating entry */
}};
We have a single unnamed entry that is meant to be used as the input
pin. No "echo" pin is defined. We register this using
"gpiod_add_lookup_table(&pps_gpio_table)".
So the first time the pps-gpio driver gets the input pin with
"devm_gpiod_get(dev, NULL, GPIOD_IN);", we will get the one and only
gpio entry in the lookup table. Then when we try to get the "echo" pin
with "devm_gpiod_get_optional(dev, "echo", GPIOD_OUT_LOW);" we will
also match the same entry in the lookup table. This is because the
matching logic in gpio_find (in "drivers/gpio/gpiolib.c") will allow an
unnamed pin in a lookup table to match any request from a gpio_get
function. When this happens, "devm_gpiod_get_optional" returns -EBUSY
because the pin is in use (as the pps-gpio input) and the pps-gpio
driver fails to load.
Ideally, I think you'd use a swnode to define the gpios (if ACPI changes
are not possible). But this change would unblock those that need to use
gpio lookup tables. Continuing after an error on a optional operation
seems okay to me, but I understand if that goes against the normal
convention.
> > fail with -EBUSY. As the ECHO pin is optional, we just warn on the error
> > and continue the initialization. This allows us to support devices
> > created using GPIO lookup tables instead of ACPI, DT, swnode, etc.
> >
> > Signed-off-by: Curtis Klein <[email protected]>
> > ---
> > drivers/pps/clients/pps-gpio.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
> > index 2f4b11b4dfcd..b7db4a3ee97e 100644
> > --- a/drivers/pps/clients/pps-gpio.c
> > +++ b/drivers/pps/clients/pps-gpio.c
> > @@ -114,9 +114,12 @@ static int pps_gpio_setup(struct device *dev)
> > device_property_read_bool(dev, "assert-falling-edge");
> >
> > data->echo_pin = devm_gpiod_get_optional(dev, "echo",
> GPIOD_OUT_LOW);
> > - if (IS_ERR(data->echo_pin))
> > - return dev_err_probe(dev, PTR_ERR(data->echo_pin),
> > - "failed to request ECHO GPIO\n");
> > + if (IS_ERR(data->echo_pin)) {
> > + dev_warn(dev, "failed to request ECHO GPIO: %ld\n",
> > + PTR_ERR(data->echo_pin));
> > + data->echo_pin = NULL;
> > + return 0;
> > + }
> >
> > if (!data->echo_pin)
> > return 0;
>
> Thanks,
>
> Rodolfo
>
> --
> GNU/Linux Solutions e-mail: [email protected]
> Linux Device Driver [email protected]
> Embedded Systems phone: +39 349 2432127
> UNIX programming
-Curtis