Re: [PATCH RFC 05/18] accel/qda: Create compute CB devices on QDA compute bus

[Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>]

On 2/24/2026 4:19 AM, Dmitry Baryshkov wrote:
> On Tue, Feb 24, 2026 at 12:38:59AM +0530, Ekansh Gupta wrote:
>> Add support for creating compute context-bank (CB) devices under
>> the QDA compute bus based on child nodes of the FastRPC RPMsg
>> device tree node. Each DT child with compatible
>> "qcom,fastrpc-compute-cb" is turned into a QDA-owned struct
>> device on qda_cb_bus_type.
>>
>> A new qda_cb_dev structure and cb_devs list in qda_dev track these
>> CB devices. qda_populate_child_devices() walks the DT children
>> during QDA RPMsg probe, creates CB devices, configures their DMA
>> and IOMMU settings using of_dma_configure(), and associates a SID
>> from the "reg" property when present.
>>
>> On RPMsg remove, qda_unpopulate_child_devices() tears down all CB
>> devices, removing them from their IOMMU groups if present and
>> unregistering the devices. This prepares the ground for using CB
>> devices as IOMMU endpoints for DSP compute workloads in later
>> patches.
> Are we loosing the nsessions support?
Yes, it's not part of this series. I'll try bringing that as well.
>
>> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>> ---
>>  drivers/accel/qda/Makefile    |   1 +
>>  drivers/accel/qda/qda_cb.c    | 150 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/accel/qda/qda_cb.h    |  26 ++++++++
>>  drivers/accel/qda/qda_drv.h   |   3 +
>>  drivers/accel/qda/qda_rpmsg.c |  40 +++++++++++
>>  5 files changed, 220 insertions(+)
>>
>> diff --git a/drivers/accel/qda/Makefile b/drivers/accel/qda/Makefile
>> index 242684ef1af7..4aded20b6bc2 100644
>> --- a/drivers/accel/qda/Makefile
>> +++ b/drivers/accel/qda/Makefile
>> @@ -8,5 +8,6 @@ obj-$(CONFIG_DRM_ACCEL_QDA)	:= qda.o
>>  qda-y := \
>>  	qda_drv.o \
>>  	qda_rpmsg.o \
>> +	qda_cb.o \
>>  
>>  obj-$(CONFIG_DRM_ACCEL_QDA_COMPUTE_BUS) += qda_compute_bus.o
>> diff --git a/drivers/accel/qda/qda_cb.c b/drivers/accel/qda/qda_cb.c
>> new file mode 100644
>> index 000000000000..77a2d8cae076
>> --- /dev/null
>> +++ b/drivers/accel/qda/qda_cb.c
>> @@ -0,0 +1,150 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +// Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> +#include <linux/dma-mapping.h>
>> +#include <linux/device.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/iommu.h>
>> +#include <linux/slab.h>
>> +#include "qda_drv.h"
>> +#include "qda_cb.h"
>> +
>> +static void qda_cb_dev_release(struct device *dev)
>> +{
>> +	kfree(dev);
> Do you need to put the reference on the OF node?
Reference put is happening as part of qda_destroy_cb_device.
>
>> +}
>> +
>> +static int qda_configure_cb_iommu(struct device *cb_dev, struct device_node *cb_node)
>> +{
>> +	int ret;
>> +
>> +	qda_dbg(NULL, "Configuring DMA/IOMMU for CB device %s\n", dev_name(cb_dev));
>> +
>> +	/* Use of_dma_configure which handles both DMA and IOMMU configuration */
>> +	ret = of_dma_configure(cb_dev, cb_node, true);
>> +	if (ret) {
>> +		qda_err(NULL, "of_dma_configure failed for %s: %d\n", dev_name(cb_dev), ret);
>> +		return ret;
>> +	}
>> +
>> +	qda_dbg(NULL, "DMA/IOMMU configured successfully for CB device %s\n", dev_name(cb_dev));
>> +	return 0;
>> +}
>> +
>> +static int qda_cb_setup_device(struct qda_dev *qdev, struct device *cb_dev)
>> +{
>> +	int rc;
>> +	u32 sid, pa_bits = 32;
>> +
>> +	qda_dbg(qdev, "Setting up CB device %s\n", dev_name(cb_dev));
>> +
>> +	if (of_property_read_u32(cb_dev->of_node, "reg", &sid)) {
>> +		qda_dbg(qdev, "No 'reg' property found, defaulting SID to 0\n");
>> +		sid = 0;
> Don't do the job of the schema validator. Are there nodes without reg?
> No.
Ack.
>
>> +	}
>> +
>> +	rc = dma_set_mask(cb_dev, DMA_BIT_MASK(pa_bits));
>> +	if (rc) {
>> +		qda_err(qdev, "%d bit DMA enable failed: %d\n", pa_bits, rc);
>> +		return rc;
>> +	}
>> +
>> +	qda_dbg(qdev, "CB device setup complete - SID: %u, PA bits: %u\n", sid, pa_bits);
>> +
>> +	return 0;
>> +}
>> +
>> +int qda_create_cb_device(struct qda_dev *qdev, struct device_node *cb_node)
>> +{
>> +	struct device *cb_dev;
>> +	int ret;
>> +	u32 sid = 0;
>> +	struct qda_cb_dev *entry;
>> +
>> +	qda_dbg(qdev, "Creating CB device for node: %s\n", cb_node->name);
>> +
>> +	of_property_read_u32(cb_node, "reg", &sid);
>> +
>> +	cb_dev = kzalloc_obj(*cb_dev, GFP_KERNEL);
>> +	if (!cb_dev)
>> +		return -ENOMEM;
>> +
>> +	device_initialize(cb_dev);
>> +	cb_dev->parent = qdev->dev;
>> +	cb_dev->bus = &qda_cb_bus_type; /* Use our custom bus type for IOMMU handling */
>> +	cb_dev->release = qda_cb_dev_release;
>> +	dev_set_name(cb_dev, "qda-cb-%s-%u", qdev->dsp_name, sid);
>> +
>> +	qda_dbg(qdev, "Initialized CB device: %s\n", dev_name(cb_dev));
>> +
>> +	cb_dev->of_node = of_node_get(cb_node);
>> +
>> +	cb_dev->dma_mask = &cb_dev->coherent_dma_mask;
>> +	cb_dev->coherent_dma_mask = DMA_BIT_MASK(32);
>> +
>> +	dev_set_drvdata(cb_dev->parent, qdev);
>> +
>> +	ret = device_add(cb_dev);
>> +	if (ret) {
>> +		qda_err(qdev, "Failed to add CB device for SID %u: %d\n", sid, ret);
>> +		goto cleanup_device_init;
>> +	}
>> +
>> +	qda_dbg(qdev, "CB device added to system\n");
>> +
>> +	ret = qda_configure_cb_iommu(cb_dev, cb_node);
>> +	if (ret) {
>> +		qda_err(qdev, "IOMMU configuration failed: %d\n", ret);
>> +		goto cleanup_device_add;
>> +	}
>> +
>> +	ret = qda_cb_setup_device(qdev, cb_dev);
>> +	if (ret) {
>> +		qda_err(qdev, "CB device setup failed: %d\n", ret);
>> +		goto cleanup_device_add;
>> +	}
>> +
>> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>> +	if (!entry) {
>> +		ret = -ENOMEM;
>> +		goto cleanup_device_add;
>> +	}
>> +
>> +	entry->dev = cb_dev;
>> +	list_add_tail(&entry->node, &qdev->cb_devs);
>> +
>> +	qda_dbg(qdev, "Successfully created CB device for SID %u\n", sid);
>> +	return 0;
>> +
>> +cleanup_device_add:
>> +	device_del(cb_dev);
>> +cleanup_device_init:
>> +	of_node_put(cb_dev->of_node);
>> +	put_device(cb_dev);
>> +	return ret;
>> +}
>> +
>> +void qda_destroy_cb_device(struct device *cb_dev)
>> +{
>> +	struct iommu_group *group;
>> +
>> +	if (!cb_dev) {
>> +		qda_dbg(NULL, "NULL CB device passed to destroy\n");
>> +		return;
>> +	}
>> +
>> +	qda_dbg(NULL, "Destroying CB device %s\n", dev_name(cb_dev));
>> +
>> +	group = iommu_group_get(cb_dev);
>> +	if (group) {
>> +		qda_dbg(NULL, "Removing %s from IOMMU group\n", dev_name(cb_dev));
>> +		iommu_group_remove_device(cb_dev);
>> +		iommu_group_put(group);
>> +	}
>> +
>> +	of_node_put(cb_dev->of_node);
>> +	cb_dev->of_node = NULL;
>> +	device_unregister(cb_dev);
>> +
>> +	qda_dbg(NULL, "CB device %s destroyed\n", dev_name(cb_dev));
>> +}
>> diff --git a/drivers/accel/qda/qda_cb.h b/drivers/accel/qda/qda_cb.h
>> new file mode 100644
>> index 000000000000..a4ae9fef142e
>> --- /dev/null
>> +++ b/drivers/accel/qda/qda_cb.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#ifndef __QDA_CB_H__
>> +#define __QDA_CB_H__
>> +
>> +#include <linux/device.h>
>> +#include <linux/of.h>
>> +#include <linux/list.h>
>> +#include <linux/qda_compute_bus.h>
>> +#include "qda_drv.h"
>> +
>> +struct qda_cb_dev {
>> +	struct list_head node;
>> +	struct device *dev;
>> +};
>> +
>> +/*
>> + * Compute bus (CB) device management
>> + */
>> +int qda_create_cb_device(struct qda_dev *qdev, struct device_node *cb_node);
>> +void qda_destroy_cb_device(struct device *cb_dev);
>> +
>> +#endif /* __QDA_CB_H__ */
>> diff --git a/drivers/accel/qda/qda_drv.h b/drivers/accel/qda/qda_drv.h
>> index bec2d31ca1bb..eb732b7d8091 100644
>> --- a/drivers/accel/qda/qda_drv.h
>> +++ b/drivers/accel/qda/qda_drv.h
>> @@ -7,6 +7,7 @@
>>  #define __QDA_DRV_H__
>>  
>>  #include <linux/device.h>
>> +#include <linux/list.h>
>>  #include <linux/mutex.h>
>>  #include <linux/rpmsg.h>
>>  #include <linux/xarray.h>
>> @@ -26,6 +27,8 @@ struct qda_dev {
>>  	atomic_t removing;
>>  	/* Name of the DSP (e.g., "cdsp", "adsp") */
>>  	char dsp_name[16];
>> +	/* Compute context-bank (CB) child devices */
>> +	struct list_head cb_devs;
>>  };
>>  
>>  /**
>> diff --git a/drivers/accel/qda/qda_rpmsg.c b/drivers/accel/qda/qda_rpmsg.c
>> index a8b24a99ca13..5a57384de6a2 100644
>> --- a/drivers/accel/qda/qda_rpmsg.c
>> +++ b/drivers/accel/qda/qda_rpmsg.c
>> @@ -7,6 +7,7 @@
>>  #include <linux/of_device.h>
>>  #include "qda_drv.h"
>>  #include "qda_rpmsg.h"
>> +#include "qda_cb.h"
>>  
>>  static int qda_rpmsg_init(struct qda_dev *qdev)
>>  {
>> @@ -25,11 +26,42 @@ static struct qda_dev *alloc_and_init_qdev(struct rpmsg_device *rpdev)
>>  
>>  	qdev->dev = &rpdev->dev;
>>  	qdev->rpdev = rpdev;
>> +	INIT_LIST_HEAD(&qdev->cb_devs);
>>  
>>  	qda_dbg(qdev, "Allocated and initialized qda_dev\n");
>>  	return qdev;
>>  }
>>  
>> +static void qda_unpopulate_child_devices(struct qda_dev *qdev)
>> +{
>> +	struct qda_cb_dev *entry, *tmp;
>> +
>> +	list_for_each_entry_safe(entry, tmp, &qdev->cb_devs, node) {
>> +		list_del(&entry->node);
>> +		qda_destroy_cb_device(entry->dev);
>> +		kfree(entry);
> Why can't you embed struct device into a structure together with the
> list_node (and possibly some other data?)?
I'll check this.
>
>> +	}
>> +}
>> +
>> +static int qda_populate_child_devices(struct qda_dev *qdev, struct device_node *parent_node)
>> +{
>> +	struct device_node *child;
>> +	int count = 0, success = 0;
>> +
>> +	for_each_child_of_node(parent_node, child) {
>> +		if (of_device_is_compatible(child, "qcom,fastrpc-compute-cb")) {
>> +			count++;
>> +			if (qda_create_cb_device(qdev, child) == 0) {
>> +				success++;
>> +				qda_dbg(qdev, "Created CB device for node: %s\n", child->name);
>> +			} else {
>> +				qda_err(qdev, "Failed to create CB device for: %s\n", child->name);
> Don't loose the error code. Instead please return it to the caller.
Ack.
>
>> +			}
>> +		}
>> +	}
>> +	return success > 0 ? 0 : (count > 0 ? -ENODEV : 0);
>> +}
>> +
>>  static int qda_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len, void *priv, u32 src)
>>  {
>>  	/* Dummy function for rpmsg driver */
>> @@ -48,6 +80,7 @@ static void qda_rpmsg_remove(struct rpmsg_device *rpdev)
>>  	qdev->rpdev = NULL;
>>  	mutex_unlock(&qdev->lock);
>>  
>> +	qda_unpopulate_child_devices(qdev);
>>  	qda_deinit_device(qdev);
>>  
>>  	qda_info(qdev, "RPMsg device removed\n");
>> @@ -83,6 +116,13 @@ static int qda_rpmsg_probe(struct rpmsg_device *rpdev)
>>  	if (ret)
>>  		return ret;
>>  
>> +	ret = qda_populate_child_devices(qdev, rpdev->dev.of_node);
>> +	if (ret) {
>> +		qda_err(qdev, "Failed to populate child devices: %d\n", ret);
>> +		qda_deinit_device(qdev);
>> +		return ret;
>> +	}
>> +
>>  	qda_info(qdev, "QDA RPMsg probe completed successfully for %s\n", qdev->dsp_name);
>>  	return 0;
>>  }
>>
>> -- 
>> 2.34.1
>>