| Message ID | 20231118042730.2799-2-quic_c_gdjako@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for Translation Buffer Units | expand |
On Fri, Nov 17, 2023 at 08:27:25PM -0800, Georgi Djakov wrote: > The "apps_smmu" on the Qualcomm sdm845 platform is an implementation > of the SMMU-500, that consists of a single TCU (Translation Control > Unit) and multiple TBUs (Translation Buffer Units). The TCU is already > being described in the generic SMMU DT schema. Add also bindings for nit for if you respin: s/Add also/Add/ or similar :) > the TBUs to describe their properties and resources that needs to be > managed in order to operate them. > > In this DT schema, the TBUs are modelled as child devices of the TCU > and each of them is described with it's register space, clocks, power > domains, interconnects etc. > > Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> > --- > .../devicetree/bindings/iommu/arm,smmu.yaml | 25 ++++++ > .../bindings/iommu/qcom,qsmmuv500-tbu.yaml | 89 +++++++++++++++++++ > 2 files changed, 114 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > index aa9e1c0895a5..f7f89be5f7a3 100644 > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > @@ -231,6 +231,18 @@ properties: > enabled for any given device. > $ref: /schemas/types.yaml#/definitions/phandle > > + '#address-cells': > + enum: [ 1, 2 ] > + > + '#size-cells': > + enum: [ 1, 2 ] > + > + ranges: true > + > +patternProperties: > + "^tbu@[0-9a-f]*": > + type: object > + > required: > - compatible > - reg > @@ -453,6 +465,19 @@ allOf: > - description: Voter clock required for HLOS SMMU access > - description: Interface clock required for register access > > + - if: > + properties: > + compatible: > + contains: > + const: qcom,smmu-500 > + then: > + patternProperties: > + "^tbu@[0-9a-f]*": > + $ref: qcom,qsmmuv500-tbu.yaml > + unevaluatedProperties: false > + properties: > + ranges: true > + > # Disallow clocks for all other platforms with specific compatibles > - if: > properties: > diff --git a/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > new file mode 100644 > index 000000000000..4dc9d0ca33c9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > @@ -0,0 +1,89 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iommu/qcom,qsmmuv500-tbu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm TBU (Translation Buffer Unit) > + > +maintainers: > + - Georgi Djakov <quic_c_gdjako@quicinc.com> > + > +description: > + The Qualcomm SMMU500 implementation consists of TCU and TBU. The TBU contains > + a Translation Lookaside Buffer (TLB) that caches page tables. TBUs provides > + debug features to trace and trigger debug transactions. There are multiple TBU > + instances distributes with each client core. > + > +properties: > + $nodename: > + pattern: "^tbu@[0-9a-f]+$" > + > + compatible: > + const: qcom,qsmmuv500-tbu > + > + reg: > + items: > + - description: Address and size of the TBU's register space. > + > + reg-names: > + items: > + - const: base > + > + clocks: > + maxItems: 1 > + > + interconnects: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + qcom,stream-id-range: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: Stream ID range (address and size) that is assigned by the TBU > + items: > + minItems: 2 > + maxItems: 2 > + > +required: > + - compatible > + - reg > + - interconnects > + - qcom,stream-id-range > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/qcom,gcc-sdm845.h> > + #include <dt-bindings/interconnect/qcom,icc.h> > + #include <dt-bindings/interconnect/qcom,sdm845.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/power/qcom-rpmpd.h> > + > + apps_smmu: iommu@15000000 { > + compatible = "qcom,sdm845-smmu-500", "arm,mmu-500"; > + reg = <0x15000000 0x80000>; > + ranges = <0 0 0 0 0xffffffff>; > + #iommu-cells = <2>; > + #global-interrupts = <1>; > + interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + anoc_1_pcie_tbu: tbu@150e1000 { > + compatible = "qcom,qsmmuv500-tbu"; > + reg = <0x0 0x150e1000 0x0 0x1000>; > + reg-names = "base"; > + clocks = <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>; > + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY > + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; > + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_PCIE_TBU_GDSC>; > + qcom,stream-id-range = <0x1c00 0x400>; > + }; > + }; > + > +... >
On Fri, Nov 17, 2023 at 08:27:25PM -0800, Georgi Djakov wrote: > The "apps_smmu" on the Qualcomm sdm845 platform is an implementation > of the SMMU-500, that consists of a single TCU (Translation Control > Unit) and multiple TBUs (Translation Buffer Units). The TCU is already > being described in the generic SMMU DT schema. Add also bindings for > the TBUs to describe their properties and resources that needs to be > managed in order to operate them. > > In this DT schema, the TBUs are modelled as child devices of the TCU > and each of them is described with it's register space, clocks, power > domains, interconnects etc. > > Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> > --- > .../devicetree/bindings/iommu/arm,smmu.yaml | 25 ++++++ > .../bindings/iommu/qcom,qsmmuv500-tbu.yaml | 89 +++++++++++++++++++ > 2 files changed, 114 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > index aa9e1c0895a5..f7f89be5f7a3 100644 > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > @@ -231,6 +231,18 @@ properties: > enabled for any given device. > $ref: /schemas/types.yaml#/definitions/phandle > > + '#address-cells': > + enum: [ 1, 2 ] > + > + '#size-cells': > + enum: [ 1, 2 ] > + > + ranges: true > + > +patternProperties: > + "^tbu@[0-9a-f]*": > + type: object > + > required: > - compatible > - reg > @@ -453,6 +465,19 @@ allOf: > - description: Voter clock required for HLOS SMMU access > - description: Interface clock required for register access > > + - if: > + properties: > + compatible: > + contains: > + const: qcom,smmu-500 Doesn't match your example. No failure either, so there's some problem with your schema. The issue is the tbu@ entry above has no constraint on child properties. Dropping it would solve the issue. However, having a TBU is not QCom specific, so that doesn't feel right. > + then: > + patternProperties: > + "^tbu@[0-9a-f]*": '+' rather than '*' as 1 is the minimum, not 0. > + $ref: qcom,qsmmuv500-tbu.yaml > + unevaluatedProperties: false > + properties: > + ranges: true > + > # Disallow clocks for all other platforms with specific compatibles > - if: > properties: > diff --git a/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > new file mode 100644 > index 000000000000..4dc9d0ca33c9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > @@ -0,0 +1,89 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iommu/qcom,qsmmuv500-tbu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm TBU (Translation Buffer Unit) > + > +maintainers: > + - Georgi Djakov <quic_c_gdjako@quicinc.com> > + > +description: > + The Qualcomm SMMU500 implementation consists of TCU and TBU. The TBU contains > + a Translation Lookaside Buffer (TLB) that caches page tables. TBUs provides > + debug features to trace and trigger debug transactions. There are multiple TBU > + instances distributes with each client core. > + > +properties: > + $nodename: > + pattern: "^tbu@[0-9a-f]+$" Drop. You defined this in the parent already. > + > + compatible: > + const: qcom,qsmmuv500-tbu > + > + reg: > + items: > + - description: Address and size of the TBU's register space. > + > + reg-names: > + items: > + - const: base Not a useful name. Drop. > + > + clocks: > + maxItems: 1 > + > + interconnects: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + qcom,stream-id-range: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: Stream ID range (address and size) that is assigned by the TBU > + items: > + minItems: 2 > + maxItems: 2 Perhaps implementations other than QCom's needs this? Rob
Hi Rob, Thanks for the feedback! On 11/27/2023 8:13 PM, Rob Herring wrote: > On Fri, Nov 17, 2023 at 08:27:25PM -0800, Georgi Djakov wrote: >> The "apps_smmu" on the Qualcomm sdm845 platform is an implementation >> of the SMMU-500, that consists of a single TCU (Translation Control >> Unit) and multiple TBUs (Translation Buffer Units). The TCU is already >> being described in the generic SMMU DT schema. Add also bindings for >> the TBUs to describe their properties and resources that needs to be >> managed in order to operate them. >> >> In this DT schema, the TBUs are modelled as child devices of the TCU >> and each of them is described with it's register space, clocks, power >> domains, interconnects etc. >> >> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> >> --- >> .../devicetree/bindings/iommu/arm,smmu.yaml | 25 ++++++ >> .../bindings/iommu/qcom,qsmmuv500-tbu.yaml | 89 +++++++++++++++++++ >> 2 files changed, 114 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml >> >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> index aa9e1c0895a5..f7f89be5f7a3 100644 >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> @@ -231,6 +231,18 @@ properties: >> enabled for any given device. >> $ref: /schemas/types.yaml#/definitions/phandle >> >> + '#address-cells': >> + enum: [ 1, 2 ] >> + >> + '#size-cells': >> + enum: [ 1, 2 ] >> + >> + ranges: true >> + >> +patternProperties: >> + "^tbu@[0-9a-f]*": >> + type: object >> + >> required: >> - compatible >> - reg >> @@ -453,6 +465,19 @@ allOf: >> - description: Voter clock required for HLOS SMMU access >> - description: Interface clock required for register access >> >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: qcom,smmu-500 > > Doesn't match your example. No failure either, so there's some problem > with your schema. The issue is the tbu@ entry above has no constraint on > child properties. Dropping it would solve the issue. However, having a > TBU is not QCom specific, so that doesn't feel right. Having a TBU is not Qcom specific. The ARM MMU-500 implementation for example has TBUs, but the registers are within the SMMU address space, there are no clocks, no power-domains or other resources. Not sure about other implementations. So should we just allow empty tbu child nodes with no properties? >> + then: >> + patternProperties: >> + "^tbu@[0-9a-f]*": > > '+' rather than '*' as 1 is the minimum, not 0. Ok. >> + $ref: qcom,qsmmuv500-tbu.yaml >> + unevaluatedProperties: false >> + properties: >> + ranges: true >> + >> # Disallow clocks for all other platforms with specific compatibles >> - if: >> properties: >> diff --git a/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml >> new file mode 100644 >> index 000000000000..4dc9d0ca33c9 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml >> @@ -0,0 +1,89 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/iommu/qcom,qsmmuv500-tbu.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm TBU (Translation Buffer Unit) >> + >> +maintainers: >> + - Georgi Djakov <quic_c_gdjako@quicinc.com> >> + >> +description: >> + The Qualcomm SMMU500 implementation consists of TCU and TBU. The TBU contains >> + a Translation Lookaside Buffer (TLB) that caches page tables. TBUs provides >> + debug features to trace and trigger debug transactions. There are multiple TBU >> + instances distributes with each client core. >> + >> +properties: >> + $nodename: >> + pattern: "^tbu@[0-9a-f]+$" > > Drop. You defined this in the parent already. Ok. >> + >> + compatible: >> + const: qcom,qsmmuv500-tbu >> + >> + reg: >> + items: >> + - description: Address and size of the TBU's register space. >> + >> + reg-names: >> + items: >> + - const: base > > Not a useful name. Drop. Agree. >> + >> + clocks: >> + maxItems: 1 >> + >> + interconnects: >> + maxItems: 1 >> + >> + power-domains: >> + maxItems: 1 >> + >> + qcom,stream-id-range: >> + $ref: /schemas/types.yaml#/definitions/uint32-array >> + description: Stream ID range (address and size) that is assigned by the TBU >> + items: >> + minItems: 2 >> + maxItems: 2 > > Perhaps implementations other than QCom's needs this? Yes, maybe. A TBU can service a fixed amount of stream IDs and this looks like something common for all TBUs. I'll drop the vendor prefix. Thanks, Georgi
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index aa9e1c0895a5..f7f89be5f7a3 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -231,6 +231,18 @@ properties: enabled for any given device. $ref: /schemas/types.yaml#/definitions/phandle + '#address-cells': + enum: [ 1, 2 ] + + '#size-cells': + enum: [ 1, 2 ] + + ranges: true + +patternProperties: + "^tbu@[0-9a-f]*": + type: object + required: - compatible - reg @@ -453,6 +465,19 @@ allOf: - description: Voter clock required for HLOS SMMU access - description: Interface clock required for register access + - if: + properties: + compatible: + contains: + const: qcom,smmu-500 + then: + patternProperties: + "^tbu@[0-9a-f]*": + $ref: qcom,qsmmuv500-tbu.yaml + unevaluatedProperties: false + properties: + ranges: true + # Disallow clocks for all other platforms with specific compatibles - if: properties: diff --git a/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml new file mode 100644 index 000000000000..4dc9d0ca33c9 --- /dev/null +++ b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml @@ -0,0 +1,89 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iommu/qcom,qsmmuv500-tbu.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm TBU (Translation Buffer Unit) + +maintainers: + - Georgi Djakov <quic_c_gdjako@quicinc.com> + +description: + The Qualcomm SMMU500 implementation consists of TCU and TBU. The TBU contains + a Translation Lookaside Buffer (TLB) that caches page tables. TBUs provides + debug features to trace and trigger debug transactions. There are multiple TBU + instances distributes with each client core. + +properties: + $nodename: + pattern: "^tbu@[0-9a-f]+$" + + compatible: + const: qcom,qsmmuv500-tbu + + reg: + items: + - description: Address and size of the TBU's register space. + + reg-names: + items: + - const: base + + clocks: + maxItems: 1 + + interconnects: + maxItems: 1 + + power-domains: + maxItems: 1 + + qcom,stream-id-range: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: Stream ID range (address and size) that is assigned by the TBU + items: + minItems: 2 + maxItems: 2 + +required: + - compatible + - reg + - interconnects + - qcom,stream-id-range + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/qcom,gcc-sdm845.h> + #include <dt-bindings/interconnect/qcom,icc.h> + #include <dt-bindings/interconnect/qcom,sdm845.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/power/qcom-rpmpd.h> + + apps_smmu: iommu@15000000 { + compatible = "qcom,sdm845-smmu-500", "arm,mmu-500"; + reg = <0x15000000 0x80000>; + ranges = <0 0 0 0 0xffffffff>; + #iommu-cells = <2>; + #global-interrupts = <1>; + interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <2>; + #size-cells = <2>; + + anoc_1_pcie_tbu: tbu@150e1000 { + compatible = "qcom,qsmmuv500-tbu"; + reg = <0x0 0x150e1000 0x0 0x1000>; + reg-names = "base"; + clocks = <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>; + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_PCIE_TBU_GDSC>; + qcom,stream-id-range = <0x1c00 0x400>; + }; + }; + +...
The "apps_smmu" on the Qualcomm sdm845 platform is an implementation of the SMMU-500, that consists of a single TCU (Translation Control Unit) and multiple TBUs (Translation Buffer Units). The TCU is already being described in the generic SMMU DT schema. Add also bindings for the TBUs to describe their properties and resources that needs to be managed in order to operate them. In this DT schema, the TBUs are modelled as child devices of the TCU and each of them is described with it's register space, clocks, power domains, interconnects etc. Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> --- .../devicetree/bindings/iommu/arm,smmu.yaml | 25 ++++++ .../bindings/iommu/qcom,qsmmuv500-tbu.yaml | 89 +++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml