[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <cjz47arpfbangtrzx6kw4ommph3zhn6xnroz34oqskafvmpnmi@xduotm2houzg>
Date: Thu, 20 Nov 2025 08:26:40 -0600
From: Bjorn Andersson <andersson@...nel.org>
To: Vishnu Saini <vishnu.saini@....qualcomm.com>
Cc: Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Vishnu Saini <vishsain@....qualcomm.com>, prahlad.valluru@....qualcomm.com,
Prahlad Valluru <vvalluru@....qualcomm.com>
Subject: Re: [PATCH 1/2] arm64: dts: qcom: monaco: add lt8713sx bridge with
displayport
On Thu, Nov 20, 2025 at 04:28:05PM +0530, Vishnu Saini wrote:
> Monaco-evk has LT8713sx which act as DP to 3 DP output
> converter. Edp PHY from monaco soc is connected to lt8713sx
> as input and output of lt8713sx is connected to 3 mini DP ports.
> Two of these ports are available in mainboard and one port
> is available on Mezz board. lt8713sx is connected to soc over
> i2c0 and with reset gpio connected to pin6 or ioexpander5.
>
This is good, you're describing the role of LT8713sx and how it's
connected on the board. Thank you for taking the time to do so!
I think one aspect that's worth pointing out on its own is the
placement of the mini DP ports. You do capture it, but the fact that
we have two on the EVK and one on the expansion board is "hidden" there
in the middle of the paragraph.
I think extracting this part of the message into its own paragraph would
improve the commit message further.
> Enable the edp nodes from monaco and enable lontium lt8713sx
> bridge node.
>
> Co-developed-by: Prahlad Valluru <vvalluru@....qualcomm.com>
> Signed-off-by: Prahlad Valluru <vvalluru@....qualcomm.com>
> Signed-off-by: Vishnu Saini <vishnu.saini@....qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/monaco-evk.dts | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/monaco-evk.dts b/arch/arm64/boot/dts/qcom/monaco-evk.dts
> index bb35893da73d..947807f8a9cb 100644
> --- a/arch/arm64/boot/dts/qcom/monaco-evk.dts
> +++ b/arch/arm64/boot/dts/qcom/monaco-evk.dts
> @@ -317,6 +317,20 @@ &gpu_zap_shader {
> firmware-name = "qcom/qcs8300/a623_zap.mbn";
> };
>
Would it be possible to add dp-connector nodes and wire them up to the
<8713sx, like I did in sa8295-adp.dts?
> +&i2c0 {
> + pinctrl-0 = <&qup_i2c0_default>;
> + pinctrl-names = "default";
> +
> + status = "okay";
> +
> + lt8713sx: lt8713sx@4f {
This should be bridge@4f.
Also, unless we need to reference this from a overlay there's no need to
give it a label.
> + /*Display bridge chip, DP1.4/HDMI2.0/DP++ hub*/
This comment explains what "lontium,lt8713sx" is. The binding call tell
me that, so the value of this comment would be for you to tell us what
it is used for on this particular board - and if that's obvious you can
omit the comment.
> + compatible = "lontium,lt8713sx";
> + reg = <0x4f>;
> + reset-gpios = <&expander5 6 GPIO_ACTIVE_HIGH>;
In addition to using an of_graph to describe the connectors that this is
wired to, it would be nice to have a port describing the relationship to
the DP controller here as well - so we know where the signal is coming
from.
Would that be possible to add?
Regards,
Bjorn
> + };
> +};
> +
> &i2c1 {
> pinctrl-0 = <&qup_i2c1_default>;
> pinctrl-names = "default";
> @@ -396,6 +410,23 @@ expander6: gpio@3e {
> };
> };
>
> +&mdss {
> + status = "okay";
> +};
> +
> +&mdss_dp0 {
> + status = "okay";
> +};
> +
> +&mdss_dp0_out {
> + data-lanes = <0 1 2 3>;
> + link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
> +};
> +
> +&mdss_dp0_phy {
> + status = "okay";
> +};
> +
> &iris {
> status = "okay";
> };
> @@ -435,6 +466,12 @@ &serdes0 {
> };
>
> &tlmm {
> + dp_hot_plug_det: dp-hot-plug-det-state {
> + pins = "gpio94";
> + function = "edp0_hot";
> + bias-disable;
> + };
> +
> ethernet0_default: ethernet0-default-state {
> ethernet0_mdc: ethernet0-mdc-pins {
> pins = "gpio5";
> @@ -451,6 +488,13 @@ ethernet0_mdio: ethernet0-mdio-pins {
> };
> };
>
> + qup_i2c0_default: qup-i2c0-state {
> + pins = "gpio17", "gpio18";
> + function = "qup0_se0";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> qup_i2c1_default: qup-i2c1-state {
> pins = "gpio19", "gpio20";
> function = "qup0_se1";
>
> --
> 2.34.1
>
Powered by blists - more mailing lists