diff mbox series

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

Message ID 20240426142442.7769-1-quic_vvalluru@quicinc.com
State New
Headers show
Series [v2] arm64: dts: qcom: qcs6490-rb3gen2: enable hdmi bridge | expand

Commit Message

Prahlad Valluru April 26, 2024, 2:24 p.m. UTC
From: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>

Enable lt9611uxc bridge for qcs6490 rb3 gen2 platform.

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
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 86 ++++++++++++++++++++
 1 file changed, 86 insertions(+)

Comments

Abhinav Kumar May 6, 2024, 7:54 p.m. UTC | #1
On 4/26/2024 7:24 AM, Prahlad Valluru wrote:
> From: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
> 
> Enable lt9611uxc bridge for qcs6490 rb3 gen2 platform.
> 
> Signed-off-by: Prahlad Valluru <quic_vvalluru@quicinc.com>
> ---

Why is there a mismatch between your author name and signed off name?

> v2: Addressed dtschema errors
> 	- Fixed lt9611-irq
> 	- vdd-supply error to be ignored, as it is connected to
> 	  input supply directly, on rb3gen2
> ---
>   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 @@
>   			};
>   		};
>   	};
> +	
> +	hdmi-connector {
> +		compatible = "hdmi-connector";
> +		label = "HDMI";

Is label mandatory? Doesn't seem so?

Rest LGTM.

> +		type = "a";
> +
> +		port {
> +			hdmi_con: endpoint {
> +				remote-endpoint = <&lt9611_out>;
> +			};
> +		};
> +	};
>   
>   	reserved-memory {
>   		xbl_mem: xbl@80700000 {
> @@ -530,6 +542,45 @@
>   			   <GCC_WPSS_RSCP_CLK>;
>   };
>   
> +&i2c0 {
> +	clock-frequency = <400000>;
> +	status = "okay";
> +
> +	lt9611_codec: hdmi-bridge@2b {
> +		compatible = "lontium,lt9611uxc";
> +		reg = <0x2b>;
> +
> +		interrupts-extended = <&tlmm 24 IRQ_TYPE_EDGE_FALLING>;
> +		reset-gpios = <&pm7250b_gpios 2 GPIO_ACTIVE_HIGH>;
> +
> +		vcc-supply = <&vreg_l11c_2p8>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&lt9611_irq_pin &lt9611_rst_pin>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				lt9611_a: endpoint {
> +					remote-endpoint = <&mdss_dsi0_out>;
> +				};
> +			};
> +
> +			port@2 {
> +				reg = <2>;
> +
> +				lt9611_out: endpoint {
> +					remote-endpoint = <&hdmi_con>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
>   &i2c1 {
>   	status = "okay";
>   
> @@ -602,6 +653,21 @@
>   	status = "okay";
>   };
>   
> +&mdss_dsi {
> +        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>;
> +                bias-disable;
> +        };
> +};
Bjorn Andersson May 6, 2024, 11:14 p.m. UTC | #2
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
> ---
>  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
>
Krzysztof Kozlowski May 7, 2024, 4:35 p.m. UTC | #3
On 07/05/2024 18:30, Venkata Prahlad Valluru wrote:
> Rb3Gen2 has a lt9611uxc DSI-to-HDMI bridge on i2c0, with
> reset gpio from pm7250b gpio2 and irq gpio from tlmm gpio24.
> Bridge supplies are Vdd connected to input supply directly
> and vcc to L11c. Enable HDMI output, bridge and corresponding
> DSI output.
> 
> Signed-off-by: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
> ---
> v3: - Updated commit text
>     - Arranged nodes in alphabetical order
>     - Fixed signoff
>     - Fixed drive strength for lt9611_irq_pin
>     - Removed 'label' from hdmi-connector, which is optional

Please respond to each Bjorn comment and explain how did you implement it...

Best regards,
Krzysztof
Abhinav Kumar May 7, 2024, 6:43 p.m. UTC | #4
On 5/7/2024 9:30 AM, Venkata Prahlad Valluru wrote:
> Rb3Gen2 has a lt9611uxc DSI-to-HDMI bridge on i2c0, with
> reset gpio from pm7250b gpio2 and irq gpio from tlmm gpio24.
> Bridge supplies are Vdd connected to input supply directly
> and vcc to L11c. Enable HDMI output, bridge and corresponding
> DSI output.
> 
> Signed-off-by: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
> ---
> v3: - Updated commit text
>      - Arranged nodes in alphabetical order
>      - Fixed signoff
>      - Fixed drive strength for lt9611_irq_pin
>      - Removed 'label' from hdmi-connector, which is optional
> 
> v2: Addressed dtschema errors
> 	- Fixed lt9611-irq
> 	- vdd-supply error to be ignored, as it is connected to
> 	  input supply directly, on rb3gen2
> ---
>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 85 ++++++++++++++++++++
>   1 file changed, 85 insertions(+)
> 

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Abhinav Kumar May 7, 2024, 7:20 p.m. UTC | #5
On 5/7/2024 9:35 AM, Krzysztof Kozlowski wrote:
> On 07/05/2024 18:30, Venkata Prahlad Valluru wrote:
>> Rb3Gen2 has a lt9611uxc DSI-to-HDMI bridge on i2c0, with
>> reset gpio from pm7250b gpio2 and irq gpio from tlmm gpio24.
>> Bridge supplies are Vdd connected to input supply directly
>> and vcc to L11c. Enable HDMI output, bridge and corresponding
>> DSI output.
>>
>> Signed-off-by: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
>> ---
>> v3: - Updated commit text
>>      - Arranged nodes in alphabetical order
>>      - Fixed signoff
>>      - Fixed drive strength for lt9611_irq_pin
>>      - Removed 'label' from hdmi-connector, which is optional
> 
> Please respond to each Bjorn comment and explain how did you implement it...
> 

Yes, agreed. Even though it seems like you mostly just agreed to mine 
and Bjorn's suggestions and decided to implement all those in v3 , it 
would have been better to explicitly ack them or tell why you agreed or 
what went wrong that you had not done it in v2 itself to close the loop.

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski May 8, 2024, 7:38 a.m. UTC | #6
On 07/05/2024 21:20, Abhinav Kumar wrote:
> 
> 
> On 5/7/2024 9:35 AM, Krzysztof Kozlowski wrote:
>> On 07/05/2024 18:30, Venkata Prahlad Valluru wrote:
>>> Rb3Gen2 has a lt9611uxc DSI-to-HDMI bridge on i2c0, with
>>> reset gpio from pm7250b gpio2 and irq gpio from tlmm gpio24.
>>> Bridge supplies are Vdd connected to input supply directly
>>> and vcc to L11c. Enable HDMI output, bridge and corresponding
>>> DSI output.
>>>
>>> Signed-off-by: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
>>> ---
>>> v3: - Updated commit text
>>>      - Arranged nodes in alphabetical order
>>>      - Fixed signoff
>>>      - Fixed drive strength for lt9611_irq_pin
>>>      - Removed 'label' from hdmi-connector, which is optional
>>
>> Please respond to each Bjorn comment and explain how did you implement it...
>>
> 
> Yes, agreed. Even though it seems like you mostly just agreed to mine 
> and Bjorn's suggestions and decided to implement all those in v3 , it 
> would have been better to explicitly ack them or tell why you agreed or 
> what went wrong that you had not done it in v2 itself to close the loop.

The problem is that one or more were ignored... By responding to each of
them, I hope they will be finally read and understood instead of
repeating the same mistake three times.

Best regards,
Krzysztof
Dmitry Baryshkov May 8, 2024, 8:13 a.m. UTC | #7
On Wed, 8 May 2024 at 10:38, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 07/05/2024 21:20, Abhinav Kumar wrote:
> >
> >
> > On 5/7/2024 9:35 AM, Krzysztof Kozlowski wrote:
> >> On 07/05/2024 18:30, Venkata Prahlad Valluru wrote:
> >>> Rb3Gen2 has a lt9611uxc DSI-to-HDMI bridge on i2c0, with
> >>> reset gpio from pm7250b gpio2 and irq gpio from tlmm gpio24.
> >>> Bridge supplies are Vdd connected to input supply directly
> >>> and vcc to L11c. Enable HDMI output, bridge and corresponding
> >>> DSI output.
> >>>
> >>> Signed-off-by: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
> >>> ---
> >>> v3: - Updated commit text
> >>>      - Arranged nodes in alphabetical order
> >>>      - Fixed signoff
> >>>      - Fixed drive strength for lt9611_irq_pin
> >>>      - Removed 'label' from hdmi-connector, which is optional
> >>
> >> Please respond to each Bjorn comment and explain how did you implement it...
> >>
> >
> > Yes, agreed. Even though it seems like you mostly just agreed to mine
> > and Bjorn's suggestions and decided to implement all those in v3 , it
> > would have been better to explicitly ack them or tell why you agreed or
> > what went wrong that you had not done it in v2 itself to close the loop.
>
> The problem is that one or more were ignored... By responding to each of
> them, I hope they will be finally read and understood instead of
> repeating the same mistake three times.

I found a mistake that was repeated two times. This leaves me
wondering which mistake was repeated three times.
Prahlad Valluru May 8, 2024, 10:22 a.m. UTC | #8
On Mon, May 06, 2024 at 06:14:10PM -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.

My internal gitconfig is not configured correctly. Fixed in v3.

> 
> > 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."
> 

Agreed. Updated the commit text to include bridge details.

> > 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
> > ---
> >  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.

Fixed the checkpatch issues.

> 
> > +	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.
> 

Moved the dsi nodes on top of edp. Didn't realise this.

> > +        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.

Accepted. I dont see any reason to have drive strength 8. 

> 
> Regards,
> Bjorn
> 
> > +                bias-disable;
> > +        };
> > +};
> > -- 
> > 2.17.1
> >
Prahlad Valluru May 8, 2024, 10:26 a.m. UTC | #9
On Wed, May 08, 2024 at 09:38:50AM +0200, Krzysztof Kozlowski wrote:
> On 07/05/2024 21:20, Abhinav Kumar wrote:
> > 
> > 
> > On 5/7/2024 9:35 AM, Krzysztof Kozlowski wrote:
> >> On 07/05/2024 18:30, Venkata Prahlad Valluru wrote:
> >>> Rb3Gen2 has a lt9611uxc DSI-to-HDMI bridge on i2c0, with
> >>> reset gpio from pm7250b gpio2 and irq gpio from tlmm gpio24.
> >>> Bridge supplies are Vdd connected to input supply directly
> >>> and vcc to L11c. Enable HDMI output, bridge and corresponding
> >>> DSI output.
> >>>
> >>> Signed-off-by: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
> >>> ---
> >>> v3: - Updated commit text
> >>>      - Arranged nodes in alphabetical order
> >>>      - Fixed signoff
> >>>      - Fixed drive strength for lt9611_irq_pin
> >>>      - Removed 'label' from hdmi-connector, which is optional
> >>
> >> Please respond to each Bjorn comment and explain how did you implement it...
> >>
> > 
> > Yes, agreed. Even though it seems like you mostly just agreed to mine 
> > and Bjorn's suggestions and decided to implement all those in v3 , it 
> > would have been better to explicitly ack them or tell why you agreed or 
> > what went wrong that you had not done it in v2 itself to close the loop.
> 
> The problem is that one or more were ignored... By responding to each of
> them, I hope they will be finally read and understood instead of
> repeating the same mistake three times.

Agreed. Will make sure to acknowledge the comments.

> 
> Best regards,
> Krzysztof
>
Dmitry Baryshkov May 8, 2024, 10:47 a.m. UTC | #10
On Wed, 8 May 2024 at 13:22, Prahlad Valluru <quic_vvalluru@quicinc.com> wrote:
>
> On Mon, May 06, 2024 at 06:14:10PM -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.
>
> My internal gitconfig is not configured correctly. Fixed in v3.

No. V3 was still sent as a reply. Please fix the way you are sending
the patches. It has nothing to do with the git config.

>
> >
> > > 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."
> >
>
> Agreed. Updated the commit text to include bridge details.

>
Rob Herring (Arm) May 8, 2024, 2:31 p.m. UTC | #11
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. 

> > ---
> >  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#
Krzysztof Kozlowski May 9, 2024, 8:31 a.m. UTC | #12
On 08/05/2024 10:13, Dmitry Baryshkov wrote:
> On Wed, 8 May 2024 at 10:38, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 07/05/2024 21:20, Abhinav Kumar wrote:
>>>
>>>
>>> On 5/7/2024 9:35 AM, Krzysztof Kozlowski wrote:
>>>> On 07/05/2024 18:30, Venkata Prahlad Valluru wrote:
>>>>> Rb3Gen2 has a lt9611uxc DSI-to-HDMI bridge on i2c0, with
>>>>> reset gpio from pm7250b gpio2 and irq gpio from tlmm gpio24.
>>>>> Bridge supplies are Vdd connected to input supply directly
>>>>> and vcc to L11c. Enable HDMI output, bridge and corresponding
>>>>> DSI output.
>>>>>
>>>>> Signed-off-by: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
>>>>> ---
>>>>> v3: - Updated commit text
>>>>>      - Arranged nodes in alphabetical order
>>>>>      - Fixed signoff
>>>>>      - Fixed drive strength for lt9611_irq_pin
>>>>>      - Removed 'label' from hdmi-connector, which is optional
>>>>
>>>> Please respond to each Bjorn comment and explain how did you implement it...
>>>>
>>>
>>> Yes, agreed. Even though it seems like you mostly just agreed to mine
>>> and Bjorn's suggestions and decided to implement all those in v3 , it
>>> would have been better to explicitly ack them or tell why you agreed or
>>> what went wrong that you had not done it in v2 itself to close the loop.
>>
>> The problem is that one or more were ignored... By responding to each of
>> them, I hope they will be finally read and understood instead of
>> repeating the same mistake three times.
> 
> I found a mistake that was repeated two times. This leaves me
> wondering which mistake was repeated three times.

That was about future, I was expecting v4 as in-reply-to :)

Best regards,
Krzysztof
Prahlad Valluru May 15, 2024, 3:04 p.m. UTC | #13
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#
> 
> 
> 
> 
>
Dmitry Baryshkov May 15, 2024, 3:28 p.m. UTC | #14
On Wed, 15 May 2024 at 17:05, Prahlad Valluru <quic_vvalluru@quicinc.com> wrote:
>
> 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.

s/add dummy vreg/describe fixed hardware register/
Dmitry Baryshkov May 28, 2024, 3 p.m. UTC | #15
On Tue, May 28, 2024 at 07:49:54PM +0530, Venkata Prahlad Valluru wrote:
> Rb3Gen2 has a lt9611uxc DSI-to-HDMI bridge on i2c0, with
> reset gpio from pm7250b gpio2 and irq gpio from tlmm gpio24.
> Bridge supplies are Vdd connected to input supply directly
> and vcc to L11c. Enable HDMI output, bridge and corresponding
> DSI output.
> 
> Signed-off-by: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
> ---
> v4: added fixed regulator for vdd

Please don't send new iterations as replies to the previous iteration.
It might be ignored or mishandled by the tools.

> 
> v3: - Updated commit text
>     - Arranged nodes in alphabetical order
>     - Fixed signoff
>     - Fixed drive strength for lt9611_irq_pin
>     - Removed 'label' from hdmi-connector, which is optional
> 
> v2: Addressed dtschema errors
> 	- Fixed lt9611-irq
> 	- vdd-supply error to be ignored, as it is connected to
> 	  input supply directly, on rb3gen2
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 94 ++++++++++++++++++++
>  1 file changed, 94 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index a085ff5b5fb2..7f00fca131a2 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -52,6 +52,25 @@
>  		};
>  	};
>  
> +	hdmi-connector {
> +		compatible = "hdmi-connector";
> +		type = "a";
> +
> +		port {
> +			hdmi_con: endpoint {
> +				remote-endpoint = <&lt9611_out>;
> +			};
> +		};
> +	};
> +
> +	lt9611_1v2: lt9611-vdd12-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "LT9611_1V2";

Is it the regulator / net name in schematics? Or is it also used by any
other consumers?

> +
> +		regulator-min-microvolt = <1200000>;
> +		regulator-max-microvolt = <1200000>;
> +	};
> +
>  	reserved-memory {
>  		xbl_mem: xbl@80700000 {
>  			reg = <0x0 0x80700000 0x0 0x100000>;
> @@ -530,6 +549,46 @@
>  			   <GCC_WPSS_RSCP_CLK>;
>  };
>  
> +&i2c0 {
> +	clock-frequency = <400000>;
> +	status = "okay";
> +
> +	lt9611_codec: hdmi-bridge@2b {
> +		compatible = "lontium,lt9611uxc";
> +		reg = <0x2b>;
> +
> +		interrupts-extended = <&tlmm 24 IRQ_TYPE_EDGE_FALLING>;
> +		reset-gpios = <&pm7250b_gpios 2 GPIO_ACTIVE_HIGH>;
> +
> +		vdd-supply = <&lt9611_1v2>;
> +		vcc-supply = <&vreg_l11c_2p8>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&lt9611_irq_pin &lt9611_rst_pin>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				lt9611_a: endpoint {
> +					remote-endpoint = <&mdss_dsi0_out>;
> +				};
> +			};
> +
> +			port@2 {
> +				reg = <2>;
> +
> +				lt9611_out: endpoint {
> +					remote-endpoint = <&hdmi_con>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
>  &i2c1 {
>  	status = "okay";
>  
> @@ -587,6 +646,21 @@
>  	remote-endpoint = <&usb_dp_qmpphy_dp_in>;
>  };
>  
> +&mdss_dsi {
> +	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";
> +};
> +
>  &mdss_edp {
>  	status = "okay";
>  };
> @@ -711,3 +785,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 = <2>;
> +		bias-disable;
> +	};
> +};
> -- 
> 2.17.1
>
Prahlad Valluru May 28, 2024, 4:24 p.m. UTC | #16
On Tue, May 28, 2024 at 06:00:26PM +0300, Dmitry Baryshkov wrote:
> On Tue, May 28, 2024 at 07:49:54PM +0530, Venkata Prahlad Valluru wrote:
> > Rb3Gen2 has a lt9611uxc DSI-to-HDMI bridge on i2c0, with
> > reset gpio from pm7250b gpio2 and irq gpio from tlmm gpio24.
> > Bridge supplies are Vdd connected to input supply directly
> > and vcc to L11c. Enable HDMI output, bridge and corresponding
> > DSI output.
> > 
> > Signed-off-by: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
> > ---
> > v4: added fixed regulator for vdd
> 
> Please don't send new iterations as replies to the previous iteration.
> It might be ignored or mishandled by the tools.
Was trying to have the context of discussion captured. Will add links
to the patchsets in the history, going forward.
> 
> > 
> > v3: - Updated commit text
> >     - Arranged nodes in alphabetical order
> >     - Fixed signoff
> >     - Fixed drive strength for lt9611_irq_pin
> >     - Removed 'label' from hdmi-connector, which is optional
> > 
> > v2: Addressed dtschema errors
> > 	- Fixed lt9611-irq
> > 	- vdd-supply error to be ignored, as it is connected to
> > 	  input supply directly, on rb3gen2
> > ---
> >  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 94 ++++++++++++++++++++
> >  1 file changed, 94 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > index a085ff5b5fb2..7f00fca131a2 100644
> > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > @@ -52,6 +52,25 @@
> >  		};
> >  	};
> >  
> > +	hdmi-connector {
> > +		compatible = "hdmi-connector";
> > +		type = "a";
> > +
> > +		port {
> > +			hdmi_con: endpoint {
> > +				remote-endpoint = <&lt9611_out>;
> > +			};
> > +		};
> > +	};
> > +
> > +	lt9611_1v2: lt9611-vdd12-regulator {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "LT9611_1V2";
> 
> Is it the regulator / net name in schematics? Or is it also used by any
> other consumers?
>
VREG_HDMI_OUT_1P2 is the naming in schematic, but i see this naming convention for rb5 based boards.
No other consumers for this.
> > +
> > +		regulator-min-microvolt = <1200000>;
> > +		regulator-max-microvolt = <1200000>;
> > +	};
> > +
> >  	reserved-memory {
> >  		xbl_mem: xbl@80700000 {
> >  			reg = <0x0 0x80700000 0x0 0x100000>;
> > @@ -530,6 +549,46 @@
> >  			   <GCC_WPSS_RSCP_CLK>;
> >  };
> >  
> > +&i2c0 {
> > +	clock-frequency = <400000>;
> > +	status = "okay";
> > +
> > +	lt9611_codec: hdmi-bridge@2b {
> > +		compatible = "lontium,lt9611uxc";
> > +		reg = <0x2b>;
> > +
> > +		interrupts-extended = <&tlmm 24 IRQ_TYPE_EDGE_FALLING>;
> > +		reset-gpios = <&pm7250b_gpios 2 GPIO_ACTIVE_HIGH>;
> > +
> > +		vdd-supply = <&lt9611_1v2>;
> > +		vcc-supply = <&vreg_l11c_2p8>;
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&lt9611_irq_pin &lt9611_rst_pin>;
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> > +
> > +				lt9611_a: endpoint {
> > +					remote-endpoint = <&mdss_dsi0_out>;
> > +				};
> > +			};
> > +
> > +			port@2 {
> > +				reg = <2>;
> > +
> > +				lt9611_out: endpoint {
> > +					remote-endpoint = <&hdmi_con>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +};
> > +
> >  &i2c1 {
> >  	status = "okay";
> >  
> > @@ -587,6 +646,21 @@
> >  	remote-endpoint = <&usb_dp_qmpphy_dp_in>;
> >  };
> >  
> > +&mdss_dsi {
> > +	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";
> > +};
> > +
> >  &mdss_edp {
> >  	status = "okay";
> >  };
> > @@ -711,3 +785,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 = <2>;
> > +		bias-disable;
> > +	};
> > +};
> > -- 
> > 2.17.1
> > 
> 
> -- 
> With best wishes
> Dmitry
Dmitry Baryshkov May 28, 2024, 4:33 p.m. UTC | #17
On Tue, May 28, 2024 at 09:54:34PM +0530, Prahlad Valluru wrote:
> On Tue, May 28, 2024 at 06:00:26PM +0300, Dmitry Baryshkov wrote:
> > On Tue, May 28, 2024 at 07:49:54PM +0530, Venkata Prahlad Valluru wrote:
> > > Rb3Gen2 has a lt9611uxc DSI-to-HDMI bridge on i2c0, with
> > > reset gpio from pm7250b gpio2 and irq gpio from tlmm gpio24.
> > > Bridge supplies are Vdd connected to input supply directly
> > > and vcc to L11c. Enable HDMI output, bridge and corresponding
> > > DSI output.
> > > 
> > > Signed-off-by: Venkata Prahlad Valluru <quic_vvalluru@quicinc.com>
> > > ---
> > > v4: added fixed regulator for vdd
> > 
> > Please don't send new iterations as replies to the previous iteration.
> > It might be ignored or mishandled by the tools.
> Was trying to have the context of discussion captured. Will add links
> to the patchsets in the history, going forward.

I'd suggest using b4 tool, which handles such issues in an automated
way. It adds links to previous iterations, changelog, etc.

> > 
> > > 
> > > v3: - Updated commit text
> > >     - Arranged nodes in alphabetical order
> > >     - Fixed signoff
> > >     - Fixed drive strength for lt9611_irq_pin
> > >     - Removed 'label' from hdmi-connector, which is optional
> > > 
> > > v2: Addressed dtschema errors
> > > 	- Fixed lt9611-irq
> > > 	- vdd-supply error to be ignored, as it is connected to
> > > 	  input supply directly, on rb3gen2
> > > ---
> > >  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 94 ++++++++++++++++++++
> > >  1 file changed, 94 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > index a085ff5b5fb2..7f00fca131a2 100644
> > > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > @@ -52,6 +52,25 @@
> > >  		};
> > >  	};
> > >  
> > > +	hdmi-connector {
> > > +		compatible = "hdmi-connector";
> > > +		type = "a";
> > > +
> > > +		port {
> > > +			hdmi_con: endpoint {
> > > +				remote-endpoint = <&lt9611_out>;
> > > +			};
> > > +		};
> > > +	};
> > > +
> > > +	lt9611_1v2: lt9611-vdd12-regulator {
> > > +		compatible = "regulator-fixed";
> > > +		regulator-name = "LT9611_1V2";
> > 
> > Is it the regulator / net name in schematics? Or is it also used by any
> > other consumers?
> >
> VREG_HDMI_OUT_1P2 is the naming in schematic, but i see this naming convention for rb5 based boards.
> No other consumers for this.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> > > +
> > > +		regulator-min-microvolt = <1200000>;
> > > +		regulator-max-microvolt = <1200000>;
> > > +	};
> > > +
> > >  	reserved-memory {
> > >  		xbl_mem: xbl@80700000 {
> > >  			reg = <0x0 0x80700000 0x0 0x100000>;
> > > @@ -530,6 +549,46 @@
> > >  			   <GCC_WPSS_RSCP_CLK>;
> > >  };
> > >  
> > > +&i2c0 {
> > > +	clock-frequency = <400000>;
> > > +	status = "okay";
> > > +
> > > +	lt9611_codec: hdmi-bridge@2b {
> > > +		compatible = "lontium,lt9611uxc";
> > > +		reg = <0x2b>;
> > > +
> > > +		interrupts-extended = <&tlmm 24 IRQ_TYPE_EDGE_FALLING>;
> > > +		reset-gpios = <&pm7250b_gpios 2 GPIO_ACTIVE_HIGH>;
> > > +
> > > +		vdd-supply = <&lt9611_1v2>;
> > > +		vcc-supply = <&vreg_l11c_2p8>;
> > > +
> > > +		pinctrl-names = "default";
> > > +		pinctrl-0 = <&lt9611_irq_pin &lt9611_rst_pin>;
> > > +
> > > +		ports {
> > > +			#address-cells = <1>;
> > > +			#size-cells = <0>;
> > > +
> > > +			port@0 {
> > > +				reg = <0>;
> > > +
> > > +				lt9611_a: endpoint {
> > > +					remote-endpoint = <&mdss_dsi0_out>;
> > > +				};
> > > +			};
> > > +
> > > +			port@2 {
> > > +				reg = <2>;
> > > +
> > > +				lt9611_out: endpoint {
> > > +					remote-endpoint = <&hdmi_con>;
> > > +				};
> > > +			};
> > > +		};
> > > +	};
> > > +};
> > > +
> > >  &i2c1 {
> > >  	status = "okay";
> > >  
> > > @@ -587,6 +646,21 @@
> > >  	remote-endpoint = <&usb_dp_qmpphy_dp_in>;
> > >  };
> > >  
> > > +&mdss_dsi {
> > > +	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";
> > > +};
> > > +
> > >  &mdss_edp {
> > >  	status = "okay";
> > >  };
> > > @@ -711,3 +785,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 = <2>;
> > > +		bias-disable;
> > > +	};
> > > +};
> > > -- 
> > > 2.17.1
> > > 
> > 
> > -- 
> > With best wishes
> > Dmitry
diff mbox series

Patch

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 @@ 
 			};
 		};
 	};
+	
+	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 @@ 
 			   <GCC_WPSS_RSCP_CLK>;
 };
 
+&i2c0 {
+	clock-frequency = <400000>;
+	status = "okay";
+
+	lt9611_codec: hdmi-bridge@2b {
+		compatible = "lontium,lt9611uxc";
+		reg = <0x2b>;
+
+		interrupts-extended = <&tlmm 24 IRQ_TYPE_EDGE_FALLING>;
+		reset-gpios = <&pm7250b_gpios 2 GPIO_ACTIVE_HIGH>;
+
+		vcc-supply = <&vreg_l11c_2p8>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&lt9611_irq_pin &lt9611_rst_pin>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				lt9611_a: endpoint {
+					remote-endpoint = <&mdss_dsi0_out>;
+				};
+			};
+
+			port@2 {
+				reg = <2>;
+
+				lt9611_out: endpoint {
+					remote-endpoint = <&hdmi_con>;
+				};
+			};
+		};
+	};
+};
+
 &i2c1 {
 	status = "okay";
 
@@ -602,6 +653,21 @@ 
 	status = "okay";
 };
 
+&mdss_dsi {
+        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>;
+                bias-disable;
+        };
+};