| Message ID | 20210603164632.1000458-3-thierry.reding@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: tegra: Prevent early SMMU faults | expand |
On Thu, Jun 3, 2021 at 10:49 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > From: Thierry Reding <treding@nvidia.com> > > The ARM SMMU instantiations found on Tegra186 and later need inter- > operation with the memory controller in order to correctly program > stream ID overrides. > > Furthermore, on Tegra194 multiple instances of the SMMU can gang up > to achieve higher throughput. In order to do this, they have to be > programmed identically so that the memory controller can interleave > memory accesses between them. > > Add the Tegra186 compatible string to make sure the interoperation > with the memory controller can be enabled on that SoC generation. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > index 9d27aa5111d4..1181b590db71 100644 > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > @@ -54,8 +54,14 @@ properties: > - const: arm,mmu-500 > - description: NVIDIA SoCs that program two ARM MMU-500s identically > items: > + - description: NVIDIA SoCs that require memory controller interaction This is not valid jsonschema: /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: properties:compatible:oneOf:4:items: 'anyOf' conditional failed, one must be fixed: None is not of type 'object', 'boolean' None is not of type 'array' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one must be fixed: None is not of type 'object' None is not of type 'array' from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one must be fixed: None is not of type 'object' None is not of type 'array' from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: properties:compatible:oneOf:5:items: 'oneOf' conditional failed, one must be fixed: [{'enum': [{'const': 'nvidia,tegra194-smmu'}, {'const': 'nvidia,tegra186-smmu'}]}, {'const': 'nvidia,smmu-500'}] is not of type 'object' {'const': 'nvidia,tegra194-smmu'} is not of type 'string' {'const': 'nvidia,tegra186-smmu'} is not of type 'string' from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# This was not reviewed nor tested since the DT list was not Cc'ed. Rob
On 18/06/2021 21:47, Rob Herring wrote: > On Thu, Jun 3, 2021 at 10:49 AM Thierry Reding <thierry.reding@gmail.com> wrote: >> >> From: Thierry Reding <treding@nvidia.com> >> >> The ARM SMMU instantiations found on Tegra186 and later need inter- >> operation with the memory controller in order to correctly program >> stream ID overrides. >> >> Furthermore, on Tegra194 multiple instances of the SMMU can gang up >> to achieve higher throughput. In order to do this, they have to be >> programmed identically so that the memory controller can interleave >> memory accesses between them. >> >> Add the Tegra186 compatible string to make sure the interoperation >> with the memory controller can be enabled on that SoC generation. >> >> Signed-off-by: Thierry Reding <treding@nvidia.com> >> --- >> Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> index 9d27aa5111d4..1181b590db71 100644 >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> @@ -54,8 +54,14 @@ properties: >> - const: arm,mmu-500 >> - description: NVIDIA SoCs that program two ARM MMU-500s identically >> items: >> + - description: NVIDIA SoCs that require memory controller interaction > > This is not valid jsonschema: > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > properties:compatible:oneOf:4:items: 'anyOf' conditional failed, one > must be fixed: > None is not of type 'object', 'boolean' > None is not of type 'array' > from schema $id: http://json-schema.org/draft-07/schema# > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one > must be fixed: > None is not of type 'object' > None is not of type 'array' > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one > must be fixed: > None is not of type 'object' > None is not of type 'array' > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > properties:compatible:oneOf:5:items: 'oneOf' conditional failed, one > must be fixed: > [{'enum': [{'const': 'nvidia,tegra194-smmu'}, {'const': > 'nvidia,tegra186-smmu'}]}, {'const': 'nvidia,smmu-500'}] is not of > type 'object' > {'const': 'nvidia,tegra194-smmu'} is not of type 'string' > {'const': 'nvidia,tegra186-smmu'} is not of type 'string' > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# > > > This was not reviewed nor tested since the DT list was not Cc'ed. Ugh, I see now weird empty item on a list... and not only DT list was skipped - Thierry did not Cc you either. My bad, I did not check that patch thoroughly before applying. Thierry, please Cc folks mentioned by get_maintainer.pl. Either sent a fix or a revert, if fix needs more time. Additionally, why the patch changes reg to "minItems: 1" for nvidia,tegra194-smmu? Best regards, Krzysztof
On Mon, Jun 21, 2021 at 08:46:54AM +0200, Krzysztof Kozlowski wrote: > On 18/06/2021 21:47, Rob Herring wrote: > > On Thu, Jun 3, 2021 at 10:49 AM Thierry Reding <thierry.reding@gmail.com> wrote: > >> > >> From: Thierry Reding <treding@nvidia.com> > >> > >> The ARM SMMU instantiations found on Tegra186 and later need inter- > >> operation with the memory controller in order to correctly program > >> stream ID overrides. > >> > >> Furthermore, on Tegra194 multiple instances of the SMMU can gang up > >> to achieve higher throughput. In order to do this, they have to be > >> programmed identically so that the memory controller can interleave > >> memory accesses between them. > >> > >> Add the Tegra186 compatible string to make sure the interoperation > >> with the memory controller can be enabled on that SoC generation. > >> > >> Signed-off-by: Thierry Reding <treding@nvidia.com> > >> --- > >> Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +++++++++-- > >> 1 file changed, 9 insertions(+), 2 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > >> index 9d27aa5111d4..1181b590db71 100644 > >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > >> @@ -54,8 +54,14 @@ properties: > >> - const: arm,mmu-500 > >> - description: NVIDIA SoCs that program two ARM MMU-500s identically > >> items: > >> + - description: NVIDIA SoCs that require memory controller interaction > > > > This is not valid jsonschema: > > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > properties:compatible:oneOf:4:items: 'anyOf' conditional failed, one > > must be fixed: > > None is not of type 'object', 'boolean' > > None is not of type 'array' > > from schema $id: http://json-schema.org/draft-07/schema# > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one > > must be fixed: > > None is not of type 'object' > > None is not of type 'array' > > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one > > must be fixed: > > None is not of type 'object' > > None is not of type 'array' > > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > properties:compatible:oneOf:5:items: 'oneOf' conditional failed, one > > must be fixed: > > [{'enum': [{'const': 'nvidia,tegra194-smmu'}, {'const': > > 'nvidia,tegra186-smmu'}]}, {'const': 'nvidia,smmu-500'}] is not of > > type 'object' > > {'const': 'nvidia,tegra194-smmu'} is not of type 'string' > > {'const': 'nvidia,tegra186-smmu'} is not of type 'string' > > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# > > > > > > This was not reviewed nor tested since the DT list was not Cc'ed. > > Ugh, I see now weird empty item on a list... and not only DT list was > skipped - Thierry did not Cc you either. This seemed like a too trivial addition to waste Rob's time on, so I didn't add him (or the DT list for that matter) on Cc. The ARM SMMU maintainers had reviewed this, which seemed like it was enough for what the DT bindings change was doing. In any case, I clearly should've checked the DT binding check output more carefully. It's rather messy for Tegra because there's quite a few that we haven't converted yet. I'll have to resume my effort to convert the remaining ones and fixup the device trees so that we can actually run the DT binding and DTB validation checks more usefully. > My bad, I did not check that patch thoroughly before applying. > > Thierry, please Cc folks mentioned by get_maintainer.pl. Either sent a > fix or a revert, if fix needs more time. I've sent out a follow-up fix that removes the two bogus lines. It looks like that was the result of a bad conflict resolution on my part. > Additionally, why the patch changes reg to "minItems: 1" for > nvidia,tegra194-smmu? This is because originally the Tegra194 SMMU nodes were supposed to only represent a "dual" instance. However, on Tegra194 there are three SMMU instances in total, with the third instance (dedicated for isochronous traffic) being completely separate and having only a single range of registers. That third instance was previously supposed to be covered by the normal "arm,mmu-500" compatible string, but given that we really need that interoperation between SMMU and memory controller for SID override programming, we need the Tegra-specific compatible for the ISO instance of the SMMU as well. And since that uses only one set of registers, minItems had to become 1. Thierry
On Mon, Jun 21, 2021 at 04:11:55PM +0200, Thierry Reding wrote: > On Mon, Jun 21, 2021 at 08:46:54AM +0200, Krzysztof Kozlowski wrote: > > On 18/06/2021 21:47, Rob Herring wrote: > > > On Thu, Jun 3, 2021 at 10:49 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > >> index 9d27aa5111d4..1181b590db71 100644 > > >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > >> @@ -54,8 +54,14 @@ properties: > > >> - const: arm,mmu-500 > > >> - description: NVIDIA SoCs that program two ARM MMU-500s identically > > >> items: > > >> + - description: NVIDIA SoCs that require memory controller interaction > > > > > > This is not valid jsonschema: > > > > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > > properties:compatible:oneOf:4:items: 'anyOf' conditional failed, one > > > must be fixed: > > > None is not of type 'object', 'boolean' > > > None is not of type 'array' > > > from schema $id: http://json-schema.org/draft-07/schema# > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one > > > must be fixed: > > > None is not of type 'object' > > > None is not of type 'array' > > > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one > > > must be fixed: > > > None is not of type 'object' > > > None is not of type 'array' > > > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > > properties:compatible:oneOf:5:items: 'oneOf' conditional failed, one > > > must be fixed: > > > [{'enum': [{'const': 'nvidia,tegra194-smmu'}, {'const': > > > 'nvidia,tegra186-smmu'}]}, {'const': 'nvidia,smmu-500'}] is not of > > > type 'object' > > > {'const': 'nvidia,tegra194-smmu'} is not of type 'string' > > > {'const': 'nvidia,tegra186-smmu'} is not of type 'string' > > > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# > > > > > > > > > This was not reviewed nor tested since the DT list was not Cc'ed. > > > > Ugh, I see now weird empty item on a list... and not only DT list was > > skipped - Thierry did not Cc you either. > > This seemed like a too trivial addition to waste Rob's time on, so I > didn't add him (or the DT list for that matter) on Cc. The ARM SMMU > maintainers had reviewed this, which seemed like it was enough for what > the DT bindings change was doing. Hmm, I didn't review it. I find the yaml stuff unreadable so I usually wait for the DT folks to ack bindings changes before I queue them in the SMMU tree. Will
On Mon, Jun 21, 2021 at 04:54:18PM +0100, Will Deacon wrote: > On Mon, Jun 21, 2021 at 04:11:55PM +0200, Thierry Reding wrote: > > On Mon, Jun 21, 2021 at 08:46:54AM +0200, Krzysztof Kozlowski wrote: > > > On 18/06/2021 21:47, Rob Herring wrote: > > > > On Thu, Jun 3, 2021 at 10:49 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > > >> index 9d27aa5111d4..1181b590db71 100644 > > > >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > > >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > > >> @@ -54,8 +54,14 @@ properties: > > > >> - const: arm,mmu-500 > > > >> - description: NVIDIA SoCs that program two ARM MMU-500s identically > > > >> items: > > > >> + - description: NVIDIA SoCs that require memory controller interaction > > > > > > > > This is not valid jsonschema: > > > > > > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > > > properties:compatible:oneOf:4:items: 'anyOf' conditional failed, one > > > > must be fixed: > > > > None is not of type 'object', 'boolean' > > > > None is not of type 'array' > > > > from schema $id: http://json-schema.org/draft-07/schema# > > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > > > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one > > > > must be fixed: > > > > None is not of type 'object' > > > > None is not of type 'array' > > > > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# > > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > > > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one > > > > must be fixed: > > > > None is not of type 'object' > > > > None is not of type 'array' > > > > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# > > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: > > > > properties:compatible:oneOf:5:items: 'oneOf' conditional failed, one > > > > must be fixed: > > > > [{'enum': [{'const': 'nvidia,tegra194-smmu'}, {'const': > > > > 'nvidia,tegra186-smmu'}]}, {'const': 'nvidia,smmu-500'}] is not of > > > > type 'object' > > > > {'const': 'nvidia,tegra194-smmu'} is not of type 'string' > > > > {'const': 'nvidia,tegra186-smmu'} is not of type 'string' > > > > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# > > > > > > > > > > > > This was not reviewed nor tested since the DT list was not Cc'ed. > > > > > > Ugh, I see now weird empty item on a list... and not only DT list was > > > skipped - Thierry did not Cc you either. > > > > This seemed like a too trivial addition to waste Rob's time on, so I > > didn't add him (or the DT list for that matter) on Cc. The ARM SMMU > > maintainers had reviewed this, which seemed like it was enough for what > > the DT bindings change was doing. > > Hmm, I didn't review it. I find the yaml stuff unreadable so I usually > wait for the DT folks to ack bindings changes before I queue them in the > SMMU tree. Alright... in the future I'll make sure to always Cc DT folks, even for trivial stuff like this. Thierry
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index 9d27aa5111d4..1181b590db71 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -54,8 +54,14 @@ properties: - const: arm,mmu-500 - description: NVIDIA SoCs that program two ARM MMU-500s identically items: + - description: NVIDIA SoCs that require memory controller interaction + and may program multiple ARM MMU-500s identically with the memory + controller interleaving translations between multiple instances + for improved performance. + items: - enum: - - nvidia,tegra194-smmu + - const: nvidia,tegra194-smmu + - const: nvidia,tegra186-smmu - const: nvidia,smmu-500 - items: - const: arm,mmu-500 @@ -165,10 +171,11 @@ allOf: contains: enum: - nvidia,tegra194-smmu + - nvidia,tegra186-smmu then: properties: reg: - minItems: 2 + minItems: 1 maxItems: 2 else: properties: