On 08/08/23 00:24, Andrew Davis wrote:
On 8/7/23 1:29 PM, Aradhya Bhatia wrote:
On 07-Aug-23 21:19, Andrew Davis wrote:
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.
The warning that Jayesh mentioned above comes when "port@0" is
mentioned, *along-with* the #address-cells/#size-cells properties.
Essentially, it wants us to not use "port@0" when only single port is
being added whose reg values is 0.
This warning does not come when only a single port other than 0,
"port@1" for e.g., is being used. That's the warning, that should get
ignored, if possible.
Ah, I see now.
Almost seems like a bug in dtc checks, but checking the code it
looks deliberate, although I cannot see why..
Rob,
Could you provide some guidance on why graph nodes are handled
this way? Seems this is valid:
ports {
#address-cells = <1>;
#size-cells = <0>;
port@1 {
reg = <1>;
};
}
but this is not:
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
};
};
I'm guessing we allow port 0 to not be numbered if it is the only
one for legacy convenience, but *forcing* it to not be numbered
when it would otherwise be more consistent seems overly strict.
Andrew
Hello Rob, Krzysztof,
For this series, v11 has been already reviewed by Roger and Aradhya:
<https://lore.kernel.org/all/77701023-7bd1-4e04-aa44-0e46aa087c4f@xxxxxxxxxx/>
Only this warning persist. Can you ACK the series so that it can
be queued/merged.
If W=1 warning is not acceptable, I can revert to port description
here in v9.
Warm Regards,
Jayesh
However, just mentioning "port@0", without the #address-cells/
#size-cells, would be plain wrong.
Regards
Aradhya
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>;
[...]
