On Fri, Oct 1, 2021 at 5:01 AM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxxxxx> wrote:
>
> On 01/10/2021 02:09, Li Yang wrote:
> > Convert the txt binding to yaml format and add description. Drop the
> > "simple-bus" compatible string from the example and not allowed by the
> > binding any more. This will help to enforce the correct probe order
> > between parent device and child devices, but will require the ifc driver
> > to probe the child devices to work properly.
> >
> > Signed-off-by: Li Yang <leoyang.li@xxxxxxx>
> > ---
> > updates from previous submission:
> > - Drop "simple-bus" from binding and only "fsl,ifc" as compatible
> > - Fix one identiation problem of "reg"
> > - Add type restriction to "little-endian" property
> >
> > .../bindings/memory-controllers/fsl/ifc.txt | 82 -----------
> > .../bindings/memory-controllers/fsl/ifc.yaml | 137 ++++++++++++++++++
> > 2 files changed, 137 insertions(+), 82 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
> > create mode 100644 Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
> > deleted file mode 100644
> > index 89427b018ba7..000000000000
> > --- a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
> > +++ /dev/null
> > @@ -1,82 +0,0 @@
> > -Integrated Flash Controller
> > -
> > -Properties:
> > -- name : Should be ifc
> > -- compatible : should contain "fsl,ifc". The version of the integrated
> > - flash controller can be found in the IFC_REV register at
> > - offset zero.
> > -
> > -- #address-cells : Should be either two or three. The first cell is the
> > - chipselect number, and the remaining cells are the
> > - offset into the chipselect.
> > -- #size-cells : Either one or two, depending on how large each chipselect
> > - can be.
> > -- reg : Offset and length of the register set for the device
> > -- interrupts: IFC may have one or two interrupts. If two interrupt
> > - specifiers are present, the first is the "common"
> > - interrupt (CM_EVTER_STAT), and the second is the NAND
> > - interrupt (NAND_EVTER_STAT). If there is only one,
> > - that interrupt reports both types of event.
> > -
> > -- little-endian : If this property is absent, the big-endian mode will
> > - be in use as default for registers.
> > -
> > -- ranges : Each range corresponds to a single chipselect, and covers
> > - the entire access window as configured.
> > -
> > -Child device nodes describe the devices connected to IFC such as NOR (e.g.
> > -cfi-flash) and NAND (fsl,ifc-nand). There might be board specific devices
> > -like FPGAs, CPLDs, etc.
> > -
> > -Example:
> > -
> > - ifc@ffe1e000 {
> > - compatible = "fsl,ifc", "simple-bus";
> > - #address-cells = <2>;
> > - #size-cells = <1>;
> > - reg = <0x0 0xffe1e000 0 0x2000>;
> > - interrupts = <16 2 19 2>;
> > - little-endian;
> > -
> > - /* NOR, NAND Flashes and CPLD on board */
> > - ranges = <0x0 0x0 0x0 0xee000000 0x02000000
> > - 0x1 0x0 0x0 0xffa00000 0x00010000
> > - 0x3 0x0 0x0 0xffb00000 0x00020000>;
> > -
> > - flash@0,0 {
> > - #address-cells = <1>;
> > - #size-cells = <1>;
> > - compatible = "cfi-flash";
> > - reg = <0x0 0x0 0x2000000>;
> > - bank-width = <2>;
> > - device-width = <1>;
> > -
> > - partition@0 {
> > - /* 32MB for user data */
> > - reg = <0x0 0x02000000>;
> > - label = "NOR Data";
> > - };
> > - };
> > -
> > - flash@1,0 {
> > - #address-cells = <1>;
> > - #size-cells = <1>;
> > - compatible = "fsl,ifc-nand";
> > - reg = <0x1 0x0 0x10000>;
> > -
> > - partition@0 {
> > - /* This location must not be altered */
> > - /* 1MB for u-boot Bootloader Image */
> > - reg = <0x0 0x00100000>;
> > - label = "NAND U-Boot Image";
> > - read-only;
> > - };
> > - };
> > -
> > - cpld@3,0 {
> > - #address-cells = <1>;
> > - #size-cells = <1>;
> > - compatible = "fsl,p1010rdb-cpld";
> > - reg = <0x3 0x0 0x000001f>;
> > - };
> > - };
> > diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml
> > new file mode 100644
> > index 000000000000..19871ce39fe3
>
> Thanks for the patch.
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml
> > @@ -0,0 +1,137 @@
> > +# SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
>
> Checkpatch should scream here. If it doesn't, maybe you work on some old
> tree, which would also explain why you send it to my old address (not
> the one from get_maintainers). Please use both checkpatch and
> get_maintainers.
>
> You basically relicense bindings from GPL-2.0 only to new license,
> including GPL-3.0.
Ok. Will update the license.
>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/memory-controllers/fsl/ifc.yaml#
>
> File name should be "fsl,ifc.yaml"
Ok. But probably it is a little bit redundant as the upper level
folder also has fsl.
>
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: FSL/NXP Integrated Flash Controller
> > +
> > +maintainers:
> > + - Li Yang <leoyang.li@xxxxxxx>
> > +
> > +description: |
> > + NXP's integrated flash controller (IFC) is an advanced version of the
> > + enhanced local bus controller which includes similar programming and signal
> > + interfaces with an extended feature set. The IFC provides access to multiple
> > + external memory types, such as NAND flash (SLC and MLC), NOR flash, EPROM,
> > + SRAM and other memories where address and data are shared on a bus.
> > +
> > +properties:
> > + $nodename:
> > + pattern: "^ifc@[0-9a-f]+$"
>
> Nodes should be generic, so this looks like "memory-controller".
Ok.
>
> > +
> > + compatible:
> > + const: fsl,ifc
> > +
> > + "#address-cells":
> > + enum: [2, 3]
> > + description: |
> > + Should be either two or three. 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.
> > +
> > + reg:
> > + maxItems: 1
> > + description: |
> > + Offset and length of the register set for the device.
>
> Skip the description, it's obvious.
Ok.
>
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 2
> > + description: |
> > + IFC may have one or two interrupts. If two interrupt specifiers are
> > + present, the first is the "common" interrupt (CM_EVTER_STAT), and the
> > + second is the NAND interrupt (NAND_EVTER_STAT). If there is only one,
> > + that interrupt reports both types of event.
> > +
> > + little-endian:
> > + $ref: '/schemas/types.yaml#/definitions/flag'
>
> type: boolean
It will not have a true or false value, but only present or not. Is
the boolean type taking care of this too?
>
>
> Best regards,
> Krzysztof
