On Wednesday 13 February 2013, Thomas Petazzoni wrote: > Dear Arnd Bergmann, > > On Tue, 12 Feb 2013 22:59:53 +0000, Arnd Bergmann wrote: > > > > <plat/pcie.h> is needed for a few PCIe functions shared with earlier > > > families of Marvell SoC. My plan is that once this PCI driver gets > > > accepted, I work on migrating the earlier Marvell SoC families to using > > > this PCI driver, and therefore those functions would ultimately move in > > > the driver in drivers/pci/host/, which would remove the <plat/pcie.h>. > > > > Hmm, although it is a bit unusual, I would actually propose duplicating > > that code for now, and merging a copy into the new driver right away. > > This gets rid of the header file dependency and lets you just delete > > the old file when the other platforms are converted. > > Hum, why not, but I would definitely prefer to wait for the conversion > of older platforms instead of duplicating this code. But if you feel > like it's the right solution, I'll do it. It's not something we do a lot, but in this case, I think it's better if it lets us avoid adding the platform specific include path, which I really want to avoid here. > > > The <mach/addr-map.h> is here to access the address decoding windows > > > allocation/free API. And for this, there is no other long term plan > > > than having an API provided by the platform code in arch/arm/, and used > > > by drivers. Some other drivers may have to use this API as well in the > > > future. > > > > This is harder to do, but I'm sure we can find a solution. At least > > the addr-map.c code has no other dependencies than the plat/addr-map.h > > header, so we are fairly free to move it elsehwere. > > The arch/arm/mach-mvebu/addr-map.c depends on > arch/arm/plat-orion/addr-map.c, so any change on this will affect > mach-kirkwood, mach-orion5x, mach-dove and mach-mv78xx0. As you can > see, we have to take into account the existing code, and I don't think > it's realistic to have a perfect solution immediately. Yes, I realize this. I was thinking we would move all at least the file from plat-orion, and the header file. I don't care much whether we also move the platform specific setup from mach-*/addr-map.c, it works either way. > This address decoding code will continue to change for two reasons: > > *) We are going to work on NOR/NAND support for mach-mvebu, and that > will also involve more interaction with the address decoding code. > > *) As we are moving the earlier Marvell SoC families (mach-kirkwood, > mach-orion5x, mach-dove, mach-mv78xx0) to the Device Tree, this > address decoding code will also evolve. > > mach-mvebu is not a standalone new platform like mach-highbank for > example, it relies on a lot of existing code from ealier platforms. It > is nice to share existing code, but it also means that cleanups and > refactoring take a lot more time. > > I think we need to leave time for all these platforms to gradually > converge and cleanup their infrastructure. It's not going to happen > overnight. We don't need to do a complete overhaul of that code right now, but if we agree on a place where it can go, then I think we should move it now as just one extra patch to get rid of the header dependency. In the worst case, moving just the header file to include/linux would work, too. > > > > > > * One described in the DT, from 0xC0000000 to 0xC00FFFFF which will be > > > used to create the address decoding windows for the I/O regions of > > > the different PCIe interfaces. The PCI I/O virtual address 0xffe00000 > > > will be mapped to those physical addresses. Those address decoding > > > windows are configured with the special "remap" mechanism that > > > ensures that if an access is made at 0xC0000000 + offset, it will > > > appear on the PCI bus as an I/O access at address "offset". > > > > Right, I got this from reading the code. Unfortunately the of_pci_process_ranges > > functions from your earlier patch makes this a little confusing as it > > marks this resource as IORESOURCE_IO when in reality it is the IORESOURCE_MEM > > resource that describes where in MMIO space the I/O window is. > > Indeed. So maybe I should mark this resource as being IORESOURCE_MEM > in the DT. The DT seems fine here, just the code that interprets it is a little unusual. Maybe you can change the calling convention of that function to pass the type of resource you want as an argument? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
