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
