Re: [PATCH v8 phy-next 15/31] drm/rockchip: dw_hdmi: avoid direct dereference of phy->dev.of_node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] A PHY specifier is parsed and passed directly to a Clock framework API, conflating two different DT domains.
--

commit 6a42fe45756ea5f0b8615efd6d337d5e679b8587
Author: Vladimir Oltean <vladimir.oltean@nxp.com>

drm/rockchip: dw_hdmi: avoid direct dereference of phy->dev.of_node

The commit refactors dw_hdmi_rockchip_bind() to perform a manual phandle
lookup on the hdmi PHY index within the controller's DT node, avoiding a
direct dereference of hdmi->phy->dev.of_node.

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 0dc1eb5d2ae39..7abb42e486c02 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c

[ ... ]

> @@ -588,13 +589,17 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
>  		return dev_err_probe(hdmi->dev, ret, "failed to get phy\n");
>  	}
>  
> -	if (hdmi->phy) {
> +	index = of_property_match_string(np, "phy-names", "hdmi");
> +	if (index >= 0) {
>  		struct of_phandle_args clkspec;
>  
> -		clkspec.np = hdmi->phy->dev.of_node;
> -		hdmi->hdmiphy_clk = of_clk_get_from_provider(&clkspec);
> -		if (IS_ERR(hdmi->hdmiphy_clk))
> -			hdmi->hdmiphy_clk = NULL;
> +		if (!of_parse_phandle_with_args(np, "phys", "#phy-cells", index,
> +						&clkspec)) {
> +			hdmi->hdmiphy_clk = of_clk_get_from_provider(&clkspec);

This isn't a bug, but does this pass a PHY specifier directly to a Clock
framework API?

of_parse_phandle_with_args() parses the phys property using #phy-cells,
populating clkspec as a PHY specifier. This is then passed to
of_clk_get_from_provider() which expects a Clock specifier governed by
#clock-cells.

While this does not cause a runtime failure here because the Rockchip HDMI
PHY clock provider uses of_clk_src_simple_get and ignores the specifier
arguments, it appears to conflate two different DT domains.

Would it be better to retrieve the device node using of_parse_phandle()
and explicitly synthesize a 0-cell clock specifier to maintain the API
abstraction?

> +			of_node_put(clkspec.np);
> +			if (IS_ERR(hdmi->hdmiphy_clk))
> +				hdmi->hdmiphy_clk = NULL;
> +		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505100523.1922388-1-vladimir.oltean@nxp.com?part=15