Re: [PATCH RFC/DO NOT MERGE 08/12] drm/msm/hdmi: Add support for PM OPP table

[sashiko-bot@kernel.org]

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 <krzysztof.kozlowski@oss.qualcomm.com>

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/20260828-drm-msm-hdmi-eliza-v1-0-67843277de17@oss.qualcomm.com?part=8