Received-SPF: pass (google.com: domain of linux-kernel+bounces-118847-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 26 Mar 2024 12:05:42 +0100
From: Niklas Cassel <cassel@kernel.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: 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
Subject: Re: [PATCH 05/11] PCI: epf-{mhi/test}: Move DMA initialization to
 EPC init callback
Message-ID: <ZgKsBoTvPWWhPO9e@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>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240326082636.GG9565@thinkpad>

On Tue, Mar 26, 2024 at 01:56:36PM +0530, Manivannan Sadhasivam wrote:
> On Fri, Mar 22, 2024 at 05:10:06PM +0100, Niklas Cassel wrote:
> > On Thu, Mar 14, 2024 at 08:53:44PM +0530, Manivannan Sadhasivam wrote:
> > > To maintain uniformity across EPF drivers, let's move the DMA
> > > initialization to EPC init callback. This will also allow us to deinit DMA
> > > during PERST# assert in the further commits.
> > > 
> > > For EPC drivers without PERST#, DMA deinit will only happen during driver
> > > unbind.
> > > 
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > ---
> > 
> > Reviewed-by: Niklas Cassel <cassel@kernel.org>
> > 
> > 
> > For the record, I was debugging a problem related to EPF DMA recently
> > and was dumping the DMA mask for the struct device of the epf driver.
> > I was a bit confused to see it as 32-bits, even though the EPC driver
> > has it set to 64-bits.
> > 
> > The current code works, because e.g., pci_epf_test_write(), etc,
> > does:
> > struct device *dma_dev = epf->epc->dev.parent;
> > dma_map_single(dma_dev, ...);
> > 
> > but it also means that all EPF drivers will do this uglyness.
> > 
> 
> This ugliness is required as long as the dmaengine is associated only with the
> EPC.
> 
> > 
> > 
> > However, if a EPF driver does e.g.
> > dma_alloc_coherent(), and sends in the struct *device for the EPF,
> > which is the most logical thing to do IMO, it will use the wrong DMA
> > mask.
> > 
> > Perhaps EPF or EPC code should make sure that the struct *device
> > for the EPF will get the same DMA mask as epf->epc->dev.parent,
> > so that EPF driver developer can use the struct *epf when calling
> > e.g. dma_alloc_coherent().
> > 
> 
> Makes sense. I think it can be done during bind() in the EPC core. Feel free to
> submit a patch if you like, otherwise I'll keep it in my todo list.

So we still want to test:
-DMA API using the eDMA
-DMA API using the "dummy" memcpy dma-channel.

However, it seems like both pci-epf-mhi.c and pci-epf-test.c
do either:
-Use DMA API
or
-Use memcpy_fromio()/memcpy_toio() instead of DMA API


To me, it seems like we should always be able to use
DMA API (using either a eDMA or "dummy" memcpy).

I don't really see the need to have the path that does:
memcpy_fromio()/memcpy_toio().

I know that for DWC, when using memcpy (and this also
memcpy via DMA API), we need to map the address using
iATU first.

But that could probably be done using another flag,
perhaps rename that flag FLAG_USE_DMA to NEEDS_MAP or
something.
(Such that we can change these drivers to only have a
code path that uses DMA API.)
(...and making sure that inheriting the DMA mask does
not affect the DMA mask for DMA_MEMCPY.)

But perhaps I am missing something... and DMA_MEMCPY is
not always available?


Kind regards,
Niklas