From patchwork Thu May 7 03:32:48 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Suravee Suthikulpanit X-Patchwork-Id: 6353581 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 8F3FB9F32E for ; Thu, 7 May 2015 03:37:25 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 59188203A0 for ; Thu, 7 May 2015 03:37:24 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2785620396 for ; Thu, 7 May 2015 03:37:23 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YqCZ5-0006Dn-KP; Thu, 07 May 2015 03:33:35 +0000 Received: from mail-bn1on0145.outbound.protection.outlook.com ([157.56.110.145] helo=na01-bn1-obe.outbound.protection.outlook.com) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YqCZ2-0006BI-30 for linux-arm-kernel@lists.infradead.org; Thu, 07 May 2015 03:33:33 +0000 Received: from BN3PR02MB1112.namprd02.prod.outlook.com (25.162.168.142) by BN3PR02MB1190.namprd02.prod.outlook.com (25.162.168.16) with Microsoft SMTP Server (TLS) id 15.1.154.19; Thu, 7 May 2015 03:33:09 +0000 Received: from CY1PR0201CA0001.namprd02.prod.outlook.com (25.163.30.139) by BN3PR02MB1112.namprd02.prod.outlook.com (25.162.168.142) with Microsoft SMTP Server (TLS) id 15.1.154.19; Thu, 7 May 2015 03:33:05 +0000 Received: from BY2FFO11FD046.protection.gbl (2a01:111:f400:7c0c::153) by CY1PR0201CA0001.outlook.office365.com (2a01:111:e400:58b9::11) with Microsoft SMTP Server (TLS) id 15.1.160.16 via Frontend Transport; Thu, 7 May 2015 03:33:05 +0000 Authentication-Results: spf=none (sender IP is 165.204.84.221) smtp.mailfrom=amd.com; arm.com; dkim=none (message not signed) header.d=none; Received-SPF: None (protection.outlook.com: amd.com does not designate permitted sender hosts) Received: from atltwp01.amd.com (165.204.84.221) by BY2FFO11FD046.mail.protection.outlook.com (10.1.15.170) with Microsoft SMTP Server id 15.1.160.8 via Frontend Transport; Thu, 7 May 2015 03:33:04 +0000 X-WSS-ID: 0NNYN72-07-2C7-02 X-M-MSG: Received: from satlvexedge02.amd.com (satlvexedge02.amd.com [10.177.96.29]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by atltwp01.amd.com (Axway MailGate 5.3.1) with ESMTPS id 2355D12C0048; Wed, 6 May 2015 23:33:01 -0400 (EDT) Received: from SATLEXDAG03.amd.com (10.181.40.7) by SATLVEXEDGE02.amd.com (10.177.96.29) with Microsoft SMTP Server (TLS) id 14.3.195.1; Wed, 6 May 2015 22:33:32 -0500 Received: from [127.0.0.1] (10.180.168.240) by satlexdag03.amd.com (10.181.40.7) with Microsoft SMTP Server id 14.3.195.1; Wed, 6 May 2015 23:33:00 -0400 Message-ID: <554ADCE0.8020603@amd.com> Date: Wed, 6 May 2015 22:32:48 -0500 From: Suravee Suthikulanit User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Bjorn Helgaas , Lorenzo Pieralisi Subject: Re: [PATCH v2 1/2] PCI: generic: remove dependency on hw_pci References: <1430791333-26013-1-git-send-email-jchandra@broadcom.com> <20150505155346.GJ1550@arm.com> <20150506141859.GC26028@red-moon> In-Reply-To: X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:165.204.84.221; CTRY:US; IPV:NLI; EFV:NLI; SFV:NSPM; SFS:(10019020)(6009001)(428002)(377454003)(24454002)(189002)(164054003)(53754006)(479174004)(51704005)(199003)(36756003)(76176999)(105586002)(15975445007)(5001770100001)(87936001)(62966003)(65816999)(50466002)(101416001)(47776003)(46102003)(2950100001)(93886004)(120886001)(87266999)(77156002)(4001350100001)(19580395003)(23676002)(77096005)(83506001)(33656002)(19580405001)(92566002)(106466001)(65956001)(86362001)(50986999)(189998001)(54356999)(41533002); DIR:OUT; SFP:1102; SCL:1; SRVR:BN3PR02MB1112; H:atltwp01.amd.com; FPR:; SPF:None; MLV:sfv; MX:1; A:1; LANG:en; X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:; SRVR:BN3PR02MB1112; UriScan:; BCL:0; PCL:0; RULEID:; SRVR:BN3PR02MB1190; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(601004)(5005006)(3002001); SRVR:BN3PR02MB1112; BCL:0; PCL:0; RULEID:; SRVR:BN3PR02MB1112; X-Forefront-PRVS: 056929CBB8 X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 May 2015 03:33:04.1094 (UTC) X-MS-Exchange-CrossTenant-Id: fde4dada-be84-483f-92cc-e026cbee8e96 X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=fde4dada-be84-483f-92cc-e026cbee8e96; Ip=[165.204.84.221]; Helo=[atltwp01.amd.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR02MB1112 X-OriginatorOrg: amd4.onmicrosoft.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150506_203332_416505_4576EE49 X-CRM114-Status: GOOD ( 28.35 ) X-Spam-Score: -0.0 (/) Cc: Jayachandran C , Arnd Bergmann , Will Deacon , Ming Lei , Liviu Dudau , "linux-pci@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 5/6/2015 10:18 AM, Bjorn Helgaas wrote: > On Wed, May 6, 2015 at 9:18 AM, Lorenzo Pieralisi > wrote: >> On Tue, May 05, 2015 at 04:53:46PM +0100, Will Deacon wrote: >>> On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote: >>>> The current code in pci-host-generic.c uses pci_common_init_dev() >>>> from the arch/arm/ to do a part of the PCI initialization, and this >>>> prevents it from being used on arm64. >>>> >>>> The initialization done by pci_common_init_dev() that is really >>>> needed by pci-host-generic.c can be done in the same file without >>>> using the hw_pci API of ARM. >>>> >>>> The ARM platform requires a pci_sys_data as sysdata for the PCI bus, >>>> this is be handled by setting up 'struct gen_pci' to embed a >>>> pci_sys_data variable as the first element on the ARM platform. >>>> >>>> Signed-off-by: Jayachandran C >>>> --- >>>> Here's v2 of the patches, this enables use of pci-host-generic on >>>> arm64. >>>> >>>> This has been tested on both qemu and fast model for arm64, and on >>>> qemu for arm32. >>>> >>>> v1->v2 >>>> - Address comments from Arnd Bergmann and Lorenzo Pieralisi >>>> - move contents of gen_pci_init to gen_pci_probe >>>> - assign resources only when !probe_only >>>> - tested on ARM32 with qemu option -M virt >>> >>> I tried this with an arm64 kernel running under kvmtool, but I get the >>> following errors (a 32-bit ARM kernel does seem to work): >>> >>> PCI host bridge /pci ranges: >>> IO 0x00000000..0x0000ffff -> 0x00000000 >>> MEM 0x41000000..0x7fffffff -> 0x41000000 >>> pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 >>> pci_bus 0000:00: root bus resource [bus 00-01] >>> pci_bus 0000:00: root bus resource [io 0x0000-0xffff] >>> pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] >>> pci_bus 0000:00: scanning bus >>> pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 >>> pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] >>> pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff] >>> pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] >>> pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 >>> pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] >>> pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff] >>> pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] >>> pci_bus 0000:00: fixups for bus >>> pci_bus 0000:00: bus scan returning with max=00 >>> pci 0000:00:00.0: fixup irq: got 10 >>> pci 0000:00:00.0: assigning IRQ 10 >>> pci 0000:00:01.0: fixup irq: got 11 >>> pci 0000:00:01.0: assigning IRQ 11 >>> virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed >>> virtio-pci: probe of 0000:00:00.0 failed with error -22 >>> virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed >>> virtio-pci: probe of 0000:00:01.0 failed with error -22 >> >> Ok, had a further look. >> >> Referring to this thread: >> >> https://lkml.org/lkml/2014/9/29/557 >> >> By looking at other architectures code, resources should be claimed >> (ie requested) even when PCI_PROBE_ONLY is set. Alpha, Sparc and PowerPC >> seem to do that, in slightly different fashions. >> >> I do not think, as Bjorn mentioned, that PCI_PROBE_ONLY should be used >> to prevent enabling resources through a PCI command, which is what >> pci_enable_resources does. >> >> What we can do, is providing a generic PCI layer API that allows claiming >> resources for a specific PCI bus, something similar if not identical >> to what is done on alpha: >> >> arch/alpha/kernel/pci.c pcibios_claim_one_bus() >> >> that is not alpha specific at all. That way, we can use the API to claim >> bus resources instead of assigning them on PCI_PROBE_ONLY (I *think* >> that alpha calls pci_assign_unassigned_resources() even if >> PCI_PROBE_ONLY is set, it should be safe since resources are claimed >> first so IIUC the PCI layer would revert to FW BAR configuration on >> assignment failure). >> >> Bjorn, any opinion on this ? Putting together a patch is easy when >> we agree on the solution. > > I would like claiming resources, i.e., pci_claim_resource(), to happen > in the core instead of in arch code because it's not inherently > arch-specific. I don't think it should depend on PCI_PROBE_ONLY. > > Bjorn > Hi All, I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the resource not claimed issue (no surprise here). So, I tried porting the pcibios_claim_one_bus() from arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please see example patch below.) The additional while loop is needed in the pci_claim_resource() since I need to reference back to the resource in the root bus, which are defined from the DT node. Does this sounds like a reasonable approach? int (*)(const struct pci_dev *, u8, u8)); -------- END PATCH ---- Thanks, Suravee diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c index e9cc559..0dfa23d 100644 --- a/drivers/pci/host/pci-host-generic.c +++ b/drivers/pci/host/pci-host-generic.c @@ -261,7 +261,10 @@ static int gen_pci_probe(struct platform_device *pdev) if (!pci_has_flag(PCI_PROBE_ONLY)) { pci_bus_size_bridges(bus); pci_bus_assign_resources(bus); + } else { + pci_claim_one_bus(bus); } + pci_bus_add_devices(bus); /* Configure PCI Express settings */ diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c index 232f925..d4b43b3 100644 --- a/drivers/pci/setup-res.c +++ b/drivers/pci/setup-res.c @@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int resource) { struct resource *res = &dev->resource[resource]; struct resource *root, *conflict; + struct pci_dev *pdev = dev; if (res->flags & IORESOURCE_UNSET) { dev_info(&dev->dev, "can't claim BAR %d %pR: no address assigned\n", @@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int resource) return -EINVAL; } - root = pci_find_parent_resource(dev, res); + while (pdev) { + root = pci_find_parent_resource(pdev, res); + if (root) + break; + + if (pci_has_flag(PCI_PROBE_ONLY) && + !pci_is_root_bus(pdev->bus)) + pdev = pdev->bus->self; + else + break; + } + if (!root) { dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible bridge window\n", resource, res); @@ -136,6 +148,36 @@ int pci_claim_resource(struct pci_dev *dev, int resource) } EXPORT_SYMBOL(pci_claim_resource); +void pci_claim_one_bus(struct pci_bus *b) +{ + struct pci_dev *pdev; + struct pci_bus *child_bus; + + list_for_each_entry(pdev, &b->devices, bus_list) { + int i; + + for (i = 0; i < PCI_NUM_RESOURCES; i++) { + struct resource *r = &pdev->resource[i]; + + if (r->parent || !r->start || !r->flags) + continue; + + if (pci_has_flag(PCI_PROBE_ONLY) || + (r->flags & IORESOURCE_PCI_FIXED)) { + if (pci_claim_resource(pdev, i) == 0) + continue; + + pci_claim_bridge_resource(pdev, i); + } + } + } + + list_for_each_entry(child_bus, &b->children, node) { + pci_claim_one_bus(child_bus); + } +} +EXPORT_SYMBOL(pci_claim_one_bus); + void pci_disable_bridge_window(struct pci_dev *dev) { dev_info(&dev->dev, "disabling bridge mem windows\n"); diff --git a/include/linux/pci.h b/include/linux/pci.h index 353db8d..b59ad4b 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1085,6 +1085,7 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge); void pci_assign_unassigned_bus_resources(struct pci_bus *bus); void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus); void pdev_enable_device(struct pci_dev *); +void pci_claim_one_bus(struct pci_bus *b); int pci_enable_resources(struct pci_dev *, int mask); void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),