Skip to contentSkip to conversation

Input: elan_i2c - do not clobber interrupt trigger on x86

PATCH In review Dmitry Torokhov 2 participants 3 messages Sep 28, 2017 – Oct 10, 2017

Subsystems (2)
lore
Share & tools
lore

Dmitry Torokhov

author maintainer
#0 lore raw

PATCH 1/1 Input: elan_i2c - do not clobber interrupt trigger on x86

On x86 we historically used falling edge interrupts in the driver because that's how first Chrome devices were configured. They also did not use ACPI to enumerate I2C devices (because back then there was no kernel support for that), so trigger was hard-coded in the driver. However the controller behavior is much more reliable if we use level triggers, and that is how we configured ARM devices, and how want to configure newer x86 devices as well. All newer x86 boxes have their I2C devices enumerated in ACPI.

Let's see if platform code (ACPI, DT) described interrupt and specified particular trigger type, and if so, let's use it instead of always clobbering trigger with IRQF_TRIGGER_FALLING. We will still use this trigger type as a fallback if platform code left interrupt trigger unconfigured.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196761

---
 drivers/input/mouse/elan_i2c_core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/input/mouse/elan_i2c_core.c +7 -3
index a2d2077303cb..c33f2cce6ba9 100644
@@ -26,6 +26,7 @@
 #include <linux/init.h>
 #include <linux/input/mt.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
@@ -1141,10 +1142,13 @@ static int elan_probe(struct i2c_client *client,
 		return error;
 
 	/*
-	 * Systems using device tree should set up interrupt via DTS,
-	 * the rest will use the default falling edge interrupts.
+	 * Platform code (ACPI, DTS) should normally set up interrupt
+	 * for us, but in case it did not let's fall back to using falling
+	 * edge to be compatible with older Chromebooks.
 	 */
-	irqflags = dev->of_node ? 0 : IRQF_TRIGGER_FALLING;
+	irqflags = irq_get_trigger_type(client->irq);
+	if (!irqflags)
+		irqflags = IRQF_TRIGGER_FALLING;
 
 	error = devm_request_threaded_irq(dev, client->irq, NULL, elan_isr,
 					  irqflags | IRQF_ONESHOT,
Signature · 5 lines
2.14.2.822.g60be5d43e6-goog


-- 
Dmitry
author maintainer
#1 lore raw
Quoted 9 lines · Dmitry Torokhov

On x86 we historically used falling edge interrupts in the driver because that's how first Chrome devices were configured. They also did not use ACPI to enumerate I2C devices (because back then there was no kernel support for that), so trigger was hard-coded in the driver. However the controller behavior is much more reliable if we use level triggers, and that is how we configured ARM devices, and how want to configure newer x86 devices as well. All newer x86 boxes have their I2C devices enumerated in ACPI.

Quoted 8 lines

Let's see if platform code (ACPI, DT) described interrupt and specified particular trigger type, and if so, let's use it instead of always clobbering trigger with IRQF_TRIGGER_FALLING. We will still use this trigger type as a fallback if platform code left interrupt trigger unconfigured.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196761

Guys, can get some revewed-bys please so I do not appear merging too much code unilaterally ;) ?

Quoted 39 lines
---
 drivers/input/mouse/elan_i2c_core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/input/mouse/elan_i2c_core.c +7 -3
index a2d2077303cb..c33f2cce6ba9 100644
@@ -26,6 +26,7 @@
 #include <linux/init.h>
 #include <linux/input/mt.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
@@ -1141,10 +1142,13 @@ static int elan_probe(struct i2c_client *client,
 		return error;
 
 	/*
-	 * Systems using device tree should set up interrupt via DTS,
-	 * the rest will use the default falling edge interrupts.
+	 * Platform code (ACPI, DTS) should normally set up interrupt
+	 * for us, but in case it did not let's fall back to using falling
+	 * edge to be compatible with older Chromebooks.
 	 */
-	irqflags = dev->of_node ? 0 : IRQF_TRIGGER_FALLING;
+	irqflags = irq_get_trigger_type(client->irq);
+	if (!irqflags)
+		irqflags = IRQF_TRIGGER_FALLING;
 
 	error = devm_request_threaded_irq(dev, client->irq, NULL, elan_isr,
 					  irqflags | IRQF_ONESHOT,
Signature · 5 lines
2.14.2.822.g60be5d43e6-goog


-- 
Dmitry
Signature · 1 line
Dmitry
Reviewed-by on v1 1/1
#2 lore raw
In reply to Dmitry Torokhov
Quoted 51 lines · Dmitry Torokhov

On x86 we historically used falling edge interrupts in the driver because that's how first Chrome devices were configured. They also did not use ACPI to enumerate I2C devices (because back then there was no kernel support for that), so trigger was hard-coded in the driver. However the controller behavior is much more reliable if we use level triggers, and that is how we configured ARM devices, and how want to configure newer x86 devices as well. All newer x86 boxes have their I2C devices enumerated in ACPI.

Let's see if platform code (ACPI, DT) described interrupt and specified particular trigger type, and if so, let's use it instead of always clobbering trigger with IRQF_TRIGGER_FALLING. We will still use this trigger type as a fallback if platform code left interrupt trigger unconfigured.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196761

---
 drivers/input/mouse/elan_i2c_core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/input/mouse/elan_i2c_core.c +7 -4
index a2d2077303cb..c33f2cce6ba9 100644
@@ -26,6 +26,7 @@
 #include <linux/init.h>
 #include <linux/input/mt.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
@@ -1141,10 +1142,13 @@ static int elan_probe(struct i2c_client *client,
                return error;
 
        /*
-        * Systems using device tree should set up interrupt via DTS,
-        * the rest will use the default falling edge interrupts.
+        * Platform code (ACPI, DTS) should normally set up interrupt
+        * for us, but in case it did not let's fall back to using falling
+        * edge to be compatible with older Chromebooks.
         */
-       irqflags = dev->of_node ? 0 : IRQF_TRIGGER_FALLING;
+       irqflags = irq_get_trigger_type(client->irq);
+       if (!irqflags)
+               irqflags = IRQF_TRIGGER_FALLING;
 
        error = devm_request_threaded_irq(dev, client->irq, NULL, elan_isr,
                                          irqflags | IRQF_ONESHOT,
--
Quoted 1 line

2.14.2.822.g60be5d43e6-goog