Add function gpiochip_wakeup_irq_setup() to configure and enable a
GPIO pin with interrupt wakeup capability according to user-defined
wakeup-gpios property in the device tree. Interrupt generated by
toggling the logic level (rising/falling edge) on the specified
GPIO pin can wake up a system from sleep mode.
Signed-off-by: Alex Soo <[email protected]>
---
drivers/gpio/gpiolib.c | 87 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 94903fc1c145..92cfbc34abb0 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -19,6 +19,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/pinctrl/consumer.h>
+#include <linux/pm_wakeirq.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
@@ -96,6 +97,7 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gc);
static int gpiochip_irqchip_init_hw(struct gpio_chip *gc);
static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gc);
static void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gc);
+static int gpiochip_wakeup_irq_setup(struct gpio_chip *gc);
static bool gpiolib_initialized;
@@ -1045,8 +1047,15 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
if (ret)
goto err_remove_irqchip;
}
+
+ ret = gpiochip_wakeup_irq_setup(gc);
+ if (ret)
+ goto err_remove_device;
+
return 0;
+err_remove_device:
+ gcdev_unregister(gdev);
err_remove_irqchip:
gpiochip_irqchip_remove(gc);
err_remove_irqchip_mask:
@@ -1874,6 +1883,84 @@ static int gpiochip_irqchip_add_allocated_domain(struct gpio_chip *gc,
return 0;
}
+static irqreturn_t gpio_wake_irq_handler(int irq, void *data)
+{
+ struct irq_data *irq_data = data;
+
+ if (!irq_data || irq != irq_data->irq)
+ return IRQ_NONE;
+
+ return IRQ_HANDLED;
+}
+
+static int gpiochip_wakeup_irq_setup(struct gpio_chip *gc)
+{
+ struct device *dev = gc->parent;
+ struct gpio_irq_chip *girq = &gc->irq;
+ struct gpio_desc *wakeup_gpiod;
+ struct irq_desc *wakeup_irqd;
+ struct irq_domain *irq_domain;
+ struct irq_data *irq_data;
+ unsigned int offset;
+ int wakeup_irq;
+ int ret;
+
+ if (!(device_property_read_bool(dev, "wakeup-source")))
+ return 0;
+
+ irq_domain = girq->domain;
+
+ if (!irq_domain) {
+ dev_err(dev, "Couldn't allocate IRQ domain\n");
+ return -ENXIO;
+ }
+
+ wakeup_gpiod = devm_gpiod_get_optional(dev, "wakeup", GPIOD_IN);
+
+ if (IS_ERR(wakeup_gpiod)) {
+ dev_err(dev, "invalid wakeup gpio: %lu\n", PTR_ERR(wakeup_gpiod));
+ return PTR_ERR(wakeup_gpiod);
+ }
+ if (!wakeup_gpiod) {
+ dev_dbg(dev, "property wakeup-gpios is not defined\n");
+ return 0;
+ }
+
+ offset = gpio_chip_hwgpio(wakeup_gpiod);
+ wakeup_irq = gpiod_to_irq(wakeup_gpiod);
+ if (wakeup_irq < 0) {
+ dev_err(dev, "failed to convert wakeup GPIO to IRQ\n");
+ return wakeup_irq;
+ }
+ irq_domain->ops->map(irq_domain, wakeup_irq, offset);
+ wakeup_irqd = irq_to_desc(wakeup_irq);
+ irq_data = irq_get_irq_data(wakeup_irq);
+ girq->handler = handle_edge_irq;
+
+ if (!(wakeup_irqd->status_use_accessors & IRQ_NOREQUEST)) {
+ device_init_wakeup(dev, 1);
+ ret = devm_request_threaded_irq(dev, wakeup_irq, NULL,
+ gpio_wake_irq_handler,
+ IRQF_TRIGGER_FALLING |
+ IRQF_TRIGGER_RISING |
+ IRQF_ONESHOT |
+ IRQF_SHARED,
+ "pm-wakeup-gpio", irq_data);
+ if (ret) {
+ dev_err(dev, "unable to request wakeup IRQ: %d\n", ret);
+ return ret;
+ }
+ }
+
+ ret = dev_pm_set_wake_irq(dev, wakeup_irq);
+ if (ret) {
+ dev_err(dev, "failed to enable gpio irq wake\n");
+ return ret;
+ }
+
+ return 0;
+}
+
/**
* gpiochip_add_irqchip() - adds an IRQ chip to a GPIO chip
* @gc: the GPIO chip to add the IRQ chip to
--
2.25.1
Fri, May 03, 2024 at 07:14:35PM +0800, Alex Soo kirjoitti:
> Add function gpiochip_wakeup_irq_setup() to configure and enable a
> GPIO pin with interrupt wakeup capability according to user-defined
> wakeup-gpios property in the device tree. Interrupt generated by
> toggling the logic level (rising/falling edge) on the specified
> GPIO pin can wake up a system from sleep mode.
..
> +static irqreturn_t gpio_wake_irq_handler(int irq, void *data)
> +{
> + struct irq_data *irq_data = data;
> +
> + if (!irq_data || irq != irq_data->irq)
Hmm... When irq_data can be NULL?
> + return IRQ_NONE;
> +
> + return IRQ_HANDLED;
> +}
..
> +static int gpiochip_wakeup_irq_setup(struct gpio_chip *gc)
> +{
> + struct device *dev = gc->parent;
> + struct gpio_irq_chip *girq = &gc->irq;
> + struct gpio_desc *wakeup_gpiod;
> + struct irq_desc *wakeup_irqd;
> + struct irq_domain *irq_domain;
> + struct irq_data *irq_data;
> + unsigned int offset;
> + int wakeup_irq;
> + int ret;
> +
> + if (!(device_property_read_bool(dev, "wakeup-source")))
Too many parentheses.
> + return 0;
> +
> + irq_domain = girq->domain;
> +
Unneeded blank line. Ditto for all similar cases.
> + if (!irq_domain) {
> + dev_err(dev, "Couldn't allocate IRQ domain\n");
> + return -ENXIO;
return dev_err_probe(...);
Ditto for all similar cases.
> + }
..
> + wakeup_gpiod = devm_gpiod_get_optional(dev, "wakeup", GPIOD_IN);
> +
> + if (IS_ERR(wakeup_gpiod)) {
> + dev_err(dev, "invalid wakeup gpio: %lu\n", PTR_ERR(wakeup_gpiod));
> + return PTR_ERR(wakeup_gpiod);
> + }
> + if (!wakeup_gpiod) {
> + dev_dbg(dev, "property wakeup-gpios is not defined\n");
> + return 0;
> + }
> +
> + offset = gpio_chip_hwgpio(wakeup_gpiod);
> + wakeup_irq = gpiod_to_irq(wakeup_gpiod);
> + if (wakeup_irq < 0) {
> + dev_err(dev, "failed to convert wakeup GPIO to IRQ\n");
> + return wakeup_irq;
> + }
> + irq_domain->ops->map(irq_domain, wakeup_irq, offset);
> + wakeup_irqd = irq_to_desc(wakeup_irq);
> + irq_data = irq_get_irq_data(wakeup_irq);
What these lines are for?
> + girq->handler = handle_edge_irq;
> +
> + if (!(wakeup_irqd->status_use_accessors & IRQ_NOREQUEST)) {
> + device_init_wakeup(dev, 1);
> + ret = devm_request_threaded_irq(dev, wakeup_irq, NULL,
> + gpio_wake_irq_handler,
> + IRQF_TRIGGER_FALLING |
> + IRQF_TRIGGER_RISING |
What's wrong with the flags from DT?
> + IRQF_ONESHOT |
> + IRQF_SHARED,
> + "pm-wakeup-gpio", irq_data);
> + if (ret) {
> + dev_err(dev, "unable to request wakeup IRQ: %d\n", ret);
> + return ret;
> + }
> + }
--
With Best Regards,
Andy Shevchenko
On Fri, May 3, 2024 at 1:15 PM Alex Soo <[email protected]> wrote:
> Add function gpiochip_wakeup_irq_setup() to configure and enable a
> GPIO pin with interrupt wakeup capability according to user-defined
> wakeup-gpios property in the device tree. Interrupt generated by
> toggling the logic level (rising/falling edge) on the specified
> GPIO pin can wake up a system from sleep mode.
>
> Signed-off-by: Alex Soo <[email protected]>
This is a very helpful patch I think.
I'm looking forward to the next iteration.
> @@ -1045,8 +1047,15 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> if (ret)
> goto err_remove_irqchip;
> }
> +
> + ret = gpiochip_wakeup_irq_setup(gc);
> + if (ret)
> + goto err_remove_device;
Do we have any in-tree drivers that do this by themselves already?
In that case we should convert them to use this function in the same
patch to avoid regressions.
> +static irqreturn_t gpio_wake_irq_handler(int irq, void *data)
> +{
> + struct irq_data *irq_data = data;
I'm minimalist so I usually just call the parameter "d" instead of "data"
and irq_data I would call *id but it's your pick.
> +
> + if (!irq_data || irq != irq_data->irq)
> + return IRQ_NONE;
> +
> + return IRQ_HANDLED;
Please add some debug print:
struct gpio_chip *gc = irq_data->chip_data;
chip_dbg(gc, "GPIO wakeup on IRQ %d\n", irq);
The rest looks good to me (after fixing Andy's comments!)
I would perhaps put some
debug print that "GPIO wakeup enabled at offset %d" in the
end as well, so people can easily follow this in the debug prints.
Yours,
Linus Walleij