2024-04-25 07:14:21

by Prahlad Valluru

[permalink] [raw]
Subject: [PATCH] arm64: dts: qcom: qcs6490-rb3gen2: enable hdmi bridge

Enable lt9611uxc bridge for qcs6490 rb3 gen2 platform.

Signed-off-by: Prahlad Valluru <[email protected]>
---
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..ff963f94c0e3 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 {
+ pins = "gpio24";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-disable;
+ };
+};
--
2.17.1



2024-04-25 14:51:45

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: qcs6490-rb3gen2: enable hdmi bridge


On Thu, 25 Apr 2024 12:43:54 +0530, Venkata Prahlad Valluru wrote:
> Enable lt9611uxc bridge for qcs6490 rb3 gen2 platform.
>
> Signed-off-by: Prahlad Valluru <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 86 ++++++++++++++++++++
> 1 file changed, 86 insertions(+)
>


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 [email protected]:

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#
arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dtb: pinctrl@f100000: Unevaluated properties are not allowed ('lt9611-irq' was unexpected)
from schema $id: http://devicetree.org/schemas/pinctrl/qcom,sc7280-pinctrl.yaml#






2024-04-26 14:27:58

by Prahlad Valluru

[permalink] [raw]
Subject: [PATCH v2] arm64: dts: qcom: qcs6490-rb3gen2: enable hdmi bridge

From: Venkata Prahlad Valluru <[email protected]>

Enable lt9611uxc bridge for qcs6490 rb3 gen2 platform.

Signed-off-by: Prahlad Valluru <[email protected]>
---
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";
+ 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;
+ };
+};
--
2.17.1


2024-05-06 19:55:01

by Abhinav Kumar

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



On 4/26/2024 7:24 AM, Prahlad Valluru wrote:
> From: Venkata Prahlad Valluru <[email protected]>
>
> Enable lt9611uxc bridge for qcs6490 rb3 gen2 platform.
>
> Signed-off-by: Prahlad Valluru <[email protected]>
> ---

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;
> + };
> +};

2024-05-06 23:14:30

by Bjorn Andersson

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

On Fri, Apr 26, 2024 at 07:54:42PM GMT, Prahlad Valluru wrote:
> From: Venkata Prahlad Valluru <[email protected]>
>

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 <[email protected]>
> ---
> 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
>

2024-05-07 16:45:01

by Prahlad Valluru

[permalink] [raw]
Subject: [PATCH v3] arm64: dts: qcom: qcs6490-rb3gen2: enable hdmi bridge

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 <[email protected]>
---
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(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index a085ff5b5fb2..24b0b9525ea4 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -52,6 +52,17 @@
};
};

+ hdmi-connector {
+ compatible = "hdmi-connector";
+ type = "a";
+
+ port {
+ hdmi_con: endpoint {
+ remote-endpoint = <&lt9611_out>;
+ };
+ };
+ };
+
reserved-memory {
xbl_mem: xbl@80700000 {
reg = <0x0 0x80700000 0x0 0x100000>;
@@ -530,6 +541,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";

@@ -587,6 +637,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 +776,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


2024-05-07 18:43:51

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [PATCH v3] arm64: dts: qcom: qcs6490-rb3gen2: enable hdmi bridge



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 <[email protected]>
> ---
> 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 <[email protected]>

2024-05-08 10:22:26

by Prahlad Valluru

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

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 <[email protected]>
> >
>
> 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 <[email protected]>
> > ---
> > 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
> >

2024-05-08 10:48:23

by Dmitry Baryshkov

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

On Wed, 8 May 2024 at 13:22, Prahlad Valluru <[email protected]> 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 <[email protected]>
> > >
> >
> > 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.

>


--
With best wishes
Dmitry

2024-05-08 14:31:43

by Rob Herring

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


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 <[email protected]>
> >
>
> 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 <[email protected]>
> > ---
> > 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#