Skip to content

Cleanups and minor fixes to the ADI remoteproc driver #3428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ukleinek wants to merge 9 commits into
base: adsp-6.18.31-y
Choose a base branch
from
Open

Cleanups and minor fixes to the ADI remoteproc driver #3428

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
95ed86e
remoteproc: adi: Fix order of resource release in remove callback
ukleinek Jul 13, 2026
df5cdbe
remoteproc: adi: Call dmaengine_get() before using DMA
ukleinek Jul 13, 2026
257f22b
remoteproc: adi: Fix error handling for devm_ioremap_wc()
ukleinek Jul 13, 2026
e2068b9
remoteproc: adi: Cleanup device_node pointers using __free
ukleinek Jul 13, 2026
1549aeb
remoteproc: adi: Don't use of_match_ptr for the match table
ukleinek Jul 13, 2026
b45e96e
remoteproc: adi: Don't use a comma after the of_device_id terminator
ukleinek Jul 13, 2026
c2fb8c5
remoteproc: adi: Make use of devm_rproc_alloc()
ukleinek Jul 13, 2026
0fc069c
remoteproc: adi: Convert to use dev_err_probe()
ukleinek Jul 13, 2026
68d061b
remoteproc: adi: Fix commenting style
ukleinek Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 61 additions & 90 deletions drivers/remoteproc/adi_remoteproc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,6 @@
struct device_node *np = dev->of_node;
struct sharc_resource_table *rsc_table;
struct rproc_mem_entry *mem;
struct device_node *node;
struct reserved_mem *rmem;
phys_addr_t size;
int ret, i, mem_regions, num;
Expand Down Expand Up @@ -569,9 +568,8 @@
*/
mem_regions = of_count_phandle_with_args(np, "vdev-vring", NULL);
for (i = 0; i < mem_regions; i++) {
node = of_parse_phandle(np, "vdev-vring", i);
struct device_node *node __free(device_node) = of_parse_phandle(np, "vdev-vring", i);

Check warning on line 571 in drivers/remoteproc/adi_remoteproc.c

View workflow job for this annotation

GitHub Actions / checks / checks 

checkpatch: line length of 101 exceeds 100 columns
+		struct device_node *node __free(device_node) = of_parse_phandle(np, "vdev-vring", i);
rmem = of_reserved_mem_lookup(node);
of_node_put(node);
if (!rmem) {
dev_err(dev, "Failed to acquire vdev-vring at idx %d\n", i);
return -EINVAL;
Expand Down Expand Up @@ -613,7 +611,8 @@
if (i > 0) {
continue;
} else {
/* Calc how many buffers we can fit in the vring region,
/*
* Calc how many buffers we can fit in the vring region,
* number of buffers must be power of 2
*/
for (num = 2; num < 0x00400000; num <<= 1) {
Expand All @@ -639,8 +638,8 @@
* uses CMA region
*/
mem_regions = of_count_phandle_with_args(np, "memory-region", NULL);
for (i = 0; i < mem_regions; i++) {

Check warning on line 641 in drivers/remoteproc/adi_remoteproc.c

View workflow job for this annotation

GitHub Actions / checks / checks 

checkpatch: line length of 104 exceeds 100 columns
+		struct device_node *node __free(device_node) = of_parse_phandle(np, "memory-region", i);
node = of_parse_phandle(np, "memory-region", i);
struct device_node *node __free(device_node) = of_parse_phandle(np, "memory-region", i);
rmem = of_reserved_mem_lookup(node);
mem = rproc_of_resm_mem_entry_init(dev, i, rmem->size,
rmem->base, "vdev%dbuffer", i);
Expand Down Expand Up @@ -687,7 +686,8 @@
return ret;
}

/* @todo store number of vrings from resource table and use it to dynamically
/*
* @todo store number of vrings from resource table and use it to dynamically
* notify the correct number of vrings
*/
static irqreturn_t sharc_virtio_irq_threaded_handler(int irq, void *p)
Expand Down Expand Up @@ -813,92 +813,75 @@
const char *name;

ret = of_property_read_string(np, "firmware-name", &name);
if (ret) {
dev_err(dev, "Unable to get firmware-name property\n");
return ret;
}
if (ret)
return dev_err_probe(dev, ret, "Unable to get firmware-name property\n");

ret = of_property_read_u32(np, "core-id", &core_id);
if (ret) {
dev_err(dev, "Unable to get core-id property\n");
return ret;
}
if (ret)
return dev_err_probe(dev, ret, "Unable to get core-id property\n");

rproc = rproc_alloc(dev, np->name, &adi_rproc_ops,
name, sizeof(*rproc_data));
if (!rproc) {
dev_err(dev, "Unable to allocate remoteproc\n");
rproc = devm_rproc_alloc(dev, np->name, &adi_rproc_ops,
name, sizeof(*rproc_data));
if (!rproc)
return -ENOMEM;
}

rproc_data = (struct adi_rproc_data *)rproc->priv;
platform_set_drvdata(pdev, rproc);

ret = of_parse_phandle_with_fixed_args(np, "adi,svect", 1, 0,
&svect_args);
if (ret) {
dev_err(dev, "Missing adi,svect property\n");
goto free_rproc;
}
if (ret)
return dev_err_probe(dev, ret, "Missing adi,svect property\n");

rproc_data->svect_regmap = syscon_node_to_regmap(svect_args.np);
of_node_put(svect_args.np);
if (IS_ERR(rproc_data->svect_regmap)) {
dev_err(dev, "Unable to get SVECT regmap\n");
ret = PTR_ERR(rproc_data->svect_regmap);
goto free_rproc;
}
if (IS_ERR(rproc_data->svect_regmap))
return dev_err_probe(dev, PTR_ERR(rproc_data->svect_regmap),
"Unable to get SVECT regmap\n");

rproc_data->svect_offset = svect_args.args[0];

rproc_data->rst_crst = devm_reset_control_get_exclusive(dev, "crst");
if (IS_ERR(rproc_data->rst_crst)) {
dev_err(dev, "Unable to get crst reset control\n");
ret = PTR_ERR(rproc_data->rst_crst);
goto free_rproc;
}
if (IS_ERR(rproc_data->rst_crst))
return dev_err_probe(dev, PTR_ERR(rproc_data->rst_crst),
"Unable to get crst reset control\n");

rproc_data->rst_start = devm_reset_control_get_exclusive(dev, "start");
if (IS_ERR(rproc_data->rst_start)) {
dev_err(dev, "Unable to get start reset control\n");
ret = PTR_ERR(rproc_data->rst_start);
goto free_rproc;
}
if (IS_ERR(rproc_data->rst_start))
return dev_err_probe(dev, PTR_ERR(rproc_data->rst_start),
"Unable to get start reset control\n");

ret = reset_control_status(rproc_data->rst_start);
if (ret < 0) {
dev_err(dev, "Unable to read core status\n");
goto free_rproc;
} else if (ret == 0) {
dev_err(dev, "Error: Core%d not idle\n", core_id);
ret = -EBUSY;
goto free_rproc;
}
if (ret < 0)
return dev_err_probe(dev, ret, "Unable to read core status\n");
else if (ret == 0)

@nunojsa nunojsa Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, style wise I really prefer just if (!ret) return. But that's me

@ukleinek ukleinek Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean s/ret == 0/!ret/, right, and not to have the return on the same line as the if statement?

return dev_err_probe(dev, -EBUSY,
"Error: Core%d not idle\n", core_id);

rproc_data->kick_client.dev = dev;
rproc_data->kick_client.tx_block = false;

rproc_data->kick_chan = mbox_request_channel_byname(&rproc_data->kick_client,
"kick");
if (IS_ERR(rproc_data->kick_chan)) {
ret = PTR_ERR(rproc_data->kick_chan);
if (ret != -EPROBE_DEFER)
dev_err(dev, "Unable to get kick mailbox channel\n");
goto free_rproc;
}
if (IS_ERR(rproc_data->kick_chan))
return dev_err_probe(dev, PTR_ERR(rproc_data->kick_chan),
"Unable to get kick mailbox channel\n");

/* for now device addresses are represented as 32 bits and expanded to 64
/*
* for now device addresses are represented as 32 bits and expanded to 64
* here in driver code
*/
if (of_property_read_u32_array(np, "adi,l1-da", addr, 2)) {
dev_err(dev, "Missing adi,l1-da with L1 device address range information\n");
ret = -ENODEV;
ret = dev_err_probe(dev, -ENODEV,
"Missing adi,l1-da with L1 device address range information\n");
goto free_mbox;
}
rproc_data->l1_da_range[0] = addr[0];
rproc_data->l1_da_range[1] = addr[1];

if (of_property_read_u32_array(np, "adi,l2-da", addr, 2)) {
dev_err(dev, "Missing adi,l2-da with L2 device address range information\n");
ret = -ENODEV;
ret = dev_err_probe(dev, -ENODEV,
"Missing adi,l2-da with L2 device address range information\n");
goto free_mbox;
}
rproc_data->l2_da_range[0] = addr[0];
Expand All @@ -910,25 +893,20 @@
dev_info(&pdev->dev, "Resource table set, enable rpmsg\n");
rmem = of_reserved_mem_lookup(node);
of_node_put(node);
if (!rmem) {
dev_err(&pdev->dev, "Translating adi,rsc-table failed\n");
ret = -ENOMEM;
if (!rmem)
goto free_mbox;
}

rproc_data->adi_rsc_table = devm_ioremap_wc(dev,
rmem->base,
rmem->size);
if (IS_ERR(rproc_data->adi_rsc_table)) {
dev_err(dev, "Can't map adi,rsc-table\n");
ret = PTR_ERR(rproc_data->adi_rsc_table);
if (!rproc_data->adi_rsc_table) {
ret = -ENOMEM;
goto free_mbox;
}

rproc_data->icc_irq = platform_get_irq(pdev, 0);
if (rproc_data->icc_irq <= 0) {
dev_err(dev, "No ICC IRQ specified\n");
ret = -ENOENT;
ret = dev_err_probe(dev, -ENOENT, "No ICC IRQ specified\n");
goto free_mbox;
}

Expand All @@ -940,39 +918,33 @@

rproc_data->core_workqueue = alloc_workqueue("Core workqueue",
WQ_UNBOUND | WQ_MEM_RECLAIM, 1);
if (!rproc_data->core_workqueue) {
dev_err(dev, "Unable to allocate core workqueue\n");
ret = -ENOMEM;
if (!rproc_data->core_workqueue)
goto free_mbox;
}

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
dev_err(dev, "Cannot get L1 base address (reg 0)\n");
ret = -ENODEV;
ret = dev_err_probe(dev, -ENODEV, "Cannot get L1 base address (reg 0)\n");
goto free_workqueue;
}
rproc_data->L1_shared_base = devm_ioremap_wc(dev,
res->start,
resource_size(res));
if (IS_ERR(rproc_data->L1_shared_base)) {
dev_err(dev, "Cannot map L1 shared memory\n");
ret = PTR_ERR(rproc_data->L1_shared_base);
if (!rproc_data->L1_shared_base) {
ret = -ENOMEM;
goto free_workqueue;
}

res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
if (!res) {
dev_err(dev, "Cannot get L2 base address (reg 1)\n");
ret = -ENODEV;
ret = dev_err_probe(dev, -ENODEV, "Cannot get L2 base address (reg 1)\n");
goto free_workqueue;
}
rproc_data->L2_shared_base = devm_ioremap_wc(dev,
res->start,
resource_size(res));
if (IS_ERR(rproc_data->L2_shared_base)) {
if (!rproc_data->L2_shared_base) {
dev_err(dev, "Cannot map L2 shared memory\n");
ret = PTR_ERR(rproc_data->L2_shared_base);
ret = -ENOMEM;
goto free_workqueue;
}

Expand All @@ -991,25 +963,25 @@
rproc_data->ldr_load_addr = SHARC_IDLE_ADDR;
rproc_data->rpmsg_state = ADI_RP_RPMSG_TIMED_OUT;

dmaengine_get();

ret = rproc_add(rproc);
if (ret) {
dev_err(dev, "Failed to add rproc\n");
goto free_workqueue;
dev_err_probe(dev, ret, "Failed to add rproc\n");
goto put_dmaengine;
}

dmaengine_get();

return 0;

put_dmaengine:
dmaengine_put();

free_workqueue:
destroy_workqueue(rproc_data->core_workqueue);

free_mbox:
mbox_free_channel(rproc_data->kick_chan);

free_rproc:
rproc_free(rproc);

return ret;
}

Expand All @@ -1018,16 +990,15 @@
struct rproc *rproc = platform_get_drvdata(pdev);
struct adi_rproc_data *rproc_data = rproc->priv;

rproc_del(rproc);
dmaengine_put();
destroy_workqueue(rproc_data->core_workqueue);
mbox_free_channel(rproc_data->kick_chan);
rproc_del(rproc);
rproc_free(rproc);
}

static const struct of_device_id adi_rproc_of_match[] = {
{ .compatible = "adi,remoteproc" },
{ },
{ }
};
MODULE_DEVICE_TABLE(of, adi_rproc_of_match);

Expand All @@ -1036,7 +1007,7 @@
.remove = adi_remoteproc_remove,
.driver = {
.name = "adi_remoteproc",
.of_match_table = of_match_ptr(adi_rproc_of_match),
.of_match_table = adi_rproc_of_match,
Comment thread
ukleinek marked this conversation as resolved.
},
};
module_platform_driver(adi_rproc_driver);
Expand Down
Loading