On Tue, Sep 09, 2025 at 02:03:09PM +0200, Marco Felsch wrote:

Hi Frank,

On 25-09-03, Frank Li wrote:

quoted

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!

quoted

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)?

we can support runtime pm later.

quoted

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, ...

Unlike other devices, like phys, regulator, mailbox..., which auto create
devlink at probe. I am not clear why dma skip this one. So I think there
should be some reason behind. Maybe other people, rob or Vinod Koul know
the reason.

static const struct supplier_bindings of_supplier_bindings[] = {
        ...
	{ .parse_prop = parse_dmas, .optional = true, },

If remove "optional = true", devlink will auto create. I am not sure why
set true here.

quoted

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.

Not really, there are other dma engineer already reuse it for other purpose.
So It needs update kernel doc for dma_chan_dev.

quoted

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

I see, this must be tested of course.

quoted

quoted

        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.

quoted

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

This is of course interessting, thanks for the hint.

quoted

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".

quoted

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

Huh.. you used the dmaengine device too?

/**
 * struct dma_chan_dev - relate sysfs device node to backing channel device
 * @chan: driver channel device
 * @device: sysfs device
 * @dev_id: parent dma_device dev_id
 * @chan_dma_dev: The channel is using custom/different dma-mapping
 * compared to the parent dma_device
 */
struct dma_chan_dev {
	struct dma_chan *chan;
	struct device device;
	int dev_id;
	bool chan_dma_dev;
};

struct dma_chan {
	struct dma_device *device; /// this one should be dmaengine
	struct dma_chan_dev *dev; /// this one is pre-chan device.
}

Frank

Regards,
  Marco

quoted

        }

Need update kernel doc

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index bb146c5ac3e4c..ffb3a8f0070ba 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h

@@ -323,7 +323,8 @@ struct dma_router {
  * @cookie: last cookie value returned to client
  * @completed_cookie: last completed cookie for this channel
  * @chan_id: channel ID for sysfs
- * @dev: class device for sysfs
+ * @dev: class device for sysfs, also use for pre channel runtime pm and
+ *       use custom/different dma-mapping

Frank

quoted

+	if (!dl) {
+		dev_err(dev, "failed to create device link to %s\n",
+			dev_name(chan->device->dev));
+		return ERR_PTR(-EINVAL);
+	}
 	chan->name = kasprintf(GFP_KERNEL, "dma:%s", name);
 	if (!chan->name)
 		return chan;

--
2.47.2