Re: [v1] arm64: dts: sc7180: add dsi controller and phy entries for idp dts
From: Matthias Kaehlcke <mka@chromium.org>
Date: 2020-01-28 19:53:27
Also in:
dri-devel, linux-arm-msm, lkml
Hi, On Tue, Jan 28, 2020 at 07:06:57PM +0530, Harigovindan P wrote:
quoted hunk ↗ jump to hunk
Adding dsi controller and phy entries for idp dt. Signed-off-by: Harigovindan P <redacted> --- arch/arm64/boot/dts/qcom/sc7180-idp.dts | 56 +++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts index 388f50a..9f42367 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts@@ -7,6 +7,7 @@ /dts-v1/; +#include <dt-bindings/gpio/gpio.h> #include <dt-bindings/regulator/qcom,rpmh-regulator.h> #include "sc7180.dtsi" #include "pm6150.dtsi"@@ -232,6 +233,50 @@ }; }; +&dsi_controller { + status = "okay"; + + vdda-supply = <&vreg_l3c_1p2>; + + panel@0 { + compatible = "visionox,rm69299-1080p-display";
This depends on the series "Add support for rm69299 Visionox panel driver and add devicetree bindings for visionox panel" (https://patchwork.kernel.org/cover/11320773/) which hasn't landed IIUC. This should be mentioned after '---'.
+ reg = <0>; + + vdda-supply = <&vreg_l8c_1p8>; + vdd3p3-supply = <&vreg_l18a_2p8>; + + pinctrl-names = "default", "suspend"; + pinctrl-0 = <&disp_pins_default>; + pinctrl-1 = <&disp_pins_default>;
Is there a point for listing a suspend configuration if it is the same as the default?
+ reset-gpios = <&pm6150l_gpio 3 GPIO_ACTIVE_HIGH>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ panel0_in: endpoint {
+ remote-endpoint = <&dsi0_out>;The 'dsi0_out' node is added by 'arm64: dts: sc7180: add display dt nodes' (https://patchwork.kernel.org/patch/11354253/), which has not landed. So either you should list it as dependency below the commit message, or (probably better) send the two patches in the same series.
+ };
+ };
+ };
+ };
+
+ ports {
+ port@1 {
+ endpoint {
+ remote-endpoint = <&panel0_in>;
+ data-lanes = <0 1 2 3>;
+ };
+ };
+ };
+};
+
+&dsi_phy {Also this node does not exist yet, it also depends on the patch mentioned above.
quoted hunk ↗ jump to hunk
+ status = "okay"; +}; + &qspi { status = "okay"; pinctrl-names = "default";@@ -289,6 +334,17 @@ /* PINCTRL - additions to nodes defined in sc7180.dtsi */ +&pm6150l_gpio { + disp_pins_default: disp-pins-default{
nit: missing blank before curly brace If there is no suspend configuration you can get rid of the 'default' suffix. Since this is only a single pin I would suggest to use a name that describes the function of the pin or the name used in the schematic instead of 'disp-pins'. Thanks Matthias