From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751986AbbIQRNd (ORCPT ); Thu, 17 Sep 2015 13:13:33 -0400 Received: from mail-bn1bon0099.outbound.protection.outlook.com ([157.56.111.99]:61072 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751723AbbIQRNa (ORCPT ); Thu, 17 Sep 2015 13:13:30 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=David.Daney@caviumnetworks.com; Message-ID: <55FAF4B1.1070109@caviumnetworks.com> Date: Thu, 17 Sep 2015 10:13:21 -0700 From: David Daney User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Lorenzo Pieralisi CC: Will Deacon , David Daney , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Rob Herring , Frank Rowand , "grant.likely@linaro.org" , Bjorn Helgaas , "linux-pci@vger.kernel.org" , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , "linux-arm-kernel@lists.infradead.org" , David Daney Subject: Re: [PATCH 2/6] PCI: generic: Only fixup irqs for bus we are creating. References: <1442013719-5001-1-git-send-email-ddaney.cavm@gmail.com> <1442013719-5001-3-git-send-email-ddaney.cavm@gmail.com> <20150915173654.GJ31157@arm.com> <55F85A24.2010600@caviumnetworks.com> <20150916103232.GB17510@red-moon> In-Reply-To: <20150916103232.GB17510@red-moon> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [64.2.3.194] X-ClientProxiedBy: BLUPR07CA0034.namprd07.prod.outlook.com (10.255.223.147) To BLUPR0701MB1713.namprd07.prod.outlook.com (25.163.85.14) X-Microsoft-Exchange-Diagnostics: 1;BLUPR0701MB1713;2:eKiuc3O9DocSvrEzpBBarfeeWs7ZJc9qpDJoFsWL23NZLoKEX96oM5lAP/DuH25yHJY2HMBEYj5Ucoc/a2+tshr1sCp1WlO7qM5bIpZ/28i1npti4yOYdocvsQT5p8BpVAhnh1Xpl10QbGt3+Q9kR6QjYGx/WoAxjgs1j/ZNBwQ=;3:WvfokTZdELzHv322oWT30OR5vjsFRutVM4gtw70uKNGbwNhjq2jj8kR5D/vX8wP+GU1zhKCyKW58Tekkd/G1SSLoNorlAd7S/MF4laV+9wohoAW9NSNiuViOGqm0Ck+6HQCztDIubh8okYxwTsLzLQ==;25:d/Yqc2nxqxSzrs763KKOjlAm6d/iN3ngJ+zzJORKp+03idYj75uklWM3gmqakLGUHcR32P9JIzkoEuclk6nD636wt2aYx9dszpgyGpxtehVtVvPLJIfKgFpH8P3D+NDo19KDOhE53NTSXHoeaRCKlOhUXGBontn3V/xzE2r8o+WS8XalXSnsbJpMU3pkJhz7Xaif7U3eaPOr2uyzW3KhbgrsSBhZ6YqdURmFhFGUvZR9GMKl0YDb3kIUp7vjzQeYwM1JzPjN1ZiVQhunMABc+Q== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR0701MB1713; X-Microsoft-Exchange-Diagnostics: 1;BLUPR0701MB1713;20:zV6+m+xMoQmWe7beMK/u7I6QBD1hxZ6EjZkOo5Y1KGeY3kGeOdYItTrAEqyXP6VeCh3K3YzeQ2EcFuhuV5fsPADOMgJNWwEfMXQK2b4tepMQVQe6wcg6VHXXMI4mcOlA/sD4IKPqaPw6v9D11hXKtH1Z8PuxiXI40V2YDNKy5bAToRQEnvaKh9KcDlWG5wfXxrMTQ/Cva0koKg6JurQPCx1NU6HmtR0AQlH81dxmFqL2w8pAkkDiYxbbb/Hne1DoV9XwQknhbcGkNZE+EKXdRixd0NcB2vd4SZPTsY1LfWlCSsI5XjlyrMKPsGbld0dIdw7ooFgNeEeGVZ12xVE/NH4ZOO69rfl/0pLTjQwQU6Z63T2YHaveM+rCGWL8znl4FMbwGR5ER9n3ytD240U1s/lpr8RJWqfO8oySMVnJRxJmTQfhMpsnNyDPgj8IVdR3Ky4UOMo2xeDQEC0d3h1fB43vgDgcBHmBM0JoZdCBOah4VppcXNdS95QZ4rN8BFSq6Da9sTkmK79Zq1cLFnHgJjwSPcBw54tAcBizZNDFLtvI6P8m4IK9VwHH5UqdS8kr04DhtiJSEvHvlj7yiI8GKXXgSA1jfy6z+AwUjgZFARA= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(8121501046)(520058)(520078)(5005006)(520075)(3002001);SRVR:BLUPR0701MB1713;BCL:0;PCL:0;RULEID:;SRVR:BLUPR0701MB1713; X-Microsoft-Exchange-Diagnostics: 1;BLUPR0701MB1713;4:6pr5lQIpMiX63mm3OaOKDa3/vug1fknvUqSPaDe+Jf/zwkANiSBrvwyLcuKh74C68O7KnKRxJlVn2k9MTAmZIxm49HQdKzMdlOLc8Ov5kGv/9bGa4aCugAd0FQG8v2V/hkIAuJDR/gkMTsVvb6G7/VDyUI0h/ZpZPH8g2QLmpJmoK7kJpVqAUeSSTIYjkxGJtr16Klq0gO2zXvC2JRncB3Uxa1FCi1CdLkf5rh2JLxEMp4da++/b8WH9084NYtZ/RNx6dEYvoPrWi6arhmBKUeEkXISaCThuq7xMo2ylgTOuF5otIeUgY6m4z2hiJeFCYg4VLRan3h2kjJGjryilTgP2lZ8rTqcfXvj9hM4UsEoFX1jfOW+7k8q4QuOnRHLZ X-Forefront-PRVS: 07025866F6 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6009001)(479174004)(377454003)(24454002)(199003)(189002)(76176999)(81156007)(87976001)(5001860100001)(47776003)(59896002)(65806001)(87266999)(66066001)(77156002)(68736005)(23756003)(62966003)(4001540100001)(77096005)(50986999)(101416001)(5001830100001)(105586002)(54356999)(65956001)(189998001)(65816999)(110136002)(106356001)(64706001)(83506001)(40100003)(53416004)(5004730100002)(575784001)(97736004)(80316001)(42186005)(92566002)(46102003)(93886004)(4001350100001)(36756003)(122386002)(5007970100001)(2950100001)(50466002)(64126003)(5001960100002)(19580395003)(19580405001)(69596002)(33656002)(41533002);DIR:OUT;SFP:1101;SCL:1;SRVR:BLUPR0701MB1713;H:dl.caveonetworks.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;BLUPR0701MB1713;23:CHIpB0KAg2XPcdl0t0f2y9FdihLBjVCGT0jFH?= =?iso-8859-1?Q?ChSuyBIh6PEp5VjhI0MqeTQGZk+GkWCA1ppgvM0IZcbLsHixm41HInRZlX?= =?iso-8859-1?Q?qDRC5grn8GSCKtKKSDBzGO+1ui0kPAutww3HcXwWbaNjej4m2VNTQ037Oe?= =?iso-8859-1?Q?1zI1q9UBryguTear4OvXACyoHl4RDII/j23xjUIx7u946cBn4JzMhJN3U7?= =?iso-8859-1?Q?Re6HESUo2zsrhOajWGVBtHWGLDemTjKg42wjxz3OewxRpUEodRBpwopRGp?= =?iso-8859-1?Q?A14xO6JCmHRVWL4azh/lenCB+CaWv7rzEnHsCLXxA9pQ+0D3BkBGhJr19L?= =?iso-8859-1?Q?LcO79hFj8ulD9RSdU6ynpipiZomILy3k/Mvojib08Ciz+uylD8iWbTD9Vd?= =?iso-8859-1?Q?iAaunBk39d/BtGTpnauo+YvdsjrSPShvx/KGO1WXhhhRD2moOLaHiFs9+N?= =?iso-8859-1?Q?/xXEo8zBWiJriac8BzYolQc7PzItClmRmtiTZjkwZdVze7LMp83SrdaEwx?= =?iso-8859-1?Q?supVjk71mihnjrHLFh4DbtL+glVQymYe/AIVR6IGyQqMIZB0pAvVJxa+86?= =?iso-8859-1?Q?C0XNVhgBZzWaj0jLPOeZ0NR9bcdaDBwAYyLcgJeoExbkRJ+SExcWrxQrnh?= =?iso-8859-1?Q?NejfVZ+j89plsS1vQuUzXwSxit0dF9gLmdwJi0nn6CZAsATnMtMCti498x?= =?iso-8859-1?Q?vk4RYrbij8gGWF0kvJvp5KZ7/BGaGJDtf1lv3QqyxiPJZw3d+4irdctUtA?= =?iso-8859-1?Q?zGll5sUiO+Ubj1EN2GnqMvJQxmkFNaAAXv9+XTfAInVtAfH9nI9eSojMxc?= =?iso-8859-1?Q?HzawD0U2+9uZlTQvF+bW/BSyENeBCVFl7Uo9JzlXlJgtF/CIavEsOVBOvQ?= =?iso-8859-1?Q?/liEEmj2aBrS22oBuZu/3kVM2IqtvqxxxlETS7FsXzQvSDDzUoOk9LXFBR?= =?iso-8859-1?Q?PZvMenvdRoI5ymxQIRvztwsjonKWR13hoWLXJYw/mEwm2Xw+oqGDLVtytD?= =?iso-8859-1?Q?iDpJ+RtdBU7XkNZktduOEYz2MvNX9XZ40OdH/ZRKEcxS2Hs5WfwaTfPdbk?= =?iso-8859-1?Q?1zIv+zX5H9FJvKzWpM3AZQsB3jRrJayoIsrFdbZP/n7+QWDMrXXQhSVHRU?= =?iso-8859-1?Q?IRPBipiBRF+SJRWOwix5o38pJ782ritj8Gbi6uA2F0FQkWrszTiZ0dgBzy?= =?iso-8859-1?Q?mtGsD05a/WjXzPszXGjvg3jyAeZK/YZKtVx7/bC8qcr0K1Wy8+dIu88+O+?= =?iso-8859-1?Q?+X5D9xp4J2TklmQ5dxEtA/T9euMbSGITueouCdgPYv5r7P0LFSdCoNO0yL?= =?iso-8859-1?Q?SeZ341o4RA9SpEqbKq8y1IrV2Ll7sJmoyYS/iNp7r34W5LG8oGEZfOrA8M?= =?iso-8859-1?Q?ebWObLBLxREDqcQR/bEvEYM5+fy2JUq72TO41unCo3/rgIGfHQpzQ1q43t?= =?iso-8859-1?Q?rrolArfbrndEvo8OvxB3s2cVJdQMHWOT9HLrRlQPkN/E20TrbtT2zvlnHQ?= =?iso-8859-1?Q?v/gDebJg0nFP9sKWmLHxNpz0e+XsR0TUtVBuhtY5V6+?= X-Microsoft-Exchange-Diagnostics: 1;BLUPR0701MB1713;5:fZTY9neCV7giAfpV8dL8Ln1fLAFxrgDLPQNqnZTOnstqQkc5h/ktFPQh6TlEd90t5w6IfS66Xb1jPz3XyNb5nDpf6/4sXfiSiiG8cK/8CbtC94xTQc/GhSzohDJ6KuArHwsF4EV/k9jXXArfVMptig==;24:AJNwG9zx8nBznGNdfdJtNHfElZNgndRy50tyCKGzB++yfEKXH2x/SaD3urj3dcfgdhb6qMlfCVRgmHQ9u9lqHudvZCzbTX2zHESuYKorjT0=;20:vX5BxkSdGeVxwyLf7a2pitLF5WMDZsAu196KIGtKaMSqT8YrWGPEe2g6Z/nX/5nFbiIxCcy4nlhY1taJxlqMbA== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Sep 2015 17:13:25.6906 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR0701MB1713 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/16/2015 03:32 AM, Lorenzo Pieralisi wrote: > On Tue, Sep 15, 2015 at 06:49:24PM +0100, David Daney wrote: >> On 09/15/2015 10:36 AM, Will Deacon wrote: >>> Hi David, >>> >>> On Sat, Sep 12, 2015 at 12:21:55AM +0100, David Daney wrote: >>>> From: David Daney >>>> >>>> Use pci_walk_bus() to restrict the fixup irq actions to only the bus >>>> being created. >>>> >>>> If we create multiple buses with pci-host-generic, or there are buses >>>> created by other drivers, we don't want to call pci_fixup_irqs() which >>>> operates on all devices, not just the devices on the bus being added. >>>> The consequence is that either the fixups are done more than once, or >>>> in some cases incorrect fixups could be applied. >>>> >>>> Signed-off-by: David Daney >>>> --- >>>> drivers/pci/host/pci-host-generic.c | 8 +++++++- >>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c >>>> index 265dd25..a0fb241 100644 >>>> --- a/drivers/pci/host/pci-host-generic.c >>>> +++ b/drivers/pci/host/pci-host-generic.c >>>> @@ -205,6 +205,12 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) >>>> return 0; >>>> } >>>> >>>> +static int gen_pci_fixup_irq_cb(struct pci_dev *dev, void *arg) >>>> +{ >>>> + pdev_fixup_irq(dev, pci_common_swizzle, of_irq_parse_and_map_pci); >>>> + return 0; >>>> +} >>>> + >>>> static int gen_pci_probe(struct platform_device *pdev) >>>> { >>>> int err; >>>> @@ -262,7 +268,7 @@ static int gen_pci_probe(struct platform_device *pdev) >>>> return -ENODEV; >>>> } >>>> >>>> - pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci); >>>> + pci_walk_bus(bus, gen_pci_fixup_irq_cb, NULL); >>> >>> Any chance we could put something in the core PCI code for this? I think >>> any host controller wanting to work with arm64 is potentially going to >>> run into the same problem. >> >> Good idea. >> >> I will move the walking code into setup-irq.c (the current home of >> pci_fixup_irqs()), and make it part of patch 1/6. > > For arm64 we could even compile out pci_fixup_irqs() since the IRQ mapping > is done in pcibios_add_device() through DT, I agree on CONFIG_ARM this > needs fixing though, so yes, make it common code and patch all host > controllers that make use of pci_fixup_irqs() since that's a common > issue as far as I can see (ARM pcibios works around it by having a > IRQ mapping function per bus and it provides an API to initialize > multiple host controllers at once and fixing up IRQs before devices > are added, still, for host controllers that do not use that > functionality - eg drivers/pci/host/pcie-designware.c - if there are > multiple host controllers instances we run into the same problem). Patching up other PCI bus drivers would be deferred to a different patch set, as I have no way to test it. > > Side note: do you have any PCI host controller where mapping IRQs through > of_irq_parse_and_map_pci() does not work ? If yes, why ? > The of_irq_parse_and_map_pci() call in arch/arm64/kernel/pci.c (pcibios_add_device) seems to work for me. I did send a patch recently to make it quite emitting erroneous error messages for PCI devices that have no irq pin. David Daney