| Message ID | 20190816083558.19189-2-jiada_wang@mentor.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | None | expand |
On Fri, Aug 16, 2019 at 05:35:36PM +0900, Jiada Wang wrote: > From: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com> > > The de-/serializer driver has defined only irq_mask "ds90ub927_irq_mask" and > irq_unmask "ds90ub927_irq_unmask" callback functions. And de-/serializer > driver doesn't implement the irq_disable and irq_enable callback functions. > Hence inorder to invoke irq_mask callback function when disable_irq_nosync is > called the IRQ_DISABLE_UNLAZY interrupt flag should be set. If not the > disable_irq_nosync will just increment the depth field in the irq > descriptor only once as shown below. > > disable_irq_nosync > __disable_irq_nosync > __disable_irq (desc->depth++) > irq_disable > if irq_disable present -----------> if IRQ_DISABLE_UNLAZYflag set > | no | > yes | yes | > | | > desc->irq_data.chip->irq_disable desc->irq_data.chip->irq_unmask > (ds90ub927_irq_mask) > disable_irq > __disable_irq_nosync > __disable_irq > (desc->depth++) > But the enable_irq will try to decrement the depth field twice which generates > the backtrace stating "Unbalanced enable for irq 293". This is because there is > no IRQ_DISABLE_UNLAZY flag check while calling irq_unmask callback function > of the "ds90ub927_irq_unmask" de-/serializer via enable_irq. > > enable_irq > __enable_irq (desc->depth--) > irq_enable > if irq_enable present -------------> desc->irq_data.chip->irq_unmask > | no (ds90ub927_irq_unmask) > yes | enable_irq > | __enable_irq (desc->depth--) > (desc->irq_data.chip->irq_enable) I'd prefer if we instead did not use the disable_irq_nosync() in the driver. Thanks.
Hi Dmitry On 2019/08/17 2:26, Dmitry Torokhov wrote: > On Fri, Aug 16, 2019 at 05:35:36PM +0900, Jiada Wang wrote: >> From: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com> >> >> The de-/serializer driver has defined only irq_mask "ds90ub927_irq_mask" and >> irq_unmask "ds90ub927_irq_unmask" callback functions. And de-/serializer >> driver doesn't implement the irq_disable and irq_enable callback functions. >> Hence inorder to invoke irq_mask callback function when disable_irq_nosync is >> called the IRQ_DISABLE_UNLAZY interrupt flag should be set. If not the >> disable_irq_nosync will just increment the depth field in the irq >> descriptor only once as shown below. >> >> disable_irq_nosync >> __disable_irq_nosync >> __disable_irq (desc->depth++) >> irq_disable >> if irq_disable present -----------> if IRQ_DISABLE_UNLAZYflag set >> | no | >> yes | yes | >> | | >> desc->irq_data.chip->irq_disable desc->irq_data.chip->irq_unmask >> (ds90ub927_irq_mask) >> disable_irq >> __disable_irq_nosync >> __disable_irq >> (desc->depth++) >> But the enable_irq will try to decrement the depth field twice which generates >> the backtrace stating "Unbalanced enable for irq 293". This is because there is >> no IRQ_DISABLE_UNLAZY flag check while calling irq_unmask callback function >> of the "ds90ub927_irq_unmask" de-/serializer via enable_irq. >> >> enable_irq >> __enable_irq (desc->depth--) >> irq_enable >> if irq_enable present -------------> desc->irq_data.chip->irq_unmask >> | no (ds90ub927_irq_unmask) >> yes | enable_irq >> | __enable_irq (desc->depth--) >> (desc->irq_data.chip->irq_enable) > > I'd prefer if we instead did not use the disable_irq_nosync() in the > driver. > sorry for the mistake, during forward port, I have already eliminated disable_irq_nosync(), so this patch is no longer needed, will drop it in v2 patch-set Thanks， Jiada > Thanks. >
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index bc94adec6631..c6ba061098c0 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -4349,6 +4349,8 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id) snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0", client->adapter->nr, client->addr); + irq_set_status_flags(client->irq, IRQ_DISABLE_UNLAZY); + data->client = client; i2c_set_clientdata(client, data); @@ -4434,6 +4436,8 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id) sysfs_remove_link(&client->dev.kobj, "reset"); gpiod_unexport(data->reset_gpio); } + if (data->irq) + irq_clear_status_flags(data->irq, IRQ_DISABLE_UNLAZY); return error; } @@ -4449,6 +4453,8 @@ static int mxt_remove(struct i2c_client *client) } mxt_debug_msg_remove(data); mxt_sysfs_remove(data); + if (data->irq) + irq_clear_status_flags(data->irq, IRQ_DISABLE_UNLAZY); mxt_free_input_device(data); mxt_free_object_table(data);