[v2,3/6] dt-bindings: input: cypress,tt21000 - fix interrupt type in dts example

Commit Message

Maximilian Weigand May 1, 2023, 11:30 a.m. UTC

Triggering the interrupt of the IRQ_TYPE_LEVEL_LOW type can lead to
probing issues with the device for the current driver (encountered on
the Pine64 PineNote). Basically the interrupt would be triggered before
certain commands were sent to the device, leading to a race between the
device responding fast enough and the irq handler fetching a data frame
from it. Actually all devices currently using the driver already use a
falling edge trigger.

Signed-off-by: Maximilian Weigand <[e-mail]>
Reviewed-by: Alistair Francis <[e-mail]>
Reviewed-by: Linus Walleij <[e-mail]>
---
 .../devicetree/bindings/input/touchscreen/cypress,tt21000.yaml  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dmitry Torokhov May 2, 2023, 12:24 a.m. UTC | #1

On Mon, May 01, 2023 at 01:30:07PM +0200, Maximilian Weigand wrote:
> Triggering the interrupt of the IRQ_TYPE_LEVEL_LOW type can lead to
> probing issues with the device for the current driver (encountered on
> the Pine64 PineNote). Basically the interrupt would be triggered before
> certain commands were sent to the device, leading to a race between the
> device responding fast enough and the irq handler fetching a data frame
> from it. Actually all devices currently using the driver already use a
> falling edge trigger.

I'd prefer we adjusted the driver to handle level interrupts properly.

Thanks.

Maximilian Weigand May 2, 2023, 2:23 p.m. UTC | #2

Hi,

On 02.05.23 02:24, Dmitry Torokhov wrote:
> On Mon, May 01, 2023 at 01:30:07PM +0200, Maximilian Weigand wrote:
>> Triggering the interrupt of the IRQ_TYPE_LEVEL_LOW type can lead to
>> probing issues with the device for the current driver (encountered on
>> the Pine64 PineNote). Basically the interrupt would be triggered before
>> certain commands were sent to the device, leading to a race between the
>> device responding fast enough and the irq handler fetching a data frame
>> from it. Actually all devices currently using the driver already use a
>> falling edge trigger.
> 
> I'd prefer we adjusted the driver to handle level interrupts properly.

Ok, I will have a look at that. Just to be clear: The driver should work
only with level interrupts, or should it optimally support both level
and falling edge triggers?

Thanks and best regards

Maximilian

Dmitry Torokhov May 2, 2023, 8:51 p.m. UTC | #3

On Tue, May 02, 2023 at 04:23:54PM +0200, Maximilian Weigand wrote:
> Hi,
> 
> On 02.05.23 02:24, Dmitry Torokhov wrote:
> > On Mon, May 01, 2023 at 01:30:07PM +0200, Maximilian Weigand wrote:
> >> Triggering the interrupt of the IRQ_TYPE_LEVEL_LOW type can lead to
> >> probing issues with the device for the current driver (encountered on
> >> the Pine64 PineNote). Basically the interrupt would be triggered before
> >> certain commands were sent to the device, leading to a race between the
> >> device responding fast enough and the irq handler fetching a data frame
> >> from it. Actually all devices currently using the driver already use a
> >> falling edge trigger.
> > 
> > I'd prefer we adjusted the driver to handle level interrupts properly.
> 
> Ok, I will have a look at that. Just to be clear: The driver should work
> only with level interrupts, or should it optimally support both level
> and falling edge triggers?

Optimally a driver would work well with both.

Thanks.

Patch

diff --git a/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml b/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml
index 1959ec394768..a77203c78d6e 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml
@@ -83,7 +83,7 @@  examples:
             pinctrl-names = "default";
             pinctrl-0 = <&tp_reset_ds203>;
             interrupt-parent = <&pio>;
-            interrupts = <1 5 IRQ_TYPE_LEVEL_LOW>;
+            interrupts = <1 5 IRQ_TYPE_EDGE_FALLING>;
             reset-gpios = <&pio 7 1 GPIO_ACTIVE_LOW>;
             vdd-supply = <&reg_touch>;