[PATCH v2 1/2] PCI: generic: remove dependency on hw_pci
Suravee Suthikulanit
suravee.suthikulpanit at amd.com
Wed May 6 20:32:48 PDT 2015
On 5/6/2015 10:18 AM, Bjorn Helgaas wrote:
> On Wed, May 6, 2015 at 9:18 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi at arm.com> 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 <jchandra at broadcom.com>
>>>> ---
>>>> 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?
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 *),
int (*)(const struct pci_dev *, u8, u8));
-------- END PATCH ----
Thanks,
Suravee
More information about the linux-arm-kernel
mailing list