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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 11, 2024 at 5:09 AM Jon Hunter <jonathanh@xxxxxxxxxx> 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@xxxxxxxxxx>
> >> ---
> >>   .../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.

I would say how you have it is fine, but that would only be for common
boolean properties with no possible constraints. Having that exception
would make the preferences less clear I think.

Rob





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux