Re: [PATCH 05/11] PCI: epf-{mhi/test}: Move DMA initialization to EPC init callback

[Niklas Cassel <cassel@kernel.org>]

On Mon, Apr 22, 2024 at 01:25:21PM +0530, Manivannan Sadhasivam wrote:
> > 
> > What I would like is more consistency between the EPF drivers.
> > 
> > I guess an if-statement that skips the pci_epc_map_addr() in pci-epf-test
> > if using eDMA would make pci-epf-mhi and pci-epf-test most consistent.
> > 
> 
> Agree.


> > 1) Do we want to rely on the fact that hopefully none of the iATUs in the DWC
> > controller has configured a mapping that might mess things up for us?
> > I don't see why the PCI/DMA address of the remote buffer, supplied to
> > pci-epf-test via test_reg BAR, might not fall within the physical iATU window
> > on the local EP system. (As long as the PCI EPF driver has mapped any address
> > using pci_epc_map_addr().)
> > 
> > This is a big argument that EPF drivers running on a DWC-based EPC should
> > definitely NOT call pci_epc_map_addr() needlessly when using eDMA, as it
> > can be catastrophic. (pci-epf-test needs to be patched.)
> > 
> 
> Right. There is no need to do iATU translation for DMA. I avoid that in MHI
> driver.

There is no need for pci_epc_map_addr() when using DMA_SLAVE *for DWC-based
controllers*.

Are we certain that this will not break pci-epf-test for non DWC-based
controllers?


> > 2) Can we really assume that both pci-epf-test and pci-epf-mhi does not need
> > to call pci_epc_map_addr() when using a DMA_SLAVE DMA controller?
> > This seems to be designed only with DWC in mind. Other PCIe endpoint
> > controllers might require this.
> > (Yes, for DWC-based controllers, this definitely should be skipped, but EPF
> > drivers are supposed to be independent from a specific EPC.)
> > 
> 
> For TEST yes, but for MHI, no. In MHI, I kind of mix both iATU and DMA to ripe
> most of the performance (small vs big transactions). But for the TEST driver, it
> is fair to not call pci_epc_map_addr() when DMA_SLAVE is supported.

I agree that we should definitely skip pci_epc_map_addr() in pci-epf-test when
using DMA_SLAVE on DWC-based controllers, but I don't feel comfortable in
submitting a patch that does this unconditionally for pci-epf-test.c,
as I don't know how the DMA hardware in:
drivers/pci/controller/cadence/pcie-cadence-ep.c
drivers/pci/controller/pcie-rcar-ep.c
drivers/pci/controller/pcie-rockchip-ep.c

works, and I do not want to regress them.

I did suggest that DWC-based drivers could set a DMA_SLAVE_SKIP_MEM_MAP flag
or similar when registering the eDMA, which pci-epf-test then could check,
but I got no response if anoyone else thought that this was a good idea.


Kind regards,
Niklas