Hi Rob, Please find my inline comments below Thank you Dragan > -----Original Message----- > From: Rob Herring [mailto:robh@xxxxxxxxxx] > Sent: Wednesday 1 May 2019 20:48 > To: Dragan Cvetic <draganc@xxxxxxxxxx> > Cc: arnd@xxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; Michal Simek <michals@xxxxxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > mark.rutland@xxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Derek Kiernan <dkiernan@xxxxxxxxxx> > Subject: Re: [PATCH V3 01/12] dt-bindings: xilinx-sdfec: Add SDFEC binding > > On Sat, Apr 27, 2019 at 11:04:55PM +0100, Dragan Cvetic wrote: > > Add the Soft Decision Forward Error Correction (SDFEC) Engine > > bindings which is available for the Zynq UltraScale+ RFSoC > > FPGA's. > > > > Signed-off-by: Dragan Cvetic <dragan.cvetic@xxxxxxxxxx> > > Signed-off-by: Derek Kiernan <derek.kiernan@xxxxxxxxxx> > > --- > > .../devicetree/bindings/misc/xlnx,sd-fec.txt | 58 ++++++++++++++++++++++ > > 1 file changed, 58 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt > > > > diff --git a/Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt b/Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt > > new file mode 100644 > > index 0000000..425b6a6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt > > @@ -0,0 +1,58 @@ > > +* 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". > > Perhaps append with '-bits' and make the values 0, 128, 256, 512. > The suggested will require the extra code for converting from 128,256,512 to 1,2,4, as HW is configured with 1, 2 and 4. > > +- 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". > > Same here. > Same comment as previous one. > > +Optional properties: > > +- interrupts: should contain SDFEC interrupt number > > The interrupt may not be wired? My mistake. It should stay: interrupt-parent = <&axi_intc>; interrupts = <1 0>; > > > + > > +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_aclk","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 = <&gic>; > > + interrupts = <0 89 4>; > > + xlnx,sdfec-code = "ldpc"; > > + xlnx,sdfec-din-words = <0>; > > + xlnx,sdfec-din-width = <2>; > > + xlnx,sdfec-dout-words = <0>; > > + xlnx,sdfec-dout-width = <1>; > > + }; > > -- > > 2.7.4 > >
