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

Krzysztof Kozlowski
2024-08-26T08:15:18Z

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