Re: [RFC PATCH v1 2/3] ARM: dts: qcom: msm8974: add support for the HDMI output

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 7.05.2023 22:28, Dmitry Baryshkov wrote:
> Add HDMI and HDMI PHY device nodes for the Qualcomm MSM8974 SoC.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm/boot/dts/qcom-msm8974.dtsi | 90 ++++++++++++++++++++++++++++-
>  1 file changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index fb661c1bd3d5..fbe74eeef863 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -1805,6 +1805,21 @@ cci_i2c1_sleep: cci-i2c1-sleep-pins {
>  				};
>  			};
>  
> +			hdmi_default: hdmi-default-state {
> +				cec-pins {
> +					pins = "gpio31";
> +					function = "hdmi_cec";
> +				};
add a newline before each consecutive subnode, please

> +				ddc-pins {
> +					pins = "gpio32", "gpio33";
> +					function = "hdmi_ddc";
> +				};
> +				hpd-pins {
> +					pins = "gpio34";
> +					function = "hdmi_hpd";
> +				};
> +			};
> +
>  			spi8_default: spi8_default-state {
>  				mosi-pins {
>  					pins = "gpio45";
> @@ -1840,7 +1855,7 @@ mmcc: clock-controller@fd8c0000 {
>  				 <&dsi0_phy 0>,
>  				 <&dsi1_phy 1>,
>  				 <&dsi1_phy 0>,
> -				 <0>,
> +				 <&mdss_hdmi_phy>,
>  				 <0>,
>  				 <0>;
>  			clock-names = "xo",
> @@ -1914,6 +1929,13 @@ mdp5_intf2_out: endpoint {
>  							remote-endpoint = <&dsi1_in>;
>  						};
>  					};
> +
> +					port@2 {
> +						reg = <2>;
> +						mdp5_intf3_out: endpoint {
> +							remote-endpoint = <&mdss_hdmi_in>;
> +						};
> +					};
>  				};
>  			};
>  
> @@ -1970,6 +1992,72 @@ dsi0_out: endpoint {
>  				};
>  			};
>  
> +			mdss_hdmi: hdmi-tx@fd922100 {
> +				compatible = "qcom,hdmi-tx-8974";
> +				reg = <0xfd922100 0x35c>,
> +				      <0xfc4b8000 0x60f0>;
> +				reg-names = "core_physical",
> +					    "qfprom_physical";
> +
> +				pinctrl-0 = <&hdmi_default>;
> +				pinctrl-names = "default";
property-n
property-names

since the pinctrl driver only allows these functions on these pins,
it's okay for it to stay in the SoC dtsi

> +
> +				interrupt-parent = <&mdss>;
> +				interrupts = <8>;
> +
> +				clocks = <&mmcc MDSS_MDP_CLK>,
> +					 <&mmcc MDSS_AHB_CLK>,
> +					 <&mmcc MDSS_HDMI_CLK>,
> +					 <&mmcc MDSS_HDMI_AHB_CLK>,
> +					 <&mmcc MDSS_EXTPCLK_CLK>;
> +				clock-names =
> +					"mdp_core",
> +					"iface",
> +					"core",
> +					"alt_iface",
> +					"extp";
broken indentation

> +
> +				phys = <&mdss_hdmi_phy>;
> +
> +				status = "disabled";
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
add a newline before properties and subnodes, please

Konrad
> +						mdss_hdmi_in: endpoint {
> +							remote-endpoint = <&mdp5_intf3_out>;
> +						};
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +						mdss_hdmi_out: endpoint {
> +						};
> +					};
> +				};
> +			};
> +
> +			mdss_hdmi_phy: hdmi-phy@fd922500 {
> +				compatible = "qcom,hdmi-phy-8974";
> +				reg = <0xfd922500 0x7c>,
> +				      <0xfd922700 0xd4>;
> +				reg-names = "hdmi_phy",
> +					    "hdmi_pll";
> +
> +				clocks = <&mmcc MDSS_AHB_CLK>,
> +					 <&mmcc MDSS_HDMI_AHB_CLK>;
> +				clock-names = "iface",
> +					      "alt_iface";
> +
> +				#phy-cells = <0>;
> +				#clock-cells = <0>;
> +
> +				status = "disabled";
> +			};
> +
>  			dsi0_phy: phy@fd922a00 {
>  				compatible = "qcom,dsi-phy-28nm-hpm";
>  				reg = <0xfd922a00 0xd4>,