On Thu, Feb 27, 2014 at 04:06:24PM +0000, Arnd Bergmann wrote:
On Thursday 27 February 2014 13:09:59 Liviu Dudau wrote:
+/*
- PCI address space differs from physical memory address space
- */
+#define PCI_DMA_BUS_IS_PHYS (0)
+extern int isa_dma_bridge_buggy;
I got curious about isa_dma_bridge_buggy: apparently this is a quirk for some old x86 bridges. We don't have those on arm64, and we also don't have ISA_DMA_API, so just define this to (0).
OK.
+static inline int pci_domain_nr(struct pci_bus *bus) +{
- struct pci_host_bridge *bridge = to_pci_host_bridge(bus->bridge);
- if (bridge)
return bridge->domain_nr;- return 0;
+}
+static inline int pci_proc_domain(struct pci_bus *bus) +{
- return pci_domain_nr(bus);
+}
And this one I would change to always return '1': we can deal with domain numbers showing up in /procfs for all buses, since there is no legacy software to worry about.
Will do, thanks for reviewing this.
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c new file mode 100644 index 0000000..496df41 --- /dev/null +++ b/arch/arm64/kernel/pci.c @@ -0,1 +1,126 @@
Ok, this is nice and short. Let's see if we can reduce it to nothing ;-)
+/*
- Called after each bus is probed, but before its children are examined
- */
+void pcibios_fixup_bus(struct pci_bus *bus) +{
- struct pci_dev *dev;
- struct resource *res;
- int i;
- if (!pci_is_root_bus(bus)) {
pci_read_bridge_bases(bus);pci_bus_for_each_resource(bus, res, i) {if (!res || !res->flags || res->parent)continue;/** If we are going to reassign everything, we can* shrink the P2P resource to have zero size to* save space*/if (pci_has_flag(PCI_REASSIGN_ALL_RSRC)) {res->flags |= IORESOURCE_UNSET;res->start = 0;res->end = -1;continue;}}- }
- list_for_each_entry(dev, &bus->devices, bus_list) {
/* Ignore fully discovered devices */if (dev->is_added)continue;set_dev_node(&dev->dev, pcibus_to_node(dev->bus));/* Read default IRQs and fixup if necessary */dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);- }
+} +EXPORT_SYMBOL(pcibios_fixup_bus);
Shrinking the P2P resources I suppose is optional, but everything else is in fact needed for any DT based architecture. Could this be turned into a generic helper function in the PCI core that we can call from architecture code?
If you name it pci_generic_fixup_bus(), we can add a weak helper like:
void __weak pcibios_fixup_bus(struct pci_bus *bus) { pci_generic_fixup_bus(bus); }
for architectures like arm64 that don't actually need to do anything else.
Sure, it can be done. Don't know what is the policy for these kind of functions that are used by architectures, but I can try sending a patch that adds the weak implementations in the core PCI code.
+/*
- We don't have to worry about legacy ISA devices, so nothing to do here
- */
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
resource_size_t size, resource_size_t align)+{
- return ALIGN(res->start, align);
+} +EXPORT_SYMBOL(pcibios_align_resource);
Where did this come from?
From an internal version that Will posted. See, we do talk to each other ;)
The most common implementation seems to be
resource_size_t pcibios_align_resource(void *data, const struct resource *res, resource_size_t size, resource_size_t align) { return start; } EXPORT_SYMBOL(pcibios_align_resource);
if you don't have to worry about ISA devices. The ALIGN() part seems to be handled by __find_resource() already.
I'd say that should be made the default implementation in the PCI core.
I'm also pretty sure you don't need the EXPORT_SYMBOL, since the PCI core cannot be a loadable module (yet).
OK.
+int pcibios_enable_device(struct pci_dev *dev, int mask) +{
- return pci_enable_resources(dev, mask);
+}
+void pcibios_fixup_bridge_ranges(struct list_head *resources) +{ +}
These are clearly the right implementations, but they should be weak functions, too.
pcibios_enable_devices is already subject to a patch series from Bjorn that make the weak implementation do the right thing for arm64, so the final version will not contain this.
+#define IO_SPACE_PAGES ((IO_SPACE_LIMIT + 1) / PAGE_SIZE) +static DECLARE_BITMAP(pci_iospace, IO_SPACE_PAGES);
+unsigned long pci_ioremap_io(const struct resource *res, phys_addr_t phys_addr) +{
- unsigned long start, len, virt_start;
- int err;
- if (res->end > IO_SPACE_LIMIT)
return -EINVAL;- /*
* try finding free space for the whole size first,* fall back to 64K if not available*/- len = resource_size(res);
- start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
res->start / PAGE_SIZE, len / PAGE_SIZE, 0);- if (start == IO_SPACE_PAGES && len > SZ_64K) {
len = SZ_64K;start = 0;start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,start, len / PAGE_SIZE, 0);- }
- /* no 64K area found */
- if (start == IO_SPACE_PAGES)
return -ENOMEM;- /* ioremap physical aperture to virtual aperture */
- virt_start = start * PAGE_SIZE + (unsigned long)PCI_IOBASE;
- err = ioremap_page_range(virt_start, virt_start + len,
phys_addr, __pgprot(PROT_DEVICE_nGnRE));- if (err)
return err;- bitmap_set(pci_iospace, start, len / PAGE_SIZE);
- /* return io_offset */
- return start * PAGE_SIZE - res->start;
+}
Maybe this can become an optional helper function with a separate Kconfig symbol to enable it.
Probably need to find a different name for it as well when it moves into core, arm already has an externalised function with this name.
Best regards, Liviu
Arnd