Re: [PATCH] dt-bindings: host1x: Add missing 'dma-coherent'

[Krzysztof Kozlowski <krzk@kernel.org>]

On 11/04/2024 12:09, Jon Hunter wrote:
> 
> On 10/04/2024 07:18, Krzysztof Kozlowski wrote:
>> On 09/04/2024 10:23, Jon Hunter wrote:
>>> The dtbs_check reports that the 'dma-coherent' property is "unevaluated
>>> and invalid" for the host1x@13e00000 device on Tegra194 and Tegra234
>>> platforms. Fix this by updating the dt-binding document for host1x to
>>> add the 'dma-coherent' property for these devices.
>>
>> That's not really proper reason. What if DTS is wrong? The reason could
>> be if device is actually DMA coherent...
> 
> In this case the DTS is correct. I guess I should have been more 
> explicit about that.
> 
>>> Fixes: 361238cdc525 ("arm64: tegra: Mark host1x as dma-coherent on Tegra194/234")
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> ---
>>>   .../bindings/display/tegra/nvidia,tegra20-host1x.yaml | 11 +++++++++++
>>>   1 file changed, 11 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
>>> index 94c5242c03b2..3563378a01af 100644
>>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
>>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
>>> @@ -177,6 +177,15 @@ allOf:
>>>   
>>>         required:
>>>           - reg-names
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - nvidia,tegra194-host1x
>>> +    then:
>>> +      properties:
>>> +        dma-coherent: true
>>
>> Do not define properties in allOf. Put it in top-level. If not all
>> devices are DMA coherent, you can disallow it for certain variants (:false).
> 
> 
> So for host1x we currently have the following devices supported ...
> 
> properties:
>    compatible:
>      oneOf:
>        - enum:
>            - nvidia,tegra20-host1x
>            - nvidia,tegra30-host1x
>            - nvidia,tegra114-host1x
>            - nvidia,tegra124-host1x
>            - nvidia,tegra210-host1x
>            - nvidia,tegra186-host1x
>            - nvidia,tegra194-host1x
>            - nvidia,tegra234-host1x
> 
>        - items:
>            - const: nvidia,tegra132-host1x
>            - const: nvidia,tegra124-host1x
> 
> 
> Now only the Tegra194 and Tegra234 are coherent (which are the latest 
> devices). So rather than add this to the top and then filter out all 
> those that are not supported, I opted for the above because there is 
> only 2 devices that need this. Admittedly, as much as I like the yaml 
> bindings, for things like this, it is not really clear which is the best 
> way to handle, so appreciate the guidance.

The way to handle is that you must define properties top-level.

For simplification you could keep the if which duplicates the
dma-coherent:true but add else: which forbids it

if:
 ...
  then:
    properties:
      dma-coherent: true
  else:
    properties:
      dma-coherent: false

Or just ignore the problem. Binding is not 1-to-1 with DTS.

Best regards,
Krzysztof