On 20-05-2026 19:53, Dmitry Baryshkov wrote:
> On Tue, May 19, 2026 at 11:45:56AM +0530, Ekansh Gupta via B4 Relay wrote:
>> From: Ekansh Gupta <[email protected]>
>>
>> Introduce the CB (compute context bank) device management layer for the
>> QDA driver. Each DSP domain node in the device tree may contain child
>> nodes with compatible "qcom,fastrpc-compute-cb", each representing one
>> IOMMU context bank. The driver enumerates those child nodes during
>> RPMsg probe and creates a corresponding device on the qda-compute-cb
>> bus for each one.
>>
>> The CB devices are created via create_qda_cb_device(), which registers
>> them on the qda-compute-cb bus so that the IOMMU subsystem assigns each
>> device its own IOMMU domain, enabling per-session address space
>> isolation for DSP buffer mapping.
>>
>> The new qda_cb.c file provides two functions:
>>
>>   qda_create_cb_device()
>>     Reads the "reg" property from the DT child node to obtain the
>>     stream ID, constructs a unique device name of the form
>>     "qda-cb-<dsp>-<sid>", and registers the device on the compute bus.
>>     A qda_cb_dev entry is allocated and appended to qdev->cb_devs so
>>     that the list can be walked during teardown.
>>
>>   qda_destroy_cb_device()
>>     Removes the device from its IOMMU group before calling
>>     device_unregister(), ensuring the IOMMU domain is released cleanly.
>>
>> CB devices are populated before the DRM device is registered and
>> destroyed before it is unplugged, so no DRM operation can race with
>> CB teardown. On probe failure after population, qda_cb_unpopulate()
>> is called to clean up any CBs that were successfully created before
>> the error.
>>
>> Assisted-by: Claude:claude-4-6-sonnet
>> Signed-off-by: Ekansh Gupta <[email protected]>
>> ---
>>  drivers/accel/qda/Makefile    |  1 +
>>  drivers/accel/qda/qda_cb.c    | 99 
>> +++++++++++++++++++++++++++++++++++++++++++
>>  drivers/accel/qda/qda_cb.h    | 32 ++++++++++++++
>>  drivers/accel/qda/qda_drv.c   |  1 +
>>  drivers/accel/qda/qda_drv.h   |  3 ++
>>  drivers/accel/qda/qda_rpmsg.c | 12 +++++-
>>  6 files changed, 147 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/accel/qda/Makefile b/drivers/accel/qda/Makefile
>> index 424176f652a5..143c9e4e789e 100644
>> --- a/drivers/accel/qda/Makefile
>> +++ b/drivers/accel/qda/Makefile
>> @@ -6,6 +6,7 @@
>>  obj-$(CONFIG_DRM_ACCEL_QDA) := qda.o
>>  
>>  qda-y := \
>> +    qda_cb.o \
>>      qda_drv.o \
>>      qda_rpmsg.o
>>  
>> diff --git a/drivers/accel/qda/qda_cb.c b/drivers/accel/qda/qda_cb.c
>> new file mode 100644
>> index 000000000000..77caf8438c67
>> --- /dev/null
>> +++ b/drivers/accel/qda/qda_cb.c
>> @@ -0,0 +1,99 @@
>> +// 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/iommu.h>
>> +#include <linux/qda_compute_bus.h>
>> +#include <linux/slab.h>
>> +#include <drm/drm_print.h>
>> +#include "qda_drv.h"
>> +#include "qda_cb.h"
>> +
>> +int qda_create_cb_device(struct qda_dev *qdev, struct device_node *cb_node)
>> +{
>> +    struct device *cb_dev;
>> +    u32 sid = 0;
>> +    char name[64];
>> +    struct qda_cb_dev *entry;
>> +
>> +    drm_dbg_driver(&qdev->drm_dev, "Creating CB device for node: %s\n", 
>> cb_node->name);
>> +
>> +    of_property_read_u32(cb_node, "reg", &sid);
>> +
>> +    snprintf(name, sizeof(name), "qda-cb-%s-%u", qdev->dsp_name, sid);
>> +
>> +    cb_dev = create_qda_cb_device(qdev->dev, name, DMA_BIT_MASK(32), 
>> cb_node);
> 
> Wrong prefix. Pass the name format and the params to this function. Use
> kasprintf in it.
ack>
>> +    if (IS_ERR(cb_dev)) {
>> +            drm_err(&qdev->drm_dev, "Failed to create CB device for SID %u: 
>> %ld\n",
>> +                    sid, PTR_ERR(cb_dev));
>> +            return PTR_ERR(cb_dev);
>> +    }
>> +
>> +    entry = kzalloc_obj(*entry);
>> +    if (!entry) {
>> +            device_unregister(cb_dev);
>> +            return -ENOMEM;
>> +    }
>> +
>> +    entry->dev = cb_dev;
>> +    list_add_tail(&entry->node, &qdev->cb_devs);
>> +
>> +    drm_dbg_driver(&qdev->drm_dev, "Successfully created CB device for SID 
>> %u\n", sid);
>> +    return 0;
>> +}
>> +
>> +void qda_cb_unpopulate(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);
>> +    }
>> +}
>> +
>> +int qda_cb_populate(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++;
>> +                            dev_dbg(qdev->dev, "Created CB device for node: 
>> %s\n",
>> +                                    child->name);
> 
> Stop counting successes.
> 
>> +                    } else {
>> +                            dev_err(qdev->dev, "Failed to create CB device 
>> for: %s\n",
>> +                                    child->name);
> 
> Unwind, return error.
> 
ack>> +                 }
>> +            }
>> +    }
>> +    if (count == 0)
>> +            return 0;
>> +    return success > 0 ? 0 : -ENODEV;
>> +}
>> +
>> +void qda_destroy_cb_device(struct device *cb_dev)
>> +{
>> +    struct iommu_group *group;
>> +
>> +    if (!cb_dev) {
> 
> How can it be?
I'll remove this.>
>> +            pr_debug("qda: NULL CB device passed to destroy\n");
>> +            return;
>> +    }
>> +
>> +    dev_dbg(cb_dev, "Destroying CB device %s\n", dev_name(cb_dev));
>> +
>> +    group = iommu_group_get(cb_dev);
>> +    if (group) {
>> +            dev_dbg(cb_dev, "Removing %s from IOMMU group\n", 
>> dev_name(cb_dev));
> 
> Be uniform. It's either drm_dbg_foo() or dev_dbg() all over the place.
> Don't mix them.
ack>
>> +            iommu_group_remove_device(cb_dev);
>> +            iommu_group_put(group);
>> +    }
>> +
>> +    device_unregister(cb_dev);
>> +}
>> @@ -59,9 +61,17 @@ static int qda_rpmsg_probe(struct rpmsg_device *rpdev)
>>      }
>>      qdev->dsp_name = label;
>>  
>> +    ret = qda_cb_populate(qdev, rpdev->dev.of_node);
>> +    if (ret) {
>> +            dev_err(qdev->dev, "Failed to populate child devices: %d\n", 
>> ret);
>> +            return ret;
>> +    }
>> +
>>      ret = qda_register_device(qdev);
>> -    if (ret)
>> +    if (ret) {
>> +            qda_cb_unpopulate(qdev);
>>              return ret;
> 
> Unwinding registration?
did I miss something here? The intention to free up the CB devices in
case the device registration fails.>
>> +    }
>>  
>>      drm_info(&qdev->drm_dev, "QDA RPMsg probe complete for %s\n", 
>> qdev->dsp_name);
>>      return 0;
>>
>> -- 
>> 2.34.1
>>
>>
> 

Reply via email to