From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754597AbdCBIqf (ORCPT ); Thu, 2 Mar 2017 03:46:35 -0500 Received: from mail-bl2nam02on0084.outbound.protection.outlook.com ([104.47.38.84]:20522 "EHLO NAM02-BL2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754338AbdCBIq2 (ORCPT ); Thu, 2 Mar 2017 03:46:28 -0500 Authentication-Results: spf=pass (sender IP is 149.199.60.100) smtp.mailfrom=xilinx.com; vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=bestguesspass action=none header.from=xilinx.com; From: Bharat Kumar Gogada To: Bjorn Helgaas , Marc Zyngier CC: "robh@kernel.org" , "bhelgaas@google.com" , "colin.king@canonical.com" , "arnd@arndb.de" , "linux-arm-kernel@lists.infradead.org" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts. Thread-Topic: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts. Thread-Index: AQHSAqrdY8Zj+O9d0E2xRiMSs6E3q6Bg5aOAgACfgzD//45TAIABrX2A//+gVoCAAbOx0IAR4lSAgAChtgCAAHv5gIELR/7w Date: Thu, 2 Mar 2017 08:46:18 +0000 Message-ID: <8520D5D51A55D047800579B094147198263F7207@XAP-PVEXMBX02.xlnx.xilinx.com> References: <1472553558-27215-1-git-send-email-bharatku@xilinx.com> <1472553558-27215-3-git-send-email-bharatku@xilinx.com> <57C57975.7040306@arm.com> <8520D5D51A55D047800579B094147198258D239D@XAP-PVEXMBX01.xlnx.xilinx.com> <57C59FE8.30307@arm.com> <8520D5D51A55D047800579B094147198258D28AF@XAP-PVEXMBX01.xlnx.xilinx.com> <57C6B7F1.5000001@arm.com> <8520D5D51A55D047800579B094147198258D2C99@XAP-PVEXMBX01.xlnx.xilinx.com> <20160912220241.GG23532@localhost> <57D7ADA8.5060201@arm.com> <20160913150511.GC27748@localhost> In-Reply-To: <20160913150511.GC27748@localhost> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.23.94.248] Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-RCIS-Action: ALLOW X-TM-AS-Product-Ver: IMSS-7.1.0.1224-8.1.0.1062-22916.005 X-TM-AS-User-Approved-Sender: Yes;Yes X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-HT: Tenant X-Forefront-Antispam-Report: CIP:149.199.60.100;IPV:NLI;CTRY:US;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(7916002)(39850400002)(39450400003)(39410400002)(2980300002)(438002)(377454003)(55674003)(189002)(24454002)(199003)(13464003)(9170700003)(2920100001)(229853002)(38730400002)(2900100001)(6246003)(63266004)(7696004)(23726003)(53546006)(3846002)(102836003)(305945005)(54356999)(106116001)(6116002)(2950100002)(7736002)(92566002)(4326008)(47776003)(55016002)(2906002)(93886004)(8746002)(5250100002)(33656002)(76176999)(46406003)(50986999)(8676002)(106466001)(5660300001)(55846006)(97756001)(356003)(50466002)(189998001)(81166006)(8936002)(626004)(107986001);DIR:OUT;SFP:1101;SCL:1;SRVR:BN1PR0201MB0707;H:xsj-pvapsmtpgw02;FPR:;SPF:Pass;MLV:sfv;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;CY1NAM02FT025;1:DJ53SYjiIFAZcNTCei5zoJbzL41hihIfcXgdKb56LFu5T7SBnKlTDhQ/RaD5XUE3j9ifZnT0eKZY8RbnkQQxa7dKD/AsatDuYM8kgMpo6jxony3NDYie2/u94VmZjdRK8jnnuoJX6tyRkaF33UDpaKf557m7nygI5Xs6+PGWWe5/SWz933J9wuMKy1rPmuUk2cbVy8qMF5phm8wtx25zTf274oBk9KQwPffEydoRl1p6ziTjvYDgCfybgrT75xaOGdxjrlpzI3A4ylQJjw7LwnOr3HRLre2SYj5H2/aBQTwuVr7GyJgUoF/PbD5BlyHz48fRtT8hxHLQ4swdb+IOFKgrgs61GrWmaNs8Kl2VHLzNDIwwnmYbOk/mA3XC7kbeYV4zD2xsr6Rpfx0CtlszUNljihaXgovGMw0Ejc3PQHaEW9cMjyGAWqbkwK5l7g+qv71KG8ljW03WhamrB9IcoqsVP2XwdEMdy1edyqICd0ZIZkLkgOo3iTzAGNTGkzGi2cQdB1oGYN+EdPfDUVdll5rsGWWM++RBlNu2E4LhGlRVWJgvpRvrcSRxU9FyiYFipRU7u3NhJUMPOQeCxStqqVXRAomk8psZqYmodh3gnwU= X-MS-Office365-Filtering-Correlation-Id: 47d811e8-1ab8-474d-7450-08d461489bd5 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(8251501002);SRVR:BN1PR0201MB0707; X-Microsoft-Exchange-Diagnostics: 1;BN1PR0201MB0707;3:Rgoqym0SW4e4UTlTUYTgmr1X9sfmALG2EtzotHKBE+a42ZpGO3RfxLppxL+UXjHHh1fpHg9yfid4sK6hwolKjODI3zKNjG2rUEOc6HIxbr54Qz1MGj13DsvmY1usVo0wNZrsUB0JbS+/9lnJ4AvYCRYEhD+ZcyW9+o5gYB9k+D3Coak5EV8GcSMZ91SfDLSxS8IGI/7i5q1luZ0z7L8qwgovrJkPqhVdafOsk6+cMOOqtJg+fL7L+g+13BJAhm++2Bj4t6mI68DjenRfhHGfsRS/vsz0bQB3BFRbg/IBwx7A9pjq6gEXOnwIgRFqpeWwqrWwrt49xB8z5gE5wfi90vh72zI1l2Ekjgxf9bMW6yHmfsQE3g+2xKLmF4886w0xj/ZwEFVfiRF38u11hmYd2g==;25:/fQZgualpH56REdSyQswYItbuSJugdnJeL4W21PVXaExuuZnp9NccntqEUhmtkZuxADNvD/rsuMaHhxBWdICxousklnR4bELmvLOzO7AjGvI8fOgAzeUSsuYd5Jj745nFtOZnU/Dd20UiLMJP8Bxcw9zl0VBS7DD3DXamQd82uYoVDo2UXMzQmBC0fHvacdsPeYLIbeChUO5TItXDnfV+5ZhVDZuHYfSP5yfSBCOiRsSH5vlbEQO3za2q7ICPQsw8lsjWliTc2yWiwWUZG/j4atqOP+7RW7buxvlcAlunTJNnO8nFybPXer8X0ZbS9p0zvXcENEjFjYwlLJcsSu26c62LkCg9qpgfamLfT57Q3BJRx6mLI3n3aGQ3HQmvpn8QoM4LogxeBB3BzKXIHddnTpAzGqNcV/vAyp32gKI2JVC16eG3qckYZL+WLRiCSl1B1tYnTYZ2PVZGe78d9efLQ== X-Microsoft-Exchange-Diagnostics: 1;BN1PR0201MB0707;31:yVr1+7xj7CnCCOC7z9go3MEhWRs+raU3ptqbH5MSTRSC8lrakrKr1Wh2G5vhxe0pB6SDzw63mETwWw8MdeBxI4pmxIEW/YW75xjndiM6fB3ThjR/9e4xdkY09EdXwOdCyDTzEvnSsy4vxVMTM5+MIj1ZZ01Lr4kv+hqzvVjCFHHoYy+0T5pjrKhqreAyn9JnyuWPc2xqhuurYRt+pPjJ6aSjhysQhP8Du0yo8FLtHSSQmHavw0K/whsNPBTSQrPCf2hrH7y493wej9VeZm91Fw==;20:4nDeLMzMF7lb7+bFPQp2saNqm9XYsRMAudswK6PdQHv0aLAB4dRVO4m53NclcquhSBAxQW+1Nbg8JPsrLoPvY9cD6S/hLsJQApd3iN57h5WvIIWaCZsS5/Bw9a+7ZINKRg6IG+p4+1LRCqm7yU7T+g9UzYwHuEmB0Lv5zhg7FO9HBaOcX7+cSJPfBJ1y5S+w11EUabdpUd2lbSfEmS1prp7/nfSaqUJYTiAHo0L79oca5DG9dOprEfuft7Dks8UUP34kX1kVJdDCrah6u2mW3GvjhlOmPOsmC9GMfTLWbre+CSlXtztkjk++0+iEYs6ZWNVrKESknRsrnd8nhzv1ZcErudCJY/bANeR4MJL5alXwAcQ+Q2A6fdYOwcA9PqTbw5tf1qTMH/Ghi9fQuv4UppVjehniWm3cs4NddqTJP5C2DzuaKWLe3DzCG8N9SQ2cJLKQbPNFsmSvhhyCSt9OFmzjmbT4Bs140WXSdi8gbEWTh5H4YIveuZ0HNiL0yZEv X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(180628864354917)(9452136761055)(258649278758335)(192813158149592)(211936372134217)(198206253151910); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(8121501046)(5005006)(13015025)(13017025)(13023025)(13024025)(13018025)(10201501046)(3002001)(6055026)(6041248)(20161123560025)(20161123555025)(20161123564025)(20161123558025)(20161123562025)(6072148);SRVR:BN1PR0201MB0707;BCL:0;PCL:0;RULEID:;SRVR:BN1PR0201MB0707; X-Microsoft-Exchange-Diagnostics: 1;BN1PR0201MB0707;4:beD/OTVfnisTZ9xXtnohijf22c9p+XBNOrvnulKnbXAHFIItHFSesjaRWGZnpNJtt512JXJm0pW4dW5xnWA9MGUfQ6G1Z+Z3a+VvfW2mmr2ekkXF0C01PypPfhBXpLpZqb25wJjjTIT8mfbMh7iN3D2Vn0xlsRnoZhLDm8g+c7dFW4ioGusN4SzVaFCAY/enrW4060qBjdAk9AZnVnN8DyU6WsdYHD2m8Nf20VMICXOWOT3nJDX/DxDZfNP5iw/GERuOb1hukItXTez5KfIDx9Z0/Agy6LKIAYO6S5D5ZgRtT6PDYsnXdSqg8a3yfP8bdC3jT/Qt1xOteIU38m91UzyRjhs1aQ6RCf2cuI0hnpxtfJKTs1zStGxHGe+5ZB9fw4yc7COIHQR2BF+TNnexnsXE4bw6eTfgpG+6lISzfm3wMCH8+XtJyoxwDkTj5SneNFW+u1OiQgI/DD0VajsVWWwT8E9p30wBXpaObScaZr53wEfOREjJ0jcUmgTZpKoSR2MVB1G2ydv7eHz0ua7UjAcO9BQ7PoxlWWJBmo7hOYd+iR/pCYQ1vhODC5OZYm9JfA/ai73b5emxt4vS6lLubd0byUtLyzSsyHufXOk1+abCOiztOBiavrtyz2dTQBd4MpO9Q9hXUTS9uHzUHVa5nM5KsZe/yI+QjN29Za6cxex6hAj4ETE65d6zjDREDbWdkFfOSosNQK81qIVKRMfWIQK9SgvC+GA6bofuXO2HTubtcbe+9cS7piF0RTWWDKQEhc54ny24Xtxjvw3ExdEsQmxI+XsKI6DsIaqGK0zlq9HAUU4k5emUMJ8tv689V8hoDu2W8NH8po7TEQucVwR3D1XMaxrzKqmxPNVtiKhRdn5Oiz0Cyf/8W+zMAPR69N33oTWdlxi5V42DGEKrGTl+g+coyvlKO5rIu2xUTx1I2ePGwY9fXBSxkEa3CymkvBvU X-Forefront-PRVS: 023495660C X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;BN1PR0201MB0707;23:vG2CZ8KcJ4PBp1x7UPYBb6EX/t9WaJL5vYb15zZ?= =?us-ascii?Q?Rt7TidhwQ3w9Sf1alehLHvBFdnGctXkGXkbWaXXE/5n2ESe7eSwgZ7jGaBMQ?= =?us-ascii?Q?hOuPna4aIh9EAByk/1FTXctxCcWGeHZXv1dRKzlKNj/0MnP9qqa15hPjOMgd?= =?us-ascii?Q?QWDcf1oNCJ0guuhN5uXkzgR96SfhK1tyiIG2cqx9DsIs5CfuSgXw10wbaSWJ?= =?us-ascii?Q?b0/GSvHUo3TvvWYx7MmKGQD43PvSG7L4QHZp9/1rRRu1pHwnRV+8HPNbmLWa?= =?us-ascii?Q?bWY8uLlbtMG5fdJyY8jHBcX12C1JzzeJtknhBfAmU4U5qdmKY7Ffjtq2duZP?= =?us-ascii?Q?+lgkgdEOMwik2mxcME7AiWXvLTrmmxg3pDablzMmZ7DVFunRLshogDZntN9p?= =?us-ascii?Q?5/1S0n2H0iE94UL+JXYTU2GQfr6IN24e2aeAr8+CMNtnLh6UB4XKfTdr9DaI?= =?us-ascii?Q?/CnqHZhOiKHAB3oAapF8qbiXkbKS8wC7UAEhp8KrVMy+T4cqCJMQ3OgKH8OM?= =?us-ascii?Q?0a55XWr27QaPdgKRu0/o1iDmDOX8bEUqPNQfTOk+mLcy4eblG/D7Br6m4Qpp?= =?us-ascii?Q?cmaOXPnGPUOa/T6dZjEhbDELsB0Mw4fa0qnq7pcQpTd7c87sWFJEWFEnkLF7?= =?us-ascii?Q?mvtRTj8qvUmfnFpXchsmNwdbNg4YMu1Kv4hz249C/9jbLUWzO/cA1KJ8JuFE?= =?us-ascii?Q?Y7wSpS49b/M0TDZXEUTN5xiCaCNPEELXV0DLSkk78qQJN9EI5+T6qnGJUEQT?= =?us-ascii?Q?od2e/zVj79BxohD+EyFLJBXWWeej+QEd7VV2tFfe1g5z91wyl+T8MyG3QLqo?= =?us-ascii?Q?Y6ydEGOMi+jnClIsH1dEtJPQOzKbv2Z4McDyqH77eYWWxdT0cJAIu8zC/Hom?= =?us-ascii?Q?Hk2DChFxquNMlIfxNCzb6Kz44r12Fm/6GbmSiQYkVEi7mm7H59hKheiEjFLX?= =?us-ascii?Q?kLNQ5ym+n0E4Z+WLaZP0gbcWIhrBjjmb0rQdvbm/u2Slzia00pJ2G6AX4XyJ?= =?us-ascii?Q?huqSzaUfc6drzwmjdSaPSeNrw3h3A2qmMO6nhYnRDzI010ptdHPB5x+Mrx63?= =?us-ascii?Q?HYLc7jtDL4ngQ79zX9F37Q2QNZhI/+AiEaXdUGYEp+t/qCm/rDGItxU+12/g?= =?us-ascii?Q?i35M5pxwaHzP21sKPauKCosO0R7mrnXcHMTzfqedfV7TgsC+J7bfPNMrPIVU?= =?us-ascii?Q?bRO266a0CwGzFk4TtoNsYzCCZyZJM0rD++wxzIQGD9Cr9YzTxqSuFFm6UxFs?= =?us-ascii?Q?Dnq2cUIsAYzHg4Zk9z71LbjmwvuLYSTutcxe6LXWYByZdbC0BYnoKsgmPm8Z?= =?us-ascii?Q?cPFx2oEqjA5VPIOgE0AthRtk=3D?= X-Microsoft-Exchange-Diagnostics: 1;BN1PR0201MB0707;6:o+fkO7cwpHN0PWrr5OROyAoARBIR77GO8ES8+h9IxHVzfgOI4R1UOxMnuWGv/pH3lPhekPde3wDYB9LZIIPJfZ8QBElYeat6AJ/ULdScOClpYCYxEBfE41aNRNo9YV3pMUckZUPeFRgPQ1HbzOq2CJyBTAKPCirLnbnBkgOW/Gs0jAMUW5Plt8GVgaG0BRRo9QBC9s7H3CnROLUBizgQN0Q6jBpL5iefYIulWhu49ozZhixjKt690GW2nYCfpu+nHTrw4OBy5E1w4i2CsWeJ8TcHjKEs+C0CuqWSWtrge1wMedG7FPDVWRduVQINBMyDQNxr4iwcbS7oDpPxFFn2c/t/3o/ynwx6S2ifxVhzr7po7SmFV6BIBhKb727RNcyFHtus256Cepo+tcqUQNtoIvSEn9wphctaK0dTayVWFwQ=;5:fX2ZStqNTJitcYiCBNLpZpSICUWEb44n2LgyVBZ3MapZfU672CPd9VC+IRheE7R3iE5VQQ404zyF+LuLRAwgOh5Gs5xtlPQumqX+N0OB9Dr6K8ji4f491kUSQvxpLDLP78AiAhFhlmBitk3V+TTo9Q==;24:uUbhMihUF8IIb8SprIb7rG5jn72yTEJ/8JQqcunTf8YrcWlctQyuCgNUiaApn1Sc7t+jnnNGUHLDqV0Ty0XrpGH2ComwZQIJgctpR55+c7Y= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;BN1PR0201MB0707;7:bjWhf3ec4l1T0PC0CFrXGoQ+Tz75iK5/LZCkeaMc7dwhfqWAYbVv7eLeC78pAO3XkOPt91obiiQkFvedps/WHKB+bMLOrNZKerbn0WdPdSvvOn/+l2uU8JnIw+6ilNxn/OKNGOAaxxHz4Jc/+sIfwDK8jRm+YH5S1M/JBx1T8wtEOcHKAEdLLkua3gBLZSsorDR+0ffF+BQJItVbyNM9Rx5yE9FlgkZmsD1Fb0mGP+Ew7aIojWdwivDkkS971lteM5fZIMIRIWo0v7Zj0hnKfBtTYmV6g3MeeDzFVhi8nf0vVIrdRtyCQtaZznyzACm0Ibjf6gw00Z/kr1Y8pokrKQ== X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Mar 2017 08:46:24.0141 (UTC) X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.60.100];Helo=[xsj-pvapsmtpgw02] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN1PR0201MB0707 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v228kfJL029552 Hi, Any one is working on fix for this issue ? Regards, Bharat > -----Original Message----- > From: Bjorn Helgaas [mailto:helgaas@kernel.org] > Sent: Tuesday, September 13, 2016 8:35 PM > To: Marc Zyngier > Cc: Bharat Kumar Gogada ; robh@kernel.org; > bhelgaas@google.com; colin.king@canonical.com; Soren Brinkmann > ; Michal Simek ; arnd@arndb.de; > linux-arm-kernel@lists.infradead.org; linux-pci@vger.kernel.org; linux- > kernel@vger.kernel.org; Ravikiran Gummaluri > Subject: Re: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device > for legacy interrupts. > > On Tue, Sep 13, 2016 at 08:41:28AM +0100, Marc Zyngier wrote: > > On 12/09/16 23:02, Bjorn Helgaas wrote: > > > On Thu, Sep 01, 2016 at 05:19:55AM +0000, Bharat Kumar Gogada wrote: > > >>>>>>> Hi Bharat, > > >>>>>>>> @@ -561,7 +561,7 @@ static int > > >>>>>>>> nwl_pcie_init_irq_domain(struct nwl_pcie > > >>>>>>> *pcie) > > >>>>>>>> } > > >>>>>>>> > > >>>>>>>> pcie->legacy_irq_domain = > irq_domain_add_linear(legacy_intc_node, > > >>>>>>>> - INTX_NUM, > > >>>>>>>> + INTX_NUM > > >>>>>>>> + + 1, > > >>>>>>>> &legacy_domain_ops, > > >>>>>>>> pcie); > > >>>>>>> > > >>>>>>> This feels like the wrong thing to do. You have INTX_NUM irqs, > > >>>>>>> so the domain allocation should reflect this. On the other > > >>>>>>> hand, the way the driver currently deals with mappings is > > >>>>>>> quite broken (consistently adding 1 to > > >>>>> the HW interrupt). > > >>>>>>> > > >>>>>> Hi Marc, > > >>>>>> > > >>>>>> Without above change I get following crash in kernel while booting. > > >>>>>> > > >>>>>> [ 2.441684] error: hwirq 0x4 is too large for dummy > > >>>>>> > > >>>>>> [ 2.441694] ------------[ cut here ]------------ > > >>>>>> > > >>>>>> [ 2.441698] WARNING: at kernel/irq/irqdomain.c:344 > > >>>>>> > > >>>>>> [ 2.441702] Modules linked in: > > >>>>>> > > >>>>>> [ 2.441706] > > >>>>>> > > >>>>>> [ 2.441714] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.4.0 #8 > > >>>>>> > > >>>>>> [ 2.441718] Hardware name: xlnx,zynqmp (DT) > > >>>>>> > > >>>>>> [ 2.441723] task: ffffffc071886b80 ti: ffffffc071888000 task.ti: > > >>>>> ffffffc071888000 > > >>>>>> > > >>>>>> [ 2.441732] PC is at irq_domain_associate+0x138/0x1c0 > > >>>>>> > > >>>>>> [ 2.441738] LR is at irq_domain_associate+0x138/0x1c0 > > >>>>>> > > >>>>>> In kernel/irq/irqdomain.c function irq_domain_associate > > >>>>>> > > >>>>>> if (WARN(hwirq >= domain->hwirq_max, > > >>>>>> "error: hwirq 0x%x is too large for %s\n", > > >>>>>> (int)hwirq, domain- > > >>>> name)) > > >>>>>> return -EINVAL; > > >>>>>> > > >>>>>> Here the hwirq and hwirq_max are equal to 4 without the above > > >>>>>> condition > > >>>>> (INTX_NUM + 1) due to which crash is coming. > > >>>>>> This is happening as the legacy interrupts are starting from 1 (INTA). > > >>>>> > > >>>>> I understood that. I'm still persisting in saying that you have the wrong > fix. > > >>>>> > > >>>>> Your domain should always allocate many interrupts as you have > > >>>>> interrupt sources. These interrupts (hwirq) should be numbered > > >>>>> from 0 to (n- > > >>> 1). > > >>>> > > >>>> Agreed, but here comes the problem the hwirq for legacy > > >>>> interrupts will start at 0x1 to 0x4 (INTA to INTD) and these > > >>>> values are as per PCIe specification for legacy interrupts. So > > >>>> these cannot be numbered from 0. So when 0x4 (INTD) for a > > >>>> multi-function device comes the crash occurs. > > >>> > > >>> So who provides this hwirq? Who calls irq_domain_associate() with > > >>> hwirq set to 4? > > >>> > > >> PCIe subsystem invokes pcibios_add_device function in > arch/arm64/kernel/pci.c for every pci device. > > >> The purpose of this function is to assign dev->irq using > of_irq_parse_and_map_pci. > > >> of_irq_parse_and_map_pci invokes of_irq_parse_pci where it reads > > >> PCI_INTERRUPT_PIN from configuration space and saves it in parameter of > struct of_phandle_args. > > >> This structure is passed to irq_create_of_mapping where it invokes > irq_create_fwspec_mapping. > > >> irq_create_fwspec_mapping invokes irq_domain_translate and gets > > >> hwirq, here the above saved PCI_INTERRUPT_PIN value is assigned to hwirq > (*hwirq = fwspec->param[0]). > > >> And then using this hwirq irq_create_mapping -> irq_domain_associate > were invoked and mapping is created for virtual irq with this hwirq. > > >> So for any end point PCI_INTERRUPT_PIN value starts from 0x1 to 0x4 and > so hwirq starts from 0x1 to 0x4. > > >> > > >> So the values are more generic w.r.t to protocol, that's why hwirq will > range from 0x1 to 0x4. > > >> And then if you check pcie-altera.c they are doing this adding one in their > handler and while creating legacy domain. > > > > > > Is this resolved yet? Marc, are you happy, or should we iterate on > > > this again? > > > > Ah, sorry to have dropped the ball on this patch. > > No problem, I wasn't making forward progress anyway. > > > I guess that given that the infrastructure imposes the hwirq range on > > the host drivers, Bharat's approach is the only way (and a number of > > other host drivers are already slightly broken). I'll try and have a > > look at solving this at the generic level. In the meantime: > > > > Acked-by: Marc Zyngier > > After looking at this myself, I'm not happy with this either. It feels like there are > bugs lurking here and we're just hiding one of them. > > Here are the callers of irq_domain_add_linear() for legacy INTx in > drivers/pci/host: > > advk_pcie_init_irq_domain LEGACY_IRQ_NUM (4) > dra7xx_pcie_init_irq_domain 4 > ks_dw_pcie_host_init MAX_LEGACY_IRQS (4) > altera_pcie_init_irq_domain INTX_NUM + 1 (5) > nwl_pcie_init_irq_domain INTX_NUM + 1 (5) > xilinx_pcie_init_irq_domain 4 > > I think all of these use the of_irq_parse_and_map_pci() path you mentioned, so > if the problem is in the way that path works, I would think these should *all* be > requesting the same number of interrupts in the domain. > > I agree with Marc that we should request 4 IRQs, because that's what we need. > If we can't do that for some reason, we ought to at least make all these callers > the same. > > Bjorn From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Bharat Kumar Gogada To: Bjorn Helgaas , Marc Zyngier CC: "robh@kernel.org" , "bhelgaas@google.com" , "colin.king@canonical.com" , "arnd@arndb.de" , "linux-arm-kernel@lists.infradead.org" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts. Date: Thu, 2 Mar 2017 08:46:18 +0000 Message-ID: <8520D5D51A55D047800579B094147198263F7207@XAP-PVEXMBX02.xlnx.xilinx.com> References: <1472553558-27215-1-git-send-email-bharatku@xilinx.com> <1472553558-27215-3-git-send-email-bharatku@xilinx.com> <57C57975.7040306@arm.com> <8520D5D51A55D047800579B094147198258D239D@XAP-PVEXMBX01.xlnx.xilinx.com> <57C59FE8.30307@arm.com> <8520D5D51A55D047800579B094147198258D28AF@XAP-PVEXMBX01.xlnx.xilinx.com> <57C6B7F1.5000001@arm.com> <8520D5D51A55D047800579B094147198258D2C99@XAP-PVEXMBX01.xlnx.xilinx.com> <20160912220241.GG23532@localhost> <57D7ADA8.5060201@arm.com> <20160913150511.GC27748@localhost> In-Reply-To: <20160913150511.GC27748@localhost> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: Hi, Any one is working on fix for this issue ?=20 Regards, Bharat > -----Original Message----- > From: Bjorn Helgaas [mailto:helgaas@kernel.org] > Sent: Tuesday, September 13, 2016 8:35 PM > To: Marc Zyngier > Cc: Bharat Kumar Gogada ; robh@kernel.org; > bhelgaas@google.com; colin.king@canonical.com; Soren Brinkmann > ; Michal Simek ; arnd@arndb.de; > linux-arm-kernel@lists.infradead.org; linux-pci@vger.kernel.org; linux- > kernel@vger.kernel.org; Ravikiran Gummaluri > Subject: Re: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi functi= on device > for legacy interrupts. >=20 > On Tue, Sep 13, 2016 at 08:41:28AM +0100, Marc Zyngier wrote: > > On 12/09/16 23:02, Bjorn Helgaas wrote: > > > On Thu, Sep 01, 2016 at 05:19:55AM +0000, Bharat Kumar Gogada wrote: > > >>>>>>> Hi Bharat, > > >>>>>>>> @@ -561,7 +561,7 @@ static int > > >>>>>>>> nwl_pcie_init_irq_domain(struct nwl_pcie > > >>>>>>> *pcie) > > >>>>>>>> } > > >>>>>>>> > > >>>>>>>> pcie->legacy_irq_domain =3D > irq_domain_add_linear(legacy_intc_node, > > >>>>>>>> - INTX_NUM, > > >>>>>>>> + INTX_NUM > > >>>>>>>> + + 1, > > >>>>>>>> &legacy_do= main_ops, > > >>>>>>>> pcie); > > >>>>>>> > > >>>>>>> This feels like the wrong thing to do. You have INTX_NUM irqs, > > >>>>>>> so the domain allocation should reflect this. On the other > > >>>>>>> hand, the way the driver currently deals with mappings is > > >>>>>>> quite broken (consistently adding 1 to > > >>>>> the HW interrupt). > > >>>>>>> > > >>>>>> Hi Marc, > > >>>>>> > > >>>>>> Without above change I get following crash in kernel while booti= ng. > > >>>>>> > > >>>>>> [ 2.441684] error: hwirq 0x4 is too large for dummy > > >>>>>> > > >>>>>> [ 2.441694] ------------[ cut here ]------------ > > >>>>>> > > >>>>>> [ 2.441698] WARNING: at kernel/irq/irqdomain.c:344 > > >>>>>> > > >>>>>> [ 2.441702] Modules linked in: > > >>>>>> > > >>>>>> [ 2.441706] > > >>>>>> > > >>>>>> [ 2.441714] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.4.0 #= 8 > > >>>>>> > > >>>>>> [ 2.441718] Hardware name: xlnx,zynqmp (DT) > > >>>>>> > > >>>>>> [ 2.441723] task: ffffffc071886b80 ti: ffffffc071888000 task.= ti: > > >>>>> ffffffc071888000 > > >>>>>> > > >>>>>> [ 2.441732] PC is at irq_domain_associate+0x138/0x1c0 > > >>>>>> > > >>>>>> [ 2.441738] LR is at irq_domain_associate+0x138/0x1c0 > > >>>>>> > > >>>>>> In kernel/irq/irqdomain.c function irq_domain_associate > > >>>>>> > > >>>>>> if (WARN(hwirq >=3D domain->hwirq_max, > > >>>>>> "error: hwirq 0x%x is too large for %s\n", > > >>>>>> (int)hwirq, domain- > > >>>> name)) > > >>>>>> return -EINVAL; > > >>>>>> > > >>>>>> Here the hwirq and hwirq_max are equal to 4 without the above > > >>>>>> condition > > >>>>> (INTX_NUM + 1) due to which crash is coming. > > >>>>>> This is happening as the legacy interrupts are starting from 1 (= INTA). > > >>>>> > > >>>>> I understood that. I'm still persisting in saying that you have t= he wrong > fix. > > >>>>> > > >>>>> Your domain should always allocate many interrupts as you have > > >>>>> interrupt sources. These interrupts (hwirq) should be numbered > > >>>>> from 0 to (n- > > >>> 1). > > >>>> > > >>>> Agreed, but here comes the problem the hwirq for legacy > > >>>> interrupts will start at 0x1 to 0x4 (INTA to INTD) and these > > >>>> values are as per PCIe specification for legacy interrupts. So > > >>>> these cannot be numbered from 0. So when 0x4 (INTD) for a > > >>>> multi-function device comes the crash occurs. > > >>> > > >>> So who provides this hwirq? Who calls irq_domain_associate() with > > >>> hwirq set to 4? > > >>> > > >> PCIe subsystem invokes pcibios_add_device function in > arch/arm64/kernel/pci.c for every pci device. > > >> The purpose of this function is to assign dev->irq using > of_irq_parse_and_map_pci. > > >> of_irq_parse_and_map_pci invokes of_irq_parse_pci where it reads > > >> PCI_INTERRUPT_PIN from configuration space and saves it in parameter= of > struct of_phandle_args. > > >> This structure is passed to irq_create_of_mapping where it invokes > irq_create_fwspec_mapping. > > >> irq_create_fwspec_mapping invokes irq_domain_translate and gets > > >> hwirq, here the above saved PCI_INTERRUPT_PIN value is assigned to h= wirq > (*hwirq =3D fwspec->param[0]). > > >> And then using this hwirq irq_create_mapping -> irq_domain_associate > were invoked and mapping is created for virtual irq with this hwirq. > > >> So for any end point PCI_INTERRUPT_PIN value starts from 0x1 to 0x4 = and > so hwirq starts from 0x1 to 0x4. > > >> > > >> So the values are more generic w.r.t to protocol, that's why hwirq w= ill > range from 0x1 to 0x4. > > >> And then if you check pcie-altera.c they are doing this adding one i= n their > handler and while creating legacy domain. > > > > > > Is this resolved yet? Marc, are you happy, or should we iterate on > > > this again? > > > > Ah, sorry to have dropped the ball on this patch. >=20 > No problem, I wasn't making forward progress anyway. >=20 > > I guess that given that the infrastructure imposes the hwirq range on > > the host drivers, Bharat's approach is the only way (and a number of > > other host drivers are already slightly broken). I'll try and have a > > look at solving this at the generic level. In the meantime: > > > > Acked-by: Marc Zyngier >=20 > After looking at this myself, I'm not happy with this either. It feels l= ike there are > bugs lurking here and we're just hiding one of them. >=20 > Here are the callers of irq_domain_add_linear() for legacy INTx in > drivers/pci/host: >=20 > advk_pcie_init_irq_domain LEGACY_IRQ_NUM (4) > dra7xx_pcie_init_irq_domain 4 > ks_dw_pcie_host_init MAX_LEGACY_IRQS (4) > altera_pcie_init_irq_domain INTX_NUM + 1 (5) > nwl_pcie_init_irq_domain INTX_NUM + 1 (5) > xilinx_pcie_init_irq_domain 4 >=20 > I think all of these use the of_irq_parse_and_map_pci() path you mentione= d, so > if the problem is in the way that path works, I would think these should = *all* be > requesting the same number of interrupts in the domain. >=20 > I agree with Marc that we should request 4 IRQs, because that's what we n= eed. > If we can't do that for some reason, we ought to at least make all these = callers > the same. >=20 > Bjorn From mboxrd@z Thu Jan 1 00:00:00 1970 From: bharat.kumar.gogada@xilinx.com (Bharat Kumar Gogada) Date: Thu, 2 Mar 2017 08:46:18 +0000 Subject: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts. In-Reply-To: <20160913150511.GC27748@localhost> References: <1472553558-27215-1-git-send-email-bharatku@xilinx.com> <1472553558-27215-3-git-send-email-bharatku@xilinx.com> <57C57975.7040306@arm.com> <8520D5D51A55D047800579B094147198258D239D@XAP-PVEXMBX01.xlnx.xilinx.com> <57C59FE8.30307@arm.com> <8520D5D51A55D047800579B094147198258D28AF@XAP-PVEXMBX01.xlnx.xilinx.com> <57C6B7F1.5000001@arm.com> <8520D5D51A55D047800579B094147198258D2C99@XAP-PVEXMBX01.xlnx.xilinx.com> <20160912220241.GG23532@localhost> <57D7ADA8.5060201@arm.com> <20160913150511.GC27748@localhost> Message-ID: <8520D5D51A55D047800579B094147198263F7207@XAP-PVEXMBX02.xlnx.xilinx.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, Any one is working on fix for this issue ? Regards, Bharat > -----Original Message----- > From: Bjorn Helgaas [mailto:helgaas at kernel.org] > Sent: Tuesday, September 13, 2016 8:35 PM > To: Marc Zyngier > Cc: Bharat Kumar Gogada ; robh at kernel.org; > bhelgaas at google.com; colin.king at canonical.com; Soren Brinkmann > ; Michal Simek ; arnd at arndb.de; > linux-arm-kernel at lists.infradead.org; linux-pci at vger.kernel.org; linux- > kernel at vger.kernel.org; Ravikiran Gummaluri > Subject: Re: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device > for legacy interrupts. > > On Tue, Sep 13, 2016 at 08:41:28AM +0100, Marc Zyngier wrote: > > On 12/09/16 23:02, Bjorn Helgaas wrote: > > > On Thu, Sep 01, 2016 at 05:19:55AM +0000, Bharat Kumar Gogada wrote: > > >>>>>>> Hi Bharat, > > >>>>>>>> @@ -561,7 +561,7 @@ static int > > >>>>>>>> nwl_pcie_init_irq_domain(struct nwl_pcie > > >>>>>>> *pcie) > > >>>>>>>> } > > >>>>>>>> > > >>>>>>>> pcie->legacy_irq_domain = > irq_domain_add_linear(legacy_intc_node, > > >>>>>>>> - INTX_NUM, > > >>>>>>>> + INTX_NUM > > >>>>>>>> + + 1, > > >>>>>>>> &legacy_domain_ops, > > >>>>>>>> pcie); > > >>>>>>> > > >>>>>>> This feels like the wrong thing to do. You have INTX_NUM irqs, > > >>>>>>> so the domain allocation should reflect this. On the other > > >>>>>>> hand, the way the driver currently deals with mappings is > > >>>>>>> quite broken (consistently adding 1 to > > >>>>> the HW interrupt). > > >>>>>>> > > >>>>>> Hi Marc, > > >>>>>> > > >>>>>> Without above change I get following crash in kernel while booting. > > >>>>>> > > >>>>>> [ 2.441684] error: hwirq 0x4 is too large for dummy > > >>>>>> > > >>>>>> [ 2.441694] ------------[ cut here ]------------ > > >>>>>> > > >>>>>> [ 2.441698] WARNING: at kernel/irq/irqdomain.c:344 > > >>>>>> > > >>>>>> [ 2.441702] Modules linked in: > > >>>>>> > > >>>>>> [ 2.441706] > > >>>>>> > > >>>>>> [ 2.441714] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.4.0 #8 > > >>>>>> > > >>>>>> [ 2.441718] Hardware name: xlnx,zynqmp (DT) > > >>>>>> > > >>>>>> [ 2.441723] task: ffffffc071886b80 ti: ffffffc071888000 task.ti: > > >>>>> ffffffc071888000 > > >>>>>> > > >>>>>> [ 2.441732] PC is at irq_domain_associate+0x138/0x1c0 > > >>>>>> > > >>>>>> [ 2.441738] LR is at irq_domain_associate+0x138/0x1c0 > > >>>>>> > > >>>>>> In kernel/irq/irqdomain.c function irq_domain_associate > > >>>>>> > > >>>>>> if (WARN(hwirq >= domain->hwirq_max, > > >>>>>> "error: hwirq 0x%x is too large for %s\n", > > >>>>>> (int)hwirq, domain- > > >>>> name)) > > >>>>>> return -EINVAL; > > >>>>>> > > >>>>>> Here the hwirq and hwirq_max are equal to 4 without the above > > >>>>>> condition > > >>>>> (INTX_NUM + 1) due to which crash is coming. > > >>>>>> This is happening as the legacy interrupts are starting from 1 (INTA). > > >>>>> > > >>>>> I understood that. I'm still persisting in saying that you have the wrong > fix. > > >>>>> > > >>>>> Your domain should always allocate many interrupts as you have > > >>>>> interrupt sources. These interrupts (hwirq) should be numbered > > >>>>> from 0 to (n- > > >>> 1). > > >>>> > > >>>> Agreed, but here comes the problem the hwirq for legacy > > >>>> interrupts will start at 0x1 to 0x4 (INTA to INTD) and these > > >>>> values are as per PCIe specification for legacy interrupts. So > > >>>> these cannot be numbered from 0. So when 0x4 (INTD) for a > > >>>> multi-function device comes the crash occurs. > > >>> > > >>> So who provides this hwirq? Who calls irq_domain_associate() with > > >>> hwirq set to 4? > > >>> > > >> PCIe subsystem invokes pcibios_add_device function in > arch/arm64/kernel/pci.c for every pci device. > > >> The purpose of this function is to assign dev->irq using > of_irq_parse_and_map_pci. > > >> of_irq_parse_and_map_pci invokes of_irq_parse_pci where it reads > > >> PCI_INTERRUPT_PIN from configuration space and saves it in parameter of > struct of_phandle_args. > > >> This structure is passed to irq_create_of_mapping where it invokes > irq_create_fwspec_mapping. > > >> irq_create_fwspec_mapping invokes irq_domain_translate and gets > > >> hwirq, here the above saved PCI_INTERRUPT_PIN value is assigned to hwirq > (*hwirq = fwspec->param[0]). > > >> And then using this hwirq irq_create_mapping -> irq_domain_associate > were invoked and mapping is created for virtual irq with this hwirq. > > >> So for any end point PCI_INTERRUPT_PIN value starts from 0x1 to 0x4 and > so hwirq starts from 0x1 to 0x4. > > >> > > >> So the values are more generic w.r.t to protocol, that's why hwirq will > range from 0x1 to 0x4. > > >> And then if you check pcie-altera.c they are doing this adding one in their > handler and while creating legacy domain. > > > > > > Is this resolved yet? Marc, are you happy, or should we iterate on > > > this again? > > > > Ah, sorry to have dropped the ball on this patch. > > No problem, I wasn't making forward progress anyway. > > > I guess that given that the infrastructure imposes the hwirq range on > > the host drivers, Bharat's approach is the only way (and a number of > > other host drivers are already slightly broken). I'll try and have a > > look at solving this at the generic level. In the meantime: > > > > Acked-by: Marc Zyngier > > After looking at this myself, I'm not happy with this either. It feels like there are > bugs lurking here and we're just hiding one of them. > > Here are the callers of irq_domain_add_linear() for legacy INTx in > drivers/pci/host: > > advk_pcie_init_irq_domain LEGACY_IRQ_NUM (4) > dra7xx_pcie_init_irq_domain 4 > ks_dw_pcie_host_init MAX_LEGACY_IRQS (4) > altera_pcie_init_irq_domain INTX_NUM + 1 (5) > nwl_pcie_init_irq_domain INTX_NUM + 1 (5) > xilinx_pcie_init_irq_domain 4 > > I think all of these use the of_irq_parse_and_map_pci() path you mentioned, so > if the problem is in the way that path works, I would think these should *all* be > requesting the same number of interrupts in the domain. > > I agree with Marc that we should request 4 IRQs, because that's what we need. > If we can't do that for some reason, we ought to at least make all these callers > the same. > > Bjorn