On Fri, Jan 26, 2018 at 12:46 PM, Jim Quinlan [off-list ref] wrote:

On Fri, Jan 26, 2018 at 2:53 AM, Christoph Hellwig [off-list ref] wrote:

quoted

On Wed, Jan 24, 2018 at 12:04:58PM -0800, Florian Fainelli wrote:

quoted

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".

quoted

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