| Message ID | 20240409082324.9928-1-jonathanh@nvidia.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | dt-bindings: host1x: Add missing 'dma-coherent' | expand |
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... > > 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). Best regards, Krzysztof
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. Jon
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
On Thu, Apr 11, 2024 at 5:09 AM Jon Hunter <jonathanh@nvidia.com> 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. 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
On Thu, Apr 11, 2024 at 5:43 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > 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 I think just 'then: true' will work here. Technically, json-schema allows: if: ... else: ... Actually any combination of the 3 keywords present is valid, but we've disallowed the above and other nonsense ones in the meta-schema. We can always relax things if there's a need. Rob
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. > > Fixes: 361238cdc525 ("arm64: tegra: Mark host1x as dma-coherent on Tegra194/234") > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- With mentioning in commit msg that these devices are actually dma-coherent (and after Rob's explanation): Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
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 - if: properties: compatible: @@ -226,6 +235,8 @@ allOf: use. Should be a mapping of IDs 0..n to IOMMU entries corresponding to usable stream IDs. + dma-coherent: true + required: - reg-names
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. 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(+)