| Message ID | 20240226172218.69486-2-quic_c_gdjako@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for Translation Buffer Units | expand |
On 26/02/2024 18:22, 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). These TBUs have > hardware debugging features that are specific and only present on > Qualcomm hardware. Represent them as independent DT nodes. List all > the resources that are needed to operate them (such as registers, > clocks, power domains and interconnects). > > Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> > --- > .../devicetree/bindings/iommu/qcom,tbu.yaml | 65 +++++++++++++++++++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iommu/qcom,tbu.yaml > > diff --git a/Documentation/devicetree/bindings/iommu/qcom,tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,tbu.yaml > new file mode 100644 > index 000000000000..6841ca9af21f > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/qcom,tbu.yaml > @@ -0,0 +1,65 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iommu/qcom,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 with each client core. > + > +properties: > + compatible: > + const: qcom,qsmmuv500-tbu Why we don't have SoC specific compatibles? If that's for SDM845, then it should be qcom,sdm845-tbu or qcom,sdm845-qsmmuv500-tbu > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + interconnects: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + qcom,stream-id-range: > + description: Phandle of a SMMU device and Stream ID range (address and size) that is assigned by the TBU Please wrap it according to coding style, so 80. > + $ref: /schemas/types.yaml#/definitions/phandle-array > + items: > + - items: > + - description: phandle of a smmu node > + - description: stream id base address > + - description: stream id size > + > +required: > + - compatible > + - reg > + - qcom,stream-id-range > + > +unevaluatedProperties: false This should be additionalProperties: false > + > +examples: Best regards, Krzysztof
On 26/02/2024 18:22, 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). These TBUs have > hardware debugging features that are specific and only present on > Qualcomm hardware. Represent them as independent DT nodes. List all > the resources that are needed to operate them (such as registers, > clocks, power domains and interconnects). > > Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> > --- > .../devicetree/bindings/iommu/qcom,tbu.yaml | 65 +++++++++++++++++++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iommu/qcom,tbu.yaml Heh, I wonder how did you solve Robin's comments. I don't see you responding to Robin. Just v5 sent... Best regards, Krzysztof
On 26/02/2024 18:22, 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). These TBUs have > hardware debugging features that are specific and only present on > Qualcomm hardware. Represent them as independent DT nodes. List all > the resources that are needed to operate them (such as registers, > clocks, power domains and interconnects). > > Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> Also one more nit, since I expect new version: A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 Best regards, Krzysztof
Hi Krzysztof, On 29.02.24 19:53, Krzysztof Kozlowski wrote: > On 26/02/2024 18:22, 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). These TBUs have >> hardware debugging features that are specific and only present on >> Qualcomm hardware. Represent them as independent DT nodes. List all >> the resources that are needed to operate them (such as registers, >> clocks, power domains and interconnects). >> >> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> >> --- >> .../devicetree/bindings/iommu/qcom,tbu.yaml | 65 +++++++++++++++++++ >> 1 file changed, 65 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iommu/qcom,tbu.yaml >> >> diff --git a/Documentation/devicetree/bindings/iommu/qcom,tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,tbu.yaml >> new file mode 100644 >> index 000000000000..6841ca9af21f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iommu/qcom,tbu.yaml >> @@ -0,0 +1,65 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/iommu/qcom,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 with each client core. >> + >> +properties: >> + compatible: >> + const: qcom,qsmmuv500-tbu > > Why we don't have SoC specific compatibles? If that's for SDM845, then > it should be qcom,sdm845-tbu or qcom,sdm845-qsmmuv500-tbu > Because they should be all compatible (as registers). Adding a SoC compatible might get overly-specific, but i can also see the benefits in that, so ok will do it! > >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + interconnects: >> + maxItems: 1 >> + >> + power-domains: >> + maxItems: 1 >> + >> + qcom,stream-id-range: >> + description: Phandle of a SMMU device and Stream ID range (address and size) that is assigned by the TBU > > Please wrap it according to coding style, so 80. > Sure, thanks! >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + items: >> + - items: >> + - description: phandle of a smmu node >> + - description: stream id base address >> + - description: stream id size >> + >> +required: >> + - compatible >> + - reg >> + - qcom,stream-id-range >> + >> +unevaluatedProperties: false > > This should be additionalProperties: false > Ok right, thanks for taking a look! BR, Georgi
Hi Krzysztof, On 29.02.24 19:59, Krzysztof Kozlowski wrote: > On 26/02/2024 18:22, 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). These TBUs have >> hardware debugging features that are specific and only present on >> Qualcomm hardware. Represent them as independent DT nodes. List all >> the resources that are needed to operate them (such as registers, >> clocks, power domains and interconnects). >> >> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> >> --- >> .../devicetree/bindings/iommu/qcom,tbu.yaml | 65 +++++++++++++++++++ >> 1 file changed, 65 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iommu/qcom,tbu.yaml > > Heh, I wonder how did you solve Robin's comments. I don't see you > responding to Robin. Just v5 sent... Yeah, i didn't respond because his response was clear to me. He responded to the fundamental question whether to model the TBUs as child DT nodes of the SMMU or as standalone nodes. So in the first versions of this patchset we tried to explore the path with "child" nodes and search if there are any other implementation than the Qualcomm one and try to find some common binding... and Robin's objection was exactly to that. It seems that adding more functionalities in TBUs (which requires resource management) is only a Qualcomm thing and common binding does not make sense, so now we are going with standalone DT nodes as he suggested. Thanks, Georgi
On Thu, Feb 29, 2024 at 10:09:34PM +0200, Georgi Djakov wrote: > Hi Krzysztof, > > On 29.02.24 19:53, Krzysztof Kozlowski wrote: > >On 26/02/2024 18:22, Georgi Djakov wrote: > >>+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>+%YAML 1.2 > >>+--- > >>+$id: http://devicetree.org/schemas/iommu/qcom,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 with each client core. > >>+ > >>+properties: > >>+ compatible: > >>+ const: qcom,qsmmuv500-tbu > > > >Why we don't have SoC specific compatibles? If that's for SDM845, then > >it should be qcom,sdm845-tbu or qcom,sdm845-qsmmuv500-tbu > > > > Because they should be all compatible (as registers). Adding a SoC compatible > might get overly-specific, but i can also see the benefits in that, so ok will > do it! > Hi Krzysztof, JFYI that the TBUs are used on our mobile SoCs going up until the SoC we commercialized in early 2022, Snapdragon 8 Gen 1. Including SDM845 there are three more premium tier SoCs using TBUs plus all of their value-tier derivatives. There will also be prior generation premium tier SoCs along with their derivatives that use the TBU as well. Does it make sense to have a target-specific compatible string given this? Thanks, Chris.
On 29/02/2024 23:24, Chris Goldsworthy wrote: > On Thu, Feb 29, 2024 at 10:09:34PM +0200, Georgi Djakov wrote: >> Hi Krzysztof, >> >> On 29.02.24 19:53, Krzysztof Kozlowski wrote: >>> On 26/02/2024 18:22, Georgi Djakov wrote: >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/iommu/qcom,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 with each client core. >>>> + >>>> +properties: >>>> + compatible: >>>> + const: qcom,qsmmuv500-tbu >>> >>> Why we don't have SoC specific compatibles? If that's for SDM845, then >>> it should be qcom,sdm845-tbu or qcom,sdm845-qsmmuv500-tbu >>> >> >> Because they should be all compatible (as registers). Adding a SoC compatible >> might get overly-specific, but i can also see the benefits in that, so ok will >> do it! >> > > Hi Krzysztof, > > JFYI that the TBUs are used on our mobile SoCs going up until the SoC > we commercialized in early 2022, Snapdragon 8 Gen 1. Including SDM845 > there are three more premium tier SoCs using TBUs plus all of their > value-tier derivatives. There will also be prior generation premium > tier SoCs along with their derivatives that use the TBU as well. Does > it make sense to have a target-specific compatible string given this? This does not explain me anything. Why an exemption from usual bindings rules should apply here? https://elixir.bootlin.com/linux/v6.8-rc6/source/Documentation/devicetree/bindings/writing-bindings.rst Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/iommu/qcom,tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,tbu.yaml new file mode 100644 index 000000000000..6841ca9af21f --- /dev/null +++ b/Documentation/devicetree/bindings/iommu/qcom,tbu.yaml @@ -0,0 +1,65 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iommu/qcom,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 with each client core. + +properties: + compatible: + const: qcom,qsmmuv500-tbu + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + interconnects: + maxItems: 1 + + power-domains: + maxItems: 1 + + qcom,stream-id-range: + description: Phandle of a SMMU device and Stream ID range (address and size) that is assigned by the TBU + $ref: /schemas/types.yaml#/definitions/phandle-array + items: + - items: + - description: phandle of a smmu node + - description: stream id base address + - description: stream id size + +required: + - compatible + - reg + - qcom,stream-id-range + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/qcom,gcc-sdm845.h> + #include <dt-bindings/interconnect/qcom,icc.h> + #include <dt-bindings/interconnect/qcom,sdm845.h> + + tbu@150e1000 { + compatible = "qcom,qsmmuv500-tbu"; + reg = <0x150e1000 0x1000>; + 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 = <&apps_smmu 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). These TBUs have hardware debugging features that are specific and only present on Qualcomm hardware. Represent them as independent DT nodes. List all the resources that are needed to operate them (such as registers, clocks, power domains and interconnects). Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> --- .../devicetree/bindings/iommu/qcom,tbu.yaml | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 Documentation/devicetree/bindings/iommu/qcom,tbu.yaml