[PATCH 1/2] bus: Add DT bindings for Integrator/AP logical modules

[PATCH 1/2] bus: Add DT bindings for Integrator/AP logical modules

[Linus Walleij]

This adds YAML device tree bindings for the Integrator/AP
logical modules. These are plug-in tiles used typically for
FPGA prototyping.

Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../bindings/bus/arm,integrator-ap-lm.yaml    | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml

diff --git a/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml b/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml
new file mode 100644
index 000000000000..dfabfa466c05
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/arm,integrator-ap-lm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Integrator/AP Logical Module extension bus
+
+maintainers:
+  - Linus Walleij <linusw@kernel.org>
+
+description: The Integrator/AP is a prototyping platform and as such has a
+  site for stacking up to four logical modules (LM) designed specifically for
+  use with this platform. A special system controller register can be read to
+  determine if a logical module is connected at index 0, 1, 2 or 3. The logical
+  module connector is described in this binding. The logical modules per se
+  then have their own specific per-module bindings and they will be described
+  as subnodes under this logical module extension bus.
+
+properties:
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 1
+
+  compatible:
+    items:
+      - const: arm,integrator-ap-lm
+
+  ranges: true
+  dma-ranges: true
+
+patternProperties:
+  "^.*@[0-3],[0-9a-f]+$":
+    description: Nodes on the Logical Module bus represent logical modules
+      and are named with index,relative-address. The first module is at
+      0x00000000, the second at 0x10000000 and so on until the top of the
+      memory of the system at 0xffffffff.
+    type: object
+    properties:
+      reg:
+        maxItems: 1
+
+    required:
+      - compatible
+
+required:
+  - compatible
+
+examples:
+  - |
+    external-bus@c0000000 {
+      compatible = "arm,integrator-ap-lm";
+      #address-cells = <2>;
+      #size-cells = <1>;
+      ranges = <0 0x0 0xc0000000 0x10000000>,
+               <1 0x0 0xd0000000 0x10000000>,
+               <2 0x0 0xe0000000 0x10000000>,
+               <3 0x0 0xf0000000 0x10000000>;
+      dma-ranges = <0 0x0 0xc0000000 0x10000000>,
+               <1 0x0 0xd0000000 0x10000000>,
+               <2 0x0 0xe0000000 0x10000000>,
+               <3 0x0 0xf0000000 0x10000000>;
+      im-pd1@0,0 {
+        compatible = "simple-bus";
+        ranges = <0 0 0 0x10000000>;
+        dma-ranges = <0 0 0 0x10000000>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        uart@c0100000 {
+          compatible = "arm,pl011", "arm,primecell";
+          reg = <0x00100000 0x1000>;
+          interrupts-extended = <&impd1_vic 1>;
+        };
+
+        impd1_vic: interrupt-controller@c3000000 {
+          compatible = "arm,pl192-vic";
+          interrupt-controller;
+          #interrupt-cells = <1>;
+          reg = <0x03000000 0x1000>;
+          valid-mask = <0x00000bff>;
+          interrupts-extended = <&pic 9>;
+        };
+      };
+    };
+
+additionalProperties: false
-- 
2.23.0

Re: [PATCH 1/2] bus: Add DT bindings for Integrator/AP logical modules

[Robin Murphy]

Hi Linus,

On 13/02/2020 12:46 pm, Linus Walleij wrote:
> This adds YAML device tree bindings for the Integrator/AP
> logical modules. These are plug-in tiles used typically for
> FPGA prototyping.

Linguistic nit: s/logical/logic/g (for both patches)

> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>   .../bindings/bus/arm,integrator-ap-lm.yaml    | 89 +++++++++++++++++++
>   1 file changed, 89 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml b/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml
> new file mode 100644
> index 000000000000..dfabfa466c05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/arm,integrator-ap-lm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Integrator/AP Logical Module extension bus
> +
> +maintainers:
> +  - Linus Walleij <linusw@kernel.org>
> +
> +description: The Integrator/AP is a prototyping platform and as such has a
> +  site for stacking up to four logical modules (LM) designed specifically for
> +  use with this platform. A special system controller register can be read to
> +  determine if a logical module is connected at index 0, 1, 2 or 3. The logical
> +  module connector is described in this binding. The logical modules per se
> +  then have their own specific per-module bindings and they will be described
> +  as subnodes under this logical module extension bus.
> +
> +properties:
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 1
> +
> +  compatible:
> +    items:
> +      - const: arm,integrator-ap-lm
> +
> +  ranges: true
> +  dma-ranges: true
> +
> +patternProperties:
> +  "^.*@[0-3],[0-9a-f]+$":
> +    description: Nodes on the Logical Module bus represent logical modules
> +      and are named with index,relative-address. The first module is at
> +      0x00000000, the second at 0x10000000 and so on until the top of the
> +      memory of the system at 0xffffffff.
> +    type: object
> +    properties:
> +      reg:
> +        maxItems: 1
> +
> +    required:
> +      - compatible
> +
> +required:
> +  - compatible
> +
> +examples:
> +  - |
> +    external-bus@c0000000 {
> +      compatible = "arm,integrator-ap-lm";
> +      #address-cells = <2>;
> +      #size-cells = <1>;
> +      ranges = <0 0x0 0xc0000000 0x10000000>,
> +               <1 0x0 0xd0000000 0x10000000>,
> +               <2 0x0 0xe0000000 0x10000000>,
> +               <3 0x0 0xf0000000 0x10000000>;
> +      dma-ranges = <0 0x0 0xc0000000 0x10000000>,
> +               <1 0x0 0xd0000000 0x10000000>,
> +               <2 0x0 0xe0000000 0x10000000>,
> +               <3 0x0 0xf0000000 0x10000000>;

Is that dma-ranges mapping definitely appropriate? My impression from 
skimming the AP manual is that logic module masters would all see SDRAM 
through the 2GB-3GB alias region, independent of how their slaves are 
decoding incoming accesses. Even in the case of peer-to-peer accesses 
between logic modules, I'd imagine that the process of obtaining the 
target address to program would inherently go through the "ranges" 
translation and result in an 'absolute' PA anyway.

Robin.

> +      im-pd1@0,0 {
> +        compatible = "simple-bus";
> +        ranges = <0 0 0 0x10000000>;
> +        dma-ranges = <0 0 0 0x10000000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        uart@c0100000 {
> +          compatible = "arm,pl011", "arm,primecell";
> +          reg = <0x00100000 0x1000>;
> +          interrupts-extended = <&impd1_vic 1>;
> +        };
> +
> +        impd1_vic: interrupt-controller@c3000000 {
> +          compatible = "arm,pl192-vic";
> +          interrupt-controller;
> +          #interrupt-cells = <1>;
> +          reg = <0x03000000 0x1000>;
> +          valid-mask = <0x00000bff>;
> +          interrupts-extended = <&pic 9>;
> +        };
> +      };
> +    };
> +
> +additionalProperties: false
> 

Re: [PATCH 1/2] bus: Add DT bindings for Integrator/AP logical modules

[Rob Herring]

On Thu, Feb 13, 2020 at 6:46 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> This adds YAML device tree bindings for the Integrator/AP
> logical modules. These are plug-in tiles used typically for
> FPGA prototyping.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../bindings/bus/arm,integrator-ap-lm.yaml    | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml
>
> diff --git a/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml b/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml
> new file mode 100644
> index 000000000000..dfabfa466c05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/arm,integrator-ap-lm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Integrator/AP Logical Module extension bus
> +
> +maintainers:
> +  - Linus Walleij <linusw@kernel.org>
> +
> +description: The Integrator/AP is a prototyping platform and as such has a
> +  site for stacking up to four logical modules (LM) designed specifically for
> +  use with this platform. A special system controller register can be read to
> +  determine if a logical module is connected at index 0, 1, 2 or 3. The logical
> +  module connector is described in this binding. The logical modules per se
> +  then have their own specific per-module bindings and they will be described
> +  as subnodes under this logical module extension bus.
> +
> +properties:
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 1
> +
> +  compatible:
> +    items:
> +      - const: arm,integrator-ap-lm
> +
> +  ranges: true
> +  dma-ranges: true
> +
> +patternProperties:
> +  "^.*@[0-3],[0-9a-f]+$":
> +    description: Nodes on the Logical Module bus represent logical modules
> +      and are named with index,relative-address. The first module is at
> +      0x00000000, the second at 0x10000000 and so on until the top of the
> +      memory of the system at 0xffffffff.

What's the point of the index if the address alone is enough?

> +    type: object
> +    properties:
> +      reg:
> +        maxItems: 1
> +
> +    required:
> +      - compatible

'reg' should be required given a unit address is.

> +
> +required:
> +  - compatible
> +
> +examples:
> +  - |
> +    external-bus@c0000000 {

Node names should be generic:

bus@...

> +      compatible = "arm,integrator-ap-lm";
> +      #address-cells = <2>;
> +      #size-cells = <1>;
> +      ranges = <0 0x0 0xc0000000 0x10000000>,
> +               <1 0x0 0xd0000000 0x10000000>,
> +               <2 0x0 0xe0000000 0x10000000>,
> +               <3 0x0 0xf0000000 0x10000000>;
> +      dma-ranges = <0 0x0 0xc0000000 0x10000000>,
> +               <1 0x0 0xd0000000 0x10000000>,
> +               <2 0x0 0xe0000000 0x10000000>,
> +               <3 0x0 0xf0000000 0x10000000>;
> +      im-pd1@0,0 {

bus@...

> +        compatible = "simple-bus";
> +        ranges = <0 0 0 0x10000000>;
> +        dma-ranges = <0 0 0 0x10000000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        uart@c0100000 {

serial@100000

> +          compatible = "arm,pl011", "arm,primecell";
> +          reg = <0x00100000 0x1000>;
> +          interrupts-extended = <&impd1_vic 1>;
> +        };
> +
> +        impd1_vic: interrupt-controller@c3000000 {
> +          compatible = "arm,pl192-vic";
> +          interrupt-controller;
> +          #interrupt-cells = <1>;
> +          reg = <0x03000000 0x1000>;
> +          valid-mask = <0x00000bff>;
> +          interrupts-extended = <&pic 9>;
> +        };
> +      };
> +    };
> +
> +additionalProperties: false
> --
> 2.23.0
>

Re: [PATCH 1/2] bus: Add DT bindings for Integrator/AP logical modules

[Rob Herring]

On Thu, 13 Feb 2020 13:46:19 +0100, Linus Walleij wrote:
> This adds YAML device tree bindings for the Integrator/AP
> logical modules. These are plug-in tiles used typically for
> FPGA prototyping.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../bindings/bus/arm,integrator-ap-lm.yaml    | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.example.dt.yaml: im-pd1@0,0: $nodename:0: 'im-pd1@0,0' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.example.dt.yaml: uart@c0100000: $nodename:0: 'uart@c0100000' does not match '^serial(@[0-9a-f,]+)*$'

See https://patchwork.ozlabs.org/patch/1237467
Please check and re-submit.