On Fri, Jan 26, 2018 at 12:46 PM, Jim Quinlan <jim2101024@xxxxxxxxx> wrote: > On Fri, Jan 26, 2018 at 2:53 AM, Christoph Hellwig <hch@xxxxxx> wrote: >> On Wed, Jan 24, 2018 at 12:04:58PM -0800, Florian Fainelli wrote: >>> This looks nicer than the current shape, but this still requires to >>> register a PCI fixup to override phys_to_dma() and dma_to_phys(), and it >>> would appear that you have dodged my question about how this is supposed >>> to fit with an entirely modular PCIe root complex driver? Are you >>> suggesting that we split the module into a built-in part and a modular part? >> >> I don't think entirely modular PCI root bridges should be a focal point >> for the design. If we happen to support them by other design choices: >> fine, but they should not be a priority. > > I disagree. If there is one common thing our customers request it is > the ability to remove (or control the insmod of after boot) the pcie > RC driver. I didn't add this in as a "nice-to-have". > >> >> That being said if we have core dma mapping or PCIe code that has >> a list of offsets and the root complex only populates them it should >> work just fine. > > I'm looking at arch/arm/include/asm/dma-mapping.h. In addition to > overriding dma_to_phsy() and phys_to_dma(), it looks like I may have > to define __arch_pfn_to_dma(), __arch_dma_to_pfn(), > __arch_dma_to_virt(), __arch_virt_to_dma(). Do you agree or is this > not necessary? If it is, this seems more intrusive than our > pcie-brcmstb-dma.c solution which doesn't require tentacles into > major include files and Kconfigs. > > Another issue is that our function wrappers -- depending upon whether > we are dealing with a pci device or not -- will have to possibly call > the actual ARM and ARM64 definitions of these functions, which have > been of course #ifdef'd out. This means that our code must contain > identical copies of these functions' code and that the code must > somehow be kept in sync. Do you see a solution to this? > > Jim Cristoph, Could you please respond to my comments? Even a negative response is better than none. The problem with doing what you suggested is with ARM -- ARM64 and MIPS relatively uncomplicated . With ARM, I have to define the aforementioned functions -- the only way of doing this is to define arch/arm/mach-bcm/include/mach/memory.h, and for that to be picked up we no longer can have CONFIG_ARCH_MULTIPLATFORM=y, which is an unacceptable cost to pay for just an unusual PCIe RC controller. Regarding my current submission -- you are right, SWIOTLB will not work for EPs that require it. However, we don't care about these devices, and can just bailout with EPs when the dma_mask is <= 0xffff_ffff or if swiotlb_force == SWIOTLB_FORCE. Note that this would only affect PCIe DMA. We also have no plan of using MIPS64. -- Jim
