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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e431d594-05cd-27b8-fcbe-11c310b99cd3@canonical.com>
Date:   Wed, 9 Jun 2021 14:12:40 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
To:     Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Tudor Ambarus <Tudor.Ambarus@...rochip.com>,
        linux-mtd@...ts.infradead.org, Rob Herring <robh+dt@...nel.org>,
        devicetree@...r.kernel.org
Cc:     Michal Simek <monstr@...str.eu>,
        Naga Sureshkumar Relli <nagasure@...inx.com>,
        Amit Kumar Mahapatra <akumarma@...inx.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        helmut.grohne@...enta.de, Srinivas Goud <sgoud@...inx.com>,
        Siva Durga Prasad Paladugu <sivadur@...inx.com>
Subject: Re: [PATCH v22 09/18] dt-binding: memory: pl353-smc: Convert to yaml

On 09/06/2021 10:01, Miquel Raynal wrote:
> Convert this binding file to yaml schema.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@...tlin.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@...tlin.com>
> +  - Naga Sureshkumar Relli <naga.sureshkumar.relli@...inx.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