From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753597AbbJGUIw (ORCPT ); Wed, 7 Oct 2015 16:08:52 -0400 Received: from mail-bl2on0094.outbound.protection.outlook.com ([65.55.169.94]:53701 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751667AbbJGUIt (ORCPT ); Wed, 7 Oct 2015 16:08:49 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=David.Daney@caviumnetworks.com; Message-ID: <56157BC8.3070406@caviumnetworks.com> Date: Wed, 7 Oct 2015 13:08:40 -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: Bjorn Helgaas CC: David Daney , , Bjorn Helgaas , , Will Deacon , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , , , Marc Zyngier , David Daney Subject: Re: [PATCH v4 1/5] PCI: Add pci_bus_fixup_irqs(). References: <1443811443-18878-1-git-send-email-ddaney.cavm@gmail.com> <1443811443-18878-2-git-send-email-ddaney.cavm@gmail.com> <20151007194426.GF27633@localhost> In-Reply-To: <20151007194426.GF27633@localhost> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [64.2.3.194] X-ClientProxiedBy: SN1PR07CA0017.namprd07.prod.outlook.com (25.162.170.155) To CY1PR07MB2133.namprd07.prod.outlook.com (25.164.112.11) X-Microsoft-Exchange-Diagnostics: 1;CY1PR07MB2133;2:Jul7RuO10hkhQ9ICqpMuM0Xwvs6yp52SJDJNJpbEZhUzgYkBnSed3hdzIQz07XaKy4bKlnVwiQ00JVUh8EJpXanu/moLf+wewCbcIYhyGf2IUHCroGMqS4Nq02zdR3V/e6HJT0gBXW6smLncn6Cq2B8SrwbeJJSOAidfcGnjhPw=;3:IUmZuDMJ+O8x7XioekKhsH4T3FISITSCaJldb/pu5lO6SEigdKcNQN5mXoRSs9oUftDcN5f9a/WjNDkE5Ocn3QLVoYjzPVG1fDN1nHrVclB/iO3aS9fOpbk4PPaxewR6Fq8bIxaYzyz1h3B+wHsKIw==;25:OfHMHDeI37qVJEfvy95m2Rj6PSiuZzNJl0pt/bqzjwrSlcksMESSAPlDtqfxq2u9VbBMxx8EvazbaeadzCwuenNe5ZWTRjOXYObpXcEo0ume6soj/26MpTcqOvLVWg2x3tW2xB8r3PbdOAKsyDbtTI0Qsen66ZPfzqzRLbvu68WR41rOO/9LNMyQoP78TS8psDD/r0UCnEwpJlIn6dj8rJ22lSlvfDCSDyO1Jl9Mo28xb+HfMzGjs1vSuMQkkNNF1H7601FOgU7eID5kBYIm6g== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CY1PR07MB2133; X-Microsoft-Exchange-Diagnostics: 1;CY1PR07MB2133;20:cLANlIr4LvMGQnY6QA2oLU7JUrG4NDI9albpItEK5SVIB8eQjpwVATII9im2W52yDJgJ7POjvXAHso3Rgu255A1EaRVpjZ83rrAJ00n8mM1taH4kHZt//1DwaeTDT2VtVMbCRPGmGbjWXSWogZXG48ulZ5rCiyZzfgJahEZ39WneKNyG8WqZN5YLMK5pr8JPkB+J5IqI7uSzqtct4jFXB/TQASePH72sQF9TyO8JcbT3D+xMpoPctuWizz4ZGLYlqMelwMWFa7YEZXxx3jQ42mqyqS/gU8ESILwZ4o9dMepe3/CVfu0IkvLbtiWwzpUc3bIk5s3yzYdCKiRgH2fRROxdcf0+5ooUAlVzAnKcwyCjy1YEY5a6O2j9iQ8lbtVVojPZcC2xVPwdKfmSzmivDCmRGZV/+RssBHIpEb4p+LKtvCVH6//mZuEV+83iiPAL+D8h5Zil/h6LM7MQXRPUCh5vasGukHAw04DjOS6LK07DUHPU6y3HRfOgdGUDvElJLp1Phs/+f66wjCXLbRJOydAw2x3l1HLT+fw6vB7ipTcO6nvAvs2Y8FVlIStLZqonTnB1LUwygZz9tt4IcZ1q6pwfJ8n8LzOhVRFx/KUy0Mk= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(520078)(5005006)(8121501046)(3002001);SRVR:CY1PR07MB2133;BCL:0;PCL:0;RULEID:;SRVR:CY1PR07MB2133; X-Microsoft-Exchange-Diagnostics: 1;CY1PR07MB2133;4:JXqXz2QPwlTNmwlCdBk9lY1eCz1ysifcfAo3nnoEc4srrZ4aY4n2GDEC4iO8YgWrgtaQqBIrw9iwwCkqau5HUWRVLYctEnFyh/4XUE9rTcibDwABaqzgrFa/2NTI35Zb9rJf30AylPKgIGAkrH5Il4Lhv48ier93vllNshZatniRTitYsnVTMWDRJIAmc4RsgX3t/aNheikVOZo7pVP955taLs5YOJcBUeYPyzAPsqSGhXQi52ZvR5YS+jKrILZXObW6uUUPc337T1y2J8P/ZYL6DKhlYCgEfpylhLjHA8+CILfSugfToLV3NdMOqD5KOY0Wr+LwbaaCoXNldpGeI7abaE2J6kxdQuboSTegplI= X-Forefront-PRVS: 0722981D2A X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6009001)(189002)(479174004)(24454002)(377454003)(199003)(92566002)(59896002)(2950100001)(5008740100001)(101416001)(4001350100001)(40100003)(64126003)(23756003)(54356999)(36756003)(19580395003)(65816999)(5007970100001)(77096005)(19580405001)(122386002)(50466002)(97736004)(65806001)(189998001)(69596002)(64706001)(80316001)(42186005)(105586002)(33656002)(87266999)(81156007)(66066001)(65956001)(5001960100002)(50986999)(53416004)(87976001)(76176999)(99136001)(5004730100002)(110136002)(83506001)(106356001)(47776003)(46102003);DIR:OUT;SFP:1101;SCL:1;SRVR:CY1PR07MB2133;H:dl.caveonetworks.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;CY1PR07MB2133;23:pHWiam9FybABbr/++O4MtbmHGANP9o5vI8umvP+?= =?iso-8859-1?Q?LCPK/GJ+a6YTgNkzXjdJFqHzWyvKPJr/V2kQw3466tRVZTnMhuGysZCcMo?= =?iso-8859-1?Q?U/AZ4miNQftp4i2y3MdNcwguqpi9tqWX5uclQNZhQOvbC4xBTwJpmMZEC1?= =?iso-8859-1?Q?OO5fPg4s9raVbXeKvS/MDIWdAYB+UeRXRvQoADVoGUzOZTcbkxKo0AgfOP?= =?iso-8859-1?Q?nLDUUNUqLiZW2a2mBoFS1dIuAYDdh8IgN4BXPKkadBJwtapjzQJUXsEvZ2?= =?iso-8859-1?Q?56K3tYqk9D+pZJE7VjBjc6y3uxsAtQZ8w/hB7mQFPrJ/2UKBMvoDlY8QMM?= =?iso-8859-1?Q?JSeHQPBvGrJa0AP7XBd/uBKe2UVJ1JW4Fp8AKpXWVWeT/GiiPPPSvErkV/?= =?iso-8859-1?Q?gDc5rseH+m8L6OkctJxW0psmAShnFh5otgjHiWy9MvcDAnb1nFBuUNnsFb?= =?iso-8859-1?Q?jrTTX8wGJA2R7S6PRMlLgu7J4s0mcH66lDhfRr+dH2ejVn4bJDIpaJRyTa?= =?iso-8859-1?Q?NWhhiERXBU/IgpZNafFZsFBZzLvKROgnoLogEwc44wVmhmd67DSslh1b0A?= =?iso-8859-1?Q?gNrRw9170MPy/Tw5fIFWjVQ5/AVrFDFlOogsV98PA5ushJd6wkW3hJ82MY?= =?iso-8859-1?Q?pB4v1Va8RKkd83OrFti6suercChkI64/CrJE2U2YP4cezGdJqbzBFd45eO?= =?iso-8859-1?Q?KIR/0P1PvV9xv9CIOgaY+T3ZVFfzJi2uUWC1Wlks9CZ2WVNklVe8GoZS5B?= =?iso-8859-1?Q?AR1rKlVCs2fiY7ZHXEzsA+C+Hf8o/bCuQCwykUbW2rN8lDv4cFtmoPVy56?= =?iso-8859-1?Q?d1eBVwjzIDrDnikpvFPxgy3xpWAo+WKDBMOmIBYuzceSBG/mUbOtfGrkQb?= =?iso-8859-1?Q?aNKv6mp/rlcnGE7weN3X8N8wVPoyRr7Yk1PijIqnCk7dO8CvE6oIIiWT1t?= =?iso-8859-1?Q?HHaBUmgz3N78Ph/wQ5zaxgjxQgvesbKhVThcZK6yp/rfONLvcnaegMCfah?= =?iso-8859-1?Q?KFEddnXheTbmODk6kVGiExFj1EbUs+JeONZTubMy0k9teYwOaryd/tmOQ3?= =?iso-8859-1?Q?4KAwRVwBfAABZ4/Ay04QmLU9hRaREDdH1zJSnjgy4Uf9cuZV1rQtPgayCj?= =?iso-8859-1?Q?nHZoOVVcGahqi6T5GwrbgOUvTHjApN22F53+xipK/4yHCCnxuXQ2HAfd5F?= =?iso-8859-1?Q?/TpuOt+biGJcg8GxsEakZUjeqSe9+LE7g5EsdFtshwUWguyHr0iKpZUQyd?= =?iso-8859-1?Q?sVedCO2/PWoPIIABThKH8hUq+ZkFZRfLUIoPiTLLy7YLvA/fa3DE6LV80F?= =?iso-8859-1?Q?ECdw97nSNkOnvI32XsCGU+J39nwVVyz0ddrGYclm4piFQ=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;CY1PR07MB2133;5:tyy2x5ztsB/iowz00KSCBys/De058XCqrGkxiM0qzw0LvfrhCE1GOyklcwBZe8CJ1Q2iumpCLyyJXB0P6hD6oMs65zRxxOj8aZuMvypTYLkFjNVD4MDAIwzfCtCK4j5Fkke628Vj9BDSynsbLPqDSQ==;24:xMTaN8264IY63s7pca6ixR1OMDC4GyjHfvlxmo2Q7laXyZJfP2faHzzgQ+xQu5DJCwmYcwr98UPj7oaXAHBIa2XlxoAZjU0gsO6x2WkXo+k=;20:ecnRMCd8V2edvA6r6A8NZuFqHqylUvE6zSlO/1dU1JumCxNl56OwLhjwz41hrM4ZXYGKjULBqoDRB6DyrKT7fQ== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Oct 2015 20:08:43.3117 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR07MB2133 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/07/2015 12:44 PM, Bjorn Helgaas wrote: > Hi David, > > On Fri, Oct 02, 2015 at 11:43:59AM -0700, David Daney wrote: >> From: David Daney >> >> pci_bus_fixup_irqs() works like pci_fixup_irqs(), except it only does >> the fixups for devices on the specified bus. >> >> Follow-on patch will use the new function. >> >> Signed-off-by: David Daney >> --- >> No change from v2. >> >> drivers/pci/setup-irq.c | 30 ++++++++++++++++++++++++++++++ >> include/linux/pci.h | 4 ++++ >> 2 files changed, 34 insertions(+) >> >> diff --git a/drivers/pci/setup-irq.c b/drivers/pci/setup-irq.c >> index 95c225b..189ad17 100644 >> --- a/drivers/pci/setup-irq.c >> +++ b/drivers/pci/setup-irq.c >> @@ -66,3 +66,33 @@ void pci_fixup_irqs(u8 (*swizzle)(struct pci_dev *, u8 *), >> pdev_fixup_irq(dev, swizzle, map_irq); >> } >> EXPORT_SYMBOL_GPL(pci_fixup_irqs); >> + >> +struct pci_bus_fixup_cb_info { >> + u8 (*swizzle)(struct pci_dev *, u8 *); >> + int (*map_irq)(const struct pci_dev *, u8, u8); >> +}; >> + >> +static int pci_bus_fixup_irq_cb(struct pci_dev *dev, void *arg) >> +{ >> + struct pci_bus_fixup_cb_info *info = arg; >> + >> + pdev_fixup_irq(dev, info->swizzle, info->map_irq); >> + return 0; >> +} >> + >> +/* >> + * Fixup the irqs only for devices on the given bus using supplied >> + * swizzle and map_irq function pointers >> + */ >> +void pci_bus_fixup_irqs(struct pci_bus *bus, >> + u8 (*swizzle)(struct pci_dev *, u8 *), >> + int (*map_irq)(const struct pci_dev *, u8, u8)) >> +{ >> + struct pci_bus_fixup_cb_info info; >> + >> + info.swizzle = swizzle; >> + info.map_irq = map_irq; >> + pci_walk_bus(bus, pci_bus_fixup_irq_cb, &info); > > I don't like the existing pci_fixup_irqs(), so by transitivity, I > don't like pci_bus_fixup_irqs() either. We are in agreement with respect to this point. > The problem is that in both > cases this is a one-time pass over the tree, so we don't handle > hot-added devices correctly. > > I think we need to get rid of pci_fixup_irqs() and somehow integrate > it into the pci_device_add() path, where it would be done once for > every device we enumerate. I also agree with this point. > If we did that, I don't think you would > need to add pci_bus_fixup_irqs(), would you? Nope. However, such a change is essentially untestable by me. So, I didn't attempt it. pci_fixup_irqs() is used by alpha, arm, m68k, mips, sh, sparc, tile, unicore32 and other things as well. If the core pci_device_add() code were to suddenly start doing the fixup, there would be the potential to break all these things I cannot test. The new pci_bus_fixup_irqs() is really an optimization so that if we have multiple buses created by pci-host-generic.c, that we only iterate over each device once. I believe that pci-host-generic.c would still operate without these patches 1/5 and 2/5, and could test that if you are OK with the remaining three patches. Or we could merge all 5 and live a while longer with the ugliness that is already there. David Daney