diff mbox series

[v4,01/10] dt-bindings: iommu: Add Translation Buffer Unit bindings

Message ID 20240201210529.7728-2-quic_c_gdjako@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add support for Translation Buffer Units | expand

Commit Message

Georgi Djakov Feb. 1, 2024, 9:05 p.m. UTC 
Add common bindings for the TBUs to describe their properties. The
TBUs are modelled as child devices of the IOMMU and each of them is
described with their compatible, reg and stream-id-range properties.
There could be other implementation specific properties to describe
any resources like clocks, regulators, power-domains, interconnects
that would be needed for TBU operation. Such properties will be
documented in a separate vendor-specific TBU schema.

Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com>
---
 .../devicetree/bindings/iommu/arm,smmu.yaml   | 14 ++++++++++
 .../devicetree/bindings/iommu/tbu-common.yaml | 28 +++++++++++++++++++
 2 files changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/tbu-common.yaml

Comments

Rob Herring (Arm) Feb. 2, 2024, 9:17 p.m. UTC | #1 
On Thu, Feb 01, 2024 at 01:05:20PM -0800, Georgi Djakov wrote:
> Add common bindings for the TBUs to describe their properties. The
> TBUs are modelled as child devices of the IOMMU and each of them is
> described with their compatible, reg and stream-id-range properties.
> There could be other implementation specific properties to describe
> any resources like clocks, regulators, power-domains, interconnects
> that would be needed for TBU operation. Such properties will be
> documented in a separate vendor-specific TBU schema.
> 
> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com>
> ---
>  .../devicetree/bindings/iommu/arm,smmu.yaml   | 14 ++++++++++
>  .../devicetree/bindings/iommu/tbu-common.yaml | 28 +++++++++++++++++++
>  2 files changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iommu/tbu-common.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> index a4042ae24770..ba3237023b39 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> @@ -235,6 +235,20 @@ 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]+$":
> +    description: TBU child nodes
> +    type: object
> +    $ref: tbu-common.yaml#

       additionalProperties: false


However, that's going to break with the extra QCom properties. In 
json-schema, you can't have 2 schemas and extend the properties of 
their child nodes. The validator doesn't "see" the child node schemas at 
the same time. You are going to have to move QCom SMMU to its own schema 
and remove it from arm,smmu.yaml.

Rob
Robin Murphy Feb. 12, 2024, 7:12 p.m. UTC | #2 
On 2024-02-02 9:17 pm, Rob Herring wrote:
> On Thu, Feb 01, 2024 at 01:05:20PM -0800, Georgi Djakov wrote:
>> Add common bindings for the TBUs to describe their properties. The
>> TBUs are modelled as child devices of the IOMMU and each of them is
>> described with their compatible, reg and stream-id-range properties.
>> There could be other implementation specific properties to describe
>> any resources like clocks, regulators, power-domains, interconnects
>> that would be needed for TBU operation. Such properties will be
>> documented in a separate vendor-specific TBU schema.
>>
>> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com>
>> ---
>>   .../devicetree/bindings/iommu/arm,smmu.yaml   | 14 ++++++++++
>>   .../devicetree/bindings/iommu/tbu-common.yaml | 28 +++++++++++++++++++
>>   2 files changed, 42 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/iommu/tbu-common.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> index a4042ae24770..ba3237023b39 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> @@ -235,6 +235,20 @@ 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]+$":
>> +    description: TBU child nodes
>> +    type: object
>> +    $ref: tbu-common.yaml#
> 
>         additionalProperties: false
> 
> 
> However, that's going to break with the extra QCom properties. In
> json-schema, you can't have 2 schemas and extend the properties of
> their child nodes. The validator doesn't "see" the child node schemas at
> the same time. You are going to have to move QCom SMMU to its own schema
> and remove it from arm,smmu.yaml.

Although this common binding is pretty pointless - sorry I missed the 
previous discussion, but these TBU registers are on obscure debugging 
feature of Qualcomm's own invention and definitely not generic. The 
internal topology of the unmodified Arm MMU-500 implementation isn't 
software-visible at all without getting into its own integration and 
debug registers (and maybe to a lesser extent the PMU), and even then 
everything is proxied through the TCU via an internal AXI stream 
interconnect, so there aren't really any TBU-owned resources which would 
warrant describing as such in DT. If anything, the way this binding is 
defined as an MMIO bus with "ranges" would actively *prevent* being able 
to describe the standard hardware this way, since the internal debug 
stuff all wants to refer to TBUs by numerical index.

Conversely, given that the Qualcomm TBU registers seem to be describing 
their own entirely independent resources and inheriting nothing from the 
parent node, I'm not sure it's necessarily worth all the bother of 
describing and supporting them them as children at all, when they could 
just as well be standalone nodes with a phandle to associate an SMMU 
instance.

Thanks,
Robin.
Robin Murphy Feb. 12, 2024, 8:25 p.m. UTC | #3 
On 2024-02-01 9:05 pm, Georgi Djakov wrote:
> Add common bindings for the TBUs to describe their properties. The
> TBUs are modelled as child devices of the IOMMU and each of them is
> described with their compatible, reg and stream-id-range properties.
> There could be other implementation specific properties to describe
> any resources like clocks, regulators, power-domains, interconnects
> that would be needed for TBU operation. Such properties will be
> documented in a separate vendor-specific TBU schema.
> 
> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com>
> ---
>   .../devicetree/bindings/iommu/arm,smmu.yaml   | 14 ++++++++++
>   .../devicetree/bindings/iommu/tbu-common.yaml | 28 +++++++++++++++++++
>   2 files changed, 42 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/iommu/tbu-common.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> index a4042ae24770..ba3237023b39 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> @@ -235,6 +235,20 @@ 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]+$":
> +    description: TBU child nodes
> +    type: object
> +    $ref: tbu-common.yaml#
> +
>   required:
>     - compatible
>     - reg
> diff --git a/Documentation/devicetree/bindings/iommu/tbu-common.yaml b/Documentation/devicetree/bindings/iommu/tbu-common.yaml
> new file mode 100644
> index 000000000000..3e95b356e572
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/tbu-common.yaml
> @@ -0,0 +1,28 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iommu/tbu-common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Translation Buffer Unit (TBU) common properties
> +
> +maintainers:
> +  - Georgi Djakov <quic_c_gdjako@quicinc.com>
> +
> +description:
> +  The SMMU implements a TBU for system masters. It consists if a
> +  Translation Lookaside Buffer (TLB) that caches page tables.
> +
> +properties:
> +  reg:
> +    maxItems: 1
> +
> +  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

Actually, even this doesn't work - for the 15-bit StreamID config, 
there's no guarantee that the devices behind each TBU will use a single 
contiguous StreamID range. Conversely, for any other config the 
StreamIDs are already uniquely associated with a TBU by their top 5 
bits, so the "size" doesn't matter.

Thanks,
Robin.

> +
> +additionalProperties: true
> +...
Krzysztof Kozlowski Feb. 29, 2024, 6:01 p.m. UTC | #4 
On 12/02/2024 20:12, Robin Murphy wrote:
>>> +  '#address-cells':
>>> +    enum: [ 1, 2 ]
>>> +
>>> +  '#size-cells':
>>> +    enum: [ 1, 2 ]
>>> +
>>> +  ranges: true
>>> +
>>> +patternProperties:
>>> +  "^tbu@[0-9a-f]+$":
>>> +    description: TBU child nodes
>>> +    type: object
>>> +    $ref: tbu-common.yaml#
>>
>>         additionalProperties: false
>>
>>
>> However, that's going to break with the extra QCom properties. In
>> json-schema, you can't have 2 schemas and extend the properties of
>> their child nodes. The validator doesn't "see" the child node schemas at
>> the same time. You are going to have to move QCom SMMU to its own schema
>> and remove it from arm,smmu.yaml.
> 
> Although this common binding is pretty pointless - sorry I missed the 
> previous discussion, but these TBU registers are on obscure debugging 

Hi Robin,

I am not sure if your comments were resolved (no response here), so
please kindly check if nothing got lost from your feedback in v5:

https://lore.kernel.org/all/20240226172218.69486-2-quic_c_gdjako@quicinc.com/

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index a4042ae24770..ba3237023b39 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -235,6 +235,20 @@  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]+$":
+    description: TBU child nodes
+    type: object
+    $ref: tbu-common.yaml#
+
 required:
   - compatible
   - reg
diff --git a/Documentation/devicetree/bindings/iommu/tbu-common.yaml b/Documentation/devicetree/bindings/iommu/tbu-common.yaml
new file mode 100644
index 000000000000..3e95b356e572
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/tbu-common.yaml
@@ -0,0 +1,28 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/tbu-common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Translation Buffer Unit (TBU) common properties
+
+maintainers:
+  - Georgi Djakov <quic_c_gdjako@quicinc.com>
+
+description:
+  The SMMU implements a TBU for system masters. It consists if a
+  Translation Lookaside Buffer (TLB) that caches page tables.
+
+properties:
+  reg:
+    maxItems: 1
+
+  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
+
+additionalProperties: true
+...