Received-SPF: pass (google.com: domain of linux-kernel+bounces-153090-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Mon, 22 Apr 2024 11:30:09 +0200
From: Niklas Cassel <cassel@kernel.org>
To: Manivannan Sadhasivam <mani@kernel.org>
Cc: Vinod Koul <vkoul@kernel.org>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
	Rob Herring <robh@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Jingoo Han <jingoohan1@gmail.com>,
	Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
	linux-pci@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org, mhi@lists.linux.dev,
	linux-tegra@vger.kernel.org, Damien Le Moal <dlemoal@kernel.org>,
	Frank Li <Frank.Li@nxp.com>
Subject: Re: [PATCH 05/11] PCI: epf-{mhi/test}: Move DMA initialization to
 EPC init callback
Message-ID: <ZiYuIaX7ZV0exKMt@ryzen>
References: <20240314-pci-epf-rework-v1-0-6134e6c1d491@linaro.org>
 <20240314-pci-epf-rework-v1-5-6134e6c1d491@linaro.org>
 <Zf2tXgKo-gc3qy1D@ryzen>
 <20240326082636.GG9565@thinkpad>
 <ZgKsBoTvPWWhPO9e@ryzen>
 <20240327055457.GA2742@thinkpad>
 <ZgQFXsgqpeLbXMTb@ryzen>
 <ZgW6KB73Wh1X6911@matsya>
 <Zg5oeDzq5u3jmKIu@ryzen>
 <20240422075521.GB9775@thinkpad>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240422075521.GB9775@thinkpad>

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