From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966492AbcA1MWU (ORCPT ); Thu, 28 Jan 2016 07:22:20 -0500 Received: from foss.arm.com ([217.140.101.70]:59977 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964822AbcA1MWR (ORCPT ); Thu, 28 Jan 2016 07:22:17 -0500 Date: Thu, 28 Jan 2016 12:22:13 +0000 From: Brian Starkey To: Thomas Gleixner Cc: LKML , Marc Zyngier , Tomasz Figa , Rob Herring , Frank Rowand , Grant Likely , devicetree@vger.kernel.org Subject: Re: [PATCH] genirq: fix trigger flags check for shared irqs Message-ID: <20160128122212.GC12884@e106950-lin.cambridge.arm.com> References: <96c068ece0f5d01fc85bdbd9e0798f68180eebb8.1453204950.git.brian.starkey@arm.com> <20160128095955.GA12884@e106950-lin.cambridge.arm.com> <20160128111041.GB12884@e106950-lin.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 28, 2016 at 12:49:37PM +0100, Thomas Gleixner wrote: >On Thu, 28 Jan 2016, Brian Starkey wrote: >> I've got a few devices on the same interrupt line. One driver does > >Just for the record: When will hardware folks finally understand that shared >interrupt lines are a nightmare? > In general, agreed. In this case though I think they get some grace - my devices are all in an FPGA which only has one interrupt line to the SoC. >> something along these lines: >> >> res = platform_get_resource(dev, IORESOURCE_IRQ, 0); >> flags = (res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED; >> request_irq(res->start, handler, flags, "name", dev); >> >> This seems pretty reasonable. The problem is since 4a43d686fe33: >> of/irq: Pass trigger type in IRQ resource flags[1] >> the trigger type information from device-tree is in res->flags. >> >> So when the other drivers don't pass in any flags, they fail the check >> in __setup_irq(). >> >> Changing the former driver to remove the flags doesn't seem right, and >> adding flags to the latter would imply adding flags to _every_ driver, >> which is an awful lot to change - and I'm not sure it would be possible >> and/or effective in all cases. > >So that commit does: > > r->flags = IORESOURCE_IRQ | irqd_get_trigger_type(irq_get_irq_data(irq)); > >which reads the current setting of the interrupt line. > >Now we pass exactly that to request_irq(). So first irq_of_parse_and_map() >configures the interrupt type when mapping it and then hands in the same type >information when requesting the irq. Right, there's some redundancy here. > >I have no idea what the purpose of this is and the changelog of that commit is >completely useless, sigh! > >I've cc'ed the author and the device tree folks. Perhaps are they able to >explain what this commit tries to 'fix'. This 'fix' is what makes me hit the problem - but even without it I think the problem still exists. It seems like in principle two drivers ought to be able to do request_irq(irq, handler, IRQF_SHARED | IRQF_TRIGGER_HIGH, ...); and request_irq(irq, handler, IRQF_SHARED, ...); without the latter call failing. Or do you disagree? -Brian > >Thanks, > > tglx >