On 2022-11-07 21:11, Serge Semin wrote:
On Tue, Nov 08, 2022 at 12:04:36AM +0300, Serge Semin wrote:
DW eDMA doesn't perform any translation of the traffic generated on the
CPU/Application side. It just generates read/write AXI-bus requests with
the specified addresses. But in case if the dma-ranges DT-property is
specified for a platform device node, Linux will use it to create a
mapping the PCIe-bus regions into the CPU memory ranges. This isn't what
we want for the eDMA embedded into the locally accessed DW PCIe Root Port
and End-point. In order to work that around let's set the chan_dma_dev
flag for each DW eDMA channel thus forcing the client drivers to getting a
custom dma-ranges-less parental device for the mappings.
Note it will only work for the client drivers using the
dmaengine_get_dma_device() method to get the parental DMA device.
@Robin, we particularly need you opinion on this patch. I did as you
said: call *_dma_configure() method to initialize the child device and
set the DMA-mask here instead of the platform driver.
Apologies, I've been busy and this series got buried in my inbox before
I'd clocked it as something I was supposed to be looking at.
@Vinoud, @Manivannan I had to drop your tags from this patch since its
content had been significantly changed.
-Sergey
Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx>
---
Changelog v2:
- Fix the comment a bit to being clearer. (@Manivannan)
Changelog v3:
- Conditionally set dchan->dev->device.dma_coherent field since it can
be missing on some platforms. (@Manivannan)
- Remove Manivannan' rb and tb tags since the patch content has been
changed.
Changelog v6:
- Directly call *_dma_configure() method on the child device used for
the DMA buffers mapping. (@Robin)
- Explicitly set the DMA-mask of the child device in the channel
allocation proecedure. (@Robin)
- Drop @Manivannan and @Vinod rb- and ab-tags due to significant patch
content change.
---
drivers/dma/dw-edma/dw-edma-core.c | 44 ++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index e3671bfbe186..846518509753 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -6,9 +6,11 @@
* Author: Gustavo Pimentel <gustavo.pimentel@xxxxxxxxxxxx>
*/
+#include <linux/acpi.h>
#include <linux/module.h>
#include <linux/device.h>
#include <linux/kernel.h>
+#include <linux/of_device.h>
#include <linux/dmaengine.h>
#include <linux/err.h>
#include <linux/interrupt.h>
@@ -711,10 +713,52 @@ static irqreturn_t dw_edma_interrupt_common(int irq, void *data)
static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
{
struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+ struct device *dev = chan->dw->chip->dev;
+ int ret;
if (chan->status != EDMA_ST_IDLE)
return -EBUSY;
+ /* Bypass the dma-ranges based memory regions mapping for the eDMA
+ * controlled from the CPU/Application side since in that case
+ * the local memory address is left untranslated.
+ */
+ if (chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
+ ret = dma_coerce_mask_and_coherent(&dchan->dev->device,
+ DMA_BIT_MASK(64));
+ if (ret) {
Setting a 64-bit mask should never fail, especially on any platform that
will actually run this code.
+ ret = dma_coerce_mask_and_coherent(&dchan->dev->device,
+ DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+ }
+
+ if (dev_of_node(dev)) {
+ struct device_node *node = dev_of_node(dev);
+
+ ret = of_dma_configure(&dchan->dev->device, node, true);
+ } else if (has_acpi_companion(dev)) {
Can this can ever happen? AFAICS there's no ACPI binding to match and
probe the DWC driver, at best it could only probe as a standard PNP0A08
host bridge which wouldn't know anything about eDMA anyway.
+ struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
+
+ ret = acpi_dma_configure(&dchan->dev->device,
+ acpi_get_dma_attr(adev));
+ } else {
+ ret = -EINVAL;
+ }
+
+ if (ret)
+ return ret;
+
+ if (dchan->dev->device.dma_range_map) {
+ kfree(dchan->dev->device.dma_range_map);
+ dchan->dev->device.dma_range_map = NULL;
+ }
Ugh, I guess this is still here because now you're passing the channel
device to of_dma_configure() such that it looks like a PCI child :(
Can we just set "chan->dev->device.of_node = dev->of_node;" beforehand
so it works as expected (with f1ad5338a4d5 in place) and we don't need
to be messing with the dma_range_map details at all? Note that that
isn't as hacky as it might sound - it's a relatively well-established
practice in places like I2C and SPI, and in this case it seems perfectly
appropriate semantically as well.
(And there should be no need to bother with of_node refcounting, since
the lifetime of the eDMA driver is bounded by the lifetime of the PCIe
driver, thus the lifetime of the DMA channel devices is bounded by the
lifetime of the PCIe platform device, which already holds a reference
from of_device_alloc().)
Thanks,
Robin.
+
+ dchan->dev->chan_dma_dev = true;
+ } else {
+ dchan->dev->chan_dma_dev = false;
+ }
+
return 0;
}
--
2.38.0
