Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Resource leak of pixel clock on bus clock devm_clk_get() failure. - [High] Improper error handling of LVDS clock retrieval breaks probe deferral. -- --- Patch [1]: [PATCH] drm/stm/ltdc: Fix clock cleanup error handling --- commit 9d3968fa3e6694cd51c4ea469a9ffb12dcc0dfc7 Author: Hongling Zeng <[email protected]> drm/stm/ltdc: Fix clock cleanup error handling This commit adds proper error handling to ensure prepared clocks are disabled and unprepared when a subsequent clock preparation fails in ltdc_resume() and ltdc_load(). Cc: [email protected] > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > index 8818dcf98da79..ac1c2f27563e3 100644 > --- a/drivers/gpu/drm/stm/ltdc.c > +++ b/drivers/gpu/drm/stm/ltdc.c > @@ -1958,12 +1967,12 @@ int ltdc_load(struct drm_device *ddev) > if (of_device_is_compatible(np, "st,stm32mp251-ltdc") || > of_device_is_compatible(np, "st,stm32mp255-ltdc")) { > ldev->bus_clk = devm_clk_get(dev, "bus"); > if (IS_ERR(ldev->bus_clk)) > return dev_err_probe(dev, PTR_ERR(ldev->bus_clk), > "Unable to get bus clock\n"); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [Severity: High] This is a pre-existing issue, but does this early return leak a pixel clock reference? If devm_clk_get(dev, "bus") fails and returns -EPROBE_DEFER, it looks like the function returns directly without disabling the already enabled ldev->pixel_clk. > > ret = clk_prepare_enable(ldev->bus_clk); > if (ret) { > drm_err(ddev, "Unable to prepare bus clock\n"); > - return ret; > + goto err_pixel; > } > } [ ... ] > @@ -2004,3 +2013,3 @@ int ltdc_load(struct drm_device *ddev) > > ldev->lvds_clk = devm_clk_get(dev, "lvds"); > if (IS_ERR(ldev->lvds_clk)) > ldev->lvds_clk = NULL; ^^^^^^^^^^^^^^^^^^^^^^ [Severity: High] This is a pre-existing issue, but does this unconditionally mask all errors from devm_clk_get(), including -EPROBE_DEFER? If the LVDS clock provider is initialized after the STM DRM driver, it seems this would silently ignore the error instead of correctly deferring the probe, which could leave the LVDS interface broken. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
