RE: [PATCH V3 01/12] dt-bindings: xilinx-sdfec: Add SDFEC binding
Hi Rob, Please find inline comments below Regards Dragan > -Original Message- > From: Rob Herring [mailto:r...@kernel.org] > Sent: Thursday 2 May 2019 21:16 > To: Dragan Cvetic > Cc: a...@arndb.de; gre...@linuxfoundation.org; Michal Simek > ; linux-arm-ker...@lists.infradead.org; > mark.rutl...@arm.com; devicet...@vger.kernel.org; > linux-kernel@vger.kernel.org; Derek Kiernan > Subject: Re: [PATCH V3 01/12] dt-bindings: xilinx-sdfec: Add SDFEC binding > > On Thu, May 2, 2019 at 6:04 AM Dragan Cvetic wrote: > > > > Hi Rob, > > > > Please find my inline comments below > > > > Thank you > > Dragan > > > > > -Original Message- > > > From: Rob Herring [mailto:r...@kernel.org] > > > Sent: Wednesday 1 May 2019 20:48 > > > To: Dragan Cvetic > > > Cc: a...@arndb.de; gre...@linuxfoundation.org; Michal Simek > > > ; linux-arm-ker...@lists.infradead.org; > > > mark.rutl...@arm.com; devicet...@vger.kernel.org; > > > linux-kernel@vger.kernel.org; Derek Kiernan > > > 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 > > > > Signed-off-by: Derek Kiernan > > > > --- > > > > .../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 000..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. > > A simple divide by 128. > > We generally prefer DT to use real units rather than register values. The data enters/exits SDFEC in 128 bits word units (__int128). 1,2,4 are not a register values only, they represent a number of units which are used in SDFEC communication. > > Rob
Re: [PATCH V3 01/12] dt-bindings: xilinx-sdfec: Add SDFEC binding
On Thu, May 2, 2019 at 6:04 AM Dragan Cvetic wrote: > > Hi Rob, > > Please find my inline comments below > > Thank you > Dragan > > > -Original Message- > > From: Rob Herring [mailto:r...@kernel.org] > > Sent: Wednesday 1 May 2019 20:48 > > To: Dragan Cvetic > > Cc: a...@arndb.de; gre...@linuxfoundation.org; Michal Simek > > ; linux-arm-ker...@lists.infradead.org; > > mark.rutl...@arm.com; devicet...@vger.kernel.org; > > linux-kernel@vger.kernel.org; Derek Kiernan > > 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 > > > Signed-off-by: Derek Kiernan > > > --- > > > .../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 000..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. A simple divide by 128. We generally prefer DT to use real units rather than register values. Rob
RE: [PATCH V3 01/12] dt-bindings: xilinx-sdfec: Add SDFEC binding
Hi Rob, Please find my inline comments below Thank you Dragan > -Original Message- > From: Rob Herring [mailto:r...@kernel.org] > Sent: Wednesday 1 May 2019 20:48 > To: Dragan Cvetic > Cc: a...@arndb.de; gre...@linuxfoundation.org; Michal Simek > ; linux-arm-ker...@lists.infradead.org; > mark.rutl...@arm.com; devicet...@vger.kernel.org; > linux-kernel@vger.kernel.org; Derek Kiernan > 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 > > Signed-off-by: Derek Kiernan > > --- > > .../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 000..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 mis
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
> Signed-off-by: Derek Kiernan
> ---
> .../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 000..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.
> +- 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.
> +Optional properties:
> +- interrupts: should contain SDFEC interrupt number
The interrupt may not be wired?
> +
> +Example
> +---
> + sd_fec_0: sd-fec@a004 {
> + 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 =
> <_clk_2>,<_clk_0>,<_clk_1>,<_clk_1>,<_clk_1>,
> <_clk_1>;
> + reg = <0x0 0xa004 0x0 0x4>;
> + interrupt-parent = <>;
> + 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
>
