From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936148AbcJUTrX (ORCPT ); Fri, 21 Oct 2016 15:47:23 -0400 Received: from smtp2-g21.free.fr ([212.27.42.2]:10056 "EHLO smtp2-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933903AbcJUTrW (ORCPT ); Fri, 21 Oct 2016 15:47:22 -0400 Subject: Re: Disabling an interrupt in the handler locks the system up To: Marc Zyngier Cc: Thomas Gleixner , Jason Cooper , LKML , Linux ARM , Sebastian Frias References: <580A4460.2090306@free.fr> <580A60ED.3030307@free.fr> <20161021201448.3f4a0a7a@arm.com> From: Mason Message-ID: <580A70B9.8060507@free.fr> Date: Fri, 21 Oct 2016 21:47:05 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0 SeaMonkey/2.40 MIME-Version: 1.0 In-Reply-To: <20161021201448.3f4a0a7a@arm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 21/10/2016 21:14, Marc Zyngier wrote: > Mason wrote: > >> On 21/10/2016 19:46, Marc Zyngier wrote: >> >>> On 21/10/16 17:37, Mason wrote: >>> >>>> On my platform, one HW block pulls the interrupt line high >>>> as long as it remains idle, and low when it is busy. >>>> >>>> The device tree node is: >>>> >>>> test@22222 { >>>> compatible = "vendor,testme"; >>>> interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; >>>> }; >>> >>> I assume that this is for the sake of the discussion, and that you do >>> not actually intend to put together such a monstrosity. >> >> It's just missing a reg properties to be a valid node, right? > > If connecting a device that signals its interrupt as level low to an > input line configured as level high doesn't strike you as a major > issue, nothing will. At that point, you can put anything you want in > your DT. If I understand correctly, you are saying that I should have specified IRQ_TYPE_LEVEL_LOW, instead of IRQ_TYPE_LEVEL_HIGH? If the HW outputs 1 when idle, and 0 when busy, that is level low? (Sorry if this is obvious, I'm absolutely clueless in this subject matter.) >>>> I wrote a minimal driver which registers the irq. >>>> And in the interrupt handler, I disable said irq. >>>> >>>> Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as >>>> it is registered (because the block is idle). >>>> >>>> Here is the code I've been running, request_irq doesn't return. >>> >>> [...] >>> >>>> And here's what I get when I try to load the module: >>>> (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) >>> >>> [...] >>> >>>> Are we not supposed to disable the irq in the handler? >>> >>> You can. It then depends on what your interrupt controller does to >>> actually ensure that the interrupt is disabled. Only you can trace it on >>> your HW to find out. >> >> I'm using an upstream driver on v4.9-rc1 >> >> http://lxr.free-electrons.com/source/drivers/irqchip/irq-tango.c >> >> Given that the system locks up, is it possible there is a bug >> in the driver? > > That's possible. > >> Which call-back handles enabling/disabling interrupts? > > How about irq_unmask/irq_mask? I tried following the source from disable_irq_nosync() as far down as I could. disable_irq_nosync -> __disable_irq_nosync -> __disable_irq -> irq_disable -> ?? http://lxr.free-electrons.com/source/kernel/irq/chip.c#L232 I don't know if desc->irq_data.chip->irq_disable is defined by the driver I'm using? I don't know how the trail goes to irq_mask? Regards. From mboxrd@z Thu Jan 1 00:00:00 1970 From: slash.tmp@free.fr (Mason) Date: Fri, 21 Oct 2016 21:47:05 +0200 Subject: Disabling an interrupt in the handler locks the system up In-Reply-To: <20161021201448.3f4a0a7a@arm.com> References: <580A4460.2090306@free.fr> <580A60ED.3030307@free.fr> <20161021201448.3f4a0a7a@arm.com> Message-ID: <580A70B9.8060507@free.fr> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 21/10/2016 21:14, Marc Zyngier wrote: > Mason wrote: > >> On 21/10/2016 19:46, Marc Zyngier wrote: >> >>> On 21/10/16 17:37, Mason wrote: >>> >>>> On my platform, one HW block pulls the interrupt line high >>>> as long as it remains idle, and low when it is busy. >>>> >>>> The device tree node is: >>>> >>>> test at 22222 { >>>> compatible = "vendor,testme"; >>>> interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; >>>> }; >>> >>> I assume that this is for the sake of the discussion, and that you do >>> not actually intend to put together such a monstrosity. >> >> It's just missing a reg properties to be a valid node, right? > > If connecting a device that signals its interrupt as level low to an > input line configured as level high doesn't strike you as a major > issue, nothing will. At that point, you can put anything you want in > your DT. If I understand correctly, you are saying that I should have specified IRQ_TYPE_LEVEL_LOW, instead of IRQ_TYPE_LEVEL_HIGH? If the HW outputs 1 when idle, and 0 when busy, that is level low? (Sorry if this is obvious, I'm absolutely clueless in this subject matter.) >>>> I wrote a minimal driver which registers the irq. >>>> And in the interrupt handler, I disable said irq. >>>> >>>> Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as >>>> it is registered (because the block is idle). >>>> >>>> Here is the code I've been running, request_irq doesn't return. >>> >>> [...] >>> >>>> And here's what I get when I try to load the module: >>>> (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) >>> >>> [...] >>> >>>> Are we not supposed to disable the irq in the handler? >>> >>> You can. It then depends on what your interrupt controller does to >>> actually ensure that the interrupt is disabled. Only you can trace it on >>> your HW to find out. >> >> I'm using an upstream driver on v4.9-rc1 >> >> http://lxr.free-electrons.com/source/drivers/irqchip/irq-tango.c >> >> Given that the system locks up, is it possible there is a bug >> in the driver? > > That's possible. > >> Which call-back handles enabling/disabling interrupts? > > How about irq_unmask/irq_mask? I tried following the source from disable_irq_nosync() as far down as I could. disable_irq_nosync -> __disable_irq_nosync -> __disable_irq -> irq_disable -> ?? http://lxr.free-electrons.com/source/kernel/irq/chip.c#L232 I don't know if desc->irq_data.chip->irq_disable is defined by the driver I'm using? I don't know how the trail goes to irq_mask? Regards.