diff mbox series

mailbox: sun6i: modernize probe and convert to fully managed

Message ID 20260727194208.10082-1-rosenp@gmail.com (mailing list archive)
State New
Headers
Series mailbox: sun6i: modernize probe and convert to fully managed |

Commit Message

Rosen Penev July 27, 2026, 7:42 p.m. UTC 
Replace irq_of_parse_and_map() with platform_get_irq() and move both
IRQ and MMIO resource acquisition to the top of probe, before any
allocations, for early error exit.

Switch from devm_clk_get() + clk_prepare_enable() to
devm_clk_get_enabled(), which combines both operations and registers
devres callbacks for automatic disable/unprepare on unbind. This
eliminates the manual err_disable_unprepare error path and the
remove callback entirely.

Use devm_mbox_controller_register() for devres-managed controller
registration, and drop the remove callback and platform_set_drvdata()
which are no longer needed.

Assisted-by: Opencode:Big-Pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/mailbox/sun6i-msgbox.c | 62 ++++++++++------------------------
 1 file changed, 18 insertions(+), 44 deletions(-)

Comments

sashiko-bot@kernel.org July 27, 2026, 7:48 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The function `devm_mbox_controller_register()` is called with one argument, but it requires two, causing a build failure.
--

--- Patch [1]: [PATCH] mailbox: sun6i: modernize probe and convert to fully managed ---
commit 6febca9f53cbed2f1affb7635d88de86beffdca8
Author: Rosen Penev <rosenp@gmail.com>

mailbox: sun6i: modernize probe and convert to fully managed

This patch replaces manual resource cleanup with devres-managed variants in
the sun6i mailbox driver. It converts IRQ, MMIO, clock, and mailbox
controller registration to use their devm_* equivalents and removes the
manual error paths and remove callback.

> diff --git a/drivers/mailbox/sun6i-msgbox.c b/drivers/mailbox/sun6i-msgbox.c
> index 6ba6920f4645e..28d8636afad26 100644
> --- a/drivers/mailbox/sun6i-msgbox.c
> +++ b/drivers/mailbox/sun6i-msgbox.c
[ ... ]
> @@ -271,29 +267,8 @@ static int sun6i_msgbox_probe(struct platform_device *pdev)
>  	mbox->controller.txpoll_period = 5;
>  
>  	spin_lock_init(&mbox->lock);
> -	platform_set_drvdata(pdev, mbox);
> -
> -	ret = mbox_controller_register(&mbox->controller);
> -	if (ret) {
> -		dev_err(dev, "Failed to register controller: %d\n", ret);
> -		goto err_disable_unprepare;
> -	}
> -
> -	return 0;
> -
> -err_disable_unprepare:
> -	clk_disable_unprepare(mbox->clk);
> -
> -	return ret;
> -}
> -
> -static void sun6i_msgbox_remove(struct platform_device *pdev)
> -{
> -	struct sun6i_msgbox *mbox = platform_get_drvdata(pdev);
>  
> -	mbox_controller_unregister(&mbox->controller);
> -	/* See the comment in sun6i_msgbox_probe about the reset line. */
> -	clk_disable_unprepare(mbox->clk);
> +	return devm_mbox_controller_register(&mbox->controller);

[Severity: Low]
Will this compile successfully?

Looking at the function signature in drivers/mailbox/mailbox.c,
devm_mbox_controller_register() requires two arguments:

int devm_mbox_controller_register(struct device *dev,
                                  struct mbox_controller *mbox)

The call here only provides the mbox_controller pointer and is missing the
struct device pointer as the first argument, which is likely to result in
a build failure.

>  }
>  
>  static const struct of_device_id sun6i_msgbox_of_match[] = {
diff mbox series

Patch

diff --git a/drivers/mailbox/sun6i-msgbox.c b/drivers/mailbox/sun6i-msgbox.c
index 6ba6920f4645..28d8636afad2 100644
--- a/drivers/mailbox/sun6i-msgbox.c
+++ b/drivers/mailbox/sun6i-msgbox.c
@@ -198,7 +198,17 @@  static int sun6i_msgbox_probe(struct platform_device *pdev)
 	struct mbox_chan *chans;
 	struct reset_control *reset;
 	struct sun6i_msgbox *mbox;
+	void __iomem *regs;
 	int i, ret;
+	int irq;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
 
 	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
 	if (!mbox)
@@ -211,24 +221,18 @@  static int sun6i_msgbox_probe(struct platform_device *pdev)
 	for (i = 0; i < NUM_CHANS; ++i)
 		chans[i].con_priv = mbox;
 
-	mbox->clk = devm_clk_get(dev, NULL);
+	mbox->clk = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(mbox->clk)) {
 		ret = PTR_ERR(mbox->clk);
 		dev_err(dev, "Failed to get clock: %d\n", ret);
 		return ret;
 	}
 
-	ret = clk_prepare_enable(mbox->clk);
-	if (ret) {
-		dev_err(dev, "Failed to enable clock: %d\n", ret);
-		return ret;
-	}
-
 	reset = devm_reset_control_get_exclusive(dev, NULL);
 	if (IS_ERR(reset)) {
 		ret = PTR_ERR(reset);
 		dev_err(dev, "Failed to get reset control: %d\n", ret);
-		goto err_disable_unprepare;
+		return ret;
 	}
 
 	/*
@@ -242,25 +246,17 @@  static int sun6i_msgbox_probe(struct platform_device *pdev)
 	ret = reset_control_deassert(reset);
 	if (ret) {
 		dev_err(dev, "Failed to deassert reset: %d\n", ret);
-		goto err_disable_unprepare;
+		return ret;
 	}
 
-	mbox->regs = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(mbox->regs)) {
-		ret = PTR_ERR(mbox->regs);
-		dev_err(dev, "Failed to map MMIO resource: %d\n", ret);
-		goto err_disable_unprepare;
-	}
+	mbox->regs = regs;
 
 	/* Disable all IRQs for this end of the msgbox. */
 	writel(0, mbox->regs + LOCAL_IRQ_EN_REG);
 
-	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);
-		goto err_disable_unprepare;
-	}
+	ret = devm_request_irq(dev, irq, sun6i_msgbox_irq, 0, dev_name(dev), mbox);
+	if (ret)
+		return ret;
 
 	mbox->controller.dev           = dev;
 	mbox->controller.ops           = &sun6i_msgbox_chan_ops;
@@ -271,29 +267,8 @@  static int sun6i_msgbox_probe(struct platform_device *pdev)
 	mbox->controller.txpoll_period = 5;
 
 	spin_lock_init(&mbox->lock);
-	platform_set_drvdata(pdev, mbox);
-
-	ret = mbox_controller_register(&mbox->controller);
-	if (ret) {
-		dev_err(dev, "Failed to register controller: %d\n", ret);
-		goto err_disable_unprepare;
-	}
-
-	return 0;
-
-err_disable_unprepare:
-	clk_disable_unprepare(mbox->clk);
-
-	return ret;
-}
-
-static void sun6i_msgbox_remove(struct platform_device *pdev)
-{
-	struct sun6i_msgbox *mbox = platform_get_drvdata(pdev);
 
-	mbox_controller_unregister(&mbox->controller);
-	/* See the comment in sun6i_msgbox_probe about the reset line. */
-	clk_disable_unprepare(mbox->clk);
+	return devm_mbox_controller_register(&mbox->controller);
 }
 
 static const struct of_device_id sun6i_msgbox_of_match[] = {
@@ -308,7 +283,6 @@  static struct platform_driver sun6i_msgbox_driver = {
 		.of_match_table = sun6i_msgbox_of_match,
 	},
 	.probe = sun6i_msgbox_probe,
-	.remove = sun6i_msgbox_remove,
 };
 module_platform_driver(sun6i_msgbox_driver);