2013-10-01 13:55:27

by Daniel Mack

[permalink] [raw]
Subject: [PATCH] drivers: misc: add gpio wakeup driver

This patch adds a very simple driver that enables GPIO lines as wakeup
sources. It only operates on information passed in via DT, and depends
on CONFIG_OF && CONFIG_PM_SLEEP. It can for example be used to connect
wake-on-LAN (WOL) signals or other electric wakeup networks.

The driver accepts a list of GPIO nodes and claims them along with their
interrupt line. During suspend, the interrupts will be enabled and
selected as wakeup source. The driver doesn't do anything else with the
GPIO lines, and will ignore occured interrupts silently.

Signed-off-by: Daniel Mack <[email protected]>
---
.../devicetree/bindings/misc/gpio-wakeup.txt | 16 ++
drivers/misc/Kconfig | 8 +
drivers/misc/Makefile | 1 +
drivers/misc/gpio-wakeup.c | 162 +++++++++++++++++++++
4 files changed, 187 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/gpio-wakeup.txt
create mode 100644 drivers/misc/gpio-wakeup.c

diff --git a/Documentation/devicetree/bindings/misc/gpio-wakeup.txt b/Documentation/devicetree/bindings/misc/gpio-wakeup.txt
new file mode 100644
index 0000000..6aacd0f
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/gpio-wakeup.txt
@@ -0,0 +1,16 @@
+GPIO WAKEUP DRIVER BINDINGS
+
+Required:
+
+ compatible: Must be "gpio-wakeup"
+ gpios: At list of GPIO nodes that are enabled as wakeup
+ sources.
+
+
+Example:
+
+ wake_up {
+ compatible = "gpio-wakeup";
+ gpios = <&gpio0 19 0>;
+ };
+
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 8dacd4c..c089280 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -528,6 +528,14 @@ config SRAM
the genalloc API. It is supposed to be used for small on-chip SRAM
areas found on many SoCs.

+config GPIO_WAKEUP
+ tristate "GPIO wakeup driver"
+ depends on PM_SLEEP && OF
+ help
+ Say Y to build a driver that can wake up a system from GPIO
+ lines. See Documentation/devicetree/bindings/gpio-wakeup.txt
+ for binding details.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c235d5b..ee4df84 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -53,3 +53,4 @@ obj-$(CONFIG_INTEL_MEI) += mei/
obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/
obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o
obj-$(CONFIG_SRAM) += sram.o
+obj-$(CONFIG_GPIO_WAKEUP) += gpio-wakeup.o
diff --git a/drivers/misc/gpio-wakeup.c b/drivers/misc/gpio-wakeup.c
new file mode 100644
index 0000000..3c1ef88
--- /dev/null
+++ b/drivers/misc/gpio-wakeup.c
@@ -0,0 +1,162 @@
+/*
+ * Driver to select GPIO lines as wakeup sources from DT.
+ *
+ * Copyright 2013 Daniel Mack
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/pm.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/of_platform.h>
+#include <linux/of_gpio.h>
+
+struct gpio_wakeup_priv {
+ unsigned int count;
+ unsigned int irq[0];
+};
+
+static irqreturn_t gpio_wakeup_isr(int irq, void *dev_id)
+{
+ return IRQ_HANDLED;
+}
+
+static int gpio_wakeup_probe(struct platform_device *pdev)
+{
+ int ret, count, i;
+ struct gpio_wakeup_priv *priv;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+
+ if (!np)
+ return -EINVAL;
+
+ count = of_gpio_count(np);
+ if (count == 0)
+ return -EINVAL;
+
+ priv = devm_kzalloc(dev, sizeof(*priv) +
+ sizeof(int) * count, GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ for (i = 0; i < count; i++) {
+ unsigned int gpio, irq;
+
+ priv->irq[i] = -EINVAL;
+
+ gpio = of_get_gpio(np, i);
+ if (gpio < 0) {
+ dev_warn(dev, "Unable to get gpio #%d\n", i);
+ continue;
+ }
+
+ irq = gpio_to_irq(gpio);
+ if (irq < 0) {
+ dev_warn(dev, "Can't map GPIO %d to an IRQ\n", gpio);
+ continue;
+ }
+
+ ret = devm_gpio_request_one(dev, gpio, GPIOF_IN, pdev->name);
+ if (ret < 0) {
+ dev_warn(dev, "Unable to request GPIO %d: %d\n",
+ gpio, ret);
+ continue;
+ }
+
+ ret = devm_request_irq(dev, irq, gpio_wakeup_isr,
+ IRQF_TRIGGER_RISING |
+ IRQF_TRIGGER_FALLING,
+ pdev->name, NULL);
+ if (ret < 0) {
+ dev_warn(dev, "Unable to request IRQ %d\n", irq);
+ continue;
+ }
+
+ disable_irq(irq);
+ priv->irq[i] = irq;
+
+ dev_info(dev, "Adding GPIO %d (IRQ %d) to wakeup sources\n",
+ gpio, irq);
+ }
+
+ priv->count = count;
+ device_init_wakeup(dev, 1);
+ platform_set_drvdata(pdev, priv);
+
+ return 0;
+}
+
+static int gpio_wakeup_suspend(struct device *dev)
+{
+ struct gpio_wakeup_priv *priv = dev_get_drvdata(dev);
+ int i;
+
+ for (i = 0; i < priv->count; i++)
+ if (priv->irq[i] >= 0) {
+ enable_irq(priv->irq[i]);
+ enable_irq_wake(priv->irq[i]);
+ }
+
+ return 0;
+}
+
+static int gpio_wakeup_resume(struct device *dev)
+{
+ struct gpio_wakeup_priv *priv = dev_get_drvdata(dev);
+ int i;
+
+ for (i = 0; i < priv->count; i++)
+ if (priv->irq[i] >= 0) {
+ disable_irq_wake(priv->irq[i]);
+ disable_irq(priv->irq[i]);
+ }
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(gpio_wakeup_pm_ops,
+ gpio_wakeup_suspend, gpio_wakeup_resume);
+
+static struct of_device_id gpio_wakeup_of_match[] = {
+ { .compatible = "gpio-wakeup", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, gpio_wakeup_of_match);
+
+static struct platform_driver gpio_wakeup_driver = {
+ .probe = gpio_wakeup_probe,
+ .driver = {
+ .name = "gpio-wakeup",
+ .owner = THIS_MODULE,
+ .pm = &gpio_wakeup_pm_ops,
+ .of_match_table = of_match_ptr(gpio_wakeup_of_match),
+ }
+};
+
+static int __init gpio_wakeup_init(void)
+{
+ return platform_driver_register(&gpio_wakeup_driver);
+}
+
+static void __exit gpio_wakeup_exit(void)
+{
+ platform_driver_unregister(&gpio_wakeup_driver);
+}
+
+late_initcall(gpio_wakeup_init);
+module_exit(gpio_wakeup_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Daniel Mack <[email protected]>");
+MODULE_DESCRIPTION("Driver to wake up systems from GPIOs");
+MODULE_ALIAS("platform:gpio-wakeup");
--
1.8.3.1


2013-10-01 14:01:20

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] drivers: misc: add gpio wakeup driver

Hi Daniel,

On Tue, Oct 1, 2013 at 10:55 AM, Daniel Mack <[email protected]> wrote:
> This patch adds a very simple driver that enables GPIO lines as wakeup
> sources. It only operates on information passed in via DT, and depends

Isn't it the same as the existing 'gpio-key,wakeup' ?

Please see Documentation/devicetree/bindings/gpio/gpio_keys.txt.

Regards,

Fabio Estevam


> on CONFIG_OF && CONFIG_PM_SLEEP. It can for example be used to connect
> wake-on-LAN (WOL) signals or other electric wakeup networks.
>
> The driver accepts a list of GPIO nodes and claims them along with their
> interrupt line. During suspend, the interrupts will be enabled and
> selected as wakeup source. The driver doesn't do anything else with the
> GPIO lines, and will ignore occured interrupts silently.
>
> Signed-off-by: Daniel Mack <[email protected]>
> ---
> .../devicetree/bindings/misc/gpio-wakeup.txt | 16 ++
> drivers/misc/Kconfig | 8 +
> drivers/misc/Makefile | 1 +
> drivers/misc/gpio-wakeup.c | 162 +++++++++++++++++++++
> 4 files changed, 187 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/misc/gpio-wakeup.txt
> create mode 100644 drivers/misc/gpio-wakeup.c
>
> diff --git a/Documentation/devicetree/bindings/misc/gpio-wakeup.txt b/Documentation/devicetree/bindings/misc/gpio-wakeup.txt
> new file mode 100644
> index 0000000..6aacd0f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/gpio-wakeup.txt
> @@ -0,0 +1,16 @@
> +GPIO WAKEUP DRIVER BINDINGS
> +
> +Required:
> +
> + compatible: Must be "gpio-wakeup"
> + gpios: At list of GPIO nodes that are enabled as wakeup
> + sources.
> +
> +
> +Example:
> +
> + wake_up {
> + compatible = "gpio-wakeup";
> + gpios = <&gpio0 19 0>;
> + };
> +
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 8dacd4c..c089280 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -528,6 +528,14 @@ config SRAM
> the genalloc API. It is supposed to be used for small on-chip SRAM
> areas found on many SoCs.
>
> +config GPIO_WAKEUP
> + tristate "GPIO wakeup driver"
> + depends on PM_SLEEP && OF
> + help
> + Say Y to build a driver that can wake up a system from GPIO
> + lines. See Documentation/devicetree/bindings/gpio-wakeup.txt
> + for binding details.
> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index c235d5b..ee4df84 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -53,3 +53,4 @@ obj-$(CONFIG_INTEL_MEI) += mei/
> obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/
> obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o
> obj-$(CONFIG_SRAM) += sram.o
> +obj-$(CONFIG_GPIO_WAKEUP) += gpio-wakeup.o
> diff --git a/drivers/misc/gpio-wakeup.c b/drivers/misc/gpio-wakeup.c
> new file mode 100644
> index 0000000..3c1ef88
> --- /dev/null
> +++ b/drivers/misc/gpio-wakeup.c
> @@ -0,0 +1,162 @@
> +/*
> + * Driver to select GPIO lines as wakeup sources from DT.
> + *
> + * Copyright 2013 Daniel Mack
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/pm.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/gpio.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_gpio.h>
> +
> +struct gpio_wakeup_priv {
> + unsigned int count;
> + unsigned int irq[0];
> +};
> +
> +static irqreturn_t gpio_wakeup_isr(int irq, void *dev_id)
> +{
> + return IRQ_HANDLED;
> +}
> +
> +static int gpio_wakeup_probe(struct platform_device *pdev)
> +{
> + int ret, count, i;
> + struct gpio_wakeup_priv *priv;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> +
> + if (!np)
> + return -EINVAL;
> +
> + count = of_gpio_count(np);
> + if (count == 0)
> + return -EINVAL;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv) +
> + sizeof(int) * count, GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + for (i = 0; i < count; i++) {
> + unsigned int gpio, irq;
> +
> + priv->irq[i] = -EINVAL;
> +
> + gpio = of_get_gpio(np, i);
> + if (gpio < 0) {
> + dev_warn(dev, "Unable to get gpio #%d\n", i);
> + continue;
> + }
> +
> + irq = gpio_to_irq(gpio);
> + if (irq < 0) {
> + dev_warn(dev, "Can't map GPIO %d to an IRQ\n", gpio);
> + continue;
> + }
> +
> + ret = devm_gpio_request_one(dev, gpio, GPIOF_IN, pdev->name);
> + if (ret < 0) {
> + dev_warn(dev, "Unable to request GPIO %d: %d\n",
> + gpio, ret);
> + continue;
> + }
> +
> + ret = devm_request_irq(dev, irq, gpio_wakeup_isr,
> + IRQF_TRIGGER_RISING |
> + IRQF_TRIGGER_FALLING,
> + pdev->name, NULL);
> + if (ret < 0) {
> + dev_warn(dev, "Unable to request IRQ %d\n", irq);
> + continue;
> + }
> +
> + disable_irq(irq);
> + priv->irq[i] = irq;
> +
> + dev_info(dev, "Adding GPIO %d (IRQ %d) to wakeup sources\n",
> + gpio, irq);
> + }
> +
> + priv->count = count;
> + device_init_wakeup(dev, 1);
> + platform_set_drvdata(pdev, priv);
> +
> + return 0;
> +}
> +
> +static int gpio_wakeup_suspend(struct device *dev)
> +{
> + struct gpio_wakeup_priv *priv = dev_get_drvdata(dev);
> + int i;
> +
> + for (i = 0; i < priv->count; i++)
> + if (priv->irq[i] >= 0) {
> + enable_irq(priv->irq[i]);
> + enable_irq_wake(priv->irq[i]);
> + }
> +
> + return 0;
> +}
> +
> +static int gpio_wakeup_resume(struct device *dev)
> +{
> + struct gpio_wakeup_priv *priv = dev_get_drvdata(dev);
> + int i;
> +
> + for (i = 0; i < priv->count; i++)
> + if (priv->irq[i] >= 0) {
> + disable_irq_wake(priv->irq[i]);
> + disable_irq(priv->irq[i]);
> + }
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(gpio_wakeup_pm_ops,
> + gpio_wakeup_suspend, gpio_wakeup_resume);
> +
> +static struct of_device_id gpio_wakeup_of_match[] = {
> + { .compatible = "gpio-wakeup", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, gpio_wakeup_of_match);
> +
> +static struct platform_driver gpio_wakeup_driver = {
> + .probe = gpio_wakeup_probe,
> + .driver = {
> + .name = "gpio-wakeup",
> + .owner = THIS_MODULE,
> + .pm = &gpio_wakeup_pm_ops,
> + .of_match_table = of_match_ptr(gpio_wakeup_of_match),
> + }
> +};
> +
> +static int __init gpio_wakeup_init(void)
> +{
> + return platform_driver_register(&gpio_wakeup_driver);
> +}
> +
> +static void __exit gpio_wakeup_exit(void)
> +{
> + platform_driver_unregister(&gpio_wakeup_driver);
> +}
> +
> +late_initcall(gpio_wakeup_init);
> +module_exit(gpio_wakeup_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Daniel Mack <[email protected]>");
> +MODULE_DESCRIPTION("Driver to wake up systems from GPIOs");
> +MODULE_ALIAS("platform:gpio-wakeup");
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-10-01 14:05:13

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH] drivers: misc: add gpio wakeup driver

Hi Fabio,

On 01.10.2013 16:01, Fabio Estevam wrote:
> On Tue, Oct 1, 2013 at 10:55 AM, Daniel Mack <[email protected]> wrote:
>> This patch adds a very simple driver that enables GPIO lines as wakeup
>> sources. It only operates on information passed in via DT, and depends
>
> Isn't it the same as the existing 'gpio-key,wakeup' ?

Of course, I know the gpio-input driver can provide similar
functionality. My intention was just provide a way to wake up the system
without registering an input device for signals nobody is interested in
eventually.

Don't know if that's reason enough to add a new driver though.


Thanks,
Daniel



>
>> on CONFIG_OF && CONFIG_PM_SLEEP. It can for example be used to connect
>> wake-on-LAN (WOL) signals or other electric wakeup networks.
>>
>> The driver accepts a list of GPIO nodes and claims them along with their
>> interrupt line. During suspend, the interrupts will be enabled and
>> selected as wakeup source. The driver doesn't do anything else with the
>> GPIO lines, and will ignore occured interrupts silently.
>>
>> Signed-off-by: Daniel Mack <[email protected]>
>> ---
>> .../devicetree/bindings/misc/gpio-wakeup.txt | 16 ++
>> drivers/misc/Kconfig | 8 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/gpio-wakeup.c | 162 +++++++++++++++++++++
>> 4 files changed, 187 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/misc/gpio-wakeup.txt
>> create mode 100644 drivers/misc/gpio-wakeup.c
>>
>> diff --git a/Documentation/devicetree/bindings/misc/gpio-wakeup.txt b/Documentation/devicetree/bindings/misc/gpio-wakeup.txt
>> new file mode 100644
>> index 0000000..6aacd0f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/gpio-wakeup.txt
>> @@ -0,0 +1,16 @@
>> +GPIO WAKEUP DRIVER BINDINGS
>> +
>> +Required:
>> +
>> + compatible: Must be "gpio-wakeup"
>> + gpios: At list of GPIO nodes that are enabled as wakeup
>> + sources.
>> +
>> +
>> +Example:
>> +
>> + wake_up {
>> + compatible = "gpio-wakeup";
>> + gpios = <&gpio0 19 0>;
>> + };
>> +
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 8dacd4c..c089280 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -528,6 +528,14 @@ config SRAM
>> the genalloc API. It is supposed to be used for small on-chip SRAM
>> areas found on many SoCs.
>>
>> +config GPIO_WAKEUP
>> + tristate "GPIO wakeup driver"
>> + depends on PM_SLEEP && OF
>> + help
>> + Say Y to build a driver that can wake up a system from GPIO
>> + lines. See Documentation/devicetree/bindings/gpio-wakeup.txt
>> + for binding details.
>> +
>> source "drivers/misc/c2port/Kconfig"
>> source "drivers/misc/eeprom/Kconfig"
>> source "drivers/misc/cb710/Kconfig"
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index c235d5b..ee4df84 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -53,3 +53,4 @@ obj-$(CONFIG_INTEL_MEI) += mei/
>> obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/
>> obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o
>> obj-$(CONFIG_SRAM) += sram.o
>> +obj-$(CONFIG_GPIO_WAKEUP) += gpio-wakeup.o
>> diff --git a/drivers/misc/gpio-wakeup.c b/drivers/misc/gpio-wakeup.c
>> new file mode 100644
>> index 0000000..3c1ef88
>> --- /dev/null
>> +++ b/drivers/misc/gpio-wakeup.c
>> @@ -0,0 +1,162 @@
>> +/*
>> + * Driver to select GPIO lines as wakeup sources from DT.
>> + *
>> + * Copyright 2013 Daniel Mack
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/pm.h>
>> +#include <linux/slab.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/gpio.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_gpio.h>
>> +
>> +struct gpio_wakeup_priv {
>> + unsigned int count;
>> + unsigned int irq[0];
>> +};
>> +
>> +static irqreturn_t gpio_wakeup_isr(int irq, void *dev_id)
>> +{
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int gpio_wakeup_probe(struct platform_device *pdev)
>> +{
>> + int ret, count, i;
>> + struct gpio_wakeup_priv *priv;
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> +
>> + if (!np)
>> + return -EINVAL;
>> +
>> + count = of_gpio_count(np);
>> + if (count == 0)
>> + return -EINVAL;
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv) +
>> + sizeof(int) * count, GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < count; i++) {
>> + unsigned int gpio, irq;
>> +
>> + priv->irq[i] = -EINVAL;
>> +
>> + gpio = of_get_gpio(np, i);
>> + if (gpio < 0) {
>> + dev_warn(dev, "Unable to get gpio #%d\n", i);
>> + continue;
>> + }
>> +
>> + irq = gpio_to_irq(gpio);
>> + if (irq < 0) {
>> + dev_warn(dev, "Can't map GPIO %d to an IRQ\n", gpio);
>> + continue;
>> + }
>> +
>> + ret = devm_gpio_request_one(dev, gpio, GPIOF_IN, pdev->name);
>> + if (ret < 0) {
>> + dev_warn(dev, "Unable to request GPIO %d: %d\n",
>> + gpio, ret);
>> + continue;
>> + }
>> +
>> + ret = devm_request_irq(dev, irq, gpio_wakeup_isr,
>> + IRQF_TRIGGER_RISING |
>> + IRQF_TRIGGER_FALLING,
>> + pdev->name, NULL);
>> + if (ret < 0) {
>> + dev_warn(dev, "Unable to request IRQ %d\n", irq);
>> + continue;
>> + }
>> +
>> + disable_irq(irq);
>> + priv->irq[i] = irq;
>> +
>> + dev_info(dev, "Adding GPIO %d (IRQ %d) to wakeup sources\n",
>> + gpio, irq);
>> + }
>> +
>> + priv->count = count;
>> + device_init_wakeup(dev, 1);
>> + platform_set_drvdata(pdev, priv);
>> +
>> + return 0;
>> +}
>> +
>> +static int gpio_wakeup_suspend(struct device *dev)
>> +{
>> + struct gpio_wakeup_priv *priv = dev_get_drvdata(dev);
>> + int i;
>> +
>> + for (i = 0; i < priv->count; i++)
>> + if (priv->irq[i] >= 0) {
>> + enable_irq(priv->irq[i]);
>> + enable_irq_wake(priv->irq[i]);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int gpio_wakeup_resume(struct device *dev)
>> +{
>> + struct gpio_wakeup_priv *priv = dev_get_drvdata(dev);
>> + int i;
>> +
>> + for (i = 0; i < priv->count; i++)
>> + if (priv->irq[i] >= 0) {
>> + disable_irq_wake(priv->irq[i]);
>> + disable_irq(priv->irq[i]);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(gpio_wakeup_pm_ops,
>> + gpio_wakeup_suspend, gpio_wakeup_resume);
>> +
>> +static struct of_device_id gpio_wakeup_of_match[] = {
>> + { .compatible = "gpio-wakeup", },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, gpio_wakeup_of_match);
>> +
>> +static struct platform_driver gpio_wakeup_driver = {
>> + .probe = gpio_wakeup_probe,
>> + .driver = {
>> + .name = "gpio-wakeup",
>> + .owner = THIS_MODULE,
>> + .pm = &gpio_wakeup_pm_ops,
>> + .of_match_table = of_match_ptr(gpio_wakeup_of_match),
>> + }
>> +};
>> +
>> +static int __init gpio_wakeup_init(void)
>> +{
>> + return platform_driver_register(&gpio_wakeup_driver);
>> +}
>> +
>> +static void __exit gpio_wakeup_exit(void)
>> +{
>> + platform_driver_unregister(&gpio_wakeup_driver);
>> +}
>> +
>> +late_initcall(gpio_wakeup_init);
>> +module_exit(gpio_wakeup_exit);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Daniel Mack <[email protected]>");
>> +MODULE_DESCRIPTION("Driver to wake up systems from GPIOs");
>> +MODULE_ALIAS("platform:gpio-wakeup");
>> --
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-10-02 10:57:29

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] drivers: misc: add gpio wakeup driver

On Tue, Oct 01, 2013 at 04:05:07PM +0200, Daniel Mack wrote:
> On 01.10.2013 16:01, Fabio Estevam wrote:

> > Isn't it the same as the existing 'gpio-key,wakeup' ?

> Of course, I know the gpio-input driver can provide similar
> functionality. My intention was just provide a way to wake up the system
> without registering an input device for signals nobody is interested in
> eventually.

> Don't know if that's reason enough to add a new driver though.

It does seem somewhat sensible - the signal might not have a sensible
representation as an input device and the gpio-keys binding needs one.


Attachments:
(No filename) (598.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-10-02 11:02:22

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH] drivers: misc: add gpio wakeup driver

On 02.10.2013 12:57, Mark Brown wrote:
> On Tue, Oct 01, 2013 at 04:05:07PM +0200, Daniel Mack wrote:
>> On 01.10.2013 16:01, Fabio Estevam wrote:
>
>>> Isn't it the same as the existing 'gpio-key,wakeup' ?
>
>> Of course, I know the gpio-input driver can provide similar
>> functionality. My intention was just provide a way to wake up the system
>> without registering an input device for signals nobody is interested in
>> eventually.
>
>> Don't know if that's reason enough to add a new driver though.
>
> It does seem somewhat sensible - the signal might not have a sensible
> representation as an input device and the gpio-keys binding needs one.
>

That was my intention as well, yes. Also, a system that does not have
any input devices could in theory disable CONFIG_INPUT alltogether. Not
sure how realisitic that scenario really is nowadays, but using the gpio
input driver for purpose of just waking up on LAN seems somewhat abusive.


Daniel

2013-10-11 11:11:55

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] drivers: misc: add gpio wakeup driver

On Tue, Oct 1, 2013 at 3:55 PM, Daniel Mack <[email protected]> wrote:

> This patch adds a very simple driver that enables GPIO lines as wakeup
> sources. It only operates on information passed in via DT, and depends
> on CONFIG_OF && CONFIG_PM_SLEEP. It can for example be used to connect
> wake-on-LAN (WOL) signals or other electric wakeup networks.
>
> The driver accepts a list of GPIO nodes and claims them along with their
> interrupt line. During suspend, the interrupts will be enabled and
> selected as wakeup source. The driver doesn't do anything else with the
> GPIO lines, and will ignore occured interrupts silently.
>
> Signed-off-by: Daniel Mack <[email protected]>

This makes a weird kind of sense.
Hm hm hm.

But I really need the misc mainatiners' help here...
possibly also irqchip maintainers.

> +++ b/Documentation/devicetree/bindings/misc/gpio-wakeup.txt
> @@ -0,0 +1,16 @@
> +GPIO WAKEUP DRIVER BINDINGS
> +
> +Required:
> +
> + compatible: Must be "gpio-wakeup"
> + gpios: At list of GPIO nodes that are enabled as wakeup
> + sources.

Reword so this is OS independent. Describe what a wakeup source is
for example.

> +Example:
> +
> + wake_up {
> + compatible = "gpio-wakeup";
> + gpios = <&gpio0 19 0>;
> + };

This will not work if that GPIO chip is not capable of supporting
interrupts on that GPIO line right?

We have recently had a very long discussion about this: such
GPIO chips will also be marked "interrupt-controller" and you
should be able to just state interrupt-parent and
interrupts = <>; for this. (And it should accept an array.)

It *may* be that we have many GPIO drivers that do not accept
that you request an interrupt on them before you have done
request_gpio() followed by gpio_to_irq() on the pin. Then this
shall be treated like a bug and the GPIO driver fixed to handle
this. (That was the outcome of this discussion.)

Since what the driver will then eventually provide is to
flag an IRQ line as wakeup, I wonder if this should not just
simply go into the interrupt core, or atleast of/irq.c.

Yours,
Linus Walleij

2013-10-11 11:40:59

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH] drivers: misc: add gpio wakeup driver

Hi Linus,

On 11.10.2013 13:11, Linus Walleij wrote:
> On Tue, Oct 1, 2013 at 3:55 PM, Daniel Mack <[email protected]> wrote:

>> +Example:
>> +
>> + wake_up {
>> + compatible = "gpio-wakeup";
>> + gpios = <&gpio0 19 0>;
>> + };
>
> This will not work if that GPIO chip is not capable of supporting
> interrupts on that GPIO line right?

Of course. It's the IRQ that wakes up the system, after all :)

> We have recently had a very long discussion about this: such
> GPIO chips will also be marked "interrupt-controller" and you
> should be able to just state interrupt-parent and
> interrupts = <>; for this. (And it should accept an array.)

Well, but that's not very intuitive. I was aiming for a drop-in
replacement for what I currently use for this purpose: a fake GPIO key
input device which is marked as wakeup source (linux,wakeup). Schematics
and everything else use GPIO notation, and so should the DTB binding here.

> Since what the driver will then eventually provide is to
> flag an IRQ line as wakeup, I wonder if this should not just
> simply go into the interrupt core, or atleast of/irq.c.

But for that, the IRQ line must be requested exclusively and handled as
well, no? If not, how would you handle cases where an interrupt is
marked as wakeup source by the core, but used by another driver which
calls disable_irq_wake() on it for whatever reason?


Thanks,
Daniel

2013-10-11 12:54:27

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] drivers: misc: add gpio wakeup driver

On Fri, Oct 11, 2013 at 1:40 PM, Daniel Mack <[email protected]> wrote:

>> Since what the driver will then eventually provide is to
>> flag an IRQ line as wakeup, I wonder if this should not just
>> simply go into the interrupt core, or atleast of/irq.c.
>
> But for that, the IRQ line must be requested exclusively and handled as
> well, no? If not, how would you handle cases where an interrupt is
> marked as wakeup source by the core, but used by another driver which
> calls disable_irq_wake() on it for whatever reason?

Does this driver handle that?

It rather looks like it hogs both the GPIO and IRQ from anyone
else who want it...

Yours,
Linus Walleij

2013-10-15 19:32:17

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] drivers: misc: add gpio wakeup driver

On Friday 11 October 2013, Linus Walleij wrote:
> On Tue, Oct 1, 2013 at 3:55 PM, Daniel Mack <[email protected]> wrote:
>
> > This patch adds a very simple driver that enables GPIO lines as wakeup
> > sources. It only operates on information passed in via DT, and depends
> > on CONFIG_OF && CONFIG_PM_SLEEP. It can for example be used to connect
> > wake-on-LAN (WOL) signals or other electric wakeup networks.
> >
> > The driver accepts a list of GPIO nodes and claims them along with their
> > interrupt line. During suspend, the interrupts will be enabled and
> > selected as wakeup source. The driver doesn't do anything else with the
> > GPIO lines, and will ignore occured interrupts silently.
> >
> > Signed-off-by: Daniel Mack <[email protected]>
>
> This makes a weird kind of sense.
> Hm hm hm.
>
> But I really need the misc mainatiners' help here...
> possibly also irqchip maintainers.

This seems like a completely generic driver, rather than some oddball
hack, so I'd prefer to not see it in drivers/misc at all. Maybe you
can find some other maintainer who is willing to put it into his
subsystem, candidates would be

* gpio
* irqchip
* power
* of/dt

I don't see anything wrong with the basic approach though.

> > +Example:
> > +
> > + wake_up {
> > + compatible = "gpio-wakeup";
> > + gpios = <&gpio0 19 0>;
> > + };
>
> This will not work if that GPIO chip is not capable of supporting
> interrupts on that GPIO line right?
>
> We have recently had a very long discussion about this: such
> GPIO chips will also be marked "interrupt-controller" and you
> should be able to just state interrupt-parent and
> interrupts = <>; for this. (And it should accept an array.)
>
> It *may* be that we have many GPIO drivers that do not accept
> that you request an interrupt on them before you have done
> request_gpio() followed by gpio_to_irq() on the pin. Then this
> shall be treated like a bug and the GPIO driver fixed to handle
> this. (That was the outcome of this discussion.)

I haven't followed that discussion, but it's good to hear that
you made some progress there. I find it a bit worrying that you
say the behavior may be dependent on the gpio driver, but maybe
I didn't fully understand what the resolution is.

> Since what the driver will then eventually provide is to
> flag an IRQ line as wakeup, I wonder if this should not just
> simply go into the interrupt core, or atleast of/irq.c.

Right.

Arnd

2013-10-15 21:35:07

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] drivers: misc: add gpio wakeup driver

On Tue, Oct 15, 2013 at 9:32 PM, Arnd Bergmann <[email protected]> wrote:
> On Friday 11 October 2013, Linus Walleij wrote:

>> It *may* be that we have many GPIO drivers that do not accept
>> that you request an interrupt on them before you have done
>> request_gpio() followed by gpio_to_irq() on the pin. Then this
>> shall be treated like a bug and the GPIO driver fixed to handle
>> this. (That was the outcome of this discussion.)
>
> I haven't followed that discussion, but it's good to hear that
> you made some progress there. I find it a bit worrying that you
> say the behavior may be dependent on the gpio driver, but maybe
> I didn't fully understand what the resolution is.

It's some consensus on how to do this, but the existing drivers
need to be augmented to implement it according to that
consensus, especially those using device tree.

If noone else helps I guess it goes on top of the pile of stuff
for the GPIO maintainer to fix...

Yours,
Linus Walleij