On Sun, Nov 29, 2020 at 3:53 AM Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > On Sat, Nov 28, 2020 at 01:37:20PM +0100, Linus Walleij wrote: > > @@ -1297,8 +1297,6 @@ static int mxt_check_retrigen(struct mxt_data *data) > > int val;is > > struct irq_data *irqd; > > > > - data->use_retrigen_workaround = false; > > - > > So this will result in data->use_retrigen_workaround staying "true" for > level interrupts, which is not needed, as with those we will never lose > an edge. So I think your patch can be reduced to simply setting > data->use_retrigen_workaround to true in mxt_probe() and leaving > mxt_check_retrigen() without any changes. I did that first but then I realized that since there is an errorpath in mxt_check_retrigen() and it starts by disabling the workaround so in an error occurs in __mxt_read_reg() it will be left disabled. But I see that I fail to account for the level-trigging case where it should disable the workaround and bail out so I anyway need to revise the patch. > However I wonder if it would not be better to simply call > mxt_check_retrigen() before calling mxt_acquire_irq() in mxt_probe() > instead of after. I don't fully understand this driver, but it seems the information whether retrigen is available only appears after talking to the device a bit, checking firmware and "objects" available on the device and then it may already be too late. Someone who knows the device better might be able to contribute here :/ > By the way, does your touchscreen work if you change interrupt trigger > to level in DTS? Nope. This happens: [ 1.824610] atmel_mxt_ts 3-004a: Failed to register interrupt [ 1.830583] atmel_mxt_ts: probe of 3-004a failed with error -22 And that in turn is because this connected to a Nomadik GPIO controller which is one of those GPIO controllers that only support edge triggered interrupts and does not support level interrupts. So it needs to be edge triggered on this platform. Yours, Linus Walleij
