Re: [PATCH 4/5] pata: ixp4xx: Add DT bindings

[Rob Herring <robh@kernel.org>]

On Wed, Jul 28, 2021 at 11:02:41AM +0200, Linus Walleij wrote:
> This adds device tree bindings for the Intel IXP4xx compact flash card
> interface.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../ata/intel,ixp4xx-compact-flash.yaml       | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml
> 
> diff --git a/Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml b/Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml
> new file mode 100644
> index 000000000000..b254be858de6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/intel,ixp4xx-compact-flash.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel IXP4xx CompactFlash Card Controller
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +description: |
> +  The IXP4xx network processors have a CompactFlash interface that presents
> +  a CompactFlash card to the system as a true IDE (parallell ATA) device. The

typo. (you can setup checkpatch.pl to do spell checking)

> +  device is always connected to the expansion bus of the IXP4xx SoCs using one
> +  or two chip select areas and address translating logic on the board. The
> +  node must be placed inside a chip select node on the IXP4xx expansion bus.
> +
> +properties:
> +  compatible:
> +    const: intel,ixp4xx-compact-flash
> +
> +  reg:
> +    minItems: 2
> +    maxItems: 2

What is each entry?

> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +allOf:
> +  - $ref: pata-common.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    bus@c4000000 {
> +      compatible = "intel,ixp43x-expansion-bus-controller", "syscon";
> +      reg = <0xc4000000 0x1000>;
> +      native-endian;
> +      #address-cells = <2>;
> +      #size-cells = <1>;
> +      ranges = <0 0x0 0x50000000 0x01000000>, <1 0x0 0x51000000 0x01000000>;
> +      dma-ranges = <0 0x0 0x50000000 0x01000000>, <1 0x0 0x51000000 0x01000000>;
> +      ide@1,0 {
> +        compatible = "intel,ixp4xx-compact-flash";

> +        intel,ixp4xx-eb-t1 = <3>;
> +        intel,ixp4xx-eb-t2 = <3>;

Can you omit these? Otherwise, this will start failing soon as 
unevaluatedProperties is implemented (I have a test branch). Or you need 
to handle child node properties as discussed.

> +        reg = <1 0x00000000 0x1000>, <1 0x00040000 0x1000>;
> +        interrupt-parent = <&gpio0>;
> +        interrupts = <12 IRQ_TYPE_EDGE_RISING>;
> +      };
> +    };
> +
> +...
> -- 
> 2.31.1
> 
>