[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221024101405.3c2e163a@xps-13>
Date: Mon, 24 Oct 2022 10:14:05 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Rob Herring <robh@...nel.org>
Cc: Naga Sureshkumar Relli <nagasure@...inx.com>,
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Sureshkumar Relli <naga.sureshkumar.relli@...inx.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: memory-controllers: arm,pl353-smc: Extend
to support 'arm,pl354' SMC
Hi Rob,
robh@...nel.org wrote on Fri, 21 Oct 2022 15:39:28 -0500:
> Add support for the Arm PL354 static memory controller to the existing
> Arm PL353 binding. Both are different configurations of the same IP with
> support for different types of memory interfaces.
>
> The 'arm,pl354' binding has already been in use upstream for a long time
> in Arm development boards. The existing users have only the controller
> without any child devices, so drop the required address properties
> (ranges, #address-cells, #size-cells). The schema for 'ranges' is too
> constrained as the order is not important and the PL354 has 8
> chipselects (And the PL353 actually has up to 8 too).
I'm not convinced the ranges constraint should be soften. For me
the order was important (and the description in the yaml useful, but
that's a personal opinion). What makes you think the ranges order is
not relevant on PL353?
Thanks,
Miquèl
> The clocks aren't really correct in either case. There's 1 bus clock and
> then a clock for each of the 2 memory interfaces.
>
> Signed-off-by: Rob Herring <robh@...nel.org>
> ---
> ...{arm,pl353-smc.yaml => arm,pl35x-smc.yaml} | 80 ++++++++++++-------
> 1 file changed, 53 insertions(+), 27 deletions(-)
> rename Documentation/devicetree/bindings/memory-controllers/{arm,pl353-smc.yaml => arm,pl35x-smc.yaml} (65%)
>
> diff --git a/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml b/Documentation/devicetree/bindings/memory-controllers/arm,pl35x-smc.yaml
> similarity index 65%
> rename from Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml
> rename to Documentation/devicetree/bindings/memory-controllers/arm,pl35x-smc.yaml
> index 01c9acf9275d..bd23257fe021 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/arm,pl35x-smc.yaml
> @@ -1,26 +1,31 @@
> # 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#
> +$id: http://devicetree.org/schemas/memory-controllers/arm,pl35x-smc.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: ARM PL353 Static Memory Controller (SMC) device-tree bindings
> +title: Arm PL35x Series Static Memory Controller (SMC)
>
> 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
> +description: |
> + The PL35x 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).
> + SRAM or NOR) depending on the specific configuration.
> +
> + The TRM is available here:
> + https://documentation-service.arm.com/static/5e8e2524fd977155116a58aa
>
> # We need a select here so we don't match all nodes with 'arm,primecell'
> select:
> properties:
> compatible:
> contains:
> - const: arm,pl353-smc-r2p1
> + enum:
> + - arm,pl353-smc-r2p1
> + - arm,pl354
> required:
> - compatible
>
> @@ -30,7 +35,9 @@ properties:
>
> compatible:
> items:
> - - const: arm,pl353-smc-r2p1
> + - enum:
> + - arm,pl353-smc-r2p1
> + - arm,pl354
> - const: arm,primecell
>
> "#address-cells":
> @@ -46,30 +53,25 @@ properties:
> The three chip select regions are defined in 'ranges'.
>
> clocks:
> - items:
> - - description: clock for the memory device bus
> - - description: main clock of the SMC
> + minItems: 1
> + maxItems: 2
>
> clock-names:
> - items:
> - - const: memclk
> - - const: apb_pclk
> + minItems: 1
> + maxItems: 2
>
> ranges:
> minItems: 1
> - 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
> + maxItems: 8
>
> - interrupts: true
> + interrupts:
> + minItems: 1
> + items:
> + - description: Combined or Memory interface 0 IRQ
> + - description: Memory interface 1 IRQ
>
> patternProperties:
> - "@[0-3],[a-f0-9]+$":
> + "@[0-7],[a-f0-9]+$":
> type: object
> description: |
> The child device node represents the controller connected to the SMC
> @@ -87,7 +89,7 @@ patternProperties:
> - description: |
> Chip-select ID, as in the parent range property.
> minimum: 0
> - maximum: 2
> + maximum: 7
> - description: |
> Offset of the memory region requested by the device.
> - description: |
> @@ -102,12 +104,36 @@ required:
> - reg
> - clock-names
> - clocks
> - - "#address-cells"
> - - "#size-cells"
> - - ranges
>
> additionalProperties: false
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: arm,pl354
> + then:
> + properties:
> + clocks:
> + # According to TRM, really should be 3 clocks
> + maxItems: 1
> +
> + clock-names:
> + const: apb_pclk
> +
> + else:
> + properties:
> + clocks:
> + items:
> + - description: clock for the memory device bus
> + - description: main clock of the SMC
> +
> + clock-names:
> + items:
> + - const: memclk
> + - const: apb_pclk
> +
> examples:
> - |
> smcc: memory-controller@...0e000 {
Powered by blists - more mailing lists