From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grygorii Strashko Subject: Re: [PATCH] gpio: pca953x: Fix warning when HW interrupts are rescheduled by the softirq tasklet Date: Tue, 23 Sep 2014 15:26:53 +0300 Message-ID: <5421670D.10200@ti.com> References: <1410251533-4990-1-git-send-email-LW@KARO-electronics.de> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from comal.ext.ti.com ([198.47.26.152]:55480 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751412AbaIWM1K (ORCPT ); Tue, 23 Sep 2014 08:27:10 -0400 In-Reply-To: Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Linus Walleij , =?UTF-8?B?TG90aGFyIFdhw59tYQ==?= =?UTF-8?B?bm4=?= Cc: "linux-gpio@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Alexandre Courbot On 09/23/2014 03:04 PM, Linus Walleij wrote: > On Tue, Sep 9, 2014 at 10:32 AM, Lothar Wa=C3=9Fmann wrote: >=20 >> When using e.g. the matrix_keymap driver with the gpio-pca953x drive= r, >> the following warning may be issued when a keypress is detected: >> WARNING: CPU: 0 PID: 3 at kernel/irq/manage.c:677 irq_nested_primary= _handler+0x18/0x2c() >> Primary handler called for nested irq 245 >> Modules linked in: evbug ci_hdrc_imx usbmisc_imx ci_hdrc udc_core eh= ci_hcd phy_mxs_usb >> CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: G W 3.16.0-karo+= #118 >> [] (unwind_backtrace) from [] (show_stack+0x10/0= x14) >> [] (show_stack) from [] (warn_slowpath_common+0x= 64/0x84) >> [] (warn_slowpath_common) from [] (warn_slowpath= _fmt+0x30/0x40) >> [] (warn_slowpath_fmt) from [] (irq_nested_prima= ry_handler+0x18/0x2c) >> [] (irq_nested_primary_handler) from [] (handle_= irq_event_percpu+0x50/0x168) >> [] (handle_irq_event_percpu) from [] (handle_irq= _event+0x60/0x7c) >> [] (handle_irq_event) from [] (handle_simple_irq= +0x78/0xdc) >> [] (handle_simple_irq) from [] (resend_irqs+0x4c= /0x78) >> [] (resend_irqs) from [] (tasklet_action+0x74/0x= d8) >> [] (tasklet_action) from [] (__do_softirq+0xd0/0= x1f4) >> [] (__do_softirq) from [] (run_ksoftirqd+0x40/0x= 64) >> [] (run_ksoftirqd) from [] (smpboot_thread_fn+0x= 174/0x234) >> [] (smpboot_thread_fn) from [] (kthread+0xb4/0xd= 0) >> [] (kthread) from [] (ret_from_fork+0x14/0x24) >> ---[ end trace a68cf7bc5348c4f7 ]--- >> >> This happens when an IRQ is detected by the GPIO driver while the GP= IO >> client driver (matrix_keypad in this case) has disabled the irq for >> the GPIO it has acquired. When the HW IRQ is being rescheduled by th= e >> softirq thread, the primary IRQ handler is called for the nested IRQ >> registered by the client driver rather than for the parent IRQ from >> the GPIO driver. >> >> This patch makes sure, that the parent_irq (gpio-pca953x) is >> rescheduled rather than the nested irq registered by the matrix_keyp= ad >> driver. >> Similar patches are most probably required for a bunch of other >> drivers too. >> >> Signed-off-by: Lothar Wa=C3=9Fmann >> --- >> drivers/gpio/gpio-pca953x.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x= =2Ec >> index e2da64a..770ef6b 100644 >> --- a/drivers/gpio/gpio-pca953x.c >> +++ b/drivers/gpio/gpio-pca953x.c >> @@ -518,6 +518,7 @@ static int pca953x_irq_setup(struct pca953x_chip= *chip, >> int irq_base) >> { >> struct i2c_client *client =3D chip->client; >> + struct gpio_chip *gpio_chip =3D &chip->gpio_chip; >> int ret, i, offset =3D 0; >> >> if (client->irq && irq_base !=3D -1 >> @@ -567,6 +568,17 @@ static int pca953x_irq_setup(struct pca953x_chi= p *chip, >> "could not connect irqchip to gpioc= hip\n"); >> return ret; >> } >> + >> + for (i =3D 0; i < NBANK(chip); i++) { >> + int j; >> + >> + for (j =3D 0; j < BANK_SZ; j++) { >> + int gpio =3D gpio_chip->base + i * B= ANK_SZ + j; >> + int irq =3D gpio_to_irq(gpio); >> + >> + irq_set_parent(irq, client->irq); >> + } >> + } >=20 > While this is fixing the problem, but isn't the right fix to patch > the function gpiochip_irq_map() in gpiolib.c to call > irq_set_parent() for each IRQ as it gets mapped? >=20 > This driver is using the gpiolib irqchip helpers... >=20 > Then you fix not just this driver but all drivers, plus the complex > loop and calls to gpio_to_irq() etc goes away. The problem here is that: - we don't know parent IRQ number inside gpiolib irqchip; - there can be more then one Parent IRQ per GPIO chip in other words, the knowledge about Parent IRQs is specific to drivers = :( Might be custom .irq_map() callback can be used to fix this issue?=20 Regards, -grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" i= n the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html