On 12/21/2018 02:44 AM, Dmitry Torokhov wrote: > Hi Marek, Hi, > On Thu, Dec 20, 2018 at 09:43:00PM +0100, Marek Vasut wrote: >> Get rid of the workqueue, just spawn a threaded IRQ and handle > > Not really... Fixed >> @@ -279,7 +275,6 @@ static int ili210x_i2c_remove(struct i2c_client *client) >> struct ili210x *priv = i2c_get_clientdata(client); >> >> sysfs_remove_group(&client->dev.kobj, &ili210x_attr_group); >> - free_irq(priv->client->irq, priv); > > Now this is unsafe, as interrupt is released after work is cancelled and > interrupt may schedule work again. > > If we can prove that it works, I liked your original change better. Can > you try addind sleep of let's say 5 seconds to the interrupt thread > just before it returns and see if you get release events reported. The > idea is to verify the sequence: > > - chip raises interrupt line > - ISR reads chip state > - in response chip makes intterrupt line inactive? No, the IRQ line stays active as long as there's touch event happening (someone has a finger on the touch panel). > - finger leaves the surface > - hopefully chip activates interrupt line again > - ISR returns > - ISR fires again, reports release. > > > >> cancel_delayed_work_sync(&priv->dwork); >> input_unregister_device(priv->input); >> > > Thanks. > -- Best regards, Marek Vasut
