diff mbox

Input: elan_i2c - do not clobber interrupt trigger on x86

Message ID 20170928170902.GA18201@dtor-ws (mailing list archive)
State New, archived
Headers show

Commit Message

Dmitry Torokhov Sept. 28, 2017, 5:09 p.m. UTC
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
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/elan_i2c_core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Dmitry Torokhov Oct. 10, 2017, 3:50 a.m. UTC | #1
On Thu, Sep 28, 2017 at 10:09:02AM -0700, Dmitry Torokhov wrote:
> 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
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

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

> ---
>  drivers/input/mouse/elan_i2c_core.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index a2d2077303cb..c33f2cce6ba9 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -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,
> -- 
> 2.14.2.822.g60be5d43e6-goog
> 
> 
> -- 
> Dmitry
Aaron Durbin Oct. 10, 2017, 3 p.m. UTC | #2
On Thu, Sep 28, 2017 at 11:09 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> 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
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/mouse/elan_i2c_core.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index a2d2077303cb..c33f2cce6ba9 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -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,
> --
> 2.14.2.822.g60be5d43e6-goog

Reviewed-by: Aaron Durbin <adurbin@chromium.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index a2d2077303cb..c33f2cce6ba9 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -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,