[v2,1/6] dt-bindings: iommu: Add Translation Buffer Unit bindings

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

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