mbox series

[0/9] YAML conversion of several Freescale/PowerPC DT bindings

Message ID 20250126-ppcyaml-v1-0-50649f51c3dd@posteo.net
Headers show
Series YAML conversion of several Freescale/PowerPC DT bindings | expand

Message

J. Neuschäfer via B4 Relay Jan. 26, 2025, 6:58 p.m. UTC
This is a spin-off of the series titled
"powerpc: MPC83xx cleanup and LANCOM NWAPP2 board".

During the development of that series, it became clear that many
devicetree bindings for Freescale MPC8xxx platforms are still in the old
plain-text format, or don't exist at all, and in any case don't mention
all valid compatible strings.

Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
J. Neuschäfer (9):
      dt-bindings: powerpc: Add binding for Freescale/NXP MPC83xx SoCs
      dt-bindings: ata: Convert fsl,pq-sata binding to YAML
      dt-bindings: crypto: Convert fsl,sec-2.0 binding to YAML
      dt-bindings: mfd: Convert fsl,mcu-mpc8349emitx binding to YAML
      dt-bindings: dma: Convert fsl,elo*-dma bindings to YAML
      dt-bindings: pci: Add fsl,mpc83xx-pcie bindings
      dt-bindings: watchdog: Convert mpc8xxx-wdt binding to YAML
      dt-bindings: spi: Convert Freescale SPI bindings to YAML
      [RFC] dt-bindings: nand: Convert fsl,elbc bindings to YAML

 .../devicetree/bindings/ata/fsl,pq-sata.yaml       |  59 ++++++
 Documentation/devicetree/bindings/ata/fsl-sata.txt |  28 ---
 .../devicetree/bindings/crypto/fsl,sec2.0.yaml     | 139 ++++++++++++++
 .../devicetree/bindings/crypto/fsl-sec2.txt        |  65 -------
 .../devicetree/bindings/dma/fsl,elo-dma.yaml       | 129 +++++++++++++
 .../devicetree/bindings/dma/fsl,elo3-dma.yaml      | 105 +++++++++++
 .../devicetree/bindings/dma/fsl,eloplus-dma.yaml   | 120 ++++++++++++
 .../bindings/mfd/fsl,mcu-mpc8349emitx.yaml         |  53 ++++++
 .../devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml |  61 ++++++
 .../devicetree/bindings/pci/fsl,mpc8xxx-pci.yaml   |  83 +++++++++
 .../devicetree/bindings/powerpc/fsl/dma.txt        | 204 ---------------------
 .../bindings/powerpc/fsl/fsl,elbc-gpcm-uio.yaml    |  55 ++++++
 .../devicetree/bindings/powerpc/fsl/fsl,elbc.yaml  | 150 +++++++++++++++
 .../bindings/powerpc/fsl/fsl,mpc83xx.yaml          |  67 +++++++
 .../devicetree/bindings/powerpc/fsl/lbc.txt        |  43 -----
 .../bindings/powerpc/fsl/mcu-mpc8349emitx.txt      |  17 --
 .../devicetree/bindings/spi/fsl,espi.yaml          |  56 ++++++
 Documentation/devicetree/bindings/spi/fsl,spi.yaml |  71 +++++++
 Documentation/devicetree/bindings/spi/fsl-spi.txt  |  62 -------
 .../devicetree/bindings/watchdog/mpc8xxx-wdt.txt   |  25 ---
 .../devicetree/bindings/watchdog/mpc8xxx-wdt.yaml  |  64 +++++++
 21 files changed, 1212 insertions(+), 444 deletions(-)
---
base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
change-id: 20250126-ppcyaml-680ccd8b3fc2

Best regards,

Comments

Krzysztof Kozlowski Jan. 27, 2025, 8:37 a.m. UTC | #1
On Sun, Jan 26, 2025 at 07:59:04PM +0100, J. Neuschäfer wrote:
> Convert the Freescale localbus controller bindings from text form to
> YAML. The list of compatible strings reflects current usage.

simple-bus and 20 other compatibles you used were not present in the
original binding. Does above "list of compatible strings" mean you just
added them?

> 
> 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.
> 
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
>  .../devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml |  61 +++++++++
>  .../bindings/powerpc/fsl/fsl,elbc-gpcm-uio.yaml    |  55 ++++++++

Please split the conversion from adding new bindings. For example above
file and its compatible fsl,elbc-gpcm-uio was not documented in original
TXT.

...

> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fsl,elbc.yaml b/Documentation/devicetree/bindings/powerpc/fsl/fsl,elbc.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..6bbceb82c77826499abe85879e9189b18d396eea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/powerpc/fsl/fsl,elbc.yaml
> @@ -0,0 +1,150 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/powerpc/fsl/fsl,elbc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Enhanced Local Bus Controller

What sort of bus is it? Memory bus? Then place it with others, see
memory directory.

> +
> +maintainers:
> +  - J. Neuschäfer <j.ne@posteo.net>
> +
> +properties:
> +  $nodename:
> +    pattern: "^localbus@[0-9a-f]+$"
> +
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - fsl,mpc8313-elbc
> +              - fsl,mpc8315-elbc
> +              - fsl,mpc8377-elbc
> +              - fsl,mpc8378-elbc
> +              - fsl,mpc8379-elbc
> +              - fsl,mpc8536-elbc
> +              - fsl,mpc8569-elbc
> +              - fsl,mpc8572-elbc
> +              - fsl,p1020-elbc
> +              - fsl,p1021-elbc
> +              - fsl,p1023-elbc
> +              - fsl,p2020-elbc
> +              - fsl,p2041-elbc
> +              - fsl,p3041-elbc
> +              - fsl,p4080-elbc
> +              - fsl,p5020-elbc
> +              - fsl,p5040-elbc
> +          - const: fsl,elbc
> +          - const: simple-bus
> +
> +      - items:
> +          - const: fsl,mpc8272-localbus
> +          - const: fsl,pq2-localbus
> +
> +      - items:
> +          - enum:
> +              - fsl,mpc8247-localbus
> +              - fsl,mpc8248-localbus
> +              - fsl,mpc8360-localbus
> +          - const: fsl,pq2pro-localbus
> +          - const: simple-bus
> +
> +      - items:
> +          - enum:
> +              - fsl,mpc8540-localbus
> +              - fsl,mpc8544-lbc
> +              - fsl,mpc8544-localbus
> +              - fsl,mpc8548-lbc
> +              - fsl,mpc8548-localbus
> +              - fsl,mpc8560-localbus
> +              - fsl,mpc8568-localbus
> +          - const: fsl,pq3-localbus
> +          - const: simple-bus
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    enum: [2, 3]
> +    description: |
> +      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:
> +  "^.*@.*$":
> +    type: object

And probably you need 
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    localbus@f0010100 {
> +        compatible = "fsl,mpc8272-localbus",
> +                     "fsl,pq2-localbus";
> +        #address-cells = <2>;
> +        #size-cells = <1>;
> +        reg = <0xf0010100 0x40>;

compatible, then reg - see DTS coding style.

> +
> +        ranges = <0x0 0x0 0xfe000000 0x02000000
> +                  0x1 0x0 0xf4500000 0x00008000
> +                  0x2 0x0 0xfd810000 0x00010000>;
> +
> +        flash@0,0 {
> +            compatible = "jedec-flash";
> +            reg = <0x0 0x0 0x2000000>;

Well, here it is correct

> +            bank-width = <4>;
> +            device-width = <1>;
> +        };
> +
> +        simple-periph@2,0 {
> +            compatible = "fsl,elbc-gpcm-uio";
> +            reg = <0x2 0x0 0x10000>;
> +            elbc-gpcm-br = <0xfd810800>;
> +            elbc-gpcm-or = <0xffff09f7>;
> +        };
> +    };
> +
> +  - |
> +    localbus@e0005000 {

compatible, reg

> +        #address-cells = <2>;
> +        #size-cells = <1>;
> +        compatible = "fsl,mpc8315-elbc", "fsl,elbc", "simple-bus";
> +        reg = <0xe0005000 0x1000>;
> +        interrupts = <77 0x8>;
> +        interrupt-parent = <&ipic>;
> +
> +        ranges = <0x0 0x0 0xfe000000 0x00800000
> +                  0x1 0x0 0xe0600000 0x00002000
> +                  0x2 0x0 0xf0000000 0x00020000
> +                  0x3 0x0 0xfa000000 0x00008000>;
> +
> +        flash@0,0 {

compatible, reg

> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            compatible = "cfi-flash";
> +            reg = <0x0 0x0 0x800000>;
> +            bank-width = <2>;
> +            device-width = <1>;
> +        };
> +
> +        nand@1,0 {

compatible, reg

> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            compatible = "fsl,mpc8315-fcm-nand",
> +                         "fsl,elbc-fcm-nand";
> +            reg = <0x1 0x0 0x2000>;
> +        };

Best regards,
Krzysztof
J. Neuschäfer Jan. 31, 2025, 11:33 a.m. UTC | #2
On Wed, Jan 29, 2025 at 05:29:41PM -0500, Frank Li wrote:
> On Sun, Jan 26, 2025 at 07:58:55PM +0100, J. Neuschäfer wrote:
> > This is a spin-off of the series titled
> > "powerpc: MPC83xx cleanup and LANCOM NWAPP2 board".
> >
> > During the development of that series, it became clear that many
> > devicetree bindings for Freescale MPC8xxx platforms are still in the old
> > plain-text format, or don't exist at all, and in any case don't mention
> > all valid compatible strings.
> >
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> 
> Please cc imx@lists.linux.dev next time
> 
> Frank

Will do.

Best regards,
J. Neuschäfer
J. Neuschäfer Feb. 4, 2025, 11:31 p.m. UTC | #3
On Sun, Jan 26, 2025 at 10:50:04PM -0600, Rob Herring wrote:
> On Sun, Jan 26, 2025 at 07:59:01PM +0100, J. Neuschäfer wrote:
> > Supplement Documentation/devicetree/bindings/pci/fsl,pci.txt with a more
> > formal binding in YAML format.
> > 
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > ---
[...]
> > +title: Freescale MPC83xx PCI/PCI-X/PCIe controllers
> > +
> > +description: |
> > +  Binding for the PCI/PCI-X/PCIe host bridges on MPC8xxx SoCs.
> > +  See also: Documentation/devicetree/bindings/pci/fsl,pci.txt
> 
> Can you move that information here.

Will do.

> 
> > +
> > +maintainers:
> > +  - J. Neuschäfer <j.neuschaefer@gmx.net>
> > +
> > +allOf:
> > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - enum:
> > +              - fsl,mpc8308-pcie
> > +              - fsl,mpc8315-pcie
> > +              - fsl,mpc8377-pcie
> > +              - fsl,mpc8378-pcie
> > +          - const: fsl,mpc8314-pcie
> > +      - const: fsl,mpc8314-pcie
> > +      - items:
> > +          - const: fsl,mpc8360-pci
> > +          - const: fsl,mpc8349-pci
> > +      - const: fsl,mpc8349-pci
> > +      - items:
> > +          - const: fsl,mpc8540-pcix
> > +          - const: fsl,mpc8540-pci
> > +      - const: fsl,mpc8540-pci
> > +      - items:
> > +          - const: fsl,mpc8540-pcix
> > +          - const: fsl,mpc8540-pci
> > +      - const: fsl,mpc8548-pcie
> > +      - const: fsl,mpc8548-pcie
> > +      - const: fsl,mpc8641-pcie
> 
> Move all the single 'const' to 1 enum entry.

Will do

> > +
> > +  reg:
> > +    minItems: 1
> > +    items:
> > +      - description: internal registers
> > +      - description: config space access registers
> > +
> > +  clock-frequency:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> Don't need a type for this.

Will change.


Thanks,
J. Neuschäfer
J. Neuschäfer Feb. 4, 2025, 11:34 p.m. UTC | #4
On Wed, Jan 29, 2025 at 05:55:26PM -0500, Frank Li wrote:
> On Sun, Jan 26, 2025 at 07:59:01PM +0100, J. Neuschäfer wrote:
> > Supplement Documentation/devicetree/bindings/pci/fsl,pci.txt with a more
> > formal binding in YAML format.
> >
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > ---
> >  .../devicetree/bindings/pci/fsl,mpc8xxx-pci.yaml   | 83 ++++++++++++++++++++++
> >  1 file changed, 83 insertions(+)
[...]
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    pci1: pcie@e0009000 {
> 
> needn't label here

Will change.


Thanks,
J. Neuschäfer
Mukesh Kumar Savaliya Feb. 6, 2025, 12:42 p.m. UTC | #5
On 2/5/2025 5:04 AM, J. Neuschäfer wrote:
> On Wed, Jan 29, 2025 at 05:55:26PM -0500, Frank Li wrote:
>> On Sun, Jan 26, 2025 at 07:59:01PM +0100, J. Neuschäfer wrote:
>>> Supplement Documentation/devicetree/bindings/pci/fsl,pci.txt with a more
>>> formal binding in YAML format.
>>>
neat: subject: since binding is already mentioned in the prefix of the 
subject, no need to add bindings word again.
>>> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
>>> ---
>>>   .../devicetree/bindings/pci/fsl,mpc8xxx-pci.yaml   | 83 ++++++++++++++++++++++
>>>   1 file changed, 83 insertions(+)
> [...]
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> +    pci1: pcie@e0009000 {
>>
>> needn't label here
> 
> Will change.
> 
> 
> Thanks,
> J. Neuschäfer
>
J. Neuschäfer Feb. 6, 2025, 10:59 p.m. UTC | #6
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:
> > 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.

> 
> > 
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > ---
> >  .../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"?

> 
> > +      - 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.

> 
> > +
> > +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


> > diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fsl,elbc.yaml b/Documentation/devicetree/bindings/powerpc/fsl/fsl,elbc.yaml
[...]
> > +  "#address-cells":
> > +    enum: [2, 3]
> > +    description: |
> 
> Don't need '|' unless there's some formatting.

Will remove.

> 
> > +      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
J. Neuschäfer Feb. 7, 2025, 1:37 p.m. UTC | #7
On Thu, Feb 06, 2025 at 06:12:47PM +0530, Mukesh Kumar Savaliya wrote:
> neat: subject: since binding is already mentioned in the prefix of the
> subject, no need to add bindings word again.

Sounds reasonable, thanks
J. Neuschäfer