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

[Aradhya Bhatia]

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 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.
>>
> 
> Do not ignore new DT check warnings, if you go with "port at 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 at 0" is
mentioned, *along-with* the #address-cells/#size-cells properties.
Essentially, it wants us to not use "port at 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 at 1" for e.g., is being used. That's the warning, that should get
ignored, if possible.

However, just mentioning "port at 0", without the #address-cells/
#size-cells, would be plain wrong.

Regards
Aradhya

> 
>> 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>;
>>>
>>>
>>> [...]
>>