of/irq: introduce of_has_named_irqs helper

PATCH In review Dmitry Torokhov 4 participants 12 messages Jul 24, 2015 – Jul 29, 2015

Subsystems (4)

I2C subsystem host drivers Input drivers Device tree I2C

lore

Share & tools

lore

Dmitry Torokhov

author maintainer

2015-07-24 18:26 UTC #0 lore raw

PATCH 1/1 of/irq: introduce of_has_named_irqs helper

Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.

Signed-off-by Dmitry Torokhov <[email protected]>

---

The intent is to it like this:

	if (of_has_named_irqs(np) {
		/* Wake IRQ is optional */
		dev->wakeirq = of_irq_get_byname(np, "wakeup");
		if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
			return dev->wakeirq;
	}

 drivers/of/irq.c       | 14 ++++++++++++++
 include/linux/of_irq.h |  5 +++++
 2 files changed, 19 insertions(+)

drivers/of/irq.c +14 -0

index 3cf7a01..e02c43e 100644
@@ -384,6 +384,19 @@ int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
 EXPORT_SYMBOL_GPL(of_irq_to_resource);
 
 /**
+ * of_has_named_irq - Check if given node contains named interrupts
+ * @dev: pointer to device tree node
+ *
+ * Returns %true if node contains non-empty "interrupt-names" property.
+ */
+bool of_has_named_irqs(struct device_node *dev)
+{
+	struct property *prop = of_find_property(dev, "interrupt-names", NULL);
+
+	return prop && prop->value;
+}
+
+/**
  * of_irq_get - Decode a node's IRQ and return it as a Linux irq number
  * @dev: pointer to device tree node
  * @index: zero-based index of the irq

@@ -410,6 +423,7 @@ int of_irq_get(struct device_node *dev, int index)
 }
 EXPORT_SYMBOL_GPL(of_irq_get);
 
+
 /**
  * of_irq_get_byname - Decode a node's IRQ and return it as a Linux irq number
  * @dev: pointer to device tree node

include/linux/of_irq.h +5 -0

index d884929..fed6ccc 100644
@@ -46,6 +46,7 @@ extern int of_irq_get(struct device_node *dev, int index);
 extern int of_irq_get_byname(struct device_node *dev, const char *name);
 extern int of_irq_to_resource_table(struct device_node *dev,
 		struct resource *res, int nr_irqs);
+extern bool of_has_named_irqs(struct device_node *dev);
 #else
 static inline int of_irq_count(struct device_node *dev)
 {

@@ -64,6 +65,10 @@ static inline int of_irq_to_resource_table(struct device_node *dev,
 {
 	return 0;
 }
+static inline bool of_has_named_irqs(struct device_node *dev)
+{
+	return false;
+}
 #endif
 
 #if defined(CONFIG_OF)

Signature · 5 lines

2.5.0.rc2.392.g76e840b


-- 
Dmitry

Rob Herring

2015-07-24 19:14 UTC #1 lore raw

Quoted 4 lines · Dmitry Torokhov

Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.

Generally, IRQs are retrieved by platform_get_irq or platform_get_irq_byname. Drivers should not call the of_irq_* functions directly in most cases.

Quoted 4 lines

Signed-off-by Dmitry Torokhov <[email protected]>
---

Quoted 8 lines

The intent is to it like this:

        if (of_has_named_irqs(np) {
                /* Wake IRQ is optional */
                dev->wakeirq = of_irq_get_byname(np, "wakeup");
                if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
                        return dev->wakeirq;
        }

of_irq_get_byname will already return an error if the property is not present. Use that.

Rob

Dmitry Torokhov

author maintainer

2015-07-24 19:26 UTC #2 lore raw

Quoted 10 lines · Rob Herring

Quoted 4 lines · Dmitry Torokhov
Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.
Generally, IRQs are retrieved by platform_get_irq or platform_get_irq_byname. Drivers should not call the of_irq_* functions directly in most cases.

That would be true for platform drivers, but not all devices are platform devices.

Quoted 14 lines

Quoted 5 lines

Signed-off-by Dmitry Torokhov <[email protected]>
---

Quoted 8 lines

The intent is to it like this:

        if (of_has_named_irqs(np) {
                /* Wake IRQ is optional */
                dev->wakeirq = of_irq_get_byname(np, "wakeup");
                if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
                        return dev->wakeirq;
        }

Quoted 2 lines

of_irq_get_byname will already return an error if the property is not present. Use that.

I do not like that it returns -EINVAL when property is missing, can we change it to return -ENODATA (so it is the same as when the property is defined but such name is missing)?

Thanks.

Signature · 5 lines

Dmitry
--
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

Dmitry Torokhov

author maintainer

2015-07-24 20:16 UTC #3 lore raw

Quoted 29 lines · Dmitry Torokhov

Quoted 10 lines · Rob Herring
Quoted 4 lines · Dmitry Torokhov
Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.
Generally, IRQs are retrieved by platform_get_irq or platform_get_irq_byname. Drivers should not call the of_irq_* functions directly in most cases.
That would be true for platform drivers, but not all devices are platform devices.
Quoted 14 lines
Quoted 13 lines
Signed-off-by Dmitry Torokhov <[email protected]>
---
The intent is to it like this:
        if (of_has_named_irqs(np) {
                /* Wake IRQ is optional */
                dev->wakeirq = of_irq_get_byname(np, "wakeup");
                if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
                        return dev->wakeirq;
        }

Quoted 6 lines

Quoted 2 lines
of_irq_get_byname will already return an error if the property is not present. Use that.
I do not like that it returns -EINVAL when property is missing, can we change it to return -ENODATA (so it is the same as when the property is defined but such name is missing)?

So here is what I had in mind.. It is based on recent patch by Vignesh for pixcir touchscreen, but I think it should be made available to all I2C devices. Completely untested at the moment.

---
 drivers/i2c/i2c-core.c |   43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

drivers/i2c/i2c-core.c +37 -6

index e6d4935..3971461 100644
@@ -47,6 +47,7 @@
 #include <linux/rwsem.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/acpi.h>
 #include <linux/jump_label.h>
 #include <asm/uaccess.h>

@@ -631,6 +632,7 @@ static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
 	struct i2c_driver	*driver;
+	int wakeirq = 0;
 	int status;
 
 	if (!client)

@@ -639,11 +641,13 @@ static int i2c_device_probe(struct device *dev)
 	if (!client->irq) {
 		int irq = -ENOENT;
 
-		if (dev->of_node)
-			irq = of_irq_get(dev->of_node, 0);
-		else if (ACPI_COMPANION(dev))
+		if (dev->of_node) {
+			irq = of_has_named_irqs(dev->of_node) ?
+				of_irq_get_byname(dev->of_node, "irq") :
+				of_irq_get(dev->of_node, 0);
+		} else if (ACPI_COMPANION(dev)) {
 			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
-
+		}
 		if (irq == -EPROBE_DEFER)
 			return irq;
 		if (irq < 0)

@@ -652,6 +656,15 @@ static int i2c_device_probe(struct device *dev)
 		client->irq = irq;
 	}
 
+	if (dev->of_node && of_has_named_irqs(dev->of_node)) {
+		wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
+		if (wakeirq < 0) {
+			if (wakeirq != -ENODATA)
+				return wakeirq;
+			wakeirq = 0;
+		}
+	}
+
 	driver = to_i2c_driver(dev->driver);
 	if (!driver->probe || !driver->id_table)
 		return -ENODEV;

@@ -659,20 +672,34 @@ static int i2c_device_probe(struct device *dev)
 	if (!device_can_wakeup(&client->dev))
 		device_init_wakeup(&client->dev,
 					client->flags & I2C_CLIENT_WAKE);
+
+	status = wakeirq > 0 ?
+			dev_pm_set_dedicated_wake_irq(dev, wakeirq) :
+			(client->irq > 0 ?
+				dev_pm_set_wake_irq(dev, client->irq) : 0);
+	if (status)
+		dev_warn(&client->dev, "failed to set up wakeup irq");
+
 	dev_dbg(dev, "probe\n");
 
 	status = of_clk_set_defaults(dev->of_node, false);
 	if (status < 0)
-		return status;
+		goto err_clear_wakeup_irq;
 
 	status = dev_pm_domain_attach(&client->dev, true);
 	if (status != -EPROBE_DEFER) {
 		status = driver->probe(client, i2c_match_id(driver->id_table,
 					client));
 		if (status)
-			dev_pm_domain_detach(&client->dev, true);
+			goto err_detach_pm_domain;
 	}
 
+	return 0;
+
+err_detach_pm_domain:
+	dev_pm_domain_detach(&client->dev, true);
+err_clear_wakeup_irq:
+	dev_pm_clear_wake_irq(&client->dev);
 	return status;
 }

@@ -692,6 +719,10 @@ static int i2c_device_remove(struct device *dev)
 	}
 
 	dev_pm_domain_detach(&client->dev, true);
+
+	dev_pm_clear_wake_irq(&client->dev);
+	device_init_wakeup(&client->dev, 0);
+
 	return status;
 }

Thanks.

Signature · 1 line

Dmitry

Vignesh R

2015-07-28 13:23 UTC #4 lore raw

Hi Dmitry,

Quoted 33 lines · Dmitry Torokhov

Quoted 32 lines · Dmitry Torokhov
Quoted 10 lines · Rob Herring
Quoted 4 lines · Dmitry Torokhov
Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.
Generally, IRQs are retrieved by platform_get_irq or platform_get_irq_byname. Drivers should not call the of_irq_* functions directly in most cases.
That would be true for platform drivers, but not all devices are platform devices.
Quoted 16 lines
Quoted 5 lines
Signed-off-by Dmitry Torokhov <[email protected]>
---
Quoted 8 lines
The intent is to it like this:
        if (of_has_named_irqs(np) {
                /* Wake IRQ is optional */
                dev->wakeirq = of_irq_get_byname(np, "wakeup");
                if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
                        return dev->wakeirq;
        }
of_irq_get_byname will already return an error if the property is not present. Use that.

Quoted 8 lines

Quoted 3 lines
I do not like that it returns -EINVAL when property is missing, can we change it to return -ENODATA (so it is the same as when the property is defined but such name is missing)?
So here is what I had in mind.. It is based on recent patch by Vignesh for pixcir touchscreen, but I think it should be made available to all I2C devices. Completely untested at the moment.

Thanks for the patch! I tested this on am437x-gp-evm and the suspend/resume worked fine (below patch + $subject patch). I was able to wake the system from low power state using touchscreen. Also verified module insertion and removal. One comment though, please see below.

Quoted 59 lines

---
 drivers/i2c/i2c-core.c |   43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

drivers/i2c/i2c-core.c +17 -4

index e6d4935..3971461 100644
@@ -47,6 +47,7 @@
 #include <linux/rwsem.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/acpi.h>
 #include <linux/jump_label.h>
 #include <asm/uaccess.h>

@@ -631,6 +632,7 @@ static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
 	struct i2c_driver	*driver;
+	int wakeirq = 0;
 	int status;
 
 	if (!client)

@@ -639,11 +641,13 @@ static int i2c_device_probe(struct device *dev)
 	if (!client->irq) {
 		int irq = -ENOENT;
 
-		if (dev->of_node)
-			irq = of_irq_get(dev->of_node, 0);
-		else if (ACPI_COMPANION(dev))
+		if (dev->of_node) {
+			irq = of_has_named_irqs(dev->of_node) ?
+				of_irq_get_byname(dev->of_node, "irq") :
+				of_irq_get(dev->of_node, 0);
+		} else if (ACPI_COMPANION(dev)) {
 			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
-
+		}
 		if (irq == -EPROBE_DEFER)
 			return irq;
 		if (irq < 0)

@@ -652,6 +656,15 @@ static int i2c_device_probe(struct device *dev)
 		client->irq = irq;
 	}
 
+	if (dev->of_node && of_has_named_irqs(dev->of_node)) {
+		wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
+		if (wakeirq < 0) {
+			if (wakeirq != -ENODATA)
+				return wakeirq;
+			wakeirq = 0;
+		}
+	}
+
 	driver = to_i2c_driver(dev->driver);
 	if (!driver->probe || !driver->id_table)
 		return -ENODEV;

drivers/i2c/i2c-core.c static int i2c_device_probe(struct device *dev) · line 679

Quoted 8 lines

drivers/i2c/i2c-core.c +5 -0

@@ -659,20 +672,34 @@ static int i2c_device_probe(struct device *dev)
 	if (!device_can_wakeup(&client->dev))
 		device_init_wakeup(&client->dev,
 					client->flags & I2C_CLIENT_WAKE);
+
+	status = wakeirq > 0 ?
+			dev_pm_set_dedicated_wake_irq(dev, wakeirq) :
+			(client->irq > 0 ?
+				dev_pm_set_wake_irq(dev, client->irq) : 0);

Above code tries to register wakeirq irrespective of whether the device is specified as wakeup-source in the dt or not. Hence, I see warn messages from every i2c device that has irq line but hasn't declared itself as wakeup-source:
For example tps is on i2c:

[    1.961613] tps65218 0-0024: forgot to call call device_init_wakeup?
[    1.968340] tps65218 0-0024: failed to set up wakeup irq

May be you can register wakeup-source only if I2C_CLIENT_WAKE flag is set in client->flags?

Signature · 2 lines

Regards
Vignesh

Dmitry Torokhov

author maintainer

2015-07-28 17:23 UTC #5 lore raw

Quoted 123 lines · Vignesh R

Hi Dmitry,
Quoted 33 lines · Dmitry Torokhov
Quoted 32 lines · Dmitry Torokhov
Quoted 10 lines · Rob Herring
Quoted 4 lines · Dmitry Torokhov
Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.
Generally, IRQs are retrieved by platform_get_irq or platform_get_irq_byname. Drivers should not call the of_irq_* functions directly in most cases.
That would be true for platform drivers, but not all devices are platform devices.
Quoted 16 lines
Quoted 5 lines
Signed-off-by Dmitry Torokhov <[email protected]>
---
Quoted 8 lines
The intent is to it like this:
        if (of_has_named_irqs(np) {
                /* Wake IRQ is optional */
                dev->wakeirq = of_irq_get_byname(np, "wakeup");
                if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
                        return dev->wakeirq;
        }
of_irq_get_byname will already return an error if the property is not present. Use that.
Quoted 8 lines
Quoted 3 lines
I do not like that it returns -EINVAL when property is missing, can we change it to return -ENODATA (so it is the same as when the property is defined but such name is missing)?
So here is what I had in mind.. It is based on recent patch by Vignesh for pixcir touchscreen, but I think it should be made available to all I2C devices. Completely untested at the moment.
Thanks for the patch! I tested this on am437x-gp-evm and the suspend/resume worked fine (below patch + $subject patch). I was able to wake the system from low power state using touchscreen. Also verified module insertion and removal. One comment though, please see below.
Quoted 59 lines
---
 drivers/i2c/i2c-core.c |   43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)
drivers/i2c/i2c-core.c +17 -4
index e6d4935..3971461 100644
@@ -47,6 +47,7 @@
 #include <linux/rwsem.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/acpi.h>
 #include <linux/jump_label.h>
 #include <asm/uaccess.h>
@@ -631,6 +632,7 @@ static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
 	struct i2c_driver	*driver;
+	int wakeirq = 0;
 	int status;
 
 	if (!client)
@@ -639,11 +641,13 @@ static int i2c_device_probe(struct device *dev)
 	if (!client->irq) {
 		int irq = -ENOENT;
 
-		if (dev->of_node)
-			irq = of_irq_get(dev->of_node, 0);
-		else if (ACPI_COMPANION(dev))
+		if (dev->of_node) {
+			irq = of_has_named_irqs(dev->of_node) ?
+				of_irq_get_byname(dev->of_node, "irq") :
+				of_irq_get(dev->of_node, 0);
+		} else if (ACPI_COMPANION(dev)) {
 			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
-
+		}
 		if (irq == -EPROBE_DEFER)
 			return irq;
 		if (irq < 0)
@@ -652,6 +656,15 @@ static int i2c_device_probe(struct device *dev)
 		client->irq = irq;
 	}
 
+	if (dev->of_node && of_has_named_irqs(dev->of_node)) {
+		wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
+		if (wakeirq < 0) {
+			if (wakeirq != -ENODATA)
+				return wakeirq;
+			wakeirq = 0;
+		}
+	}
+
 	driver = to_i2c_driver(dev->driver);
 	if (!driver->probe || !driver->id_table)
 		return -ENODEV;
drivers/i2c/i2c-core.c static int i2c_device_probe(struct device *dev) · line 679
Quoted 8 lines
drivers/i2c/i2c-core.c +5 -0
@@ -659,20 +672,34 @@ static int i2c_device_probe(struct device *dev)
 	if (!device_can_wakeup(&client->dev))
 		device_init_wakeup(&client->dev,
 					client->flags & I2C_CLIENT_WAKE);
+
+	status = wakeirq > 0 ?
+			dev_pm_set_dedicated_wake_irq(dev, wakeirq) :
+			(client->irq > 0 ?
+				dev_pm_set_wake_irq(dev, client->irq) : 0);
Above code tries to register wakeirq irrespective of whether the device is specified as wakeup-source in the dt or not. Hence, I see warn messages from every i2c device that has irq line but hasn't declared itself as wakeup-source:
For example tps is on i2c:

Quoted 5 lines

[    1.961613] tps65218 0-0024: forgot to call call device_init_wakeup?
[    1.968340] tps65218 0-0024: failed to set up wakeup irq
May be you can register wakeup-source only if I2C_CLIENT_WAKE flag is set in client->flags?

Makes sense, I'll prepare a new version.

Just to confirm: do you see any issues with these 2 patches if you do not modify the drivers to drop calls enable_irq_wake() from them?

Thanks.

Signature · 5 lines

Dmitry
--
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

R, Vignesh

2015-07-28 17:46 UTC #6 lore raw

Hi,

Quoted 97 lines · Dmitry Torokhov

Quoted 96 lines · Vignesh R
Hi Dmitry,
Quoted 33 lines · Dmitry Torokhov
Quoted 32 lines · Dmitry Torokhov
Quoted 10 lines · Rob Herring
Quoted 4 lines · Dmitry Torokhov
Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.
Generally, IRQs are retrieved by platform_get_irq or platform_get_irq_byname. Drivers should not call the of_irq_* functions directly in most cases.
That would be true for platform drivers, but not all devices are platform devices.
Quoted 16 lines
Quoted 5 lines
Signed-off-by Dmitry Torokhov <[email protected]>
---
Quoted 8 lines
The intent is to it like this:
        if (of_has_named_irqs(np) {
                /* Wake IRQ is optional */
                dev->wakeirq = of_irq_get_byname(np, "wakeup");
                if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
                        return dev->wakeirq;
        }
of_irq_get_byname will already return an error if the property is not present. Use that.
Quoted 8 lines
Quoted 3 lines
I do not like that it returns -EINVAL when property is missing, can we change it to return -ENODATA (so it is the same as when the property is defined but such name is missing)?
So here is what I had in mind.. It is based on recent patch by Vignesh for pixcir touchscreen, but I think it should be made available to all I2C devices. Completely untested at the moment.
Thanks for the patch! I tested this on am437x-gp-evm and the suspend/resume worked fine (below patch + $subject patch). I was able to wake the system from low power state using touchscreen. Also verified module insertion and removal. One comment though, please see below.
Quoted 46 lines
---
 drivers/i2c/i2c-core.c |   43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)
drivers/i2c/i2c-core.c +8 -4
index e6d4935..3971461 100644
@@ -47,6 +47,7 @@
 #include <linux/rwsem.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/acpi.h>
 #include <linux/jump_label.h>
 #include <asm/uaccess.h>
@@ -631,6 +632,7 @@ static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
 	struct i2c_driver	*driver;
+	int wakeirq = 0;
 	int status;
 
 	if (!client)
@@ -639,11 +641,13 @@ static int i2c_device_probe(struct device *dev)
 	if (!client->irq) {
 		int irq = -ENOENT;
 
-		if (dev->of_node)
-			irq = of_irq_get(dev->of_node, 0);
-		else if (ACPI_COMPANION(dev))
+		if (dev->of_node) {
+			irq = of_has_named_irqs(dev->of_node) ?
+				of_irq_get_byname(dev->of_node, "irq") :
+				of_irq_get(dev->of_node, 0);
+		} else if (ACPI_COMPANION(dev)) {
 			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
-
+		}
 		if (irq == -EPROBE_DEFER)
 			return irq;
 		if (irq < 0)
@@ -652,6 +656,15 @@ static int i2c_device_probe(struct device *dev)
 		client->irq = irq;
 	}
 

Quoted 12 lines

Quoted 4 lines

drivers/i2c/i2c-core.c +4 -0

+	if (dev->of_node && of_has_named_irqs(dev->of_node)) {
+		wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
+		if (wakeirq < 0) {
+			if (wakeirq != -ENODATA)

Quoted 8 lines

drivers/i2c/i2c-core.c static int i2c_device_probe(struct device *dev) · line 670

Quoted 8 lines

drivers/i2c/i2c-core.c +5 -0

+				return wakeirq;
+			wakeirq = 0;
+		}
+	}
+
 	driver = to_i2c_driver(dev->driver);
 	if (!driver->probe || !driver->id_table)
 		return -ENODEV;diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c

index 8f3e243a62bf..6bd1d1ca9883 100644

drivers/input/touchscreen/pixcir_i2c_ts.c +0 -0

@@ -365,7 +365,6 @@ static int __maybe_unused

pixcir_i2c_ts_suspend(struct device *dev)

                        }
                }

-               enable_irq_wake(client->irq);
        } else if (input->users) {
                ret = pixcir_stop(ts);
        }
@@ -386,7 +385,6 @@ static int __maybe_unused
pixcir_i2c_ts_resume(struct device *dev)
        mutex_lock(&input->mutex);

        if (device_may_wakeup(&client->dev)) {

- disable_irq_wake(client->irq);

                if (!input->users) {
                        ret = pixcir_stop(ts);
@@ -464,7 +462,7 @@ static int pixcir_i2c_ts_probe(struct i2c_client
*client,
        struct pixcir_i2c_ts_data *tsdata;
        struct input_dev *input;
        int error;
-
+printk("PROBING PIXCIR\n");
        if (np && !pdata) {
                pdata = pixcir_parse_dt(dev);
                if (IS_ERR(pdata))
@@ -562,14 +560,6 @@ static int pixcir_i2c_ts_probe(struct i2c_client
*client,
                return error;

        i2c_set_clientdata(client, tsdata);

- device_init_wakeup(&client->dev, 1);
-
- return 0;
-}
-
-static int pixcir_i2c_ts_remove(struct i2c_client *client) -{
- device_init_wakeup(&client->dev, 0);

        return 0;

}
@@ -608,7 +598,6 @@ static struct i2c_driver pixcir_i2c_ts_driver = {
.of_match_table = of_match_ptr(pixcir_of_match),
},
.probe = pixcir_i2c_ts_probe,
- .remove = pixcir_i2c_ts_remove,
.id_table = pixcir_i2c_ts_id,
};

Quoted 18 lines

Quoted 18 lines
drivers/i2c/i2c-core.c static int i2c_device_probe(struct device *dev) · line 679
Quoted 9 lines
drivers/i2c/i2c-core.c +5 -0
@@ -659,20 +672,34 @@ static int i2c_device_probe(struct device *dev)
 	if y i2c(!device_can_wakeup(&client->dev))
 		device_init_wakeup(&client->dev,
 					client->flags & I2C_CLIENT_WAKE);
+
+	status = wakeirq > 0 ?
+			dev_pm_set_dedicated_wake_irq(dev, wakeirq) :
+			(client->irq > 0 ?
+				dev_pm_set_wake_irq(dev, client->irq) : 0);
Above code tries to register wakeirq irrespective of whether the device is specified as wakeup-source in the dt or not. Hence, I see warn messages from every i2c device that has irq line but hasn't declared itself as wakeup-source:
For example tps is on i2c:
[    1.961613] tps65218 0-0024: forgot to call call device_init_wakeup?
[    1.968340] tps65218 0-0024: failed to set up wakeup irq

Quoted 7 lines

Quoted 2 lines
May be you can register wakeup-source only if I2C_CLIENT_WAKE flag is set in client->flags?
Makes sense, I'll prepare a new version.
Just to confirm: do you see any issues with these 2 patches if you do not modify the drivers to drop calls enable_irq_wake() from them?

No, I didn't test w/o dropping enable_irq_wake() calls. I will do that tomo. Is this to verify that these 2 patches can be added w/o dropping enable_irq_wake() from other i2c drivers currently using them?

Regards
Vignesh

Dmitry Torokhov

author maintainer

2015-07-28 18:07 UTC #7 lore raw

Quoted 187 lines · Vignesh

Hi,
Quoted 97 lines · Dmitry Torokhov
Quoted 96 lines · Vignesh R
Hi Dmitry,
Quoted 33 lines · Dmitry Torokhov
Quoted 32 lines · Dmitry Torokhov
Quoted 10 lines · Rob Herring
Quoted 4 lines · Dmitry Torokhov
Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.
Generally, IRQs are retrieved by platform_get_irq or platform_get_irq_byname. Drivers should not call the of_irq_* functions directly in most cases.
That would be true for platform drivers, but not all devices are platform devices.
Quoted 16 lines
Quoted 5 lines
Signed-off-by Dmitry Torokhov <[email protected]>
---
Quoted 8 lines
The intent is to it like this:
        if (of_has_named_irqs(np) {
                /* Wake IRQ is optional */
                dev->wakeirq = of_irq_get_byname(np, "wakeup");
                if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
                        return dev->wakeirq;
        }
of_irq_get_byname will already return an error if the property is not present. Use that.
Quoted 8 lines
Quoted 3 lines
I do not like that it returns -EINVAL when property is missing, can we change it to return -ENODATA (so it is the same as when the property is defined but such name is missing)?
So here is what I had in mind.. It is based on recent patch by Vignesh for pixcir touchscreen, but I think it should be made available to all I2C devices. Completely untested at the moment.
Thanks for the patch! I tested this on am437x-gp-evm and the suspend/resume worked fine (below patch + $subject patch). I was able to wake the system from low power state using touchscreen. Also verified module insertion and removal. One comment though, please see below.
Quoted 46 lines
---
 drivers/i2c/i2c-core.c |   43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)
drivers/i2c/i2c-core.c +8 -4
index e6d4935..3971461 100644
@@ -47,6 +47,7 @@
 #include <linux/rwsem.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/acpi.h>
 #include <linux/jump_label.h>
 #include <asm/uaccess.h>
@@ -631,6 +632,7 @@ static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
 	struct i2c_driver	*driver;
+	int wakeirq = 0;
 	int status;
 
 	if (!client)
@@ -639,11 +641,13 @@ static int i2c_device_probe(struct device *dev)
 	if (!client->irq) {
 		int irq = -ENOENT;
 
-		if (dev->of_node)
-			irq = of_irq_get(dev->of_node, 0);
-		else if (ACPI_COMPANION(dev))
+		if (dev->of_node) {
+			irq = of_has_named_irqs(dev->of_node) ?
+				of_irq_get_byname(dev->of_node, "irq") :
+				of_irq_get(dev->of_node, 0);
+		} else if (ACPI_COMPANION(dev)) {
 			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
-
+		}
 		if (irq == -EPROBE_DEFER)
 			return irq;
 		if (irq < 0)
@@ -652,6 +656,15 @@ static int i2c_device_probe(struct device *dev)
 		client->irq = irq;
 	}
 
Quoted 12 lines
Quoted 4 lines
Quoted 4 lines
drivers/i2c/i2c-core.c +4 -0
+	if (dev->of_node && of_has_named_irqs(dev->of_node)) {
+		wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
+		if (wakeirq < 0) {
+			if (wakeirq != -ENODATA)
Quoted 8 lines
drivers/i2c/i2c-core.c static int i2c_device_probe(struct device *dev) · line 670
Quoted 8 lines
drivers/i2c/i2c-core.c +5 -0
+				return wakeirq;
+			wakeirq = 0;
+		}
+	}
+
 	driver = to_i2c_driver(dev->driver);
 	if (!driver->probe || !driver->id_table)
 		return -ENODEV;diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
index 8f3e243a62bf..6bd1d1ca9883 100644
drivers/input/touchscreen/pixcir_i2c_ts.c +0 -1
@@ -365,7 +365,6 @@ static int __maybe_unused
 pixcir_i2c_ts_suspend(struct device *dev)
                        }
                }
 
-               enable_irq_wake(client->irq);
        } else if (input->users) {
                ret = pixcir_stop(ts);
        }
drivers/input/touchscreen/pixcir_i2c_ts.c +0 -1
@@ -386,7 +385,6 @@ static int __maybe_unused
 pixcir_i2c_ts_resume(struct device *dev)
        mutex_lock(&input->mutex);
 
        if (device_may_wakeup(&client->dev)) {
-               disable_irq_wake(client->irq);
 
                if (!input->users) {
                        ret = pixcir_stop(ts);
drivers/input/touchscreen/pixcir_i2c_ts.c +1 -1
@@ -464,7 +462,7 @@ static int pixcir_i2c_ts_probe(struct i2c_client
 *client,
        struct pixcir_i2c_ts_data *tsdata;
        struct input_dev *input;
        int error;
-
+printk("PROBING PIXCIR\n");
        if (np && !pdata) {
                pdata = pixcir_parse_dt(dev);
                if (IS_ERR(pdata))
drivers/input/touchscreen/pixcir_i2c_ts.c +0 -8
@@ -562,14 +560,6 @@ static int pixcir_i2c_ts_probe(struct i2c_client
 *client,
                return error;
 
        i2c_set_clientdata(client, tsdata);
-       device_init_wakeup(&client->dev, 1);
-
-       return 0;
-}
-
-static int pixcir_i2c_ts_remove(struct i2c_client *client)
-{
-       device_init_wakeup(&client->dev, 0);
 
        return 0;
}
drivers/input/touchscreen/pixcir_i2c_ts.c +0 -1
@@ -608,7 +598,6 @@ static struct i2c_driver pixcir_i2c_ts_driver = {
                .of_match_table = of_match_ptr(pixcir_of_match),
        },
        .probe          = pixcir_i2c_ts_probe,
-       .remove         = pixcir_i2c_ts_remove,
        .id_table       = pixcir_i2c_ts_id,
 };
 >>> @@ -659,20 +672,34 @@ static int i2c_device_probe(struct device *dev)
Quoted 20 lines
Quoted 19 lines
drivers/i2c/i2c-core.c
Quoted 8 lines
drivers/i2c/i2c-core.c +5 -0
 	if y i2c(!device_can_wakeup(&client->dev))
 		device_init_wakeup(&client->dev,
 					client->flags & I2C_CLIENT_WAKE);
+
+	status = wakeirq > 0 ?
+			dev_pm_set_dedicated_wake_irq(dev, wakeirq) :
+			(client->irq > 0 ?
+				dev_pm_set_wake_irq(dev, client->irq) : 0);
Above code tries to register wakeirq irrespective of whether the device is specified as wakeup-source in the dt or not. Hence, I see warn messages from every i2c device that has irq line but hasn't declared itself as wakeup-source:
For example tps is on i2c:
[    1.961613] tps65218 0-0024: forgot to call call device_init_wakeup?
[    1.968340] tps65218 0-0024: failed to set up wakeup irq
May be you can register wakeup-source only if I2C_CLIENT_WAKE flag is set in client->flags?

Quoted 8 lines

Quoted 4 lines
Makes sense, I'll prepare a new version.
Just to confirm: do you see any issues with these 2 patches if you do not modify the drivers to drop calls enable_irq_wake() from them?
No, I didn't test w/o dropping enable_irq_wake() calls. I will do that tomo. Is this to verify that these 2 patches can be added w/o dropping enable_irq_wake() from other i2c drivers currently using them?

Yes.

Thanks.

Signature · 1 line

Dmitry

Vignesh R

2015-07-29 04:33 UTC #8 lore raw

Quoted 190 lines · Dmitry Torokhov

Quoted 189 lines · Vignesh
Hi,
Quoted 97 lines · Dmitry Torokhov
Quoted 96 lines · Vignesh R
Hi Dmitry,
Quoted 33 lines · Dmitry Torokhov
Quoted 32 lines · Dmitry Torokhov
Quoted 10 lines · Rob Herring
Quoted 4 lines · Dmitry Torokhov
Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.
Generally, IRQs are retrieved by platform_get_irq or platform_get_irq_byname. Drivers should not call the of_irq_* functions directly in most cases.
That would be true for platform drivers, but not all devices are platform devices.
Quoted 16 lines
Quoted 5 lines
Signed-off-by Dmitry Torokhov <[email protected]>
---
Quoted 8 lines
The intent is to it like this:
        if (of_has_named_irqs(np) {
                /* Wake IRQ is optional */
                dev->wakeirq = of_irq_get_byname(np, "wakeup");
                if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
                        return dev->wakeirq;
        }
of_irq_get_byname will already return an error if the property is not present. Use that.
Quoted 8 lines
Quoted 3 lines
I do not like that it returns -EINVAL when property is missing, can we change it to return -ENODATA (so it is the same as when the property is defined but such name is missing)?
So here is what I had in mind.. It is based on recent patch by Vignesh for pixcir touchscreen, but I think it should be made available to all I2C devices. Completely untested at the moment.
Thanks for the patch! I tested this on am437x-gp-evm and the suspend/resume worked fine (below patch + $subject patch). I was able to wake the system from low power state using touchscreen. Also verified module insertion and removal. One comment though, please see below.
Quoted 46 lines
---
 drivers/i2c/i2c-core.c |   43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)
drivers/i2c/i2c-core.c +8 -4
index e6d4935..3971461 100644
@@ -47,6 +47,7 @@
 #include <linux/rwsem.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/acpi.h>
 #include <linux/jump_label.h>
 #include <asm/uaccess.h>
@@ -631,6 +632,7 @@ static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
 	struct i2c_driver	*driver;
+	int wakeirq = 0;
 	int status;
 
 	if (!client)
@@ -639,11 +641,13 @@ static int i2c_device_probe(struct device *dev)
 	if (!client->irq) {
 		int irq = -ENOENT;
 
-		if (dev->of_node)
-			irq = of_irq_get(dev->of_node, 0);
-		else if (ACPI_COMPANION(dev))
+		if (dev->of_node) {
+			irq = of_has_named_irqs(dev->of_node) ?
+				of_irq_get_byname(dev->of_node, "irq") :
+				of_irq_get(dev->of_node, 0);
+		} else if (ACPI_COMPANION(dev)) {
 			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
-
+		}
 		if (irq == -EPROBE_DEFER)
 			return irq;
 		if (irq < 0)
@@ -652,6 +656,15 @@ static int i2c_device_probe(struct device *dev)
 		client->irq = irq;
 	}
 
Quoted 12 lines
Quoted 4 lines
Quoted 4 lines
drivers/i2c/i2c-core.c +4 -0
+	if (dev->of_node && of_has_named_irqs(dev->of_node)) {
+		wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
+		if (wakeirq < 0) {
+			if (wakeirq != -ENODATA)
Quoted 8 lines
drivers/i2c/i2c-core.c static int i2c_device_probe(struct device *dev) · line 670
Quoted 8 lines
drivers/i2c/i2c-core.c +5 -0
+				return wakeirq;
+			wakeirq = 0;
+		}
+	}
+
 	driver = to_i2c_driver(dev->driver);
 	if (!driver->probe || !driver->id_table)
 		return -ENODEV;diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
index 8f3e243a62bf..6bd1d1ca9883 100644
drivers/input/touchscreen/pixcir_i2c_ts.c +0 -1
@@ -365,7 +365,6 @@ static int __maybe_unused
 pixcir_i2c_ts_suspend(struct device *dev)
                        }
                }
 
-               enable_irq_wake(client->irq);
        } else if (input->users) {
                ret = pixcir_stop(ts);
        }
drivers/input/touchscreen/pixcir_i2c_ts.c +0 -1
@@ -386,7 +385,6 @@ static int __maybe_unused
 pixcir_i2c_ts_resume(struct device *dev)
        mutex_lock(&input->mutex);
 
        if (device_may_wakeup(&client->dev)) {
-               disable_irq_wake(client->irq);
 
                if (!input->users) {
                        ret = pixcir_stop(ts);
drivers/input/touchscreen/pixcir_i2c_ts.c +1 -1
@@ -464,7 +462,7 @@ static int pixcir_i2c_ts_probe(struct i2c_client
 *client,
        struct pixcir_i2c_ts_data *tsdata;
        struct input_dev *input;
        int error;
-
+printk("PROBING PIXCIR\n");
        if (np && !pdata) {
                pdata = pixcir_parse_dt(dev);
                if (IS_ERR(pdata))
drivers/input/touchscreen/pixcir_i2c_ts.c +0 -8
@@ -562,14 +560,6 @@ static int pixcir_i2c_ts_probe(struct i2c_client
 *client,
                return error;
 
        i2c_set_clientdata(client, tsdata);
-       device_init_wakeup(&client->dev, 1);
-
-       return 0;
-}
-
-static int pixcir_i2c_ts_remove(struct i2c_client *client)
-{
-       device_init_wakeup(&client->dev, 0);
 
        return 0;
}
drivers/input/touchscreen/pixcir_i2c_ts.c +0 -1
@@ -608,7 +598,6 @@ static struct i2c_driver pixcir_i2c_ts_driver = {
                .of_match_table = of_match_ptr(pixcir_of_match),
        },
        .probe          = pixcir_i2c_ts_probe,
-       .remove         = pixcir_i2c_ts_remove,
        .id_table       = pixcir_i2c_ts_id,
 };
 >>> @@ -659,20 +672,34 @@ static int i2c_device_probe(struct device *dev)
Quoted 22 lines
Quoted 14 lines
drivers/i2c/i2c-core.c
Quoted 8 lines
drivers/i2c/i2c-core.c +5 -0
 	if y i2c(!device_can_wakeup(&client->dev))
 		device_init_wakeup(&client->dev,
 					client->flags & I2C_CLIENT_WAKE);
+
+	status = wakeirq > 0 ?
+			dev_pm_set_dedicated_wake_irq(dev, wakeirq) :
+			(client->irq > 0 ?
+				dev_pm_set_wake_irq(dev, client->irq) : 0);
Above code tries to register wakeirq irrespective of whether the device is specified as wakeup-source in the dt or not. Hence, I see warn messages from every i2c device that has irq line but hasn't declared itself as wakeup-source:
For example tps is on i2c:
Quoted 5 lines
[    1.961613] tps65218 0-0024: forgot to call call device_init_wakeup?
[    1.968340] tps65218 0-0024: failed to set up wakeup irq
May be you can register wakeup-source only if I2C_CLIENT_WAKE flag is set in client->flags?
Makes sense, I'll prepare a new version.

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
Just to confirm: do you see any issues with these 2 patches if you do not modify the drivers to drop calls enable_irq_wake() from them?
No, I didn't test w/o dropping enable_irq_wake() calls. I will do that tomo. Is this to verify that these 2 patches can be added w/o dropping enable_irq_wake() from other i2c drivers currently using them?
Yes.

Tested these 2 patches with enable/disable_irq_wake() calls in pixcir driver and I did not see any issues.

Signature · 2 lines

Regards
Vignesh

Rob Herring

2015-07-24 20:59 UTC #9 lore raw

In reply to Dmitry Torokhov

Quoted 8 lines · Dmitry Torokhov

Quoted 7 lines · Rob Herring
Quoted 4 lines · Dmitry Torokhov
Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.

Quoted 6 lines

Quoted 3 lines
Generally, IRQs are retrieved by platform_get_irq or platform_get_irq_byname. Drivers should not call the of_irq_* functions directly in most cases.
That would be true for platform drivers, but not all devices are platform devices.

I should say for drivers, IRQs are retrieved by a bus specific method, not DT functions. There are 3 cases of drivers using them:

arch/arm/mach-omap2/prm3xxx.c:                  irq_num = of_irq_get(np, 0);
arch/arm/mach-omap2/prm44xx.c:  irq_num = of_irq_get(prm_init_data->np, 0);

drivers/pinctrl/sirf/pinctrl-atlas7.c: ret = of_irq_get(np, idx);

Quoted 12 lines

Quoted 11 lines

Signed-off-by Dmitry Torokhov <[email protected]>

---

The intent is to it like this:

        if (of_has_named_irqs(np) {
                /* Wake IRQ is optional */
                dev->wakeirq = of_irq_get_byname(np, "wakeup");
                if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
                        return dev->wakeirq;
        }

Quoted 6 lines

Quoted 2 lines
of_irq_get_byname will already return an error if the property is not present. Use that.
I do not like that it returns -EINVAL when property is missing, can we change it to return -ENODATA (so it is the same as when the property is defined but such name is missing)?

It may be useful to distinguish between those cases. If you want audit all the callers we could possibly change it though.

Really, I don't understand why you would care about any error code other than EPROBE_DEFER. If it is optional, then you would want to continue no matter what is returned.

Rob
--
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

Dmitry Torokhov

author maintainer

2015-07-24 21:22 UTC #10 lore raw

Quoted 17 lines · Rob Herring

Quoted 8 lines · Dmitry Torokhov
Quoted 7 lines · Rob Herring
Quoted 4 lines · Dmitry Torokhov
Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.
Quoted 6 lines
Quoted 3 lines
Generally, IRQs are retrieved by platform_get_irq or platform_get_irq_byname. Drivers should not call the of_irq_* functions directly in most cases.
That would be true for platform drivers, but not all devices are platform devices.

Quoted 6 lines

I should say for drivers, IRQs are retrieved by a bus specific method, not DT functions. There are 3 cases of drivers using them:
arch/arm/mach-omap2/prm3xxx.c:                  irq_num = of_irq_get(np, 0);
arch/arm/mach-omap2/prm44xx.c:  irq_num = of_irq_get(prm_init_data->np, 0);
drivers/pinctrl/sirf/pinctrl-atlas7.c: ret = of_irq_get(np, idx);

Right, but I think with the newer wakeirq infrastructure we may start seeing them for non-platform devices unless we push it into bus code (as I've attempted to do in the patch I sent in the other email).

Quoted 16 lines

Quoted 16 lines
Quoted 15 lines
Quoted 4 lines
Signed-off-by Dmitry Torokhov <[email protected]>
---
Quoted 8 lines
The intent is to it like this:
        if (of_has_named_irqs(np) {
                /* Wake IRQ is optional */
                dev->wakeirq = of_irq_get_byname(np, "wakeup");
                if (dev->wakeirq < 0 && dev->wakeirq != -ENODATA)
                        return dev->wakeirq;
        }
of_irq_get_byname will already return an error if the property is not present. Use that.

Quoted 6 lines

Quoted 3 lines
I do not like that it returns -EINVAL when property is missing, can we change it to return -ENODATA (so it is the same as when the property is defined but such name is missing)?
It may be useful to distinguish between those cases. If you want audit all the callers we could possibly change it though.

It looks like the only user of of_irq_get_byname() is platform_get_irq_byname() and it doe snot care about anything but -EPROBE_DEFER, so my audit effort is complete ;)

Quoted 4 lines

Really, I don't understand why you would care about any error code other than EPROBE_DEFER. If it is optional, then you would want to continue no matter what is returned.

No, I only want to proceed if it is not there. If there are errors (in DTS, elsewhere in the kernel) I want to let the user know and abort.

Thanks.

Signature · 1 line

Dmitry

Florian Fainelli

2015-07-24 19:35 UTC #11 lore raw

In reply to Dmitry Torokhov

Quoted 7 lines · Dmitry Torokhov

Sometimes drivers might wish to transition from index-based to named interrupt descriptions. To aid in decision-making when parsing device tree data let's provide a helper that will indicate the scheme that is being used.
Signed-off-by Dmitry Torokhov <[email protected]>
---

[snip]

Quoted 9 lines

drivers/of/irq.c +1 -0

index 3cf7a01..e02c43e 100644
@@ -384,6 +384,19 @@ int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
 EXPORT_SYMBOL_GPL(of_irq_to_resource);
 
 /**
+ * of_has_named_irq - Check if given node contains named interrupts

drivers/of/irq.c int of_irq_to_resource(struct device_node *dev, int index, struct resource *r) · line 397

Quoted 10 lines

drivers/of/irq.c +10 -0

+ * @dev: pointer to device tree node
+ *
+ * Returns %true if node contains non-empty "interrupt-names" property.
+ */
+bool of_has_named_irqs(struct device_node *dev)
+{
+	struct property *prop = of_find_property(dev, "interrupt-names", NULL);
+
+	return prop && prop->value;
+}

Missing EXPORT_SYMBOL_GPL() here?

Signature · 5 lines

Florian
--
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