lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <cofwijgk2dgg5i5xhvhq3exug4o77mttmozw5amtc3myn4zzq5@m5x44hswwsmt>
Date: Tue, 17 Sep 2024 09:29:46 +0200
From: Bjorn Andersson <andersson@...nel.org>
To: Soutrik Mukhopadhyay <quic_mukhopad@...cinc.com>
Cc: konradybcio@...nel.org, robh@...nel.org, krzk+dt@...nel.org, 
	conor+dt@...nel.org, linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, quic_riteshk@...cinc.com, quic_vproddut@...cinc.com, 
	quic_abhinavk@...cinc.com
Subject: Re: [PATCH] arm64: dts: qcom: sa8775p: add DisplayPort device node

On Mon, Sep 16, 2024 at 02:43:44PM GMT, Soutrik Mukhopadhyay wrote:
> Add device tree node for the DisplayPort controller
> and eDP PHY found on the Qualcomm SA8775P SoC.
> 

Please split this in a change for the platform (.dtsi) which defines
_all_ the DPTX and DP PHYs, and then a ride dts change which enables all
the ports available on the Ride.

If there are platform ports that are not accessible on any hardware,
state in the commit message which ones you tested and which ones you
didn't test.

> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@...cinc.com>
> ---
> This patch depends on following series:
> https://lore.kernel.org/all/20240816-sa8775p-mm-v3-v1-0-77d53c3c0cef@quicinc.com/
> https://lore.kernel.org/all/20240912071437.1708969-1-quic_mahap@quicinc.com/
> https://lore.kernel.org/all/20240913103755.7290-1-quic_mukhopad@quicinc.com/

Please hold off resubmitting this series until there's conclusion on
these dependencies.

>  
> ---
>  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi |  23 +++++
>  arch/arm64/boot/dts/qcom/sa8775p.dtsi      | 114 ++++++++++++++++++++-
>  2 files changed, 136 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> index 0c1b21def4b6..728b4cda8353 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> @@ -421,6 +421,23 @@
>  	status = "okay";
>  };
>  
> +&mdss0 {
> +	status = "okay";
> +};
> +
> +&mdss0_dp0 {
> +	status = "okay";
> +};
> +
> +&mdss0_dp0_out {
> +	data-lanes = <0 1 2 3>;
> +	link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
> +};
> +
> +&mdss0_edp_phy0 {
> +	status = "okay";
> +};
> +
>  &pmm8654au_0_gpios {
>  	gpio-line-names = "DS_EN",
>  			  "POFF_COMPLETE",
> @@ -527,6 +544,12 @@
>  };
>  
>  &tlmm {
> +	dp_hot_plug_det: dp-hot-plug-det-state {
> +		pins = "gpio101";
> +		function = "edp0_hot";
> +		bias-disable;
> +	};
> +
>  	ethernet0_default: ethernet0-default-state {
>  		ethernet0_mdc: ethernet0-mdc-pins {
>  			pins = "gpio8";
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> index 7747965e7e46..a04150c29565 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> @@ -3339,6 +3339,18 @@
>  				interrupt-parent = <&mdss0>;
>  				interrupts = <0>;
>  
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +						dpu_intf0_out: endpoint {
> +							remote-endpoint = <&mdss0_dp0_in>;
> +						};
> +					};
> +				};
> +
>  				mdss0_mdp_opp_table: opp-table {
>  					compatible = "operating-points-v2";
>  
> @@ -3363,6 +3375,106 @@
>  					};
>  				};
>  			};
> +
> +			mdss0_edp_phy0: phy@...2a00 {
> +				compatible = "qcom,sa8775p-edp-phy";

Is this really a eDP PHY, not a DP/eDP combo phy?

I would prefer that we keep the label prefix "mdssM_dpN" on the DP TX
and DP PHY nodes, to keep them neatly sorted in the dts. (If you name
half mdssM_edp_phyN and half mdssM_dp_phyN we're going to have a mess)

> +
> +				reg = <0x0 0xaec2a00 0x0 0x200>,
> +					<0x0 0xaec2200 0x0 0xd0>,
> +					<0x0 0xaec2600 0x0 0xd0>,
> +					<0x0 0xaec2000 0x0 0x1c8>;
> +
> +				clocks = <&rpmhcc RPMH_CXO_CLK>,
> +					 <&gcc GCC_EDP_REF_CLKREF_EN>;
> +				clock-names = "aux",
> +					      "cfg_ahb";
> +
> +				vdda-phy-supply = <&vreg_l1c>;
> +				vdda-pll-supply = <&vreg_l4a>;
> +				#clock-cells = <1>;
> +				#phy-cells = <0>;
> +
> +				status = "disabled";
> +			};
> +
> +			mdss0_dp0: displayport-controller@...4000 {
> +				compatible = "qcom,sa8775p-dp";
> +
> +				pinctrl-0 = <&dp_hot_plug_det>;

Don't make references from .dtsi to labels defined in .dts.

Regards,
Bjorn

> +				pinctrl-names = "default";
> +
> +				reg = <0x0 0xaf54000 0x0 0x104>,
> +					<0x0 0xaf54200 0x0 0x0c0>,
> +					<0x0 0xaf55000 0x0 0x770>,
> +					<0x0 0xaf56000 0x0 0x09c>;
> +
> +				interrupt-parent = <&mdss0>;
> +				interrupts = <12>;
> +
> +				clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>,
> +					 <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_AUX_CLK>,
> +					 <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK>,
> +					 <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>,
> +					 <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK>;
> +				clock-names = "core_iface",
> +						"core_aux",
> +						"ctrl_link",
> +						"ctrl_link_iface",
> +						"stream_pixel";
> +				assigned-clocks = <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>,
> +						  <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>;
> +				assigned-clock-parents = <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>;
> +				phys = <&mdss0_edp_phy0>;
> +				phy-names = "dp";
> +
> +				operating-points-v2 = <&dp_opp_table>;
> +				power-domains = <&rpmhpd SA8775P_MMCX>;
> +
> +				#sound-dai-cells = <0>;
> +
> +				status = "disabled";
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +						mdss0_dp0_in: endpoint {
> +							remote-endpoint = <&dpu_intf0_out>;
> +						};
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +						mdss0_dp0_out: endpoint { };
> +					};
> +				};
> +
> +				dp_opp_table: opp-table {
> +					compatible = "operating-points-v2";
> +
> +					opp-160000000 {
> +						opp-hz = /bits/ 64 <160000000>;
> +						required-opps = <&rpmhpd_opp_low_svs>;
> +					};
> +
> +					opp-270000000 {
> +						opp-hz = /bits/ 64 <270000000>;
> +						required-opps = <&rpmhpd_opp_svs>;
> +					};
> +
> +					opp-540000000 {
> +						opp-hz = /bits/ 64 <540000000>;
> +						required-opps = <&rpmhpd_opp_svs_l1>;
> +					};
> +
> +					opp-810000000 {
> +						opp-hz = /bits/ 64 <810000000>;
> +						required-opps = <&rpmhpd_opp_nom>;
> +					};
> +				};
> +			};
>  		};
>  
>  		dispcc0: clock-controller@...0000 {
> @@ -3372,7 +3484,7 @@
>  				 <&rpmhcc RPMH_CXO_CLK>,
>  				 <&rpmhcc RPMH_CXO_CLK_A>,
>  				 <&sleep_clk>,
> -				 <0>, <0>, <0>, <0>,
> +				 <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>, <0>, <0>,
>  				 <0>, <0>, <0>, <0>;
>  			power-domains = <&rpmhpd SA8775P_MMCX>;
>  			#clock-cells = <1>;
> -- 
> 2.17.1
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ