Re: [PATCH v6 3/4] 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 f7a9ae47dd93488d5a85b663ab5846186e851cbe
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 syscon, child eDP PHY, and eDP
controller device tree nodes to rk356x-base.dtsi for RK3566 and RK3568.

> 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 removed from the parent
edp_phy_grf node (and from rockchip,rk3568-edp-phy-grf.yaml)?

The changelog states that PCLK_EDPPHY_GRF was moved from the parent
syscon node to the child PHY node, and rockchip_edp_phy_probe() maps its
own MMIO window and enables the "apb" clock directly on edp_phy without
using the parent syscon.

[ ... ]

> @@ -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 node pass dtbs_check or probe when enabled on a board?

"rockchip,rk3568-edp" is not documented in
Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
or matched by rockchip_dp_dt_ids[] in
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c.  In addition, the node
omits the required rockchip,grf property, which causes
rockchip_dp_of_probe() -> syscon_regmap_lookup_by_phandle(np,
"rockchip,grf") to fail with -ENODEV.

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