diff mbox series

[v2,7/11] drm: kmb: Use devm_of_reserved_mem_device_init()

Message ID 20260902201640.2024648-8-mukesh.ojha@oss.qualcomm.com (mailing list archive)
State New
Headers
Series drm: Use devm_of_reserved_mem_device_init() |

Commit Message

Mukesh Ojha Sept. 2, 2026, 8:16 p.m. UTC 
Use the devres-managed devm_of_reserved_mem_device_init() instead of
the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
pair, letting the device resource manager handle cleanup automatically.
Also simplify the IRQ error path since the release before init was a no-op.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/gpu/drm/kmb/kmb_drv.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

Comments

sashiko-bot@kernel.org Sept. 2, 2026, 8:31 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The return value of kmb_initialize_clocks() is ignored in kmb_hw_init(), leading to potential hardware hangs.
- [High] Calling drm_atomic_helper_shutdown() after disabling clocks and PM runtime leads to hardware access on a powered-down device and a system hang.
- [High] Dereferencing an ERR_PTR during error handling of DSI initialization causes a kernel panic.
--

commit ce6024d25a2085ae8c8a68d80d92293c0cb8ad15
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

drm: kmb: Use devm_of_reserved_mem_device_init()

This commit updates the driver to use the devres-managed variant of
of_reserved_mem_device_init(), simplifying the error and removal paths by
removing manual cleanup calls.

> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index 7c2eb1152fc2f..88fe377940f30 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -140,13 +140,12 @@ static int kmb_hw_init(struct drm_device *drm, unsigned long flags)
>  	/* Allocate LCD interrupt resources */
>  	irq_lcd = platform_get_irq(pdev, 0);

[Severity: High]
This is a pre-existing issue, but I noticed that just before this hunk in
kmb_hw_init(), the return value of kmb_initialize_clocks() is discarded:

	/* Enable display clocks */
	kmb_initialize_clocks(kmb, &pdev->dev);

	/* Register irqs here - section 17.3 in databook

If kmb_initialize_clocks() fails, the driver proceeds with probe and will
eventually access unclocked MMIO registers during modeset. Can this lead to
synchronous external aborts and system hangs?

[ ... ]

> @@ -465,8 +459,6 @@ static void kmb_remove(struct platform_device *pdev)
>  	pm_runtime_put_sync(drm->dev);
>  	pm_runtime_disable(drm->dev);
>  
> -	of_reserved_mem_device_release(drm->dev);
> -
>  	/* Release clks */
>  	kmb_display_clk_disable(kmb);
>  

[Severity: High]
This isn't a bug introduced by this patch, but does the shutdown sequence
in kmb_remove() cause hardware access on a powered-down device?

Right after this hunk, the function completes with:

	/* Unregister DSI host */
	kmb_dsi_host_unregister(kmb->kmb_dsi);
	drm_atomic_helper_shutdown(drm);
}

Since pm_runtime_disable() and kmb_display_clk_disable() are called prior to
drm_atomic_helper_shutdown(), atomic_disable callbacks will read and write
hardware registers while clocks are disabled. Will this cause a bus fault or
system hang during driver unbind?

[Severity: High]
This is also a pre-existing issue, but outside the diff in kmb_probe(), does
the error handling for kmb_dsi_init() cause an invalid pointer dereference?

If kmb_dsi_init() fails, it returns an ERR_PTR:

	kmb->kmb_dsi = kmb_dsi_init(dsi_pdev);
	if (IS_ERR(kmb->kmb_dsi)) {
		drm_err(&kmb->drm, "failed to initialize DSI\n");
		ret = PTR_ERR(kmb->kmb_dsi);
		goto err_free1;
	}

The error handling path unconditionally passes this ERR_PTR to
kmb_dsi_host_unregister(), which passes it to kmb_dsi_clk_disable(). This
dereferences the pointer when attempting to access kmb_dsi->clk_mipi:

drivers/gpu/drm/kmb/kmb_dsi.c:kmb_dsi_clk_disable() {
	clk_disable_unprepare(kmb_dsi->clk_mipi);

Will this result in a kernel panic due to dereferencing the ERR_PTR on memory
allocation failure during initialization?
diff mbox series

Patch

diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index 7c2eb1152fc2..88fe377940f3 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -140,13 +140,12 @@  static int kmb_hw_init(struct drm_device *drm, unsigned long flags)
 	/* Allocate LCD interrupt resources */
 	irq_lcd = platform_get_irq(pdev, 0);
 	if (irq_lcd < 0) {
-		ret = irq_lcd;
 		drm_err(&kmb->drm, "irq_lcd not found");
-		goto setup_fail;
+		return irq_lcd;
 	}
 
 	/* Get the optional framebuffer memory resource */
-	ret = of_reserved_mem_device_init(drm->dev);
+	ret = devm_of_reserved_mem_device_init(drm->dev);
 	if (ret && ret != -ENODEV)
 		return ret;
 
@@ -155,11 +154,6 @@  static int kmb_hw_init(struct drm_device *drm, unsigned long flags)
 	kmb->irq_lcd = irq_lcd;
 
 	return 0;
-
- setup_fail:
-	of_reserved_mem_device_release(drm->dev);
-
-	return ret;
 }
 
 static const struct drm_mode_config_funcs kmb_mode_config_funcs = {
@@ -465,8 +459,6 @@  static void kmb_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(drm->dev);
 	pm_runtime_disable(drm->dev);
 
-	of_reserved_mem_device_release(drm->dev);
-
 	/* Release clks */
 	kmb_display_clk_disable(kmb);