From ee67247843a2b62d1473cfa4df300e69b5190ccf Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Fri, 17 Oct 2025 09:56:24 +0800 Subject: [PATCH 1/8] firmware: imx: scu-irq: fix OF node leak in imx_scu_enable_general_irq_channel() calls of_parse_phandle_with_args(), but does not release the OF node reference. Add a of_node_put() call to release the reference. Fixes: 851826c7566e ("firmware: imx: enable imx scu general irq function") Reviewed-by: Frank Li Signed-off-by: Peng Fan Signed-off-by: Shawn Guo --- drivers/firmware/imx/imx-scu-irq.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c index 6125cccc9ba7..f2b902e95b73 100644 --- a/drivers/firmware/imx/imx-scu-irq.c +++ b/drivers/firmware/imx/imx-scu-irq.c @@ -226,8 +226,10 @@ int imx_scu_enable_general_irq_channel(struct device *dev) INIT_WORK(&imx_sc_irq_work, imx_scu_irq_work_handler); if (!of_parse_phandle_with_args(dev->of_node, "mboxes", - "#mbox-cells", 0, &spec)) + "#mbox-cells", 0, &spec)) { i = of_alias_get_id(spec.np, "mu"); + of_node_put(spec.np); + } /* use mu1 as general mu irq channel if failed */ if (i < 0) From 62c740fb11ea7790f6bcd4f72c563f9ae2a42e9a Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Fri, 17 Oct 2025 09:56:25 +0800 Subject: [PATCH 2/8] firmware: imx: scu-irq: Free mailbox client on failure at imx_scu_enable_general_irq_channel() The IRQ mailbox is an optional channel and does not need to be kept until driver removal when an error occurs. Free the allocated memory in the error path. Add 'goto free_cl' when mbox_request_channel_byname() fails, to keep free at one place. Signed-off-by: Peng Fan Reviewed-by: Frank Li Signed-off-by: Shawn Guo --- drivers/firmware/imx/imx-scu-irq.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c index f2b902e95b73..d6fd04404e2a 100644 --- a/drivers/firmware/imx/imx-scu-irq.c +++ b/drivers/firmware/imx/imx-scu-irq.c @@ -219,8 +219,7 @@ int imx_scu_enable_general_irq_channel(struct device *dev) if (IS_ERR(ch)) { ret = PTR_ERR(ch); dev_err(dev, "failed to request mbox chan gip3, ret %d\n", ret); - devm_kfree(dev, cl); - return ret; + goto free_cl; } INIT_WORK(&imx_sc_irq_work, imx_scu_irq_work_handler); @@ -255,6 +254,8 @@ int imx_scu_enable_general_irq_channel(struct device *dev) free_ch: mbox_free_channel(ch); +free_cl: + devm_kfree(dev, cl); return ret; } From 81fb53feb66a3aefbf6fcab73bb8d06f5b0c54ad Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Fri, 17 Oct 2025 09:56:26 +0800 Subject: [PATCH 3/8] firmware: imx: scu-irq: Init workqueue before request mbox channel With mailbox channel requested, there is possibility that interrupts may come in, so need to make sure the workqueue is initialized before the queue is scheduled by mailbox rx callback. Reviewed-by: Frank Li Signed-off-by: Peng Fan Signed-off-by: Shawn Guo --- drivers/firmware/imx/imx-scu-irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c index d6fd04404e2a..2d7237188932 100644 --- a/drivers/firmware/imx/imx-scu-irq.c +++ b/drivers/firmware/imx/imx-scu-irq.c @@ -214,6 +214,8 @@ int imx_scu_enable_general_irq_channel(struct device *dev) cl->dev = dev; cl->rx_callback = imx_scu_irq_callback; + INIT_WORK(&imx_sc_irq_work, imx_scu_irq_work_handler); + /* SCU general IRQ uses general interrupt channel 3 */ ch = mbox_request_channel_byname(cl, "gip3"); if (IS_ERR(ch)) { @@ -222,8 +224,6 @@ int imx_scu_enable_general_irq_channel(struct device *dev) goto free_cl; } - INIT_WORK(&imx_sc_irq_work, imx_scu_irq_work_handler); - if (!of_parse_phandle_with_args(dev->of_node, "mboxes", "#mbox-cells", 0, &spec)) { i = of_alias_get_id(spec.np, "mu"); From ff3f9913bc0749364fbfd86ea62ba2d31c6136c8 Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Fri, 17 Oct 2025 09:56:27 +0800 Subject: [PATCH 4/8] firmware: imx: scu-irq: Set mu_resource_id before get handle mu_resource_id is referenced in imx_scu_irq_get_status() and imx_scu_irq_group_enable() which could be used by other modules, so need to set correct value before using imx_sc_irq_ipc_handle in SCU API call. Reviewed-by: Frank Li Signed-off-by: Peng Fan Signed-off-by: Shawn Guo --- drivers/firmware/imx/imx-scu-irq.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c index 2d7237188932..3dda42be4e6b 100644 --- a/drivers/firmware/imx/imx-scu-irq.c +++ b/drivers/firmware/imx/imx-scu-irq.c @@ -203,6 +203,18 @@ int imx_scu_enable_general_irq_channel(struct device *dev) struct mbox_chan *ch; int ret = 0, i = 0; + if (!of_parse_phandle_with_args(dev->of_node, "mboxes", + "#mbox-cells", 0, &spec)) { + i = of_alias_get_id(spec.np, "mu"); + of_node_put(spec.np); + } + + /* use mu1 as general mu irq channel if failed */ + if (i < 0) + i = 1; + + mu_resource_id = IMX_SC_R_MU_0A + i; + ret = imx_scu_get_handle(&imx_sc_irq_ipc_handle); if (ret) return ret; @@ -224,18 +236,6 @@ int imx_scu_enable_general_irq_channel(struct device *dev) goto free_cl; } - if (!of_parse_phandle_with_args(dev->of_node, "mboxes", - "#mbox-cells", 0, &spec)) { - i = of_alias_get_id(spec.np, "mu"); - of_node_put(spec.np); - } - - /* use mu1 as general mu irq channel if failed */ - if (i < 0) - i = 1; - - mu_resource_id = IMX_SC_R_MU_0A + i; - /* Create directory under /sysfs/firmware */ wakeup_obj = kobject_create_and_add("scu_wakeup_source", firmware_kobj); if (!wakeup_obj) { From ea2f83c6aa027d42249fa73e833213c0d919d1fd Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Fri, 17 Oct 2025 09:56:28 +0800 Subject: [PATCH 5/8] firmware: imx: scu-irq: Remove unused export of imx_scu_enable_general_irq_channel Since its introduction, this symbol has not been used by any loadable modules. It remains only referenced within imx-scu.c, which is always built together with imx-scu-irq.c As such, exporting imx_scu_enable_general_irq_channel is unnecessary, so remove the export. Reviewed-by: Frank Li Signed-off-by: Peng Fan Signed-off-by: Shawn Guo --- drivers/firmware/imx/imx-scu-irq.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c index 3dda42be4e6b..a68d38f89254 100644 --- a/drivers/firmware/imx/imx-scu-irq.c +++ b/drivers/firmware/imx/imx-scu-irq.c @@ -259,4 +259,3 @@ free_cl: return ret; } -EXPORT_SYMBOL(imx_scu_enable_general_irq_channel); From 27d408697f0ca09806e49ae5e5d3fceae7a671d4 Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Fri, 17 Oct 2025 09:56:29 +0800 Subject: [PATCH 6/8] firmware: imx: scu: Update error code IMX_SC_ERR_NOTFOUND should map with -ENOENT, not -EEXIST. -ENODEV makes more sense for IMX_SC_ERR_NOPOWER, and -ECOMM makes more sense for IMX_SC_ERR_IPC. Reviewed-by: Frank Li Signed-off-by: Peng Fan Signed-off-by: Shawn Guo --- drivers/firmware/imx/imx-scu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c index 8c28e25ddc8a..6046156bc3c9 100644 --- a/drivers/firmware/imx/imx-scu.c +++ b/drivers/firmware/imx/imx-scu.c @@ -73,9 +73,9 @@ static int imx_sc_linux_errmap[IMX_SC_ERR_LAST] = { -EACCES, /* IMX_SC_ERR_NOACCESS */ -EACCES, /* IMX_SC_ERR_LOCKED */ -ERANGE, /* IMX_SC_ERR_UNAVAILABLE */ - -EEXIST, /* IMX_SC_ERR_NOTFOUND */ - -EPERM, /* IMX_SC_ERR_NOPOWER */ - -EPIPE, /* IMX_SC_ERR_IPC */ + -ENOENT, /* IMX_SC_ERR_NOTFOUND */ + -ENODEV, /* IMX_SC_ERR_NOPOWER */ + -ECOMM, /* IMX_SC_ERR_IPC */ -EBUSY, /* IMX_SC_ERR_BUSY */ -EIO, /* IMX_SC_ERR_FAIL */ }; From ff79af939d84791b1af49492431c1520f31e671b Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Fri, 17 Oct 2025 09:56:30 +0800 Subject: [PATCH 7/8] firmware: imx: scu: Suppress bind attrs The SCU driver is critical for system working properly, it should never be removed and binded again. So suppress the bind attrs Reviewed-by: Frank Li Signed-off-by: Peng Fan Signed-off-by: Shawn Guo --- drivers/firmware/imx/imx-scu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c index 6046156bc3c9..630e3dba4db1 100644 --- a/drivers/firmware/imx/imx-scu.c +++ b/drivers/firmware/imx/imx-scu.c @@ -352,6 +352,7 @@ static struct platform_driver imx_scu_driver = { .driver = { .name = "imx-scu", .of_match_table = imx_scu_match, + .suppress_bind_attrs = true, }, .probe = imx_scu_probe, }; From 97a07dd2b559d149598cd49574ce50614752211a Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Fri, 17 Oct 2025 09:56:31 +0800 Subject: [PATCH 8/8] firmware: imx: scu: Use devm_mutex_init In normal case, there is no need to invoke mutex_destroy in error path, but it is useful when CONFIG_DEBUG_MUTEXES, so use devm_mutex_init(). Reviewed-by: Frank Li Signed-off-by: Peng Fan Signed-off-by: Shawn Guo --- drivers/firmware/imx/imx-scu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c index 630e3dba4db1..67b267a7408a 100644 --- a/drivers/firmware/imx/imx-scu.c +++ b/drivers/firmware/imx/imx-scu.c @@ -324,7 +324,9 @@ static int imx_scu_probe(struct platform_device *pdev) } sc_ipc->dev = dev; - mutex_init(&sc_ipc->lock); + ret = devm_mutex_init(dev, &sc_ipc->lock); + if (ret) + return ret; init_completion(&sc_ipc->done); imx_sc_ipc_handle = sc_ipc;