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: 3759061 Return-Path: X-Original-To: patchwork-linux-sh@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 7E0969F1EE for ; Tue, 4 Mar 2014 06:04:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7EA57203EC for ; Tue, 4 Mar 2014 06:04:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4D773203E5 for ; Tue, 4 Mar 2014 06:04:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751783AbaCDGEI (ORCPT ); Tue, 4 Mar 2014 01:04:08 -0500 Received: from mail-we0-f181.google.com ([74.125.82.181]:64969 "EHLO mail-we0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751518AbaCDGEG (ORCPT ); Tue, 4 Mar 2014 01:04:06 -0500 Received: by mail-we0-f181.google.com with SMTP id q58so4364144wes.12 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=VqDNRG1d0i3YOX53B1qV0PCOtenF+MWeKsxPAgow+xXKzBil/09hAng8PJh0kUiD6e kE5ocGo8tcyiSig+SvQoNdfn2eLPVhn/bMqwbuKs3wTGxVgW7as62QOW0gpnTq1OScE8 CuP69p+Z8rOyeKtKSN5eqtcLwFnpU3KyLQ8EbAasa7Dx8WLlsRYtoU7FV81m2bQev0Ub bY3aT+TJlFZge+ak2kUYNT3PfvWQyA/YqXFV0p0kHa4gM2EPEpO/CXjdX9jDGnUEdBe+ 9KmOHrX63iNI00QYkgrhI+FIvPXilmVyOGkULQTW7xeGsY6xWsnHoeVcAEIHs8mCYC1X RHig== X-Gm-Message-State: ALoCoQn/FSODHvQ1cYBqSh+JpxeNnll77zJMjCtKYOLb9hsE66x3vooefe2C4XcrYtUBb4nE3Abl 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 Cc: Jingoo Han , Lucas Stach , Mark Rutland , "devicetree@vger.kernel.org" , linux-samsung-soc , Richard Zhu , Sascha Hauer , Arnd Bergmann , linux-sh , Stephen Warren , Bjorn Helgaas , Simon Horman , Thierry Reding , Ben Dooks , linux-tegra , Kukjin Kim , Shawn Guo , "linux-arm-kernel@lists.infradead.org" , Grant Likely Sender: linux-sh-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sh@vger.kernel.org X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00,KHOP_BIG_TO_CC, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham 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 --- To unsubscribe from this list: send the line "unsubscribe linux-sh" 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/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); }