Hi Linus,
On Sat, Nov 28, 2020 at 01:37:20PM +0100, Linus Walleij wrote:
After commit 74d905d2d38a devices requiring the workaround for edge triggered interrupts stopped working.
This is because the "data" state container defaults to *not* using the workaround, but the workaround gets used *before* the check of whether it is needed or not. This semantic is not obvious from just looking on the patch, but related to the program flow.
The hardware needs the quirk to be used before even proceeding to check if the quirk is needed.
This patch makes the quirk be used until we determine it is *not* needed.
Thank you very much for root-causing the issue!
Cc: Andre andre.muller@web.de Cc: Nick Dyer nick.dyer@itdev.co.uk Cc: Jiada Wang jiada_wang@mentor.com Cc: stable@vger.kernel.org Fixes: 74d905d2d38a ("Input: atmel_mxt_ts - only read messages in mxt_acquire_irq() when necessary") Signed-off-by: Linus Walleij linus.walleij@linaro.org
drivers/input/touchscreen/atmel_mxt_ts.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index e34984388791..f25b2f6038a7 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -1297,8 +1297,6 @@ static int mxt_check_retrigen(struct mxt_data *data) int val; 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.
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.
irqd = irq_get_irq_data(data->irq); if (!irqd) return -EINVAL; @@ -1313,8 +1311,10 @@ static int mxt_check_retrigen(struct mxt_data *data) if (error) return error;
if (val & MXT_COMMS_RETRIGEN)
if (val & MXT_COMMS_RETRIGEN) {data->use_retrigen_workaround = false; return 0;}dev_warn(&client->dev, "Enabling RETRIGEN workaround\n"); @@ -3117,6 +3117,7 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id) data = devm_kzalloc(&client->dev, sizeof(struct mxt_data), GFP_KERNEL); if (!data) return -ENOMEM;
- data->use_retrigen_workaround = true;
snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0", client->adapter->nr, client->addr); -- 2.26.2
By the way, does your touchscreen work if you change interrupt trigger to level in DTS?
Thanks.