* [PATCH] mailbox: Remove redundant dev_err()/dev_err_probe()
@ 2026-07-16 2:52 Pan Chuang
2026-07-16 3:02 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Pan Chuang @ 2026-07-16 2:52 UTC (permalink / raw)
To: Jassi Brar, Viresh Kumar, Tushar Khandelwal, Sudeep Holla,
Cristian Marussi, Marek Behún, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Conor Dooley, Daire McNamara, Sibi Sankar, Manivannan Sadhasivam,
Orson Zhai, Baolin Wang, Chunyan Zhang, Maxime Coquelin,
Alexandre Torgue, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Thierry Reding, Jonathan Hunter, open list:MAILBOX API,
moderated list:MAILBOX ARM MHUv3,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:RISC-V MICROCHIP SUPPORT,
open list:QUALCOMM CPUCP MAILBOX DRIVER,
moderated list:ARM/STM32 ARCHITECTURE,
open list:ARM/Allwinner sunXi SoC support,
open list:TEGRA ARCHITECTURE SUPPORT
Cc: Pan Chuang
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_irq() and devm_request_threaded_irq()
automatically log detailed error messages on failure. Remove the
now-redundant driver-specific dev_err() and dev_err_probe() calls.
Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
drivers/mailbox/arm_mhu_db.c | 1 -
drivers/mailbox/arm_mhuv2.c | 9 ++-------
drivers/mailbox/arm_mhuv3.c | 5 ++---
drivers/mailbox/armada-37xx-rwtm-mailbox.c | 4 +---
drivers/mailbox/bcm-pdc-mailbox.c | 5 +----
drivers/mailbox/bcm2835-mailbox.c | 5 +----
drivers/mailbox/bcm74110-mailbox.c | 2 +-
drivers/mailbox/cv1800-mailbox.c | 2 +-
drivers/mailbox/hi6220-mailbox.c | 5 +----
drivers/mailbox/mailbox-mpfs.c | 2 --
drivers/mailbox/mailbox-sti.c | 4 +---
drivers/mailbox/qcom-cpucp-mbox.c | 2 +-
drivers/mailbox/qcom-ipcc.c | 4 +---
drivers/mailbox/sprd-mailbox.c | 12 +++---------
drivers/mailbox/stm32-ipcc.c | 4 +---
drivers/mailbox/sun6i-msgbox.c | 4 +---
drivers/mailbox/tegra-hsp.c | 11 ++---------
17 files changed, 20 insertions(+), 61 deletions(-)
diff --git a/drivers/mailbox/arm_mhu_db.c b/drivers/mailbox/arm_mhu_db.c
index 9e937b09c5fb..a39239e38a47 100644
--- a/drivers/mailbox/arm_mhu_db.c
+++ b/drivers/mailbox/arm_mhu_db.c
@@ -318,7 +318,6 @@ static int mhu_db_probe(struct amba_device *adev, const struct amba_id *id)
mhu_db_mbox_rx_handler,
IRQF_ONESHOT, "mhu_db_link", mhu);
if (err) {
- dev_err(dev, "Can't claim IRQ %d\n", irq);
mbox_controller_unregister(&mhu->mbox);
return err;
}
diff --git a/drivers/mailbox/arm_mhuv2.c b/drivers/mailbox/arm_mhuv2.c
index f035284944c0..213fd4a39993 100644
--- a/drivers/mailbox/arm_mhuv2.c
+++ b/drivers/mailbox/arm_mhuv2.c
@@ -986,10 +986,7 @@ static int mhuv2_tx_init(struct amba_device *adev, struct mhuv2 *mhu,
ret = devm_request_threaded_irq(dev, adev->irq[0], NULL,
mhuv2_sender_interrupt,
IRQF_ONESHOT, "mhuv2-tx", mhu);
- if (ret) {
- dev_err(dev, "Failed to request tx IRQ, fallback to polling mode: %d\n",
- ret);
- } else {
+ if (!ret) {
mhu->mbox.txdone_irq = true;
mhu->mbox.txdone_poll = false;
mhu->irq = adev->irq[0];
@@ -1039,10 +1036,8 @@ static int mhuv2_rx_init(struct amba_device *adev, struct mhuv2 *mhu,
ret = devm_request_threaded_irq(dev, mhu->irq, NULL,
mhuv2_receiver_interrupt, IRQF_ONESHOT,
"mhuv2-rx", mhu);
- if (ret) {
- dev_err(dev, "Failed to request rx IRQ\n");
+ if (ret)
return ret;
- }
/* Mask all the channel windows */
for (i = 0; i < mhu->windows; i++)
diff --git a/drivers/mailbox/arm_mhuv3.c b/drivers/mailbox/arm_mhuv3.c
index a1c528be47f3..6569c3dda377 100644
--- a/drivers/mailbox/arm_mhuv3.c
+++ b/drivers/mailbox/arm_mhuv3.c
@@ -982,8 +982,7 @@ static int mhuv3_setup_pbx(struct mhuv3 *mhu)
mhuv3_pbx_comb_interrupt,
IRQF_ONESHOT, "mhuv3-pbx", mhu);
if (ret)
- return dev_err_probe(dev, ret,
- "Failed to request PBX IRQ\n");
+ return ret;
mhu->mbox.txdone_irq = true;
mhu->mbox.txdone_poll = false;
@@ -1020,7 +1019,7 @@ static int mhuv3_setup_mbx(struct mhuv3 *mhu)
mhuv3_mbx_comb_interrupt, IRQF_ONESHOT,
"mhuv3-mbx", mhu);
if (ret)
- return dev_err_probe(dev, ret, "Failed to request MBX IRQ\n");
+ return ret;
for (i = 0; i < NUM_EXT; i++)
if (mhu->ext[i])
diff --git a/drivers/mailbox/armada-37xx-rwtm-mailbox.c b/drivers/mailbox/armada-37xx-rwtm-mailbox.c
index 456a117a65fd..047926711e96 100644
--- a/drivers/mailbox/armada-37xx-rwtm-mailbox.c
+++ b/drivers/mailbox/armada-37xx-rwtm-mailbox.c
@@ -108,10 +108,8 @@ static int a37xx_mbox_startup(struct mbox_chan *chan)
ret = devm_request_irq(mbox->dev, mbox->irq, a37xx_mbox_irq_handler, 0,
DRIVER_NAME, chan);
- if (ret < 0) {
- dev_err(mbox->dev, "Cannot request irq\n");
+ if (ret < 0)
return ret;
- }
/* enable IRQ generation */
reg = readl(mbox->base + RWTM_HOST_INT_MASK);
diff --git a/drivers/mailbox/bcm-pdc-mailbox.c b/drivers/mailbox/bcm-pdc-mailbox.c
index 406bc41cba60..6fcc4002ad4f 100644
--- a/drivers/mailbox/bcm-pdc-mailbox.c
+++ b/drivers/mailbox/bcm-pdc-mailbox.c
@@ -1403,11 +1403,8 @@ static int pdc_interrupts_init(struct pdc_state *pdcs)
err = devm_request_irq(dev, pdcs->pdc_irq, pdc_irq_handler, 0,
dev_name(dev), dev);
- if (err) {
- dev_err(dev, "IRQ %u request failed with err %d\n",
- pdcs->pdc_irq, err);
+ if (err)
return err;
- }
return PDC_SUCCESS;
}
diff --git a/drivers/mailbox/bcm2835-mailbox.c b/drivers/mailbox/bcm2835-mailbox.c
index ea12fb8d2401..5af1e434c71c 100644
--- a/drivers/mailbox/bcm2835-mailbox.c
+++ b/drivers/mailbox/bcm2835-mailbox.c
@@ -147,11 +147,8 @@ static int bcm2835_mbox_probe(struct platform_device *pdev)
ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
bcm2835_mbox_irq, IRQF_NO_SUSPEND, dev_name(dev),
mbox);
- if (ret) {
- dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n",
- ret);
+ if (ret)
return -ENODEV;
- }
mbox->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(mbox->regs)) {
diff --git a/drivers/mailbox/bcm74110-mailbox.c b/drivers/mailbox/bcm74110-mailbox.c
index 344cfc35984b..c8709d509912 100644
--- a/drivers/mailbox/bcm74110-mailbox.c
+++ b/drivers/mailbox/bcm74110-mailbox.c
@@ -601,7 +601,7 @@ static int bcm74110_mbox_probe(struct platform_device *pdev)
ret = devm_request_irq(dev, mbox->rx_irq, bcm74110_mbox_isr,
IRQF_NO_SUSPEND, pdev->name, mbox);
if (ret)
- return dev_err_probe(dev, ret, "Failed to request irq\n");
+ return ret;
mbox->controller.ops = &bcm74110_mbox_chan_ops;
mbox->controller.dev = dev;
diff --git a/drivers/mailbox/cv1800-mailbox.c b/drivers/mailbox/cv1800-mailbox.c
index 4bca9d8be4ba..b062c2c4fc61 100644
--- a/drivers/mailbox/cv1800-mailbox.c
+++ b/drivers/mailbox/cv1800-mailbox.c
@@ -189,7 +189,7 @@ static int cv1800_mbox_probe(struct platform_device *pdev)
cv1800_mbox_isr, IRQF_ONESHOT,
dev_name(&pdev->dev), mb);
if (err < 0)
- return dev_err_probe(dev, err, "Failed to register irq\n");
+ return err;
for (idx = 0; idx < MAILBOX_MAX_CHAN; idx++) {
mb->priv[idx].idx = idx;
diff --git a/drivers/mailbox/hi6220-mailbox.c b/drivers/mailbox/hi6220-mailbox.c
index 69d15b6283e9..8eb8242209be 100644
--- a/drivers/mailbox/hi6220-mailbox.c
+++ b/drivers/mailbox/hi6220-mailbox.c
@@ -297,11 +297,8 @@ static int hi6220_mbox_probe(struct platform_device *pdev)
err = devm_request_irq(dev, mbox->irq, hi6220_mbox_interrupt, 0,
dev_name(dev), mbox);
- if (err) {
- dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n",
- err);
+ if (err)
return -ENODEV;
- }
mbox->controller.dev = dev;
mbox->controller.chans = &mbox->chan[0];
diff --git a/drivers/mailbox/mailbox-mpfs.c b/drivers/mailbox/mailbox-mpfs.c
index 6c40d865b3f1..704a259bbca8 100644
--- a/drivers/mailbox/mailbox-mpfs.c
+++ b/drivers/mailbox/mailbox-mpfs.c
@@ -219,8 +219,6 @@ static int mpfs_mbox_startup(struct mbox_chan *chan)
return -EINVAL;
ret = devm_request_irq(mbox->dev, mbox->irq, mpfs_mbox_inbox_isr, 0, "mpfs-mailbox", chan);
- if (ret)
- dev_err(mbox->dev, "failed to register mailbox interrupt:%d\n", ret);
return ret;
}
diff --git a/drivers/mailbox/mailbox-sti.c b/drivers/mailbox/mailbox-sti.c
index b6c9ecbbc8ec..4fb5d1b5504a 100644
--- a/drivers/mailbox/mailbox-sti.c
+++ b/drivers/mailbox/mailbox-sti.c
@@ -468,10 +468,8 @@ static int sti_mbox_probe(struct platform_device *pdev)
sti_mbox_irq_handler,
sti_mbox_thread_handler,
IRQF_ONESHOT, mdev->name, mdev);
- if (ret) {
- dev_err(&pdev->dev, "Can't claim IRQ %d\n", irq);
+ if (ret)
return -EINVAL;
- }
dev_info(&pdev->dev, "%s: Registered Tx/Rx Mailbox\n", mdev->name);
diff --git a/drivers/mailbox/qcom-cpucp-mbox.c b/drivers/mailbox/qcom-cpucp-mbox.c
index 862e45e8fbd5..8d0000e2b2f9 100644
--- a/drivers/mailbox/qcom-cpucp-mbox.c
+++ b/drivers/mailbox/qcom-cpucp-mbox.c
@@ -156,7 +156,7 @@ static int qcom_cpucp_mbox_probe(struct platform_device *pdev)
ret = devm_request_irq(dev, irq, qcom_cpucp_mbox_irq_fn,
IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND, "apss_cpucp_mbox", cpucp);
if (ret < 0)
- return dev_err_probe(dev, ret, "Failed to register irq: %d\n", irq);
+ return ret;
writeq(APSS_CPUCP_RX_MBOX_CMD_MASK, cpucp->rx_base + APSS_CPUCP_RX_MBOX_MAP);
diff --git a/drivers/mailbox/qcom-ipcc.c b/drivers/mailbox/qcom-ipcc.c
index d957d989c0ce..e5ebb4a237ff 100644
--- a/drivers/mailbox/qcom-ipcc.c
+++ b/drivers/mailbox/qcom-ipcc.c
@@ -323,10 +323,8 @@ static int qcom_ipcc_probe(struct platform_device *pdev)
ret = devm_request_irq(&pdev->dev, ipcc->irq, qcom_ipcc_irq_fn,
IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND |
IRQF_NO_THREAD, name, ipcc);
- if (ret < 0) {
- dev_err(&pdev->dev, "Failed to register the irq: %d\n", ret);
+ if (ret < 0)
goto err_req_irq;
- }
platform_set_drvdata(pdev, ipcc);
diff --git a/drivers/mailbox/sprd-mailbox.c b/drivers/mailbox/sprd-mailbox.c
index 565502904e1f..9013d2ece55f 100644
--- a/drivers/mailbox/sprd-mailbox.c
+++ b/drivers/mailbox/sprd-mailbox.c
@@ -377,10 +377,8 @@ static int sprd_mbox_probe(struct platform_device *pdev)
ret = devm_request_irq(dev, inbox_irq, sprd_mbox_inbox_isr,
IRQF_NO_SUSPEND, dev_name(dev), priv);
- if (ret) {
- dev_err(dev, "failed to request inbox IRQ: %d\n", ret);
+ if (ret)
return ret;
- }
outbox_irq = platform_get_irq_byname(pdev, "outbox");
if (outbox_irq < 0)
@@ -388,20 +386,16 @@ static int sprd_mbox_probe(struct platform_device *pdev)
ret = devm_request_irq(dev, outbox_irq, sprd_mbox_outbox_isr,
IRQF_NO_SUSPEND, dev_name(dev), priv);
- if (ret) {
- dev_err(dev, "failed to request outbox IRQ: %d\n", ret);
+ if (ret)
return ret;
- }
/* Supplementary outbox IRQ is optional */
supp_irq = platform_get_irq_byname(pdev, "supp-outbox");
if (supp_irq > 0) {
ret = devm_request_irq(dev, supp_irq, sprd_mbox_supp_isr,
IRQF_NO_SUSPEND, dev_name(dev), priv);
- if (ret) {
- dev_err(dev, "failed to request outbox IRQ: %d\n", ret);
+ if (ret)
return ret;
- }
if (!priv->info->supp_id) {
dev_err(dev, "no supplementary outbox specified\n");
diff --git a/drivers/mailbox/stm32-ipcc.c b/drivers/mailbox/stm32-ipcc.c
index 4f63f1a14ca6..c6134de1a93b 100644
--- a/drivers/mailbox/stm32-ipcc.c
+++ b/drivers/mailbox/stm32-ipcc.c
@@ -263,10 +263,8 @@ static int stm32_ipcc_probe(struct platform_device *pdev)
ret = devm_request_threaded_irq(dev, ipcc->irqs[i], NULL,
irq_thread[i], IRQF_ONESHOT,
dev_name(dev), ipcc);
- if (ret) {
- dev_err(dev, "failed to request irq %lu (%d)\n", i, ret);
+ if (ret)
goto err_clk;
- }
}
/* mask and enable rx/tx irq */
diff --git a/drivers/mailbox/sun6i-msgbox.c b/drivers/mailbox/sun6i-msgbox.c
index 6ba6920f4645..1cbc4d478beb 100644
--- a/drivers/mailbox/sun6i-msgbox.c
+++ b/drivers/mailbox/sun6i-msgbox.c
@@ -257,10 +257,8 @@ static int sun6i_msgbox_probe(struct platform_device *pdev)
ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
sun6i_msgbox_irq, 0, dev_name(dev), mbox);
- if (ret) {
- dev_err(dev, "Failed to register IRQ handler: %d\n", ret);
+ if (ret)
goto err_disable_unprepare;
- }
mbox->controller.dev = dev;
mbox->controller.ops = &sun6i_msgbox_chan_ops;
diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
index 500fa77c7d53..03ee73aad488 100644
--- a/drivers/mailbox/tegra-hsp.c
+++ b/drivers/mailbox/tegra-hsp.c
@@ -708,11 +708,8 @@ static int tegra_hsp_request_shared_irq(struct tegra_hsp *hsp)
err = devm_request_irq(hsp->dev, irq, tegra_hsp_shared_irq, 0,
dev_name(hsp->dev), hsp);
- if (err < 0) {
- dev_err(hsp->dev, "failed to request interrupt: %d\n",
- err);
+ if (err < 0)
continue;
- }
hsp->shared_irq = i;
@@ -856,12 +853,8 @@ static int tegra_hsp_probe(struct platform_device *pdev)
err = devm_request_irq(&pdev->dev, hsp->doorbell_irq,
tegra_hsp_doorbell_irq, IRQF_NO_SUSPEND,
dev_name(&pdev->dev), hsp);
- if (err < 0) {
- dev_err(&pdev->dev,
- "failed to request doorbell IRQ#%u: %d\n",
- hsp->doorbell_irq, err);
+ if (err < 0)
return err;
- }
}
if (hsp->shared_irqs) {
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] mailbox: Remove redundant dev_err()/dev_err_probe()
2026-07-16 2:52 [PATCH] mailbox: Remove redundant dev_err()/dev_err_probe() Pan Chuang
@ 2026-07-16 3:02 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-07-16 3:02 UTC (permalink / raw)
To: Pan Chuang; +Cc: linux-sunxi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Double unregister of the mailbox controller on the IRQ request error path due to mixing devres with manual cleanup.
--
--- Patch [1]: [PATCH] mailbox: Remove redundant dev_err()/dev_err_probe() ---
commit 4d3272d7b1e42964746554b2fa5d303d597704a7
Author: Pan Chuang <panchuang@vivo.com>
mailbox: Remove redundant dev_err()/dev_err_probe()
This commit removes redundant driver-specific dev_err() and dev_err_probe()
calls after devm_request_irq() and devm_request_threaded_irq(), since those
functions automatically log detailed error messages on failure now.
> diff --git a/drivers/mailbox/arm_mhu_db.c b/drivers/mailbox/arm_mhu_db.c
> index 9e937b09c5fbc..a39239e38a479 100644
> --- a/drivers/mailbox/arm_mhu_db.c
> +++ b/drivers/mailbox/arm_mhu_db.c
> @@ -318,7 +318,6 @@ static int mhu_db_probe(struct amba_device *adev, const struct amba_id *id)
> mhu_db_mbox_rx_handler,
> IRQF_ONESHOT, "mhu_db_link", mhu);
> if (err) {
> - dev_err(dev, "Can't claim IRQ %d\n", irq);
> mbox_controller_unregister(&mhu->mbox);
[Severity: High]
This is a pre-existing issue, but since this patch touches the error handling
path, does calling mbox_controller_unregister() manually on a devm-managed
controller cause list corruption?
Looking at mhu_db_probe(), the mailbox controller is registered using
devm_mbox_controller_register() earlier in the function. If
devm_request_threaded_irq() fails, this explicit unregister is called.
When the probe returns an error, the devres framework will automatically
unregister it a second time.
Since mbox_controller_unregister() calls list_del() on the mailbox node
without re-initializing it, would a second call dereference poisoned pointers
and cause a kernel panic?
> return err;
> }
[ ... ]
> diff --git a/drivers/mailbox/qcom-ipcc.c b/drivers/mailbox/qcom-ipcc.c
> index d957d989c0cee..e5ebb4a237ffd 100644
> --- a/drivers/mailbox/qcom-ipcc.c
> +++ b/drivers/mailbox/qcom-ipcc.c
> @@ -323,10 +323,8 @@ static int qcom_ipcc_probe(struct platform_device *pdev)
> ret = devm_request_irq(&pdev->dev, ipcc->irq, qcom_ipcc_irq_fn,
> IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND |
> IRQF_NO_THREAD, name, ipcc);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "Failed to register the irq: %d\n", ret);
> + if (ret < 0)
> goto err_req_irq;
[Severity: High]
This is also a pre-existing issue on the modified error path. When the
devm_request_irq() call fails, the code jumps to the err_req_irq label:
err_req_irq:
if (ipcc->num_chans)
mbox_controller_unregister(&ipcc->mbox);
Since the controller was also registered with devm_mbox_controller_register(),
will devres unregister it again during cleanup, leading to the same double
unregister list corruption mentioned above?
> - }
>
> platform_set_drvdata(pdev, ipcc);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716025215.33592-1-panchuang@vivo.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-16 3:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 2:52 [PATCH] mailbox: Remove redundant dev_err()/dev_err_probe() Pan Chuang
2026-07-16 3:02 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox