diff mbox series

dt-bindings: memory-controllers: arm,pl353-smc: Extend to support 'arm,pl354' SMC

Message ID 20221021203928.286169-1-robh@kernel.org
State Accepted
Commit de67fa80c66992b13dd018ec18e8c91156522c18
Headers show
Series dt-bindings: memory-controllers: arm,pl353-smc: Extend to support 'arm,pl354' SMC | expand

Commit Message

Rob Herring (Arm) Oct. 21, 2022, 8:39 p.m. UTC 
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).

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@kernel.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%)

Comments

Miquel Raynal Oct. 25, 2022, 7:49 a.m. UTC | #1 
Hi Rob,

robh@kernel.org wrote on Mon, 24 Oct 2022 09:31:41 -0500:

> On Mon, Oct 24, 2022 at 3:14 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Rob,
> >
> > robh@kernel.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?  
> 
> Address translation looks for a matching entry, so order doesn't
> matter. However, we have seen cases in PCI hosts where the driver
> populates registers based on the order of ranges. That's a driver
> problem IMO. For PCI, it was multiple entries of the same type. For
> this, we have the chip select number in the entry, so we shouldn't
> have the same sort of problem. Except there is another issue that the
> SRAM interface chipselects are numbered 1 and 2. The PL353 can have 4
> NAND chipselects, I don't think the host addresses are necessarily in
> order or contiguous either, so you could need 4 entries for NAND. The
> existing description doesn't handle that, and the chipselects for the
> SRAM interface should have been numbered 4-7. I don't mind saying the
> entries should be in order by chipselect, but we can't define indices
> of the entries as was done. It's all kind of academic because we don't
> have any h/w needing anything else though the Arm boards would if the
> child nodes actually got defined (not likely at this point).

Alright, thanks for the feedback.

Cheers,
Miquèl
diff mbox series

Patch

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@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
+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@e000e000 {