On Sun, May 23, 2021 at 7:24 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
> The driver previous supported using GPIO requests to retrieve
previously
> multiple interrupt lines. As existing firmware may be using
> this method, we need to continue to support it. However, that doesn't
> stop us also supporting just getting irqs directly.
>
> The handling of irqflags has to take into account the fact that using
> a GPIO method to identify the interrupt does not convey direction of
> the trigger that fwnode_irq_get() will. So we need to set the
> IRQF_TRIGGER_RISING in that path but not otherwise, where it will
> cause an issue if we reprobe the driver after removal.
...
> + /* fwnode_irq_get() returns 0 for not present on OF, and -EINVAL for ACPI */
> + if (ret == 0 || ret == -EINVAL) {
> + gpio = devm_gpiod_get_index(dev, NULL, i, GPIOD_IN);
> + if (IS_ERR(gpio)) {
> + dev_err(dev, "gpio get index failed\n");
> + return PTR_ERR(gpio);
This should be dev_err_probe().
(I guess you need to prepend this patch with one that switches to
dev_err_probe() API)
> + }
> +
> + ret = gpiod_to_irq(gpio);
> + if (ret < 0)
> + return ret;
> + /* GPIO interrupt does npt have a specified direction */
> + irqflags |= IRQF_TRIGGER_RISING;
I'm not sure I understand this part. If we are talking about the ACPI
GpioInt() resource, then it should have this flag. If GpioIo() is in
use (which is already a sign of either using the line in dual
direction mode, but this needs to be described in the data sheet and
thus used in the driver, or misdesigned ACPI tables). DT, I suppose,
should have all necessary information.
> + }
--
With Best Regards,
Andy Shevchenko
