| Message ID | 1477035088-20587-1-git-send-email-Gang.Liu@nxp.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Oct 21, 2016 at 9:31 AM, Liu Gang <Gang.Liu@nxp.com> wrote: > From the beginning of the gpio-mpc8xxx.c, the "handle_level_irq" > has being used to handle GPIO interrupts in the PowerPC/Layerscape > platforms. But actually, almost all PowerPC/Layerscape platforms > assert an interrupt request upon either a high-to-low change or > any change on the state of the signal. > > So the "handle_level_irq" is not reasonable for PowerPC/Layerscape > GPIO interrupt, it should be "handle_edge_irq". Otherwise the system > may lost some interrupts from the PIN's state changes. > > Signed-off-by: Liu Gang <Gang.Liu@nxp.com> Yes and especially so since this irqchip implements .irq_ack() and unless you use the edge handler it will never be called. Patch applied for fixes, tell me if it also needs to go into stable. If you want to look closer at this driver, I think it is possible to simplify it quite a bit by using GPIOLIB_IRQCHIP like a few other drivers, for example gpio-pl061.c. Patches accepted :) Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
IA0KPiBPbiBGcmksIE9jdCAyMSwgMjAxNiBhdCA5OjMxIEFNLCBMaXUgR2FuZyA8R2FuZy5MaXVA bnhwLmNvbT4gd3JvdGU6DQo+IA0KPiA+IEZyb20gdGhlIGJlZ2lubmluZyBvZiB0aGUgZ3Bpby1t cGM4eHh4LmMsIHRoZSAiaGFuZGxlX2xldmVsX2lycSINCj4gPiBoYXMgYmVpbmcgdXNlZCB0byBo YW5kbGUgR1BJTyBpbnRlcnJ1cHRzIGluIHRoZSBQb3dlclBDL0xheWVyc2NhcGUNCj4gPiBwbGF0 Zm9ybXMuIEJ1dCBhY3R1YWxseSwgYWxtb3N0IGFsbCBQb3dlclBDL0xheWVyc2NhcGUgcGxhdGZv cm1zDQo+ID4gYXNzZXJ0IGFuIGludGVycnVwdCByZXF1ZXN0IHVwb24gZWl0aGVyIGEgaGlnaC10 by1sb3cgY2hhbmdlIG9yIGFueQ0KPiA+IGNoYW5nZSBvbiB0aGUgc3RhdGUgb2YgdGhlIHNpZ25h bC4NCj4gPg0KPiA+IFNvIHRoZSAiaGFuZGxlX2xldmVsX2lycSIgaXMgbm90IHJlYXNvbmFibGUg Zm9yIFBvd2VyUEMvTGF5ZXJzY2FwZQ0KPiA+IEdQSU8gaW50ZXJydXB0LCBpdCBzaG91bGQgYmUg ImhhbmRsZV9lZGdlX2lycSIuIE90aGVyd2lzZSB0aGUgc3lzdGVtDQo+ID4gbWF5IGxvc3Qgc29t ZSBpbnRlcnJ1cHRzIGZyb20gdGhlIFBJTidzIHN0YXRlIGNoYW5nZXMuDQo+ID4NCj4gPiBTaWdu ZWQtb2ZmLWJ5OiBMaXUgR2FuZyA8R2FuZy5MaXVAbnhwLmNvbT4NCj4gDQo+IFllcyBhbmQgZXNw ZWNpYWxseSBzbyBzaW5jZSB0aGlzIGlycWNoaXAgaW1wbGVtZW50cyAuaXJxX2FjaygpIGFuZCB1 bmxlc3MgeW91DQo+IHVzZSB0aGUgZWRnZSBoYW5kbGVyIGl0IHdpbGwgbmV2ZXIgYmUgY2FsbGVk Lg0KPiANCj4gUGF0Y2ggYXBwbGllZCBmb3IgZml4ZXMsIHRlbGwgbWUgaWYgaXQgYWxzbyBuZWVk cyB0byBnbyBpbnRvIHN0YWJsZS4NCj4gDQpbTGl1IEdhbmddIHllcywgcGxlYXNlIGFsc28gcHV0 IGl0IGludG8gc3RhYmxlLiBUaGFua3MhDQoNCj4gSWYgeW91IHdhbnQgdG8gbG9vayBjbG9zZXIg YXQgdGhpcyBkcml2ZXIsIEkgdGhpbmsgaXQgaXMgcG9zc2libGUgdG8gc2ltcGxpZnkgaXQgcXVp dGUgYQ0KPiBiaXQgYnkgdXNpbmcgR1BJT0xJQl9JUlFDSElQIGxpa2UgYSBmZXcgb3RoZXIgZHJp dmVycywgZm9yIGV4YW1wbGUgZ3Bpby1wbDA2MS5jLg0KPiBQYXRjaGVzIGFjY2VwdGVkIDopDQo+ IA0KPiBZb3VycywNCj4gTGludXMgV2FsbGVpag0K -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 24, 2016 at 5:46 AM, Gang Liu <gang.liu@nxp.com> wrote: >> On Fri, Oct 21, 2016 at 9:31 AM, Liu Gang <Gang.Liu@nxp.com> wrote: >> >> > From the beginning of the gpio-mpc8xxx.c, the "handle_level_irq" >> > has being used to handle GPIO interrupts in the PowerPC/Layerscape >> > platforms. But actually, almost all PowerPC/Layerscape platforms >> > assert an interrupt request upon either a high-to-low change or any >> > change on the state of the signal. >> > >> > So the "handle_level_irq" is not reasonable for PowerPC/Layerscape >> > GPIO interrupt, it should be "handle_edge_irq". Otherwise the system >> > may lost some interrupts from the PIN's state changes. >> > >> > Signed-off-by: Liu Gang <Gang.Liu@nxp.com> >> >> Yes and especially so since this irqchip implements .irq_ack() and unless you >> use the edge handler it will never be called. >> >> Patch applied for fixes, tell me if it also needs to go into stable. >> > [Liu Gang] yes, please also put it into stable. Thanks! Ooops too late. Will tell Greg to pick this separately. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c index 425501c..793518a 100644 --- a/drivers/gpio/gpio-mpc8xxx.c +++ b/drivers/gpio/gpio-mpc8xxx.c @@ -239,7 +239,7 @@ static int mpc8xxx_gpio_irq_map(struct irq_domain *h, unsigned int irq, irq_hw_number_t hwirq) { irq_set_chip_data(irq, h->host_data); - irq_set_chip_and_handler(irq, &mpc8xxx_irq_chip, handle_level_irq); + irq_set_chip_and_handler(irq, &mpc8xxx_irq_chip, handle_edge_irq); return 0; }
From the beginning of the gpio-mpc8xxx.c, the "handle_level_irq" has being used to handle GPIO interrupts in the PowerPC/Layerscape platforms. But actually, almost all PowerPC/Layerscape platforms assert an interrupt request upon either a high-to-low change or any change on the state of the signal. So the "handle_level_irq" is not reasonable for PowerPC/Layerscape GPIO interrupt, it should be "handle_edge_irq". Otherwise the system may lost some interrupts from the PIN's state changes. Signed-off-by: Liu Gang <Gang.Liu@nxp.com> --- drivers/gpio/gpio-mpc8xxx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)