From 7378aeb664e5ebc396950b36a1f2dedf5aabec20 Mon Sep 17 00:00:00 2001 From: Arnaud Pouliquen Date: Fri, 22 Nov 2024 18:51:27 +0100 Subject: [PATCH 01/13] remoteproc: core: Fix ida_free call while not allocated In the rproc_alloc() function, on error, put_device(&rproc->dev) is called, leading to the call of the rproc_type_release() function. An error can occurs before ida_alloc is called. In such case in rproc_type_release(), the condition (rproc->index >= 0) is true as rproc->index has been initialized to 0. ida_free() is called reporting a warning: [ 4.181906] WARNING: CPU: 1 PID: 24 at lib/idr.c:525 ida_free+0x100/0x164 [ 4.186378] stm32-display-dsi 5a000000.dsi: Fixed dependency cycle(s) with /soc/dsi@5a000000/panel@0 [ 4.188854] ida_free called for id=0 which is not allocated. [ 4.198256] mipi-dsi 5a000000.dsi.0: Fixed dependency cycle(s) with /soc/dsi@5a000000 [ 4.203556] Modules linked in: panel_orisetech_otm8009a dw_mipi_dsi_stm(+) gpu_sched dw_mipi_dsi stm32_rproc stm32_crc32 stm32_ipcc(+) optee(+) [ 4.224307] CPU: 1 UID: 0 PID: 24 Comm: kworker/u10:0 Not tainted 6.12.0 #442 [ 4.231481] Hardware name: STM32 (Device Tree Support) [ 4.236627] Workqueue: events_unbound deferred_probe_work_func [ 4.242504] Call trace: [ 4.242522] unwind_backtrace from show_stack+0x10/0x14 [ 4.250218] show_stack from dump_stack_lvl+0x50/0x64 [ 4.255274] dump_stack_lvl from __warn+0x80/0x12c [ 4.260134] __warn from warn_slowpath_fmt+0x114/0x188 [ 4.265199] warn_slowpath_fmt from ida_free+0x100/0x164 [ 4.270565] ida_free from rproc_type_release+0x38/0x60 [ 4.275832] rproc_type_release from device_release+0x30/0xa0 [ 4.281601] device_release from kobject_put+0xc4/0x294 [ 4.286762] kobject_put from rproc_alloc.part.0+0x208/0x28c [ 4.292430] rproc_alloc.part.0 from devm_rproc_alloc+0x80/0xc4 [ 4.298393] devm_rproc_alloc from stm32_rproc_probe+0xd0/0x844 [stm32_rproc] [ 4.305575] stm32_rproc_probe [stm32_rproc] from platform_probe+0x5c/0xbc Calling ida_alloc earlier in rproc_alloc ensures that the rproc->index is properly set. Fixes: 08333b911f01 ("remoteproc: Directly use ida_alloc()/free()") Signed-off-by: Arnaud Pouliquen Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20241122175127.2188037-1-arnaud.pouliquen@foss.st.com Signed-off-by: Mathieu Poirier --- drivers/remoteproc/remoteproc_core.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index eb66f78ec8b77..c2cf0d2777296 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -2486,6 +2486,13 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, rproc->dev.driver_data = rproc; idr_init(&rproc->notifyids); + /* Assign a unique device index and name */ + rproc->index = ida_alloc(&rproc_dev_index, GFP_KERNEL); + if (rproc->index < 0) { + dev_err(dev, "ida_alloc failed: %d\n", rproc->index); + goto put_device; + } + rproc->name = kstrdup_const(name, GFP_KERNEL); if (!rproc->name) goto put_device; @@ -2496,13 +2503,6 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, if (rproc_alloc_ops(rproc, ops)) goto put_device; - /* Assign a unique device index and name */ - rproc->index = ida_alloc(&rproc_dev_index, GFP_KERNEL); - if (rproc->index < 0) { - dev_err(dev, "ida_alloc failed: %d\n", rproc->index); - goto put_device; - } - dev_set_name(&rproc->dev, "remoteproc%d", rproc->index); atomic_set(&rproc->power, 0); From 1dc7c8ed7cb378dd3974387692dfa833aee718d4 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Mon, 28 Oct 2024 17:58:35 +0000 Subject: [PATCH 02/13] remoteproc: omap: Handle ARM dma_iommu_mapping It's no longer practical for the OMAP IOMMU driver to trick arm_setup_iommu_dma_ops() into ignoring its presence, so let's use the same tactic as other IOMMU API users on 32-bit ARM and explicitly kick the arch code's dma_iommu_mapping out of the way to avoid problems. Fixes: 4720287c7bf7 ("iommu: Remove struct iommu_ops *iommu from arch_setup_dma_ops()") Signed-off-by: Robin Murphy Tested-by: Beleswar Padhi Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/6186e311cb6f64a787f87fd41e49a73f409b789c.1730136799.git.robin.murphy@arm.com [Fixed changelog title] Signed-off-by: Mathieu Poirier --- drivers/remoteproc/omap_remoteproc.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c index 9ae2e831456d5..3260dd512491e 100644 --- a/drivers/remoteproc/omap_remoteproc.c +++ b/drivers/remoteproc/omap_remoteproc.c @@ -37,6 +37,10 @@ #include +#ifdef CONFIG_ARM_DMA_USE_IOMMU +#include +#endif + #include "omap_remoteproc.h" #include "remoteproc_internal.h" @@ -1323,6 +1327,19 @@ static int omap_rproc_probe(struct platform_device *pdev) /* All existing OMAP IPU and DSP processors have an MMU */ rproc->has_iommu = true; +#ifdef CONFIG_ARM_DMA_USE_IOMMU + /* + * Throw away the ARM DMA mapping that we'll never use, so it doesn't + * interfere with the core rproc->domain and we get the right DMA ops. + */ + if (pdev->dev.archdata.mapping) { + struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(&pdev->dev); + + arm_iommu_detach_device(&pdev->dev); + arm_iommu_release_mapping(mapping); + } +#endif + ret = omap_rproc_of_get_internal_memories(pdev, rproc); if (ret) return ret; From dbb9c372555c0b2a5a9264418bfba6d017752808 Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 11 Dec 2024 15:20:07 +0800 Subject: [PATCH 03/13] remoteproc: mtk_scp: Only populate devices for SCP cores When multi-core SCP support was added, the driver was made to populate platform devices for all the sub-nodes. This ended up adding platform devices for the rpmsg sub-nodes as well, which never actually get used, since rpmsg devices are registered through the rpmsg interface. Limit of_platform_populate() to just populating the SCP cores with a compatible string match list. Fixes: 1fdbf0cdde98 ("remoteproc: mediatek: Probe SCP cluster on multi-core SCP") Cc: Tinghan Shen Signed-off-by: Chen-Yu Tsai Link: https://lore.kernel.org/r/20241211072009.120511-1-wenst@chromium.org Signed-off-by: Mathieu Poirier --- drivers/remoteproc/mtk_scp.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c index 0f4a7065d0bd9..8206a17664818 100644 --- a/drivers/remoteproc/mtk_scp.c +++ b/drivers/remoteproc/mtk_scp.c @@ -1326,6 +1326,11 @@ static int scp_cluster_init(struct platform_device *pdev, struct mtk_scp_of_clus return ret; } +static const struct of_device_id scp_core_match[] = { + { .compatible = "mediatek,scp-core" }, + {} +}; + static int scp_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -1357,13 +1362,15 @@ static int scp_probe(struct platform_device *pdev) INIT_LIST_HEAD(&scp_cluster->mtk_scp_list); mutex_init(&scp_cluster->cluster_lock); - ret = devm_of_platform_populate(dev); + ret = of_platform_populate(dev_of_node(dev), scp_core_match, NULL, dev); if (ret) return dev_err_probe(dev, ret, "Failed to populate platform devices\n"); ret = scp_cluster_init(pdev, scp_cluster); - if (ret) + if (ret) { + of_platform_depopulate(dev); return ret; + } return 0; } @@ -1379,6 +1386,7 @@ static void scp_remove(struct platform_device *pdev) rproc_del(scp->rproc); scp_free(scp); } + of_platform_depopulate(&pdev->dev); mutex_destroy(&scp_cluster->cluster_lock); } From 972361e397797320a624d1a5b457520c10ab4a28 Mon Sep 17 00:00:00 2001 From: Beleswar Padhi Date: Thu, 19 Dec 2024 16:35:41 +0530 Subject: [PATCH 04/13] remoteproc: k3-r5: Add devm action to release reserved memory Use a device lifecycle managed action to release reserved memory. This helps prevent mistakes like releasing out of order in cleanup functions and forgetting to release on error paths. Signed-off-by: Beleswar Padhi Reviewed-by: Andrew Davis Link: https://lore.kernel.org/r/20241219110545.1898883-2-b-padhi@ti.com Signed-off-by: Mathieu Poirier --- drivers/remoteproc/ti_k3_r5_remoteproc.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c index 6560b7954027f..d51f88a5abc13 100644 --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c @@ -955,6 +955,13 @@ static int k3_r5_rproc_configure(struct k3_r5_rproc *kproc) return ret; } +static void k3_r5_mem_release(void *data) +{ + struct device *dev = data; + + of_reserved_mem_device_release(dev); +} + static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc) { struct device *dev = kproc->dev; @@ -985,12 +992,14 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc) return ret; } + ret = devm_add_action_or_reset(dev, k3_r5_mem_release, dev); + if (ret) + return ret; + num_rmems--; kproc->rmem = kcalloc(num_rmems, sizeof(*kproc->rmem), GFP_KERNEL); - if (!kproc->rmem) { - ret = -ENOMEM; - goto release_rmem; - } + if (!kproc->rmem) + return -ENOMEM; /* use remaining reserved memory regions for static carveouts */ for (i = 0; i < num_rmems; i++) { @@ -1041,8 +1050,6 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc) for (i--; i >= 0; i--) iounmap(kproc->rmem[i].cpu_addr); kfree(kproc->rmem); -release_rmem: - of_reserved_mem_device_release(dev); return ret; } @@ -1053,8 +1060,6 @@ static void k3_r5_reserved_mem_exit(struct k3_r5_rproc *kproc) for (i = 0; i < kproc->num_rmems; i++) iounmap(kproc->rmem[i].cpu_addr); kfree(kproc->rmem); - - of_reserved_mem_device_release(kproc->dev); } /* From f2e3d0d70986b1f135963dc28462fce4e65c0fc4 Mon Sep 17 00:00:00 2001 From: Beleswar Padhi Date: Thu, 19 Dec 2024 16:35:42 +0530 Subject: [PATCH 05/13] remoteproc: k3-r5: Use devm_kcalloc() helper Use a device lifecycle managed action to free memory. This helps prevent mistakes like freeing out of order in cleanup functions and forgetting to free on error paths. Signed-off-by: Beleswar Padhi Reviewed-by: Andrew Davis Link: https://lore.kernel.org/r/20241219110545.1898883-3-b-padhi@ti.com Signed-off-by: Mathieu Poirier --- drivers/remoteproc/ti_k3_r5_remoteproc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c index d51f88a5abc13..0753a5c35c7e5 100644 --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c @@ -997,7 +997,7 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc) return ret; num_rmems--; - kproc->rmem = kcalloc(num_rmems, sizeof(*kproc->rmem), GFP_KERNEL); + kproc->rmem = devm_kcalloc(dev, num_rmems, sizeof(*kproc->rmem), GFP_KERNEL); if (!kproc->rmem) return -ENOMEM; @@ -1049,7 +1049,6 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc) unmap_rmem: for (i--; i >= 0; i--) iounmap(kproc->rmem[i].cpu_addr); - kfree(kproc->rmem); return ret; } @@ -1059,7 +1058,6 @@ static void k3_r5_reserved_mem_exit(struct k3_r5_rproc *kproc) for (i = 0; i < kproc->num_rmems; i++) iounmap(kproc->rmem[i].cpu_addr); - kfree(kproc->rmem); } /* From a572439f7143db5ea8446b7d755a16dfb12da7c7 Mon Sep 17 00:00:00 2001 From: Beleswar Padhi Date: Thu, 19 Dec 2024 16:35:43 +0530 Subject: [PATCH 06/13] remoteproc: k3-r5: Use devm_ioremap_wc() helper Use a device lifecycle managed ioremap helper function. This helps prevent mistakes like unmapping out of order in cleanup functions and forgetting to unmap on all error paths. Signed-off-by: Beleswar Padhi Reviewed-by: Andrew Davis Link: https://lore.kernel.org/r/20241219110545.1898883-4-b-padhi@ti.com Signed-off-by: Mathieu Poirier --- drivers/remoteproc/ti_k3_r5_remoteproc.c | 38 +++++------------------- 1 file changed, 8 insertions(+), 30 deletions(-) diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c index 0753a5c35c7e5..3493bae95fdfa 100644 --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c @@ -1004,17 +1004,13 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc) /* use remaining reserved memory regions for static carveouts */ for (i = 0; i < num_rmems; i++) { rmem_np = of_parse_phandle(np, "memory-region", i + 1); - if (!rmem_np) { - ret = -EINVAL; - goto unmap_rmem; - } + if (!rmem_np) + return -EINVAL; rmem = of_reserved_mem_lookup(rmem_np); of_node_put(rmem_np); - if (!rmem) { - ret = -EINVAL; - goto unmap_rmem; - } + if (!rmem) + return -EINVAL; kproc->rmem[i].bus_addr = rmem->base; /* @@ -1029,12 +1025,11 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc) */ kproc->rmem[i].dev_addr = (u32)rmem->base; kproc->rmem[i].size = rmem->size; - kproc->rmem[i].cpu_addr = ioremap_wc(rmem->base, rmem->size); + kproc->rmem[i].cpu_addr = devm_ioremap_wc(dev, rmem->base, rmem->size); if (!kproc->rmem[i].cpu_addr) { dev_err(dev, "failed to map reserved memory#%d at %pa of size %pa\n", i + 1, &rmem->base, &rmem->size); - ret = -ENOMEM; - goto unmap_rmem; + return -ENOMEM; } dev_dbg(dev, "reserved memory%d: bus addr %pa size 0x%zx va %pK da 0x%x\n", @@ -1045,19 +1040,6 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc) kproc->num_rmems = num_rmems; return 0; - -unmap_rmem: - for (i--; i >= 0; i--) - iounmap(kproc->rmem[i].cpu_addr); - return ret; -} - -static void k3_r5_reserved_mem_exit(struct k3_r5_rproc *kproc) -{ - int i; - - for (i = 0; i < kproc->num_rmems; i++) - iounmap(kproc->rmem[i].cpu_addr); } /* @@ -1286,8 +1268,8 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) ret = rproc_add(rproc); if (ret) { - dev_err(dev, "rproc_add failed, ret = %d\n", ret); - goto err_add; + dev_err_probe(dev, ret, "rproc_add failed\n"); + goto out; } /* create only one rproc in lockstep, single-cpu or @@ -1333,8 +1315,6 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) err_powerup: rproc_del(rproc); -err_add: - k3_r5_reserved_mem_exit(kproc); out: /* undo core0 upon any failures on core1 in split-mode */ if (cluster->mode == CLUSTER_MODE_SPLIT && core == core1) { @@ -1379,8 +1359,6 @@ static void k3_r5_cluster_rproc_exit(void *data) mbox_free_channel(kproc->mbox); rproc_del(rproc); - - k3_r5_reserved_mem_exit(kproc); } } From de182d2f5ca0801425919f38ec955033c09c601d Mon Sep 17 00:00:00 2001 From: Beleswar Padhi Date: Thu, 19 Dec 2024 16:35:44 +0530 Subject: [PATCH 07/13] remoteproc: k3-r5: Use devm_rproc_add() helper Use device lifecycle managed devm_rproc_add() helper function. This helps prevent mistakes like deleting out of order in cleanup functions and forgetting to delete on all error paths. Signed-off-by: Beleswar Padhi Reviewed-by: Andrew Davis Link: https://lore.kernel.org/r/20241219110545.1898883-5-b-padhi@ti.com Signed-off-by: Mathieu Poirier --- drivers/remoteproc/ti_k3_r5_remoteproc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c index 3493bae95fdfa..30b72bcb4d689 100644 --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c @@ -1266,7 +1266,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) goto out; } - ret = rproc_add(rproc); + ret = devm_rproc_add(dev, rproc); if (ret) { dev_err_probe(dev, ret, "rproc_add failed\n"); goto out; @@ -1297,7 +1297,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) dev_err(dev, "Timed out waiting for %s core to power up!\n", rproc->name); - goto err_powerup; + goto out; } } @@ -1313,8 +1313,6 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) } } -err_powerup: - rproc_del(rproc); out: /* undo core0 upon any failures on core1 in split-mode */ if (cluster->mode == CLUSTER_MODE_SPLIT && core == core1) { @@ -1357,8 +1355,6 @@ static void k3_r5_cluster_rproc_exit(void *data) } mbox_free_channel(kproc->mbox); - - rproc_del(rproc); } } From 8c8df9bd3851978c2645f1a3837230adef82fe0e Mon Sep 17 00:00:00 2001 From: Beleswar Padhi Date: Thu, 19 Dec 2024 16:35:45 +0530 Subject: [PATCH 08/13] remoteproc: k3-r5: Add devm action to release tsp Use a device lifecycle managed action to release tsp ti_sci_proc handle. This helps prevent mistakes like releasing out of order in cleanup functions and forgetting to release on error paths. Signed-off-by: Beleswar Padhi Reviewed-by: Andrew Davis Link: https://lore.kernel.org/r/20241219110545.1898883-6-b-padhi@ti.com Signed-off-by: Mathieu Poirier --- drivers/remoteproc/ti_k3_r5_remoteproc.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c index 30b72bcb4d689..dbc513c5569cb 100644 --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c @@ -1487,6 +1487,13 @@ static int k3_r5_core_of_get_sram_memories(struct platform_device *pdev, return 0; } +static void k3_r5_release_tsp(void *data) +{ + struct ti_sci_proc *tsp = data; + + ti_sci_proc_release(tsp); +} + static int k3_r5_core_of_init(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -1580,6 +1587,10 @@ static int k3_r5_core_of_init(struct platform_device *pdev) goto err; } + ret = devm_add_action_or_reset(dev, k3_r5_release_tsp, core->tsp); + if (ret) + goto err; + platform_set_drvdata(pdev, core); devres_close_group(dev, k3_r5_core_of_init); @@ -1596,13 +1607,7 @@ static int k3_r5_core_of_init(struct platform_device *pdev) */ static void k3_r5_core_of_exit(struct platform_device *pdev) { - struct k3_r5_core *core = platform_get_drvdata(pdev); struct device *dev = &pdev->dev; - int ret; - - ret = ti_sci_proc_release(core->tsp); - if (ret) - dev_err(dev, "failed to release proc, ret = %d\n", ret); platform_set_drvdata(pdev, NULL); devres_release_group(dev, k3_r5_core_of_init); From bd047185432839d058d58b71751321724cc88f3a Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Sat, 11 Jan 2025 19:42:49 +0100 Subject: [PATCH 09/13] remoteproc: keystone: Simplify returning syscon PTR_ERR No need to store PTR_ERR into temporary, local 'ret' variable. Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20250111-b4-syscon-phandle-args-remoteproc-v1-1-73ed6fafa1e3@linaro.org Signed-off-by: Mathieu Poirier --- drivers/remoteproc/keystone_remoteproc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c index 6e54093d17323..83c8e568f7ce3 100644 --- a/drivers/remoteproc/keystone_remoteproc.c +++ b/drivers/remoteproc/keystone_remoteproc.c @@ -335,7 +335,6 @@ static int keystone_rproc_of_get_dev_syscon(struct platform_device *pdev, { struct device_node *np = pdev->dev.of_node; struct device *dev = &pdev->dev; - int ret; if (!of_property_read_bool(np, "ti,syscon-dev")) { dev_err(dev, "ti,syscon-dev property is absent\n"); @@ -344,10 +343,8 @@ static int keystone_rproc_of_get_dev_syscon(struct platform_device *pdev, ksproc->dev_ctrl = syscon_regmap_lookup_by_phandle(np, "ti,syscon-dev"); - if (IS_ERR(ksproc->dev_ctrl)) { - ret = PTR_ERR(ksproc->dev_ctrl); - return ret; - } + if (IS_ERR(ksproc->dev_ctrl)) + return PTR_ERR(ksproc->dev_ctrl); if (of_property_read_u32_index(np, "ti,syscon-dev", 1, &ksproc->boot_offset)) { From cf1d4fdb4716c8a734431a1b99012ac55754b63d Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Sat, 11 Jan 2025 19:42:50 +0100 Subject: [PATCH 10/13] remoteproc: omap: Simplify returning syscon PTR_ERR No need to store PTR_ERR into temporary, local 'ret' variable. Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20250111-b4-syscon-phandle-args-remoteproc-v1-2-73ed6fafa1e3@linaro.org Signed-off-by: Mathieu Poirier --- drivers/remoteproc/omap_remoteproc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c index 3260dd512491e..5f463937cbbff 100644 --- a/drivers/remoteproc/omap_remoteproc.c +++ b/drivers/remoteproc/omap_remoteproc.c @@ -1137,7 +1137,6 @@ static int omap_rproc_get_boot_data(struct platform_device *pdev, struct device_node *np = pdev->dev.of_node; struct omap_rproc *oproc = rproc->priv; const struct omap_rproc_dev_data *data; - int ret; data = of_device_get_match_data(&pdev->dev); if (!data) @@ -1153,10 +1152,8 @@ static int omap_rproc_get_boot_data(struct platform_device *pdev, oproc->boot_data->syscon = syscon_regmap_lookup_by_phandle(np, "ti,bootreg"); - if (IS_ERR(oproc->boot_data->syscon)) { - ret = PTR_ERR(oproc->boot_data->syscon); - return ret; - } + if (IS_ERR(oproc->boot_data->syscon)) + return PTR_ERR(oproc->boot_data->syscon); if (of_property_read_u32_index(np, "ti,bootreg", 1, &oproc->boot_data->boot_reg)) { From 266ce6e408c91a54f01175f496d6f2fe682900fb Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Sat, 11 Jan 2025 19:42:51 +0100 Subject: [PATCH 11/13] remoteproc: st: Simplify with dev_err_probe Use dev_err_probe() to make error code handling simpler and handle deferred probe. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Patrice Chotard Link: https://lore.kernel.org/r/20250111-b4-syscon-phandle-args-remoteproc-v1-3-73ed6fafa1e3@linaro.org Signed-off-by: Mathieu Poirier --- drivers/remoteproc/st_remoteproc.c | 44 ++++++++++++++---------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c index 5df99bae7131a..d1f35e8a83ba5 100644 --- a/drivers/remoteproc/st_remoteproc.c +++ b/drivers/remoteproc/st_remoteproc.c @@ -290,26 +290,23 @@ static int st_rproc_parse_dt(struct platform_device *pdev) if (ddata->config->sw_reset) { ddata->sw_reset = devm_reset_control_get_exclusive(dev, "sw_reset"); - if (IS_ERR(ddata->sw_reset)) { - dev_err(dev, "Failed to get S/W Reset\n"); - return PTR_ERR(ddata->sw_reset); - } + if (IS_ERR(ddata->sw_reset)) + return dev_err_probe(dev, PTR_ERR(ddata->sw_reset), + "Failed to get S/W Reset\n"); } if (ddata->config->pwr_reset) { ddata->pwr_reset = devm_reset_control_get_exclusive(dev, "pwr_reset"); - if (IS_ERR(ddata->pwr_reset)) { - dev_err(dev, "Failed to get Power Reset\n"); - return PTR_ERR(ddata->pwr_reset); - } + if (IS_ERR(ddata->pwr_reset)) + return dev_err_probe(dev, PTR_ERR(ddata->pwr_reset), + "Failed to get Power Reset\n"); } ddata->clk = devm_clk_get(dev, NULL); - if (IS_ERR(ddata->clk)) { - dev_err(dev, "Failed to get clock\n"); - return PTR_ERR(ddata->clk); - } + if (IS_ERR(ddata->clk)) + return dev_err_probe(dev, PTR_ERR(ddata->clk), + "Failed to get clock\n"); err = of_property_read_u32(np, "clock-frequency", &ddata->clk_rate); if (err) { @@ -318,10 +315,9 @@ static int st_rproc_parse_dt(struct platform_device *pdev) } ddata->boot_base = syscon_regmap_lookup_by_phandle(np, "st,syscfg"); - if (IS_ERR(ddata->boot_base)) { - dev_err(dev, "Boot base not found\n"); - return PTR_ERR(ddata->boot_base); - } + if (IS_ERR(ddata->boot_base)) + return dev_err_probe(dev, PTR_ERR(ddata->boot_base), + "Boot base not found\n"); err = of_property_read_u32_index(np, "st,syscfg", 1, &ddata->boot_offset); @@ -395,32 +391,32 @@ static int st_rproc_probe(struct platform_device *pdev) */ chan = mbox_request_channel_byname(&ddata->mbox_client_vq0, "vq0_rx"); if (IS_ERR(chan)) { - dev_err(&rproc->dev, "failed to request mbox chan 0\n"); - ret = PTR_ERR(chan); + ret = dev_err_probe(&rproc->dev, PTR_ERR(chan), + "failed to request mbox chan 0\n"); goto free_clk; } ddata->mbox_chan[ST_RPROC_VQ0 * MBOX_MAX + MBOX_RX] = chan; chan = mbox_request_channel_byname(&ddata->mbox_client_vq0, "vq0_tx"); if (IS_ERR(chan)) { - dev_err(&rproc->dev, "failed to request mbox chan 0\n"); - ret = PTR_ERR(chan); + ret = dev_err_probe(&rproc->dev, PTR_ERR(chan), + "failed to request mbox chan 0\n"); goto free_mbox; } ddata->mbox_chan[ST_RPROC_VQ0 * MBOX_MAX + MBOX_TX] = chan; chan = mbox_request_channel_byname(&ddata->mbox_client_vq1, "vq1_rx"); if (IS_ERR(chan)) { - dev_err(&rproc->dev, "failed to request mbox chan 1\n"); - ret = PTR_ERR(chan); + ret = dev_err_probe(&rproc->dev, PTR_ERR(chan), + "failed to request mbox chan 1\n"); goto free_mbox; } ddata->mbox_chan[ST_RPROC_VQ1 * MBOX_MAX + MBOX_RX] = chan; chan = mbox_request_channel_byname(&ddata->mbox_client_vq1, "vq1_tx"); if (IS_ERR(chan)) { - dev_err(&rproc->dev, "failed to request mbox chan 1\n"); - ret = PTR_ERR(chan); + ret = dev_err_probe(&rproc->dev, PTR_ERR(chan), + "failed to request mbox chan 1\n"); goto free_mbox; } ddata->mbox_chan[ST_RPROC_VQ1 * MBOX_MAX + MBOX_TX] = chan; From fed4ec7442eb0e37eefae4a9602d9d93335f251c Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Sat, 11 Jan 2025 19:42:52 +0100 Subject: [PATCH 12/13] remoteproc: keystone: Use syscon_regmap_lookup_by_phandle_args Use syscon_regmap_lookup_by_phandle_args() which is a wrapper over syscon_regmap_lookup_by_phandle() and getting the argument. Except simpler code this annotates within one line that given phandle has arguments, so grepping for code would be easier. There is also no real benefit in printing errors on missing syscon argument, because this is done just too late: runtime check on static/build-time data. Dtschema and Devicetree bindings offer the static/build-time check for this already. Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20250111-b4-syscon-phandle-args-remoteproc-v1-4-73ed6fafa1e3@linaro.org Signed-off-by: Mathieu Poirier --- drivers/remoteproc/keystone_remoteproc.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c index 83c8e568f7ce3..7b41b4547fa80 100644 --- a/drivers/remoteproc/keystone_remoteproc.c +++ b/drivers/remoteproc/keystone_remoteproc.c @@ -341,17 +341,11 @@ static int keystone_rproc_of_get_dev_syscon(struct platform_device *pdev, return -EINVAL; } - ksproc->dev_ctrl = - syscon_regmap_lookup_by_phandle(np, "ti,syscon-dev"); + ksproc->dev_ctrl = syscon_regmap_lookup_by_phandle_args(np, "ti,syscon-dev", + 1, &ksproc->boot_offset); if (IS_ERR(ksproc->dev_ctrl)) return PTR_ERR(ksproc->dev_ctrl); - if (of_property_read_u32_index(np, "ti,syscon-dev", 1, - &ksproc->boot_offset)) { - dev_err(dev, "couldn't read the boot register offset\n"); - return -EINVAL; - } - return 0; } From 3a53ff95b0be9a5d0ef5037e539558d0041f9a89 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Sat, 11 Jan 2025 19:42:53 +0100 Subject: [PATCH 13/13] remoteproc: st: Use syscon_regmap_lookup_by_phandle_args Use syscon_regmap_lookup_by_phandle_args() which is a wrapper over syscon_regmap_lookup_by_phandle() and getting the argument. Except simpler code this annotates within one line that given phandle has arguments, so grepping for code would be easier. There is also no real benefit in printing errors on missing syscon argument, because this is done just too late: runtime check on static/build-time data. Dtschema and Devicetree bindings offer the static/build-time check for this already. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Patrice Chotard Link: https://lore.kernel.org/r/20250111-b4-syscon-phandle-args-remoteproc-v1-5-73ed6fafa1e3@linaro.org Signed-off-by: Mathieu Poirier --- drivers/remoteproc/st_remoteproc.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c index d1f35e8a83ba5..e6566a9839dc5 100644 --- a/drivers/remoteproc/st_remoteproc.c +++ b/drivers/remoteproc/st_remoteproc.c @@ -314,18 +314,12 @@ static int st_rproc_parse_dt(struct platform_device *pdev) return err; } - ddata->boot_base = syscon_regmap_lookup_by_phandle(np, "st,syscfg"); + ddata->boot_base = syscon_regmap_lookup_by_phandle_args(np, "st,syscfg", + 1, &ddata->boot_offset); if (IS_ERR(ddata->boot_base)) return dev_err_probe(dev, PTR_ERR(ddata->boot_base), "Boot base not found\n"); - err = of_property_read_u32_index(np, "st,syscfg", 1, - &ddata->boot_offset); - if (err) { - dev_err(dev, "Boot offset not found\n"); - return -EINVAL; - } - err = clk_prepare(ddata->clk); if (err) dev_err(dev, "failed to get clock\n");