Hi Frank,

On 25-09-03, Frank Li wrote:

On Wed, Sep 03, 2025 at 03:06:17PM +0200, Marco Felsch wrote:

quoted

Add support to create device_links between dmaengine suppliers and the
dma consumers. This shifts the device dep-chain teardown/bringup logic
to the driver core.

Moving this to the core allows the dmaengine drivers to simplify the
.remove() hooks and also to ensure that no dmaengine driver is ever
removed before the consumer is removed.

Signed-off-by: Marco Felsch <redacted>
---

Thank you work for devlink between dmaengine and devices. I have similar
idea.

This patch should be first patch.

I can shuffle it of course!

The below what planned commit message in my local tree.

Okay, so you focused on runtime PM handling. Not quite sure if I can
test this feature with the SDMA engine. I also have limited time for
this feature.

Is it okay for you and the DMA maintainers to add the runtime PM feature
as separate patch (provided by NXP/Frank)?

Implementing runtime PM for DMA channels is challenging. If a channel
resumes at allocation and suspends at free, the DMA engine often remains on
because most drivers request a channel at probe.

Tracking the number of pending DMA descriptors is also problematic, as some
consumers append new descriptors in atomic contexts, such as IRQ handlers,
where runtime resume cannot be called.

Using a device link simplifies this issue. If a consumer requires data
transfer, it must be in a runtime-resumed state, ensuring that the DMA
channel is also active by device link. This allows safe operations, like
appending new descriptors. Conversely, when the consumer no longer requires
data transfer, both it and the supplier (DMA channel) can enter a suspended
state if no other consumer is using it.

Introduce the `create_link` flag to enable this feature.

also suggest add create_link flag to enable this feature in case some
side impact to other dma-engine. After some time test, we can enable it
default.

What regressions do you have in mind? I wouldn't hide the feature behind
a flag because this may slow done the convert process, because no one is
interessted in, or has no time for testing, ...

quoted

 drivers/dma/dmaengine.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 758fcd0546d8bde8e8dddc6039848feeb1e24475..a50652bc70b8ce9d4edabfaa781b3432ee47d31e 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c

@@ -817,6 +817,7 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name)
 	struct fwnode_handle *fwnode = dev_fwnode(dev);
 	struct dma_device *d, *_d;
 	struct dma_chan *chan = NULL;
+	struct device_link *dl;

 	if (is_of_node(fwnode))
 		chan = of_dma_request_slave_channel(to_of_node(fwnode), name);

@@ -858,6 +859,13 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name)
 	/* No functional issue if it fails, users are supposed to test before use */
 #endif

+	dl = device_link_add(dev, chan->device->dev, DL_FLAG_AUTOREMOVE_CONSUMER);

chan->device->dev is dmaengine devices. But some dmaengine's each channel
have device, consumer should link to chan's device, not dmaengine device
because some dmaengine support per channel clock\power management.

I get your point. Can you give me some pointers please? To me it seems
like the dma_chan_dev is only used for sysfs purpose according the
dmaengine.h.

chan's device's parent devices is dmaengine devices. it should also work
for sdma case

I see, this must be tested of course.

        if (chan->device->create_devlink) {
                u32 flags = DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_CONSUMER;

According device_link.rst: using DL_FLAG_STATELESS and
DL_FLAG_AUTOREMOVE_CONSUMER is invalid.

                if (pm_runtime_active(dev))
                        flags |= DL_FLAG_RPM_ACTIVE;

This is of course interessting, thanks for the hint.

When create device link (apply channel), consume may active.

I have read it as: "resue the supplier and ensure that the supplier
follows the consumer runtime state".

                dl = device_link_add(chan->slave, &chan->dev->device, flags);

Huh.. you used the dmaengine device too?

Regards,
  Marco