Re: [PATCH v3 2/5] dt-bindings: display: tegra: nvidia,tegra20-dc: Add parallel RGB output port node

[Maxim Schwalm]

Hi Rob,

On 21.08.23 18:14, Rob Herring wrote:
> On Mon, Aug 07, 2023 at 05:35:12PM +0300, Svyatoslav Ryhel wrote:
>> From: Maxim Schwalm <maxim.schwalm@xxxxxxxxx>
>>
>> Either this node, which is optional, or the nvidia,panel property can be
>> present.
>>
>> Signed-off-by: Maxim Schwalm <maxim.schwalm@xxxxxxxxx>
>> Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
>> ---
>>  .../display/tegra/nvidia,tegra20-dc.yaml      | 31 +++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-dc.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-dc.yaml
>> index 69be95afd562..102304703062 100644
>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-dc.yaml
>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-dc.yaml
>> @@ -127,6 +127,37 @@ allOf:
>>                $ref: /schemas/types.yaml#/definitions/phandle
>>                description: phandle of a display panel
>>  
>> +            port:
>> +              $ref: /schemas/graph.yaml#/$defs/port-base
>> +              description: Parallel RGB output port
>> +
>> +              properties:
>> +                endpoint:
>> +                  $ref: /schemas/media/video-interfaces.yaml#
> 
> Just to make sure, what properties are you using from this? Usually 
> we'll list them though not a hard requirement. If none, then you just 
> need to ref graph.yaml#/properties/port instead and can drop the rest.

currently, just bus-width is used in devicetrees, but I don't think that
it is needed at the moment. So perhaps the property can be dropped.

>> +                  unevaluatedProperties: false
>> +
>> +              unevaluatedProperties: false
> 
> In the indented cases, it's easier to read if this is before 
> properties/patternProperties.
> 
>> +
>> +          anyOf:
>> +            - if:
>> +                not:
>> +                  properties:
>> +                    nvidia,panel: false
>> +              then:
>> +                not:
>> +                  properties:
>> +                    port: true
>> +            - if:
>> +                not:
>> +                  properties:
>> +                    port: false
>> +              then:
>> +                not:
>> +                  properties:
>> +                    nvidia,panel: true
> 
> I would prefer to drop this and mark "nvidia,panel" as deprecated. 
> Eventually I plan to add a mode to the tools to warn on using deprecated 
> properties. Having both could be perfectly fine too. You have the 
> "nvidia,panel" for compatibility with an old OS version and 'port' to 
> work with newer users.

The reason for adding this was that just one of them can be utilized at
the same time. Having both could potentially break the display output.
I think that all the other nvidia,* properties could marked as
deprecated as well because they don't seem to be doing much since commit
d9f980ebcd01 ("drm/tegra: output: rgb: Wrap directly-connected panel
into DRM bridge").

>> +
>> +          additionalProperties: false
> 
> Move this up too.
> 

Best regards,
Maxim