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

[Aradhya Bhatia]

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 at ti.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 at ti.com>
>>> [j-choudhary at ti.com: move all the changes together to enable DP-0 in
>>> EVM]
>>> Signed-off-by: Jayesh Choudhary <j-choudhary at ti.com>
>>> ---
>>>   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 at 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 at 100000/dss at 4a00000/ports: graph node has
> single child node 'port at 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 at 0" should be
specified instead of just "port" and the warning should be ignored, if
possible.

If there were only a "port at 1" child node, this warning would not have
come up, and I believe "port at 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>;
> 
> 
> [...]