On Sun, Jan 26, 2025 at 10:23:21PM -0600, Rob Herring wrote:

On Sun, Jan 26, 2025 at 07:59:04PM +0100, J. Neuschäfer wrote:

quoted

Convert the Freescale localbus controller bindings from text form to
YAML. The list of compatible strings reflects current usage.

Changes compared to the txt version:
 - removed the board-control (fsl,mpc8272ads-bcsr) node because it only
   appears in this example and nowhere else
 - added a new example with NAND flash

Remaining issues:
 - The localbus is not really a simple-bus: Unit addresses are not simply
   addresses on a memory bus. Instead, they have a format: The first cell
   is a chip select number, the remaining one or two cells are bus
   addresses.

That's every external parallel bus. See bindings/memory-controllers/*

Probably fine to leave 'simple-bus' if that's your question. It's more 
that there is configuration for the chipselect timings that make's this 
not a simple-bus. But the address translation should work just fine.

My concern mainly stems from the resulting warnings if I allow/use simple-bus:

Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dts:77.23-84.15:
  Warning (simple_bus_reg): /example-1/localbus@e0005000/flash@0,0: simple-bus unit address format error, expected "0"
Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dts:86.22-92.15:
  Warning (simple_bus_reg): /example-1/localbus@e0005000/nand@1,0: simple-bus unit address format error, expected "100000000"

Existing devicetrees specify the eLBC with compatible = ..., "simple-bus",
which lead me to include the simple-bus compatible both in the binding
itself and in the examples, which in turn leads to (correct) warnings
from DTC about node names such as nand@1,0 (it expects 100000000).
nand@1,0 was however completely correct for the eLBC bus, because it's
not one big linear address, but rather a chip select (1) and an address (0).

My current idea to resolve this contradiction is to remove simple-bus
from the binding and change affected devicetrees later.

quoted

Signed-off-by: J. Neuschäfer <redacted>
---
 .../devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml |  61 +++++++++
 .../bindings/powerpc/fsl/fsl,elbc-gpcm-uio.yaml    |  55 ++++++++
 .../devicetree/bindings/powerpc/fsl/fsl,elbc.yaml  | 150 +++++++++++++++++++++
 .../devicetree/bindings/powerpc/fsl/lbc.txt        |  43 ------
 4 files changed, 266 insertions(+), 43 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml b/Documentation/devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..127f164443972bbaf50fd9daa80c504577ddd7bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml

@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/fsl,elbc-fcm-nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NAND flash attached to Freescale eLBC
+
+maintainers:
+  - J. Neuschäfer <j.ne@posteo.net>
+
+allOf:
+  - $ref: nand-chip.yaml#
+
+properties:
+  compatible:
+    oneOf:

Don't need oneOf.

How would I express "either one of various chip-specific strings
followed by fsl,elbc-fcm-nand, or fsl,elbc-fcm-nand alone"?

quoted

+      - items:
+          - enum:
+              - fsl,mpc8313-fcm-nand
+              - fsl,mpc8315-fcm-nand
+              - fsl,mpc8377-fcm-nand
+              - fsl,mpc8378-fcm-nand
+              - fsl,mpc8379-fcm-nand
+              - fsl,mpc8536-fcm-nand
+              - fsl,mpc8569-fcm-nand
+              - fsl,mpc8572-fcm-nand
+              - fsl,p1020-fcm-nand
+              - fsl,p1021-fcm-nand
+              - fsl,p1025-fcm-nand
+              - fsl,p2020-fcm-nand
+          - const: fsl,elbc-fcm-nand
+      - const: fsl,elbc-fcm-nand
+
+  reg:
+    maxItems: 1
+
+  "#address-cells": true
+
+  "#size-cells": true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false

If you use anything from nand-chip.yaml, then you need 
unevaluatedProperties here.

Noted, will fix.

quoted

+
+examples:
+  - |
+    localbus {
+        #address-cells = <2>;
+        #size-cells = <1>;
+
+        nand@1,0 {
+            #address-cells = <1>;
+            #size-cells = <1>;
+            compatible = "fsl,mpc8315-fcm-nand",
+                         "fsl,elbc-fcm-nand";
+            reg = <0x1 0x0 0x2000>;
+        };
+    };

diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fsl,elbc-gpcm-uio.yaml b/Documentation/devicetree/bindings/powerpc/fsl/fsl,elbc-gpcm-uio.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..60f849b79c11a4060f2fa4ab163f9fa9317df130
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/fsl,elbc-gpcm-uio.yaml

@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/powerpc/fsl/fsl,elbc-gpcm-uio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Userspace I/O interface for Freescale eLBC devices
+
+maintainers:
+  - J. Neuschäfer <j.ne@posteo.net>
+
+properties:
+  compatible:
+    const: fsl,elbc-gpcm-uio
+
+  reg:
+    maxItems: 1
+
+  elbc-gpcm-br:
+    description: Base Register (BR) value to set
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  elbc-gpcm-or:
+    description: Option Register (OR) value to set
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  device_type: true

This should be dropped.

Will do

quoted

diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fsl,elbc.yaml b/Documentation/devicetree/bindings/powerpc/fsl/fsl,elbc.yaml

[...]

quoted

+  "#address-cells":
+    enum: [2, 3]
+    description: |

Don't need '|' unless there's some formatting.

Will remove.

quoted

+      The first cell is the chipselect number, and the remaining cells are the
+      offset into the chipselect.
+
+  "#size-cells":
+    enum: [1, 2]
+    description: |
+      Either one or two, depending on how large each chipselect can be.
+
+  ranges:
+    description: |
+      Each range corresponds to a single chipselect, and covers the entire
+      access window as configured.
+
+patternProperties:
+  "^.*@.*$":

You should define the unit-address format here: @<chipselect>,<offset>

Will do.



Thanks,
J. Neuschäfer