On Wed, Jul 02, 2014 at 06:23:55PM +0100, Liviu Dudau wrote: > On Wed, Jul 02, 2014 at 12:22:30PM +0100, Will Deacon wrote: > > On Tue, Jul 01, 2014 at 07:43:33PM +0100, Liviu Dudau wrote: > > > Several platforms use a rather generic version of parsing > > > the device tree to find the host bridge ranges. Move the common code > > > into the generic PCI code and use it to create a pci_host_bridge > > > structure that can be used by arch code. > > > > > > Based on early attempts by Andrew Murray to unify the code. > > > Used powerpc and microblaze PCI code as starting point. > > > > I just had a quick look at this to see how it differs from the parsing in > > pci-host-generic.c and there a few small differences worth discussing. [...] > > > + } > > > + > > > + /* Apply architecture specific fixups for the ranges */ > > > + return pcibios_fixup_bridge_ranges(resources); > > > > I currently mandate at least one non-prefetchable resource in the > > device-tree. Should I simply drop this restriction, or do I have to somehow > > hook this into the pcibios callback? > > Don't think I understand why you need at least one non-prefetcheable resource > but if you want to mandate that then the pcibios_fixup_bridge_ranges() would > be the place to put that check. I think it was Arnd's idea at the time: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-February/232225.html and it's probably worth keeping if possible (just to avoid changes to the behaviour of the existing driver). However, that means I already need a host-controller callback via pcibios_fixup_bridge_ranges... > > > + err = of_pci_parse_bus_range(parent->of_node, bus_range); > > > + if (err) { > > > + dev_info(parent, "No bus range for %s, using default [0-255]\n", > > > + parent->of_node->full_name); > > > + bus_range->start = 0; > > > + bus_range->end = 255; > > > + bus_range->flags = IORESOURCE_BUS; > > > > What about bus_range->name? > > Don't know! Is anyone using it? I guess /proc/iomem prints it out? I set it in my current driver, if you want to take a look. > > > > > + } > > > + busno = bus_range->start; > > > + pci_add_resource(&res, bus_range); > > > > I currently truncate the bus range to fit inside the Configuration Space > > window I have (in the reg property). How can I continue to do that with this > > patch? > > Not easily. Unless I add an argument to this function that will allow you to > pass in the max number for the bus range, then the code becomes: > > + err = of_pci_parse_bus_range(parent->of_node, bus_range); > + if (err) { > + dev_info(parent, "No bus range for %s, using default [0-%d]\n", > + parent->of_node->full_name, max_range); > + bus_range->start = 0; > + bus_range->end = max_range; > + bus_range->flags = IORESOURCE_BUS; > + } else { > + if (bus_range->end > bus_range->start + max_range) { > + bus_range->end = bus_range->start + max_range; > + } > + } > > Or something like that. Again, take a look at my driver (it's in mainline now) to see how I deal with this. Will -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
