[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9f19e01e-6211-16eb-c911-998ee2d46161@ti.com>
Date: Sat, 5 Aug 2023 00:52:25 +0530
From: Aradhya Bhatia <a-bhatia1@...com>
To: Jayesh Choudhary <j-choudhary@...com>, <nm@...com>,
<vigneshr@...com>, <afd@...com>, <rogerq@...nel.org>
CC: <s-vadapalli@...com>, <kristo@...nel.org>, <robh+dt@...nel.org>,
<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
<r-ravikumar@...com>, <sabiya.d@...com>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v9 4/5] arm64: dts: ti: k3-j784s4-evm: Enable
DisplayPort-0
Hi Jayesh,
On 03-Aug-23 13:34, Jayesh Choudhary wrote:
> From: Rahul T R <r-ravikumar@...com>
>
> Enable display for J784S4 EVM.
>
> Add assigned clocks for DSS, DT node for DisplayPort PHY and pinmux for
> DP HPD. Add the clock frequency for serdes_refclk.
>
> Add the endpoint nodes to describe connection from:
> DSS => MHDP => DisplayPort connector.
>
> Also add the GPIO expander-4 node and pinmux for main_i2c4 which is
> required for controlling DP power. Set status for all required nodes
> for DP-0 as "okay".
>
> Signed-off-by: Rahul T R <r-ravikumar@...com>
> [j-choudhary@...com: move all the changes together to enable DP-0 in EVM]
> Signed-off-by: Jayesh Choudhary <j-choudhary@...com>
> ---
> arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 119 +++++++++++++++++++++++
> 1 file changed, 119 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> index 7ad152a1b90f..005357d70122 100644
> --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> @@ -249,6 +249,28 @@ vdd_sd_dv: regulator-TLV71033 {
> states = <1800000 0x0>,
> <3300000 0x1>;
> };
> +
> + dp0_pwr_3v3: regulator-dp0-prw {
> + compatible = "regulator-fixed";
> + regulator-name = "dp0-pwr";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&exp4 0 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + dp0: connector-dp0 {
> + compatible = "dp-connector";
> + label = "DP0";
> + type = "full-size";
> + dp-pwr-supply = <&dp0_pwr_3v3>;
> +
> + port {
> + dp0_connector_in: endpoint {
> + remote-endpoint = <&dp0_out>;
> + };
> + };
> + };
> };
>
> &main_pmx0 {
> @@ -286,6 +308,19 @@ vdd_sd_dv_pins_default: vdd-sd-dv-default-pins {
> J784S4_IOPAD(0x020, PIN_INPUT, 7) /* (AJ35) MCAN15_RX.GPIO0_8 */
> >;
> };
> +
> + dp0_pins_default: dp0-default-pins {
> + pinctrl-single,pins = <
> + J784S4_IOPAD(0x0cc, PIN_INPUT, 12) /* (AM37) SPI0_CS0.DP0_HPD */
> + >;
> + };
> +
> + main_i2c4_pins_default: main-i2c4-default-pins {
> + pinctrl-single,pins = <
> + J784S4_IOPAD(0x014, PIN_INPUT_PULLUP, 8) /* (AG33) MCAN14_TX.I2C4_SCL */
> + J784S4_IOPAD(0x010, PIN_INPUT_PULLUP, 8) /* (AH33) MCAN13_RX.I2C4_SDA */
> + >;
> + };
> };
>
> &wkup_pmx2 {
> @@ -827,3 +862,87 @@ adc {
> ti,adc-channels = <0 1 2 3 4 5 6 7>;
> };
> };
> +
> +&serdes_refclk {
> + status = "okay";
> + clock-frequency = <100000000>;
> +};
> +
> +&dss {
> + status = "okay";
> + assigned-clocks = <&k3_clks 218 2>,
> + <&k3_clks 218 5>,
> + <&k3_clks 218 14>,
> + <&k3_clks 218 18>;
> + assigned-clock-parents = <&k3_clks 218 3>,
> + <&k3_clks 218 7>,
> + <&k3_clks 218 16>,
> + <&k3_clks 218 22>;
> +};
> +
> +&serdes_wiz4 {
> + status = "okay";
> +};
> +
> +&serdes4 {
> + status = "okay";
> + serdes4_dp_link: phy@0 {
> + reg = <0>;
> + cdns,num-lanes = <4>;
> + #phy-cells = <0>;
> + cdns,phy-type = <PHY_TYPE_DP>;
> + resets = <&serdes_wiz4 1>, <&serdes_wiz4 2>,
> + <&serdes_wiz4 3>, <&serdes_wiz4 4>;
> + };
> +};
> +
> +&mhdp {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&dp0_pins_default>;
> + phys = <&serdes4_dp_link>;
> + phy-names = "dpphy";
> +};
> +
> +&dss_ports {
> + port {
Port index has not been added here. Since this port outputs to MHDP
bridge, this should be "port@0", and a "reg = <0>;" property should be
added below (along with the address and size cells properties).
I suppose this works functionally in this case, because the port gets
defaulted to "0" by the driver. But in future, when we add support for
other dss output(s) on j784s4-evm, the driver will need indices to
distinguish among them.
> + dpi0_out: endpoint {
> + remote-endpoint = <&dp0_in>;
> + };
> + };
> +};
> +
> +&main_i2c4 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&main_i2c4_pins_default>;
> + clock-frequency = <400000>;
> +
> + exp4: gpio@20 {
> + compatible = "ti,tca6408";
> + reg = <0x20>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +};
> +
> +&dp0_ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
These properties are being repeated from the MHDP node in the main.dtsi
file, in the previous patch. You can drop them from one of the places.
I would suggest keeping them in the main.dtsi file and removing them
here, so that repetition is avoided across different platform.dts files,
but I will leave the call up to you. In any case, they need to be
removed from one of the two places.
Btw, same will be applicable for dss_ports as well (once you add port
index). Separate address and size cells properties won't be required in
the j784s4-evm.dts and am69-sk.dts platform files, once they are already
there in j784s4-main.dtsi.
With the changes suggested above,
Reviewed-by: Aradhya Bhatia <a-bhatia1@...com>
> +
> + port@0 {
> + reg = <0>;
> +
> + dp0_in: endpoint {
> + remote-endpoint = <&dpi0_out>;
> + };
> + };
> +
> + port@4 {
> + reg = <4>;
> +
> + dp0_out: endpoint {
> + remote-endpoint = <&dp0_connector_in>;
> + };
> + };
> +};
Powered by blists - more mailing lists