Re: [PATCH v22 09/18] dt-binding: memory: pl353-smc: Convert to yaml

[Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>]

On 09/06/2021 10:01, Miquel Raynal wrote:
> Convert this binding file to yaml schema.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../memory-controllers/arm,pl353-smc.yaml     | 133 ++++++++++++++++++
>  .../bindings/memory-controllers/pl353-smc.txt |  45 ------
>  2 files changed, 133 insertions(+), 45 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml
>  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/pl353-smc.txt
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml b/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml
> new file mode 100644
> index 000000000000..1de6f87d4986
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml
> @@ -0,0 +1,133 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/arm,pl353-smc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM PL353 Static Memory Controller (SMC) device-tree bindings
> +
> +maintainers:
> +  - Miquel Raynal <miquel.raynal@bootlin.com>
> +  - Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com>
> +
> +description:
> +  The PL353 Static Memory Controller is a bus where you can connect two kinds
> +  of memory interfaces, which are NAND and memory mapped interfaces (such as
> +  SRAM or NOR).
> +
> +# We need a select here so we don't match all nodes with 'arm,primecell'
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - arm,pl353-smc-r2p1

That's a const... but also I don't get the need for select.

> +  required:
> +    - compatible
> +
> +properties:
> +  $nodename:
> +    pattern: "^memory-controller@[0-9a-f]+$"
> +
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - arm,pl353-smc-r2p1
> +          - enum:
> +              - arm,primecell

This looks unusual. Basically you change the bindings, because before
they required "arm,pl353-smc-r2p1", "arm,primecell".

Don't you want here items:
 - const: ...
 - const: ...
?

> +
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 1
> +
> +  reg:
> +    items:
> +      - description: configuration registers for the host and sub-controllers

Just maxItems. Description is obvious.

> +
> +  clocks:
> +    items:
> +      - description: the clock for the memory device bus
> +      - description: the main clock of the controller

Isn't apb_pclk the bus clock (so second item below)?

> +
> +  clock-names:
> +    items:
> +      - const: memclk
> +      - const: apb_pclk


> +
> +  ranges:
> +    minItems: 1
> +    maxItems: 3
> +    description: |
> +      Memory bus areas for interacting with the devices. Reflects
> +      the memory layout with four integer values following:
> +      <cs-number> 0 <offset> <size>
> +    items:
> +      - description: NAND bank 0
> +      - description: NOR/SRAM bank 0
> +      - description: NOR/SRAM bank 1
> +
> +  interrupts: true
> +
> +patternProperties:
> +  ".*@[0-9]+,[0-9]+$":

Match with start ^. I think you cannot have 9 nodes and hex can appear
in address so maybe:
"^.*@[0-3],[a-f0-9]+$":


> +    type: object
> +    description: |
> +      The child device node represents the controller connected to the SMC
> +      bus. The controller can be a NAND controller or a pair of any memory
> +      mapped controllers such as NOR and SRAM controllers.
> +

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/