Re: [PATCH v5 3/3] arm64: dts: rockchip: rk356x: Add eDP controller and phy nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] arm64: dts: rockchip: rk3568-edp lacks binding/driver match
  and rockchip,grf
- [Low] arm64: dts: rockchip: rk356x: duplicate PCLK_EDPPHY_GRF clock in
  edp_phy_grf
--

commit 39d000ce8ae0719491fd06506c51a275929fc4ce
Author: Damon Ding <damon.ding@rock-chips.com>

arm64: dts: rockchip: rk356x: Add eDP controller and phy nodes

This commit adds the eDP PHY GRF, eDP PHY child node, and eDP controller
nodes to rk356x-base.dtsi for the RK3568/RK3566 SoCs.

> diff --git a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
> index 1a4562df20ee5..e3f370cad5d51 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
> @@ -402,6 +402,24 @@ usb2phy1_grf: syscon@fdca8000 {
>  		reg = <0x0 0xfdca8000 0x0 0x8000>;
>  	};
>  
> +	edp_phy_grf: syscon@fdcb0000 {
> +		compatible = "rockchip,rk3568-edp-phy-grf", "syscon", "simple-mfd";
> +		reg = <0x0 0xfdcb0000 0x0 0x8000>;
> +		clocks = <&cru PCLK_EDPPHY_GRF>;
> +		ranges = <0x0 0x0 0xfdcb0000 0x100>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		edp_phy: phy@0 {
> +			compatible = "rockchip,rk3568-edp-phy";
> +			reg = <0x0 0x100>;
> +			clocks = <&pmucru XIN_OSC0_EDPPHY_G>, <&cru PCLK_EDPPHY_GRF>;
> +			clock-names = "ref", "apb";

[Severity: Low]
Should clocks = <&cru PCLK_EDPPHY_GRF> be dropped from the parent
edp_phy_grf node (and from rockchip,rk3568-edp-phy-grf.yaml in commit
"dt-bindings: phy: Add rockchip,rk3568-edp-phy") now that the child
edp_phy node claims and enables PCLK_EDPPHY_GRF directly?

> @@ -878,6 +896,33 @@ hdmi_out: port@1 {
>  		};
>  	};
>  
> +	edp: edp@fe0c0000 {
> +		compatible = "rockchip,rk3568-edp";
> +		reg = <0x0 0xfe0c0000 0x0 0x10000>;
> +		clocks = <&pmucru XIN_OSC0_EDPPHY_G>, <&cru PCLK_EDP_CTRL>, <&cru HCLK_VO>;
> +		clock-names = "dp", "pclk", "hclk";
> +		interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> +		phys = <&edp_phy>;
> +		phy-names = "dp";
> +		power-domains = <&power RK3568_PD_VO>;
> +		resets = <&cru SRST_EDP_24M>, <&cru SRST_P_EDP_CTRL>;
> +		reset-names = "dp", "apb";
> +		status = "disabled";

[Severity: Medium]
Can the edp@fe0c0000 node pass dtbs_check or probe when enabled?

"rockchip,rk3568-edp" is not documented in
Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
or matched in rockchip_dp_dt_ids[].  In addition, the binding requires
rockchip,grf and rockchip_dp_of_probe() fails with -ENODEV when that
property is absent:

drivers/gpu/drm/rockchip/analogix_dp-rockchip.c:rockchip_dp_of_probe() {
	...
	dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
	if (IS_ERR(dp->grf))
		return dev_err_probe(dev, PTR_ERR(dp->grf),
				     "failed to get rockchip,grf property\n");
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110074533.4052196-1-damon.ding@rock-chips.com?part=3