Re: [PATCH v3 3/4] dt-bindings: display: ti,am65x-dss: Add OLDI properties for AM625 DSS

[Krzysztof Kozlowski <krzk@kernel.org>]

On 26/08/2024 09:32, Aradhya Bhatia wrote:
> Hi Krzysztof,
> 
> 
> On 7/21/24 21:09, Krzysztof Kozlowski wrote:
>> On 16/07/2024 10:42, Aradhya Bhatia wrote:
>>> The DSS in AM625 SoC has 2 OLDI TXes. Refer the OLDI schema to add the
>>> support for the OLDI TXes.
>>>
>>> The AM625 DSS VP1 (port@0) can connect and control 2 OLDI TXes, to use
>>> them in dual-link or cloned single-link OLDI modes. Add support for an
>>> additional endpoint under the port@0 to accurately depict the data flow
>>> path.
>>>
>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>>> ---
>>>  .../bindings/display/ti/ti,am65x-dss.yaml     | 135 ++++++++++++++++++
>>>  1 file changed, 135 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>>> index 399d68986326..249597455d34 100644
>>> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>>> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>>> @@ -91,6 +91,24 @@ properties:
>>>            For AM625 DSS, the internal DPI output port node from video
>>>            port 1.
>>>            For AM62A7 DSS, the port is tied off inside the SoC.
>>> +        properties:
>>> +          endpoint@0:
>>> +            $ref: /schemas/graph.yaml#/properties/endpoint
>>> +            description:
>>> +              For AM625 DSS, VP Connection to OLDI0.
>>> +              For AM65X DSS, OLDI output from the SoC.
>>> +
>>> +          endpoint@1:
>>> +            $ref: /schemas/graph.yaml#/properties/endpoint
>>> +            description:
>>> +              For AM625 DSS, VP Connection to OLDI1.
>>
>> Eh, that's confusing. Why do you have graph to your children? Isn't this
>> entirely pointless?
> 
> I am not sure I fully understand. The same display source video port can
> connect up to 2 OLDI TXes - hence 2 endpoints which connect to the OLDI
> that were described in the previous patch. The idea has been to
> accurately depict the connections of the hardware.
> 
> What am I missing here?

You are missing the explanation: why do you need to represent internal
parts of a device with graph. Where does this endpoint point?

Provide some diagram showing the architecture, because either it is
wrong or I do not understand what hardware you want to represent here.

> 
> 
> side-note: I do realize, as I write this, that it has been quite a while
> since you reviewed, and that you may have, rightfully, lost context.
> I apologize for that.
> 
>>
>>> +
>>> +        anyOf:
>>> +          - required:
>>> +              - endpoint
>>> +          - required:
>>> +              - endpoint@0
>>> +              - endpoint@1
>>>  
>>>        port@1:
>>>          $ref: /schemas/graph.yaml#/properties/port
>>> @@ -112,6 +130,23 @@ properties:
>>>        Input memory (from main memory to dispc) bandwidth limit in
>>>        bytes per second
>>>  
>>> +  oldi-txes:
>>> +    type: object
>>> +    additionalProperties: true
>>
>> Why? This looks wrong.
> 
> This, I will admit, was a shot in the dark. The binding check asked me
> that I was missing either this or unevaluatedProperties. I tried to make
> sense of the two, but with little luck. Eventually, I went with this.
> 
> I could change it to unevaluatedProperties if that is indeed correct. I
> could also use some comprehensive resource to understand this, if you
> have something to recommend. =)

This must be additionalProperties false. See example schema or writing
schema.


Best regards,
Krzysztof