From patchwork Tue Mar 4 06:04:04 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Harvey X-Patchwork-Id: 3759081 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 987BA9F1EE for ; Tue, 4 Mar 2014 06:04:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A7B56203EC for ; Tue, 4 Mar 2014 06:04:45 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9E3D9203E5 for ; Tue, 4 Mar 2014 06:04:44 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WKiSv-0001g6-90; Tue, 04 Mar 2014 06:04:33 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WKiSs-0008Vz-P0; Tue, 04 Mar 2014 06:04:30 +0000 Received: from mail-wg0-f49.google.com ([74.125.82.49]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WKiSp-0008VT-R6 for linux-arm-kernel@lists.infradead.org; Tue, 04 Mar 2014 06:04:28 +0000 Received: by mail-wg0-f49.google.com with SMTP id b13so2572885wgh.8 for ; Mon, 03 Mar 2014 22:04:04 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=GW9nIWW9iu9RQfcRe+PMkxYrav8Ct0CqF7vG3m41bxc=; b=CuZZGj6r+0TOXwveGdpR+2XfgFt55ckpj4v4rQ1aX/UFel6VOYldqREcLxBYrfSCqw qrhImq5s+T/op3lhVK0awIOoZiIRjC8W6eIV5C5vG4jwsnrA90thnS5jGbA3SOzejG3x DW5XmabJN2+RXyGe1EnOOwtJYBAB7QRlCZeQJZi5MbKDUp4HySh1j4FeiPIk49pgSZCJ /FW+fsxcGVaRvwpyYd8EzVqa4xhMONxohiiacvDvzrR54jkKhLK+awF29sg4bGtCtTac yQ8rvGLNzqVd0P2huEiBnWepwboSFx0LsnD2ApHpXW2HELvWVELiCtwvdRLaRxeJC53X MOmQ== X-Gm-Message-State: ALoCoQnDqV2QkIfsiIsec+zYi81FPjBLWWolvhLZQvudHCay2FsVfd8sNY/uRWuw8HkadxwSu1FU MIME-Version: 1.0 X-Received: by 10.194.8.196 with SMTP id t4mr15945687wja.49.1393913044630; Mon, 03 Mar 2014 22:04:04 -0800 (PST) Received: by 10.194.139.114 with HTTP; Mon, 3 Mar 2014 22:04:04 -0800 (PST) In-Reply-To: <20140304000102.GC5603@obsidianresearch.com> References: <1393608523-17509-1-git-send-email-l.stach@pengutronix.de> <20140301183059.GA6315@obsidianresearch.com> <000601cf36b8$330b3a90$9921afb0$%han@samsung.com> <20140304000102.GC5603@obsidianresearch.com> Date: Mon, 3 Mar 2014 22:04:04 -0800 Message-ID: Subject: Re: [PATCH 0/7] PCI irq mapping fixes and cleanups From: Tim Harvey To: Jason Gunthorpe X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140304_010427_997460_8957649B X-CRM114-Status: GOOD ( 20.31 ) X-Spam-Score: 0.4 (/) Cc: Mark Rutland , "devicetree@vger.kernel.org" , linux-samsung-soc , linux-tegra , Ben Dooks , Arnd Bergmann , linux-sh , Jingoo Han , Richard Zhu , Simon Horman , Thierry Reding , Sascha Hauer , Stephen Warren , Kukjin Kim , Shawn Guo , Bjorn Helgaas , Grant Likely , "linux-arm-kernel@lists.infradead.org" , Lucas Stach X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,KHOP_BIG_TO_CC, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Mar 3, 2014 at 4:01 PM, Jason Gunthorpe wrote: > On Mon, Mar 03, 2014 at 03:40:43PM -0800, Tim Harvey wrote: > >> of_irq_parse_and_map_pci(). The GIC function that translates the >> interrupt per domain is given irq_data: 0x123 0x04 0x00 > > This has been shifted by 1 byte.. > >> IRQ 123, which should get 32 added to it for irq155). Instead, the >> implementation of gic_irq_domain_xlate() >> (http://lxr.missinglinkelectronics.com/linux/drivers/irqchip/irq-gic.c#L832) >> adds 32 to the 0x04 returning 20: >> [ 1.841781] of_irq_parse_raw: /soc/pcie@0x01000000:00000001 >> [ 1.841813] of_irq_parse_raw: ipar=/soc/pcie@0x01000000, size=1 >> [ 1.841838] -> addrsize=3 >> [ 1.841870] -> match=1 (imaplen=28) > ^^^^^^^^^^^^^ > > That looks odd, it should be the number of dwords in the > interrupt-map, you have 4 lines of 8 dwords each, so it should be > 32. (+cc Grant Likely) imaplen does indeed get initialized to 32 (size of interrupt-map / sizeof(u32)) but its printed above after its been decremented in the loop which is misleading (http://lxr.missinglinkelectronics.com/linux/drivers/of/irq.c#L201) The issue appears to me to be a bug in of_irq_parse_raw() which has been around since Graht's original commit: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/of/irq.c?id=7dc2e1134a22dc242175d5321c0c9e97d16eb87b The issue is that the interrupt-map table point (imap) needs to be incremented over the parent unit interrupt specifier which is newintsize cells, not newaddrsize + newintsize cells. The invalid calculation would cause the pointer to get mis-aligned and thus only the first interrupt entry would ever get properly checked for a match. It looks like of_irq_parse_raw() is only called from of_irq_parse_pci() which prior to Lucas' patch was only called from pci_read_irq_line() called from pcibios_setup_device() used in arch/arm/powerpc, so perhaps this function isn't widely used explaining why the bug was never caught. I'll post a patch shortly with the above fix. >> [ 1.841972] irq_create_of_mapping: calling xlate for 123/4/0 3 > > And it is the wrong data.. 123/4/0 is right - this is shifted because of the issue above. With the above patch Lucas' original patch now operates correctly to resolve the 4 legacy PCI interrupts required when using a P2P bridge on the IMX6. Tim diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 9bcf2cf..8829197 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -237,11 +237,11 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle /* Check for malformed properties */ if (WARN_ON(newaddrsize + newintsize > MAX_PHANDLE_ARGS) goto fail; - if (imaplen < (newaddrsize + newintsize)) + if (imaplen < newintsize) goto fail; - imap += newaddrsize + newintsize; - imaplen -= newaddrsize + newintsize; + imap += newintsize; + imaplen -= newintsize; pr_debug(" -> imaplen=%d\n", imaplen); }