Re: [PATCH v9 4/5] arm64: dts: ti: k3-j784s4-evm: Enable DisplayPort-0

From: Andrew Davis
Date: Mon Aug 07 2023 - 11:49:32 EST


On 8/7/23 7:56 AM, Aradhya Bhatia wrote:
Hi Jayesh,

On 07-Aug-23 17:54, Jayesh Choudhary wrote:
Hello Aradhya,

Thank you for the review.

On 05/08/23 00:52, Aradhya Bhatia wrote:
Hi Jayesh,


On 03-Aug-23 13:34, Jayesh Choudhary wrote:
From: Rahul T R <r-ravikumar@xxxxxx>

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@xxxxxx>
[j-choudhary@xxxxxx: move all the changes together to enable DP-0 in
EVM]
Signed-off-by: Jayesh Choudhary <j-choudhary@xxxxxx>
---
  arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 119 +++++++++++++++++++++++
  1 file changed, 119 insertions(+)

[...]

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


Okay. It makes sense.
Just one thing here. Adding reg here would require it to have #address-
cells and #size-cell but since we have only single child port that too
at reg=<0>, it would throw dtbs_check warning:

arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi:1828.20-1831.5: Warning
(graph_child_address): /bus@100000/dss@4a00000/ports: graph node has
single child node 'port@0', #address-cells/#size-cells are not necessary
  also defined at arch/arm64/boot/dts/ti/k3-j784s4-evm.dts:911.12-919.3


Okay! Was not aware about this. I still think "port@0" should be
specified instead of just "port" and the warning should be ignored, if
possible.


Do not ignore new DT check warnings, if you go with "port@0" (which you
need to do as the "ti,j721e-dss" binding requires it) you must also add
the #address-cells/#size-cells.

Andrew

If there were only a "port@1" child node, this warning would not have
come up, and I believe "port@0" should be treated just the same.

Moreover, while we can add these properties at a later stage as an
incremental patch, adding the size and address cells in the dtsi would
affect other platform dts files as well, that use this SoC.

For e.g., the patch 5/5 of this series, on AM69-SK will still require
the size and address cells for its ports. The clean up then will be that
much more, when adding those incremental patches.

Anyway, I will let Nishanth and Vignesh take the final call on this.

Regards
Aradhya


+        dpi0_out: endpoint {
+            remote-endpoint = <&dp0_in>;


[...]