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

[Rob Herring <robh@kernel.org>]

On Wed, Feb 19, 2020 at 04:08:33PM +0100, Linus Walleij wrote:
> This adds YAML device tree bindings for the Integrator/AP
> logic modules. These are plug-in tiles used typically for
> FPGA prototyping.
> 
> Cc: devicetree@vger.kernel.org
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Fix Logical->Logic spelling error
> - Set generic names for bus and serial
> - Just map the addresses 1:1 and use empty ranges, as the
>   LM's don't really translate the address.
> - Provide proper DMA ranges: the LM modules see the RAM at
>   the system-wide alias @80000000 rather than 1:1.
> - Drop the reg from the bus node (we just need the ranges)
> - Make the regexp for the bus node such that @address is
>   optional, as we don't require any reg on the node
> ---
>  .../bindings/bus/arm,integrator-ap-lm.yaml    | 83 +++++++++++++++++++
>  1 file changed, 83 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..5db5267410a6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/arm,integrator-ap-lm.yaml
> @@ -0,0 +1,83 @@
> +# 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 Logic 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 logic modules (LM) designed specifically for
> +  use with this platform. A special system controller register can be read to
> +  determine if a logic module is connected at index 0, 1, 2 or 3. The logic
> +  module connector is described in this binding. The logic modules per se
> +  then have their own specific per-module bindings and they will be described
> +  as subnodes under this logic module extension bus.
> +
> +properties:
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +  compatible:
> +    items:
> +      - const: arm,integrator-ap-lm
> +
> +  ranges: true
> +  dma-ranges: true
> +
> +patternProperties:
> +  "^bus(@[0-9a-f]*)?$":
> +    description: Nodes on the Logic Module bus represent logic modules
> +      and are named with bus. The first module is at 0xc0000000, the second
> +      at 0xd0000000 and so on until the top of the memory of the system at
> +      0xffffffff. All information about the memory used by the module is
> +      in ranges and dma-ranges.
> +    type: object
> +
> +    required:
> +      - compatible
> +
> +required:
> +  - compatible
> +
> +examples:
> +  - |
> +    bus@c0000000 {
> +      compatible = "arm,integrator-ap-lm";
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +      ranges;

ranges = <0xc0000000 0xc0000000 0x10000000>;

Because you have a unit-address.

> +      dma-ranges;
> +
> +      bus {

bus@c0000000

Though, now I'm wondering this shouldn't just be 1 level with:

compatible = "arm,integrator-ap-lm", "simple-bus";

> +        compatible = "simple-bus";
> +        ranges = <0x00000000 0xc0000000 0x10000000>;
> +        /* The Logic Modules sees the Core Module 0 RAM @80000000 */
> +        dma-ranges = <0x00000000 0x80000000 0x10000000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        serial@00100000 {

Drop leading 0s.

> +          compatible = "arm,pl011", "arm,primecell";
> +          reg = <0x00100000 0x1000>;
> +          interrupts-extended = <&impd1_vic 1>;
> +        };
> +
> +        impd1_vic: interrupt-controller@03000000 {

Drop leading 0s.

Try 'make W=1 dt_binding_check' as that will warn on all these things.

> +          compatible = "arm,pl192-vic";
> +          interrupt-controller;
> +          #interrupt-cells = <1>;
> +          reg = <0x03000000 0x1000>;
> +          valid-mask = <0x00000bff>;
> +          interrupts-extended = <&pic 9>;
> +        };
> +      };
> +    };
> +
> +additionalProperties: false
> -- 
> 2.24.1
>