From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751493AbcIAFUG (ORCPT ); Thu, 1 Sep 2016 01:20:06 -0400 Received: from mail-sn1nam01on0086.outbound.protection.outlook.com ([104.47.32.86]:45856 "EHLO NAM01-SN1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751015AbcIAFUD (ORCPT ); Thu, 1 Sep 2016 01:20:03 -0400 X-Greylist: delayed 153219 seconds by postgrey-1.27 at vger.kernel.org; Thu, 01 Sep 2016 01:20:03 EDT 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: Marc Zyngier , "robh@kernel.org" , "bhelgaas@google.com" , "colin.king@canonical.com" , Soren Brinkmann , Michal Simek , "arnd@arndb.de" CC: "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. Thread-Topic: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts. Thread-Index: AQHSAqrdY8Zj+O9d0E2xRiMSs6E3q6Bg5aOAgACfgzD//45TAIABrX2A//+gVoCAAbOx0A== Date: Thu, 1 Sep 2016 05:19:55 +0000 Message-ID: <8520D5D51A55D047800579B094147198258D2C99@XAP-PVEXMBX01.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> In-Reply-To: <57C6B7F1.5000001@arm.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.23.94.45] 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.0.0.1202-22546.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)(2980300002)(438002)(189002)(199003)(2900100001)(76176999)(50466002)(5001770100001)(19580395003)(86362001)(33656002)(626004)(50986999)(106466001)(47776003)(63266004)(8746002)(2201001)(54356999)(55846006)(8936002)(11100500001)(3846002)(93886004)(189998001)(106116001)(2501003)(92566002)(2906002)(97756001)(107886002)(5250100002)(8676002)(2950100001)(4326007)(305945005)(7846002)(81156014)(356003)(87936001)(4001430100002)(23726003)(7696003)(46406003)(81166006)(6116002)(102836003)(586003)(5660300001)(2920100001)(7736002)(107986001)(5001870100001);DIR:OUT;SFP:1101;SCL:1;SRVR:CY1PR0201MB1483;H:xsj-pvapsmtpgw02;FPR:;SPF:Pass;PTR:unknown-60-100.xilinx.com,xapps1.xilinx.com;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;CY1NAM02FT030;1:4ZJqLfKQEpmy3mivzyZ72SGbhWZzCTRwo3QpGJ5GjkvShWOn89AjTDNKbbEQKT2WWWrsbFNF0w0Vz7ZPlTpi5uL17t1z3Ag8g9swlvCesNF8XA/UPcn5MjzX0d+EIx/3Lu6GW3T2cYad15aiGQq2BfrP3xkPHu0S5/i51bIPheEe7//V/GrjvgnjE13uIpFUH3BU51ru4Gc3KW1DyFx3SJvH7wJJ621HGkdMSA6LeP9eSavbmAI0qVYBqvzaccwauvng6im5pp5QxWciUoWS+cz236Ym0wkl1T66KfhmaOkRuIae/5hXS1kCRkAwVBfV4cgsBdzZOyZ6NJMQ/zqlsugMTI+Fv6TIwbVTjVuhuNIoGm4SL3pP+/ODJocmVm7F9xm7CwM2BFyvKwakKFcKGIGmgXTmUAjHzyOsbCObtpi5M6lQwuIQVSoYpDWBAcRCpucS9PSoykmzsW/dtf1hjbZ3uvz+OKqqUdVMsIkJi+BE5qnW//fY4zQpHBxTGoh3N41DqaatrCz/astf20Ks3rdxwhkI83BjI/tb1uyMhdjpNloDc0ABBWAaHxYekI17lyG7l2nzI+KqK3HRYACNCzleTyBuUATeBbNGlZmQDgCfwR1RPthSSdDtuQtY8mBZ X-MS-Office365-Filtering-Correlation-Id: a7b89f5b-28b6-428b-f9fa-08d3d2279f25 X-Microsoft-Exchange-Diagnostics: 1;CY1PR0201MB1483;2:e1riR4FMlya0uDvs2ZYfdCBJRR3gum2RUO+kjmMJc5i2QCaV9BlG9Hbfj7emK9vTuuuT/OI3O96ab+so6NMdu4DvPWSF2R96K8E6r/q30e/YRCpNVtPtKq7SdxNEdrRRhYWmerO3qMK+arcRPweCEnexVz1fRFVwMP2HZvoELfQ1n9HV/ZML09ht0CLJevX4;3:8pHkfA0M2tYouF6v5QSgt7RuEGHMGgititJkIsGwittdq84MJhcoWe0fT+5D1RrwqjPoNNVjfoH5UauXVCktWfFBsjtF3bLery0g7U/K0rcNmmOee9QnfuE3QCRxRvXJXjIobiOXX/QiBJjPH5resTHkw84Xzff1iPw6hhDZxGVvI/hHcFiM1OUFJkPV5LnvF/M/Iye2plK82iOaqjQttuZ/CmZ+LucypaUuSis+rsKbQU7+yONKbOuEqOuYDpEnCtr6qgfZk9uwkfE5snmAvw== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(8251501002);SRVR:CY1PR0201MB1483; X-Microsoft-Exchange-Diagnostics: 1;CY1PR0201MB1483;25:9N4+WffM6D7wluWNIK/B/d27R2QYdJPUhffSWHecu4NxQsvirT2fQ+74rEFRrbHNx/Z1gKxPeYLe/1yPIRn8bMYgx+kmIdwuSZV4e97Pz9B9nFdmfd2wKcsP+CnVLSmdYUiSojP9sPOug/Mw/oxojmmFcjSgtW6eQV40tOdMVNDZmFSntq23tPnlstgy//N2x/cQIDbcb6NElnUa6TrinkWT2SULWy+tXo8X1Vp1xGE8/1cXASp4yR+el6ga3j2TSNj/1UufA2yk/gZ86MzDx7TwkoRWaSrSPPJodVkdADbje4BomimCJhzX6TSlAYYy2A28woGdR80SYA27ybQSsmXBHZaQIziSZ6No1oCdvYd053/lEVTqbdxuFlESoIkHdqq8n2LA9bCaDqYY35jhImJkmh1frSIoGZJ88XDVxi/R6O7W8RDdAR7ecJvl/VUafCRf7bDuqhjJ9CvmUdbrYNmIPjBniNhJTtR35ogJ8qAgc3YuC3Sv5oaBXtWMvaAUxEX7ZPsCFGTj7Zp6c1De8UJifEXruKMjKu1cjJkPTbHIsfxXXd85Ag9KK1gnSZynrVDgKyYPfa/6z7tGPWJyJT2E2oBiH71VOu+Jzxu497bUwpEk7ZqeePAstpiUf61CiFSHK5llgGi7lcu3C2ZohHVBWu+1KnrsvXTfyCyAYtVzy4vg2/zEjRFr3qjiCKkjKGQuMzcHuJ/qWlnOJyEjRXepgOUoDSATqXuX0tUSp90j8LCXgT3EZUHPzGB0V8Ew+xDxxjyhben7HXBbIgeItA== X-Microsoft-Exchange-Diagnostics: 1;CY1PR0201MB1483;31:OvmxomQ6sZwhImZtRMfAW/aosetKJRz9ONN3ghW/LIR759TxUc1/ELTygTGgjIQezhZmBvyxCO88+0VG6FvP7HcWYx97GFgjsA9Cb/MxF/a03COoyu6rZ9khH0Md3OBxI3AwJwrgtGB84kF/2vz7yGbeeAi6eMNF+BeVGGfEYwpRFe8FmQYvI/YG4NEDVIBW8HTCtOuniRG9PAy+QhuU9ZlR+zfvny1cPxhco5ao2E4=;20:pBE6maRJLe1ZtTZTGQhq6NPXEy7JR7UKPK8huSn9ypPglQvX7BwQKiE9VlwNGClONuuX53MZoEmlvzLv9uyBmHClHhdqVJkJyH4V4fkBYp291pbEHWgpNbNu3ay4DdnyjZAd7+uzNJxM5iL1jAILjkdLE2okBa9egRT+AqGH7GY3k2387jt7TN/8xWgncBir7j8fP4tM5NAd2nbdvy8GYD7u6wsuNyrBvEG7CkyvclZVhw2VCu60fPYw7zQKE9RKRsR9o+hc5Ej2smlXL4W1CiVHpqULPWwHtbbXWAKIL/zDp+iFLHeAXCCsDQR8Yk39/cuSb6Aq2nOt2CcN4uA+n0B595RP35OqrOpe1UPY+1YoLssu7K/iOgqOr0Lv2KuKeNHshj/9HBdNkvTXkBfTWIyct1bjT/VJcuOIBcSfHjlWhurTY/jizxH65l0aFDkTB/7HeduL4Ygk587MLzZ+xh9B4InWZ61dnldtAvIJEyBlJFw6A0Jlm15XRiux2K3W X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(13017025)(13015025)(13023025)(13024025)(13018025)(3002001)(10201501046)(6055026);SRVR:CY1PR0201MB1483;BCL:0;PCL:0;RULEID:;SRVR:CY1PR0201MB1483; X-Microsoft-Exchange-Diagnostics: 1;CY1PR0201MB1483;4:FV/AZCh2mq1Ri+bIhHxcwd0Gu53QPQtAwdguLUhlvbQyIAA2OiTROJY2JMrXks4VigIX/7O1H7yVXPr7gCiuAbMtxh6GSuSJ90HVo7mTD2d+f0I0ugWR2wdSOZMgejgksDKS3vmk25IE+cVwaKOj6tXfdZ82+aGg0UZqiG0j2g1V+RSOL7hHAm1iYwlp3cK12shhfvbB268EvQTJryCtXC41feY7sMLgS/pHLPiHlM6eLztcTs4p4QZEJxU9ZmuuQ0/gCQlz7Ea4oOYNsJSs1xDA96DmyzjMNNeZY5q9IqOex/aYsmfHI7Y1xhi9lcqSNelJ4N0hvv4luss9/w0BmwHhljPiKLjnOv0fFKriqYLVZ+j5sS98/bVeFPb2CbaG4sngdotP/DF22/7oep69DZid88uiib2a69LBCKOg1jvESgovbG2Vslzaph9pmGqUJBqfKmBOkb7DCQzUB4X8fCklH8N2ZAbJCjycbqPC1vRPxw1s87Zy9odtPPQu5QVt X-Forefront-PRVS: 0052308DC6 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;CY1PR0201MB1483;23:Pa6ZcxzjZmMCHulbez7nEsyf5hfSQ3joHRG60gL?= =?us-ascii?Q?dBK2m6JeLg+ZcvIZrNkv0BZ79Swvvt7bSPjg7Amz+dN+r0Lt1jJ0N6DV9RJ1?= =?us-ascii?Q?R425ZB89UWNXgtFQLPtbqCmBAZi/UcX1+LYb6L6qjyL4yuAMVWkmwn3hLCEv?= =?us-ascii?Q?mru83uJ6WZ7xL7U1HKiBwgspjkU34qXsrfjPereK1Vls9t8Q3+Zgnn5cWwXy?= =?us-ascii?Q?65sgRofUmdT8HzVF/qtIDpDovyL9TPZZAU6Xyf4JdK1c3oHWPP/3ygh4E8ti?= =?us-ascii?Q?jslAavhEYGN35oh8tnnxa3TYcuyZjdxj6B94on8SemGKBKd9AiLWrLQJv/RO?= =?us-ascii?Q?ai4CA4d6KjlUuXdv7+CJzwkMQZp93AgySyqae9NnMP4n/IwyyL9KhaAZpwdg?= =?us-ascii?Q?9+O/2YSKafVYEvttW+Rx+ELATN3RuRTtuwaJJHqg27i0xaw2gT6BvlnOvWX/?= =?us-ascii?Q?z6rkWNKSKnYvY3FOgEr/rmdCDF8XE+Zw/GtnyJl5bM3K5hEdqIn9abBj0aYv?= =?us-ascii?Q?U0bdD51bibmqGka2ZdaxovoJYrjPeAfwMvhtub21oJZk2WcHjN6CIFgM+ZvM?= =?us-ascii?Q?fJYJHP1/DuMHCTa6/TaNYg9sYbL1snj0zTQcfk0AwWLSYM4+B6zOXbt5XBQ7?= =?us-ascii?Q?7ih6baYxGRaQFwB6qsCDDXd8H7VGuiVCfQPioZDFHrasdJ/rQeZQxOL+0uI7?= =?us-ascii?Q?wsS9F8d1sXxryvCbcquFK/Xsqc5S/5LBKz2Y2R+Z+lPDMp+s1X1kgKYXS912?= =?us-ascii?Q?ZPrp1A2YO6SKhQK72/8XQQn1dW2yM6PaNrNZ/eQNsop8+EeCqcc5flvNTav2?= =?us-ascii?Q?oR8apHBtk6BoAULI03egTEDAyjhfvtGN4FZmr6gKE0buzSuDtl8Maug/jtHf?= =?us-ascii?Q?t2sweTSkjPbBPBpvhRuctghfOtlYQuSzXTRkVoEkkr3JBiFY5aeVmZGdYNLJ?= =?us-ascii?Q?fi68IAVZgYj6yKfalZ0cLD0i7BTNVQumyTnZbyFUVVEs9QmZMzI04MCg7Bl6?= =?us-ascii?Q?BeH8vu9gPzZsXCjH6NUqP7IQfrxMSG9VIS9VK1U9tHwkBb0087xnHzNgk1sD?= =?us-ascii?Q?Xj6T0ELR+dZpCNF4Ti0ZeVG606qxdG8NNB9JMKIBe5Xd4mN7UFGf/08L7ZsL?= =?us-ascii?Q?/j+SzVe/Co/CRZsfKvHo8xC7vL1AOA5n2zJqpbPvDpmF0bP41WoHyIqxy4K1?= =?us-ascii?Q?aiQ8USJ/W5Eqlc8bu0vkeXSpMA2SxtyJISiFMO+GIx9BhT30eFdcQafkgzB+?= =?us-ascii?Q?JbQGy+/JbdbXnKFPZTRWMcwbXqoqAoVUSCJczI5jbIFd6r3oKFUl7YWQZt+P?= =?us-ascii?Q?TzVHWqX5SCUcS5Q59Otb30Ew=3D?= X-Microsoft-Exchange-Diagnostics: 1;CY1PR0201MB1483;6:cgFCs9CGLA+ZODL7d3IqKIHz4KJiLBkbFGOT8Yo4jRlaOT0jdsMJy9i/mbJznUdjrnd2kzYwuDss2Sny6aRpBsI2WrNLkJEyEro6pu9/peF9oErj7YeuaQJX9njUkGypwyArWZLRUBNbo6WWtG77uu/JWWUgXlS5Thy5/KT78SzpeF/VlVRWcQ4TTvgPqB3A3yqpeqZk3qrDXSi0kJZHPLz7ifFta3kRHf9+hKZ6QSBuVe2bkXZYc10KBEFgONv53tx6uaxQnjmEK7/Jo47hcOsAE7TjKvGvW8wPc52/fe3ZTEvUbnQIihOAOU8dnPX0Lzx3jk126Hwcv5zrtDCMOQ==;5:rHMG4z9H4/bgOnPvle9CBMrUb+CXzacjp+Oc/4z36IXSS4bxsUtYNf4ptrzXVrHB6tInwnLTFsmxgDpRvsL5+DPfR+AfbP7XvAilK8GNKfiDivPdVGkGiYvFA33ql4rPigVfZcF8F17E0EnrTIzUPQ==;24:zs0lSSyPseT1Q52oSU1rjxCwuld8K1EqWJfMIvgOJAr8mX0dl/ZjOXH+h0mS8rdpfmA1pf/5RDvEzpFpPSa1w7OVYERzTkyUGAZTTASPipI=;7:bA1aUl+ByDkb0bsTxaeBJmf7T5obiZAjRVZTIoj1YO+IN2OlPEQFHmFjqhCBtrCxy4BUOF4eggKr4Jm+kI2qzTjLZBprEEuedEAdB9/p5TLFt2y5P4sUCU15dV8z/x0J2Zmy95dOxxDH1o9lbONQIIuKwaJoBrs5miFcTfXk13bvMptBjgcdWWB6af0cQOFyiW9SZ5xwDx//1JTrsFulfWmQXsUOb/YyDJ/fExaaq2WdC6RJjKATwW4jjxmbqLjy SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Sep 2016 05:19:59.9214 (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: CY1PR0201MB1483 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 u815KDnY009546 > >>>> 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. Thanks & Regards, Bharat From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Bharat Kumar Gogada To: Marc Zyngier , "robh@kernel.org" , "bhelgaas@google.com" , "colin.king@canonical.com" , Soren Brinkmann , Michal Simek , "arnd@arndb.de" CC: "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. Date: Thu, 1 Sep 2016 05:19:55 +0000 Message-ID: <8520D5D51A55D047800579B094147198258D2C99@XAP-PVEXMBX01.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> In-Reply-To: <57C6B7F1.5000001@arm.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: > >>>> 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_n= ode, > >>>>> - 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 >=3D domain->hwirq_max, > >>> "error: hwirq 0x%x is too large for %s\n", (int)hwir= q, 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 wr= ong 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. >=20 > So who provides this hwirq? Who calls irq_domain_associate() with hwirq s= et to > 4? >=20 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_m= ap_pci. of_irq_parse_and_map_pci invokes of_irq_parse_pci where it reads PCI_INTERR= UPT_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_crea= te_fwspec_mapping. irq_create_fwspec_mapping invokes irq_domain_translate and gets hwirq, here= the above saved PCI_INTERRUPT_PIN value is assigned=20 to hwirq (*hwirq =3D fwspec->param[0]). And then using this hwirq irq_create_mapping -> irq_domain_associate were i= nvoked 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 ran= ge from 0x1 to 0x4.=20 And then if you check pcie-altera.c they are doing this adding one in their= handler and while creating legacy domain. =20 Thanks & Regards, Bharat =20 =20 From mboxrd@z Thu Jan 1 00:00:00 1970 From: bharat.kumar.gogada@xilinx.com (Bharat Kumar Gogada) Date: Thu, 1 Sep 2016 05:19:55 +0000 Subject: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts. In-Reply-To: <57C6B7F1.5000001@arm.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> Message-ID: <8520D5D51A55D047800579B094147198258D2C99@XAP-PVEXMBX01.xlnx.xilinx.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org > >>>> 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. Thanks & Regards, Bharat