Re: [PATCH v2 14/15] riscv: dts: starfive: jh7110: Update DT for display subsystem

[Maud Spierings]

Hi Michal,

Very happy to see an update to this series!

I have some very minor nitpicks I found in the devicetree

> Activate the display subsystem by adding the dc8200, hdmi and voutcrg
> nodes as direct soc children. Each references the PD_VOUT power domain
> through power-domains, so genpd powers the domain on demand, and the
> HDMI-PHY -> voutcrg -> HDMI-controller probe order is resolved by
> deferred probe.
>
> The monolithic hdmi node is modelled as a container with hdmi_phy
> and hdmi_controller children. This lets the PHY (clock provider,
> depending only on xin24m) and the controller (clock consumer) probe
> independently, breaking the circular clock dependency with voutcrg.
>
> voutcrg consumes the pixel clock from the &hdmi_phy node instead of the
> old fixed-clock, and dc8200 gets its pixel clocks from voutcrg's MUXes
> and is marked dma-noncoherent for framebuffer DMA. The old, incorrect
> hdmitx0-pixel-clock fixed-clock node is removed.
>
> Co-developed-by: Dominique Belhachemi <db@xxxxxxxxxx>
> Signed-off-by: Dominique Belhachemi <db@xxxxxxxxxx>
> Signed-off-by: Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>
> ---
>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 119 +++++++++++++++++++++++-
>  arch/riscv/boot/dts/starfive/jh7110.dtsi        |  87 +++++++++++++++--
>  2 files changed, 196 insertions(+), 10 deletions(-)

[snip]

>  
> +&dc8200 {
> +	status = "okay";
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		dpu_port0: port@0 {
> +			reg = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			dpu_out_dpi0: endpoint@0 {
> +				reg = <0>;
> +				remote-endpoint = <&hdmi_in>;
> +			};
> +		};
> +
> +		dpu_port1: port@1 {
> +			reg = <1>;
> +		};
> +	};
> +};
> +
>  &dvp_clk {
>  	clock-frequency = <74250000>;
>  };
> @@ -99,8 +153,31 @@ &gmac1_rmii_refin {
>  	clock-frequency = <50000000>;
>  };
>  
> -&hdmitx0_pixelclk {
> -	clock-frequency = <297000000>;
> +&hdmi_controller {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&hdmi_pins>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			hdmi_in: endpoint {
> +				remote-endpoint = <&dpu_out_dpi0>;
> +			};
> +		};
> +
> +		hdmi_out_port: port@1 {
> +			reg = <1>;

missing newline here

> +			hdmi_out_con: endpoint {
> +				remote-endpoint = <&hdmi_con_in>;
> +			};
> +

one newline to much here

> +		};
> +	};
>  };
>  

Kind regards,
Maud