RE: [PATCH v4] dt-bindings: misc: xlnx,sd-fec: convert bindings to yaml

[Cvetic, Dragan]

Hi Rob,

> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: Wednesday, January 31, 2024 2:23 PM
> To: Cvetic, Dragan <dragan.cvetic@xxxxxxx>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley
> <conor+dt@xxxxxxxxxx>; Kiernan, Derek <derek.kiernan@xxxxxxx>; Jonathan
> Corbet <corbet@xxxxxxx>; Simek, Michal <michal.simek@xxxxxxx>; Erim,
> Salih <Salih.Erim@xxxxxxx>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <devicetree@xxxxxxxxxxxxxxx>; open list <linux-
> kernel@xxxxxxxxxxxxxxx>; open list:DOCUMENTATION <linux-
> doc@xxxxxxxxxxxxxxx>; moderated list:ARM/ZYNQ ARCHITECTURE <linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v4] dt-bindings: misc: xlnx,sd-fec: convert bindings to
> yaml
> 
> On Tue, Jan 30, 2024 at 04:12:58PM +0000, Dragan Cvetic wrote:
> > Convert AMD (Xilinx) sd-fec bindings to yaml format, so it can validate
> > dt-entries as well as any future additions to yaml.
> > Change in clocks is due to IP is itself configurable and
> > only the first two clocks are in all combinations. The last
> > 6 clocks can be present in some of them. It means order is
> > not really fixed and any combination is possible.
> > Interrupt may or may not be present.
> > The documentation for sd-fec bindings is now YAML, so update the
> > MAINTAINERS file.
> > Update the link to the new yaml file in xilinx_sdfec.rst.
> >
> > Signed-off-by: Dragan Cvetic <dragan.cvetic@xxxxxxx>
> > ---
> > Changes in v2:
> > ---
> > Drop clocks description.
> > Use "contains:" with enum for optional clock-names and update
> > comment explaining diference from the original DT binding file.
> > Remove trailing full stops.
> > Add more details in sdfec-code description.
> > Set sdfec-code to "string" not "string-array"
> > ---
> > Changes in v3:
> > Fix a mistake in example, set interrupt type to 0.
> > ---
> > Changes in v4:
> > Set interrupt type to high level sensitive.
> > Remove '|' from descriptions, no need to preserve format.
> > Remove not needed empty line.
> > ---
> >  .../devicetree/bindings/misc/xlnx,sd-fec.txt  |  58 --------
> >  .../devicetree/bindings/misc/xlnx,sd-fec.yaml | 137 ++++++++++++++++++
> >  Documentation/misc-devices/xilinx_sdfec.rst   |   2 +-
> >  MAINTAINERS                                   |   2 +-
> >  4 files changed, 139 insertions(+), 60 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/misc/xlnx,sd-
> fec.txt
> >  create mode 100644 Documentation/devicetree/bindings/misc/xlnx,sd-
> fec.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt
> b/Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt
> > deleted file mode 100644
> > index e3289634fa30..000000000000
> > --- a/Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt
> > +++ /dev/null
> > @@ -1,58 +0,0 @@
> > -* Xilinx SDFEC(16nm) IP *
> > -
> > -The Soft Decision Forward Error Correction (SDFEC) Engine is a Hard IP block
> > -which provides high-throughput LDPC and Turbo Code implementations.
> > -The LDPC decode & encode functionality is capable of covering a range of
> > -customer specified Quasi-cyclic (QC) codes. The Turbo decode functionality
> > -principally covers codes used by LTE. The FEC Engine offers significant
> > -power and area savings versus implementations done in the FPGA fabric.
> > -
> > -
> > -Required properties:
> > -- compatible: Must be "xlnx,sd-fec-1.1"
> > -- clock-names : List of input clock names from the following:
> > -    - "core_clk", Main processing clock for processing core (required)
> > -    - "s_axi_aclk", AXI4-Lite memory-mapped slave interface clock (required)
> > -    - "s_axis_din_aclk", DIN AXI4-Stream Slave interface clock (optional)
> > -    - "s_axis_din_words-aclk", DIN_WORDS AXI4-Stream Slave interface clock
> (optional)
> > -    - "s_axis_ctrl_aclk",  Control input AXI4-Stream Slave interface clock
> (optional)
> > -    - "m_axis_dout_aclk", DOUT AXI4-Stream Master interface clock
> (optional)
> > -    - "m_axis_dout_words_aclk", DOUT_WORDS AXI4-Stream Master
> interface clock (optional)
> > -    - "m_axis_status_aclk", Status output AXI4-Stream Master interface clock
> (optional)
> > -- clocks : Clock phandles (see clock_bindings.txt for details).
> > -- reg: Should contain Xilinx SDFEC 16nm Hardened IP block registers
> > -  location and length.
> > -- xlnx,sdfec-code : Should contain "ldpc" or "turbo" to describe the codes
> > -  being used.
> > -- xlnx,sdfec-din-words : A value 0 indicates that the DIN_WORDS interface is
> > -  driven with a fixed value and is not present on the device, a value of 1
> > -  configures the DIN_WORDS to be block based, while a value of 2
> configures the
> > -  DIN_WORDS input to be supplied for each AXI transaction.
> > -- xlnx,sdfec-din-width : Configures the DIN AXI stream where a value of 1
> > -  configures a width of "1x128b", 2 a width of "2x128b" and 4 configures a
> width
> > -  of "4x128b".
> > -- xlnx,sdfec-dout-words : A value 0 indicates that the DOUT_WORDS
> interface is
> > -  driven with a fixed value and is not present on the device, a value of 1
> > -  configures the DOUT_WORDS to be block based, while a value of 2
> configures the
> > -  DOUT_WORDS input to be supplied for each AXI transaction.
> > -- xlnx,sdfec-dout-width : Configures the DOUT AXI stream where a value of
> 1
> > -  configures a width of "1x128b", 2 a width of "2x128b" and 4 configures a
> width
> > -  of "4x128b".
> > -Optional properties:
> > -- interrupts: should contain SDFEC interrupt number
> > -
> > -Example
> > ----------------------------------------
> > -	sd_fec_0: sd-fec@a0040000 {
> > -		compatible = "xlnx,sd-fec-1.1";
> > -		clock-names =
> "core_clk","s_axi_aclk","s_axis_ctrl_aclk","s_axis_din_aclk","m_axis_status_acl
> k","m_axis_dout_aclk";
> > -		clocks =
> <&misc_clk_2>,<&misc_clk_0>,<&misc_clk_1>,<&misc_clk_1>,<&misc_clk_1>,
> <&misc_clk_1>;
> > -		reg = <0x0 0xa0040000 0x0 0x40000>;
> > -		interrupt-parent = <&axi_intc>;
> > -		interrupts = <1 0>;
> > -		xlnx,sdfec-code = "ldpc";
> > -		xlnx,sdfec-din-words = <0>;
> > -		xlnx,sdfec-din-width = <2>;
> > -		xlnx,sdfec-dout-words = <0>;
> > -		xlnx,sdfec-dout-width = <1>;
> > -	};
> > diff --git a/Documentation/devicetree/bindings/misc/xlnx,sd-fec.yaml
> b/Documentation/devicetree/bindings/misc/xlnx,sd-fec.yaml
> > new file mode 100644
> > index 000000000000..7be8439861a9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/xlnx,sd-fec.yaml
> > @@ -0,0 +1,137 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/misc/xlnx,sd-fec.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx SDFEC(16nm) IP
> > +
> > +maintainers:
> > +  - Cvetic, Dragan <dragan.cvetic@xxxxxxx>
> > +  - Erim, Salih <salih.erim@xxxxxxx>
> > +
> > +description:
> > +  The Soft Decision Forward Error Correction (SDFEC) Engine is a Hard IP
> block
> > +  which provides high-throughput LDPC and Turbo Code implementations.
> > +  The LDPC decode & encode functionality is capable of covering a range of
> > +  customer specified Quasi-cyclic (QC) codes. The Turbo decode
> functionality
> > +  principally covers codes used by LTE. The FEC Engine offers significant
> > +  power and area savings versus implementations done in the FPGA fabric.
> > +
> > +properties:
> > +  compatible:
> > +    const: xlnx,sd-fec-1.1
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 2
> > +    maxItems: 8
> > +    additionalItems: true
> > +    items:
> > +      - description: Main processing clock for processing core
> > +      - description: AXI4-Lite memory-mapped slave interface clock
> > +      - description: Control input AXI4-Stream Slave interface clock
> > +      - description: DIN AXI4-Stream Slave interface clock
> > +      - description: Status output AXI4-Stream Master interface clock
> > +      - description: DOUT AXI4-Stream Master interface clock
> > +      - description: DIN_WORDS AXI4-Stream Slave interface clock
> > +      - description: DOUT_WORDS AXI4-Stream Master interface clock
> > +
> > +  clock-names:
> > +    minItems: 2
> > +    maxItems: 8
> > +    additionalItems: true
> > +    items:
> > +      - const: core_clk
> > +      - const: s_axi_aclk
> > +    contains:
> > +      enum:
> > +        - s_axis_ctrl_aclk
> > +        - s_axis_din_aclk
> > +        - m_axis_status_aclk
> > +        - m_axis_dout_aclk
> > +        - s_axis_din_words_aclk
> > +        - m_axis_dout_words_aclk
> 
> This doesn't do what you think. It requires at least one of these clocks
> be present, so then at least 3 clocks. It also allows anything else to
> be present. You need:
> 
> allOf:
>   - minItems: 2
>     maxItems: 8
>     additionalItems: true
>     items:
>       - const: core_clk
>       - const: s_axi_aclk
>   - items:
>       enum:
>         - core_clk
>         - s_axi_aclk
>         - s_axis_ctrl_aclk
>         - s_axis_din_aclk
>         - m_axis_status_aclk
>         - m_axis_dout_aclk
>         - s_axis_din_words_aclk
>         - m_axis_dout_words_aclk

Will do it in the next patch

Kind Regards
Dragan