Re: [PATCH v2] arm64: dts: qcom: qcs6490-rb3gen2: enable hdmi bridge

[Prahlad Valluru <quic_vvalluru@quicinc.com>]

On Wed, May 08, 2024 at 09:31:24AM -0500, Rob Herring (Arm) wrote:
> 
> On Mon, 06 May 2024 18:14:10 -0500, Bjorn Andersson wrote:
> > On Fri, Apr 26, 2024 at 07:54:42PM GMT, Prahlad Valluru wrote:
> > > From: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
> > >
> > 
> > Please don't thread new versions off existing version. b4 helps you with
> > getting these things right, please check go/upstream for more details.
> > 
> > > Enable lt9611uxc bridge for qcs6490 rb3 gen2 platform.
> > >
> > 
> > Even if it's clear what this is, I would prefer if you described the
> > hardware a little bit in your commit message.
> > "Rb3Gen2 has a HDMI connector, connected to DSI via a LT on i2cX.... reset and
> > irq pins comes from x and y. Describe this."
> > 
> > > Signed-off-by: Prahlad Valluru <quic_vvalluru@quicinc.com>
> > > ---
> > > v2: Addressed dtschema errors
> > > 	- Fixed lt9611-irq
> > > 	- vdd-supply error to be ignored, as it is connected to
> > > 	  input supply directly, on rb3gen2
> 
> The choice is either fix the dts or fix the binding. 

vdd-supply is mandatory for lt9611. Only in case of rb3gen2, we are seeing this
error, since it is connected to supply directly. Will add dummy vreg to address this.

> 
> > > ---
> > >  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 86 ++++++++++++++++++++
> > >  1 file changed, 86 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > index a085ff5b5fb2..c14d4a4bb3ce 100644
> > > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > @@ -51,6 +51,18 @@
> > >  			};
> > >  		};
> > >  	};
> > > +
> > 
> > There's a stray tab here, please run checkpatch --strict on your
> > patches.
> > 
> > > +	hdmi-connector {
> > > +		compatible = "hdmi-connector";
> > > +		label = "HDMI";
> > > +		type = "a";
> > > +
> > > +		port {
> > > +			hdmi_con: endpoint {
> > > +				remote-endpoint = <&lt9611_out>;
> > > +			};
> > > +		};
> > > +	};
> > >
> > >  	reserved-memory {
> > >  		xbl_mem: xbl@80700000 {
> > > @@ -530,6 +542,45 @@
> > [..]
> > > @@ -602,6 +653,21 @@
> > >  	status = "okay";
> > >  };
> > >
> > > +&mdss_dsi {
> > 
> > We want to keep nodes sorted (by address if applicable, otherwise
> > alphabetically on node names and on labels). "mdss_dsi" < "mdss_edp".
> > 
> > So please move this up where it belongs.
> > 
> > > +        vdda-supply = <&vreg_l6b_1p2>;
> > > +        status = "okay";
> > > +};
> > > +
> > > +&mdss_dsi0_out {
> > > +        remote-endpoint = <&lt9611_a>;
> > > +        data-lanes = <0 1 2 3>;
> > > +};
> > > +
> > > +&mdss_dsi_phy {
> > > +        vdds-supply = <&vreg_l10c_0p88>;
> > > +        status = "okay";
> > > +};
> > > +
> > >  &qupv3_id_0 {
> > >  	status = "okay";
> > >  };
> > > @@ -711,3 +777,23 @@
> > >  	function = "gpio";
> > >  	bias-disable;
> > >  };
> > > +
> > > +&pm7250b_gpios {
> > > +        lt9611_rst_pin: lt9611-rst-state {
> > > +                pins = "gpio2";
> > > +                function = "normal";
> > > +
> > > +                output-high;
> > > +                input-disable;
> > > +                power-source = <0>;
> > > +        };
> > > +};
> > > +
> > > +&tlmm {
> > > +        lt9611_irq_pin: lt9611-irq-state {
> > > +                pins = "gpio24";
> > > +                function = "gpio";
> > > +                drive-strength = <8>;
> > 
> > I'd expect a 2 here, so please document why this is 8.
> > 
> > Regards,
> > Bjorn
> > 
> > > +                bias-disable;
> > > +        };
> > > +};
> > > --
> > > 2.17.1
> > >
> > 
> > 
> 
> 
> My bot found new DTB warnings on the .dts files added or changed in this
> series.
> 
> Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
> are fixed by another series. Ultimately, it is up to the platform
> maintainer whether these warnings are acceptable or not. No need to reply
> unless the platform maintainer has comments.
> 
> If you already ran DT checks and didn't see these error(s), then
> make sure dt-schema is up to date:
> 
>   pip3 install dtschema --upgrade
> 
> 
> New warnings running 'make CHECK_DTBS=y qcom/qcs6490-rb3gen2.dtb' for jr3ble6sxr5mr6cvm6ldvpyk5j4rucj3xy6vbha6ttoecte3d7@llu6qf6oasuc:
> 
> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dtb: hdmi-bridge@2b: 'vdd-supply' is a required property
> 	from schema $id: http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
> 
> 
> 
> 
>