Re: [PATCH v8 5/5] arm64: dts: ti: k3-am69-sk: Add DP and HDMI support

[Jayesh Choudhary]

Hello Roger,

On 01/08/23 22:28, Roger Quadros wrote:
> +Tomi (fixed email)
>
> Need to fix this in
> Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
>
> On 01/08/2023 19:48, Roger Quadros wrote:
> > On 01/08/2023 10:00, Jayesh Choudhary wrote:
> > > From: Dasnavis Sabiya <sabiya.d@xxxxxx>
> > >
> > > AM69 starter kit features an HDMI port and an eDP port.
> > >
> > > Add assigned clocks for DSS, DT node for DisplayPort PHY,
> > > pinmux for HDMI hotplug and power down, mcu_i2c1 and dss_vout
> > > for HDMI.
> > > Also enable Serdes4 settings for DP display.
> > >
> > > Add the endpoint nodes to describe connection from:
> > > DSS => MHDP => DisplayPort connector
> > > DSS => TI TFP410 DPI-to-DVI Bridge => HDMI connector
> > >
> > > Signed-off-by: Dasnavis Sabiya <sabiya.d@xxxxxx>
> > > [j-choudhary@xxxxxx: Fix dvi-bridge, dss, mhdp and serdes-refclk]
> > > Signed-off-by: Jayesh Choudhary <j-choudhary@xxxxxx>
> > > ---
> > >   arch/arm64/boot/dts/ti/k3-am69-sk.dts | 237 ++++++++++++++++++++++++++
> > >   1 file changed, 237 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/ti/k3-am69-sk.dts b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> > > index d282c2c633c1..d9a0794bcf6e 100644
> > > --- a/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> > > +++ b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> > > @@ -107,6 +107,83 @@ vdd_sd_dv: regulator-tlv71033 {
> > >   		states = <1800000 0x0>,
> > >   			 <3300000 0x1>;
> > >   	};
> > > +
> > > +	dp0_pwr_3v3: regulator {
> >
> > To be consistent with the rest of the file please
> > use regulator-dp0-pwr

Sure.

> > > +		compatible = "regulator-fixed";
> > > +		regulator-name = "dp0-pwr";
> > > +		regulator-min-microvolt = <3300000>;
> > > +		regulator-max-microvolt = <3300000>;
> > > +		pinctrl-names = "default";
> > > +		pinctrl-0 = <&dp_pwr_en_pins_default>;
> > > +		gpio = <&main_gpio0 4 0>;	/* DP0_3V3 _EN */
> > > +		enable-active-high;
> > > +	};
> > > +
> > > +	dp0: connector {
> >
> > connector-dp0

Ok

> > > +		compatible = "dp-connector";
> > > +		label = "DP0";
> > > +		type = "full-size";
> > > +		dp-pwr-supply = <&dp0_pwr_3v3>;
> > > +
> > > +		port {
> > > +			dp0_connector_in: endpoint {
> > > +				remote-endpoint = <&dp0_out>;
> > > +			};
> > > +		};
> > > +	};
> > > +
> > > +	hdmi-connector {
> >
> > connector-hdmi

Ok.

> > > +		compatible = "hdmi-connector";
> > > +		label = "hdmi";
> > > +		type = "a";
> > > +
> > > +		pinctrl-names = "default";
> > > +		pinctrl-0 = <&hdmi_hpd_pins_default>;
> > > +
> > > +		ddc-i2c-bus = <&mcu_i2c1>;
> > > +
> > > +		/* HDMI_HPD */
> > > +		hpd-gpios = <&main_gpio0 0 GPIO_ACTIVE_HIGH>;
> > > +
> > > +		port {
> > > +			hdmi_connector_in: endpoint {
> > > +				remote-endpoint = <&tfp410_out>;
> > > +			};
> > > +		};
> > > +	};
> > > +
> > > +	dvi-bridge {
> >
> > Although DT binding doc for ti,tfp410 says encoder
> > a name starting with bridge maybe more appropriate.

Will keep it as bridge-dvi.

Will edit the node names in suggested in 3/5 and 4/5 as well.
I will address the new-line comments too.

Thanks
-Jayesh

> >

[...]