Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Unconditional call to `devm_pm_opp_set_clkname()` causes probe failure on platforms without the optional "extp" clock. - [Medium] Memory and reference leak of `hdmi->next_bridge` on error paths in probe due to bypassed cleanup. --
commit 44f5c22fff51888fa0578000b738b8abea1a33b0 Author: Krzysztof Kozlowski <[email protected]> drm/msm/hdmi: Add support for PM OPP table This commit adds PM OPP table support to allow scaling domain rails using the pixel clock ("extp"). This enables proper votes depending on the requested HDMI pixel rate. > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c > index 4740060846335..c46f30063f142 100644 > --- a/drivers/gpu/drm/msm/hdmi/hdmi.c > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c [ ... ] > @@ -368,6 +368,14 @@ static int msm_hdmi_dev_probe(struct platform_device > *pdev) > goto err_put_bridge; > } > > + ret = devm_pm_opp_set_clkname(dev, "extp"); > + if (ret) > + return ret; [Severity: High] Does this code cause a probe failure on platforms without the "extp" clock? Earlier in msm_hdmi_dev_probe(), the "extp" clock is fetched using devm_clk_get_optional(): hdmi->extp_clk = devm_clk_get_optional(dev, "extp"); If the platform does not provide this clock, devm_pm_opp_set_clkname() will return an error (such as -ENOENT). Returning that error here will unconditionally abort the probe, preventing display output on hardware where this clock is legitimately absent. [Severity: Medium] Also, does this code leak the hdmi->next_bridge reference by returning directly? If devm_pm_opp_set_clkname() fails, returning the error directly skips the goto err_put_bridge; label. This bypasses the cleanup of hdmi->next_bridge, which was acquired earlier in msm_hdmi_dev_probe() via of_drm_get_bridge_by_endpoint(). > + > + ret = devm_pm_opp_of_add_table(dev); > + if (ret && ret != -ENODEV) > + return dev_err_probe(dev, ret, "invalid OPP table in device > tree\n"); [Severity: Medium] Similarly, does this direct return also leak the hdmi->next_bridge reference if devm_pm_opp_of_add_table() fails? Using the goto err_put_bridge; path instead of returning directly would ensure the bridge reference is correctly dropped. > + > if (!hdmi->hpd_gpiod) > DBG("failed to get HPD gpio"); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
