Hi Gab, Am Mittwoch, den 19.08.2015, 16:34 +0000 schrieb Gabriele Paoloni: > Hi Lucas > > First of all many thanks for the quick reply, really appreciated > > > -----Original Message----- > > From: Lucas Stach [mailto:l.stach@xxxxxxxxxxxxxx] > > Sent: Wednesday, August 19, 2015 4:37 PM > > To: Gabriele Paoloni > > Cc: Wangzhou (B); Bjorn Helgaas; jingoohan1@xxxxxxxxx; Pratyush Anand; > > Arnd Bergmann; linux@xxxxxxxxxxxxxxxx; thomas.petazzoni@free- > > electrons.com; lorenzo.pieralisi@xxxxxxx; james.morse@xxxxxxx; > > Liviu.Dudau@xxxxxxx; jason@xxxxxxxxxxxxxx; robh@xxxxxxxxxx; linux- > > pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > > devicetree@xxxxxxxxxxxxxxx; Yuanzhichang; Zhudacai; zhangjukuo; > > qiuzhenfa; liudongdong (C); qiujiang; xuwei (O); Liguozhu (Kenneth) > > Subject: Re: [PATCH v7 3/6] PCI: designware: Add ARM64 support > > > > Hi Gab, > > > > Am Mittwoch, den 19.08.2015, 15:16 +0000 schrieb Gabriele Paoloni: > > > Hi Lucas > > > > > > I have rewritten the patch to take into account multiple controllers. > > > > > > As you can see now there is a static var in dw_pcie_host_init() that > > tracks > > > the bus numbers used. > > > > This is wrong. The DT specifies the valid bus number range. You can not > > just assign the next free bus number to the root bus. > > I think this is what is being done in > http://lxr.free-electrons.com/source/arch/arm/kernel/bios32.c#L495 > and currently designware assigns the root bus number in > http://lxr.free-electrons.com/source/drivers/pci/host/pcie-designware.c#L730 > You found the right spot. It works a bit different than I remember, but is less broken than your current code. :) It actually assigns the next instance a root bus number behind the current instances bus range. Please note the difference to your code which assigns the next free bus number, which may still lay within the current instances bus range. > > In general I agree with you but if you look at all the current drivers > based on designware none of them define the "bus-range" dtb property. > Therefore doing as you say would break the current driver when we have > multiple controllers...am I right? > The current _code_ works fine for multiple controllers. It's just that you must define the bus range property in the DTB if you want to enable multiple instances of one controller and support a kernel without PCI domains support. As all current implementations have only a single instance of the controller per SoC, or depend on PCI domain support, it's totally fine for them to not define the bus ranges property, as it's an optional property and it is well defined how things behave if it is absent. We absolutely need to keep that specified behavior. > If that is the case in order to fix this in the way you say I would need > to assign "bus-range" for all the PCIe drivers with multiple controllers: > in this case I would split the default range evenly (that is, if we have > two controllers I would define "bus-range" 0-127 and 128-255) > > If you think this solution is ok I can go for it. My only doubt was about > touching other vendors DTBs.... > You don't need to touch any of the current DTBs, as they are fine with the default bus range behavior. You need to keep the specified behavior of the bus range property with the new code. Regards, Lucas > > > > > It is perfectly valid to have a bus range of 0x00-0x10 assigned to one > > instance and 0x50-0xff to the next instance. Additional with PCIe > > hotplug you may not use the full range of the bus numbers on one > > instance at the first scan, but only later populate more buses when > > more > > bridges are added to the tree. > > > > > Drivers that do not specify the bus range in the DTB set pp- > > >root_bus_nr = DW_ROOT_NR_UNDEFINED. > > > Designware will check if the flag is set and will use the automatic > > bus range > > > assignment. > > > > No, please lets get rid of this assignment altogether. The glue drivers > > have no business in assigning the bus range. Please remove the > > pp->root_bus_nr assignment from all the glue drivers. > > > > "bus range" is a generic DW PCIe property, so just parse the root bus > > number from the DT, it is handled the same way for all the DW based > > PCIe > > drivers. The bindings specifies that if the bus range property is > > missing the range is 0x00-0xff, so you can default to 0 as the root bus > > number in that case. > > > > Also I would think this conversion warrants a patch on its own and > > should not be mixed in the ARM64 support patch. > > > > Regards, > > Lucas > > -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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
